Closed Bug 1285174 Opened 5 years ago Closed 5 years ago

Layer is not painted until the end of CSS transition

Categories

(Core :: Graphics: Layers, defect)

49 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox49 + fixed
firefox50 + fixed
firefox51 --- fixed

People

(Reporter: agibson, Assigned: mattwoodrow)

References

(Depends on 1 open bug, )

Details

(Keywords: regression, Whiteboard: [gfx-noted])

Attachments

(2 files, 1 obsolete file)

"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:49.0) Gecko/20100101 Firefox/49.0"

STR:

1.) Visit https://www.mozilla.org/en-US/firefox/47.0.1/tour/
2.) In the door-hanger that appears, click "Let's go" and wait for bottom tour pannel to appear and show step 1.
3.) Click the right arrow to proceed to the next panel.

Expected results:

The second panel should transition in from the right as opacity changes.

Actual results:

The second panel is not painted until the end of the CSS transition.

Note this doesn't happen *every* time, but happen in most attempts. This bug is not present in the current release Firefox (47.0.1).
Attached video firefox-tour.mov
Attached screen recording
This is not happening for me on windows, Matt, can you repro this?
Flags: needinfo?(matt.woodrow)
Whiteboard: [gfx-noted]
Depends on: 1287654
When we choose to prerender in BuildDisplayListForStackingContext we override the dirty rect for items within the transform (which gets passed into the nsDisplayTransform ctor as aChildrenVisibleRect).

We also need to override the post-transform visible region on the transform item, which is what this patch fixes.

I filed bug 1287654 for adding support to reftests so we can add tests for this.
Assignee: nobody → matt.woodrow
Flags: needinfo?(matt.woodrow)
Attachment #8772247 - Flags: review?(mstange)
Duplicate of this bug: 1288244
Comment on attachment 8772247 [details] [diff] [review]
Make sure the visible region is correct

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

Did we override mVisibleRect before bug 1141468? If not, how did this work?
What part of the machinery requires this expanded visible rect? Does FrameLayerBuilder not enter into the nsDisplayTransform otherwise?
Attachment #8772247 - Flags: review?(mstange) → review+
(In reply to Markus Stange [:mstange] from comment #6)
> Comment on attachment 8772247 [details] [diff] [review]
> Make sure the visible region is correct
> 
> Review of attachment 8772247 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Did we override mVisibleRect before bug 1141468? If not, how did this work?

Yes it did, this is just restoring existing behaviour.


> What part of the machinery requires this expanded visible rect? Does
> FrameLayerBuilder not enter into the nsDisplayTransform otherwise?

ClientContainerLayer::RenderLayer skips invisible children, and the transform container layer is invisible to begin with without this.
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3531847a7419
Make sure we override the visible region of nsDisplayTransform when we're prerendering. r=mstange
Hi Matt, sorry had to back out your push due to Reftest failures, e.g. https://treeherder.mozilla.org/logviewer.html#?job_id=32746888&repo=mozilla-inbound#L24744
Flags: needinfo?(matt.woodrow)
Backout by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5484e1694dc5
Backed out changeset 3531847a7419 for Reftest failures
This is simpler, fixes the bug and passes try.
Attachment #8772247 - Attachment is obsolete: true
Flags: needinfo?(matt.woodrow)
Attachment #8775870 - Flags: review?(mstange)
Comment on attachment 8775870 [details] [diff] [review]
Make sure we render currently invisible layers

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

I'm not a fan, but what we really want here is for all container layers to know what their potential bounds under async animations are, and we don't currently have a good system for determining that. So this is fine as a stop gap.
Attachment #8775870 - Flags: review?(mstange) → review+
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8fd9da290db7
Make sure we render layers even if they are currently hidden (since that can change asynchronously). r=mstange
https://hg.mozilla.org/mozilla-central/rev/8fd9da290db7
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Migrating branch tracking flags over from DUPEd bug 1288244.

Matt can you request Aurora and Beta uplift? (Patch looks fairly innocent)
Flags: needinfo?(matt.woodrow)
Comment on attachment 8775870 [details] [diff] [review]
Make sure we render currently invisible layers

Approval Request Comment
[Feature/regressing bug #]: Bug 1141468
[User impact if declined]: Broken CSS transitions/animations of transforms in some cases.
[Describe test coverage new/current, TreeHerder]: Manually tested.
[Risks and why]: Very low risk.
[String/UUID change made/needed]: None.
Flags: needinfo?(matt.woodrow)
Attachment #8775870 - Flags: approval-mozilla-beta?
Attachment #8775870 - Flags: approval-mozilla-aurora?
Comment on attachment 8775870 [details] [diff] [review]
Make sure we render currently invisible layers

Let's take it to beta to fix the regression. Should be in 49 beta 2.
Attachment #8775870 - Flags: approval-mozilla-beta?
Attachment #8775870 - Flags: approval-mozilla-beta+
Attachment #8775870 - Flags: approval-mozilla-aurora?
Attachment #8775870 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.