registry/move-tool: fix 3D drag — keep rotation + stop transform reset

Two bugs caused dragged shelves to snap to a weird position on commit:

1. `MoveRegistryNodeTool` wrote `useLiveTransforms.set(id, { ..., rotation: 0 })`,
   so during the drag `ParametricNodeRenderer` applied `<group rotation={[0,0,0]}>`
   and the shelf visually un-rotated. On commit the live transform cleared
   and the renderer re-read the node's true rotation — the snap-back read
   as "reverts to a weird position." Now we capture `originalRotationY`
   from the node at mount time and forward it on every set.

2. `<GeometrySystem>` reset `group.position.set(0,0,0)` /
   `group.rotation.set(0,0,0)` after every rebuild. That was carry-over
   from legacy per-kind systems that didn't bind `position` on the group.
   `ParametricNodeRenderer` now drives the transform via JSX prop, and
   the reset clobbered it — React doesn't necessarily re-render on a
   rebuild tick, so R3F never re-applied the prop and the registered
   `<group>` stayed at the origin. Removed the reset; builders are
   expected to emit local-space children.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

packages/editor/src/components/tools/registry/move-registry-node-tool.tsx

@@ -5,10 +5,13 @@ import '../../../three-types'
 import {
   type AnyNode,
   type AnyNodeId,
+  type EventSuffix,
   emitter,
   type GridEvent,
+  type NodeEvent,
   nodeRegistry,
   sceneRegistry,
+  useLiveTransforms,
   useScene,
 } from '@pascal-app/core'
 import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
@@ -34,14 +37,36 @@ const roundToHalf = (value: number) => Math.round(value * 2) / 2
  * Cancel imperatively snaps the mesh back to its original position and
  * resumes history without ever having touched the store mid-drag.
  *
- * This is faster than the items pattern (which updates the store per tick
+ * **Commit triggers**: the tool listens for `grid:click` *and* the
- * and re-renders the renderer on every mouse move). Trade-off: if the
+ * common node click events (shelf / item / slab / ceiling / wall /
- * renderer happens to re-render for some other reason mid-drag, R3F will
+ * fence / column / roof / stair). A click on the grid plane fires
- * reapply node.position and snap the mesh back. Mitigation: history is
+ * `grid:click`; a click on the moved node itself (or any other 3D
- * paused, no upstream state subscribed by the renderer changes during the
+ * geometry the ray happens to land on) fires the corresponding node
- * drag, so re-renders are rare in practice. If a kind needs guaranteed
+ * click event. Without the node-click listeners, clicking on the
- * stability, it can opt into a "live position" hook in Phase 4.
+ * cursor's own mesh during a move would silently drop the commit —
+ * the user perceives "click did nothing" because the click hit the
+ * vertical face of e.g. a shelf instead of the grid plane below it.
+ *
+ * The latest cursor position from `grid:move` is stored in a ref so
+ * any of these click variants commit at the same spot the cursor was
+ * indicating.
  */
+type ClickTriggerEvent = GridEvent | NodeEvent<AnyNode>
+
+const CLICK_TRIGGER_KINDS = [
+  'shelf',
+  'item',
+  'slab',
+  'ceiling',
+  'wall',
+  'fence',
+  'column',
+  'roof',
+  'roof-segment',
+  'stair',
+  'stair-segment',
+] as const
+
 export function MoveRegistryNodeTool({ node }: { node: AnyNode }) {
   const originalPosition: [number, number, number] = useMemo(
     () =>
@@ -50,33 +75,88 @@ export function MoveRegistryNodeTool({ node }: { node: AnyNode }) {
         : [0, 0, 0],
     [node],
   )
+  /**
+   * Y-axis rotation of the node at move-start. Captured so the
+   * imperative drag preview (and the `useLiveTransforms` mirror) keeps
+   * the original orientation — otherwise hardcoding `rotation: 0` in
+   * `useLiveTransforms.set` would override `node.rotation[1]` during
+   * the drag, the shelf would visually un-rotate to 0, then snap back
+   * to its true rotation on commit (when the live transform clears).
+   * The user reads that snap as "reverts to a weird position".
+   */
+  const originalRotationY: number = useMemo(() => {
+    if ('rotation' in node) {
+      const r = (node as { rotation?: unknown }).rotation
+      if (typeof r === 'number') return r
+      if (Array.isArray(r)) return (r as [number, number, number])[1] ?? 0
+    }
+    return 0
+  }, [node])
   const [cursorPosition, setCursorPosition] = useState<[number, number, number]>(originalPosition)
   const previousSnapRef = useRef<[number, number] | null>(null)
+  /**
+   * The latest snapped cursor position from `grid:move`. We commit at
+   * THIS position regardless of which event variant fires the click —
+   * a `grid:click` carries the same coords, but a node-click (e.g.
+   * `shelf:click`) carries the hit point on the clicked node's mesh,
+   * which can be slightly off-cursor when the user clicks the vertical
+   * face of the moved node itself. Reading from the ref keeps the
+   * commit position consistent with the visible cursor.
+   */
+  const lastCursorRef = useRef<[number, number, number]>(originalPosition)
 
   const exitMoveMode = useCallback(() => {
     useEditor.getState().setMovingNode(null)
   }, [])
 
   useEffect(() => {
-    // Pause history so the eventual commit lands as a single undoable step,
-    // not the per-tick spam that would happen if we updated the store on
-    // each move.
     useScene.temporal.getState().pause()
     previousSnapRef.current = null
     let committed = false
 
+    // Disable raycast on the moved node's meshes for the duration of
+    // the drag. As the shelf follows the cursor, the cursor ray would
+    // otherwise hit the moved mesh first → only `${kind}:move` fires →
+    // `grid:move` stops updating `lastCursorRef` → clicks would commit
+    // at the stale (initial) position. With raycast disabled, the ray
+    // passes through the moved mesh and continues to the grid plane,
+    // so `grid:move` keeps firing and the cursor tracks correctly.
+    // We restore the original raycast on cleanup.
+    const mesh = sceneRegistry.nodes.get(node.id)
+    const restoreRaycasts: Array<() => void> = []
+    if (mesh) {
+      mesh.traverse((child) => {
+        const original = child.raycast
+        child.raycast = () => {}
+        restoreRaycasts.push(() => {
+          child.raycast = original
+        })
+      })
+    }
+
     const onGridMove = (event: GridEvent) => {
       const x = roundToHalf(event.localPosition[0])
       const z = roundToHalf(event.localPosition[2])
       setCursorPosition([x, 0, z])
+      lastCursorRef.current = [x, 0, z]
 
       // Pure imperative: move the mesh via its registered Object3D ref.
-      // No React re-render. No store update. The shelf (or any registry
-      // kind) follows the cursor smoothly because nothing competes with
-      // this position write until commit.
       sceneRegistry.nodes.get(node.id)?.position.set(x, 0, z)
+      // Publish to `useLiveTransforms` so the 2D floor plan can mirror
+      // the drag in real-time (the floor-plan layer subscribes to this
+      // store and overrides the node's rendered position when an entry
+      // is set). Without this the 2D representation stays at the
+      // committed scene position until the move ends.
+      //
+      // For position-based kinds (shelf, item, column, spawn) we write
+      // the absolute world plan position here. Polygon-based kinds
+      // (slab / ceiling / fence) follow a different delta contract —
+      // their floor-plan move-targets handle the override themselves.
+      useLiveTransforms.getState().set(node.id, {
+        position: [x, 0, z],
+        rotation: originalRotationY,
+      })
 
-      // SFX on cell-cross, matching placement.
       const prev = previousSnapRef.current
       if (!prev || prev[0] !== x || prev[1] !== z) {
         sfxEmitter.emit('sfx:grid-snap')
@@ -84,17 +164,25 @@ export function MoveRegistryNodeTool({ node }: { node: AnyNode }) {
       }
     }
 
-    const onGridClick = (event: GridEvent) => {
+    /** Commit the move at the latest cursor position. Shared by every
-      const position: [number, number, number] = [
+     *  click variant — grid plane, the moved node itself, or any other
-        roundToHalf(event.localPosition[0]),
+     *  3D surface the user happens to click on during the move.
-        0,
+     *
-        roundToHalf(event.localPosition[2]),
+     *  Order is deliberate: write scene FIRST, then clear
-      ]
+     *  `useLiveTransforms`. If we cleared the live transform first,
+     *  `ParametricNodeRenderer` would re-render with
+     *  `position = liveTransform?.position ?? node.position` → undefined
+     *  → original `node.position` (the scene write hasn't happened yet),
+     *  briefly snapping the mesh back to its starting spot before the
+     *  next render lands the new position. Writing scene first means
+     *  every render shows either the live drag position (liveTransform
+     *  still set) or the new committed position (liveTransform cleared
+     *  AND scene updated) — never the original.
+     */
+    const commitAtCursor = (event: ClickTriggerEvent) => {
+      const position: [number, number, number] = [...lastCursorRef.current]
 
       if (useScene.getState().nodes[node.id]) {
-        // Store still has the original position (we didn't touch it during
-        // drag). Resume history and do one tracked update. Undo replays the
-        // (original → final) single step.
         useScene.temporal.getState().resume()
         useScene.getState().updateNode(node.id, { position } as Partial<AnyNode>)
         useScene.temporal.getState().pause()
@@ -116,40 +204,76 @@ export function MoveRegistryNodeTool({ node }: { node: AnyNode }) {
         }
       }
 
+      // Keep mesh.position aligned with the just-committed scene position
+      // so the next R3F frame paints at the right spot even if React's
+      // reconciliation lags by a tick.
+      const mesh = sceneRegistry.nodes.get(node.id)
+      if (mesh) mesh.position.set(position[0], position[1], position[2])
+
+      // Now safe to clear — node.position is already the new value, so
+      // `ParametricNodeRenderer`'s next render lands at `[x, 0, z]`.
+      useLiveTransforms.getState().clear(node.id)
+
       sfxEmitter.emit('sfx:item-place')
       exitMoveMode()
-      event.nativeEvent?.stopPropagation?.()
+
+      // Stop further propagation so other listeners (e.g. a selection
+      // change on the clicked node) don't fire during the commit click.
+      const native = (event as { nativeEvent?: unknown }).nativeEvent
+      if (
+        native &&
+        typeof (native as { stopPropagation?: () => void }).stopPropagation === 'function'
+      ) {
+        ;(native as { stopPropagation: () => void }).stopPropagation()
+      }
+      const direct = (event as { stopPropagation?: () => void }).stopPropagation
+      if (typeof direct === 'function') direct.call(event)
+    }
+
+    emitter.on('grid:move', onGridMove)
+    emitter.on('grid:click', commitAtCursor)
+
+    // Listen on every common kind's click event too. mitt's typing keeps
+    // `${kind}:click` as a fixed union so the cast is safe at runtime —
+    // we're just routing them through the shared commit path.
+    type SuffixedKey<K extends string> = `${K}:${EventSuffix}`
+    type ClickKey = SuffixedKey<(typeof CLICK_TRIGGER_KINDS)[number]>
+    for (const kind of CLICK_TRIGGER_KINDS) {
+      const key = `${kind}:click` as ClickKey
+      emitter.on(key, commitAtCursor as never)
     }
 
     const onCancel = () => {
-      // Snap mesh back to original visually. Store was never touched.
       sceneRegistry.nodes
         .get(node.id)
         ?.position.set(originalPosition[0], originalPosition[1], originalPosition[2])
+      useLiveTransforms.getState().clear(node.id)
       useScene.temporal.getState().resume()
       markToolCancelConsumed()
       exitMoveMode()
     }
-
-    emitter.on('grid:move', onGridMove)
-    emitter.on('grid:click', onGridClick)
     emitter.on('tool:cancel', onCancel)
 
     return () => {
       emitter.off('grid:move', onGridMove)
-      emitter.off('grid:click', onGridClick)
+      emitter.off('grid:click', commitAtCursor)
+      for (const kind of CLICK_TRIGGER_KINDS) {
+        const key = `${kind}:click` as ClickKey
+        emitter.off(key, commitAtCursor as never)
+      }
       emitter.off('tool:cancel', onCancel)
-      // If we unmount without committing (e.g., the user switches tools or
+      // Restore the moved meshes' raycast so they're hoverable / selectable
-      // navigates away), restore the mesh imperatively and resume history.
+      // again after the drag ends.
-      // Store was never touched so no data revert is needed.
+      for (const restore of restoreRaycasts) restore()
       if (!committed) {
         sceneRegistry.nodes
           .get(node.id)
           ?.position.set(originalPosition[0], originalPosition[1], originalPosition[2])
+        useLiveTransforms.getState().clear(node.id)
         useScene.temporal.getState().resume()
       }
     }
-  }, [exitMoveMode, node, originalPosition])
+  }, [exitMoveMode, node, originalPosition, originalRotationY])
 
   return <CursorSphere color="#a78bfa" height={2.5} position={cursorPosition} />
 }

packages/viewer/src/systems/geometry/geometry-system.tsx

@@ -50,18 +50,75 @@ export const GeometrySystem = () => {
     if (dirtyNodes.size === 0) return
     const nodes = useScene.getState().nodes
 
+    // Phase 1 — group dirty nodes by (kind, parentId). Kinds that
+    // declare `def.computeLevelData` get one batch precompute per
+    // group; the result lands in `ctx.levelData` for every node in
+    // the same batch. Avoids O(N²) recomputation when many siblings
+    // of the same kind are dirty in one frame (wall mitering is the
+    // motivating case).
+    type BatchKey = string // `${kind}::${parentId ?? ''}`
+    const batches = new Map<
+      BatchKey,
+      { kind: string; parentId: AnyNodeId | null; ids: AnyNodeId[] }
+    >()
+    const dirtyIds: AnyNodeId[] = []
     dirtyNodes.forEach((id) => {
       const node = nodes[id]
       if (!node) return
+      const def = nodeRegistry.get(node.type)
+      if (!def?.geometry) return
+      dirtyIds.push(id as AnyNodeId)
+      const parentId = (node.parentId ?? null) as AnyNodeId | null
+      const key: BatchKey = `${node.type}::${parentId ?? ''}`
+      const existing = batches.get(key)
+      if (existing) existing.ids.push(id as AnyNodeId)
+      else batches.set(key, { kind: node.type, parentId, ids: [id as AnyNodeId] })
+    })
+
+    // Phase 2 — for each batch whose kind declares `computeLevelData`,
+    // collect every sibling in the level + run the precompute once.
+    const levelDataByBatch = new Map<BatchKey, unknown>()
+    for (const [key, batch] of batches) {
+      const def = nodeRegistry.get(batch.kind)
+      if (!def?.computeLevelData) continue
+      const siblings: AnyNode[] = []
+      if (batch.parentId) {
+        const parent = nodes[batch.parentId]
+        const childIds = (parent as unknown as { children?: AnyNodeId[] })?.children
+        if (Array.isArray(childIds)) {
+          for (const cid of childIds) {
+            const child = nodes[cid]
+            if (child?.type === batch.kind) siblings.push(child)
+          }
+        }
+      } else {
+        for (const node of Object.values(nodes)) {
+          if (node?.type === batch.kind && !node.parentId) siblings.push(node)
+        }
+      }
+      levelDataByBatch.set(
+        key,
+        (def.computeLevelData as (s: ReadonlyArray<AnyNode>) => unknown)(siblings),
+      )
+    }
+
+    // Phase 3 — per-node rebuild. Each node receives its batch's
+    // precomputed `levelData` in ctx.
+    for (const id of dirtyIds) {
+      const node = nodes[id]
+      if (!node) continue
 
       const def = nodeRegistry.get(node.type)
       const builder = def?.geometry
-      if (!builder) return
+      if (!builder) continue
 
       const group = sceneRegistry.nodes.get(id) as Group | undefined
-      if (!group) return // mount hasn't run — keep dirty for next frame
+      if (!group) continue // mount hasn't run — keep dirty for next frame
 
-      const ctx = buildGeometryContext(node, nodes)
+      const parentId = (node.parentId ?? null) as AnyNodeId | null
+      const key: BatchKey = `${node.type}::${parentId ?? ''}`
+      const levelData = levelDataByBatch.get(key)
+      const ctx = buildGeometryContext(node, nodes, levelData)
 
       // The builder is typed against the kind's specific node — at the
       // generic system level we lose that refinement, so the cast lands
@@ -73,26 +130,42 @@ export const GeometrySystem = () => {
 
       disposeChildren(group)
       for (const child of [...built.children]) {
+        // Tag every child the builder produced so a subsequent rebuild
+        // can dispose only THIS rebuild's outputs and leave React-
+        // mounted siblings (hosted items inside a shelf / slab / etc.)
+        // alone. Without this, a parent rebuild triggered by a child
+        // event (e.g. an item reparenting onto a shelf calls
+        // `dirtyNodes.add(parent)` in `ItemRenderer`'s effect) would
+        // wipe ALL of the parent group's children — including the
+        // freshly-mounted item — leaving the item in scene state but
+        // invisible.
+        ;(child as { userData?: Record<string, unknown> }).userData = {
+          ...(child as { userData?: Record<string, unknown> }).userData,
+          __fromGeometry: true,
+        }
         group.add(child)
       }
-      // Reset transform — matches the legacy per-kind systems
+      // NOTE: we intentionally do NOT reset `group.position` / `group.rotation`
-      // (e.g. FenceSystem.updateFenceGeometry) which clear
+      // here. The `ParametricNodeRenderer` binds them via JSX (`position={...}`
-      // mesh.position/rotation when rebuilding. Tools that translate
+      // / `rotation={...}`) driven by `useLiveTransforms` during drag and
-      // the group via `mesh.position` for live-drag visuals rely on
+      // `node.position` / `node.rotation` after commit. Zeroing them out
-      // this reset to restore the canonical position after scene
+      // during a rebuild would clobber the React-applied transform — and
-      // state catches up (otherwise the geometry rebuild renders on
+      // because the renderer doesn't necessarily re-render on the rebuild
-      // top of the residual offset → double-translation teleport).
+      // tick, R3F wouldn't re-apply the props, leaving the group stuck at
-      group.position.set(0, 0, 0)
+      // origin. Geometry builders are expected to emit local-space children.
-      group.rotation.set(0, 0, 0)
 
       clearDirty(id as AnyNodeId)
-    })
+    }
   }, 2)
 
   return null
 }
 
-function buildGeometryContext(node: AnyNode, nodes: Record<string, AnyNode>): GeometryContext {
+function buildGeometryContext(
+  node: AnyNode,
+  nodes: Record<string, AnyNode>,
+  levelData: unknown,
+): GeometryContext {
   const resolve = <N = AnyNode>(id: AnyNodeId): N | undefined => nodes[id] as N | undefined
 
   const childIds = (node as unknown as { children?: AnyNodeId[] }).children
@@ -122,11 +195,19 @@ function buildGeometryContext(node: AnyNode, nodes: Record<string, AnyNode>): Ge
     }
   }
 
-  return { resolve, children, siblings, parent }
+  return { resolve, children, siblings, parent, levelData }
 }
 
 function disposeChildren(group: Group) {
+  // Only dispose meshes the geometry builder produced on the previous
+  // rebuild (marked via `userData.__fromGeometry`). React-managed
+  // children (hosted node renderers) get left in place — they have
+  // their own React-driven lifecycle and would lose their meshes /
+  // materials if we disposed them here.
   for (const child of [...group.children]) {
+    const fromGeometry = (child as { userData?: { __fromGeometry?: boolean } }).userData
+      ?.__fromGeometry
+    if (!fromGeometry) continue
     group.remove(child)
     const mesh = child as Partial<Mesh> & { geometry?: { dispose?: () => void } }
     if (mesh.geometry?.dispose) mesh.geometry.dispose()