Closed
Bug 1285174
Opened 9 years ago
Closed 9 years ago
Layer is not painted until the end of CSS transition
Categories
(Core :: Graphics: Layers, defect)
Tracking
()
RESOLVED
FIXED
mozilla51
People
(Reporter: agibson, Assigned: mattwoodrow)
References
(Depends on 1 open bug, )
Details
(Keywords: regression, Whiteboard: [gfx-noted])
Attachments
(2 files, 1 obsolete file)
4.97 MB,
video/quicktime
|
Details | |
1008 bytes,
patch
|
mstange
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
"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).
Reporter | ||
Updated•9 years ago
|
Reporter | ||
Comment 1•9 years ago
|
||
Attached screen recording
Comment 2•9 years ago
|
||
Blocks: 1141468
Keywords: regression
Comment 3•9 years ago
|
||
This is not happening for me on windows, Matt, can you repro this?
Flags: needinfo?(matt.woodrow)
Whiteboard: [gfx-noted]
Assignee | ||
Comment 4•9 years ago
|
||
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)
Comment 6•9 years ago
|
||
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+
Assignee | ||
Comment 7•9 years ago
|
||
(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
Comment 9•9 years ago
|
||
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)
Comment 10•9 years ago
|
||
Backout by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5484e1694dc5
Backed out changeset 3531847a7419 for Reftest failures
Assignee | ||
Comment 11•9 years ago
|
||
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 12•9 years ago
|
||
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+
Comment 13•9 years ago
|
||
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
Comment 14•9 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Comment 15•9 years ago
|
||
Migrating branch tracking flags over from DUPEd bug 1288244.
Matt can you request Aurora and Beta uplift? (Patch looks fairly innocent)
status-firefox49:
--- → affected
status-firefox50:
--- → affected
tracking-firefox49:
--- → +
tracking-firefox50:
--- → +
Flags: needinfo?(matt.woodrow)
Assignee | ||
Comment 16•9 years ago
|
||
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 17•9 years ago
|
||
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+
Comment 18•9 years ago
|
||
bugherder uplift |
Comment 19•9 years ago
|
||
bugherder uplift |
You need to log in
before you can comment on or make changes to this bug.
Description
•