document splitting + RDL chrome: Page jumps up when hovering a different tab
Categories
(Core :: Graphics: WebRender, defect, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox69 | --- | disabled |
People
(Reporter: jan, Assigned: kats)
References
Details
(Keywords: nightly-community)
Attachments
(4 files)
Debian Testing, KDE, X11, Macbook Pro
Document splitting depends on bug 1413546, so I tested it.
Scrolled down by touchpad.
mozregression --launch 2019-05-28 --pref gfx.webrender.all:true gfx.webrender.split-render-roots:true layout.display-list.retain.chrome:true -a https://bugzilla.mozilla.org -a https://hg.mozilla.org/try/rev/2b6744c368170b768d913ef443407eb5f716ff59
Not reproducible if either document splitting or RDL chrome is disabled.
Updated•5 years ago
|
Comment 2•5 years ago
|
||
For clarity - this only seems to happen with both document splitting and parent process RDL enabled.
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Assignee | ||
Comment 4•5 years ago
|
||
I can repro on Windows, will investigate. At first glance it's also related to paint skipping, since I can't reproduce with apz.paint_skipping.enabled=false. That also explains why it jumps up just a little bit - it's probably reverting to the last un-paint-skipped scroll position.
Assignee | ||
Comment 5•5 years ago
|
||
Looking into this a bit deeper. Looks like when the tooltip appears on the background tab, it destroys the APZC tree and all the APZC instances. Then the subsequent transaction rebuilds it. But the rebuild (which presumably comes from a retained display list in the UI process) has the "old" scroll offset, because paint skipping means that was the last one that was baked into the display list.
The next question is why does the tooltip destroy/rebuild the APZC tree. This doesn't seem to happen with WR disabled, and I believe tooltips should get their own widgets and so would have a separate APZCTreeManager, if they have one at all. Probably somewhere in the document splitting code we're getting wires crossed and sending a DLs from one widget to another widget, or something along those lines.
Assignee | ||
Comment 6•5 years ago
|
||
More investigation debunked my theory about wires getting crossed. What's actually happening is that around the same time the tooltip shows up, the main window has another transaction, but the render root boundary object that's supposed to be persisted from one transaction to the next isn't persisted. In code terms, this call to EnsureHasBoundary emplaces a new boundary instead of reusing the old one (because the old one was destroyed for reasons I have yet to uncover). Because the boundary id is different, walking the WebRenderScrollDataWrapper tree on the compositor side doesn't work quite as expected because the chrome document's scene build finishes before the content document's scene build (i.e. we go into this codepath). This results in APZ getting a truncated tree which results in the destroy/rebuild in my last comment.
Assignee | ||
Comment 7•5 years ago
|
||
Ah, with retained display list building we apparently explicitly mark render roots as needing a build if they are invalidated here. And if they aren't marked as needing a build, they get skipped over here because the condition forces an early-exit. This causes the WebRenderUserData attached to the node to not be touched, and if it's not touched, it gets recycled at the end of the transaction. That triggers the new boundary creation on the next transaction. I think the whole tooltip thing was probably a red herring, the more important thing is that the background tab gets a highlight when you mouse over it which triggers the transaction sans content document and destroys the WebRenderUserData.
Assignee | ||
Comment 8•5 years ago
|
||
No functional changes here.
Assignee | ||
Comment 9•5 years ago
|
||
Depends on D36385
Assignee | ||
Comment 10•5 years ago
|
||
With retained display lists, a content render root might get marked as not
needing a build, in which case the nsDisplayRenderRoot::CreateWRCommands
function does an early exit. In this case, we don't mark the associated
WebRenderUserData as used during the display list build, which causes it to
get deleted at the end of the transaction. The next transaction that
doesn't early-exit will re-create the WebRenderUserData with a new boundary
object. The compositor therefore thinks it's a brand new thing and, if
conditions are right, could end up destroying and re-creating much of the
APZC tree. That in turn can have effects like discarding paint-skipped
scrolling.
This patch ensures we always touch the WebRenderUserData during the display
list build, so we don't discard it. This problem may still affect nested
nsDisplayRenderRoot instances but I don't think we ever cases where those
occur.
Depends on D36386
Comment 11•5 years ago
|
||
Comment 12•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8edcb5f6ceff
https://hg.mozilla.org/mozilla-central/rev/e16eda1824f7
https://hg.mozilla.org/mozilla-central/rev/5ebb4441aa24
Description
•