Avoid zooming during fast two-finger panning

src/display/touch_manager.js

@@ -84,6 +84,8 @@ class TouchManager {
 
   #touchMoveAC = null;
 
+  #unconfirmedPinch = 0;
+
   /**
    * @param {TouchManagerOptions} options
    */
@@ -127,6 +129,18 @@ class TouchManager {
     return 35 / OutputScale.pixelRatio;
   }
 
+  /**
+   * Once pinching, the span must change by at least 4 device pixels to update
+   * the scale: below that it's contact jitter (a moving finger is reported
+   * with a slight lag and its position is quantized), not an intentional
+   * pinch, hence a two-finger pan doesn't make the zoom drift.
+   *
+   * NOTE: Don't shadow this value since `devicePixelRatio` may change.
+   */
+  get MIN_TOUCH_DISTANCE_TO_SCALE() {
+    return 4 / OutputScale.pixelRatio;
+  }
+
   #onTouchStart(evt) {
     if (this.#isPinchingDisabled?.()) {
       return;
@@ -281,6 +295,9 @@ class TouchManager {
       // Distances use screen coordinates; zoom and scrolling use client ones.
       panX: (touch0.clientX + touch1.clientX) / 2,
       panY: (touch0.clientY + touch1.clientY) / 2,
+      // Screen-coordinate midpoint used by the sampling allowance.
+      screenPanX: (touch0.screenX + touch1.screenX) / 2,
+      screenPanY: (touch0.screenY + touch1.screenY) / 2,
     };
   }
 
@@ -330,13 +347,28 @@ class TouchManager {
     const dx = panX - pPanX;
     const dy = panY - pPanY;
 
+    // Measure midpoint motion in the same coordinates as the span.
+    const screenPanX = (screen0X + screen1X) / 2;
+    const screenPanY = (screen0Y + screen1Y) / 2;
+    const translation = Math.hypot(
+      screenPanX - touchInfo.screenPanX,
+      screenPanY - touchInfo.screenPanY
+    );
+    touchInfo.screenPanX = screenPanX;
+    touchInfo.screenPanY = screenPanY;
+
     const distance = Math.hypot(currGapX, currGapY);
     const pDistance = Math.hypot(prevGapX, prevGapY);
+    // Before pinching, moving one contact by d moves the midpoint by d / 2 and
+    // changes the span by at most d. Add that sampling allowance to the
+    // threshold.
+    const minDistance = this.#isPinching
+      ? this.MIN_TOUCH_DISTANCE_TO_SCALE
+      : this.MIN_TOUCH_DISTANCE_TO_PINCH + 2 * translation;
     if (
       distance < MIN_TOUCH_SPAN ||
       pDistance < MIN_TOUCH_SPAN ||
-      (!this.#isPinching &&
-        Math.abs(pDistance - distance) <= this.MIN_TOUCH_DISTANCE_TO_PINCH)
+      Math.abs(pDistance - distance) <= minDistance
     ) {
       // Keep the distance baseline so a slow pinch can cross the threshold, or
       // a degenerate span recover, but the midpoint still moved.
@@ -351,9 +383,12 @@ class TouchManager {
     touchInfo.touch1X = screen1X;
     touchInfo.touch1Y = screen1Y;
 
+    const direction = Math.sign(distance - pDistance);
+
     if (!this.#isPinching) {
       // Start pinching.
       this.#isPinching = true;
+      this.#unconfirmedPinch = direction;
 
       // Skip the first scale update, as before, but keep its translation.
       if (dx || dy) {
@@ -362,6 +397,23 @@ class TouchManager {
       return;
     }
 
+    if (this.#unconfirmedPinch) {
+      const unconfirmed = this.#unconfirmedPinch;
+      this.#unconfirmedPinch = 0;
+      if (
+        direction !== unconfirmed &&
+        Math.abs(distance - pDistance) <= 2 * translation
+      ) {
+        // This reversal matches a lagging contact catching up: its span change
+        // is at most twice the midpoint movement. Discard the tentative pinch.
+        this.#isPinching = false;
+        if (dx || dy) {
+          this.#onPanning?.(dx, dy);
+        }
+        return;
+      }
+    }
+
     // The distances are in screen CSS pixels, but the origin must be in client
     // coordinates, like the one coming from a wheel event.
     // Zoom around the previous midpoint and apply its movement in the same
@@ -399,6 +451,7 @@ class TouchManager {
     // `#setTouchInfo` may already have cleared the baseline.
     this.#touchInfo = null;
     this.#isPinching = false;
+    this.#unconfirmedPinch = 0;
     this.#ownsGesture = false;
     if (this.#touchMoveAC) {
       this.#touchMoveAC.abort();

test/unit/touch_manager_spec.js

@@ -29,6 +29,14 @@ describe("TouchManager", function () {
     };
   }
 
+  // A pair with the given span and midpoint.
+  function pair(span, center = 100, y = 0) {
+    return [
+      makeTouch(0, center - span / 2, y),
+      makeTouch(1, center + span / 2, y),
+    ];
+  }
+
   class TouchManagerHelper {
     #ac = new AbortController();
 
@@ -177,25 +185,25 @@ describe("TouchManager", function () {
     helper.dispatch("touchstart", [touch0, touch1], [touch1]);
 
     // Past the dead zone: the first move only re-baselines...
-    const spread1 = makeTouch(1, 400);
-    helper.dispatch("touchmove", [touch0, spread1], [spread1]);
+    const spread1 = pair(400);
+    helper.dispatch("touchmove", spread1, spread1);
     expect(helper.pinchings).toEqual([]);
 
     // ...and the second one is reported, hence pinching is in progress.
-    const spread2 = makeTouch(1, 600);
-    helper.dispatch("touchmove", [touch0, spread2], [spread2]);
+    const spread2 = pair(600);
+    helper.dispatch("touchmove", spread2, spread2);
     expect(helper.pinchings.length).toEqual(1);
 
     // A third finger lands and is lifted right away.
     const touch2 = makeTouch(2, 500, 400);
-    helper.dispatch("touchstart", [touch0, spread2, touch2], [touch2]);
-    helper.dispatch("touchend", [touch0, spread2], [touch2]);
+    helper.dispatch("touchstart", [...spread2, touch2], [touch2]);
+    helper.dispatch("touchend", spread2, [touch2]);
 
     // The pinch is still in progress, hence a move well inside the dead zone is
     // still reported instead of having to earn it all over again.
     const nudge = minDistance / 2;
-    const spread3 = makeTouch(1, 600 + nudge);
-    helper.dispatch("touchmove", [touch0, spread3], [spread3]);
+    const spread3 = pair(600 + nudge);
+    helper.dispatch("touchmove", spread3, spread3);
     expect(helper.pinchings.length).toEqual(2);
     expect(helper.pinchings[1].prevDistance).toEqual(600);
     expect(helper.pinchings[1].distance).toBeCloseTo(600 + nudge);
@@ -211,16 +219,16 @@ describe("TouchManager", function () {
 
     helper.dispatch("touchstart", [touch0], [touch0]);
     helper.dispatch("touchstart", [touch0, touch1], [touch1]);
-    const spread1 = makeTouch(1, 400);
-    const spread2 = makeTouch(1, 600);
-    helper.dispatch("touchmove", [touch0, spread1], [spread1]);
-    helper.dispatch("touchmove", [touch0, spread2], [spread2]);
+    const spread1 = pair(400);
+    const spread2 = pair(600);
+    helper.dispatch("touchmove", spread1, spread1);
+    helper.dispatch("touchmove", spread2, spread2);
     expect(helper.pinchings.length).toEqual(1);
 
     // A third finger breaks the pair, and then everything is lifted.
     const touch2 = makeTouch(2, 500, 400);
-    helper.dispatch("touchstart", [touch0, spread2, touch2], [touch2]);
-    helper.dispatch("touchend", [touch0], [spread2, touch2]);
+    helper.dispatch("touchstart", [...spread2, touch2], [touch2]);
+    helper.dispatch("touchend", [touch0], [...spread2.slice(1), touch2]);
     helper.dispatch("touchend", [], [touch0]);
     expect(helper.pinchEnds).toEqual(1);
 
@@ -283,26 +291,25 @@ describe("TouchManager", function () {
     const helper = new TouchManagerHelper();
     const { MIN_TOUCH_DISTANCE_TO_PINCH: minDistance } = helper.manager;
     expect(minDistance).toBeGreaterThan(0);
-    const touch0 = makeTouch(0, 0);
-    const touch1 = makeTouch(1, 200);
+    const touches = pair(200);
 
-    helper.dispatch("touchstart", [touch0], [touch0]);
-    helper.dispatch("touchstart", [touch0, touch1], [touch1]);
+    helper.dispatch("touchstart", [touches[0]], [touches[0]]);
+    helper.dispatch("touchstart", touches, [touches[1]]);
 
-    // Stay below the pinch threshold; only midpoint movement is reported.
-    const nearly = makeTouch(1, 200 + minDistance - 1);
-    helper.dispatch("touchmove", [touch0, nearly], [nearly]);
+    // Moving the midpoint by one pixel adds two pixels to the allowance.
+    const nearly = pair(200 + minDistance - 1, 101);
+    helper.dispatch("touchmove", nearly, nearly);
     expect(helper.pinchings).toEqual([]);
     expect(helper.pannings.length).toEqual(1);
 
     // The original distance baseline lets this move cross the threshold.
-    const past = makeTouch(1, 200 + minDistance + 1);
-    helper.dispatch("touchmove", [touch0, past], [past]);
+    const past = pair(200 + minDistance + 5, 102);
+    helper.dispatch("touchmove", past, past);
     expect(helper.pinchings).toEqual([]);
     expect(helper.pannings.length).toEqual(2);
 
-    const further = makeTouch(1, 200 + minDistance + 21);
-    helper.dispatch("touchmove", [touch0, further], [further]);
+    const further = pair(200 + minDistance + 25, 103);
+    helper.dispatch("touchmove", further, further);
     expect(helper.pinchings.length).toEqual(1);
     expect(
       helper.pinchings[0].distance - helper.pinchings[0].prevDistance
@@ -311,6 +318,116 @@ describe("TouchManager", function () {
     helper.destroy();
   });
 
+  it("pans, without zooming, inside the dead zone while pinching", function () {
+    const helper = new TouchManagerHelper();
+    const {
+      MIN_TOUCH_DISTANCE_TO_PINCH: minDistance,
+      MIN_TOUCH_DISTANCE_TO_SCALE: minScaleDistance,
+    } = helper.manager;
+    expect(minScaleDistance).toBeGreaterThan(0);
+    expect(minScaleDistance).toBeLessThan(minDistance);
+    const touches = pair(200);
+
+    helper.dispatch("touchstart", [touches[0]], [touches[0]]);
+    helper.dispatch("touchstart", touches, [touches[1]]);
+
+    // Start pinching; the first scale update is skipped.
+    const baselineSpan = 200 + minDistance + 1;
+    const spread = pair(baselineSpan);
+    helper.dispatch("touchmove", spread, spread);
+    expect(helper.pinchings).toEqual([]);
+    expect(helper.pannings).toEqual([]);
+
+    // Alternate sub-threshold span changes while moving one pixel at a time.
+    for (let i = 1; i <= 4; i++) {
+      const jittered = pair(
+        baselineSpan + (i % 2 ? minScaleDistance / 2 : 0),
+        100 + i
+      );
+      helper.dispatch("touchmove", jittered, jittered);
+    }
+    expect(helper.pinchings).toEqual([]);
+    expect(helper.pannings).toEqual([
+      [1, 0],
+      [1, 0],
+      [1, 0],
+      [1, 0],
+    ]);
+
+    // The retained span baseline lets a larger change zoom.
+    const past = pair(baselineSpan + minScaleDistance + 20, 105);
+    helper.dispatch("touchmove", past, past);
+    expect(helper.pinchings.length).toEqual(1);
+    expect(
+      helper.pinchings[0].distance - helper.pinchings[0].prevDistance
+    ).toEqual(minScaleDistance + 20);
+
+    helper.destroy();
+  });
+
+  it("doesn't start pinching when the fingers are sampled one at a time", function () {
+    const helper = new TouchManagerHelper();
+    const { MIN_TOUCH_DISTANCE_TO_PINCH: minDistance } = helper.manager;
+    const touches = pair(200);
+
+    helper.dispatch("touchstart", [touches[0]], [touches[0]]);
+    helper.dispatch("touchstart", touches, [touches[1]]);
+
+    // Report each step of a rigid translation one contact at a time.
+    const step = minDistance + 5;
+    const [{ clientX: x0 }, { clientX: x1 }] = touches;
+    for (let i = 1; i <= 4; i++) {
+      const lagging = [
+        makeTouch(0, x0 + i * step),
+        makeTouch(1, x1 + (i - 1) * step),
+      ];
+      helper.dispatch("touchmove", lagging, [lagging[0]]);
+      const caughtUp = [
+        makeTouch(0, x0 + i * step),
+        makeTouch(1, x1 + i * step),
+      ];
+      helper.dispatch("touchmove", caughtUp, [caughtUp[1]]);
+    }
+
+    expect(helper.pinchings).toEqual([]);
+    expect(helper.pannings.reduce((sum, [dx]) => sum + dx, 0)).toEqual(
+      4 * step
+    );
+
+    helper.destroy();
+  });
+
+  it("un-latches a pinch started by a finger sampled late", function () {
+    const helper = new TouchManagerHelper();
+    const { MIN_TOUCH_DISTANCE_TO_PINCH: minDistance } = helper.manager;
+    const touches = pair(200);
+
+    helper.dispatch("touchstart", [touches[0]], [touches[0]]);
+    helper.dispatch("touchstart", touches, [touches[1]]);
+
+    // Leave the second contact two samples behind, so the cumulative span
+    // change exceeds the per-event allowance.
+    const step = minDistance + 5;
+    const [{ clientX: x0 }, { clientX: x1 }] = touches;
+    for (let i = 1; i <= 2; i++) {
+      const lagging = [makeTouch(0, x0 + i * step), makeTouch(1, x1)];
+      helper.dispatch("touchmove", lagging, [lagging[0]]);
+    }
+    expect(helper.pinchings).toEqual([]);
+
+    // Catching up reverses the span by twice the midpoint movement, so the
+    // tentative pinch is discarded.
+    const caughtUp = [makeTouch(0, x0 + 2 * step), makeTouch(1, x1 + 2 * step)];
+    helper.dispatch("touchmove", caughtUp, [caughtUp[1]]);
+    const next = [makeTouch(0, x0 + 3 * step), makeTouch(1, x1 + 2 * step)];
+    helper.dispatch("touchmove", next, [next[0]]);
+
+    expect(helper.pinchings).toEqual([]);
+    expect(helper.pannings.length).toEqual(4);
+
+    helper.destroy();
+  });
+
   it("reports the previous midpoint, in client coordinates, as the origin", function () {
     const helper = new TouchManagerHelper();
     const touch0 = makeTouch(0, 0, 500);

web/app.js

@@ -30,6 +30,8 @@ import {
   isValidRotation,
   isValidScrollMode,
   isValidSpreadMode,
+  MAX_SCALE,
+  MIN_SCALE,
   normalizeWheelEventDirection,
   parseQueryString,
   ProgressBar,
@@ -49,6 +51,7 @@ import {
   InvalidPDFException,
   isDataScheme,
   isPdfFile,
+  MathClamp,
   OutputScale,
   PDFWorker,
   ResponseException,
@@ -2642,17 +2645,16 @@ const PDFViewerApplication = {
     if (factor === 1) {
       return 1;
     }
-    // If the direction changed, reset the accumulated factor.
-    if ((this[prop] > 1 && factor < 1) || (this[prop] < 1 && factor > 1)) {
-      this[prop] = 1;
-    }
-
-    const newFactor =
-      Math.floor(previousScale * factor * this[prop] * 100) /
-      (100 * previousScale);
-    this[prop] = factor / newFactor;
+    // Carry scale-rounding error into the next factor.
+    const target = MathClamp(
+      previousScale * factor * this[prop],
+      MIN_SCALE,
+      MAX_SCALE
+    );
+    const newScale = Math.round(target * 100) / 100;
+    this[prop] = target / newScale;
 
-    return newFactor;
+    return newScale / previousScale;
   },
 
   /**