From 8705a5e45d5e8269be8ca09ff148b19ad5440ab4 Mon Sep 17 00:00:00 2001 From: Wassim SAMAD Date: Thu, 8 Oct 2026 16:17:30 -0400 Subject: [PATCH 1/2] fix(editor): saves never include a placement draft Clicking a preset tile clones it into the level before any click, and the window/door tools keep a hover draft in the store; every save read the raw store, so an unplaced window could be autosaved (and survive a reload). Saves now go through getSavedSceneDocument, which leaves out drafts an interaction owns (beginSceneHistoryDraft, the active move-tool scope) and keeps moved nodes at their pre-move state; GLB/JSON exports use it too. Nodes are not filtered by isNew: placed nodes can keep that flag. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RzGbHVg2Tpv4zEw9yLaT5X --- packages/core/src/index.ts | 1 + .../core/src/store/history-drafts.test.ts | 31 +++ .../core/src/store/use-scene-commits.test.ts | 49 +++++ packages/core/src/store/use-scene.ts | 133 +++++++------ .../src/components/editor/export-manager.tsx | 13 +- .../ui/command-palette/editor-commands.tsx | 3 +- packages/editor/src/hooks/use-auto-save.ts | 8 +- packages/editor/src/index.tsx | 2 +- packages/editor/src/lib/glb-export.test.ts | 16 ++ packages/editor/src/lib/glb-export.ts | 5 +- packages/editor/src/lib/scene.ts | 15 ++ packages/nodes/src/door/move-tool.tsx | 35 ++-- packages/nodes/src/door/tool.tsx | 35 +++- .../src/shared/opening-move-history.test.ts | 180 +++++++++++++++++- packages/nodes/src/window/move-tool.tsx | 32 ++-- packages/nodes/src/window/tool.tsx | 39 +++- 16 files changed, 494 insertions(+), 103 deletions(-) diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 832032cb01..916bcf608e 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -587,6 +587,7 @@ export { beginSceneHistoryDraft, clearSceneHistory, default as useScene, + getSceneDocument, runSceneHistoryDraftWrite, type SceneMaterialPatch, type SceneNodePatch, diff --git a/packages/core/src/store/history-drafts.test.ts b/packages/core/src/store/history-drafts.test.ts index 094cebbddd..0f0719759d 100644 --- a/packages/core/src/store/history-drafts.test.ts +++ b/packages/core/src/store/history-drafts.test.ts @@ -13,6 +13,7 @@ import useScene, { applySceneSnapshot, beginSceneHistoryDraft, clearSceneHistory, + getSceneDocument, runSceneHistoryDraftWrite, sceneHistoryDraftRevertUpdates, } from './use-scene' @@ -71,6 +72,36 @@ const wallStart = () => (node(wallId) as WallNode).start const levelChildren = () => (node(levelId) as LevelNode).children describe('scene history drafts', () => { + test('saving during a move keeps its original pose and host, then saves the committed drop', () => { + const end = beginSceneHistoryDraft(itemId, node(itemId)!) + try { + runSceneHistoryDraftWrite(() => + useScene.getState().updateNode(itemId, { + position: [3, 0, 3], + parentId: wallId, + visible: false, + metadata: { isTransient: true }, + }), + ) + useScene.getState().updateNode(itemId, { name: 'Renamed while moving' }) + const document = getSceneDocument() + expect(document.nodes[itemId]).toMatchObject({ + position: [1, 0, 1], + parentId: levelId, + visible: true, + name: 'Renamed while moving', + }) + expect(document.nodes[itemId]!.metadata?.isTransient).toBeUndefined() + expect((document.nodes[levelId] as LevelNode).children).toContain(itemId) + expect((document.nodes[wallId] as WallNode).children).not.toContain(itemId) + expect((node(itemId) as ItemNode).position).toEqual([3, 0, 3]) + } finally { + end() + } + expect(getSceneDocument().nodes[itemId]).toBe(node(itemId)!) + expect((getSceneDocument().nodes[itemId] as ItemNode).position).toEqual([3, 0, 3]) + }) + test.each([ [true, false], [true, true], diff --git a/packages/core/src/store/use-scene-commits.test.ts b/packages/core/src/store/use-scene-commits.test.ts index 9b1adc2a1f..edecc255e6 100644 --- a/packages/core/src/store/use-scene-commits.test.ts +++ b/packages/core/src/store/use-scene-commits.test.ts @@ -9,6 +9,7 @@ import { ItemNode } from '../schema/nodes/item' import { LevelNode } from '../schema/nodes/level' import { SlabNode } from '../schema/nodes/slab' import { WallNode } from '../schema/nodes/wall' +import { WindowNode } from '../schema/nodes/window' import { SceneMaterial, type SceneMaterialId } from '../schema/scene-material' import type { AnyNode, AnyNodeId } from '../schema/types' import { migrateCeilingRoomLinks, migrateRoomZones } from '../utils/room-zone-migration' @@ -28,7 +29,9 @@ import useScene, { applySceneOperationPatch, applyScenePatch, applySceneSnapshot, + beginSceneHistoryDraft, clearSceneHistory, + getSceneDocument, type SceneOperationPatch, } from './use-scene' @@ -234,6 +237,52 @@ describe('scene commit boundary', () => { expect(useScene.getState().nodes[draftWall.id]).toBeDefined() }) + test('saves leave out only the drafts an interaction owns, never a node by its metadata', () => { + expect(getSceneDocument().nodes).toBe(useScene.getState().nodes) + // A preset tile clones its subtree into the level before any click (place-preset). + const presetDraft = WindowNode.parse({ + id: 'window_preset_draft', + parentId: LEVEL_ID, + metadata: { isNew: true, presetId: 'item_window_preset' }, + }) + // A draw tool's draft is registered as a created carry draft instead. + const toolDraft = ItemNode.parse({ + id: 'item_tool_draft', + parentId: LEVEL_ID, + metadata: { isTransient: true }, + asset: { id: 'chair', name: 'Chair', category: 'seating', thumbnail: '', src: '/chair.glb' }, + }) + // Some movers commit a duplicate in place and leave its `isNew` flag on: it is the user's. + const placedDuplicate = WallNode.parse({ + id: 'wall_placed_duplicate', + parentId: LEVEL_ID, + start: [0, 0], + end: [4, 0], + metadata: { isNew: true }, + }) + useScene.getState().createNode(placedDuplicate, LEVEL_ID) + useScene.getState().createNode(presetDraft, LEVEL_ID) + pauseSceneHistory(useScene) + useScene.getState().createNode(toolDraft, LEVEL_ID) + const endToolDraft = beginSceneHistoryDraft(toolDraft.id, null) + + const armed = getSceneDocument([presetDraft.id]) + expect(armed.nodes[presetDraft.id]).toBeUndefined() + expect(armed.nodes[toolDraft.id]).toBeUndefined() + expect(armed.nodes[placedDuplicate.id]).toBeDefined() + expect((armed.nodes[LEVEL_ID] as { children: string[] }).children).toEqual([placedDuplicate.id]) + expect(useScene.getState().nodes[presetDraft.id]).toBeDefined() + + useScene.getState().deleteNode(toolDraft.id) + endToolDraft() + resumeSceneHistory(useScene) + useScene.getState().updateNode(presetDraft.id, { metadata: { presetId: 'item_window_preset' } }) + + const placed = Object.values(getSceneDocument().nodes).filter((node) => node.type === 'window') + expect(placed.map((node) => node.id)).toEqual([presetDraft.id]) + expect(useScene.temporal.getState().pastStates).toHaveLength(1) + }) + test('coalesces a compound transaction into one commit and one undo step', () => { const commits: SceneCommit[] = [] unsubscribe = subscribeSceneCommits((commit) => commits.push(commit)) diff --git a/packages/core/src/store/use-scene.ts b/packages/core/src/store/use-scene.ts index fbfb12cbbd..94a3493603 100644 --- a/packages/core/src/store/use-scene.ts +++ b/packages/core/src/store/use-scene.ts @@ -1215,53 +1215,19 @@ type UseSceneStore = UseBoundStore> & { temporal: StoreApi> } -function sceneHistorySnapshotFromState( - state: Pick< - SceneState, - | 'nodes' - | 'rootNodeIds' - | 'collections' - | 'materials' - | 'installedPlugins' - | 'hasExplicitPluginInstallState' - >, -): SceneSnapshot { - const { - nodes, - rootNodeIds, - collections, - materials, - installedPlugins, - hasExplicitPluginInstallState, - } = state - // Fresh placement nodes are renderable drafts, not document history. Excluding their - // entire subtree here protects both local undo and external commit subscribers. Carried - // drafts (history-drafts.ts) are kept out the same way. - const transientNodeIds = new Set( - createdSceneHistoryDraftIds().filter((id) => Boolean(nodes[id])), - ) - for (const node of Object.values(nodes)) { - const metadata = node.metadata - if ( - metadata && - typeof metadata === 'object' && - !Array.isArray(metadata) && - (metadata as Record).isNew === true - ) { - transientNodeIds.add(node.id) - } - } +type SceneDocumentParts = Pick - if (transientNodeIds.size === 0) { - return { - nodes: withAdoptedDraftsAsOriginal(nodes, nodes), - rootNodeIds, - collections, - materials, - installedPlugins, - hasExplicitPluginInstallState, - } - } +/** + * `parts` without the subtrees of `draftIds`, and without the references to them (children, + * procedural attachments, collections, roots). Returns `parts` itself when there is nothing to drop. + */ +function withoutDraftSubtrees( + parts: SceneDocumentParts, + draftIds: Iterable, +): SceneDocumentParts { + const { nodes, rootNodeIds, collections } = parts + const transientNodeIds = new Set([...draftIds].filter((id) => Boolean(nodes[id]))) + if (transientNodeIds.size === 0) return parts const childIdsByParentId = new Map>() const addChild = (parentId: AnyNodeId, childId: AnyNodeId) => { @@ -1285,7 +1251,7 @@ function sceneHistorySnapshotFromState( } } - const historyNodes = {} as Record + const documentNodes = {} as Record for (const [id, sourceNode] of Object.entries(nodes) as [AnyNodeId, AnyNode][]) { if (transientNodeIds.has(id)) continue let node = sourceNode @@ -1299,38 +1265,97 @@ function sceneHistorySnapshotFromState( if (attachments) node = { ...node, attachments } } if (!('children' in node && Array.isArray(node.children))) { - historyNodes[id] = node + documentNodes[id] = node continue } const children = (node.children as AnyNodeId[]).filter( (childId) => !transientNodeIds.has(childId as AnyNodeId), ) - historyNodes[id] = + documentNodes[id] = children.length === node.children.length ? node : ({ ...node, children } as AnyNode) } - const historyCollections = {} as Record + const documentCollections = {} as Record for (const [id, collection] of Object.entries(collections) as [CollectionId, Collection][]) { const nodeIds = collection.nodeIds.filter((nodeId) => !transientNodeIds.has(nodeId)) if (collection.controlNodeId && transientNodeIds.has(collection.controlNodeId)) { const { controlNodeId: _controlNodeId, ...rest } = collection - historyCollections[id] = { ...rest, nodeIds } + documentCollections[id] = { ...rest, nodeIds } } else { - historyCollections[id] = + documentCollections[id] = nodeIds.length === collection.nodeIds.length ? collection : { ...collection, nodeIds } } } return { - nodes: withAdoptedDraftsAsOriginal(nodes, historyNodes), + nodes: documentNodes, rootNodeIds: rootNodeIds.filter((id) => !transientNodeIds.has(id)), - collections: historyCollections, + collections: documentCollections, + } +} + +function isFreshPlacementNode(node: AnyNode): boolean { + const metadata = node.metadata + return ( + !!metadata && + typeof metadata === 'object' && + !Array.isArray(metadata) && + (metadata as Record).isNew === true + ) +} + +/** + * Fresh placement nodes are renderable drafts, not document history. Excluding their entire + * subtree here protects both local undo and external commit subscribers. Carried drafts + * (history-drafts.ts) are kept out the same way. + */ +function sceneHistorySnapshotFromState( + state: Pick< + SceneState, + | 'nodes' + | 'rootNodeIds' + | 'collections' + | 'materials' + | 'installedPlugins' + | 'hasExplicitPluginInstallState' + >, +): SceneSnapshot { + const { nodes, materials, installedPlugins, hasExplicitPluginInstallState } = state + const kept = withoutDraftSubtrees(state, [ + ...createdSceneHistoryDraftIds(), + ...Object.values(nodes) + .filter(isFreshPlacementNode) + .map((node) => node.id), + ]) + return { + nodes: withAdoptedDraftsAsOriginal(nodes, kept.nodes), + rootNodeIds: kept.rootNodeIds, + collections: kept.collections, materials, installedPlugins, hasExplicitPluginInstallState, } } +/** + * The current scene as every save writes it: without the drafts an interaction is carrying — + * created carry drafts (history-drafts.ts) and the `drafts` the caller owns — and with adopted + * carry drafts as they were before the carry. Never inferred from metadata: committed nodes can + * still carry `metadata.isNew`, so only explicitly owned drafts are left out. + */ +export function getSceneDocument(drafts: Iterable = []): SceneSnapshot { + const state = useScene.getState() + const kept = withoutDraftSubtrees(state, [...createdSceneHistoryDraftIds(), ...drafts]) + return { + nodes: withAdoptedDraftsAsOriginal(state.nodes, kept.nodes), + rootNodeIds: kept.rootNodeIds, + collections: kept.collections, + materials: state.materials, + installedPlugins: state.installedPlugins, + hasExplicitPluginInstallState: state.hasExplicitPluginInstallState, + } +} + /** * A dirty mark is a promise that some system will rebuild the node and clear * the mark, so marks are only accepted for kinds with a dirty consumer: kinds diff --git a/packages/editor/src/components/editor/export-manager.tsx b/packages/editor/src/components/editor/export-manager.tsx index 3320cabda9..ebbf761453 100644 --- a/packages/editor/src/components/editor/export-manager.tsx +++ b/packages/editor/src/components/editor/export-manager.tsx @@ -1,6 +1,6 @@ 'use client' -import { emitter, useScene } from '@pascal-app/core' +import { emitter } from '@pascal-app/core' import { disposeObject3DResources, snapLevelsToTruePositions, useViewer } from '@pascal-app/viewer' import { useThree } from '@react-three/fiber' import { useEffect } from 'react' @@ -25,6 +25,7 @@ import { filterPreparedSceneForPrintContent } from '../../lib/print-content-scop import { exportSceneToPrintStl, mergePrintExportDiagnostics } from '../../lib/print-export' import { applySemanticPrintFeatureThickness } from '../../lib/print-feature-thickness' import { compileSemanticPrintShellWithManifold } from '../../lib/print-shell-compiler-manifold-worker' +import { getSavedSceneDocument } from '../../lib/scene' import { exportSceneToUsdz } from '../../lib/usdz-export' import useEditor from '../../store/use-editor' @@ -75,11 +76,12 @@ export function ExportManager() { // without it every plant exports as its raycast collider, a white box). useViewer.getState().setExporting(true) try { - await waitForExportGeometry(useScene.getState().nodes, options) + await waitForExportGeometry(getSavedSceneDocument().nodes, options) + const { nodes, collections } = getSavedSceneDocument() if (format === 'glb') { const warnings: string[] = [] - const buffer = await exportSceneToGlb(sceneGroup, useScene.getState().nodes, { + const buffer = await exportSceneToGlb(sceneGroup, nodes, { ...options, onWarning: (warning) => warnings.push(warning), }) @@ -89,7 +91,7 @@ export function ExportManager() { if (format === 'usdz') { const warnings: string[] = [] - const data = await exportSceneToUsdz(sceneGroup, useScene.getState().nodes, { + const data = await exportSceneToUsdz(sceneGroup, nodes, { ...options, onWarning: (warning) => warnings.push(warning), }) @@ -103,7 +105,6 @@ export function ExportManager() { // window, so the export snapshots the clean building, then restore. emitter.emit('thumbnail:before-capture', undefined) const restoreLevels = snapLevelsToTruePositions() - const nodes = useScene.getState().nodes let prepared: ReturnType try { prepared = prepareSceneForExport(sceneGroup, nodes, { @@ -216,7 +217,7 @@ export function ExportManager() { if (format === 'ifc') { const { data, warnings } = exportPreparedSceneToIfc(exportScene, nodes, { - collections: useScene.getState().collections, + collections, projectName: options.projectName, onlyVisible: options.onlyVisible, excludedNodeTypes: options.excludedNodeTypes, diff --git a/packages/editor/src/components/ui/command-palette/editor-commands.tsx b/packages/editor/src/components/ui/command-palette/editor-commands.tsx index 173fa90b34..36a306f3cb 100644 --- a/packages/editor/src/components/ui/command-palette/editor-commands.tsx +++ b/packages/editor/src/components/ui/command-palette/editor-commands.tsx @@ -39,6 +39,7 @@ import { import { useEffect } from 'react' import { getHistoryCommandState, runRedo, runUndo } from '../../../lib/history' import { deleteLevelWithFallbackSelection } from '../../../lib/level-selection' +import { getSavedSceneDocument } from '../../../lib/scene' import { createUnitInBuilding, enterUnitFocus, leaveUnitFocus } from '../../../lib/units' import { useCommandRegistry } from '../../../store/use-command-registry' import type { StructureTool } from '../../../store/use-editor' @@ -397,7 +398,7 @@ export function EditorCommands() { keywords: ['export', 'download', 'json', 'save', 'data'], execute: () => run(() => { - const { nodes, rootNodeIds } = useScene.getState() + const { nodes, rootNodeIds } = getSavedSceneDocument() const blob = new Blob([JSON.stringify({ nodes, rootNodeIds }, null, 2)], { type: 'application/json', }) diff --git a/packages/editor/src/hooks/use-auto-save.ts b/packages/editor/src/hooks/use-auto-save.ts index 553f4eed3e..134dfb6df7 100644 --- a/packages/editor/src/hooks/use-auto-save.ts +++ b/packages/editor/src/hooks/use-auto-save.ts @@ -2,7 +2,7 @@ import { useScene } from '@pascal-app/core' import { useCallback, useEffect, useRef } from 'react' -import { type SceneGraph, saveSceneToLocalStorage } from '../lib/scene' +import { getSavedSceneDocument, type SceneGraph, saveSceneToLocalStorage } from '../lib/scene' const AUTOSAVE_DEBOUNCE_MS = 1000 const STRUCTURAL_NODE_COUNT = 4 @@ -170,7 +170,8 @@ export function useAutoSave({ return } - const { nodes, rootNodeIds, collections, materials, installedPlugins } = useScene.getState() + const { nodes, rootNodeIds, collections, materials, installedPlugins } = + getSavedSceneDocument() const sceneGraph = { nodes, rootNodeIds, @@ -278,7 +279,8 @@ export function useAutoSave({ // would otherwise drop the change entirely. `pagehide` fires in cases // (mobile Safari, bfcache) where `beforeunload` does not. function flushOnExit() { - const { nodes, rootNodeIds, collections, materials, installedPlugins } = useScene.getState() + const { nodes, rootNodeIds, collections, materials, installedPlugins } = + getSavedSceneDocument() const currentNodeCount = Object.keys(nodes).length const previousNodeCount = storedNodeCount.count const decision = decideExitFlush({ diff --git a/packages/editor/src/index.tsx b/packages/editor/src/index.tsx index e957332d8b..774f2c6a3d 100644 --- a/packages/editor/src/index.tsx +++ b/packages/editor/src/index.tsx @@ -748,7 +748,7 @@ export { type ElementActionOrigin, } from './lib/room-zone-routing' export type { SceneGraph } from './lib/scene' -export { applySceneGraphToEditor } from './lib/scene' +export { applySceneGraphToEditor, getSavedSceneDocument } from './lib/scene' export { forEachSceneMaterialRef, referencedSceneMaterialIds, diff --git a/packages/editor/src/lib/glb-export.test.ts b/packages/editor/src/lib/glb-export.test.ts index bad2aac8dc..b426355848 100644 --- a/packages/editor/src/lib/glb-export.test.ts +++ b/packages/editor/src/lib/glb-export.test.ts @@ -95,6 +95,22 @@ function sceneWithVisibleAndHiddenBoxes(): { } describe('prepareSceneForExport', () => { + test.each([ + true, + false, + ])('omits registered draft geometry absent from the saved document (onlyVisible=%s)', (onlyVisible) => { + const { root, nodes } = sceneWithVisibleAndHiddenBoxes() + const draft = sceneRegistry.nodes.get('item_hidden')! + draft.name = 'Unplaced preview' + nodes.item_hidden!.visible = true + delete nodes.item_hidden + + const result = prepareSceneForExport(root, nodes, { onlyVisible }) + expect(result.scene.getObjectByName('Unplaced preview')).toBeUndefined() + expect(result.scene.children).toHaveLength(1) + expect(root.getObjectByName('Unplaced preview')).toBe(draft) + }) + test('skips a refused subtree and reports its warning while exporting other objects', () => { const root = new THREE.Group() const refused = new THREE.Group() diff --git a/packages/editor/src/lib/glb-export.ts b/packages/editor/src/lib/glb-export.ts index 9e115d477c..b25f8efe78 100644 --- a/packages/editor/src/lib/glb-export.ts +++ b/packages/editor/src/lib/glb-export.ts @@ -482,7 +482,10 @@ function startSceneExportPreparation( for (const [id, original] of registryEntries) { const node = nodes[id] - if (!node) continue + if (!node) { + excludedObjects.add(original) + continue + } const installedPlugins = sceneState.hasExplicitPluginInstallState ? sceneState.installedPlugins : undefined diff --git a/packages/editor/src/lib/scene.ts b/packages/editor/src/lib/scene.ts index ad76a52f10..38c65947f9 100644 --- a/packages/editor/src/lib/scene.ts +++ b/packages/editor/src/lib/scene.ts @@ -1,7 +1,9 @@ 'use client' import { + type AnyNodeId, clearSceneHistory, + getSceneDocument, materializeRegisteredNodeDefaults, nodeRegistry, resolveLevelId, @@ -14,6 +16,7 @@ import useEditor, { hasCustomPersistedEditorUiState, type PersistedEditorUiState, } from '../store/use-editor' +import useInteractionScope from '../store/use-interaction-scope' import { editorHostPanelRegistry } from './plugin-panels' export type SceneGraph = { @@ -26,6 +29,18 @@ export type SceneGraph = { installedPlugins?: string[] } +/** + * The scene as saves and exports write it. A clone in hand — a preset or duplicate the user is + * placing — is in the store from the tile click so tools can render and host it, but it only + * becomes part of the document when its placement commits. + */ +export function getSavedSceneDocument() { + const { scope } = useInteractionScope.getState() + const placing = + scope.kind === 'placing' && scope.driver === 'move-tool' ? [scope.nodeId as AnyNodeId] : [] + return getSceneDocument(placing) +} + type PersistedSelectionPath = { buildingId: string | null levelId: string | null diff --git a/packages/nodes/src/door/move-tool.tsx b/packages/nodes/src/door/move-tool.tsx index e92cea170e..f921bef7f5 100644 --- a/packages/nodes/src/door/move-tool.tsx +++ b/packages/nodes/src/door/move-tool.tsx @@ -1,5 +1,6 @@ import { type AnyNodeId, + beginSceneHistoryDraft, collectionIdsOf, DoorNode, emitter, @@ -9,6 +10,7 @@ import { isCurvedWall, type RoofEvent, type RoofNode, + runSceneHistoryDraftWrite, sceneRegistry, spatialGridManager, useLiveTransforms, @@ -127,6 +129,9 @@ const MoveDoorTool: React.FC<{ node: DoorNode }> = ({ node: movingDoorNode }) => ? (movingDoorNode.metadata as Record) : {} const isNew = !!meta.isNew + const endDraft = isNew ? null : beginSceneHistoryDraft(movingDoorNode.id, movingDoorNode) + const updateDraft = (data: Partial) => + runSceneHistoryDraftWrite(() => useScene.getState().updateNode(movingDoorNode.id, data)) const original = { position: [...movingDoorNode.position] as [number, number, number], @@ -147,7 +152,7 @@ const MoveDoorTool: React.FC<{ node: DoorNode }> = ({ node: movingDoorNode }) => } if (!isNew) { - useScene.getState().updateNode(movingDoorNode.id, { + updateDraft({ metadata: { ...meta, isTransient: true }, }) } @@ -405,7 +410,7 @@ const MoveDoorTool: React.FC<{ node: DoorNode }> = ({ node: movingDoorNode }) => doorMesh.updateMatrixWorld(true) } } else { - useScene.getState().updateNode(movingDoorNode.id, { + updateDraft({ position: [target.clampedX, target.clampedY, planeOffsetOn(target.wallId)], rotation: [0, target.itemRotation, 0], side: target.side, @@ -584,7 +589,7 @@ const MoveDoorTool: React.FC<{ node: DoorNode }> = ({ node: movingDoorNode }) => // Move mode: restore the exact pre-drag state while history is still // paused (the clean undo baseline), then apply the drop as the // gesture's ONE tracked write — undo reverts to the original state. - useScene.getState().updateNode(movingDoorNode.id, { + updateDraft({ position: original.position, rotation: original.rotation, side: original.side, @@ -620,6 +625,7 @@ const MoveDoorTool: React.FC<{ node: DoorNode }> = ({ node: movingDoorNode }) => triggerSFX('sfx:structure-build') hideCursor() useViewer.getState().setSelection({ selectedIds: [placedId] }) + endDraft?.() exitMoveMode() } @@ -665,7 +671,7 @@ const MoveDoorTool: React.FC<{ node: DoorNode }> = ({ node: movingDoorNode }) => clearPlacementSurface() const live = useScene.getState().nodes[movingDoorNode.id as AnyNodeId] as DoorNode | undefined if (live && live.visible === false) { - useScene.getState().updateNode(movingDoorNode.id, { visible: true }) + updateDraft({ visible: true }) } } @@ -694,7 +700,7 @@ const MoveDoorTool: React.FC<{ node: DoorNode }> = ({ node: movingDoorNode }) => // the next mousemove. const yaw = sideOverride === 'back' ? Math.PI : 0 if (currentHostId === levelId) { - useScene.getState().updateNode(movingDoorNode.id, { + updateDraft({ position: [localX, y, localZ], rotation: [0, yaw, 0], side: sideOverride, @@ -702,7 +708,7 @@ const MoveDoorTool: React.FC<{ node: DoorNode }> = ({ node: movingDoorNode }) => }) } else { if (currentHostId && currentHostId !== levelId) markHostDirty(currentHostId) - useScene.getState().updateNode(movingDoorNode.id, { + updateDraft({ position: [localX, y, localZ], rotation: [0, yaw, 0], side: sideOverride, @@ -785,13 +791,13 @@ const MoveDoorTool: React.FC<{ node: DoorNode }> = ({ node: movingDoorNode }) => // and reveal the node. revealRealNode() if (currentHostId === target.segment.id) { - useScene.getState().updateNode(movingDoorNode.id, { + updateDraft({ position: target.position, rotation: [0, 0, 0], roofFace: target.face.id, }) } else { - useScene.getState().updateNode(movingDoorNode.id, { + updateDraft({ position: target.position, rotation: [0, 0, 0], side: 'front', @@ -849,7 +855,7 @@ const MoveDoorTool: React.FC<{ node: DoorNode }> = ({ node: movingDoorNode }) => } else { // See commitToWall — restore the pre-drag baseline paused, drop as // the ONE tracked write. - useScene.getState().updateNode(movingDoorNode.id, { + updateDraft({ position: original.position, rotation: original.rotation, side: original.side, @@ -886,6 +892,7 @@ const MoveDoorTool: React.FC<{ node: DoorNode }> = ({ node: movingDoorNode }) => triggerSFX('sfx:structure-build') hideCursor() useViewer.getState().setSelection({ selectedIds: [placedId] }) + endDraft?.() exitMoveMode() event.stopPropagation() } @@ -906,7 +913,7 @@ const MoveDoorTool: React.FC<{ node: DoorNode }> = ({ node: movingDoorNode }) => useScene.getState().deleteNode(movingDoorNode.id) if (currentHostId) markHostDirty(currentHostId) } else { - useScene.getState().updateNode(movingDoorNode.id, { + updateDraft({ position: original.position, rotation: original.rotation, side: original.side, @@ -924,6 +931,7 @@ const MoveDoorTool: React.FC<{ node: DoorNode }> = ({ node: movingDoorNode }) => // entirely. `end` is idempotent — the effect cleanup's end() is a no-op. history.end() hideCursor() + endDraft?.() exitMoveMode() } @@ -978,7 +986,7 @@ const MoveDoorTool: React.FC<{ node: DoorNode }> = ({ node: movingDoorNode }) => // No preview yet (R pressed before the first pointermove at initial // placement): flip the hidden node so the FIRST preview/commit already // reflects the chosen side. - useScene.getState().updateNode(movingDoorNode.id, { + updateDraft({ side: sideOverride, rotation: [0, sideOverride === 'back' ? Math.PI : 0, 0], }) @@ -1064,7 +1072,7 @@ const MoveDoorTool: React.FC<{ node: DoorNode }> = ({ node: movingDoorNode }) => useScene.getState().deleteNode(movingDoorNode.id) if (currentHostId) markHostDirty(currentHostId) } else { - useScene.getState().updateNode(movingDoorNode.id, { + updateDraft({ position: original.position, rotation: original.rotation, side: original.side, @@ -1083,7 +1091,7 @@ const MoveDoorTool: React.FC<{ node: DoorNode }> = ({ node: movingDoorNode }) => // hidden — reveal it so it never becomes an invisible orphan. (The // `place-preset` movingNode subscription deletes a truly-cancelled // clone separately.) - useScene.getState().updateNode(movingDoorNode.id, { visible: true }) + updateDraft({ visible: true }) } useLiveTransforms.getState().clear(movingDoorNode.id) useAlignmentGuides.getState().clear() @@ -1092,6 +1100,7 @@ const MoveDoorTool: React.FC<{ node: DoorNode }> = ({ node: movingDoorNode }) => clearPlacementSurface() releaseHiddenWallHold() history.end() + endDraft?.() emitter.off('wall:enter', onWallEnter) emitter.off('wall:move', onWallMove) emitter.off('wall:click', onWallClick) diff --git a/packages/nodes/src/door/tool.tsx b/packages/nodes/src/door/tool.tsx index cef02e1aca..ed59f1bde9 100644 --- a/packages/nodes/src/door/tool.tsx +++ b/packages/nodes/src/door/tool.tsx @@ -1,6 +1,7 @@ import { type AnyNode, type AnyNodeId, + beginSceneHistoryDraft, DoorNode, emitter, type GridEvent, @@ -119,6 +120,29 @@ const DoorTool: React.FC = () => { useEffect(() => { useScene.temporal.getState().pause() + // The draft is a created carry draft: undo history and every save leave it out + // (core `getSceneDocument`), so a reload mid-placement never finds it in the scene. + const draftHistoryEnds = new Map void>() + const createDraft = (node: DoorNode, parentId: AnyNodeId) => { + const end = beginSceneHistoryDraft(node.id as AnyNodeId, null) + draftHistoryEnds.set(node.id, end) + try { + useScene.getState().createNode(node, parentId) + } catch (error) { + end() + draftHistoryEnds.delete(node.id) + throw error + } + } + const deleteDraft = (id: string) => { + try { + useScene.getState().deleteNode(id as AnyNodeId) + } finally { + draftHistoryEnds.get(id)?.() + draftHistoryEnds.delete(id) + } + } + const ownedPreviewIds = new Set() const fallbackPreview = () => DoorNode.parse({ @@ -193,7 +217,7 @@ const DoorTool: React.FC = () => { return } const wallId = draft.parentId - useScene.getState().deleteNode(draft.id) + deleteDraft(draft.id) draftRef.current = null clearPlacementPreview() if (wallId) markHostDirty(wallId) @@ -343,7 +367,7 @@ const DoorTool: React.FC = () => { parentId: wall.id, metadata: { isTransient: true }, }) - useScene.getState().createNode(node, wall.id as AnyNodeId) + createDraft(node, wall.id as AnyNodeId) draftRef.current = node } @@ -421,7 +445,7 @@ const DoorTool: React.FC = () => { draftRef.current = null hostKind = null - useScene.getState().deleteNode(draft.id) + deleteDraft(draft.id) useScene.temporal.getState().resume() const levelId = getLevelId() @@ -639,7 +663,7 @@ const DoorTool: React.FC = () => { parentId: segment.id, metadata: { isTransient: true }, }) - useScene.getState().createNode(node, segment.id as AnyNodeId) + createDraft(node, segment.id as AnyNodeId) draftRef.current = node } publishDraftPreview(segment) @@ -662,7 +686,7 @@ const DoorTool: React.FC = () => { draftRef.current = null hostKind = null - useScene.getState().deleteNode(draft.id) + deleteDraft(draft.id) useScene.temporal.getState().resume() const state = useScene.getState() @@ -796,6 +820,7 @@ const DoorTool: React.FC = () => { return () => { destroyDraft() + for (const end of draftHistoryEnds.values()) end() hideCursor() clearPlacementPreview() useAlignmentGuides.getState().clear() diff --git a/packages/nodes/src/shared/opening-move-history.test.ts b/packages/nodes/src/shared/opening-move-history.test.ts index 9fb9e83024..df53483428 100644 --- a/packages/nodes/src/shared/opening-move-history.test.ts +++ b/packages/nodes/src/shared/opening-move-history.test.ts @@ -1,17 +1,33 @@ -import { afterEach, beforeEach, describe, expect, test } from 'bun:test' +import { afterEach, beforeEach, describe, expect, spyOn, test } from 'bun:test' import { type AnyNode, type AnyNodeId, BuildingNode, clearSceneHistory, DoorNode, + emitter, getSceneHistoryPauseDepth, LevelNode, pauseSceneHistory, + registerNode, resumeSceneHistory, useScene, WallNode, + WindowNode, } from '@pascal-app/core' +import { + getSavedSceneDocument, + preloadRegistryToolModules, + ToolManager, + useEditor, + useOpeningGuides, +} from '@pascal-app/editor' +import { useViewer } from '@pascal-app/viewer' +import { act, create } from '@react-three/test-renderer' +import { createElement } from 'react' +import { Group } from 'three' +import { doorDefinition } from '../door' +import { windowDefinition } from '../window' import { beginOpeningMoveHistorySession } from './opening-move-history' // `updateNodesAction` batches dirty-marking through requestAnimationFrame. @@ -189,6 +205,168 @@ describe('opening move history session', () => { clearSceneHistory() }) + test.each( + [doorDefinition, windowDefinition].flatMap((definition) => + (['cancel', 'unmount', 'commit'] as const).map((finish) => ({ definition, finish })), + ), + )('a mounted $definition.kind mover saves the original until $finish and releases its draft', async ({ + definition, + finish, + }) => { + const globals = ['window', 'document'] as const + const descriptors = globals.map((key) => Object.getOwnPropertyDescriptor(globalThis, key)) + globalThis.window = new EventTarget() as Window & typeof globalThis + globalThis.document = { body: { style: { cursor: '' } } } as Document + const original = + definition.kind === 'door' + ? node(DOOR_ID) + : WindowNode.parse({ + id: 'window_history', + parentId: WALL_A_ID, + wallId: WALL_A_ID, + position: [1.5, 1.05, 0], + }) + if (definition.kind === 'window') { + useScene.getState().deleteNode(DOOR_ID) + useScene.getState().createNode(original, WALL_A_ID) + clearSceneHistory() + } + const initialViewer = useViewer.getState() + const initialEditor = useEditor.getState() + registerNode(definition) + useEditor.setState({ phase: 'building', mode: 'select', tool: null }) + useViewer.getState().setSelection({ levelId: LEVEL_ID }) + useEditor.getState().setMovingNode(original) + await preloadRegistryToolModules(definition.kind) + const renderer = await create(createElement(ToolManager)) + const now = spyOn(performance, 'now').mockReturnValue(performance.now() + 1000) + try { + expect(useScene.getState().nodes[original.id]!.metadata?.isTransient).toBe(true) + expect(getSavedSceneDocument().nodes[original.id]).toEqual(original) + await act(async () => + emitter.emit('grid:move', { + position: [8, 0, 8], + localPosition: [8, 0, 8], + nativeEvent: {} as never, + }), + ) + expect(useScene.getState().nodes[original.id]!.parentId).toBe(LEVEL_ID) + expect(getSavedSceneDocument().nodes[original.id]).toEqual(original) + if (finish === 'cancel') await act(async () => emitter.emit('tool:cancel')) + if (finish === 'commit') { + const event = { + node: node(WALL_B_ID) as WallNode, + position: [3, 1.05, -2.5] as [number, number, number], + localPosition: [3, 1.05, 0] as [number, number, number], + normal: [0, 0, 1] as [number, number, number], + object: new Group(), + stopPropagation: () => {}, + nativeEvent: {} as never, + } + await act(async () => { + emitter.emit('wall:move', event) + emitter.emit('wall:click', event) + }) + expect(useScene.getState().nodes[original.id]!.parentId).toBe(WALL_B_ID) + expect(getSavedSceneDocument().nodes[original.id]).toEqual( + useScene.getState().nodes[original.id], + ) + } + await renderer.unmount() + expect(getSavedSceneDocument().nodes[original.id]).toEqual( + useScene.getState().nodes[original.id], + ) + useScene.getState().updateNode(original.id, { position: [4, 1, 0] } as never) + expect(getSavedSceneDocument().nodes[original.id]).toEqual( + useScene.getState().nodes[original.id], + ) + } finally { + now.mockRestore() + await renderer.unmount() + useEditor.getState().setMovingNode(null) + useEditor.setState(initialEditor) + useViewer.setState(initialViewer) + globals.forEach((key, index) => { + const descriptor = descriptors[index] + if (descriptor) Object.defineProperty(globalThis, key, descriptor) + else Reflect.deleteProperty(globalThis, key) + }) + } + }) + + test.each( + [doorDefinition, windowDefinition].flatMap((definition) => + (['cancel', 'unmount', 'commit'] as const).map((finish) => ({ definition, finish })), + ), + )('a mounted $definition.kind draw tool excludes its hover draft until $finish', async ({ + definition, + finish, + }) => { + const globals = ['window', 'document'] as const + const descriptors = globals.map((key) => Object.getOwnPropertyDescriptor(globalThis, key)) + globalThis.window = new EventTarget() as Window & typeof globalThis + globalThis.document = { body: { style: { cursor: '' } } } as Document + const initialViewer = useViewer.getState() + const initialEditor = useEditor.getState() + registerNode(definition) + useEditor.getState().armToolMode({ mode: 'build', tool: definition.kind as 'door' | 'window' }) + useViewer.getState().setSelection({ levelId: LEVEL_ID }) + await preloadRegistryToolModules(definition.kind) + const renderer = await create(createElement(ToolManager)) + const event = { + node: node(WALL_B_ID) as WallNode, + position: [3, 1.05, -2.5] as [number, number, number], + localPosition: [3, 1.05, 0] as [number, number, number], + normal: [0, 0, 1] as [number, number, number], + object: new Group(), + stopPropagation: () => {}, + nativeEvent: {} as never, + } + try { + const baseline = getSavedSceneDocument().nodes + const published: Array['nodes']> = [] + const stop = useScene.subscribe(() => published.push(getSavedSceneDocument().nodes)) + try { + await act(async () => { + emitter.emit('wall:move', event) + // Dimension labels require a DOM; the draft and its host do not. + useOpeningGuides.getState().clear() + }) + } finally { + stop() + } + const drafts = Object.values(useScene.getState().nodes).filter( + (entry) => entry.metadata?.isTransient, + ) + expect(drafts).toHaveLength(1) + expect(published.length).toBeGreaterThan(0) + for (const snapshot of published) expect(snapshot).toEqual(baseline) + expect(getSavedSceneDocument().nodes).toEqual(baseline) + if (finish === 'cancel') await act(async () => emitter.emit('tool:cancel')) + if (finish === 'commit') { + await act(async () => emitter.emit('wall:click', event)) + const saved = Object.values(getSavedSceneDocument().nodes).filter( + (entry) => !baseline[entry.id], + ) + expect(saved).toHaveLength(1) + expect(saved[0]!.type).toBe(definition.kind) + } + await renderer.unmount() + expect(useScene.getState().nodes[drafts[0]!.id]).toBeUndefined() + expect(getSavedSceneDocument().nodes).toEqual(useScene.getState().nodes) + } finally { + await renderer.unmount() + useEditor.getState().setMovingNode(null) + useEditor.setState(initialEditor) + useViewer.setState(initialViewer) + globals.forEach((key, index) => { + const descriptor = descriptors[index] + if (descriptor) Object.defineProperty(globalThis, key, descriptor) + else Reflect.deleteProperty(globalThis, key) + }) + } + }) + test('a completed gesture is EXACTLY ONE undo entry; undo restores the pre-drag state', () => { const cooperating = attachCooperatingSubscriber() try { diff --git a/packages/nodes/src/window/move-tool.tsx b/packages/nodes/src/window/move-tool.tsx index f3f62499ef..201fb07e32 100644 --- a/packages/nodes/src/window/move-tool.tsx +++ b/packages/nodes/src/window/move-tool.tsx @@ -1,5 +1,6 @@ import { type AnyNodeId, + beginSceneHistoryDraft, collectionIdsOf, type DormerEvent, dormerWallFacePointToDormer, @@ -10,6 +11,7 @@ import { isCurvedWall, type RoofEvent, type RoofNode, + runSceneHistoryDraftWrite, sceneRegistry, spatialGridManager, useLiveNodeOverrides, @@ -159,6 +161,9 @@ const MoveWindowTool: React.FC<{ node: WindowNode }> = ({ node: movingWindowNode ? (movingWindowNode.metadata as Record) : {} const isNew = !!meta.isNew + const endDraft = isNew ? null : beginSceneHistoryDraft(movingWindowNode.id, movingWindowNode) + const updateDraft = (data: Partial) => + runSceneHistoryDraftWrite(() => useScene.getState().updateNode(movingWindowNode.id, data)) // Save original state (only used in move mode) const original = { @@ -186,7 +191,7 @@ const MoveWindowTool: React.FC<{ node: WindowNode }> = ({ node: movingWindowNode // preview on the wall and can't be placed consecutively without leaving/re-entering. This // mirrors MoveDoorTool. if (!isNew) { - useScene.getState().updateNode(movingWindowNode.id, { + updateDraft({ metadata: { ...meta, isTransient: true }, }) } @@ -479,7 +484,7 @@ const MoveWindowTool: React.FC<{ node: WindowNode }> = ({ node: movingWindowNode windowMesh.updateMatrixWorld(true) } } else { - useScene.getState().updateNode(movingWindowNode.id, { + updateDraft({ position: [target.clampedX, target.clampedY, planeOffsetOn(target.wallId)], rotation: [0, target.itemRotation, 0], side: target.side, @@ -651,7 +656,7 @@ const MoveWindowTool: React.FC<{ node: WindowNode }> = ({ node: movingWindowNode // Move mode: restore the exact pre-drag state while history is still // paused (the clean undo baseline), then apply the drop as the // gesture's ONE tracked write — undo reverts to the original state. - useScene.getState().updateNode(movingWindowNode.id, { + updateDraft({ position: original.position, rotation: original.rotation, side: original.side, @@ -689,6 +694,7 @@ const MoveWindowTool: React.FC<{ node: WindowNode }> = ({ node: movingWindowNode triggerSFX('sfx:structure-build') hideCursor() selectNode(placedId as AnyNodeId) + endDraft?.() exitMoveMode() } @@ -760,7 +766,7 @@ const MoveWindowTool: React.FC<{ node: WindowNode }> = ({ node: movingWindowNode // window from the wall's `children` or the CSG cut trails the ghost // around the old wall (see the wall-branch note in `applyPreview`). if (currentHostId === levelId) { - useScene.getState().updateNode(movingWindowNode.id, { + updateDraft({ position: [localX, sillCenterY, localZ], rotation: [0, yaw, 0], side: sideOverride, @@ -768,7 +774,7 @@ const MoveWindowTool: React.FC<{ node: WindowNode }> = ({ node: movingWindowNode }) } else { if (currentHostId && currentHostId !== levelId) markHostDirty(currentHostId) - useScene.getState().updateNode(movingWindowNode.id, { + updateDraft({ position: [localX, sillCenterY, localZ], rotation: [0, yaw, 0], side: sideOverride, @@ -907,7 +913,7 @@ const MoveWindowTool: React.FC<{ node: WindowNode }> = ({ node: movingWindowNode }) placedId = committedNode.id } else { - useScene.getState().updateNode(movingWindowNode.id, { + updateDraft({ position: original.position, rotation: original.rotation, side: original.side, @@ -945,6 +951,7 @@ const MoveWindowTool: React.FC<{ node: WindowNode }> = ({ node: movingWindowNode triggerSFX('sfx:structure-build') hideCursor() selectNode(placedId as AnyNodeId) + endDraft?.() exitMoveMode() event.stopPropagation() } @@ -1119,7 +1126,7 @@ const MoveWindowTool: React.FC<{ node: WindowNode }> = ({ node: movingWindowNode } else { // See commitToWall — restore the pre-drag baseline paused, drop as // the ONE tracked write. - useScene.getState().updateNode(movingWindowNode.id, { + updateDraft({ position: original.position, rotation: original.rotation, side: original.side, @@ -1158,6 +1165,7 @@ const MoveWindowTool: React.FC<{ node: WindowNode }> = ({ node: movingWindowNode triggerSFX('sfx:structure-build') hideCursor() selectNode(placedId as AnyNodeId) + endDraft?.() exitMoveMode() event.stopPropagation() } @@ -1180,7 +1188,7 @@ const MoveWindowTool: React.FC<{ node: WindowNode }> = ({ node: movingWindowNode useScene.getState().deleteNode(movingWindowNode.id) if (currentHostId) markHostDirty(currentHostId) } else { - useScene.getState().updateNode(movingWindowNode.id, { + updateDraft({ position: original.position, rotation: original.rotation, side: original.side, @@ -1200,6 +1208,7 @@ const MoveWindowTool: React.FC<{ node: WindowNode }> = ({ node: movingWindowNode // entirely. `end` is idempotent — the effect cleanup's end() is a no-op. history.end() hideCursor() + endDraft?.() exitMoveMode() } @@ -1254,7 +1263,7 @@ const MoveWindowTool: React.FC<{ node: WindowNode }> = ({ node: movingWindowNode } else { // No preview yet (R before the first pointermove): flip the hidden node // so the first preview/commit already reflects the chosen side. - useScene.getState().updateNode(movingWindowNode.id, { + updateDraft({ side: sideOverride, rotation: [0, sideOverride === 'back' ? Math.PI : 0, 0], }) @@ -1347,7 +1356,7 @@ const MoveWindowTool: React.FC<{ node: WindowNode }> = ({ node: movingWindowNode useScene.getState().deleteNode(movingWindowNode.id) if (currentHostId) markHostDirty(currentHostId) } else { - useScene.getState().updateNode(movingWindowNode.id, { + updateDraft({ position: original.position, rotation: original.rotation, side: original.side, @@ -1366,7 +1375,7 @@ const MoveWindowTool: React.FC<{ node: WindowNode }> = ({ node: movingWindowNode // Safety net: a fresh (isNew) clone isn't marked `isTransient`; if we // unmount mid-free-follow it would be left hidden. Reveal it so it never // becomes an invisible orphan (place-preset deletes a true cancel). - useScene.getState().updateNode(movingWindowNode.id, { visible: true }) + updateDraft({ visible: true }) } useLiveNodeOverrides.getState().clear(movingWindowNode.id) useLiveTransforms.getState().clear(movingWindowNode.id) @@ -1376,6 +1385,7 @@ const MoveWindowTool: React.FC<{ node: WindowNode }> = ({ node: movingWindowNode clearPlacementSurface() releaseHiddenWallHold() history.end() + endDraft?.() emitter.off('wall:enter', onWallEnter) emitter.off('wall:move', onWallMove) emitter.off('wall:click', onWallClick) diff --git a/packages/nodes/src/window/tool.tsx b/packages/nodes/src/window/tool.tsx index 997b7f1218..8a615e424f 100644 --- a/packages/nodes/src/window/tool.tsx +++ b/packages/nodes/src/window/tool.tsx @@ -1,6 +1,7 @@ import { type AnyNode, type AnyNodeId, + beginSceneHistoryDraft, type DormerEvent, type DormerNode, dormerWallFacePointToDormer, @@ -150,6 +151,29 @@ const WindowTool: React.FC = () => { useEffect(() => { useScene.temporal.getState().pause() + // The draft is a created carry draft: undo history and every save leave it out + // (core `getSceneDocument`), so a reload mid-placement never finds it in the scene. + const draftHistoryEnds = new Map void>() + const createDraft = (node: WindowNode, parentId: AnyNodeId) => { + const end = beginSceneHistoryDraft(node.id as AnyNodeId, null) + draftHistoryEnds.set(node.id, end) + try { + useScene.getState().createNode(node, parentId) + } catch (error) { + end() + draftHistoryEnds.delete(node.id) + throw error + } + } + const deleteDraft = (id: string) => { + try { + useScene.getState().deleteNode(id as AnyNodeId) + } finally { + draftHistoryEnds.get(id)?.() + draftHistoryEnds.delete(id) + } + } + const ownedPreviewIds = new Set() const fallbackPreview = () => WindowNode.parse({ @@ -225,7 +249,7 @@ const WindowTool: React.FC = () => { } const wallId = draft.parentId useLiveNodeOverrides.getState().clear(draft.id) - useScene.getState().deleteNode(draft.id) + deleteDraft(draft.id) draftRef.current = null clearPlacementPreview() // Rebuild wall so it removes the cutout from the deleted draft @@ -361,7 +385,7 @@ const WindowTool: React.FC = () => { dormerFace: target.face, metadata: { isTransient: true }, }) - useScene.getState().createNode(node, event.node.id as AnyNodeId) + createDraft(node, event.node.id as AnyNodeId) draftRef.current = node } @@ -478,7 +502,7 @@ const WindowTool: React.FC = () => { parentId: wall.id, metadata: { isTransient: true }, }) - useScene.getState().createNode(node, wall.id as AnyNodeId) + createDraft(node, wall.id as AnyNodeId) draftRef.current = node } @@ -559,7 +583,7 @@ const WindowTool: React.FC = () => { hostKind = null useLiveNodeOverrides.getState().clear(draft.id) - useScene.getState().deleteNode(draft.id) + deleteDraft(draft.id) useScene.temporal.getState().resume() const levelId = getLevelId() @@ -617,7 +641,7 @@ const WindowTool: React.FC = () => { hostKind = null useLiveNodeOverrides.getState().clear(draft.id) - useScene.getState().deleteNode(draft.id) + deleteDraft(draft.id) useScene.temporal.getState().resume() const state = useScene.getState() @@ -942,7 +966,7 @@ const WindowTool: React.FC = () => { parentId: segment.id, metadata: { isTransient: true }, }) - useScene.getState().createNode(node, segment.id as AnyNodeId) + createDraft(node, segment.id as AnyNodeId) draftRef.current = node } publishDraftPreview(segment) @@ -966,7 +990,7 @@ const WindowTool: React.FC = () => { hostKind = null useLiveNodeOverrides.getState().clear(draft.id) - useScene.getState().deleteNode(draft.id) + deleteDraft(draft.id) useScene.temporal.getState().resume() const state = useScene.getState() @@ -1103,6 +1127,7 @@ const WindowTool: React.FC = () => { return () => { destroyDraft() + for (const end of draftHistoryEnds.values()) end() hideCursor() clearPlacementPreview() useAlignmentGuides.getState().clear() From 75c9ab99ceb41988095b13f736c4f766e2cb375c Mon Sep 17 00:00:00 2001 From: Wassim SAMAD Date: Thu, 8 Oct 2026 16:24:05 -0400 Subject: [PATCH 2/2] fix(rooms): pills move aside instead of vanishing, and sit under the UI Pill sizes were measured once, so fonts arriving, renames or unit changes left pills overlapping while the declutter hid others. Sizes are now observed, and a placement pass (larger room first) keeps a pill on its room, nudges it to the nearest free slot within one pill height, collapses it to its colour dot (which also shifts, and expands on hover) and hides it only as a last resort; applied at camera rest with eased style writes. 3D pills render in an isolated layer inside the canvas, the 2D pane inside the plan, so panels always sit above. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RzGbHVg2Tpv4zEw9yLaT5X --- .../src/components/editor/room-labels.test.ts | 295 +++++++--- .../src/components/editor/room-labels.tsx | 521 ++++++++++++------ 2 files changed, 578 insertions(+), 238 deletions(-) diff --git a/packages/editor/src/components/editor/room-labels.test.ts b/packages/editor/src/components/editor/room-labels.test.ts index bdaf5d0c2f..ff7bcd6975 100644 --- a/packages/editor/src/components/editor/room-labels.test.ts +++ b/packages/editor/src/components/editor/room-labels.test.ts @@ -1,11 +1,18 @@ import { describe, expect, test } from 'bun:test' +import { ZoneNode } from '@pascal-app/core/schema' +import type { RoomSelectionRecord } from '../../lib/room-selection' import { DeclutterClock, - declutterLabels, formatRoomArea, - PillFades, + measureFullPill, + PILL_DOT_SIZE, + type PillCandidate, + type PillPlacement, + PillPlacer, pillRect, + placePills, type RoomLabelVisibilityState, + roomLabelEntries, roomLabelsVisible, } from './room-labels' @@ -52,27 +59,141 @@ describe('room label pills', () => { expect(formatRoomArea(10, 'imperial')).toBe('107.6 ft²') }) - test('overlapping pills keep the nearer one; apart, all stay', () => { - const rect = (left: number, top: number) => ({ left, top, right: left + 100, bottom: top + 24 }) - expect( - declutterLabels([ - { id: 'far', distance: 12, rect: rect(40, 10) }, - { id: 'near', distance: 4, rect: rect(0, 0) }, - { id: 'apart', distance: 20, rect: rect(300, 0) }, - { id: 'behind-far', distance: 30, rect: rect(60, 20) }, - ]), - ).toEqual(new Set(['far', 'behind-far'])) - expect( - declutterLabels([ - { id: 'a', distance: 1, rect: rect(0, 0) }, - { id: 'b', distance: 2, rect: rect(0, 40) }, - ]).size, - ).toBe(0) - }) - test('a pill is placed centred on where its anchor lands', () => { expect(pillRect(100, 50, [80, 20])).toEqual({ left: 60, top: 40, right: 140, bottom: 60 }) }) + + test('a collapsed pill measures its updated full label and stays collapsed', () => { + const element = { dataset: { roomLabelCollapsed: '' } } as unknown as HTMLElement + let width = 161.5 + const read = (target: HTMLElement) => ({ + width: 'roomLabelCollapsed' in target.dataset ? '16px' : `${width}px`, + height: 'roomLabelCollapsed' in target.dataset ? '16px' : '26px', + }) + expect(measureFullPill(element, read)).toEqual([161.5, 26]) + width = 176.25 + expect(measureFullPill(element, read)).toEqual([176.25, 26]) + expect(element.dataset.roomLabelCollapsed).toBe('') + }) + + test('invisible rooms and drawn zones do not reserve space for a pill', () => { + const polygon: [number, number][] = [ + [0, 0], + [3, 0], + [3, 3], + [0, 3], + ] + const room = { + zoneId: 'zone_room', + polygon, + holes: [], + area: 9, + } as unknown as RoomSelectionRecord + const zones = [ + ZoneNode.parse({ id: 'zone_room', name: 'Room', polygon, visible: false }), + ZoneNode.parse({ id: 'zone_hidden', name: 'Hidden', polygon, visible: false }), + ZoneNode.parse({ id: 'zone_visible', name: 'Visible', polygon }), + ] + expect(roomLabelEntries([room], zones).map((entry) => entry.zoneId)).toEqual(['zone_visible']) + expect(roomLabelEntries([room], [{ ...zones[0]!, visible: true }])[0]).toEqual({ + zoneId: room.zoneId, + polygon, + holes: [], + area: 9, + }) + }) +}) + +describe('pill placement', () => { + const pill = (id: string, x: number, y: number, area: number, width = 120): PillCandidate => ({ + id, + x, + y, + size: [width, 24], + area, + }) + const rectOf = (candidate: PillCandidate, dy: number, dot: boolean) => + pillRect(candidate.x, candidate.y + dy, dot ? [PILL_DOT_SIZE, PILL_DOT_SIZE] : candidate.size) + const apart = (a: ReturnType, b: ReturnType) => + a.right <= b.left || b.right <= a.left || a.bottom <= b.top || b.bottom <= a.top + + test('apart, every pill stays on its room', () => { + const placements = placePills([pill('a', 100, 100, 20), pill('b', 400, 100, 10)]) + expect([...placements.values()]).toEqual([ + { mode: 'full', dy: 0 }, + { mode: 'full', dy: 0 }, + ]) + }) + + test('two overlapping pills: the larger room keeps its place, the other moves the least it can', () => { + const big = pill('big', 100, 100, 30) + const small = pill('small', 110, 106, 10) + const placements = placePills([small, big]) + expect(placements.get('big')).toEqual({ mode: 'full', dy: 0 }) + const moved = placements.get('small')! + expect(moved.mode).toBe('full') + expect(Math.abs(moved.dy)).toBeLessThanOrEqual(24) + expect(apart(rectOf(big, 0, false), rectOf(small, moved.dy, false))).toBe(true) + // 24 px tall, 6 px apart, 2 px gap: 20 px down is the nearest clear slot (up needs 32). + expect(moved.dy).toBe(20) + }) + + test('crowded beyond a pill height, a pill collapses to its dot; hidden only if the dot collides too', () => { + const wide = pill('wide', 200, 100, 40, 300) + const above = pill('above', 200, 74, 30, 300) + const below = pill('below', 200, 126, 20, 300) + const squeezed = pill('squeezed', 120, 100, 10) + const placements = placePills([squeezed, below, above, wide]) + expect(placements.get('wide')?.mode).toBe('full') + expect(placements.get('above')?.mode).toBe('full') + expect(placements.get('below')?.mode).toBe('full') + // Its dot sits inside the wide pill: nothing left but to hide it. + expect(placements.get('squeezed')).toEqual({ mode: 'hidden', dy: 0 }) + + const nearEdge = pill('near-edge', 30, 100, 10) + const crowded = placePills([nearEdge, below, above, wide]) + // Its full pill still collides, but its dot at the anchor is clear. + expect(crowded.get('near-edge')).toEqual({ mode: 'dot', dy: 0 }) + }) + + test('a dot with no room at its anchor takes the nearest free slot instead of hiding', () => { + const big = pill('big', 100, 100, 30) + const closet = pill('closet', 100, 100, 5) + const placements = placePills([closet, big]) + // The full pill would need 26 px (past its 24 px budget); its dot clears at 24 px up. + expect(placements.get('closet')).toEqual({ mode: 'dot', dy: -24 }) + expect(apart(rectOf(big, 0, false), rectOf(closet, -24, true))).toBe(true) + }) + + test('the order is fixed by room size and id, not by input order', () => { + const candidates = [pill('b', 100, 100, 10), pill('a', 105, 100, 10), pill('c', 110, 100, 10)] + const first = placePills(candidates) + const shuffled = placePills([candidates[2]!, candidates[0]!, candidates[1]!]) + expect(shuffled).toEqual(first) + expect(first.get('a')).toEqual({ mode: 'full', dy: 0 }) + }) + + test('no two placed boxes ever overlap', () => { + const candidates = Array.from({ length: 40 }, (_, i) => + pill( + `p${i}`, + 100 + ((i * 37) % 220), + 100 + ((i * 53) % 140), + (i * 7) % 13, + 90 + (i % 5) * 20, + ), + ) + const placements = placePills(candidates) + const boxes = candidates.flatMap((candidate) => { + const placement = placements.get(candidate.id)! + return placement.mode === 'hidden' + ? [] + : [rectOf(candidate, placement.dy, placement.mode === 'dot')] + }) + for (let i = 0; i < boxes.length; i++) { + for (let j = 0; j < i; j++) expect(apart(boxes[i]!, boxes[j]!)).toBe(true) + } + }) }) describe('when pills re-read their overlaps', () => { @@ -101,34 +222,39 @@ describe('when pills re-read their overlaps', () => { expect(firstRead * frame).toBeLessThan(0.2) }) - test('once per rest, every so often while moving, and at once when the pills change', () => { + test('once per rest, not while moving, and again when the pills change at rest', () => { const clock = new DeclutterClock() const at = (x: number) => { const view = new Float64Array(50) view[12] = x return view } - expect(clock.shouldRead(at(0), 0)).toBe(true) + expect(clock.shouldRead(at(0), 0)).toBe(false) expect(clock.shouldRead(at(0), 0.2)).toBe(true) expect(clock.shouldRead(at(0), 0.4)).toBe(false) - // Moving: a read every 0.3 s, not every frame. - const moving = [0.5, 0.55, 0.6, 0.65, 0.7, 0.75, 0.8, 0.85].map((t) => - clock.shouldRead(at(t), t), - ) - expect(moving.filter(Boolean).length).toBe(2) + // Moving: pills ride their rooms, nothing is re-placed until it rests. + const moving = [0.5, 0.55, 0.6, 0.65, 0.7].map((t) => clock.shouldRead(at(t), t)) + expect(moving.some(Boolean)).toBe(false) + expect(clock.shouldRead(at(0.7), 0.85)).toBe(true) clock.invalidate() - expect(clock.shouldRead(at(0.85), 0.86)).toBe(true) + expect(clock.shouldRead(at(0.7), 0.86)).toBe(true) }) }) -describe('pill fades', () => { +describe('applying placements', () => { function setup() { - const elements = new Map( - ['near', 'far'].map((id) => [id, { dataset: {} as Record } as HTMLElement]), - ) + const element = () => + ({ + dataset: {} as Record, + style: { translate: '' }, + }) as unknown as HTMLElement + const elements = new Map([ + ['near', element()], + ['far', element()], + ]) const timers: Array<{ run: () => void; ms: number; cancelled: boolean }> = [] const changes: ReadonlySet[] = [] - const fades = new PillFades( + const placer = new PillPlacer( elements, (gone) => changes.push(gone), (run, ms) => { @@ -140,47 +266,92 @@ describe('pill fades', () => { ;(timer as unknown as { cancelled: boolean }).cancelled = true }, ) - return { elements, timers, changes, fades } + return { elements, timers, changes, placer } } - const hidden = (element: HTMLElement | undefined) => 'roomLabelHidden' in element!.dataset + const has = (element: HTMLElement | undefined, flag: string) => flag in element!.dataset + const full = (dy = 0) => ({ mode: 'full' as const, dy }) + + test('a nudge is a translate, a dot is a collapsed pill, both undone when placed back', () => { + const { elements, placer } = setup() + placer.apply( + new Map([ + ['near', full(-8)], + ['far', { mode: 'dot' as const, dy: 0 }], + ]), + 180, + ) + expect(elements.get('near')!.style.translate).toBe('0 -8px') + expect(has(elements.get('far'), 'roomLabelCollapsed')).toBe(true) + placer.apply( + new Map([ + ['near', full()], + ['far', full()], + ]), + 180, + ) + expect(elements.get('near')!.style.translate).toBe('') + expect(has(elements.get('far'), 'roomLabelCollapsed')).toBe(false) + }) - test('a pill that steps aside fades out without taking the pointer, then leaves the DOM', () => { - const { elements, timers, changes, fades } = setup() - fades.apply(new Set(['far']), 180) - expect(hidden(elements.get('far'))).toBe(true) - expect(hidden(elements.get('near'))).toBe(false) + test('a hidden pill fades out without taking the pointer, then leaves the DOM', () => { + const { elements, timers, changes, placer } = setup() + placer.apply( + new Map([ + ['near', full()], + ['far', { mode: 'hidden' as const, dy: 0 }], + ]), + 180, + ) + expect(has(elements.get('far'), 'roomLabelHidden')).toBe(true) expect(timers.map(({ ms }) => ms)).toEqual([180]) expect(changes).toEqual([]) timers[0]!.run() - expect(changes.at(-1)).toEqual(new Set(['far'])) - expect(fades.gone).toEqual(new Set(['far'])) + expect(placer.gone).toEqual(new Set(['far'])) }) - test('a pill given room again mounts and fades back in', () => { - const { elements, timers, fades } = setup() - fades.apply(new Set(['far']), 180) + test('a pill given room again mounts in its new place, or turns back mid-fade', () => { + const { elements, timers, placer } = setup() + placer.apply(new Map([['far', { mode: 'hidden' as const, dy: 0 }]]), 180) timers[0]!.run() + const element = elements.get('far')! elements.delete('far') - fades.apply(new Set(), 180) - expect(fades.gone).toEqual(new Set()) + placer.apply(new Map([['far', full(12)]]), 180) + expect(placer.gone).toEqual(new Set()) + placer.mount('far', element) + expect(element.style.translate).toBe('0 12px') + + const again = setup() + again.placer.apply( + new Map([['far', { mode: 'hidden' as const, dy: 0 }]]), + 180, + ) + again.placer.apply(new Map([['far', full()]]), 180) + expect(again.timers[0]!.cancelled).toBe(true) + expect(has(again.elements.get('far'), 'roomLabelHidden')).toBe(false) }) - test('a pill given room mid-fade turns back without leaving', () => { - const { elements, timers, changes, fades } = setup() - fades.apply(new Set(['far']), 180) - fades.apply(new Set(), 180) - expect(timers[0]!.cancelled).toBe(true) - expect(hidden(elements.get('far'))).toBe(false) - expect(changes).toEqual([]) + test('hidden pills can refresh their text measurements without becoming visible', () => { + const { elements, timers, placer } = setup() + placer.apply(new Map([['far', { mode: 'hidden', dy: 0 }]]), 180) + timers[0]!.run() + elements.delete('far') + placer.remeasure() + expect(placer.gone.size).toBe(0) + const fresh = { dataset: {}, style: { translate: '' } } as unknown as HTMLElement + elements.set('far', fresh) + placer.mount('far', fresh) + expect(has(fresh, 'roomLabelHidden')).toBe(true) + placer.apply(new Map([['far', full()]]), 180) + expect(has(fresh, 'roomLabelHidden')).toBe(false) }) - test('the whole layer fades out and back in, except pills still stepping aside', () => { - const { elements, fades } = setup() - fades.apply(new Set(['far']), 180) - fades.showAll(false) - expect(hidden(elements.get('near'))).toBe(true) - fades.showAll(true) - expect(hidden(elements.get('near'))).toBe(false) - expect(hidden(elements.get('far'))).toBe(true) + test('the whole layer fades out and back in, except pills still hidden', () => { + const { elements, placer } = setup() + placer.apply(new Map([['far', { mode: 'hidden' as const, dy: 0 }]]), 180) + placer.showAll(false) + expect(has(elements.get('near'), 'roomLabelHidden')).toBe(true) + placer.showAll(true) + expect(has(elements.get('near'), 'roomLabelHidden')).toBe(false) + expect(has(elements.get('far'), 'roomLabelHidden')).toBe(true) }) }) diff --git a/packages/editor/src/components/editor/room-labels.tsx b/packages/editor/src/components/editor/room-labels.tsx index 5bf8a549e5..e1474cfd39 100644 --- a/packages/editor/src/components/editor/room-labels.tsx +++ b/packages/editor/src/components/editor/room-labels.tsx @@ -13,14 +13,17 @@ import { } from '@pascal-app/core' import { useViewer } from '@pascal-app/viewer' import { Html } from '@react-three/drei' -import { useFrame } from '@react-three/fiber' +import { useFrame, useThree } from '@react-three/fiber' import { + createContext, Fragment, type MouseEvent as ReactMouseEvent, type ReactNode, type PointerEvent as ReactPointerEvent, useCallback, + useContext, useEffect, + useLayoutEffect, useMemo, useRef, useState, @@ -119,33 +122,82 @@ function useRoomLabelsVisible() { } type LabelRect = { left: number; top: number; right: number; bottom: number } -type ScreenLabel = { id: string; distance: number; rect: LabelRect } -/** - * Which pills step aside so none overlap: nearest first, each pill that - * overlaps one already kept is hidden. Returns the hidden ids. - */ -export function declutterLabels(labels: readonly ScreenLabel[], gap = 2): Set { - const kept: LabelRect[] = [] - const hidden = new Set() - for (const label of [...labels].sort((a, b) => a.distance - b.distance)) { - const { rect } = label - const overlaps = kept.some( - (other) => - rect.left < other.right + gap && - other.left < rect.right + gap && - rect.top < other.bottom + gap && - other.top < rect.bottom + gap, - ) - if (overlaps) hidden.add(label.id) - else kept.push(rect) +/** A pill's screen rect: centred on where its anchor lands, at its size. */ +export function pillRect(x: number, y: number, [width, height]: readonly [number, number]) { + return { left: x - width / 2, top: y - height / 2, right: x + width / 2, bottom: y + height / 2 } +} + +/** A pill to place: where its room's anchor lands on screen, its full size and its room's area. */ +export type PillCandidate = { + id: string + x: number + y: number + size: readonly [number, number] + area: number +} + +/** Where a pill goes: in full (moved `dy` px off its anchor), as its colour dot, or nowhere. */ +export type PillPlacement = { mode: 'full' | 'dot' | 'hidden'; dy: number } + +/** The collapsed pill: its colour dot in a small round box. */ +export const PILL_DOT_SIZE = 16 +const PILL_GAP = 2 +const NUDGE_STEP = 4 + +function overlaps(a: LabelRect, b: LabelRect) { + return ( + a.left < b.right + PILL_GAP && + b.left < a.right + PILL_GAP && + a.top < b.bottom + PILL_GAP && + b.top < a.bottom + PILL_GAP + ) +} + +/** 0, then up and down by growing steps, up to `budget` px either way. */ +function* nudges(budget: number) { + yield 0 + for (let d = NUDGE_STEP; d <= budget; d += NUDGE_STEP) { + yield -d + yield d } - return hidden } -/** A pill's screen rect: centred on where its anchor lands, at its measured size. */ -export function pillRect(x: number, y: number, [width, height]: readonly [number, number]) { - return { left: x - width / 2, top: y - height / 2, right: x + width / 2, bottom: y + height / 2 } +/** + * Where every pill goes so none overlap. Larger rooms are placed first (ties + * by id, so the order is stable while the camera moves). A pill that overlaps + * one already placed takes the nearest free slot above or below its room, + * within one pill height; failing that it collapses to its colour dot, which + * takes the nearest free slot the same way; only a dot with no free slot is + * hidden. + */ +export function placePills(candidates: readonly PillCandidate[]): Map { + const placed: LabelRect[] = [] + const placements = new Map() + const order = [...candidates].sort( + (a, b) => b.area - a.area || (a.id < b.id ? -1 : a.id > b.id ? 1 : 0), + ) + for (const pill of order) { + let placement: PillPlacement = { mode: 'hidden', dy: 0 } + for (const dy of nudges(pill.size[1])) { + const rect = pillRect(pill.x, pill.y + dy, pill.size) + if (placed.some((other) => overlaps(rect, other))) continue + placed.push(rect) + placement = { mode: 'full', dy } + break + } + if (placement.mode === 'hidden') { + for (const dy of nudges(pill.size[1])) { + const dot = pillRect(pill.x, pill.y + dy, [PILL_DOT_SIZE, PILL_DOT_SIZE]) + if (placed.some((other) => overlaps(dot, other))) continue + placed.push(dot) + placement = { mode: 'dot', dy } + break + } + } + placements.set(pill.id, placement) + } + return placements } export type RoomLabelEntry = { @@ -158,13 +210,12 @@ type LabelEntry = RoomLabelEntry // Mid-height of a default storey: above furniture, below the ceiling. const LABEL_HEIGHT = DEFAULT_LEVEL_HEIGHT / 2 -// The view has to hold still this long before overlaps are re-read… +// The view has to hold still this long before pills are re-placed; while it +// moves they keep their places and ride their rooms, so nothing jitters. const DECLUTTER_REST_SECONDS = 0.12 -// …and while it moves they are re-read this often. -const DECLUTTER_MOVING_SECONDS = 0.3 // Below this the view counts as still: well under a pixel on screen. const VIEW_EPSILON = 1e-4 -/** How long a pill takes to fade in or out. */ +/** How long a pill takes to fade, move or collapse. */ export const PILL_FADE_MS = 180 const labelWorld = new Vector3() @@ -177,26 +228,24 @@ function pillFadeMs(): number { } /** - * When a layer re-reads overlaps: once the view has held still, every so often - * while it moves, and as soon as its pills change. Still means within an - * epsilon of the view last seen moving — camera damping keeps nudging the - * camera by fractions of a millimetre for a second or more after it visibly - * stops, and waiting for it to stop exactly left pills hidden, with nothing - * overlapping them, for that long. + * When a layer re-places its pills: once the view has held still, and again + * whenever its pills change while it rests. Still means within an epsilon of + * the view last seen moving — camera damping keeps nudging the camera by + * fractions of a millimetre for a second or more after it visibly stops, and + * waiting for it to stop exactly left pills misplaced for that long. */ export class DeclutterClock { private reference: number[] | null = null private movedAt = Number.NEGATIVE_INFINITY - private readAt = Number.NEGATIVE_INFINITY private settled = false private dirty = true - /** The pills changed (mounted, unmounted, resized): read again now. */ + /** The pills changed (mounted, unmounted, resized): place them again. */ invalidate() { this.dirty = true } - /** Whether to read overlaps for this view (any flat list of numbers) at `now` seconds. */ + /** Whether to place pills for this view (any flat list of numbers) at `now` seconds. */ shouldRead(view: ArrayLike, now: number): boolean { const reference = this.reference let moved = !reference || reference.length !== view.length @@ -208,49 +257,64 @@ export class DeclutterClock { this.movedAt = now this.settled = false } - const resting = now - this.movedAt >= DECLUTTER_REST_SECONDS - const due = resting - ? !this.settled || this.dirty - : this.dirty || now - this.readAt >= DECLUTTER_MOVING_SECONDS - if (!due) return false - this.settled = resting + if (now - this.movedAt < DECLUTTER_REST_SECONDS || (this.settled && !this.dirty)) return false + this.settled = true this.dirty = false - this.readAt = now return true } } -/** - * The pills a layer has mounted, by zone id. A pill coming or going tells the - * layer to re-read overlaps; while the layer fades out, one mounting joins it. - */ +/** The pills a layer has mounted, by zone id; the layer hears each one come and go. */ export class PillElements extends Map { - onChange: (() => void) | null = null - hideAll = false + onMount: ((id: string, element: HTMLElement) => void) | null = null + onUnmount: ((element: HTMLElement) => void) | null = null + onMeasure: ((id: string, element: HTMLElement) => void) | null = null + + measure(id: string) { + const element = this.get(id) + if (element) this.onMeasure?.(id, element) + } override set(id: string, element: HTMLElement) { - if (this.hideAll) element.dataset.roomLabelHidden = '' super.set(id, element) - this.onChange?.() + this.onMount?.(id, element) return this } override delete(id: string) { + const element = this.get(id) const removed = super.delete(id) - if (removed) this.onChange?.() + if (element) this.onUnmount?.(element) return removed } } +/** Read the full border-box in CSS pixels, even when the dot is showing. */ +export function measureFullPill( + element: HTMLElement, + read: (element: HTMLElement) => Pick = getComputedStyle, +): readonly [number, number] { + const collapsed = element.dataset.roomLabelCollapsed + delete element.dataset.roomLabelCollapsed + try { + const style = read(element) + return [Number.parseFloat(style.width), Number.parseFloat(style.height)] + } finally { + if (collapsed !== undefined) element.dataset.roomLabelCollapsed = collapsed + } +} + type Timer = ReturnType /** - * Pills the declutter hides fade out, then leave the DOM (`gone`); one given - * room again is dropped from `gone`, mounts and fades in. A hidden pill takes - * no pointer from the moment it starts to fade. + * Applies placements to the mounted pills as direct style writes: the nudge + * as a `translate`, the dot as `data-room-label-collapsed`, both eased by the + * pill's transition. A hidden pill fades out without taking the pointer, then + * leaves the DOM (`gone`); one given room again mounts and fades back in. */ -export class PillFades { +export class PillPlacer { gone: ReadonlySet = new Set() + private placements = new Map() private readonly fading = new Map() constructor( @@ -260,27 +324,46 @@ export class PillFades { private readonly cancel: (timer: Timer) => void = clearTimeout, ) {} - apply(hidden: ReadonlySet, fadeMs = pillFadeMs()) { + apply(placements: ReadonlyMap, fadeMs = pillFadeMs()) { + this.placements = new Map([...this.placements, ...placements]) for (const [id, element] of this.elements) { - const timer = this.fading.get(id) - if (hidden.has(id)) { - if (timer !== undefined) continue - element.dataset.roomLabelHidden = '' - this.fading.set( - id, - this.schedule(() => { - this.fading.delete(id) - this.setGone(new Set(this.gone).add(id)) - }, fadeMs), - ) - } else { + const placement = placements.get(id) + if (!placement) continue + if (placement.mode !== 'hidden') { + const timer = this.fading.get(id) if (timer !== undefined) this.cancel(timer) this.fading.delete(id) delete element.dataset.roomLabelHidden + placePill(element, placement) + continue } + if (this.fading.has(id)) continue + element.dataset.roomLabelHidden = '' + this.fading.set( + id, + this.schedule(() => { + this.fading.delete(id) + this.setGone(new Set(this.gone).add(id)) + }, fadeMs), + ) } - const back = [...this.gone].filter((id) => !hidden.has(id)) - if (back.length > 0) this.setGone(new Set([...this.gone].filter((id) => hidden.has(id)))) + const kept = [...this.gone].filter((id) => { + const placement = placements.get(id) + return !placement || placement.mode === 'hidden' + }) + if (kept.length !== this.gone.size) this.setGone(new Set(kept)) + } + + /** A pill mounting takes its last placement at once. */ + mount(id: string, element: HTMLElement) { + const placement = this.placements.get(id) + if (placement?.mode === 'hidden') element.dataset.roomLabelHidden = '' + else if (placement) placePill(element, placement) + } + + /** Remount hidden pills invisibly so changed text or fonts can be measured. */ + remeasure() { + if (this.gone.size) this.setGone(new Set()) } /** Fade every mounted pill out (the layer is leaving), or back in. */ @@ -302,6 +385,12 @@ export class PillFades { } } +function placePill(element: HTMLElement, placement: PillPlacement) { + element.style.translate = placement.dy ? `0 ${placement.dy}px` : '' + if (placement.mode === 'dot') element.dataset.roomLabelCollapsed = '' + else delete element.dataset.roomLabelCollapsed +} + /** True while `shown`, and through the fade once it turns false; then false, so the pills unmount. */ export function useFadePresence(shown: boolean, onGone?: () => void): boolean { const [present, setPresent] = useState(shown) @@ -341,38 +430,86 @@ function useFadingLevels(levelId: string | null, visible: boolean) { } /** - * The declutter state of one layer: its mounted pills, their last measured - * sizes (kept for pills that have left the DOM), and their fades. + * The placement state of one layer: its mounted pills, the full size each + * last rendered at (observed, so a web font arriving, a rename or a unit + * change re-places them; kept for pills that have left the DOM), and the + * placer that moves, collapses and fades them. */ -function usePillDeclutter(shown: boolean) { +function usePillPlacement(shown: boolean, entries: readonly LabelEntry[]) { const [elements] = useState(() => new PillElements()) const [clock] = useState(() => new DeclutterClock()) const [gone, setGone] = useState>(() => new Set()) - const [fades] = useState(() => new PillFades(elements, setGone)) + const [placer] = useState(() => new PillPlacer(elements, setGone)) + const unit = useViewer((s) => s.unit) const sizes = useRef(new Map()) + const shownRef = useRef(shown) + shownRef.current = shown useEffect(() => { - elements.onChange = () => clock.invalidate() + elements.onMeasure = (id, element) => { + const size = measureFullPill(element) + if (!(size[0] > 0 && size[1] > 0)) return + const previous = sizes.current.get(id) + if ( + previous && + Math.abs(previous[0] - size[0]) < 0.01 && + Math.abs(previous[1] - size[1]) < 0.01 + ) + return + sizes.current.set(id, size) + clock.invalidate() + } + const observer = + typeof ResizeObserver === 'undefined' + ? null + : new ResizeObserver((records) => { + for (const record of records) { + const element = record.target as HTMLElement + const id = element.dataset.roomLabel + const box = record.borderBoxSize?.[0] + if (!(id && box && box.inlineSize > 0)) continue + // A dot coming back on screen may have changed text while its + // wrapper was display:none, when its full width was unreadable. + if ('roomLabelCollapsed' in element.dataset) elements.measure(id) + else sizes.current.set(id, [box.inlineSize, box.blockSize]) + } + clock.invalidate() + }) + elements.onMount = (id, element) => { + if (!shownRef.current) element.dataset.roomLabelHidden = '' + placer.mount(id, element) + elements.measure(id) + observer?.observe(element) + clock.invalidate() + } + elements.onUnmount = (element) => { + observer?.unobserve(element) + clock.invalidate() + } + for (const [id, element] of elements) elements.onMount(id, element) + const fontsChanged = () => { + placer.remeasure() + for (const id of elements.keys()) elements.measure(id) + } + document.fonts?.addEventListener('loadingdone', fontsChanged) return () => { - elements.onChange = null - fades.dispose() + elements.onMount = null + elements.onUnmount = null + elements.onMeasure = null + document.fonts?.removeEventListener('loadingdone', fontsChanged) + observer?.disconnect() + placer.dispose() } - }, [clock, elements, fades]) + }, [clock, elements, placer]) + // biome-ignore lint/correctness/useExhaustiveDependencies: Hidden pills must remount to measure changed entries or units. + useEffect(() => { + placer.remeasure() + clock.invalidate() + }, [clock, entries, placer, unit]) useEffect(() => { - elements.hideAll = !shown - fades.showAll(shown) + placer.showAll(shown) if (shown) clock.invalidate() - }, [clock, elements, fades, shown]) - /** Measure the mounted pills; returns the size of each pill to place. */ - const measure = useCallback(() => { - for (const [id, element] of elements) { - // A pill out of view (its wrapper not displayed) keeps its last size. - if (element.offsetWidth > 0) { - sizes.current.set(id, [element.offsetWidth, element.offsetHeight]) - } - } - return sizes.current - }, [elements]) - return { elements, clock, gone, fades, measure } + }, [clock, placer, shown]) + return { elements, clock, gone, placer, sizes: sizes.current } } /** The pills of one level: every detected room, and every drawn zone that bounds none. */ @@ -380,16 +517,21 @@ export function roomLabelEntries( rooms: readonly RoomSelectionRecord[], zones: readonly ZoneNode[], ): RoomLabelEntry[] { + const hiddenIds = new Set( + zones.filter((zone) => zone.visible === false).map((zone) => zone.id), + ) const roomIds = new Set(rooms.map((room) => room.zoneId)) return [ - ...rooms.map((room) => ({ - zoneId: room.zoneId, - polygon: room.polygon, - holes: room.holes, - area: room.area, - })), + ...rooms + .filter((room) => !hiddenIds.has(room.zoneId)) + .map((room) => ({ + zoneId: room.zoneId, + polygon: room.polygon, + holes: room.holes, + area: room.area, + })), ...zones - .filter((zone) => !roomIds.has(zone.id) && zone.polygon.length >= 3) + .filter((zone) => zone.visible !== false && !roomIds.has(zone.id) && zone.polygon.length >= 3) .map((zone) => ({ zoneId: zone.id, polygon: zone.polygon, @@ -436,6 +578,10 @@ export function RoomPillChip({ onLeave: () => void }) { const unit = useViewer((s) => s.unit) + // biome-ignore lint/correctness/useExhaustiveDependencies: A dot's border box does not resize when its hidden text changes. + useLayoutEffect(() => { + if (elements instanceof PillElements) elements.measure(zoneId) + }, [areaM2, elements, name, unit, zoneId]) const register = useCallback( (element: HTMLDivElement | null) => { if (element) elements?.set(zoneId, element) @@ -445,8 +591,9 @@ export function RoomPillChip({ ) return (
{ @@ -462,8 +609,10 @@ export function RoomPillChip({ ref={register} > - {name} - {formatRoomArea(areaM2, unit)} + {name} + + {formatRoomArea(areaM2, unit)} +
) } @@ -556,9 +705,9 @@ function EditorRoomLabel3D({ /** * The pills of one level in 3D, riding the level's object (so they follow - * level stacking), decluttered from where their anchors land on screen. A pill - * that steps aside, or the whole layer when `shown` turns false, fades out; a - * hidden pill then leaves the DOM. + * level stacking) and placed from where their anchors land on screen. They + * live in their own stacking layer clipped to the canvas, so every piece of + * UI over the canvas stays above them. */ export function RoomPillsLayer3D({ entries, @@ -575,18 +724,27 @@ export function RoomPillsLayer3D({ ) => ReactNode shown?: boolean }) { - const unit = useViewer((s) => s.unit) const ref = useRef(null) const anchors = useRef(new Map()) const view = useRef(new Float64Array(50)) - const { elements, clock, gone, fades, measure } = usePillDeclutter(shown) + const { elements, clock, gone, placer, sizes } = usePillPlacement(shown, entries) + const host = useThree((s) => (s.events.connected ?? s.gl.domElement.parentNode) as HTMLElement) + const [container] = useState(() => + typeof document === 'undefined' ? null : document.createElement('div'), + ) + const portal = useMemo(() => (container ? { current: container } : null), [container]) + useEffect(() => { + if (!(host && container)) return + container.className = 'pointer-events-none absolute inset-0 isolate z-0 overflow-hidden' + host.appendChild(container) + return () => container.remove() + }, [container, host]) - // biome-ignore lint/correctness/useExhaustiveDependencies: a new set of pills or unit re-reads overlaps. useEffect(() => { const ids = new Set(entries.map((entry) => entry.zoneId)) for (const id of anchors.current.keys()) if (!ids.has(id)) anchors.current.delete(id) clock.invalidate() - }, [entries, unit]) + }, [clock, entries]) useFrame(({ camera, clock: frameClock, size }) => { if (!ref.current) return @@ -604,42 +762,42 @@ export function RoomPillsLayer3D({ flat[49] = size.height if (!(shown && clock.shouldRead(flat, frameClock.elapsedTime))) return - const sizes = measure() - const labels: ScreenLabel[] = [] + const candidates: PillCandidate[] = [] for (const entry of entries) { const anchor = anchors.current.get(entry.zoneId) const pill = sizes.get(entry.zoneId) if (!(anchor && pill)) continue - labelWorld.copy(anchor).applyMatrix4(level.matrixWorld) - const distance = labelWorld.distanceTo(camera.position) - labelWorld.project(camera) - // Behind the camera or past its far plane: not on screen to overlap anything. + labelWorld.copy(anchor).applyMatrix4(level.matrixWorld).project(camera) + // Behind the camera or past its far plane: not on screen to collide. if (Math.abs(labelWorld.z) > 1) continue - labels.push({ + candidates.push({ id: entry.zoneId, - distance, - rect: pillRect( - ((labelWorld.x + 1) / 2) * size.width, - ((1 - labelWorld.y) / 2) * size.height, - pill, - ), + x: ((labelWorld.x + 1) / 2) * size.width, + y: ((1 - labelWorld.y) / 2) * size.height, + size: pill, + area: entry.area, }) } - fades.apply(declutterLabels(labels)) + placer.apply(placePills(candidates)) }) - if (entries.length === 0) return null + if (entries.length === 0 || !portal) return null return ( - - {entries - .filter((entry) => !gone.has(entry.zoneId)) - .map((entry) => ( - {renderLabel(entry, anchors.current, elements)} - ))} - + + + + {entries.map((entry) => ( + {renderLabel(entry, anchors.current, elements)} + ))} + + + ) } +const PillPortalContext = createContext<{ current: HTMLElement } | null>(null) +const HiddenPillsContext = createContext>(new Set()) + /** Where a room's pill floats: over its interior, at mid-storey above its floor. */ export function RoomLabelAnchor3D({ entry, @@ -652,18 +810,29 @@ export function RoomLabelAnchor3D({ anchors: Map children: ReactNode }) { + const portal = useContext(PillPortalContext) + const gone = useContext(HiddenPillsContext) const position = useMemo(() => { const [x, z] = polygonInteriorPoint({ polygon: entry.polygon, holes: entry.holes }) return new Vector3(x, floorY + LABEL_HEIGHT, z) }, [entry.polygon, entry.holes, floorY]) - // Kept after unmount: a pill that stepped aside is placed from its anchor to - // learn when it has room again. The layer drops anchors of rooms that left. + // Hidden HTML still needs a live anchor so moving a room can give its pill + // room again. The layer drops anchors of rooms that left. useEffect(() => { anchors.set(entry.zoneId, position) }, [anchors, entry.zoneId, position]) + if (gone.has(entry.zoneId)) return null return ( - // The wrapper lets the canvas keep everything but the pill itself. - + {children} ) @@ -671,8 +840,8 @@ export function RoomLabelAnchor3D({ /** * The same pills over the 2D plan: screen-positioned from the plan's transform, - * re-placed only when the plan pans or zooms, decluttered as in 3D (bigger - * rooms keep theirs), and clipped to the plan's pane. + * re-placed as in 3D once the plan rests, and clipped to the plan's pane in the + * plan's own stacking layer, under the UI over it. */ export function RoomLabels2D({ levelId }: { levelId: string | null }) { const planShown = useEditor((s) => s.viewMode !== '3d') @@ -694,11 +863,11 @@ function LevelRoomLabels2D({ }) { const present = useFadePresence(shown, () => onGone(levelId)) const entries = useRoomLabelEntries(present ? levelId : null) - const unit = useViewer((s) => s.unit) const ref = useRef(null) const paneRef = useRef(null) + const [host, setHost] = useState(null) const [nodes] = useState(() => new PillElements()) - const { elements, clock, gone, fades, measure } = usePillDeclutter(shown) + const { elements, clock, gone, placer, sizes } = usePillPlacement(shown, entries) const shownRef = useRef(shown) shownRef.current = shown const anchors = useMemo( @@ -712,14 +881,17 @@ function LevelRoomLabels2D({ [entries], ) - // biome-ignore lint/correctness/useExhaustiveDependencies: changing units resizes the pills. + useEffect(() => { + setHost(ref.current?.ownerSVGElement?.parentElement ?? null) + }, []) + useEffect(() => { if (!present || entries.length === 0) return let raf = 0 let lastKey = '' const view = new Float64Array(10) // A pill wrapper that mounts is placed on the next frame. - nodes.onChange = () => { + nodes.onMount = () => { lastKey = '' } clock.invalidate() @@ -728,9 +900,11 @@ function LevelRoomLabels2D({ const group = ref.current const svg = group?.ownerSVGElement const ctm = group?.getScreenCTM() - if (!(svg && ctm)) return + const paneElement = paneRef.current + if (!(svg && ctm && paneElement)) return const pane = svg.getBoundingClientRect() - const key = `${ctm.a},${ctm.b},${ctm.c},${ctm.d},${ctm.e},${ctm.f},${pane.left},${pane.top},${pane.width},${pane.height}` + const origin = paneElement.offsetParent?.getBoundingClientRect() ?? { left: 0, top: 0 } + const key = `${ctm.a},${ctm.b},${ctm.c},${ctm.d},${ctm.e},${ctm.f},${pane.left},${pane.top},${pane.width},${pane.height},${origin.left},${origin.top}` const point = svg.createSVGPoint() const place = (anchor: [number, number]) => { point.x = anchor[0] @@ -742,14 +916,12 @@ function LevelRoomLabels2D({ x >= 0 && x <= pane.width && y >= 0 && y <= pane.height if (key !== lastKey) { lastKey = key - if (paneRef.current) { - Object.assign(paneRef.current.style, { - left: `${pane.left}px`, - top: `${pane.top}px`, - width: `${pane.width}px`, - height: `${pane.height}px`, - }) - } + Object.assign(paneElement.style, { + left: `${pane.left - origin.left}px`, + top: `${pane.top - origin.top}px`, + width: `${pane.width}px`, + height: `${pane.height}px`, + }) for (const [id, element] of nodes) { const anchor = anchors.get(id) if (!anchor) continue @@ -759,49 +931,46 @@ function LevelRoomLabels2D({ element.style.display = inside(at) ? '' : 'none' } } - view.set([ - ctm.a, - ctm.b, - ctm.c, - ctm.d, - ctm.e, - ctm.f, - pane.left, - pane.top, - pane.width, - pane.height, - ]) + view[0] = ctm.a + view[1] = ctm.b + view[2] = ctm.c + view[3] = ctm.d + view[4] = ctm.e + view[5] = ctm.f + view[6] = pane.left + view[7] = pane.top + view[8] = pane.width + view[9] = pane.height if (!(shownRef.current && clock.shouldRead(view, now / 1000))) return - const sizes = measure() - const labels: ScreenLabel[] = [] + const candidates: PillCandidate[] = [] for (const entry of entries) { const anchor = anchors.get(entry.zoneId) const pill = sizes.get(entry.zoneId) if (!(anchor && pill)) continue const at = place(anchor) if (!inside(at)) continue - labels.push({ id: entry.zoneId, distance: -entry.area, rect: pillRect(at[0], at[1], pill) }) + candidates.push({ id: entry.zoneId, x: at[0], y: at[1], size: pill, area: entry.area }) } - fades.apply(declutterLabels(labels)) + placer.apply(placePills(candidates)) } raf = requestAnimationFrame(tick) return () => { cancelAnimationFrame(raf) - nodes.onChange = null + nodes.onMount = null } - }, [present, entries, anchors, unit]) + }, [anchors, clock, entries, nodes, placer, present, sizes]) return ( {present && - typeof document !== 'undefined' && + host && createPortal( -
+
{entries .filter((entry) => !gone.has(entry.zoneId)) .map((entry) => (
{ if (element) nodes.set(entry.zoneId, element) @@ -813,7 +982,7 @@ function LevelRoomLabels2D({
))}
, - document.body, + host, )} )