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)

RESOLVED FIXED in Firefox 62

Status

()

defect
RESOLVED FIXED
a year ago
a year ago

People

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

Tracking

60 Branch
mozilla62
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox62 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Assignee

Description

a year ago
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.

Environment:
Originally observed on Firefox 60 on Android (Galaxy Note 5) and reproduced on myriad devices.
Assignee

Updated

a year ago
Depends on: 1111333
Component: Toolbar → Panning and Zooming
Product: Firefox for Android → Core
Version: Firefox 60 → 60 Branch
Assignee

Comment 2

a year ago
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
Status: UNCONFIRMED → NEW
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)
Assignee

Comment 5

a year ago
I had to recreate my build environment but I'll submit the new patch before too long.
Assignee

Comment 6

a year ago
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

Comment 8

a year ago
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/90d7fb47fbe9
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

Comment 9

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/90d7fb47fbe9
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.