1528180 - above-the-fold animations in scrollport (of e.g. 'height') can cause visible elements to jitter if WebRender is enabled

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect, P3)

Description

[Alice0775 White]

+++ This bug was initially created as a clone of Bug #1527899 +++

Build ID 20190215045130
User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:67.0) Gecko/20100101 Firefox/67.0

STR:
0. WebRender turn on and restart

1. Launch Firefox.
2. Navigate to testcase of Bug 1519553 attachment 9036063 [details]
3. Scroll up by 20-30px with drag scrollbar thumb
   or Scroll up or down with mouse wheel one notch
4. Observe

(optionally if not reproduced)
5. Repeat step3

ER: Solid scroll position
AR: The contents jitter.

Comment 1

[Alice0775 White]

Attachment: about:support

Comment 2

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

Hi :alice, I noticed you had set a priority on this bug, normally the triage owner would be doing that, but I wasn't sure if you were helping out :jbonisteel's team with WebRender triaging.

Thanks!

Comment 3

[Alice0775 White]

(In reply to Emma Humphries, Bugmaster ☕️🎸🧞‍♀️✨ (she/her) [:emceeaich] (UTC-8) needinfo? me from comment #2)

Hi :alice, I noticed you had set a priority on this bug, normally the triage
owner would be doing that, but I wasn't sure if you were helping out
:jbonisteel's team with WebRender triaging.

Thanks!

Sorry. I forgot to clear the flag.
|Clone This Bug| inherits the flag....

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Taking since I'm looking at this for the moment. Andrew was also looking at more general snapping, so this may also be interesting to him.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

So I did a bit of debugging. What's going on is:

• Scroll anchoring adjusts to the offset of the frame.
• APC requests a fractionally different offset:

(rr) p geckoScrollPosition
$7 = {<mozilla::gfx::BasePoint<float, mozilla::gfx::PointTyped<mozilla::CSSPixel, float>, mozilla::gfx::CoordTyped<mozilla::CSSPixel, float> >> = {{{x = 0, y = 290.683319}, components = {0, 
        290.683319}}}, <mozilla::CSSPixel> = {<No data fields>}, <No data fields>}
(rr) p targetScrollPosition
$8 = {<mozilla::gfx::BasePoint<float, mozilla::gfx::PointTyped<mozilla::CSSPixel, float>, mozilla::gfx::CoordTyped<mozilla::CSSPixel, float> >> = {{{x = 0, y = 290.68335}, components = {0, 
        290.68335}}}, <mozilla::CSSPixel> = {<No data fields>}, <No data fields>}

• The APC scrolling ends up in ScrollToImpl, where we change the offset, invalidate the anchor, and start again.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1528180 - Don't align scroll offsets to layer pixels when using WebRender. r=kats,jrmuizel

This position is only invalidated from FrameLayerBuilder, that's not a very good
signal.

This would get out of sync when using WebRender, and APZ messages will send
sub-pixel scroll positions that will get wrongly adjusted, causing small scroll
offsets which scroll anchoring would incorrectly try to correct, which is the
ultimate cause of the jittering.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Mentioned this on IRC but: I'm only mildly confident in https://phabricator.services.mozilla.com/D37875 being the 100% correct fix, but I'm pretty sure that what we're doing right now is completely wrong. I'm unsure what the WebRender equivalent to the two calls of https://searchfox.org/mozilla-central/rev/15be167a5b436b57fef944b84eef061d24c1af8c/layout/painting/FrameLayerBuilder.cpp#2550 are, but an alternative that probably also works is calling that from the WebRender equivalents.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

(It is unclear there is a WebRender equivalent at all to the point where we create a layer for a scrollable frame. :))

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Try looks good btw: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ba18d6285d64968b1379ca41c5145784d797ece7

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a37238f046d5
Don't align scroll offsets to layer pixels when using WebRender. r=tnikkel

Comment 12

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/a37238f046d5

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9077691 [details]
Bug 1528180 - Don't align scroll offsets to layer pixels when using WebRender. r=kats,jrmuizel

Beta/Release Uplift Approval Request

• User impact if declined: Bug 1554861 + jittering in some pages.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: comment 0
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Very low effort patch that fixes a WR correctness bug. The risk of this change having fallout is pretty low I'd say.
• String changes made/needed: none

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9077691 [details]
Bug 1528180 - Don't align scroll offsets to layer pixels when using WebRender. r=kats,jrmuizel

Fixes a scroll anchoring bug with WebRender. Approved for 69.0b6.

Comment 17

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a60290b9de2c

Comment 18

[Hani Yacoub, Desktop QA]

Verified as fixed on Firefox 70.0a1 (2019-07-17) on Windows 10 x 64, Windows 7 x32, Mac OS X 10.14 and on Ubuntu 18.04 x64.

Comment 19

[Hani Yacoub, Desktop QA]

Verified as fixed on Firefox 69.0b6 on Windows 10 x 64, Mac OS X 10.14 and on Ubuntu 18.04 x64.