We seem to frequently get (inactive) opacity layers in the front end that contain only text. We can avoid this by folding the opacity value down into the text display item and including it in the colour we render the text. We'd probably need to skip this if the text contains complex extra (that require multiple draw passes) like underlines, selection and shadows. We need to implement CanApplyOpacity() and ApplyOpacity() on the nsDisplayText display item, store the opacity on the item (if we decide its ok), and then pass it into nsTextFrame::DrawText.

Are there issues with multiple nsDisplayText display items that might overlap each other, inside a single element with opacity? Or are you thinking of doing this only for a single nsDisplayText?

The current code [1] will attempt to flatten up to 3 child display items, as long as they all support this, and don't overlap each other. [1] http://mxr.mozilla.org/mozilla-central/source/layout/base/nsDisplayList.cpp#3947

The "don't overlap" test sounds good enough for me. At least as long as there aren't fun things text does with self-overlapping within a single run of text that wouldn't be handled by drawing with rgba colors.

Comment on attachment 8677729 [details] [diff] [review] flatten-text-opacity Review of attachment 8677729 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/base/nsDisplayList.cpp @@ +3927,5 @@ > for (; numChildren < ArrayLength(children) && child; numChildren++, child = child->GetAbove()) { > + if (child->GetType() == nsDisplayItem::TYPE_LAYER_EVENT_REGIONS) { > + numChildren--; > + continue; > + } Wot? Remove this before landing

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #5) > Comment on attachment 8677729 [details] [diff] [review] > flatten-text-opacity > > Review of attachment 8677729 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: layout/base/nsDisplayList.cpp > @@ +3927,5 @@ > > for (; numChildren < ArrayLength(children) && child; numChildren++, child = child->GetAbove()) { > > + if (child->GetType() == nsDisplayItem::TYPE_LAYER_EVENT_REGIONS) { > > + numChildren--; > > + continue; > > + } > > Wot? Remove this before landing This was intentional. I don't think we want nsDisplayLayerEventRegions contributing to our count of 3 items, and we don't want to check for intersections with it either. I'll split it out and put it up as a separate patch though.

Comment on attachment 8677739 [details] [diff] [review] Don't include EventRegions in our limit of 3 items to optimize Review of attachment 8677739 [details] [diff] [review]: ----------------------------------------------------------------- Right OK

https://hg.mozilla.org/integration/mozilla-inbound/rev/0e8e25c04938 https://hg.mozilla.org/integration/mozilla-inbound/rev/9ea2c1e54b0f

sorry had to back this out in https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=e1a81ef86201 something made the reftest fail like https://treeherder.mozilla.org/logviewer.html#?job_id=16483782&repo=mozilla-inbound

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/31fda88c4ae6

https://hg.mozilla.org/integration/mozilla-inbound/rev/fb0f5b430939 https://hg.mozilla.org/integration/mozilla-inbound/rev/0b903f78f2ce

https://hg.mozilla.org/mozilla-central/rev/fb0f5b430939 https://hg.mozilla.org/mozilla-central/rev/0b903f78f2ce

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/fb0f5b430939 https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/0b903f78f2ce

removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion