diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e0792c..730a228 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,18 @@ workflow copies it into the GitHub release notes. the same depth band as doors (Overdoors), and the layer filter sorted purely by draw depth, so all markers landed in the Doors bucket. Markers now classify by what they are instead of where they draw. +- Reworked canvas input from the ground up to end the pan/drag view desyncs. + The canvas now uses pointer capture: a pan or drag that starts on the + canvas owns the mouse until release, so dragging across side panels, out + of the window, or over other UI can no longer strand a gesture halfway. + Three desync sources this removes outright: leaving the canvas mid-drag + used to silently commit the drag at the edge (marquees ended early, + entity moves dropped where you never released); a release that arrived + without its matching press could fire a phantom click that selected or + dropped at a spot you never clicked; and pressing a second mouse button + mid-drag started a pan underneath the active drag, shifting the view out + from under it. One gesture at a time now, and releases always reach the + editor. ## [1.4.0] - 2026-07-28 diff --git a/src/components/EditorCanvas.tsx b/src/components/EditorCanvas.tsx index 832bdc4..57ae04c 100644 --- a/src/components/EditorCanvas.tsx +++ b/src/components/EditorCanvas.tsx @@ -194,10 +194,10 @@ export const EditorCanvas: React.FC = ({ return button === 1 || isSpaceHeldRef.current || toolRef.current?.name === 'pan'; }, []); - // Safety net: end panning on any release or focus loss anywhere, not just on - // the canvas. A release over a side panel (or a pointer grab we didn't see) - // would otherwise leave isPanning stuck on. Pan-only; tool drags are ended by - // the canvas handlers. + // Belt-and-braces: end panning on focus loss or a release anywhere. Pointer + // capture below is the primary guarantee that releases reach the canvas; + // this backstops the cases capture can't make promises about (capture call + // failed, alt-tab mid-gesture on platforms that skip pointercancel). useEffect(() => { const endPan = () => { isPanning.current = false; @@ -210,18 +210,38 @@ export const EditorCanvas: React.FC = ({ }; }, []); - const handleMouseDown = useCallback( - (e: React.MouseEvent) => { + // Capture the pointer for the duration of a gesture: the browser then + // delivers every move and the release to the canvas no matter where the + // cursor goes, including outside the window. Without capture, a release + // off-canvas is simply never seen, and no amount of bookkeeping downstream + // can fully recover (the old stuck-pan / eaten-release desync family). + const capturePointer = useCallback((e: React.PointerEvent) => { + try { + (e.currentTarget as HTMLElement).setPointerCapture?.(e.pointerId); + } catch { + // Capture is an enhancement; the e.buttons liveness heal still backstops. + } + }, []); + + const handlePointerDown = useCallback( + (e: React.PointerEvent) => { setContextMenu(null); + // One gesture at a time. Chorded mouse buttons don't fire pointerdown + // again, but a second pointer (touch, pen) or a synthetic event could + // otherwise start a pan mid-drag and move the camera under the tool's + // anchor. + if (isPanning.current || toolDragActive.current) return; if (shouldPan(e.button)) { // Middle-button mousedown otherwise triggers the browser's autoscroll, // which captures the pointer and swallows the matching mouseup, leaving // the pan stuck on (view drifts, clicks stop selecting). e.preventDefault(); + capturePointer(e); isPanning.current = true; lastMouse.current = { x: e.clientX, y: e.clientY }; return; } + capturePointer(e); isShiftHeldRef.current = e.shiftKey; isCtrlHeldRef.current = e.ctrlKey || e.metaKey; @@ -279,11 +299,11 @@ export const EditorCanvas: React.FC = ({ tool?.onMouseDown(getToolContext(), tile.x, tile.y, e.button); } }, - [screenToWorld, getToolContext, shouldPan], + [screenToWorld, getToolContext, shouldPan, capturePointer], ); - const handleMouseMove = useCallback( - (e: React.MouseEvent) => { + const handlePointerMove = useCallback( + (e: React.PointerEvent) => { isShiftHeldRef.current = e.shiftKey; isCtrlHeldRef.current = e.ctrlKey || e.metaKey; const tile = screenToWorld(e.clientX, e.clientY); @@ -292,14 +312,14 @@ export const EditorCanvas: React.FC = ({ cursorWorld.current = world; markOverlayDirty(); - // Liveness check: e.buttons is ground truth for what is held RIGHT NOW. A - // mousemove with no buttons down while we still think a drag is in progress - // means the matching mouseup was eaten (released outside the window, native - // autoscroll, a menu swallowing it). Trust the hardware over our own - // bookkeeping and close the stale state out BEFORE acting on it; otherwise a - // stuck pan drags the world along with the bare cursor, and the next click - // hit-tests against a camera the user never chose (the "selection lands on a - // random spot / view snaps back" desync). + // Liveness check: e.buttons is ground truth for what is held RIGHT NOW. + // Pointer capture should make an eaten release impossible, but this is + // the backstop for when capture wasn't granted (setPointerCapture threw, + // synthetic events): a move with no buttons down while we still think a + // drag is live means the release was missed. Trust the hardware and + // close the stale state out BEFORE acting on it; otherwise a stuck pan + // drags the world along with the bare cursor, and the next click + // hit-tests against a camera the user never chose. if (e.buttons === 0) { isPanning.current = false; if (toolDragActive.current) { @@ -329,13 +349,19 @@ export const EditorCanvas: React.FC = ({ [camera, screenToWorld, getToolContext], ); - const handleMouseUp = useCallback( - (e: React.MouseEvent) => { + const handlePointerUp = useCallback( + (e: React.PointerEvent) => { isShiftHeldRef.current = e.shiftKey; if (isPanning.current) { isPanning.current = false; return; } + // Only a live canvas-originated drag gets its release. An unpaired up + // (down happened on a panel, or the gesture was already closed out by a + // heal or cancel) must not reach the tool: tools treat onMouseUp as a + // click/commit, and a phantom one selects or drops at a spot the user + // never clicked. + if (!toolDragActive.current) return; markOverlayDirty(); toolDragActive.current = false; const tool = toolRef.current; @@ -346,6 +372,27 @@ export const EditorCanvas: React.FC = ({ [screenToWorld, getToolContext], ); + // The browser or OS took the gesture away (pointercancel, capture lost to a + // native drag or window change). Close out live state the same way the + // liveness heal does: a pan just ends, a tool drag gets its release at the + // last known position. Also wired to lostpointercapture, which fires after + // every normal pointerup too; by then both flags are already false and this + // is a no-op. + const handlePointerCancel = useCallback( + (e: React.PointerEvent) => { + isPanning.current = false; + if (!toolDragActive.current) return; + toolDragActive.current = false; + const tool = toolRef.current; + const world = screenToWorld(e.clientX, e.clientY, true); + const usePrecise = isShiftHeldRef.current && (tool?.name === 'entityPlace' || tool?.name === 'entitySelect'); + const coord = usePrecise ? world : { x: Math.floor(world.x), y: Math.floor(world.y) }; + tool?.onMouseUp(getToolContext(), coord.x, coord.y); + markOverlayDirty(); + }, + [screenToWorld, getToolContext], + ); + const handleWheel = useCallback( (e: React.WheelEvent) => { e.preventDefault(); @@ -953,11 +1000,21 @@ export const EditorCanvas: React.FC = ({ <> diff --git a/src/components/__tests__/EditorCanvas.panLiveness.test.tsx b/src/components/__tests__/EditorCanvas.panLiveness.test.tsx index b000e5b..fdf5787 100644 --- a/src/components/__tests__/EditorCanvas.panLiveness.test.tsx +++ b/src/components/__tests__/EditorCanvas.panLiveness.test.tsx @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; import { render } from '@testing-library/react'; import { fireEvent } from '@testing-library/dom'; import { EditorCanvas } from '../EditorCanvas'; @@ -9,21 +9,25 @@ import { DEFAULT_DECAL_PLACEMENT_SETTINGS } from '../DecalPalette'; import type { ITool } from '../../tools/toolTypes'; /** - * Regression cover for the stuck-pan selection desync. + * Gesture integrity for the canvas input stack. * - * The pan/drag state machine used to trust that every mousedown gets a matching - * mouseup. Releases can be eaten (outside the window, native autoscroll, menus), - * after which `isPanning` stayed true with NO button held: every bare mousemove - * kept panning the camera, the world tracked the cursor back toward where the - * pan started, and the next click hit-tested against a camera position the user - * never chose. Symptom in the field: "middle-click pan, click something else, - * selection lands on a random spot and the view snaps back to the previous pan - * location." + * The stack is pointer-events + setPointerCapture: a gesture that starts on + * the canvas owns the pointer until release or cancel, so moves and the + * release reach the canvas no matter where the cursor goes. On top of capture + * sit three invariants these tests pin: * - * The fix reads `e.buttons` (ground truth for what is held right now) on every - * mousemove and closes out any drag state whose release was missed, before - * acting on it. These tests simulate the eaten release directly by never firing - * mouseup and moving with `buttons: 0`. + * 1. LIVENESS: `e.buttons` is ground truth. If a move arrives with no buttons + * held while drag state is live (capture failed, synthetic events), the + * stale gesture is closed out before anything acts on it. This killed the + * field desync "middle-click pan, click something else, selection lands on + * a random spot and the view snaps back." + * 2. PAIRING: tools only ever see onMouseUp for a drag the canvas started. + * Unpaired releases (down on a panel, gesture already healed or cancelled) + * never reach the tool: tools treat onMouseUp as click/commit, so a + * phantom one selects or drops at a spot the user never clicked. + * 3. ONE GESTURE AT A TIME: while a pan or tool drag is live, further downs + * are ignored: a second pointer must not move the camera under an active + * drag's anchor. */ // TILE_SIZE is 32 and the test camera stays at zoom 1, so 32 screen px = 1 tile. @@ -66,43 +70,69 @@ function setup(tool: ITool | null = null) { return { camera, canvas }; } -beforeEach(() => { - // The component pans via deltas between events, so absolute coordinates only - // need to be self-consistent within a test. +describe('pointer capture', () => { + it('requests capture when a gesture starts', () => { + const { canvas } = setup(makeTool()); + const captureSpy = vi.fn(); + canvas.setPointerCapture = captureSpy; + + fireEvent.pointerDown(canvas, { pointerId: 7, button: 0, buttons: 1, clientX: 100, clientY: 100 }); + expect(captureSpy).toHaveBeenCalledWith(7); + }); + + it('requests capture for a middle-button pan too', () => { + const { canvas } = setup(); + const captureSpy = vi.fn(); + canvas.setPointerCapture = captureSpy; + + fireEvent.pointerDown(canvas, { pointerId: 3, button: 1, buttons: 4, clientX: 400, clientY: 300 }); + expect(captureSpy).toHaveBeenCalledWith(3); + }); + + it('survives setPointerCapture throwing (dead pointerId, jsdom)', () => { + const { camera, canvas } = setup(); + canvas.setPointerCapture = () => { + throw new DOMException('InvalidPointerId'); + }; + + fireEvent.pointerDown(canvas, { button: 1, buttons: 4, clientX: 400, clientY: 300 }); + fireEvent.pointerMove(canvas, { buttons: 4, clientX: 400 - TILE, clientY: 300 }); + // The gesture still works without capture; liveness backstops it. + expect(camera.x).toBeCloseTo(1); + }); }); -describe('EditorCanvas pan liveness', () => { +describe('pan liveness', () => { it('pans normally while the middle button is genuinely held', () => { const { camera, canvas } = setup(); - fireEvent.mouseDown(canvas, { button: 1, buttons: 4, clientX: 400, clientY: 300 }); - fireEvent.mouseMove(canvas, { buttons: 4, clientX: 400 - TILE, clientY: 300 }); + fireEvent.pointerDown(canvas, { button: 1, buttons: 4, clientX: 400, clientY: 300 }); + fireEvent.pointerMove(canvas, { buttons: 4, clientX: 400 - TILE, clientY: 300 }); // Dragging left by one tile moves the camera right by one tile (world follows cursor). expect(camera.x).toBeCloseTo(1); expect(camera.y).toBeCloseTo(0); - fireEvent.mouseUp(canvas, { button: 1, buttons: 0, clientX: 400 - TILE, clientY: 300 }); - fireEvent.mouseMove(canvas, { buttons: 0, clientX: 400, clientY: 300 }); + fireEvent.pointerUp(canvas, { button: 1, buttons: 0, clientX: 400 - TILE, clientY: 300 }); + fireEvent.pointerMove(canvas, { buttons: 0, clientX: 400, clientY: 300 }); // After a real release, bare movement must not pan. expect(camera.x).toBeCloseTo(1); }); - it('stops panning on the first bare mousemove when the middle release was eaten', () => { + it('stops panning on the first bare move when the release was eaten', () => { const { camera, canvas } = setup(); - fireEvent.mouseDown(canvas, { button: 1, buttons: 4, clientX: 400, clientY: 300 }); - fireEvent.mouseMove(canvas, { buttons: 4, clientX: 400 - TILE, clientY: 300 }); + fireEvent.pointerDown(canvas, { button: 1, buttons: 4, clientX: 400, clientY: 300 }); + fireEvent.pointerMove(canvas, { buttons: 4, clientX: 400 - TILE, clientY: 300 }); expect(camera.x).toBeCloseTo(1); - // No mouseup: the release was eaten. The user now moves the bare cursor back - // toward something they want to click. - fireEvent.mouseMove(canvas, { buttons: 0, clientX: 400, clientY: 300 }); - fireEvent.mouseMove(canvas, { buttons: 0, clientX: 400 + 3 * TILE, clientY: 300 - 2 * TILE }); + // No pointerup: the release was eaten. The user now moves the bare cursor + // back toward something they want to click. + fireEvent.pointerMove(canvas, { buttons: 0, clientX: 400, clientY: 300 }); + fireEvent.pointerMove(canvas, { buttons: 0, clientX: 400 + 3 * TILE, clientY: 300 - 2 * TILE }); - // Before the fix these moves kept panning (camera.x would walk back toward 0 - // and past it). The camera must stay exactly where the real pan left it. + // The camera must stay exactly where the real pan left it. expect(camera.x).toBeCloseTo(1); expect(camera.y).toBeCloseTo(0); }); @@ -111,37 +141,53 @@ describe('EditorCanvas pan liveness', () => { const tool = makeTool(); const { camera, canvas } = setup(tool); - fireEvent.mouseDown(canvas, { button: 1, buttons: 4, clientX: 400, clientY: 300 }); - fireEvent.mouseMove(canvas, { buttons: 4, clientX: 336, clientY: 300 }); + fireEvent.pointerDown(canvas, { button: 1, buttons: 4, clientX: 400, clientY: 300 }); + fireEvent.pointerMove(canvas, { buttons: 4, clientX: 336, clientY: 300 }); // Eaten release, then the user travels to a new target. - fireEvent.mouseMove(canvas, { buttons: 0, clientX: 200, clientY: 200 }); + fireEvent.pointerMove(canvas, { buttons: 0, clientX: 200, clientY: 200 }); const xAfterHeal = camera.x; - // The click must go to the tool, not be swallowed by pan state, and must not - // move the camera. - fireEvent.mouseDown(canvas, { button: 0, buttons: 1, clientX: 200, clientY: 200 }); - fireEvent.mouseUp(canvas, { button: 0, buttons: 0, clientX: 200, clientY: 200 }); + // The click must go to the tool, not be swallowed by pan state, and must + // not move the camera. + fireEvent.pointerDown(canvas, { button: 0, buttons: 1, clientX: 200, clientY: 200 }); + fireEvent.pointerUp(canvas, { button: 0, buttons: 0, clientX: 200, clientY: 200 }); expect(tool.onMouseDown).toHaveBeenCalledTimes(1); expect(tool.onMouseUp).toHaveBeenCalledTimes(1); expect(camera.x).toBeCloseTo(xAfterHeal); }); + it('ends the pan on pointercancel', () => { + const { camera, canvas } = setup(); + + fireEvent.pointerDown(canvas, { button: 1, buttons: 4, clientX: 400, clientY: 300 }); + fireEvent.pointerMove(canvas, { buttons: 4, clientX: 400 - TILE, clientY: 300 }); + expect(camera.x).toBeCloseTo(1); + + fireEvent.pointerCancel(canvas, { clientX: 400 - TILE, clientY: 300 }); + // A move that still claims a held button must not resurrect the pan: the + // gesture is over. + fireEvent.pointerMove(canvas, { buttons: 4, clientX: 400, clientY: 300 }); + expect(camera.x).toBeCloseTo(1); + }); +}); + +describe('tool drag pairing', () => { it('closes out a tool drag whose release was eaten', () => { const tool = makeTool(); const { canvas } = setup(tool); - fireEvent.mouseDown(canvas, { button: 0, buttons: 1, clientX: 100, clientY: 100 }); + fireEvent.pointerDown(canvas, { button: 0, buttons: 1, clientX: 100, clientY: 100 }); expect(tool.onMouseDown).toHaveBeenCalledTimes(1); - // Release eaten; bare movement afterward must deliver exactly one synthetic - // mouseup so the tool's drag state (box select, move drag) cannot stay open. - fireEvent.mouseMove(canvas, { buttons: 0, clientX: 150, clientY: 150 }); + // Release eaten; bare movement afterward must deliver exactly one + // synthetic mouseup so the tool's drag state cannot stay open. + fireEvent.pointerMove(canvas, { buttons: 0, clientX: 150, clientY: 150 }); expect(tool.onMouseUp).toHaveBeenCalledTimes(1); // Further hovering must not fabricate more mouseups. - fireEvent.mouseMove(canvas, { buttons: 0, clientX: 180, clientY: 180 }); - fireEvent.mouseMove(canvas, { buttons: 0, clientX: 220, clientY: 140 }); + fireEvent.pointerMove(canvas, { buttons: 0, clientX: 180, clientY: 180 }); + fireEvent.pointerMove(canvas, { buttons: 0, clientX: 220, clientY: 140 }); expect(tool.onMouseUp).toHaveBeenCalledTimes(1); }); @@ -149,25 +195,78 @@ describe('EditorCanvas pan liveness', () => { const tool = makeTool(); const { canvas } = setup(tool); - fireEvent.mouseDown(canvas, { button: 0, buttons: 1, clientX: 100, clientY: 100 }); - fireEvent.mouseMove(canvas, { buttons: 1, clientX: 150, clientY: 150 }); - fireEvent.mouseMove(canvas, { buttons: 1, clientX: 200, clientY: 200 }); + fireEvent.pointerDown(canvas, { button: 0, buttons: 1, clientX: 100, clientY: 100 }); + fireEvent.pointerMove(canvas, { buttons: 1, clientX: 150, clientY: 150 }); + fireEvent.pointerMove(canvas, { buttons: 1, clientX: 200, clientY: 200 }); expect(tool.onMouseUp).not.toHaveBeenCalled(); expect(tool.onMouseMove).toHaveBeenCalled(); - fireEvent.mouseUp(canvas, { button: 0, buttons: 0, clientX: 200, clientY: 200 }); + fireEvent.pointerUp(canvas, { button: 0, buttons: 0, clientX: 200, clientY: 200 }); expect(tool.onMouseUp).toHaveBeenCalledTimes(1); }); - it('a real mouseup still reaches the tool exactly once', () => { + it('a real release reaches the tool exactly once', () => { const tool = makeTool(); const { canvas } = setup(tool); - fireEvent.mouseDown(canvas, { button: 0, buttons: 1, clientX: 100, clientY: 100 }); - fireEvent.mouseUp(canvas, { button: 0, buttons: 0, clientX: 100, clientY: 100 }); - fireEvent.mouseMove(canvas, { buttons: 0, clientX: 160, clientY: 160 }); + fireEvent.pointerDown(canvas, { button: 0, buttons: 1, clientX: 100, clientY: 100 }); + fireEvent.pointerUp(canvas, { button: 0, buttons: 0, clientX: 100, clientY: 100 }); + fireEvent.pointerMove(canvas, { buttons: 0, clientX: 160, clientY: 160 }); expect(tool.onMouseUp).toHaveBeenCalledTimes(1); }); + + it('an unpaired release never reaches the tool', () => { + const tool = makeTool(); + const { canvas } = setup(tool); + + // Down happened elsewhere (a panel, another window); only the release + // lands on the canvas. Tools treat onMouseUp as click/commit, so this + // must not arrive. + fireEvent.pointerUp(canvas, { button: 0, buttons: 0, clientX: 100, clientY: 100 }); + expect(tool.onMouseUp).not.toHaveBeenCalled(); + }); + + it('pointercancel closes the drag exactly once', () => { + const tool = makeTool(); + const { canvas } = setup(tool); + + fireEvent.pointerDown(canvas, { button: 0, buttons: 1, clientX: 100, clientY: 100 }); + fireEvent.pointerCancel(canvas, { clientX: 140, clientY: 140 }); + expect(tool.onMouseUp).toHaveBeenCalledTimes(1); + + // lostpointercapture follows every cancel (and every normal up); by then + // the gesture is closed and nothing more may reach the tool. + fireEvent.lostPointerCapture(canvas, { clientX: 140, clientY: 140 }); + fireEvent.pointerMove(canvas, { buttons: 0, clientX: 180, clientY: 180 }); + expect(tool.onMouseUp).toHaveBeenCalledTimes(1); + expect(tool.onMouseDown).toHaveBeenCalledTimes(1); + }); +}); + +describe('one gesture at a time', () => { + it('ignores a second down while a tool drag is live', () => { + const tool = makeTool(); + const { camera, canvas } = setup(tool); + + fireEvent.pointerDown(canvas, { button: 0, buttons: 1, clientX: 100, clientY: 100 }); + // A second pointer presses the middle button mid-drag. It must not start + // a pan: the camera moving would shift the drag's anchor under the tool. + fireEvent.pointerDown(canvas, { button: 1, buttons: 5, clientX: 200, clientY: 200 }); + fireEvent.pointerMove(canvas, { buttons: 5, clientX: 200 - TILE, clientY: 200 }); + + expect(camera.x).toBeCloseTo(0); + expect(tool.onMouseDown).toHaveBeenCalledTimes(1); + }); + + it('ignores a tool down while a pan is live', () => { + const tool = makeTool(); + const { canvas } = setup(tool); + + fireEvent.pointerDown(canvas, { button: 1, buttons: 4, clientX: 400, clientY: 300 }); + fireEvent.pointerDown(canvas, { button: 0, buttons: 5, clientX: 400, clientY: 300 }); + + expect(tool.onMouseDown).not.toHaveBeenCalled(); + }); }); diff --git a/src/test-utils/setupDom.ts b/src/test-utils/setupDom.ts index 98ff5ae..5e540f1 100644 --- a/src/test-utils/setupDom.ts +++ b/src/test-utils/setupDom.ts @@ -69,6 +69,15 @@ if (typeof HTMLCanvasElement !== 'undefined') { } as unknown as typeof HTMLCanvasElement.prototype.getContext; } +// jsdom does not implement pointer capture. EditorCanvas requests capture on +// pointerdown so drags survive leaving the canvas; a no-op is fine in tests, +// where events are dispatched straight at the canvas anyway. +if (typeof Element !== 'undefined' && !Element.prototype.setPointerCapture) { + Element.prototype.setPointerCapture = () => {}; + Element.prototype.releasePointerCapture = () => {}; + Element.prototype.hasPointerCapture = () => false; +} + // jsdom does not implement ResizeObserver, which EditorCanvas uses to track its parent's // size. A no-op observer is enough: size-driven behavior isn't testable in jsdom anyway // (every element measures 0x0), so tests assert on input handling and state instead.