1727016 - slow scroll leaves the image in a fixed position until its heights has passed

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect, P2)

Description

[Karl Dubost💡 :karlcow]

1. Go to https://performive.com/managed-cloud/private-cloud
2. Slow scroll

Actual:
some images stay in a fixed position until they slide in place after a bit.

Expected:
Images scroll with text.

This is not happening on Safari or Chrome.
I wonder if it's related or a duplicate of Bug 1492155

See the analysis on
https://github.com/webcompat/web-bugs/issues/83776

Comment 1

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=a518405abe4c93e6d8df622f59d5f01c4766e1d5&tochange=a09e4ce9ebba8fa4a29a4833560c8ee807018c03

Comment 2

[Timothy Nikkel (:tnikkel)]

Regression range with allow_zooming prefed on

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=31fb4a2a69124b6a21e5d8f44a0407d489749407&tochange=47f18d1138df7f10a4d6a0a92d00e5b7cfc8ca42

Probably either bug 1648500 or bug 1647034

Comment 3

[Timothy Nikkel (:tnikkel)]

Guessing

https://hg.mozilla.org/mozilla-central/rev/9505095bc965807c39cf71f27c3eba4a9e6fa9b2

Comment 4

[Timothy Nikkel (:tnikkel)]

Attachment: mostly reduced testcase

Comment 5

[Timothy Nikkel (:tnikkel)]

(In reply to Timothy Nikkel (:tnikkel) from comment #2)

Regression range with allow_zooming prefed on

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=31fb4a2a69124b6a21e5d8f44a0407d489749407&tochange=47f18d1138df7f10a4d6a0a92d00e5b7cfc8ca42

Probably either bug 1648500 or bug 1647034

(In reply to Timothy Nikkel (:tnikkel) from comment #3)

Guessing

https://hg.mozilla.org/mozilla-central/rev/9505095bc965807c39cf71f27c3eba4a9e6fa9b2

These guesses were wrong. It's actually https://hg.mozilla.org/mozilla-central/rev/96cd358e3eedc981b0bf4e00294ed73ce099d1c0 (Bug 1649270 - Add Navi12, Navi14, Arcturus and Renoir to AMD WebRender rollout. r=aosmond) which enabled webrender on my machine.

With webrender and allow_zooming both preffed on the testcase fails as far back as 2018-06-01 when webrender was still being stood up, not sure a proper regression range is do-able or useful.

Comment 6

[Timothy Nikkel (:tnikkel)]

Whether or not we build the async zoom display item is what triggers this, not sure why though.

Comment 7

[Timothy Nikkel (:tnikkel)]

The wr animation prop that we create here

https://searchfox.org/mozilla-central/rev/4cca5d2f257c6f1bcef50a0debcbd66524add703/layout/painting/nsDisplayList.cpp#5993

in nsDisplayOwnLayer::CreateWebRenderCommands is what causes this.

Oddly, if I make all references to that zoom animation id assume the id is not set in APZCTreeManager::SampleForWebRender then the problem still exists.

Comment 8

[Timothy Nikkel (:tnikkel)]

And the values we set in APZCTreeManager::SampleForWebRender seem to be nice and smoothly increasing.

Comment 9

[Timothy Nikkel (:tnikkel)]

It seems as though the WebRenderCommandBuilder/ClipManager code is giving it the wrong spatial id (which is linked to the scroll id), so webrender doesn't know that it scrolls when we scroll the root scroll frame, and only updates it when it gets a new display list from content.

Comment 10

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1727016. nsDisplayMasksAndClipPaths::CreateWebRenderCommands should only instruct WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList to create an override for the current ASR if it created a StackingContextHelper. r?miko

When we call WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList one of the first things it does is call mClipManager.BeginList(aSc) which checks the StackingContextHelper to see if it needs to push an override for the scrollid of the current ASR. The problem is that sometimes we don't create a new StackingContextHelper, so we are pushing an override for a different ASR but for the same StackingContextHelper we've already pushed an override for.

In this case nsDisplayMasksAndClipPaths::CreateWebRenderCommands sometimes creates a StackingContextHelper, and sometimes it does not: https://searchfox.org/mozilla-central/rev/1e7f7235cf822e79cd79ba9e200329ede3d37925/layout/painting/nsDisplayList.cpp#8117 when it does not create a StackingContextHelper (in this case because the clip-path was not found) we experience the bug.

I audited other callers of WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList and fixed the ones that also do not create a StackingContextHelper.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8d3b9fa4a1e7
nsDisplayMasksAndClipPaths::CreateWebRenderCommands should only instruct WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList to create an override for the current ASR if it created a StackingContextHelper. r=miko

Comment 12

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/8d3b9fa4a1e7