diff --git a/src/display/touch_manager.js b/src/display/touch_manager.js index de7230e514ca6..f97fa4d15d4ff 100644 --- a/src/display/touch_manager.js +++ b/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(); diff --git a/test/unit/touch_manager_spec.js b/test/unit/touch_manager_spec.js index 5f341321f0420..9fad6c4c5f028 100644 --- a/test/unit/touch_manager_spec.js +++ b/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); diff --git a/web/app.js b/web/app.js index 62d0a4da0ea48..f4fa774edb22b 100644 --- a/web/app.js +++ b/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; }, /**