1186061 - off-main-thread (OMT) animation of a transform doesn't honor backface-visibility when front/back switches mid-animation

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[vincent]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/43.0.2357.134 Safari/537.36

Steps to reproduce:

1. Open this jsbin: http://jsbin.com/huboco/edit?html,css,output
2. Hover over elements
3. backface-visibility should work in both cases
  - perspective on parent
  - perspective on transform element


Actual results:

This not works


Expected results:

This used to work. When I tested back in 2013, this works. I've just installed Firefox v21 (May 2013) and this works, so this could be a regression.

Comment 1

[Alice0775 White]

regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ef53c6c25fb1&tochange=eed5d2d610e2

Regressed by: eed5d2d610e2	L. David Baron — Bug 980770 - Enable off-main-thread animations on all platforms other than Linux (and other X11 based platforms) with off-main-thread compositing, for nightly/aurora. r=birtles

Comment 2

[Alice0775 White]

Regression window(force layers.offmainthreadcomposition.async-animations = true)
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=d009ce8e199e&tochange=ad0fa6692a65

Suspect: Bug 873944

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: reporter's testcase

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I think the "regression" was likely actually from https://hg.mozilla.org/integration/mozilla-inbound/rev/ad0fa6692a65, which might have been fixing OMT animations having been completely broken, in turn a regression from https://hg.mozilla.org/integration/mozilla-inbound/rev/22d939c989a0 .

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Well, hooray for spending 5 minutes "fixing" the bug and nearly 2 hours writing a working test for it...

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch 1 - Add feature to reftest harness to allow skipping the flush

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch 2 - Disable compositor thread animation of transforms when backface-visibility is hidden

We don't currently have a mechanism for rerendering when the front/back
flips, so we should disable running such animations on the compositor
thread for now until we do.

Bug 1186204 covers reenabling.

The reftest fails without the patch (showing a blue almost-square
rectangle), and passes with the patch.

The use of reftest-no-flush (added in patch 1) is needed to achieve the
failure without the patch, because the flushWindow() function in
reftest-content.js calls getBoundingClientRect() to flush rendering,
which has the side-effect of flushing style updates that have been
suppressed on the main thread while we're running an animation off the
main thread, which in turn covers up the bug.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=eb3d0a8b9a42

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d82d621f2351
https://hg.mozilla.org/integration/mozilla-inbound/rev/bc7a6de058f2

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8636840 [details] [diff] [review]
patch 2 - Disable compositor thread animation of transforms when backface-visibility is hidden

Approval Request Comment
[Feature/regressing bug #]: bug 980770
[User impact if declined]: Animations incorrectly show backface when they shouldn't, or vice-versa, so that things disappear when they should appear or appear when they should disappear.
[Describe test coverage new/current, TreeHerder]: Has reftest.
[Risks and why]: Low risk; it disables compositor-thread animations (new feature) in one case where it doesn't work.
[String/UUID change made/needed]: No.

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d82d621f2351
https://hg.mozilla.org/mozilla-central/rev/bc7a6de058f2

Comment 12

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8636840 [details] [diff] [review]
patch 2 - Disable compositor thread animation of transforms when backface-visibility is hidden

Reftests included so should be safe. Let's land it in Aurora.

Comment 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Tracked now.

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/241229427c71
https://hg.mozilla.org/releases/mozilla-aurora/rev/f58734b8d5ee

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1186061 - Allow animations with backface-visibility: hidden to run in the compositor. r=#animation!

The check is not fully correct as per the comment (bug 779598 removed
the preserve-3d check).

I went through the relevant bugs and nothing misbehaves with this
afaict.

Comment 16

[Gregory Pappas [:gregp]]

🙂 wrong bug?

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Err, yes

Comment 18

[Phabricator Automation]

Comment on attachment 9376401 [details]
Bug 1186061 - Allow animations with backface-visibility: hidden to run in the compositor. r=#animation!

Revision D199609 was moved to bug 1186204. Setting attachment 9376401 [details] to obsolete.