From 2adb50a340d32aa47a63e94590bef3c545f6beab Mon Sep 17 00:00:00 2001 From: Wassim SAMAD Date: Wed, 22 Jul 2026 09:46:06 -0400 Subject: [PATCH] editor: stop wall endpoint drag snapping back to the stale junction corner (#533) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(wall): stop endpoint drag snapping back to the stale junction corner Walls attached to the moving corner cascade with the drag, but the snap pipeline reads the scene store, which keeps their pre-drag coordinates until commit. Their stale corners recreated the old junction as a snap / alignment target, so inside the connect radius (5cm, 70cm magnetic) the endpoint could never land closer than that to where it started — sub-5cm corrections (e.g. squaring a scan-imported 91° junction) were impossible in every snapping mode. Both endpoint-move paths (3D tool + 2D floorplan affordance) now exclude the walls linked at the moving corner from snap candidates and alignment anchors while attached; Alt-detach keeps them (they stay put, so they're live geometry). The 2D path previously over-excluded: walls linked at the FIXED corner don't move, and their anchors are exactly what lets the dragged corner align back onto a true axis. Co-Authored-By: Claude Fable 5 * fix(wall): re-run full snap pipeline on Alt toggle during endpoint drag The snap/alignment candidate set now depends on Alt (stale-junction exclusion), so a keyboard detach/re-attach must re-resolve from the raw cursor point instead of re-applying the previously snapped one — matching what the 2D dispatcher already does by re-invoking apply() with the raw planPoint on modifier changes. Flagged by Bugbot on #533. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Claude Fable 5 --- .../tools/wall/wall-drafting.test.ts | 31 +++++++ .../nodes/src/wall/floorplan-affordances.ts | 31 +++++-- .../nodes/src/wall/move-endpoint-tool.tsx | 81 ++++++++++++++----- 3 files changed, 113 insertions(+), 30 deletions(-) diff --git a/packages/editor/src/components/tools/wall/wall-drafting.test.ts b/packages/editor/src/components/tools/wall/wall-drafting.test.ts index b9a22d71..611d3c8f 100644 --- a/packages/editor/src/components/tools/wall/wall-drafting.test.ts +++ b/packages/editor/src/components/tools/wall/wall-drafting.test.ts @@ -327,4 +327,35 @@ describe('snapWallDraftPointDetailed', () => { expect(result.point).toEqual([3.99, 0.03]) expect(result.snap).toBeNull() }) + + // Endpoint-move regression: walls attached to the moving corner keep their + // pre-drag coordinates in the scene during the drag, so their stale corner + // recreates the old junction inside the connect radius. The move tools must + // pass those walls in `ignoreWallIds` (attached mode) or a sub-5cm corner + // correction — e.g. squaring a scan-imported 91° junction — can never land. + test('a stale linked-wall corner swallows a sub-connect-radius correction unless ignored', () => { + // `wall_d` shares the dragged corner of `wall_c` at [2, 0.03]; the user + // drops 3cm away at [2, 0] to square the junction. + const linked = makeWall([2, 0.03], [2, 2], 'wall_d') + + const captured = snapWallDraftPointDetailed({ + point: [2, 0], + walls: [linked], + ignoreWallIds: ['wall_c'], + magnetic: false, + step: 0, + }) + expect(captured.point).toEqual([2, 0.03]) + expect(captured.snap).toBe('endpoint') + + const freed = snapWallDraftPointDetailed({ + point: [2, 0], + walls: [linked], + ignoreWallIds: ['wall_c', 'wall_d'], + magnetic: false, + step: 0, + }) + expect(freed.point).toEqual([2, 0]) + expect(freed.snap).toBeNull() + }) }) diff --git a/packages/nodes/src/wall/floorplan-affordances.ts b/packages/nodes/src/wall/floorplan-affordances.ts index 4bb0c18d..86dd79df 100644 --- a/packages/nodes/src/wall/floorplan-affordances.ts +++ b/packages/nodes/src/wall/floorplan-affordances.ts @@ -169,6 +169,18 @@ export const wallMoveEndpointAffordance: FloorplanAffordance = { const originalEnd: WallPlanPoint = [...node.end] as WallPlanPoint const linkedWalls = collectLinkedWalls(nodes, node.id, originalStart, originalEnd) const affectedIds: AnyNodeId[] = [node.id, ...linkedWalls.map((w) => w.id)] + const movingOriginal: WallPlanPoint = endpoint === 'start' ? originalStart : originalEnd + // Walls attached to the MOVING corner cascade with the drag, but the snap + // pipeline reads the scene store, which keeps their pre-drag coordinates + // until commit. Their stale corners would recreate the old junction as a + // snap/alignment target: inside the connect radius the endpoint could + // never land closer than ~5cm to where it started, making sub-5cm + // corrections (e.g. squaring a scan-imported 91° corner) impossible. + // Excluded while attached; under Alt-detach they stay put and remain + // legitimate targets. Mirrors the 3D move-endpoint tool. + const movingLinkedWallIds = linkedWalls + .filter((w) => pointsEqual(w.start, movingOriginal) || pointsEqual(w.end, movingOriginal)) + .map((w) => w.id) // Remember the latest preview so `commit()` can write it tracked. let lastPrimaryStart: WallPlanPoint = originalStart @@ -181,11 +193,12 @@ export const wallMoveEndpointAffordance: FloorplanAffordance = { // Re-collect walls every tick so the snap pipeline sees fresh // positions (matters when the user releases + re-grabs without // unmounting the layer). Snap reads from scene — which holds - // the pre-drag positions throughout — so the linked-wall snap - // targets stay anchored to where corners *were*, exactly like - // the legacy flow. + // the pre-drag positions throughout — so walls that cascade with + // the moving corner are excluded (stale coordinates); under + // Alt-detach they stay put, so they rejoin the candidate pool. const sceneNodes = useScene.getState().nodes const walls = collectLevelWalls(sceneNodes, node.id) + const staleWallIds = modifiers.altKey ? [node.id] : [node.id, ...movingLinkedWallIds] // The grid step follows the active snapping mode (`getSegmentGridStep()` // is 0 outside grid mode), so `'lines' / 'angles' / 'off'` no longer // force a grid snap the mode chip says is inactive. In `'angles'` mode @@ -195,7 +208,7 @@ export const wallMoveEndpointAffordance: FloorplanAffordance = { const snapped = snapWallDraftPoint({ point: planPoint as WallPlanPoint, walls, - ignoreWallIds: [node.id], + ignoreWallIds: staleWallIds, start: angleLocked ? fixedPoint : undefined, angleSnap: angleLocked, magnetic: isMagneticSnapActive(), @@ -205,13 +218,15 @@ export const wallMoveEndpointAffordance: FloorplanAffordance = { // object's edge / wall face and publishes a guide. The guide is // DISPLAYED in every mode except Off (isAlignmentGuideActive); the // magnetic pull onto it is applied only in 'lines' mode - // (isMagneticSnapActive), like the draft tool does. The dragged wall and - // its linked siblings (which cascade with the corner) are excluded from - // the candidate pool. Alt is detach, NOT bypass. + // (isMagneticSnapActive), like the draft tool does. Only the dragged + // wall and the siblings cascading with the moving corner are excluded + // from the candidate pool — walls linked at the FIXED corner don't + // move, and their anchors are what let the dragged corner align back + // onto a true axis. Alt is detach, NOT bypass. const aligned = alignFloorplanDraftPoint(snapped, { applySnap: isMagneticSnapActive(), bypass: !isAlignmentGuideActive(), - excludeIds: [node.id, ...linkedWalls.map((w) => w.id)], + excludeIds: staleWallIds, }) as WallPlanPoint const primaryStart: WallPlanPoint = endpoint === 'start' ? aligned : fixedPoint diff --git a/packages/nodes/src/wall/move-endpoint-tool.tsx b/packages/nodes/src/wall/move-endpoint-tool.tsx index f8bcc86b..d81d8d8d 100644 --- a/packages/nodes/src/wall/move-endpoint-tool.tsx +++ b/packages/nodes/src/wall/move-endpoint-tool.tsx @@ -229,6 +229,21 @@ export const MoveWallEndpointTool: React.FC<{ target: MovingWallEndpoint }> = ({ const originalStart = originalStartRef.current const originalEnd = originalEndRef.current const fixedPoint = fixedPointRef.current + const movingOriginalPoint = target.endpoint === 'start' ? originalStart : originalEnd + // Walls attached to the MOVING corner cascade with the drag, but the snap + // pipeline reads the scene store, which keeps their pre-drag coordinates + // until commit. Their stale corners would recreate the old junction as a + // snap/alignment target: inside the connect radius the endpoint could + // never land closer than ~5cm to where it started, making sub-5cm + // corrections (e.g. squaring a scan-imported 91° corner) impossible. + // Excluded while attached; under Alt-detach they stay put and remain + // legitimate targets. + const movingLinkedWallIds = linkedOriginalsRef.current + .filter( + (wall) => + samePoint(wall.start, movingOriginalPoint) || samePoint(wall.end, movingOriginalPoint), + ) + .map((wall) => wall.id) const levelWalls = Object.values(useScene.getState().nodes).filter( (node): node is WallNode => node?.type === 'wall' && (node.parentId ?? null) === (target.wall.parentId ?? null), @@ -238,14 +253,24 @@ export const MoveWallEndpointTool: React.FC<{ target: MovingWallEndpoint }> = ({ // fences, items, slabs, ceilings, columns), gathered once (the set is // stable during the drag). Coords are building-local, the same frame as // the cursor and the 3D guide layer, so the published guide lines up. + // The attached variant additionally drops anchors owned by walls that + // follow the moving corner (see `movingLinkedWallIds` above) — their + // scene coordinates are stale during the drag. const wallAlignmentCandidates = collectAlignmentAnchors(useScene.getState().nodes, nodeId) + const movingLinkedIdSet = new Set(movingLinkedWallIds) + const attachedAlignmentCandidates = wallAlignmentCandidates.filter( + (anchor) => !movingLinkedIdSet.has(anchor.nodeId), + ) pauseSceneHistory(useScene) let wasCommitted = false - // Last point handed to `applyPreview` — lets the Alt keydown/keyup - // handlers re-run the preview immediately on a modifier change instead of - // waiting for the next mousemove. - let lastMovedPoint: WallPlanPoint | null = null + // Last RAW cursor point from `grid:move` — lets the Alt keydown/keyup + // handlers re-run the FULL snap pipeline immediately on a modifier change + // instead of waiting for the next mousemove. The raw point (not the + // snapped one) matters: the snap/alignment candidate set depends on Alt + // (stale-junction exclusion above), so a point snapped under the previous + // modifier state must not be reused as-is. + let lastRawPoint: WallPlanPoint | null = null // The first pointer-up is the *grab* of a click-to-move; later ones are // drops. See the `!hasChanged` branch in `onPointerUp`. let hasReleasedOnce = false @@ -289,7 +314,6 @@ export const MoveWallEndpointTool: React.FC<{ target: MovingWallEndpoint }> = ({ } const applyPreview = (movingPoint: WallPlanPoint, detachLinkedWalls = false) => { - lastMovedPoint = movingPoint const nextStart = target.endpoint === 'start' ? movingPoint : fixedPoint const nextEnd = target.endpoint === 'end' ? movingPoint : fixedPoint const linkedUpdates = detachLinkedWalls @@ -353,16 +377,18 @@ export const MoveWallEndpointTool: React.FC<{ target: MovingWallEndpoint }> = ({ setTimeout(() => window.removeEventListener('click', swallow, { capture: true }), 300) } - const onGridMove = (event: GridEvent) => { - const planPoint: WallPlanPoint = [event.localPosition[0], event.localPosition[2]] - // Endpoint move honours the active snapping mode (the HUD chip): grid → - // lattice; lines → magnetic corner/alignment snap; angles → lock the - // segment to 15° rays from the FIXED corner; off → raw. No Shift bypass — - // Shift cycles the mode now, and Off is the bypass. + // Full snap pipeline from a RAW cursor point to the applied endpoint — + // shared by `grid:move` and the Alt keydown/keyup handlers, since the + // candidate set (stale-junction exclusion) flips with the modifier. + // Endpoint move honours the active snapping mode (the HUD chip): grid → + // lattice; lines → magnetic corner/alignment snap; angles → lock the + // segment to 15° rays from the FIXED corner; off → raw. No Shift bypass — + // Shift cycles the mode now, and Off is the bypass. + const resolveDragPoint = (planPoint: WallPlanPoint): WallPlanPoint => { const snapResult = snapWallDraftPointDetailed({ point: planPoint, walls: levelWalls, - ignoreWallIds: [nodeId], + ignoreWallIds: altPressedRef.current ? [nodeId] : [nodeId, ...movingLinkedWallIds], start: fixedPoint, angleSnap: isAngleSnapActive(), magnetic: isMagneticSnapActive(), @@ -379,10 +405,13 @@ export const MoveWallEndpointTool: React.FC<{ target: MovingWallEndpoint }> = ({ // (isAlignmentGuideActive); the magnetic pull onto them is applied only in // 'lines' mode (isMagneticSnapActive). let alignedPoint = snappedPoint - if (isAlignmentGuideActive() && wallAlignmentCandidates.length > 0) { + const alignmentCandidates = altPressedRef.current + ? wallAlignmentCandidates + : attachedAlignmentCandidates + if (isAlignmentGuideActive() && alignmentCandidates.length > 0) { const ar = resolveAlignment({ moving: [{ nodeId, kind: 'corner', x: snappedPoint[0], z: snappedPoint[1] }], - candidates: wallAlignmentCandidates, + candidates: alignmentCandidates, threshold: ALIGNMENT_THRESHOLD_M, }) const magnetic = isMagneticSnapActive() @@ -428,14 +457,21 @@ export const MoveWallEndpointTool: React.FC<{ target: MovingWallEndpoint }> = ({ : null, ) + return alignedPoint + } + + const onGridMove = (event: GridEvent) => { + const planPoint: WallPlanPoint = [event.localPosition[0], event.localPosition[2]] + lastRawPoint = planPoint // The keydown listener can't observe an Alt press that predates the // tool mounting; the pointer event can. Sync the shared ref (single Alt - // source for preview, HUD badge, and commit) before applying. + // source for snap targets, preview, HUD badge, and commit) before the + // snap pipeline reads it. if (event.nativeEvent.altKey !== altPressedRef.current) { altPressedRef.current = event.nativeEvent.altKey setAltPressed(event.nativeEvent.altKey) } - applyPreview(alignedPoint, altPressedRef.current) + applyPreview(resolveDragPoint(planPoint), altPressedRef.current) } const onPointerUp = () => { @@ -557,16 +593,17 @@ export const MoveWallEndpointTool: React.FC<{ target: MovingWallEndpoint }> = ({ exitMoveMode() } - // Single Alt writer for keyboard transitions. Re-running the preview on - // the flip keeps geometry and the HUD badge in lockstep — detach reverts - // the linked walls instantly, re-attach snaps them onto the dragged point - // — without waiting for the next mousemove. + // Single Alt writer for keyboard transitions. Re-running the FULL snap + // pipeline from the raw cursor on the flip keeps geometry and the HUD + // badge in lockstep — detach reverts the linked walls instantly and + // re-snaps against their (now live) corners, re-attach drops them from + // the candidate set again — without waiting for the next mousemove. const setAltState = (pressed: boolean) => { if (altPressedRef.current === pressed) return altPressedRef.current = pressed setAltPressed(pressed) - if (lastMovedPoint) { - applyPreview(lastMovedPoint, pressed) + if (lastRawPoint) { + applyPreview(resolveDragPoint(lastRawPoint), pressed) } }