fix(composer): Fix e.removeFromParent and camera focus (#350)

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
       }
     }

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',

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`] = `
   <group
     name="CAMERA_COMPONENT_testRef"
   >
-    <primitive
-      object="[object Object]"
-    />
+    <div
+      bottom="-3"
+      data-mocked="OrthographicCamera"
+      far="100"
+      left="-3"
+      near="0"
+      right="3"
+      top="3"
+      zoom="1"
+    >
+      <meshbasicmaterial
+        attach="material"
+      />
+    </div>
   </group>
 </div>
 `;
@@ -25,9 +36,17 @@ exports[`CameraComponent should render correctly for perspective camera 1`] = `
   <group
     name="CAMERA_COMPONENT_testRef"
   >
-    <primitive
-      object="[object Object]"
-    />
+    <div
+      aspect="1.7777777777777777"
+      data-mocked="PerspectiveCamera"
+      far="100"
+      fov="60"
+      near="0"
+    >
+      <meshbasicmaterial
+        attach="material"
+      />
+    </div>
   </group>
 </div>
 `;

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<ICameraComponentProps> = ({ node, component }: ICameraComponentProps) => {
   const sceneComposerId = useSceneComposerId();
   useLifecycleLogging('CameraComponent');
-  const { isEditing, getObject3DBySceneNodeRef } = useEditorState(sceneComposerId);
+  const camera = React.useRef<Camera>();
+
   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<ICameraComponentProps> = ({ 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 = (
+      <OrthographicCamera
+        // TODO: Make Editable once we expose this camera type
+        top={3}
+        bottom={-3}
+        left={-3}
+        right={3}
+        far={far}
+        zoom={1}
+        near={near}
+        ref={camera}
+      >
+        <meshBasicMaterial attach='material' />
+      </OrthographicCamera>
+    );
+  } else {
+    cameraNode = (
+      <PerspectiveCamera fov={fov} far={far} near={near} aspect={size.width / size.height} ref={camera}>
+        <meshBasicMaterial attach='material' />
+      </PerspectiveCamera>
+    );
+  }
 
   return (
     <group name={getComponentGroupName(node.ref, 'CAMERA')}>
-      {isEditing() && selectedSceneNodeRef === node.ref && <primitive object={cameraHelper} />}
+      {isEditing() && selectedSceneNodeRef === node.ref && cameraNode}
     </group>
   );
 };

packages/scene-composer/src/hooks/useEditorHelper.ts

@@ -12,15 +12,15 @@ 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
  * @param args - args to be passed to the constructor
  * @returns
  */
 export function useEditorHelper<T>(
-  isEditing: boolean,
+  isRendered: boolean,
   sceneComposerId: string,
   object3D: MutableRefObject<THREE.Object3D | undefined>,
   proto: T,
@@ -31,7 +31,7 @@ export function useEditorHelper<T>(
   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<T>(
         scene.remove(helper.current);
       }
     };
-  }, [isEditing, scene, proto, object3D, args]);
+  }, [isRendered, scene, proto, object3D, args]);
 
   useStore(sceneComposerId).subscribe((state) => {
     if (helper.current) {

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<THREE.Object3D, THREE.Object3D> = new Map<THREE.Object3D, THREE.Object3D>();
-  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);
 };

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);
-  });
 });