Canvas gestures own the pointer until release - #82
Merged
Conversation
The pan/drag desyncs kept coming back because the input stack was mouse events plus compensating patches, and element-level mouse events structurally cannot see a release that happens off-canvas. Worse, two of the patches caused their own desyncs: mouseleave committed drags at the canvas edge, and an unpaired mouseup fired a phantom click into the tool. Mouse chords could also start a pan under an active drag. Rework the entry layer to pointer events with setPointerCapture: a gesture that starts on the canvas owns the pointer until pointerup or pointercancel, wherever the cursor goes. Three invariants, each pinned by tests: - liveness: e.buttons is ground truth; stale gestures close out before anything acts on them (kept as backstop for when capture fails) - pairing: tools only see onMouseUp for a drag the canvas started - one gesture at a time: downs are ignored while a gesture is live Tools keep their onMouseDown/Move/Up interface; only the canvas entry layer changed. jsdom lacks pointer capture, so setupDom stubs it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
About
Field report: still getting view desyncs when panning and dragging on v1.4.0, despite the #74 self-heal work. So this takes the whole input stack from the top instead of patching again.
The verdict: the architecture was the bug. The canvas listened to element-level mouse events, which structurally cannot see a release that happens off-canvas. Every prior fix (the window mouseup listener, the mouseleave handler, the e.buttons liveness check) shrank the desync window without being able to close it, and two of the patches were causing desyncs of their own:
onMouseLeavecommitted drags at the canvas edge. Graze the edge mid-marquee or mid-entity-move and the drag ended where you never released; pans died crossing onto a side panel.The rework moves the canvas to pointer events with
setPointerCapture: a gesture that starts on the canvas owns the pointer until release or cancel, and the browser delivers every move and the release to the canvas no matter where the cursor goes, including outside the window. Eaten releases stop existing as a category instead of being healed after the fact.Three invariants, each pinned by tests:
e.buttonsis ground truth; a stale gesture closes out before anything acts on it.onMouseUpfor a drag the canvas started. Phantom releases never reach them.Tools keep their existing
onMouseDown/Move/Upinterface untouched; only the canvas entry layer changed.pointercancel/lostpointercapturenow close gestures properly when the OS takes the pointer. jsdom has no pointer capture, so the dom test setup stubs it.Worth flagging: with capture, dragging past the canvas edge now continues the gesture (marquee keeps growing, pan keeps panning) instead of ending it at the edge. That is the desktop-standard behavior, but it is a feel change you will notice immediately.
I have not been able to reproduce the original field desync on demand, so this fixes the mechanisms found by reading the stack rather than a confirmed repro. Worth a hands-on session before merging.
Testing
Checklist