'Async' in animation performance messages should be rewritten with 'Compositor'

RESOLVED FIXED in Firefox 48

Status

()

Core
DOM: Animation
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: hiro, Assigned: birtles)

Tracking

Trunk
mozilla48
Points:
---

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

2 years ago
https://dxr.mozilla.org/mozilla-central/rev/dd1abe874252e507b825a0a4e1063b0e13578288/dom/locales/en-US/chrome/layout/layout_errors.properties#19-34
(Assignee)

Comment 1

a year ago
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?
Flags: needinfo?(hholmes)
(Assignee)

Comment 2

a year ago
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?
(Reporter)

Comment 3

a year ago
(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.
(Assignee)

Comment 4

a year ago
(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.
(Reporter)

Comment 5

a year ago
(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.
Flags: needinfo?(hiikezoe)
(Reporter)

Comment 6

a year ago
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.
Flags: needinfo?(hiikezoe)
(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.
Flags: needinfo?(hholmes)
(Assignee)

Comment 8

a year ago
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
Attachment #8741658 - Flags: review?(hholmes)
(Assignee)

Updated

a year ago
Assignee: nobody → bbirtles
Status: NEW → ASSIGNED
(Assignee)

Comment 9

a year ago
Created attachment 8741659 [details] [diff] [review]
Rewrite some animation performance string to refer to compositor animations instead of 'Async animations'

A few minor tweaks
Attachment #8741659 - Flags: review?(hholmes)
(Assignee)

Updated

a year ago
Attachment #8741658 - Attachment is obsolete: true
Attachment #8741658 - Flags: review?(hholmes)
Attachment #8741659 - Flags: review?(hholmes) → review+
(Assignee)

Comment 10

a year ago
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
Attachment #8744201 - Flags: review?(hiikezoe)
(Assignee)

Updated

a year ago
Attachment #8741659 - Attachment is obsolete: true
(Reporter)

Comment 11

a year ago
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.
Attachment #8744201 - Flags: review?(hiikezoe) → review+

Comment 12

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/543eb9757c11
(Reporter)

Comment 13

a year ago
(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

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/543eb9757c11
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox48: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Depends on: 1267251
You need to log in before you can comment on or make changes to this bug.