From 1ec65acc5feba910f8bc83f1a6429705d9525177 Mon Sep 17 00:00:00 2001 From: Wassim SAMAD Date: Tue, 19 May 2026 17:32:11 -0400 Subject: [PATCH] =?UTF-8?q?registry:=20post-migration=20polish=20=E2=80=94?= =?UTF-8?q?=20ceiling=203D=20selection,=20ceiling=20item=20commit,=20floor?= =?UTF-8?q?plan=20move=20drift?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three bugs surfaced after the Stage E node-registry migration: 1. Ceiling intercepts 3D hover/click selection Selecting via the floor-plan helper or the boundary-editor handles is the intended flow; a direct 3D click on the ceiling should fall through to whatever's underneath. `SelectionManager` now early-returns on `ceiling` in onEnter/onLeave/onClick, so `event.stopPropagation` is skipped and the ray reaches the item/wall/floor below. 2. Ceiling item placement: final click does nothing When a ceiling-attached draft hangs in front of the ceiling-grid mesh, the click ray hits the draft first and fires `item:click`, not `ceiling:click`. `onItemClick` already forwards self-clicks to shelf-surface / item-surface hosts; this PR adds the matching ceiling branch so the commit lands on the ceiling under the cursor. 3. Floor-plan item move drift after the commit click Two contributing causes, both fixed: - `usePlacementCoordinator`'s `useFrame` lerped the draft mesh toward `gridPosition.current` (the item's pre-move spot) every frame, fighting React's render from `scene.position` while the 2D `FloorplanRegistryMoveOverlay` drove the move. Gated the lerp on a `has3DPointerDrivenMoveRef` flag set on first 3D pointer event — pure 3D drags are unchanged. - The overlay's pointer-up handler skipped a final `session.apply` and committed at the last pointermove position. Browsers don't guarantee a pointermove right before pointerup, so a quick click after a drag could land a few pixels off. Re-apply at pointer-up coords so commit matches where the user actually released. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../floorplan-registry-move-overlay.tsx | 21 +++++++ .../tools/item/use-placement-coordinator.tsx | 58 +++++++++++++++++++ .../components/viewer/selection-manager.tsx | 7 +++ 3 files changed, 86 insertions(+) diff --git a/packages/editor/src/components/editor-2d/floorplan-registry-move-overlay.tsx b/packages/editor/src/components/editor-2d/floorplan-registry-move-overlay.tsx index 71aefda8..2eb5f496 100644 --- a/packages/editor/src/components/editor-2d/floorplan-registry-move-overlay.tsx +++ b/packages/editor/src/components/editor-2d/floorplan-registry-move-overlay.tsx @@ -209,6 +209,27 @@ export function FloorplanRegistryMoveOverlay() { // inside the SVG viewport, including empty grid background. if (!isPointerOverFloorplanScene(event.clientX, event.clientY)) return + // Apply once more at the pointer-up coords before committing. + // Browsers don't guarantee a pointermove fires right before + // pointerup — a quick click after a drag can land pointerup a + // few pixels past the last pointermove. Without this re-apply, + // the commit would freeze the item at the stale pointermove + // position, leaving a visible drift between where the user + // released the click and where the item lands. + const finalPlanPoint = toMeters(event.clientX, event.clientY) + if (finalPlanPoint) { + hasMovedSinceStart = true + session.apply({ + planPoint: finalPlanPoint, + modifiers: { + shiftKey: event.shiftKey, + altKey: event.altKey, + ctrlKey: event.ctrlKey, + metaKey: event.metaKey, + }, + }) + } + commitFinalStateOrRevert() setMovingNode(null) diff --git a/packages/editor/src/components/tools/item/use-placement-coordinator.tsx b/packages/editor/src/components/tools/item/use-placement-coordinator.tsx index 65300bbf..38c845bf 100644 --- a/packages/editor/src/components/tools/item/use-placement-coordinator.tsx +++ b/packages/editor/src/components/tools/item/use-placement-coordinator.tsx @@ -298,6 +298,14 @@ export function usePlacementCoordinator(config: PlacementCoordinatorConfig): Rea ) const shiftFreeRef = useRef(false) const previewBoundsSignatureRef = useRef(null) + // Goes true the first time a 3D pointer event drives this coordinator. + // The per-frame mesh-position lerp below is only useful for that path; + // when the move is being driven externally (2D `FloorplanRegistryMoveOverlay` + // writing scene.position directly), the lerp fights React's render and + // pulls the rendered item back toward its pre-move position. Gating + // the lerp on this flag keeps 3D placement smooth without hijacking + // 2D drags that share the same draft. + const has3DPointerDrivenMoveRef = useRef(false) const [dimensionBounds, setDimensionBounds] = useState(null) // Store config callbacks in refs to avoid re-running effect when they change @@ -561,6 +569,7 @@ export function usePlacementCoordinator(config: PlacementCoordinatorConfig): Rea configRef.current.initDraft(gridPosition.current) } + has3DPointerDrivenMoveRef.current = true lastRawPos.current.set(event.localPosition[0], event.localPosition[1], event.localPosition[2]) const result = floorStrategy.move(getContext(), event) if (!result) return @@ -618,6 +627,7 @@ export function usePlacementCoordinator(config: PlacementCoordinatorConfig): Rea // ---- Wall Handlers ---- const onWallEnter = (event: WallEvent) => { + has3DPointerDrivenMoveRef.current = true const nodes = useScene.getState().nodes const result = wallStrategy.enter( getContext(), @@ -643,6 +653,7 @@ export function usePlacementCoordinator(config: PlacementCoordinatorConfig): Rea } const onWallMove = (event: WallEvent) => { + has3DPointerDrivenMoveRef.current = true const ctx = getContext() if (ctx.state.surface !== 'wall') { @@ -833,6 +844,7 @@ export function usePlacementCoordinator(config: PlacementCoordinatorConfig): Rea const onItemEnter = (event: ItemEvent) => { if (event.node.id === draftNode.current?.id) return + has3DPointerDrivenMoveRef.current = true const result = itemSurfaceStrategy.enter(getContext(), event) if (!result) return @@ -849,6 +861,7 @@ export function usePlacementCoordinator(config: PlacementCoordinatorConfig): Rea const onItemMove = (event: ItemEvent) => { if (event.node.id === draftNode.current?.id) return + has3DPointerDrivenMoveRef.current = true const ctx = getContext() if (ctx.state.surface !== 'item-surface') { @@ -990,6 +1003,41 @@ export function usePlacementCoordinator(config: PlacementCoordinatorConfig): Rea } } } + // Ceiling-hosted draft: when placing a ceiling-attached item the + // draft hangs below the ceiling and intercepts the click ray + // before the ceiling-grid mesh does — so `ceiling:click` never + // fires and the user's commit click is dropped. Forward the + // self-click to `ceilingStrategy.click` so placement commits the + // same way it would from a click on the ceiling itself. + if (ctx.state.surface === 'ceiling' && ctx.state.ceilingId) { + const ceilingNode = useScene.getState().nodes[ctx.state.ceilingId as AnyNodeId] + if (ceilingNode && ceilingNode.type === 'ceiling') { + const synthetic = { ...event, node: ceilingNode } as unknown as CeilingEvent + const result = ceilingStrategy.click(ctx, synthetic, getActiveValidators()) + if (result) { + event.stopPropagation() + if (draftNode.current) { + useLiveTransforms.getState().clear(draftNode.current.id) + } + draftNode.commit(result.nodeUpdate) + if (configRef.current.onCommitted()) { + const nodes = useScene.getState().nodes + const enterResult = ceilingStrategy.enter( + getContext(), + synthetic, + resolveLevelId, + nodes, + ) + if (enterResult) { + applyTransition(enterResult) + } else { + revalidate() + } + } + return + } + } + } return } @@ -1017,6 +1065,7 @@ export function usePlacementCoordinator(config: PlacementCoordinatorConfig): Rea // ---- Ceiling Handlers ---- const onCeilingEnter = (event: CeilingEvent) => { + has3DPointerDrivenMoveRef.current = true const nodes = useScene.getState().nodes const result = ceilingStrategy.enter(getContext(), event, resolveLevelId, nodes) if (!result) return @@ -1036,6 +1085,7 @@ export function usePlacementCoordinator(config: PlacementCoordinatorConfig): Rea } const onCeilingMove = (event: CeilingEvent) => { + has3DPointerDrivenMoveRef.current = true if (!draftNode.current && placementState.current.surface === 'ceiling') { const nodes = useScene.getState().nodes const setup = ceilingStrategy.enter(getContext(), event, resolveLevelId, nodes) @@ -1142,6 +1192,7 @@ export function usePlacementCoordinator(config: PlacementCoordinatorConfig): Rea // to the cursor's local-Y so the user can target a specific row. const onShelfEnter = (event: ShelfEvent) => { + has3DPointerDrivenMoveRef.current = true const result = shelfSurfaceStrategy.enter(getContext(), event) if (!result) return @@ -1156,6 +1207,7 @@ export function usePlacementCoordinator(config: PlacementCoordinatorConfig): Rea } const onShelfMove = (event: ShelfEvent) => { + has3DPointerDrivenMoveRef.current = true const ctx = getContext() if (ctx.state.surface !== 'shelf-surface') { // Cursor entered via a move event without an enter — try @@ -1466,6 +1518,12 @@ export function usePlacementCoordinator(config: PlacementCoordinatorConfig): Rea useFrame((_, delta) => { if (!asset) return if (!draftNode.current) return + // The mesh-position lerp below only makes sense once this coordinator + // owns the move via a 3D pointer event. Skip until then so that + // external drivers (e.g. the 2D `FloorplanRegistryMoveOverlay` + // writing scene.position directly) aren't fought by useFrame pulling + // the mesh back to its pre-move location. + if (!has3DPointerDrivenMoveRef.current) return const mesh = sceneRegistry.nodes.get(draftNode.current.id) if (!mesh) return diff --git a/packages/viewer/src/components/viewer/selection-manager.tsx b/packages/viewer/src/components/viewer/selection-manager.tsx index 30de805f..874d6c2c 100644 --- a/packages/viewer/src/components/viewer/selection-manager.tsx +++ b/packages/viewer/src/components/viewer/selection-manager.tsx @@ -304,6 +304,11 @@ export const SelectionManager = () => { const onEnter = (event: NodeEvent) => { const strategy = getStrategy() if (!strategy) return + // Ceilings are selected via their floor-plan helper and the + // boundary-editor vertex handles, never via a direct 3D click on + // the polygon. Skipping selection routing here means a click on a + // ceiling falls through to the item / wall / floor below it. + if (event.node.type === 'ceiling') return if (strategy.isValid(event.node)) { event.stopPropagation() if (event.node.type === 'slab') { @@ -317,6 +322,7 @@ export const SelectionManager = () => { const onLeave = (event: NodeEvent) => { const strategy = getStrategy() if (!strategy) return + if (event.node.type === 'ceiling') return if (strategy.isValid(event.node)) { event.stopPropagation() useViewer.setState({ hoveredId: null }) @@ -326,6 +332,7 @@ export const SelectionManager = () => { const onClick = (event: NodeEvent) => { const strategy = getStrategy() if (!strategy) return + if (event.node.type === 'ceiling') return if (!strategy.isValid(event.node)) return event.stopPropagation()