Layer is not painted until the end of CSS transition

RESOLVED FIXED in Firefox 49

Status

()

RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: agibson, Assigned: mattwoodrow)

Tracking

(Depends on: 1 bug, {regression})

49 Branch
mozilla51
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox49+ fixed, firefox50+ fixed, firefox51 fixed)

Details

(Whiteboard: [gfx-noted], URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

3 years ago
"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)

Comment 1

3 years ago
Created attachment 8768685 [details]
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]
(Assignee)

Updated

2 years ago
Depends on: 1287654
(Assignee)

Comment 4

2 years ago
Created attachment 8772247 [details] [diff] [review]
Make sure the visible region is correct

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)
(Assignee)

Updated

2 years ago
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+
(Assignee)

Comment 7

2 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.

Comment 8

2 years ago
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)

Comment 10

2 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

2 years ago
Created attachment 8775870 [details] [diff] [review]
Make sure we render currently invisible layers

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+

Comment 13

2 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

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8fd9da290db7
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
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)
status-firefox49: --- → affected
status-firefox50: --- → affected
tracking-firefox49: --- → +
tracking-firefox50: --- → +
Flags: needinfo?(matt.woodrow)
(Assignee)

Comment 16

2 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 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

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/ac9f7f2ed879
status-firefox50: affected → fixed

Comment 19

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/f98982223d23
status-firefox49: affected → fixed
You need to log in before you can comment on or make changes to this bug.