Summary: | REGRESSION (r245170): gmail.com inbox table header flickers | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||
Component: | Compositing | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | koivisto, simon.fraser, zalan | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Simon Fraser (smfr)
2019-05-17 15:11:14 PDT
Created attachment 370164 [details]
Testcase
I think the issue is that the layer for "header" uses "paints into composited ancestor" and paints via the root, but that ancestor is obscured behind an intermediate layer (the backdrop). S--------C-c- -- ------ ------ 0x57ef772b0 (0,0) width=875 height=835 (layerID 16) {sc 2} RenderView S----------c- -- ------ ------ + 0x57ef77408 (0,0) width=875 height=328 <html> S-------XC--- -- ------ ------ + 0x57ef77560 (10,10) width=550 height=350 (layerID 21) {vc 3} <div> class='backdrop' ---------C--- -- ------ ------ + 0x57ef776b8 (20,20) width=528 height=328 (layerID 22) <div> class='container' S-O------Cac- -- ------ ------ + 0x57ef77810 (14,14) width=504 height=304 (layerID 28) <div> class='header' ------------- -- ------ ------ + 0x57ef77968 (12,12) width=200 height=100 <div> class='target' S--A-----C--- -- ------ ------ + 0x57ef77ac0 (252,122) width=220 height=120 (layerID 27) <div> class='animating changed' Odd that we haven't seen this before, since the "paints into composited ancestor" logic has never consulted overlap. OIC. We used to leave an "overlap" indirect composited reason on the layer which prevented painting into an ancestor, but now with backing sharing that gets cleared. Subtle. Created attachment 370171 [details]
Patch
Comment on attachment 370171 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=370171&action=review > Source/WebCore/ChangeLog:16 > + Mak some logging changes to help diagnose things like this. Typo > Source/WebCore/rendering/RenderLayerCompositor.cpp:935 > + if (layerPaintsIntoProvidedBacking) { > + layerPaintsIntoProvidedBacking = false; > + // layerPaintsIntoProvidedBacking was only true for layers that would otherwise composite because of overlap. If we can > + // no longer share, put this this indirect reason back on the layer so that requiresOwnBackingStore() sees it. > + layer.setIndirectCompositingReason(IndirectCompositingReason::Overlap); It is bit non-obvious that layerPaintsIntoProvidedBacking implies IndirectCompositingReason::Overlap. Maybe rename it to layerPaintsIntoProvidedBackingDueToOverlap or similar to avoid future mistakes? (In reply to Antti Koivisto from comment #7) > Comment on attachment 370171 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=370171&action=review > > > Source/WebCore/rendering/RenderLayerCompositor.cpp:935 > > + if (layerPaintsIntoProvidedBacking) { > > + layerPaintsIntoProvidedBacking = false; > > + // layerPaintsIntoProvidedBacking was only true for layers that would otherwise composite because of overlap. If we can > > + // no longer share, put this this indirect reason back on the layer so that requiresOwnBackingStore() sees it. > > + layer.setIndirectCompositingReason(IndirectCompositingReason::Overlap); > > It is bit non-obvious that layerPaintsIntoProvidedBacking implies > IndirectCompositingReason::Overlap. Maybe rename it to > layerPaintsIntoProvidedBackingDueToOverlap or similar to avoid future > mistakes? I think making "layer needs to overlap" not an IndirectCompositingReason, but a bit on the layer would have helped avoid this, but that can happen later. |