1237097 - Changes to background-color trigger invalidations, even on an "opacity:0" element

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Daniel Holbert [:dholbert]]

STR:
 1. Enable paint flashing. ( nglayout.debug.paint_flashing )
 2. Load attached testcase.

ACTUAL RESULTS:
The area above the text gets invalidated (repainted in a different paint-flashing color) every 500ms, from the periodic style.backgroundColor tweak.

EXPECTED RESULTS: No invalidation, since the periodic backgroundColor tweak is happening on an element with "opacity:0".

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: reference case 1 (adding "will-change:opacity" to force layerization)

As shown here, we can work around this by adding "will-change: opacity" on the element whose background-color is changing (I think because we can figure out that the whole layer is opacity:0 and optimize away the change).

Comment 3

[Daniel Holbert [:dholbert]]

NOTE: I'm not 100% sure my EXPECTED RESULTS are reasonable here, or that this causes problems in the real world.

Comment 4

[Daniel Holbert [:dholbert]]

(Sorry, accidentally tweaked component; reverting.)

Also didn't finish my thought in comment 3 -- compared to this bug here, I'm *more* that related-bug 1237102 is problematic, given that bug's dependence on a plugin being present, and given that we hit a version of that bug in the wild at Twitch.

Comment 5

[Markus Stange [:mstange]]

I agree that this is a bug.

(In reply to Daniel Holbert [:dholbert] from comment #2)
> Created attachment 8704373 [details]
> reference case 1 (adding "will-change:opacity" to force layerization)
> 
> As shown here, we can work around this by adding "will-change: opacity" on
> the element whose background-color is changing

This actually not true. What happens here is that the paint flashing ends up in a PaintedLayer that's invisible. We're still invalidating and painting.
With will-change: opacity, Gecko makes sure that the element can be made visible at any time, so it needs to have up-to-date content in a (currently invisible) layer. In other words, with will-change:opacity, invalidating is in fact the right thing to do.

Comment 6

[Matt Woodrow (:mattwoodrow)]

Attachment: dont-invalidate-opacity-0-item

Changing the background-color is a style change which sets the 'invalid' bit on the frame.

This makes us treat the nsDisplayOpacity items belonging to the same frame as dirty, and we mark its bound as invalid in the PaintedLayer.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cbcdad90154e
Don't invalidate opacity:0 nsDisplayOpacity items. r=tnikkel

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cbcdad90154e