Create a layer for opacity:0 display items if opacity is animated

RESOLVED FIXED in Firefox 19

Status

()

Core
Layout
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: roc, Assigned: roc)

Tracking

18 Branch
mozilla20
x86_64
Windows 7
Points:
---

Firefox Tracking Flags

(firefox19- fixed, firefox20 fixed)

Details

(Whiteboard: [qa?])

Attachments

(1 attachment)

Comment hidden (empty)
Created attachment 683730 [details] [diff] [review]
fix
Attachment #683730 - Flags: review?(matt.woodrow)
Blocks: 811831
Comment on attachment 683730 [details] [diff] [review]
fix

Review of attachment 683730 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/nsDisplayList.cpp
@@ +2687,5 @@
>  nsDisplayOpacity::BuildLayer(nsDisplayListBuilder* aBuilder,
>                               LayerManager* aManager,
>                               const ContainerParameters& aContainerParameters) {
> +  if (mFrame->GetStyleDisplay()->mOpacity == 0 && mFrame->GetContent() &&
> +      !nsLayoutUtils::HasAnimationsForCompositor(mFrame->GetContent(), eCSSProperty_opacity)) {

(!mFrame->GetContent() || !nsLayout.....)

I think not having a nsIContent* means that we definitely don't have animations, so we can skip building the layer.
Attachment #683730 - Flags: review?(matt.woodrow) → review+
We can never have nsDisplayOpacity on the viewport frame (null mContent), so no point in optimizing for it. I'm only checking for sanity's sake, and the simple thing to do is to not use this optimization if mContent happens to be null

Comment 4

5 years ago
Does this patch handle Animations as well as Transitions? I don't know if Firefox handles them separately internally. 

Plus, this bug might have to be uplifted to the branch wherever Bug 811831 landed (looks like that one made it into Gecko 19 while this one seems to land in Gecko 20).
(In reply to Florian Bender from comment #4)
> Does this patch handle Animations as well as Transitions? I don't know if
> Firefox handles them separately internally.

The function I'm calling does handle both, yes.

> Plus, this bug might have to be uplifted to the branch wherever Bug 811831
> landed (looks like that one made it into Gecko 19 while this one seems to
> land in Gecko 20).

You are correct.
tracking-firefox19: --- → ?
https://hg.mozilla.org/integration/mozilla-inbound/rev/6152b934d1c7

Comment 7

5 years ago
https://hg.mozilla.org/mozilla-central/rev/6152b934d1c7
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Comment on attachment 683730 [details] [diff] [review]
fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 811831
User impact if declined: Could possibly have a situation where off-main-thread animations of opacity start jerkily in FF19
Testing completed (on m-c, etc.): just landed on m-c
Risk to taking this patch (and alternatives if risky): Very low-risk patch. We just disable an optimization in an another case
String or UUID changes made by this patch: None
Attachment #683730 - Flags: approval-mozilla-aurora?

Comment 9

5 years ago
Not tracking since this doesn't have a demonstrated user impact at this point, but approving for Aurora given where we are in the cycle.
tracking-firefox19: ? → -

Updated

5 years ago
Attachment #683730 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/9e255aaa583c
status-firefox19: --- → fixed
status-firefox20: --- → fixed
Is there any way QA can verify this fix?
Whiteboard: [qa?]
You need to log in before you can comment on or make changes to this bug.