1498873 - Demo with depth change bounces instead of smooth animation

Status: RESOLVED FIXED

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

Description

[Mayank Bansal]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:64.0) Gecko/20100101 Firefox/64.0

Steps to reproduce:

enable WR
go to https://lab.hakim.se/avgrund/
click on the blue "Open Modal" button 


Actual results:

the demo bounces/shudders a couple of times before settling


Expected results:

no shudder/bounce
non-WR is smooth

Comment 1

[Jeff Muizelaar [:jrmuizel]]

Attachment: Standalone test case

Comment 2

[Darkspirit]

Doesn't happen with layers.async-pan-zoom.enabled;false.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

Interestingly enough it seems to jitter more when the mouse is moving.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

It might be related to bug 1498221 which I planned to investigate this week, but if Kats has a hunch about why this is that'd be amazing of course :)

That test-case uses fallback, but IIUC this one shouldn't.

Comment 5

[Markus Stange [:mstange]]

Yes, this test case does not use fallback, so it seems like an independent bug in WebRender to me.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Taking this tentatively as this is likely the same underlying issue as bug 1498221 and I'll look into that tomorrow morning. 

Though if somebody finds it earlier, please steal :)

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: A bit more reduced.

This is not a transforms issue, fwiw. I can reproduce flickering by causing a repaint in any way that is not the transition, like `document.documentElement.style.color = "white"`.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Err, a transitions issue I meant. It probably is a sort of transforms issue.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Attachment: Even more reduced.

No transitions needed indeed.

Seems like it is code getting confused about fixed pos containing blocks.

What the code is doing is adding a transform: scale(0.8) dynamically to the body, which causes it to become the fixed-pos CB of the cover.

If I either make that change not change the CB of the fixed-pos element, by virtue of adding an scale(1) transform to the body by default, or I remove the fixed-pos element (the cover), the bug suddenly disappears.

I don't know what differences exist between FLB and WR regarding the changes to fixed-pos containing blocks, so any pointers appreciated.

I'll try to reduce it further later / tomorrow.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Ah, forgot to mention, the body being a flex container also matters, which is really weird.

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Attachment: As reduced as it can get.

Now automated, could do a reftest out of this.

Removing the margins and centering of the popup clearly shows what's going on. 

Still not sure where the bug is of course.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #10)
> Ah, forgot to mention, the body being a flex container also matters, which
> is really weird.

Flex or grid are bogus, block is not o.O. That's really weird.

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Disabling RDL does fix this as noticed by Matt on IRC.

Comment 14

[Kartikaya Gupta (email:[e-mail])]

Sounds like you have a handle on this. I don't have any hunch offhand about the bug.

Comment 15

[Emilio Cobos Álvarez (:emilio)]

On a debug build this hits the assertion from bug 1422908.

Comment 16

[Emilio Cobos Álvarez (:emilio)]

Ok, I know what's going on, this is a frame constructor bug, yay.

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Attachment: Pass the right frame to PushAbsoluteContainingBlock to determine whether we're a fixed-pos containing block.

When we're creating a scrollframe with let's say, display: flex or grid, the
containing block is the grid container itself, but the transformed frame is the
scroll frame.

This is the only caller that (incorrectly) passes the same frame to
PushAbsoluteContainingBlock.

Our painting code deals with it, mostly, because it starts from the placeholder
to paint fixed items, and it hits the scrollframe, but it gets confused
sometimes causing the issue described here.

I'll find a way to add a crashtest for this, and maybe a reftest, though this
works in non-WR.

We should probably add a few more assertions to the frame constructor...

Comment 18

[Emilio Cobos Álvarez (:emilio)]

The reason it only happens on grid and flex containers is because grid items become pseudo-stacking-contexts:

https://searchfox.org/mozilla-central/rev/3a54520d8d2319a4116866371ed3d9ed2ec0cc2b/layout/generic/nsFlexContainerFrame.cpp#2465

Comment 19

[Emilio Cobos Álvarez (:emilio)]

Attachment: Simplify a check.

If I'm reading the code right this shouldn't change behavior, and matches what
we do in other callers.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b77bde545276
Pass the right frame to PushAbsoluteContainingBlock to determine whether we're a fixed-pos containing block. r=bzbarsky

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f6404da759c5
Simplify a check. r=bzbarsky

Comment 22

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/13588 for changes under testing/web-platform/tests

Comment 23

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 24

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/b77bde545276
https://hg.mozilla.org/mozilla-central/rev/f6404da759c5

Comment 25

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged