1285174 - Layer is not painted until the end of CSS transition

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Alex Gibson [:agibson]]

"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).

Comment 1

[Alex Gibson [:agibson]]

Attachment: firefox-tour.mov

Attached screen recording

Comment 2

[Hiroyuki Ikezoe (:hiro)]

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=204bdc8b929c88d164e165fd8f28e4c1cd002bef&tochange=7fb6f9a2d7fee76746c73fa5ba1a5c091073e199

Comment 3

[Bas Schouten (:bas.schouten)]

This is not happening for me on windows, Matt, can you repro this?

Comment 4

[Matt Woodrow (:mattwoodrow)]

Attachment: 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.

Comment 6

[Markus Stange [:mstange]]

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?

Comment 7

[Matt Woodrow (:mattwoodrow)]

(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

[Pulsebot]

Pushed by [e-mail]:
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

[Iris Hsiao [:ihsiao]]

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

Comment 10

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5484e1694dc5
Backed out changeset 3531847a7419 for Reftest failures

Comment 11

[Matt Woodrow (:mattwoodrow)]

Attachment: Make sure we render currently invisible layers

This is simpler, fixes the bug and passes try.

Comment 12

[Markus Stange [: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.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
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

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8fd9da290db7

Comment 15

[David Bolter [:davidb] (NeedInfo me for attention)]

Migrating branch tracking flags over from DUPEd bug 1288244.

Matt can you request Aurora and Beta uplift? (Patch looks fairly innocent)

Comment 16

[Matt Woodrow (:mattwoodrow)]

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.

Comment 17

[Sylvestre Ledru [:Sylvestre]]

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.

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ac9f7f2ed879

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f98982223d23