From 85151ffe8f1f114659d8e4e3d4b36bf27b20123b Mon Sep 17 00:00:00 2001 From: Pierre Jacquier Date: Tue, 30 May 2023 06:52:53 -0400 Subject: [PATCH] Model centering can be off on Unified mode (#193) Fixes #182 --- src/components/diff/BaseModel.tsx | 17 +++++--------- src/components/diff/CadDiff.tsx | 1 + src/components/diff/UnifiedModel.tsx | 31 ++++++++++++++++++++++---- src/components/diff/WireframeModel.tsx | 2 +- 4 files changed, 35 insertions(+), 16 deletions(-) diff --git a/src/components/diff/BaseModel.tsx b/src/components/diff/BaseModel.tsx index fd64991..d8206df 100644 --- a/src/components/diff/BaseModel.tsx +++ b/src/components/diff/BaseModel.tsx @@ -1,17 +1,17 @@ import { useThree } from '@react-three/fiber' import type { MutableRefObject, PropsWithChildren } from 'react' import { Suspense, useEffect, useRef } from 'react' -import { BufferGeometry } from 'three' +import { Sphere } from 'three' import { Vector3 } from 'three' import { calculateFovFactor } from './Camera' type BaseModelProps = { cameraRef: MutableRefObject - geometry: BufferGeometry + boundingSphere: Sphere | null | undefined } export function BaseModel({ - geometry, + boundingSphere, cameraRef, children, }: PropsWithChildren) { @@ -21,14 +21,9 @@ export function BaseModel({ // Camera view, adapted from KittyCAD/website useEffect(() => { - if (geometry && cameraRef.current) { - geometry.computeBoundingSphere() - // TODO: understand the implications of this, - // it's been disabled as it was causing before and after to be misaligned - // geometry.center() - + if (boundingSphere && cameraRef.current) { // move the camera away so the object fits in the view - const { radius } = geometry.boundingSphere || { radius: 1 } + const { radius } = boundingSphere || { radius: 1 } if (!camera.position.length()) { const arbitraryNonZeroStartPosition = new Vector3(0.5, 0.5, 1) camera.position.copy(arbitraryNonZeroStartPosition) @@ -42,7 +37,7 @@ export function BaseModel({ camera.zoom = fovFactor / camera.position.length() camera.updateProjectionMatrix() } - }, [geometry, camera, cameraRef, canvasHeight]) + }, [boundingSphere, camera, cameraRef, canvasHeight]) return ( diff --git a/src/components/diff/CadDiff.tsx b/src/components/diff/CadDiff.tsx index a050bed..716ca31 100644 --- a/src/components/diff/CadDiff.tsx +++ b/src/components/diff/CadDiff.tsx @@ -26,6 +26,7 @@ function loadGeometry(file: string, checkUV = false): BufferGeometry { new BufferAttribute(new Float32Array([]), 1) ) } + geometry.computeBoundingSphere() // will be used for auto-centering return geometry } diff --git a/src/components/diff/UnifiedModel.tsx b/src/components/diff/UnifiedModel.tsx index 4f81e19..5ea2ebf 100644 --- a/src/components/diff/UnifiedModel.tsx +++ b/src/components/diff/UnifiedModel.tsx @@ -1,6 +1,14 @@ import type { MutableRefObject } from 'react' import { useTheme } from '@primer/react' -import { BufferGeometry } from 'three' +import { + Box3, + BufferGeometry, + Group, + Mesh, + MeshBasicMaterial, + Sphere, + Vector3, +} from 'three' import { Geometry, Base, Subtraction, Intersection } from '@react-three/csg' import { BaseModel } from './BaseModel' @@ -13,6 +21,20 @@ type UnifiedModelProps = { showDeletions: boolean } +function getCommonSphere( + beforeGeometry: BufferGeometry, + afterGeometry: BufferGeometry +) { + const group = new Group() + const dummyMaterial = new MeshBasicMaterial() + group.add(new Mesh(beforeGeometry, dummyMaterial)) + group.add(new Mesh(afterGeometry, dummyMaterial)) + const boundingBox = new Box3().setFromObject(group) + const center = new Vector3() + boundingBox.getCenter(center) + return boundingBox.getBoundingSphere(new Sphere(center)) +} + export function UnifiedModel({ beforeGeometry, afterGeometry, @@ -27,9 +49,10 @@ export function UnifiedModel({ const deletionsColor = theme?.colors.danger.muted return ( - // TODO: here we give beforeGeometry for auto camera centering, - // for the lack of something better. Need to check the implications - + {/* Unchanged */} +