Enable asynchronous opacity and transform animations

RESOLVED DUPLICATE of bug 980770

Status

()

P5
normal
RESOLVED DUPLICATE of bug 980770
6 years ago
4 years ago

People

(Reporter: cwiiis, Assigned: dzbarsky)

Tracking

(Depends on: 1 bug, Blocks: 1 bug, {productwanted})

Trunk
ARM
Android
productwanted
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(fennec+)

Details

(Reporter)

Description

6 years ago
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.
Let's separate out the evangelism issue. I filed bug 788535.
(Assignee)

Updated

6 years ago
Assignee: nobody → dzbarsky
(Assignee)

Updated

6 years ago
Depends on: 788549
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.
Duplicate of this bug: 754832
tracking-fennec: --- → ?
Keywords: productwanted

Updated

6 years ago
Blocks: 790567

Updated

6 years ago
Depends on: 736219, 717925
tracking-fennec: ? → +
Is this the same change as bug 785069 comment 10?
Blocks: 785069
Depends on: 822455
(Reporter)

Comment 5

6 years ago
(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)
(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

5 years ago
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

5 years ago
(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.
(Assignee)

Comment 9

5 years ago
(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

5 years ago
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).
We're awful at transitions on Android. Any chance we can flip this on nightly/aurora for awhile and see what happens?

Comment 12

5 years ago
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.
(Reporter)

Comment 13

5 years ago
(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

5 years ago
(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).
Depends on: 964646

Updated

5 years ago
Depends on: 965408
Unless I did this wrong, things look pretty good on Try right now with this enabled. https://tbpl.mozilla.org/?tree=Try&rev=053150583b17
Duplicate of this bug: 749322
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?
Flags: needinfo?(snorp)
(Assignee)

Comment 18

5 years ago
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.
(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.
Flags: needinfo?(snorp)
(Assignee)

Comment 20

5 years ago
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.
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).
Depends on: 1018862
filter on [mass-p5]
Priority: -- → P5
Can this land now?
Flags: needinfo?(dzbarsky)
(Assignee)

Comment 24

4 years ago
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
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: needinfo?(dzbarsky)
Resolution: --- → DUPLICATE
Duplicate of bug: 980770
You need to log in before you can comment on or make changes to this bug.