From 8458e50f4ec87aa5e7e4f722017766f447d71b5e Mon Sep 17 00:00:00 2001 From: Jonathan Wills <65614034+jwills-jdubs@users.noreply.github.com> Date: Thu, 10 Nov 2022 13:57:44 -0800 Subject: [PATCH] fix(composer): Fix e.removeFromParent and camera focus (#350) --- packages/scene-composer/package.json | 4 +- .../__tests__/CameraComponent.spec.tsx | 4 ++ .../CameraComponent.spec.tsx.snap | 31 +++++++++--- .../three-fiber/CameraComponent/index.tsx | 50 +++++++++++++------ .../src/hooks/useEditorHelper.ts | 8 +-- .../src/utils/objectThreeUtils.ts | 39 ++------------- .../tests/utils/objectThreeUtils.spec.ts | 23 --------- 7 files changed, 73 insertions(+), 86 deletions(-) diff --git a/packages/scene-composer/package.json b/packages/scene-composer/package.json index 3aaeb53bb..03796c203 100644 --- a/packages/scene-composer/package.json +++ b/packages/scene-composer/package.json @@ -156,8 +156,8 @@ "global": { "lines": 77.49, "statements": 76.63, - "functions": 77.08, - "branches": 63.36, + "functions": 77.01, + "branches": 63.31, "branchesTrue": 100 } } diff --git a/packages/scene-composer/src/components/three-fiber/CameraComponent/__tests__/CameraComponent.spec.tsx b/packages/scene-composer/src/components/three-fiber/CameraComponent/__tests__/CameraComponent.spec.tsx index 2a1a812b9..4caba1f73 100644 --- a/packages/scene-composer/src/components/three-fiber/CameraComponent/__tests__/CameraComponent.spec.tsx +++ b/packages/scene-composer/src/components/three-fiber/CameraComponent/__tests__/CameraComponent.spec.tsx @@ -38,6 +38,10 @@ jest.mock('../../../../hooks/useActiveCamera', () => { return jest.fn().mockReturnValue({ activeCameraName: 'test-camera', setActiveCameraName: jest.fn() }); }); +jest.mock('../../../../hooks/useEditorHelper', () => { + return { useEditorHelper: jest.fn() }; +}); + describe('CameraComponent', () => { const node = { ref: 'testRef', diff --git a/packages/scene-composer/src/components/three-fiber/CameraComponent/__tests__/__snapshots__/CameraComponent.spec.tsx.snap b/packages/scene-composer/src/components/three-fiber/CameraComponent/__tests__/__snapshots__/CameraComponent.spec.tsx.snap index 3729673e7..ca75ea67d 100644 --- a/packages/scene-composer/src/components/three-fiber/CameraComponent/__tests__/__snapshots__/CameraComponent.spec.tsx.snap +++ b/packages/scene-composer/src/components/three-fiber/CameraComponent/__tests__/__snapshots__/CameraComponent.spec.tsx.snap @@ -13,9 +13,20 @@ exports[`CameraComponent should render correctly for orthographic camera 1`] = ` - +
+ +
`; @@ -25,9 +36,17 @@ exports[`CameraComponent should render correctly for perspective camera 1`] = ` - +
+ +
`; diff --git a/packages/scene-composer/src/components/three-fiber/CameraComponent/index.tsx b/packages/scene-composer/src/components/three-fiber/CameraComponent/index.tsx index 108aeedba..eb6ecdff5 100644 --- a/packages/scene-composer/src/components/three-fiber/CameraComponent/index.tsx +++ b/packages/scene-composer/src/components/three-fiber/CameraComponent/index.tsx @@ -1,6 +1,7 @@ import * as THREE from 'three'; -import React, { useEffect, useMemo } from 'react'; -import { useThree } from '@react-three/fiber'; +import React, { useEffect } from 'react'; +import { OrthographicCamera, PerspectiveCamera } from '@react-three/drei'; +import { Camera, useThree } from '@react-three/fiber'; import useLifecycleLogging from '../../../logger/react-logger/hooks/useLifecycleLogging'; import { ISceneNodeInternal, useEditorState, ICameraComponentInternal } from '../../../store'; @@ -9,6 +10,7 @@ import { getComponentGroupName } from '../../../utils/objectThreeUtils'; import useSelectedNode from '../../../hooks/useSelectedNode'; import useActiveCamera from '../../../hooks/useActiveCamera'; import { getCameraSettings } from '../../../utils/cameraUtils'; +import { useEditorHelper } from '../../../hooks'; interface ICameraComponentProps { node: ISceneNodeInternal; @@ -18,13 +20,18 @@ interface ICameraComponentProps { const CameraComponent: React.FC = ({ node, component }: ICameraComponentProps) => { const sceneComposerId = useSceneComposerId(); useLifecycleLogging('CameraComponent'); - const { isEditing, getObject3DBySceneNodeRef } = useEditorState(sceneComposerId); + const camera = React.useRef(); + const size = useThree((state) => state.size); const { fov, far, near, zoom } = component; + const { isEditing, getObject3DBySceneNodeRef } = useEditorState(sceneComposerId); + const { selectedSceneNodeRef } = useSelectedNode(); const { activeCameraName, setActiveCameraSettings } = useActiveCamera(); + useEditorHelper(isEditing() && selectedSceneNodeRef === node.ref, sceneComposerId, camera, THREE.CameraHelper); + useEffect(() => { if (activeCameraName === node.name) { const object3D = getObject3DBySceneNodeRef(node.ref); @@ -41,21 +48,34 @@ const CameraComponent: React.FC = ({ node, component }: I } }, [activeCameraName]); - const cameraHelper = useMemo(() => { - let camera: THREE.OrthographicCamera | THREE.PerspectiveCamera; - if (component.cameraType === 'Orthographic') { - camera = new THREE.OrthographicCamera(-3, 3, 3, -3, near, far); - camera.zoom = zoom; - } else { - camera = new THREE.PerspectiveCamera(fov, size.width / size.height, near, far); - camera.zoom = zoom; - } - return new THREE.CameraHelper(camera); - }, [component.cameraType]); + let cameraNode: JSX.Element; + if (component.cameraType === 'Orthographic') { + cameraNode = ( + + + + ); + } else { + cameraNode = ( + + + + ); + } return ( - {isEditing() && selectedSceneNodeRef === node.ref && } + {isEditing() && selectedSceneNodeRef === node.ref && cameraNode} ); }; diff --git a/packages/scene-composer/src/hooks/useEditorHelper.ts b/packages/scene-composer/src/hooks/useEditorHelper.ts index b6b52dd67..3fde83ad8 100644 --- a/packages/scene-composer/src/hooks/useEditorHelper.ts +++ b/packages/scene-composer/src/hooks/useEditorHelper.ts @@ -12,7 +12,7 @@ type Helper = THREE.Object3D & { * Create a Helper that respond to the state of the editor. For example, automatically * hide the helper when the editor is in loading state. * - * @param isEditing - true if the SceneComposer is in editing mode, false otherwise + * @param isRendered - true if the SceneComposer is in editing mode, false otherwise * @param sceneComposerId - the Id of the SceneComposer instance * @param object3D - ref to the object that is the target of the Helper * @param proto - the constructor of the Helper class @@ -20,7 +20,7 @@ type Helper = THREE.Object3D & { * @returns */ export function useEditorHelper( - isEditing: boolean, + isRendered: boolean, sceneComposerId: string, object3D: MutableRefObject, proto: T, @@ -31,7 +31,7 @@ export function useEditorHelper( const scene = useThree((state) => state.scene); useEffect(() => { - if (isEditing) { + if (isRendered) { if (proto && object3D.current) { helper.current = new (proto as any)(object3D.current, ...args); if (helper.current) { @@ -45,7 +45,7 @@ export function useEditorHelper( scene.remove(helper.current); } }; - }, [isEditing, scene, proto, object3D, args]); + }, [isRendered, scene, proto, object3D, args]); useStore(sceneComposerId).subscribe((state) => { if (helper.current) { diff --git a/packages/scene-composer/src/utils/objectThreeUtils.ts b/packages/scene-composer/src/utils/objectThreeUtils.ts index 020f3099a..1e65b124b 100644 --- a/packages/scene-composer/src/utils/objectThreeUtils.ts +++ b/packages/scene-composer/src/utils/objectThreeUtils.ts @@ -2,7 +2,6 @@ import * as THREE from 'three'; import { acceleratedRaycast, computeBoundsTree, disposeBoundsTree } from 'three-mesh-bvh'; import { IModelRefComponentInternal } from '../store'; -import useLifecycleLogging from '../logger/react-logger/hooks/useLifecycleLogging'; export function getEntityGroupName(nodeRef: string) { return `ENTITY_GROUP_${nodeRef}`; @@ -76,39 +75,7 @@ export const resetObjectCenter = (obj: THREE.Object3D) => { }; export const getSafeBoundingBox = (obj: THREE.Object3D): THREE.Box3 => { - // Because LineSegments have absurd sizes in ThreeJS we need to account for these in the scene and ignore them. - // Map all Line Segments to their parent for re-parenting - const lineMap: Map = new Map(); - obj.traverse((child) => { - if ((child as THREE.LineSegments).isLineSegments && child.parent) { - lineMap.set(child.parent, child); - } - }); - - // Severe the connection - lineMap.forEach((line) => { - try { - line?.removeFromParent(); - } catch (e) { - const error = e as Error; - // This is used to potentially catch a scenario where the line that was uncovered is removed in execution - console.warn(error.message, error, lineMap); - } - }); - - const safeBoundingBox = new THREE.Box3().setFromObject(obj); - - // Re-connect - lineMap.forEach((line, parent) => { - try { - if (line) { - parent?.add(line); - } - } catch (e) { - const error = e as Error; - // This is used to potentially catch a scenario where the line that was uncovered is removed in execution - console.warn(error.message, error, lineMap); - } - }); - return safeBoundingBox; + // This wrapper is being maintained should we need to alter anything about bounding box calculation. + // Since there was a moment in which we thought we needed this it gives the flexibility of making a change in one place later + return new THREE.Box3().setFromObject(obj); }; diff --git a/packages/scene-composer/tests/utils/objectThreeUtils.spec.ts b/packages/scene-composer/tests/utils/objectThreeUtils.spec.ts index 83a874036..238d89245 100644 --- a/packages/scene-composer/tests/utils/objectThreeUtils.spec.ts +++ b/packages/scene-composer/tests/utils/objectThreeUtils.spec.ts @@ -109,27 +109,4 @@ describe('objectThreeUtils', () => { expect(object.receiveShadow).toBeTruthy(); expect(object.material.map?.anisotropy).toEqual(16); }); - - it('should get the safe bounding box ignoring lineSegments', () => { - const cube = new THREE.Mesh(); - const boxGeometry = new THREE.BoxGeometry(2, 2, 2); - const material = new THREE.MeshBasicMaterial(); - cube.geometry = boxGeometry; - cube.material = material; - - const expectedBoundingBox = new THREE.Box3().setFromObject(cube); - const originalBoundingBox = getSafeBoundingBox(cube); - expect(originalBoundingBox).toEqual(expectedBoundingBox); - - const line = new THREE.LineSegments(); - const lineGeometry = new THREE.BufferGeometry(); - lineGeometry.setFromPoints([new THREE.Vector3(0, 0, 0), new THREE.Vector3(1000, 1000, 1000)]); - line.geometry = lineGeometry; - line.material = material; - - cube.add(line); - - const safeBoundingBox = getSafeBoundingBox(cube); - expect(safeBoundingBox).toEqual(originalBoundingBox); - }); });