Closed Bug 1463139 Opened 2 years ago Closed 2 years ago

One-touch-pinch to zoom gesture (aka double-tap-drag) begins after a delay but zoom calculation does not take that delay into account, leading to a visual snap when zoom suddenly jumps from 1.00 to 1.05 (for example)


(Core :: Panning and Zooming, defect)

60 Branch
Not set



Tracking Status
firefox62 --- fixed


(Reporter: adam.walling, Assigned: adam.walling)




(1 file, 1 obsolete file)

Steps to reproduce:
Double-tap-drag zoom in and out.

Expected result
After a short delay, the touch gesture is interpreted as being a one-touch-pinch and zooming begins, starting at no zoom (1.00) and smoothly increasing (or decreasing) in proportion to the drag distance.

Actual result
Since there is a brief delay before the gesture begins, there is also a brief distance dragged before the gesture begins as well. When the zooming starts, it does so as if the zoom was running since the very first tap. This causes the perception of lost frames or an animation jerk since the zoom starts immediately with a non-zero distance yielding (eg) 1.05 zoom instead of 1.00.

The calculation of the distance dragged for the zoom should use the delta of the current touch position and the position at which the gesture began (after the delay) rather than the position that was initially tapped.

This behavior better emulates the default gesture behavior on Android and, for example, within Chrome.

Originally observed on Firefox 60 on Android (Galaxy Note 5) and reproduced on myriad devices.
Depends on: 1111333
Component: Toolbar → Panning and Zooming
Product: Firefox for Android → Core
Version: Firefox 60 → 60 Branch
Attached patch appears to work great on the devices I've tested with, though those are narrow (Galaxy Note 5, Galaxy S9)

I can adb shell screenrecord later if necessary to show the difference between the two (firefox beta vs build with this patch for example)
Attachment #8979292 - Flags: review?(bugmail)
Assignee: nobody → adam.walling
Ever confirmed: true
Comment on attachment 8979292 [details] [diff] [review]
calculate Y-span for one-touch-pinch gesture from point that gesture began

Review of attachment 8979292 [details] [diff] [review]:

I can't say I've noticed this behaviour, but the patch looks reasonable enough. One small naming nit below. With that addressed I'm happy to land the patch.

::: gfx/layers/apz/src/GestureEventListener.h
@@ +215,5 @@
> +   * the threshold for the gesture, there is already a span that the zoom
> +   * is calculated from, instead of starting at 1.0 when the threshold gets
> +   * passed.
> +   */
> +  ParentLayerPoint mTouchOneTouchPinchPosition;

I'd prefer to call this "mOneTouchPinchStartPosition" rather than having "touch" in the name twice.
Attachment #8979292 - Flags: review?(bugmail) → review+
Hi Adam, are you planning on updating the patch to address the review comment above?
Flags: needinfo?(adam.walling)
I had to recreate my build environment but I'll submit the new patch before too long.
Flags: needinfo?(adam.walling)
Comment on attachment 8984846 [details] [diff] [review]
renamed mTouchOneTouchPinchPosition to mOneTouc hPinchStartPosition

Looks good, thanks!
Attachment #8984846 - Flags: review+
Attachment #8979292 - Attachment is obsolete: true
Pushed by
calculate Y-span for one-touch-pinch gesture from point that gesture began (which occurs after a brief delay) instead of first touch point. r=kats
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.