Closed Bug 1237097 Opened 4 years ago Closed 3 years ago

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

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: dholbert, Assigned: mattwoodrow)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

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".
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).
NOTE: I'm not 100% sure my EXPECTED RESULTS are reasonable here, or that this causes problems in the real world.
Component: Layout → CSS Parsing and Computation
(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.
Component: CSS Parsing and Computation → Layout
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.
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.
Assignee: nobody → matt.woodrow
Attachment #8777662 - Flags: review?(tnikkel)
Attachment #8777662 - Flags: review?(tnikkel) → review+
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cbcdad90154e
Don't invalidate opacity:0 nsDisplayOpacity items. r=tnikkel
https://hg.mozilla.org/mozilla-central/rev/cbcdad90154e
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.