From 5dc91adfb995ee1440a6e1464569f97b9e9a6ec1 Mon Sep 17 00:00:00 2001 From: billy Date: Wed, 15 Apr 2026 00:31:49 -0400 Subject: [PATCH] =?UTF-8?q?fix:=20scene=20robustness=20=E2=80=94=20remount?= =?UTF-8?q?,=20empty=20geometry,=20asset=20URLs,=20first-person=20camera?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A bundle of four defensive fixes to the viewer and editor: 1. markDirty on mount in door/window/slab/ceiling renderers so their systems regenerate real geometry after a remount (preview mode toggle, view mode switch) instead of keeping the 0×0×0 placeholder box forever. Matches the existing WallRenderer pattern. 2. Defensive empty-geometry guard in WallSystem.updateWallGeometry: when generateExtrudedWall returns a BufferGeometry with no position attribute (zero-length wall, pathological miter input), dispose it and hide the mesh instead of assigning it — the WebGPU renderer was crashing reading .count on an undefined position attribute. 3. ItemRenderer: guard useGLTF from empty URLs (resolveCdnUrl returns null for asset:// URLs, which fell through to useGLTF('') and crashed GLTFLoader's JSON parser on the HTML-404 response). Added a dedicated PlaceholderBox with a solid opaque material so items without a loadable model render as plain grey boxes that respect depth, rather than reusing PreviewModel's animated transparent material which rendered through walls. 4. CustomCameraControls: return null when useEditor.isFirstPersonMode is true, so drei's CameraControls doesn't fight FirstPersonControls for the camera. Before this the desktop "walkthrough" button appeared to do nothing because CameraControls was still winning the race. --- .../core/src/systems/wall/wall-system.tsx | 22 +++++++- .../editor/custom-camera-controls.tsx | 12 ++++ .../renderers/ceiling/ceiling-renderer.tsx | 12 +++- .../renderers/door/door-renderer.tsx | 14 ++++- .../renderers/item/item-renderer.tsx | 55 ++++++++++++++++--- .../renderers/slab/slab-renderer.tsx | 11 +++- .../renderers/window/window-renderer.tsx | 13 ++++- 7 files changed, 122 insertions(+), 17 deletions(-) diff --git a/packages/core/src/systems/wall/wall-system.tsx b/packages/core/src/systems/wall/wall-system.tsx index 50c60e4b..a5a2aafe 100644 --- a/packages/core/src/systems/wall/wall-system.tsx +++ b/packages/core/src/systems/wall/wall-system.tsx @@ -121,14 +121,32 @@ function updateWallGeometry(wallId: string, miterData: WallMiterData) { const newGeo = generateExtrudedWall(node, childrenNodes, miterData, slabElevation) + // Defensive: if the wall collapsed to zero length (bad data, or a + // cluster pass that over-merged short walls), `generateExtrudedWall` + // returns an empty BufferGeometry with no position attribute. The + // WebGPU renderer crashes reading `.count` on undefined, so hide the + // mesh instead of assigning the empty geometry. The wall stays in + // the scene graph (so Ctrl+Z can still recover it) but draws + // nothing until the start/end become valid again. + if (!newGeo.attributes.position) { + newGeo.dispose() + mesh.visible = false + return + } + mesh.visible = node.visible ?? true + mesh.geometry.dispose() mesh.geometry = newGeo // Update collision mesh const collisionMesh = mesh.getObjectByName('collision-mesh') as THREE.Mesh if (collisionMesh) { const collisionGeo = generateExtrudedWall(node, [], miterData, slabElevation) - collisionMesh.geometry.dispose() - collisionMesh.geometry = collisionGeo + if (collisionGeo.attributes.position) { + collisionMesh.geometry.dispose() + collisionMesh.geometry = collisionGeo + } else { + collisionGeo.dispose() + } } mesh.position.set(node.start[0], slabElevation, node.start[1]) diff --git a/packages/editor/src/components/editor/custom-camera-controls.tsx b/packages/editor/src/components/editor/custom-camera-controls.tsx index 0bee7583..5850468e 100644 --- a/packages/editor/src/components/editor/custom-camera-controls.tsx +++ b/packages/editor/src/components/editor/custom-camera-controls.tsx @@ -22,6 +22,7 @@ const DEBUG_MAX_POLAR_ANGLE = Math.PI - 0.05 export const CustomCameraControls = () => { const controls = useRef(null!) const isPreviewMode = useEditor((s) => s.isPreviewMode) + const isFirstPersonMode = useEditor((s) => s.isFirstPersonMode) const walkthroughMode = useViewer((s) => s.walkthroughMode) const allowUndergroundCamera = useEditor((s) => s.allowUndergroundCamera) const selection = useViewer((s) => s.selection) @@ -365,6 +366,17 @@ export const CustomCameraControls = () => { useViewer.getState().setCameraDragging(false) }, []) + // The editor's first-person mode is driven by + // (mounted as a sibling in editor/index.tsx via isFirstPersonMode). + // It takes over the camera with pointer lock + WASD, so we must + // return null here — otherwise drei's CameraControls runs in parallel + // and fights FirstPersonControls for the camera, which is exactly why + // the "walkthrough" button on desktop appeared to do nothing (the + // user saw orbit behaviour because CameraControls was still winning). + if (isFirstPersonMode) { + return null + } + if (walkthroughMode) { return } diff --git a/packages/viewer/src/components/renderers/ceiling/ceiling-renderer.tsx b/packages/viewer/src/components/renderers/ceiling/ceiling-renderer.tsx index 7c2f37ef..a290f612 100644 --- a/packages/viewer/src/components/renderers/ceiling/ceiling-renderer.tsx +++ b/packages/viewer/src/components/renderers/ceiling/ceiling-renderer.tsx @@ -1,5 +1,5 @@ -import { type CeilingNode, resolveMaterial, useRegistry } from '@pascal-app/core' -import { useMemo, useRef } from 'react' +import { type CeilingNode, resolveMaterial, useRegistry, useScene } from '@pascal-app/core' +import { useLayoutEffect, useMemo, useRef } from 'react' import { float, mix, positionWorld, smoothstep } from 'three/tsl' import { BackSide, FrontSide, type Mesh, MeshBasicNodeMaterial } from 'three/webgpu' import { useNodeEvents } from '../../../hooks/use-node-events' @@ -38,6 +38,14 @@ export const CeilingRenderer = ({ node }: { node: CeilingNode }) => { useRegistry(node.id, 'ceiling', ref) const handlers = useNodeEvents(node, 'ceiling') + // Mark dirty on mount so CeilingSystem regenerates the polygon + // geometry after a remount. Without this the placeholder + // zero-size box persists and the ceiling disappears. See + // WallRenderer for the same pattern. + useLayoutEffect(() => { + useScene.getState().markDirty(node.id) + }, [node.id]) + const materials = useMemo(() => { const props = resolveMaterial(node.material) const color = props.color || '#999999' diff --git a/packages/viewer/src/components/renderers/door/door-renderer.tsx b/packages/viewer/src/components/renderers/door/door-renderer.tsx index 9d87eba1..1cdfa129 100644 --- a/packages/viewer/src/components/renderers/door/door-renderer.tsx +++ b/packages/viewer/src/components/renderers/door/door-renderer.tsx @@ -1,5 +1,5 @@ -import { type DoorNode, useRegistry } from '@pascal-app/core' -import { useMemo, useRef } from 'react' +import { type DoorNode, useRegistry, useScene } from '@pascal-app/core' +import { useLayoutEffect, useMemo, useRef } from 'react' import type { Mesh } from 'three' import { useNodeEvents } from '../../../hooks/use-node-events' import { createMaterial, DEFAULT_DOOR_MATERIAL } from '../../../lib/materials' @@ -11,6 +11,16 @@ export const DoorRenderer = ({ node }: { node: DoorNode }) => { const handlers = useNodeEvents(node, 'door') const isTransient = !!(node.metadata as Record | null)?.isTransient + // Mark this node dirty on mount so DoorSystem regenerates its + // geometry on the next frame. Without this, the DoorRenderer keeps + // its zero-size placeholder box forever whenever the + // remounts (e.g. entering preview mode, switching view modes), and + // the door visually disappears — DoorSystem only processes nodes in + // the dirtyNodes set. See WallRenderer for the same pattern. + useLayoutEffect(() => { + useScene.getState().markDirty(node.id) + }, [node.id]) + const material = useMemo(() => { const mat = node.material if (!mat) return DEFAULT_DOOR_MATERIAL diff --git a/packages/viewer/src/components/renderers/item/item-renderer.tsx b/packages/viewer/src/components/renderers/item/item-renderer.tsx index 5366d3d5..53157519 100644 --- a/packages/viewer/src/components/renderers/item/item-renderer.tsx +++ b/packages/viewer/src/components/renderers/item/item-renderer.tsx @@ -48,13 +48,34 @@ export const ItemRenderer = ({ node }: { node: ItemNode }) => { useRegistry(node.id, node.type, ref) + // Pick a render path based on whether the item has a loadable model. + // + // - Items with a resolvable `asset.src` → load the GLTF via useGLTF, + // show the animated `PreviewModel` as the Suspense fallback while + // the model downloads. + // + // - Items without a resolvable src (e.g. scanned furniture from + // RoomPlan that ships with a `placeholder` asset and no URL) → + // render `PlaceholderBox` instead. This is a SOLID opaque box, NOT + // the animated preview material. Using PreviewModel as a + // permanent render looks broken: it has `depthTest: false` and an + // animated time-based opacity, so it renders on top of walls and + // pulses, which is what the "flashing / see-through furniture" + // bug report turned out to be. + const src = node.asset.src + const resolvedUrl = src ? resolveCdnUrl(src) : null + return ( - }> - }> - - - + {resolvedUrl ? ( + }> + }> + + + + ) : ( + + )} {node.children?.map((childId) => ( ))} @@ -84,13 +105,33 @@ const PreviewModel = ({ node }: { node: ItemNode }) => { ) } +// Opaque stand-in for items that have no GLTF model to load. Unlike +// `previewMaterial`, this one has normal depth testing and no animated +// transparency, so scanned furniture renders as plain grey boxes that +// sit behind walls correctly instead of pulsing through them. +const placeholderMaterial = new MeshStandardNodeMaterial({ + color: '#a8adb3', + roughness: 0.75, + metalness: 0.05, +}) + +const PlaceholderBox = ({ node }: { node: ItemNode }) => { + const handlers = useNodeEvents(node, 'item') + const [w, h, d] = node.asset.dimensions + return ( + + + + ) +} + const multiplyScales = ( a: [number, number, number], b: [number, number, number], ): [number, number, number] => [a[0] * b[0], a[1] * b[1], a[2] * b[2]] -const ModelRenderer = ({ node }: { node: ItemNode }) => { - const { scene, nodes, animations } = useGLTF(resolveCdnUrl(node.asset.src) || '') +const ModelRenderer = ({ modelUrl, node }: { modelUrl: string; node: ItemNode }) => { + const { scene, nodes, animations } = useGLTF(modelUrl) const ref = useRef(null!) const { actions } = useAnimations(animations, ref) // Freeze the interactive definition at mount — asset schemas don't change at runtime diff --git a/packages/viewer/src/components/renderers/slab/slab-renderer.tsx b/packages/viewer/src/components/renderers/slab/slab-renderer.tsx index a73d9622..fb1d529b 100644 --- a/packages/viewer/src/components/renderers/slab/slab-renderer.tsx +++ b/packages/viewer/src/components/renderers/slab/slab-renderer.tsx @@ -1,5 +1,5 @@ -import { type SlabNode, useRegistry } from '@pascal-app/core' -import { useMemo, useRef } from 'react' +import { type SlabNode, useRegistry, useScene } from '@pascal-app/core' +import { useLayoutEffect, useMemo, useRef } from 'react' import type { Mesh } from 'three' import { useNodeEvents } from '../../../hooks/use-node-events' import { createMaterial, DEFAULT_SLAB_MATERIAL } from '../../../lib/materials' @@ -11,6 +11,13 @@ export const SlabRenderer = ({ node }: { node: SlabNode }) => { const handlers = useNodeEvents(node, 'slab') + // Mark dirty on mount so SlabSystem regenerates the polygon geometry + // after a remount (preview mode, view mode switches). + // Otherwise the zero-size placeholder persists. See WallRenderer. + useLayoutEffect(() => { + useScene.getState().markDirty(node.id) + }, [node.id]) + const material = useMemo(() => { const mat = node.material if (!mat) return DEFAULT_SLAB_MATERIAL diff --git a/packages/viewer/src/components/renderers/window/window-renderer.tsx b/packages/viewer/src/components/renderers/window/window-renderer.tsx index 1f5c2c8e..a96cea5b 100644 --- a/packages/viewer/src/components/renderers/window/window-renderer.tsx +++ b/packages/viewer/src/components/renderers/window/window-renderer.tsx @@ -1,5 +1,5 @@ -import { useRegistry, type WindowNode } from '@pascal-app/core' -import { useMemo, useRef } from 'react' +import { useRegistry, useScene, type WindowNode } from '@pascal-app/core' +import { useLayoutEffect, useMemo, useRef } from 'react' import type { Mesh } from 'three' import { useNodeEvents } from '../../../hooks/use-node-events' import { createMaterial, DEFAULT_WINDOW_MATERIAL } from '../../../lib/materials' @@ -11,6 +11,15 @@ export const WindowRenderer = ({ node }: { node: WindowNode }) => { const handlers = useNodeEvents(node, 'window') const isTransient = !!(node.metadata as Record | null)?.isTransient + // Mark dirty on mount so WindowSystem regenerates the geometry when + // the component remounts (entering preview mode, view mode + // switches, etc.). Without this, the placeholder zero-size box + // persists forever because WindowSystem only walks dirtyNodes. Same + // pattern as WallRenderer. + useLayoutEffect(() => { + useScene.getState().markDirty(node.id) + }, [node.id]) + const material = useMemo(() => { const mat = node.material if (!mat) return DEFAULT_WINDOW_MATERIAL