If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

We're still repainting more often than the displayport changes during APZ scrolling

RESOLVED DUPLICATE of bug 1192910

Status

()

Core
Panning and Zooming
RESOLVED DUPLICATE of bug 1192910
2 years ago
2 years ago

People

(Reporter: botond, Unassigned)

Tracking

Trunk
x86_64
Linux
Points:
---

Firefox Tracking Flags

(firefox46 affected)

Details

(Reporter)

Description

2 years ago
STR:
  1. Run Nightly on Linux
  2. Load the test page from bug 1238571
     (https://bug1238571.bmoattachments.org/attachment.cgi?id=8706396)
  3. Scroll around with the mouse wheel

Expected results:
  As per bug 1187432, we only repaint when the displayport changes

Actual results:
  We repaint on almost every composite
(Reporter)

Comment 1

2 years ago
diagnosis-part1
When calculating the position of the displayport relative to the scroll port, we factor in the current velocity [1]. While the displayport remains the same size, this results in the distribution of that size between the top and bottom margins to shift slightly from composite to composite. As a result, we fail to return early from SetDisplayPortMargins() here [2], and schedule a paint.

[1] https://dxr.mozilla.org/mozilla-central/rev/5acc2a44834ce0614f98466475e674517daf0041/gfx/layers/apz/src/AsyncPanZoomController.cpp#2722
[2] https://dxr.mozilla.org/mozilla-central/rev/5acc2a44834ce0614f98466475e674517daf0041/layout/base/nsLayoutUtils.cpp#1154
(Reporter)

Comment 2

2 years ago
(In reply to Botond Ballo [:botond] from comment #1)
> When calculating the position of the displayport relative to the scroll
> port, we factor in the current velocity [1]. While the displayport remains
> the same size, this results in the distribution of that size between the top
> and bottom margins to shift slightly from composite to composite.

Actually, this is not the only source of small shifts in the display port margins. Even with apz.velocity_bias set to 0, which prevents the shift described above, SetDisplayPortMargins() is seeing small variations.
(Reporter)

Comment 3

2 years ago
diagnosis-part2
The other source of small shifts in the display port margins is AdjustDisplayPortForScrollDelta() [1], which takes the small (fractional) difference between the scroll offset we tried to set on Gecko, and the one it actually accepted, and adjusts the display port margins by that value.

[1] https://dxr.mozilla.org/mozilla-central/rev/5acc2a44834ce0614f98466475e674517daf0041/gfx/layers/apz/util/APZCCallbackHelper.cpp#143
(Reporter)

Comment 4

2 years ago
With the above two issues worked around, we are no longer painting on every composite, as expected.

If these two issues aren't occurring on OS X (as suggested by Markus' observations in bug 1238571), I'd be curious to know why.
Interesting diagnosis!

(In reply to Botond Ballo [:botond] from comment #4)
> If these two issues aren't occurring on OS X (as suggested by Markus'
> observations in bug 1238571), I'd be curious to know why.

I have apz.velocity_bias set to zero in my profile, and I'm scrolling with the touchpad, which sends scroll deltas in integer pixel increments.

The patch in bug 1192910 will fix this bug as well, I think.
(Reporter)

Comment 6

2 years ago
(In reply to Markus Stange [:mstange] from comment #5)
> The patch in bug 1192910 will fix this bug as well, I think.

Confirmed that this is the case. Closing as dupe.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1192910
You need to log in before you can comment on or make changes to this bug.