fix: a palm on the glass no longer drives the pen's stroke (#338)

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`
 

src/App.tsx

@@ -1488,6 +1488,9 @@ export default function App() {
     const [hoverHandle, setHoverHandle] = useState<string | null>(null);
 
     const onDraw = (e: React.PointerEvent<HTMLCanvasElement>) => {
+        // 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.

src/hooks/useGesture.ts

@@ -17,6 +17,30 @@ export type Gesture = ReturnType<typeof useGesture>;
 // 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
     );
 }

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);
+});