788522 - Enable asynchronous opacity and transform animations

Status: RESOLVED DUPLICATE of bug 980770

(Firefox for Android Graveyard :: Toolbar, defect, P5)

Description

[Chris Lord [:cwiiis]]

As far as I'm aware, asynchronous animations via the compositor landed for Firefox OS. These are controlled by two prefs;

layers.offmainthreadcomposition.animate-opacity
layers.offmainthreadcomposition.animate-transform

which are default off on Firefox Mobile. We should be working towards enabling these so that we can compete with Mobile Chrome on demos like this: http://www.bravestman.com/ (aside - we need evangelism so they at least have a pass-through option for non-Chrome (and don't use just -webkit prefixed stuff))

Currently, if you enable these two prefs and navigate to a page which would use either feature, the browser immediately crashes. I've yet to debug this.

Comment 1

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

Let's separate out the evangelism issue. I filed bug 788535.

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Absolutely.  Like async video, there's nothing platform- or frontend-specific about the async animations code.  However unlike async video, animations are still pretty buggy, and are not tested on tinderbox.  Caveat emptor.

Comment 4

[Kevin Brosnan [Ex-Mozilla]]

Is this the same change as bug 785069 comment 10?

Comment 5

[Chris Lord [:cwiiis]]

(In reply to Chris Lord [:cwiiis] from comment #0)
> Currently, if you enable these two prefs and navigate to a page which would
> use either feature, the browser immediately crashes. I've yet to debug this.

Trying to debug this, it seems it's a SIGKILL instead of a segfault... Could indicate it tries to do a huge memory allocation or something like that? Trying to verify on an arm device (testing on x86 atm)

Comment 6

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

(In reply to Chris Lord [:cwiiis] from comment #5)
> Trying to debug this, it seems it's a SIGKILL instead of a segfault... Could
> indicate it tries to do a huge memory allocation or something like that?

Yeah, I usually see SIGKILLs when android decides we've had enough memory and kills us off.

Comment 7

[Nick Cameron [:nrc]]

try run: https://tbpl.mozilla.org/?tree=Try&rev=f21fbcbc661b

(Although possibly a little premature as we don't have opacity tests yet).

Comment 8

[Nick Cameron [:nrc]]

(In reply to Nick Cameron [:nrc] from comment #7)
> try run: https://tbpl.mozilla.org/?tree=Try&rev=f21fbcbc661b
> 
> (Although possibly a little premature as we don't have opacity tests yet).

So apparently, inbound was busted when I grabbed this, so we can ignore it. Android is looking green so far though. dzbarsky just landed opacity tests, so I'll spin up another try run once that and mattwoodrow's last mac/OMTC patches get merged to m-c.

Comment 9

[David Zbarsky (:dzbarsky)]

(In reply to Nick Cameron [:nrc] from comment #8)
> (In reply to Nick Cameron [:nrc] from comment #7)
> > try run: https://tbpl.mozilla.org/?tree=Try&rev=f21fbcbc661b
> > 
> > (Although possibly a little premature as we don't have opacity tests yet).
> 
> So apparently, inbound was busted when I grabbed this, so we can ignore it.
> Android is looking green so far though. dzbarsky just landed opacity tests,
> so I'll spin up another try run once that and mattwoodrow's last mac/OMTC
> patches get merged to m-c.

I didn't land them yet, but you can apply https://bugzilla.mozilla.org/attachment.cgi?id=765665

Comment 10

[Nick Cameron [:nrc]]

Try push: https://tbpl.mozilla.org/?tree=Try&rev=5aeded0c7ae4

Looks good on Android, not so much on Mac. I suspect that the mac problems affect Android, but are just not picked up there. Therefore, I think we should fix the Mac test failage before turning on anywhere. (Note that bug 878142 just landed on inbound so might fix some of these problems, but I doubt all of them, if any).

Comment 11

[Wesley Johnston (:wesj)]

We're awful at transitions on Android. Any chance we can flip this on nightly/aurora for awhile and see what happens?

Comment 12

[Nick Cameron [:nrc]]

We were blocked on getting tests for OMTA (bug 788549), and possibly some correctness issues. dbaron knows what the correctness issues were, I don't recall now, sorry.

Comment 13

[Chris Lord [:cwiiis]]

(In reply to Nick Cameron [:nrc] from comment #12)
> We were blocked on getting tests for OMTA (bug 788549), and possibly some
> correctness issues. dbaron knows what the correctness issues were, I don't
> recall now, sorry.

It's kind of funny that we're ok to ship FirefoxOS with it turned on, but correctness stops us from shipping it in fennec :) Maybe we should do some testing and reassess this position?

Comment 14

[Nick Cameron [:nrc]]

(In reply to Chris Lord [:cwiiis] from comment #13)
> (In reply to Nick Cameron [:nrc] from comment #12)
> > We were blocked on getting tests for OMTA (bug 788549), and possibly some
> > correctness issues. dbaron knows what the correctness issues were, I don't
> > recall now, sorry.
> 
> It's kind of funny that we're ok to ship FirefoxOS with it turned on, but
> correctness stops us from shipping it in fennec :) Maybe we should do some
> testing and reassess this position?

That was a very concious decision at the time - performance was more important than correctness. The justification was that OMTA is used for things like opening an app, and without it, FxOS was awful. For Fennec/desktop, it is used only for web content - so correctness is more important (this at least is my understanding, I wasn't part of either decision, so I may be missing some aspects).

Comment 15

[James Willcox (:snorp) ([e-mail]) (he/him)]

Unless I did this wrong, things look pretty good on Try right now with this enabled. https://tbpl.mozilla.org/?tree=Try&rev=053150583b17

Comment 17

[Wesley Johnston (:wesj)]

Since this bug is for Android, I doubt bug 875209 has anything to do with it (since it seems to deal with drawing popup windows on Windows). Snorp, try looked good. Can we land? Pref this nightly only to debug?

Comment 18

[David Zbarsky (:dzbarsky)]

Our try coverage for async animations is essentially non-existent.  Brian has found some issues with animations that have a delay not starting at the right time (See bug 964646), so please coordinate with him or wait for that to be fixed before enabling async animations on more platforms.

Comment 19

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to David Zbarsky (:dzbarsky) from comment #18)
> Our try coverage for async animations is essentially non-existent.  Brian
> has found some issues with animations that have a delay not starting at the
> right time (See bug 964646), so please coordinate with him or wait for that
> to be fixed before enabling async animations on more platforms.

Alright, I thought it was a little fishy that things just worked.

Comment 20

[David Zbarsky (:dzbarsky)]

The issue is that in some cases we don't start painting at the right time, but querying the style (like tests do) forces a flush, so the values appear to be correct.

Comment 21

[Brian Birtles (:birtles)]

This needs to wait until at least bug 964646 lands (which is dependent on a bunch of other issues I've discovered in the process). CSS transitions tests also need more work (delay seems to be buggy there too).

Comment 22

[Brad Lassey [:blassey] (use needinfo?)]

filter on [mass-p5]

Comment 23

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

Can this land now?

Comment 24

[David Zbarsky (:dzbarsky)]

Dbaron has been doing a lot of work on fixing OMTA correctness (cascading, interaction with transitions, etc.) and it looks like he is trying to enable OMTA now in bug 980770