From 8d7c2bb0ce6b0153103a70c8da189a8c01c032bd Mon Sep 17 00:00:00 2001 From: difficcd Date: Thu, 1 Oct 2026 15:08:06 +0900 Subject: [PATCH] fix: a palm on the glass no longer drives the pen's stroke (#338) A PC has one pointer, so every event on the canvas belonged to the stroke being drawn and no handler had to ask which pointer it came from. A tablet has several: the pen, and the palm resting on the glass beside it. `startDraw` refused to *begin* a gesture from a touch - the palm rejection that has always been there - but nothing stopped a touch from feeding the gesture already running: - `onDraw` checked only `gesture.drawing.current`, so the palm's moves were appended to the pen's stroke as points, putting a jump across the drawing. - `stopDraw` took no event at all, so it could not check anything: the palm coming off the glass committed the pen's stroke mid-line. The pen then went on moving over a gesture that had ended, drawing nothing until it was lifted and put down again. Which is a line in pieces, on a tablet and nowhere else - #338's "strokes break up". The gesture already knew whose pointer it was; `pointerId` was set by begin() and read by nobody. It is now read, through one predicate both handlers share. Events with no pointer to name still pass, because onPointerLeaveCanvas ends a stroke without one, and so does every event while nothing is in flight, because onDraw does its hover work through the same guard. Co-Authored-By: Claude Opus 5 --- HELPERS.md | 1 + src/App.tsx | 9 ++++++- src/hooks/useGesture.ts | 32 ++++++++++++++++++++++- test/hooks/gesturePointer.test.js | 42 +++++++++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 test/hooks/gesturePointer.test.js diff --git a/HELPERS.md b/HELPERS.md index fb0f511..db58032 100644 --- a/HELPERS.md +++ b/HELPERS.md @@ -992,6 +992,7 @@ What is happening between the pen going down and coming back up. | | | |---|---| | `useGesture` | Owns the scratch state of one pointer gesture — the stroke, the lasso loop, the layers or selection being dragged, the path being recorded, the layer the stroke will commit to — plus `begin`/`end`, which take and give back pointer capture. All refs: a pointer move arrives far more often than a frame. The returned object is stable, so an effect can list it. | +| `pointerIsOwner` | Whether a pointer event belongs to the gesture in flight — on a touchscreen the palm is a second pointer, and its moves and its lift were being taken for the pen's (#338). Passes everything through when nothing is in flight, and when the call has no event to name. | ## `src/hooks/useHistory.ts` diff --git a/src/App.tsx b/src/App.tsx index f9cd2b4..c51e822 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -1488,6 +1488,9 @@ export default function App() { const [hoverHandle, setHoverHandle] = useState(null); const onDraw = (e: React.PointerEvent) => { + // Not from the pointer that started this. A palm on the glass used to have its moves + // appended to the pen's stroke, which put a jump across the drawing (#338). + if (!gesture.isOurs(e)) return; // Hovering, not drawing: the only thing to work out is what the cursor should say. A // selection has eight handles and hitTestSelection already knows which one a point is // over; without this the cursor said "move" over all of them, so the one gesture that @@ -1513,7 +1516,11 @@ export default function App() { TOOLS[etool]?.move?.(toolCtx(e, pos, null)); }; - const stopDraw = () => { + // The event is optional: onPointerLeaveCanvas ends a stroke without one to name. + const stopDraw = (e?: { pointerId?: number } | null) => { + // A second pointer lifting is not this gesture ending. A palm coming off the glass used + // to commit the pen's stroke mid-line, which is what made strokes break up (#338). + if (!gesture.isOurs(e)) return; // A whole-layer move commits its offset into every stroke. if (layerDrag.end()) return; // Curve ruler: one anchor placed or fine-tuned; the done button commits it. diff --git a/src/hooks/useGesture.ts b/src/hooks/useGesture.ts index 2bca271..d113fe4 100644 --- a/src/hooks/useGesture.ts +++ b/src/hooks/useGesture.ts @@ -17,6 +17,30 @@ export type Gesture = ReturnType; // and re-rendering React on each one is the difference between a pen that keeps up and one that // does not. Nothing here should ever be read during render. +/** + * Whether a pointer event belongs to the gesture in flight. + * + * A tablet has more than one pointer. A palm resting on the glass while the pen draws is a + * second one, and while `startDraw` refused to *begin* a gesture from a touch, nothing stopped a + * touch from feeding the gesture already running: the palm's moves were appended to the pen's + * stroke as points, and the palm lifting ran stopDraw and committed the stroke early - after + * which the pen kept moving over a gesture that had already ended. That is #338's "strokes break + * up", and it cannot happen on a PC, where there is only ever one pointer. + * + * While nothing is in flight every pointer passes, so hovering still updates the cursor. A call + * made from code rather than from an event passes too - `onPointerLeaveCanvas` ends a stroke + * without having an event to name. + * + * @param {boolean} active whether a gesture is in flight + * @param {number|null} ownerId the pointer that began it + * @param {number|null} [eventId] the pointer the event came from; absent for a call from code + * @returns {boolean} + */ +export function pointerIsOwner(active: boolean, ownerId: number | null, eventId?: number | null): boolean { + if (!active || ownerId === null || eventId === null || eventId === undefined) return true; + return eventId === ownerId; +} + /** * @param {object} opts * @param {{current: HTMLCanvasElement|null}} opts.canvasRef the element that captures the pointer @@ -63,11 +87,17 @@ export function useGesture({ canvasRef }: { canvasRef: { current: HTMLCanvasElem pointerId.current = null; }; + /** + * Whether an event is from the pointer that began what is in flight - the guard every + * handler needs on a touchscreen. See pointerIsOwner for what it is protecting against. + */ + const isOurs = (e?: { pointerId?: number } | null) => pointerIsOwner(drawing.current, pointerId.current, e?.pointerId); + // One object, made once. Effects that read a gesture ref have to list this in their // dependencies, and a fresh object each render would make every one of them run every // render. Everything in here is a ref or closes over one, so there is nothing to refresh. return useMemo( - () => ({ drawing, pointerId, stroke, lineStart, lasso, layerDrag, selectionDrag, pathPts, target, clearPending, begin, end }), + () => ({ drawing, pointerId, stroke, lineStart, lasso, layerDrag, selectionDrag, pathPts, target, clearPending, begin, end, isOurs }), [], // eslint-disable-line react-hooks/exhaustive-deps -- refs and functions over refs ); } diff --git a/test/hooks/gesturePointer.test.js b/test/hooks/gesturePointer.test.js new file mode 100644 index 0000000..6eeb367 --- /dev/null +++ b/test/hooks/gesturePointer.test.js @@ -0,0 +1,42 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { pointerIsOwner } from '../../src/hooks/useGesture.ts'; + +// #338, the "strokes break up on the tablet" half. +// +// A PC has one pointer, so every event on the canvas belonged to the stroke being drawn and no +// handler had to ask. A tablet has several: the pen, and the palm resting on the glass beside +// it. `startDraw` refused to *begin* a gesture from a touch - that is the palm rejection that +// has always been there - but nothing stopped a touch from feeding the gesture already running. +// +// So while the pen drew: the palm's moves were appended to the pen's stroke as points, putting a +// jump across the drawing, and the palm lifting ran stopDraw and committed the stroke early. The +// pen then went on moving over a gesture that had ended, drawing nothing until it was lifted and +// put down again. A line in pieces, only ever on the tablet. + +test('while a gesture is in flight, only the pointer that began it is its own', () => { + assert.equal(pointerIsOwner(true, 1, 1), true); + assert.equal(pointerIsOwner(true, 1, 2), false, 'the palm got to drive the pen\'s stroke'); +}); + +test('with nothing in flight every pointer passes, so hovering still updates the cursor', () => { + // onDraw does its hover work through the same guard. Refusing a pointer here would leave the + // cursor stale over a selection's resize handles, which is the bug the hover branch fixed. + assert.equal(pointerIsOwner(false, null, 7), true); + assert.equal(pointerIsOwner(false, 1, 7), true, 'not drawing: an owner left over is not a reason to refuse'); +}); + +test('a call made from code, with no event to name, is always allowed through', () => { + // onPointerLeaveCanvas ends a stroke without an event. If the guard refused those, leaving + // the canvas mid-stroke would no longer stop drawing. + assert.equal(pointerIsOwner(true, 1, undefined), true); + assert.equal(pointerIsOwner(true, 1, null), true); +}); + +test('pointer id 0 is a real pointer, not a missing one', () => { + // The guard cannot test `eventId` for truthiness: a first pointer is legitimately id 0 on + // some browsers, and `!0` would hand every stroke to whichever pointer came second. + assert.equal(pointerIsOwner(true, 0, 0), true); + assert.equal(pointerIsOwner(true, 0, 3), false); + assert.equal(pointerIsOwner(true, 3, 0), false); +});