980770 - (enable-omt-animations) get off main thread animations (OMT Animations, OMTA) good enough to ship

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P3)

Description

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

Tracking bug for what we need to make off main thread animations (OMT Animations, OMTA) good enough to ship.

Comment 1

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

(Note that it's already shipping on B2G, but with some pretty hideous bugs, and poor test coverage.)

Comment 2

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

Let's see how things look on try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0a310d156884

Comment 3

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

The try run isn't quite done yet, but the main thing it turned up is already bug 1090555 (test was previously disabled on Mulet; also now shows up on Android mochitest and on Linux mochitest-e10s).

Comment 4

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

A push with bug 1090555:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f5d5324ba171

Comment 5

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

Attachment: Enable off-main-thread animations on all platforms with off-main-thread compositing

Comment 6

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Comment on attachment 8586449 [details] [diff] [review]
Enable off-main-thread animations on all platforms with off-main-thread compositing

I thought we were only turning this on for nightly/aurora until blockers like bug 1122526 were resolved?

Comment 7

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

I guess we could do nightly/aurora first, although I'd hope to be able to ship to beta/release this cycle.

Comment 8

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

(Also, it's not clear to me that bug 1122526 is even related to OMT Animations.)

Comment 9

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

Attachment: Enable off-main-thread animatinos on all platforms with off-main-thread compositing, for nightly/aurora

Comment 10

[Brian Birtles (:birtles, Feb 26~ parental leave)]

(In reply to David Baron [:dbaron] ⏰UTC-7 from comment #8)
> (Also, it's not clear to me that bug 1122526 is even related to OMT
> Animations.)

Sorry, that should be bug 1149865 which should definitely block shipping OMTA.

Comment 11

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

Landed enabling on nightly/aurora:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a765547ac232

Comment 12

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

Backed out in:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1083d2b2217
since there were some test failures that snuck in since my successful try run a few days ago:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6622f9ac9faf

Comment 13

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

(That successful try run was on top of https://hg.mozilla.org/mozilla-central/rev/e9769c80ee59 plus my patches to bug 847287.)

Comment 14

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

The relevant changes that are likely to have introduced the test failures are:
  bug 1109390 (patches 16 to 18 or patches 20 to 28)
  bug 1074630
  bug 1145246

Comment 15

[Brian Birtles (:birtles, Feb 26~ parental leave)]

(In reply to David Baron [:dbaron] ⏰UTC-7 from comment #14)
> The relevant changes that are likely to have introduced the test failures
> are:
>   bug 1109390 (patches 16 to 18 or patches 20 to 28)
>   bug 1074630
>   bug 1145246

Debugging locally I can reproduce the failure in test_deferred_start.html.

The first changeset that fails for me is:

  https://hg.mozilla.org/integration/mozilla-inbound/rev/ded0fc853a7b

i.e. part 22 from bug 1109390

which is odd because that patch shouldn't really do anything (i.e. we shouldn't ever actually end up calling PauseAt without the later patches applied). Perhaps this only intermittently fails and I got lucky when testing the changeset below it.

Comment 16

[Brian Birtles (:birtles, Feb 26~ parental leave)]

(In reply to Brian Birtles (:birtles) from comment #15)
> Perhaps this only intermittently fails and I got lucky when testing the changeset below it.

This appears to be the case. I still see the failure with patches 20 to 28 of bug 1109390 backed out.

Comment 17

[Brian Birtles (:birtles, Feb 26~ parental leave)]

I've narrowed down the failure in test_deferred_start.html to this changeset:

  https://hg.mozilla.org/integration/mozilla-inbound/rev/d4306ea579d9
  i.e. part 18 of bug 1109390

The problem appears to be that we no longer treat play-pending animations as eligible compositor animations. This is a pretty significant bug so it's good we found it now. Bug and patch coming up.

Comment 18

[Brian Birtles (:birtles, Feb 26~ parental leave)]

(In reply to Brian Birtles (:birtles) from comment #17)
> Bug and patch coming up.

That would be bug 1149906.

Comment 19

[Brian Birtles (:birtles, Feb 26~ parental leave)]

(In reply to Brian Birtles (:birtles) from comment #18)
> (In reply to Brian Birtles (:birtles) from comment #17)
> > Bug and patch coming up.
> 
> That would be bug 1149906.

That bug appears to fix the failures in test_deferred_start.html but not the failures in test_animations_omta.html. I'll hopefully be able to look into that tomorrow.

Comment 20

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

Two new try pushes:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c0a1042706d0 with bug 1150288 and bug 1149906

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fbcfd0e9880b with only bug 1150288

Comment 21

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/288ad0365a5e

Comment 22

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

Landed a followup to disable one test under some conditions after discussion with birtles:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6ea87def95b9

I filed bug 1150351 on reenabling the test.

Comment 23

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

Second followup, to requestLongerTimeout(2):
https://hg.mozilla.org/integration/mozilla-inbound/rev/d2f72af9fc50

Comment 24

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

Backed out in
https://hg.mozilla.org/integration/mozilla-inbound/rev/bce1203ac470
for high-frequency intermittent Mac talos glterrain crashes:
https://treeherder.mozilla.org/logviewer.html#?job_id=8386168&repo=mozilla-inbound
https://treeherder.mozilla.org/logviewer.html#?job_id=8386163&repo=mozilla-inbound

Comment 25

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d2f72af9fc50

Comment 26

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

I relanded the enabling on platforms other than Mac:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9002c68ad577
but left it disabled on Mac, given bug 1150535.

Comment 27

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

Backed out again:
https://hg.mozilla.org/integration/mozilla-inbound/rev/de9e98d2658a

Comment 29

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

Given that the Mac problem went away (see bug 1150535 comment 1), I relanded for all platforms other than Linux:
https://hg.mozilla.org/integration/mozilla-inbound/rev/eed5d2d610e2

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/eed5d2d610e2

Comment 31

[Gary [:streetwolf52]]

The PanelUI contents appear washed out for a split second when displayed with the pref enabled on Windows 8.1.

Comment 32

[Jim Jeffery not reading bug-mail 1/2/11]

(In reply to Gary [:streetwolf] from comment #31)
> The PanelUI contents appear washed out for a split second when displayed
> with the pref enabled on Windows 8.1.

Seeing the same on Win7 x64 m-c builds (tinderbox) win32

Comment 33

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

(In reply to Gary [:streetwolf] from comment #31)
> The PanelUI contents appear washed out for a split second when displayed
> with the pref enabled on Windows 8.1.

Could you file a separate bug on that -- and one that describes steps to reproduce for people who don't know what "PanelUI contents" are?

Comment 34

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

Bugs asking for additional features definitely don't block shipping the ones we have.

Comment 35

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

Bug 1105509 is definitely worth fixing, but it's no worse with OMT animations enabled than it was without it, so it definitely doesn't block shipping OMT animations.

Comment 36

[Lawrence Mandel [:lmandel] (use needinfo)]

I'm tracking 40 so that we can keep an eye on this work and whether it is ready to ship in 40.

Release Note Request (optional, but appreciated)
[Why is this notable]: Performance improvement for opacity and transform animations.
[Suggested wording]: Better performance of opacity and transform animations
[Links (documentation, blog post, etc)]:

Comment 37

[Gary [:streetwolf52]]

The problem appears to be this one....  https://bugzilla.mozilla.org/show_bug.cgi?id=1122526

Comment 38

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

I don't think bug 1103207 blocks shipping OMTA on desktop either.  It's something we've lived with on Firefox OS, and OMTA doesn't change the fact that we're doing stuff at 60Hz when there's an animation that requires less frequency.  (We've never had lower-rate ticking of the refresh driver just like we don't have lower-rate ticking of the compositor.  OMT Animations moves that high-frequency work to different places; more on the compositor thread and less on the main thread.)  But I don't think it's a regression that requires us to fix before shipping.

Comment 39

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

(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #36)
> I'm tracking 40 so that we can keep an eye on this work and whether it is
> ready to ship in 40.

So far we've only enabled ifndef RELEASE_BUILD, i.e., for nightly and aurora, so I think this probably isn't needed (yet).

Comment 40

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

I'm planning to enable on Linux as well now that bug 1150619 has gone away.

(I believe turning it on on Linux will affect only nightly and not other channels, because off-main-thread compositing (OMTC) is enabled on Linux only when Electrolysis (e10s) is enabled, which is only on nightly.)

Comment 41

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/76671894fe90

Comment 42

[Kartikaya Gupta (email:[e-mail])]

Bug 994541 just turned on OMTC on Linux by default, even without e10s. It should ride the trains on 40.

Comment 43

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/76671894fe90

Comment 44

[Lawrence Mandel [:lmandel] (use needinfo)]

Release Note Request (optional, but appreciated)
[Why is this notable]: OMTA provide smoother animations
[Suggested wording]: Smoother and more reliable CSS animations via asynchronous animations
[Links (documentation, blog post, etc)]:
https://wiki.mozilla.org/Platform/GFX/OffMainThreadCompositing#CSS_Animations

Happy to have input on better wording for the note. ;)

Comment 45

[Sylvestre Ledru [:Sylvestre]]

Added to the release notes with Lawrence's wording.

Comment 46

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

The release notes for what?  It's currently enabled only for nightly/aurora (i.e., in an #else of #ifdef RELEASE_BUILD).

Comment 47

[Sylvestre Ledru [:Sylvestre]]

You don't think it is worth mentioning for the aurora release notes?
I can disable it after the merge.

Comment 48

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

Attachment: Fully enable (for RELEASE_BUILD) off-main-thread animations on all platforms with off-main-thread compositing

Comment 49

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b9808ed3cacf

Comment 50

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

Comment on attachment 8627751 [details] [diff] [review]
Fully enable (for RELEASE_BUILD) off-main-thread animations on all platforms with off-main-thread compositing

Approval Request Comment
[Feature/regressing bug #]: OMT animations
[User impact if declined]: better performance of animations of opacity and transform
[Describe test coverage new/current, TreeHerder]: substantial mochitest and reftest coverage
[Risks and why]: it's a somewhat large feature, but I think it's now ready to ride the trains.  All this patch does is change it from enabled on nightly and aurora only to being enabled on all channels, since I believe that if bug 1122526 and bug 1176969 are approved, it's in good enough shape to ride the trains for this release.
[String/UUID change made/needed]: no

Comment 51

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b9808ed3cacf

Comment 52

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

Comment on attachment 8627751 [details] [diff] [review]
Fully enable (for RELEASE_BUILD) off-main-thread animations on all platforms with off-main-thread compositing

Approving for uplift to Aurora. The patch has been in m-c for a few days and hasn't had a negative impact so far.

Comment 53

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/95ff4ba30e93

Comment 54

[Liz Henry (:lizzard) (relman/hg->git project)]

We should probably relnote this for 41 now that it's going to release. Same wording as in comment 44 should be ok.

Comment 55

[Sylvestre Ledru [:Sylvestre]]

Indeed. I did it for 41! Thanks

Comment 56

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

Blog post about shipping: http://dbaron.org/log/20150916-compositor-animations