All users were logged out of Bugzilla on October 13th, 2018

Remove incorrect 'devicePixelRatio' usage in gesture handling.

RESOLVED FIXED in 2.6 S2 - 12/4

Status

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mcav, Assigned: mcav)

Tracking

unspecified
2.6 S2 - 12/4
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [systemsfe])

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
We should not use devicePixelRatio when calculating gesture distance thresholds. (I am equally guilty!)

Touch Event coordinates are already DIP (device-independent pixels), corresponding to the same pixels we'd see in CSS. Including devicePixelRatio is incorrect and causes device-specific differences.

We've mostly gotten this right, except for a few instances, including one by me. :)
Created attachment 8693821 [details] [review]
[gaia] mcav:dpr > mozilla-b2g:master
Whiteboard: [systemsfe]
(Assignee)

Comment 2

3 years ago
Comment on attachment 8693821 [details] [review]
[gaia] mcav:dpr > mozilla-b2g:master

Please correct me if I misunderstand what's going on here:

const kSignificant = 5 * window.devicePixelRatio;

On flame, that's 7.5 DIP; on Aries, 11.25 DIP. (Where DIP = device-independent pixels). However, when we do the math for these constants, we're comparing them to touch event coordinates, which are already DIP, not device pixels. 

So if we assume that devicePixelRatio is scaled such that interface elements are the same physical size on Aries and Flame, the bug is that kSignificant is physically, arbitrarily larger on an Aries than a Flame.

In other words, we shouldn't be multiplying by devicePixelRatio in these places, because touch event coordinates are already multiplied by devicePixelRatio.

In bug 1170863, this made things better because it made the touch area bigger, both on Flame _and_ Aries, but both by different amounts.

For this patch, I just chose to assume an average devicePixelRatio of 2 as a happy medium, hence the values are doubled. I could be convinced to change it to other constant values.
Attachment #8693821 - Flags: review?(etienne)
(Assignee)

Updated

3 years ago
Assignee: nobody → m
Comment on attachment 8693821 [details] [review]
[gaia] mcav:dpr > mozilla-b2g:master

Makes sense.
I think it worked out before because screens with higher DPI tend to have better touchscreens too.
Attachment #8693821 - Flags: review?(etienne) → review+
Target Milestone: --- → 2.6 S2 - 12/4
(Assignee)

Comment 4

3 years ago
master: https://github.com/mozilla-b2g/gaia/commit/a287b7383e3666cac446c1d2c503c53453f76bed
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.