Closed Bug 1498873 Opened Last year Closed Last year

Demo with depth change bounces instead of smooth animation

Categories

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

Unspecified
All
defect

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox-esr60 --- unaffected
firefox62 --- disabled
firefox63 --- disabled
firefox64 --- fixed

People

(Reporter: mayankleoboy1, Assigned: emilio)

References

(Blocks 1 open bug, )

Details

(Keywords: nightly-community, regression)

Attachments

(3 files, 3 obsolete files)

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
Priority: -- → P2
Attached file Standalone test case (obsolete) —
Doesn't happen with layers.async-pan-zoom.enabled;false.
OS: Unspecified → All
Interestingly enough it seems to jitter more when the mouse is moving.
Flags: needinfo?(kats)
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.
Yes, this test case does not use fallback, so it seems like an independent bug in WebRender to me.
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 :)
Assignee: nobody → emilio
Attached file A bit more reduced. (obsolete) —
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"`.
Attachment #9017010 - Attachment is obsolete: true
Err, a transitions issue I meant. It probably is a sort of transforms issue.
Attached file Even more reduced. (obsolete) —
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.
Attachment #9017024 - Attachment is obsolete: true
Ah, forgot to mention, the body being a flex container also matters, which is really weird.
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.
Attachment #9017029 - Attachment is obsolete: true
(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.
Flags: needinfo?(emilio)
Disabling RDL does fix this as noticed by Matt on IRC.
Sounds like you have a handle on this. I don't have any hunch offhand about the bug.
Flags: needinfo?(kats)
On a debug build this hits the assertion from bug 1422908.
See Also: → 1422908
Ok, I know what's going on, this is a frame constructor bug, yay.
Flags: needinfo?(emilio)
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...
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
Attached file Simplify a check.
If I'm reading the code right this shouldn't change behavior, and matches what
we do in other callers.
Pushed by emilio@crisal.io:
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
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/13588 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
https://hg.mozilla.org/mozilla-central/rev/b77bde545276
https://hg.mozilla.org/mozilla-central/rev/f6404da759c5
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Flags: qe-verify-
Depends on: 1513908
Duplicate of this bug: 1428263
You need to log in before you can comment on or make changes to this bug.