editor: stop wall endpoint drag snapping back to the stale junction corner (#533)
* 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
0fba611a05
commit
2adb50a340
@@ -327,4 +327,35 @@ describe('snapWallDraftPointDetailed', () => {
|
|||||||
expect(result.point).toEqual([3.99, 0.03])
|
expect(result.point).toEqual([3.99, 0.03])
|
||||||
expect(result.snap).toBeNull()
|
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()
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -169,6 +169,18 @@ export const wallMoveEndpointAffordance: FloorplanAffordance<WallNode> = {
|
|||||||
const originalEnd: WallPlanPoint = [...node.end] as WallPlanPoint
|
const originalEnd: WallPlanPoint = [...node.end] as WallPlanPoint
|
||||||
const linkedWalls = collectLinkedWalls(nodes, node.id, originalStart, originalEnd)
|
const linkedWalls = collectLinkedWalls(nodes, node.id, originalStart, originalEnd)
|
||||||
const affectedIds: AnyNodeId[] = [node.id, ...linkedWalls.map((w) => w.id)]
|
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.
|
// Remember the latest preview so `commit()` can write it tracked.
|
||||||
let lastPrimaryStart: WallPlanPoint = originalStart
|
let lastPrimaryStart: WallPlanPoint = originalStart
|
||||||
@@ -181,11 +193,12 @@ export const wallMoveEndpointAffordance: FloorplanAffordance<WallNode> = {
|
|||||||
// Re-collect walls every tick so the snap pipeline sees fresh
|
// Re-collect walls every tick so the snap pipeline sees fresh
|
||||||
// positions (matters when the user releases + re-grabs without
|
// positions (matters when the user releases + re-grabs without
|
||||||
// unmounting the layer). Snap reads from scene — which holds
|
// unmounting the layer). Snap reads from scene — which holds
|
||||||
// the pre-drag positions throughout — so the linked-wall snap
|
// the pre-drag positions throughout — so walls that cascade with
|
||||||
// targets stay anchored to where corners *were*, exactly like
|
// the moving corner are excluded (stale coordinates); under
|
||||||
// the legacy flow.
|
// Alt-detach they stay put, so they rejoin the candidate pool.
|
||||||
const sceneNodes = useScene.getState().nodes
|
const sceneNodes = useScene.getState().nodes
|
||||||
const walls = collectLevelWalls(sceneNodes, node.id)
|
const walls = collectLevelWalls(sceneNodes, node.id)
|
||||||
|
const staleWallIds = modifiers.altKey ? [node.id] : [node.id, ...movingLinkedWallIds]
|
||||||
// The grid step follows the active snapping mode (`getSegmentGridStep()`
|
// The grid step follows the active snapping mode (`getSegmentGridStep()`
|
||||||
// is 0 outside grid mode), so `'lines' / 'angles' / 'off'` no longer
|
// is 0 outside grid mode), so `'lines' / 'angles' / 'off'` no longer
|
||||||
// force a grid snap the mode chip says is inactive. In `'angles'` mode
|
// force a grid snap the mode chip says is inactive. In `'angles'` mode
|
||||||
@@ -195,7 +208,7 @@ export const wallMoveEndpointAffordance: FloorplanAffordance<WallNode> = {
|
|||||||
const snapped = snapWallDraftPoint({
|
const snapped = snapWallDraftPoint({
|
||||||
point: planPoint as WallPlanPoint,
|
point: planPoint as WallPlanPoint,
|
||||||
walls,
|
walls,
|
||||||
ignoreWallIds: [node.id],
|
ignoreWallIds: staleWallIds,
|
||||||
start: angleLocked ? fixedPoint : undefined,
|
start: angleLocked ? fixedPoint : undefined,
|
||||||
angleSnap: angleLocked,
|
angleSnap: angleLocked,
|
||||||
magnetic: isMagneticSnapActive(),
|
magnetic: isMagneticSnapActive(),
|
||||||
@@ -205,13 +218,15 @@ export const wallMoveEndpointAffordance: FloorplanAffordance<WallNode> = {
|
|||||||
// object's edge / wall face and publishes a guide. The guide is
|
// object's edge / wall face and publishes a guide. The guide is
|
||||||
// DISPLAYED in every mode except Off (isAlignmentGuideActive); the
|
// DISPLAYED in every mode except Off (isAlignmentGuideActive); the
|
||||||
// magnetic pull onto it is applied only in 'lines' mode
|
// magnetic pull onto it is applied only in 'lines' mode
|
||||||
// (isMagneticSnapActive), like the draft tool does. The dragged wall and
|
// (isMagneticSnapActive), like the draft tool does. Only the dragged
|
||||||
// its linked siblings (which cascade with the corner) are excluded from
|
// wall and the siblings cascading with the moving corner are excluded
|
||||||
// the candidate pool. Alt is detach, NOT bypass.
|
// 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, {
|
const aligned = alignFloorplanDraftPoint(snapped, {
|
||||||
applySnap: isMagneticSnapActive(),
|
applySnap: isMagneticSnapActive(),
|
||||||
bypass: !isAlignmentGuideActive(),
|
bypass: !isAlignmentGuideActive(),
|
||||||
excludeIds: [node.id, ...linkedWalls.map((w) => w.id)],
|
excludeIds: staleWallIds,
|
||||||
}) as WallPlanPoint
|
}) as WallPlanPoint
|
||||||
|
|
||||||
const primaryStart: WallPlanPoint = endpoint === 'start' ? aligned : fixedPoint
|
const primaryStart: WallPlanPoint = endpoint === 'start' ? aligned : fixedPoint
|
||||||
|
|||||||
@@ -229,6 +229,21 @@ export const MoveWallEndpointTool: React.FC<{ target: MovingWallEndpoint }> = ({
|
|||||||
const originalStart = originalStartRef.current
|
const originalStart = originalStartRef.current
|
||||||
const originalEnd = originalEndRef.current
|
const originalEnd = originalEndRef.current
|
||||||
const fixedPoint = fixedPointRef.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(
|
const levelWalls = Object.values(useScene.getState().nodes).filter(
|
||||||
(node): node is WallNode =>
|
(node): node is WallNode =>
|
||||||
node?.type === 'wall' && (node.parentId ?? null) === (target.wall.parentId ?? null),
|
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
|
// fences, items, slabs, ceilings, columns), gathered once (the set is
|
||||||
// stable during the drag). Coords are building-local, the same frame as
|
// 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 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 wallAlignmentCandidates = collectAlignmentAnchors(useScene.getState().nodes, nodeId)
|
||||||
|
const movingLinkedIdSet = new Set<string>(movingLinkedWallIds)
|
||||||
|
const attachedAlignmentCandidates = wallAlignmentCandidates.filter(
|
||||||
|
(anchor) => !movingLinkedIdSet.has(anchor.nodeId),
|
||||||
|
)
|
||||||
|
|
||||||
pauseSceneHistory(useScene)
|
pauseSceneHistory(useScene)
|
||||||
let wasCommitted = false
|
let wasCommitted = false
|
||||||
// Last point handed to `applyPreview` — lets the Alt keydown/keyup
|
// Last RAW cursor point from `grid:move` — lets the Alt keydown/keyup
|
||||||
// handlers re-run the preview immediately on a modifier change instead of
|
// handlers re-run the FULL snap pipeline immediately on a modifier change
|
||||||
// waiting for the next mousemove.
|
// instead of waiting for the next mousemove. The raw point (not the
|
||||||
let lastMovedPoint: WallPlanPoint | null = null
|
// 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
|
// The first pointer-up is the *grab* of a click-to-move; later ones are
|
||||||
// drops. See the `!hasChanged` branch in `onPointerUp`.
|
// drops. See the `!hasChanged` branch in `onPointerUp`.
|
||||||
let hasReleasedOnce = false
|
let hasReleasedOnce = false
|
||||||
@@ -289,7 +314,6 @@ export const MoveWallEndpointTool: React.FC<{ target: MovingWallEndpoint }> = ({
|
|||||||
}
|
}
|
||||||
|
|
||||||
const applyPreview = (movingPoint: WallPlanPoint, detachLinkedWalls = false) => {
|
const applyPreview = (movingPoint: WallPlanPoint, detachLinkedWalls = false) => {
|
||||||
lastMovedPoint = movingPoint
|
|
||||||
const nextStart = target.endpoint === 'start' ? movingPoint : fixedPoint
|
const nextStart = target.endpoint === 'start' ? movingPoint : fixedPoint
|
||||||
const nextEnd = target.endpoint === 'end' ? movingPoint : fixedPoint
|
const nextEnd = target.endpoint === 'end' ? movingPoint : fixedPoint
|
||||||
const linkedUpdates = detachLinkedWalls
|
const linkedUpdates = detachLinkedWalls
|
||||||
@@ -353,16 +377,18 @@ export const MoveWallEndpointTool: React.FC<{ target: MovingWallEndpoint }> = ({
|
|||||||
setTimeout(() => window.removeEventListener('click', swallow, { capture: true }), 300)
|
setTimeout(() => window.removeEventListener('click', swallow, { capture: true }), 300)
|
||||||
}
|
}
|
||||||
|
|
||||||
const onGridMove = (event: GridEvent) => {
|
// Full snap pipeline from a RAW cursor point to the applied endpoint —
|
||||||
const planPoint: WallPlanPoint = [event.localPosition[0], event.localPosition[2]]
|
// shared by `grid:move` and the Alt keydown/keyup handlers, since the
|
||||||
// Endpoint move honours the active snapping mode (the HUD chip): grid →
|
// candidate set (stale-junction exclusion) flips with the modifier.
|
||||||
// lattice; lines → magnetic corner/alignment snap; angles → lock the
|
// Endpoint move honours the active snapping mode (the HUD chip): grid →
|
||||||
// segment to 15° rays from the FIXED corner; off → raw. No Shift bypass —
|
// lattice; lines → magnetic corner/alignment snap; angles → lock the
|
||||||
// Shift cycles the mode now, and Off is the bypass.
|
// 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({
|
const snapResult = snapWallDraftPointDetailed({
|
||||||
point: planPoint,
|
point: planPoint,
|
||||||
walls: levelWalls,
|
walls: levelWalls,
|
||||||
ignoreWallIds: [nodeId],
|
ignoreWallIds: altPressedRef.current ? [nodeId] : [nodeId, ...movingLinkedWallIds],
|
||||||
start: fixedPoint,
|
start: fixedPoint,
|
||||||
angleSnap: isAngleSnapActive(),
|
angleSnap: isAngleSnapActive(),
|
||||||
magnetic: isMagneticSnapActive(),
|
magnetic: isMagneticSnapActive(),
|
||||||
@@ -379,10 +405,13 @@ export const MoveWallEndpointTool: React.FC<{ target: MovingWallEndpoint }> = ({
|
|||||||
// (isAlignmentGuideActive); the magnetic pull onto them is applied only in
|
// (isAlignmentGuideActive); the magnetic pull onto them is applied only in
|
||||||
// 'lines' mode (isMagneticSnapActive).
|
// 'lines' mode (isMagneticSnapActive).
|
||||||
let alignedPoint = snappedPoint
|
let alignedPoint = snappedPoint
|
||||||
if (isAlignmentGuideActive() && wallAlignmentCandidates.length > 0) {
|
const alignmentCandidates = altPressedRef.current
|
||||||
|
? wallAlignmentCandidates
|
||||||
|
: attachedAlignmentCandidates
|
||||||
|
if (isAlignmentGuideActive() && alignmentCandidates.length > 0) {
|
||||||
const ar = resolveAlignment({
|
const ar = resolveAlignment({
|
||||||
moving: [{ nodeId, kind: 'corner', x: snappedPoint[0], z: snappedPoint[1] }],
|
moving: [{ nodeId, kind: 'corner', x: snappedPoint[0], z: snappedPoint[1] }],
|
||||||
candidates: wallAlignmentCandidates,
|
candidates: alignmentCandidates,
|
||||||
threshold: ALIGNMENT_THRESHOLD_M,
|
threshold: ALIGNMENT_THRESHOLD_M,
|
||||||
})
|
})
|
||||||
const magnetic = isMagneticSnapActive()
|
const magnetic = isMagneticSnapActive()
|
||||||
@@ -428,14 +457,21 @@ export const MoveWallEndpointTool: React.FC<{ target: MovingWallEndpoint }> = ({
|
|||||||
: null,
|
: 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
|
// The keydown listener can't observe an Alt press that predates the
|
||||||
// tool mounting; the pointer event can. Sync the shared ref (single Alt
|
// 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) {
|
if (event.nativeEvent.altKey !== altPressedRef.current) {
|
||||||
altPressedRef.current = event.nativeEvent.altKey
|
altPressedRef.current = event.nativeEvent.altKey
|
||||||
setAltPressed(event.nativeEvent.altKey)
|
setAltPressed(event.nativeEvent.altKey)
|
||||||
}
|
}
|
||||||
applyPreview(alignedPoint, altPressedRef.current)
|
applyPreview(resolveDragPoint(planPoint), altPressedRef.current)
|
||||||
}
|
}
|
||||||
|
|
||||||
const onPointerUp = () => {
|
const onPointerUp = () => {
|
||||||
@@ -557,16 +593,17 @@ export const MoveWallEndpointTool: React.FC<{ target: MovingWallEndpoint }> = ({
|
|||||||
exitMoveMode()
|
exitMoveMode()
|
||||||
}
|
}
|
||||||
|
|
||||||
// Single Alt writer for keyboard transitions. Re-running the preview on
|
// Single Alt writer for keyboard transitions. Re-running the FULL snap
|
||||||
// the flip keeps geometry and the HUD badge in lockstep — detach reverts
|
// pipeline from the raw cursor on the flip keeps geometry and the HUD
|
||||||
// the linked walls instantly, re-attach snaps them onto the dragged point
|
// badge in lockstep — detach reverts the linked walls instantly and
|
||||||
// — without waiting for the next mousemove.
|
// 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) => {
|
const setAltState = (pressed: boolean) => {
|
||||||
if (altPressedRef.current === pressed) return
|
if (altPressedRef.current === pressed) return
|
||||||
altPressedRef.current = pressed
|
altPressedRef.current = pressed
|
||||||
setAltPressed(pressed)
|
setAltPressed(pressed)
|
||||||
if (lastMovedPoint) {
|
if (lastRawPoint) {
|
||||||
applyPreview(lastMovedPoint, pressed)
|
applyPreview(resolveDragPoint(lastRawPoint), pressed)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user