Closed Bug 1236046 Opened 4 years ago Closed 4 years ago
[checkerboard-experiment] Velocity-based display port position prediction is erratic
Steps to reproduce: 0. Turn on apz.minimap.enabled. 1. Go to http://hpmor.com/chapter/6 and scroll to the middle of the page. 2. On your Macbook Pro touchpad, alternatingly scroll up and down, lifting your fingers from the touchpad every time you change the scroll direction. Basically a series of small flings. 3. Observe the display port position in the minimap. The displayport sometimes randomly jumps to the top or the bottom of the page, and in general looks very twitchy. Setting apz.velocity_bias to zero is a workaround that works.
This is basically a more general description of the specific bug 1235906 that Jeff filed yesterday. I'll dupe that over.
Duplicate of this bug: 1235906
Once the telemetry is in we can land a patch to drop the velocity bias to 0 and see how it affects things.
Depends on: 1238040
Summary: Velocity-based display port position prediction is erratic → [checkerboard-experiment] Velocity-based display port position prediction is erratic
Duplicate of this bug: 1237791
So I dug into this a bit more and the first move event definitely can generate some very crazy velocities. I expanded the logging in Axis::UpdateWithTouchAtDevicePoint to also log the distance and time deltas that result in the |newVelocity| computation and I see that sometimes the time delta is 1ms (which is what results in the giant velocities) and in fact sometimes it is -1ms. Individual move events tend to be a more reasonable 16-17ms apart. Fixing the velocity computation should fix this bug, I think.
Assignee: nobody → bugmail.mozilla
Can you try this patch and see if you still encounter the problem? I don't see it any more with this patch applied. (Note you'll want to set velocity bias back to 1.0 to make sure).
Attachment #8713373 - Flags: review?(mstange)
Comment on attachment 8713373 [details] [diff] [review] Patch Markus is going on PTO. Jeff, can you apply this patch and see if it fixes your velocity bias problems?
Attachment #8713373 - Flags: review?(mstange) → feedback?(jmuizelaar)
Here's a more cleaned-up patch that can actually be landed.
This is in the Feb05 nightly onwards, so we should check telemetry after a few days to see if it made a difference in checkerboarding.
Based on the telemetry data I can't see a visible improvement. However it also didn't make anything worse, and while scrolling around on pages I see the displayport jump erratically much less, so I think this is worth uplifting anyway.
Comment on attachment 8715316 [details] [diff] [review] Patch (cleaned up) Approval Request Comment [Feature/regressing bug #]: APZ [User impact if declined]: When starting a scroll the displayport can jump erratically, causing checkerboarding. This patch fixes the velocity computation which fixes the erratic jumps, and should reduce instances of checkerboarding. [Describe test coverage new/current, TreeHerder]: locally, on m-c for a while now [Risks and why]: low risk, it just avoids sampling velocity over very small timespans. Code is well understood. [String/UUID change made/needed]: none
Attachment #8715316 - Flags: approval-mozilla-aurora?
Depends on: 1246056
Comment on attachment 8715316 [details] [diff] [review] Patch (cleaned up) Sounds like this may avoid some apz scrolling issues, please uplift to aurora.
Attachment #8715316 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.