Last Comment Bug 1255683 - 'Async' in animation performance messages should be rewritten with 'Compositor'
: 'Async' in animation performance messages should be rewritten with 'Compositor'
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Animation (show other bugs)
: Trunk
: Unspecified Unspecified
-- normal (vote)
: mozilla48
Assigned To: Brian Birtles (:birtles)
:
: Brian Birtles (:birtles)
Mentors:
Depends on: 1267251
Blocks:
  Show dependency treegraph
 
Reported: 2016-03-10 19:38 PST by Hiroyuki Ikezoe (:hiro)
Modified: 2016-04-25 06:42 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
An example which outputs the inactive frame warning (331 bytes, text/html)
2016-03-30 01:56 PDT, Hiroyuki Ikezoe (:hiro)
no flags Details
Rewrite some animation performance string to refer to compositor animations instead of 'Async animations' (4.03 KB, patch)
2016-04-15 00:33 PDT, Brian Birtles (:birtles)
no flags Details | Diff | Splinter Review
Rewrite some animation performance string to refer to compositor animations instead of 'Async animations' (4.03 KB, patch)
2016-04-15 00:35 PDT, Brian Birtles (:birtles)
hholmes: review+
Details | Diff | Splinter Review
Rewrite some animation performance string to refer to compositor animations instead of 'Async animations', (5.55 KB, patch)
2016-04-22 00:15 PDT, Brian Birtles (:birtles)
hikezoe: review+
Details | Diff | Splinter Review

Comment 1 User image Brian Birtles (:birtles) 2016-03-25 23:53:44 PDT
Hi Helen, this comes back to bug 1254408, and especially attachment 8733708 [details] where in one place we refer to animations that run on the "compositor thread" and in another place to "Async animations" even though they refer to the same thing.

Do you think "compositor" is understandable? (We probably should *not* refer to "compositor thread" in any case since in future the compositor may actually run on a separate process; we should just say "running on the compositor".)

I spoke to Rob here and we thought of other possible words:

* animation is running optimally
* accelerated animation (I don't like this because it sounds too much like GPU acceleration which is unrelated)
* independent animation

We were leaning towards "suboptimal animations" for the warning case.

i.e. for the main lightning bolt tooltip we would have "All animations are running optimally" / "Some animations are running optimally" / (Nothing for the case where everything is running on the main thread)

For the expanded property view we could have something like "Transform animations with 'backface-visibility: hidden' are suboptimal". I'm not entirely sure that's clear and perhaps it's ok to use "compositor" in the expanded view?

i.e.
* Use "optimal" for the main lighning bolt tooltip message and,
* Use the term "compositor" in the expanded property view (e.g. "Animation of 'backface-visibility: hidden' transforms cannot be run on the compositor")

What do you think?
Comment 2 User image Brian Birtles (:birtles) 2016-03-30 00:27:38 PDT
Also, we need to rework:

  "Async animation disabled because frame was not marked active for 'transform' animation"

What on earth does that mean? What is an author supposed to do to fix that?
Comment 3 User image Hiroyuki Ikezoe (:hiro) 2016-03-30 00:34:04 PDT
(In reply to Brian Birtles (:birtles) from comment #2)
> Also, we need to rework:
> 
>   "Async animation disabled because frame was not marked active for
> 'transform' animation"
> 
> What on earth does that mean? What is an author supposed to do to fix that?

Actually I think web developers cannot do anything for that.  We should probably not expose it to devtools, should just for output in stdout.
Comment 4 User image Brian Birtles (:birtles) 2016-03-30 00:34:29 PDT
(In reply to Brian Birtles (:birtles) from comment #2)
>   "Async animation disabled because frame was not marked active for
> 'transform' animation"

According to Patrick, that warning sometimes shows up on apple.com if you select div#ac-gn-searchresults and click on the search icon in the menu bar (top right)

As well as just generally trying to identify from the code what conditions can produce that warning, we should also work out what causes the error in the apple.com case and write a message that makes sense there.
Comment 5 User image Hiroyuki Ikezoe (:hiro) 2016-03-30 00:36:23 PDT
(In reply to Brian Birtles (:birtles) from comment #4)
> (In reply to Brian Birtles (:birtles) from comment #2)
> >   "Async animation disabled because frame was not marked active for
> > 'transform' animation"
> 
> According to Patrick, that warning sometimes shows up on apple.com if you
> select div#ac-gn-searchresults and click on the search icon in the menu bar
> (top right)
> 
> As well as just generally trying to identify from the code what conditions
> can produce that warning, we should also work out what causes the error in
> the apple.com case and write a message that makes sense there.

OK. I will do it.
Comment 6 User image Hiroyuki Ikezoe (:hiro) 2016-03-30 01:56:45 PDT
Created attachment 8736223 [details]
An example which outputs the inactive frame warning

The warning message seems to be related to fill:forwards.  This example outputs the warning message *after* the animation is finished.

At least in this case the message is not useful for web developers.
Comment 7 User image Helen V. Holmes (:helenvholmes) (:✨) 2016-04-04 05:45:59 PDT
(In reply to Brian Birtles (:birtles, away 2-10 April) from comment #1)
> Hi Helen, this comes back to bug 1254408, and especially attachment 8733708 [details]
> [details] where in one place we refer to animations that run on the
> "compositor thread" and in another place to "Async animations" even though
> they refer to the same thing.
> 
> Do you think "compositor" is understandable? (We probably should *not* refer
> to "compositor thread" in any case since in future the compositor may
> actually run on a separate process; we should just say "running on the
> compositor".)
Nah, "compositor" only means something to me because I work here. :)

> I spoke to Rob here and we thought of other possible words:
> 
> * animation is running optimally
> * accelerated animation (I don't like this because it sounds too much like
> GPU acceleration which is unrelated)
> * independent animation
> 
> We were leaning towards "suboptimal animations" for the warning case.
I think "optimized" might be a better choice of word. I like the sentences you've chosen, though:

- All animations are optimized
- Some animations are optimized
- [nothing]

> For the expanded property view we could have something like "Transform
> animations with 'backface-visibility: hidden' are suboptimal". I'm not
> entirely sure that's clear and perhaps it's ok to use "compositor" in the
> expanded view?
I think it would be okay to use compositor here—if you're trying to figure out more I think it's fair to say you want at least the correct terms to Google.
Comment 8 User image Brian Birtles (:birtles) 2016-04-15 00:33:27 PDT
Created attachment 8741658 [details] [diff] [review]
Rewrite some animation performance string to refer to compositor animations instead of 'Async animations'

Here are updated strings. The last two are complex but probably shouldn't be reported to regular web developers anyway. We'll fix the strings for the main tooltip (e.g. 'All animations are optimized') over in bug 1254408
Comment 9 User image Brian Birtles (:birtles) 2016-04-15 00:35:48 PDT
Created attachment 8741659 [details] [diff] [review]
Rewrite some animation performance string to refer to compositor animations instead of 'Async animations'

A few minor tweaks
Comment 10 User image Brian Birtles (:birtles) 2016-04-22 00:15:47 PDT
Created attachment 8744201 [details] [diff] [review]
Rewrite some animation performance string to refer to compositor animations instead of 'Async animations',

Oops, forgot I need to update test_animation_performance_warning.html too
Comment 11 User image Hiroyuki Ikezoe (:hiro) 2016-04-22 00:21:05 PDT
Comment on attachment 8744201 [details] [diff] [review]
Rewrite some animation performance string to refer to compositor animations instead of 'Async animations',

Review of attachment 8744201 [details] [diff] [review]:
-----------------------------------------------------------------

I suddenly came up with an idea to eliminate this.  I will do it at some point.
Comment 13 User image Hiroyuki Ikezoe (:hiro) 2016-04-22 02:25:02 PDT
(In reply to Hiroyuki Ikezoe (:hiro) from comment #11)
> Comment on attachment 8744201 [details] [diff] [review]
> Rewrite some animation performance string to refer to compositor animations
> instead of 'Async animations',
> 
> Review of attachment 8744201 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I suddenly came up with an idea to eliminate this.  I will do it at some
> point.

Filed. Bug 1266677
Comment 14 User image Carsten Book [:Tomcat] 2016-04-25 02:59:34 PDT
https://hg.mozilla.org/mozilla-central/rev/543eb9757c11

Note You need to log in before you can comment on or make changes to this bug.