Closed Bug 1255683 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: DOM: Animation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: hiro, Assigned: birtles)

References

Details

Attachments

(2 files, 2 obsolete files)

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)
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?
(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.
(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.
(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)
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)
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: nobody → bbirtles
Status: NEW → ASSIGNED
Attachment #8741658 - Attachment is obsolete: true
Attachment #8741658 - Flags: review?(hholmes)
Attachment #8741659 - Flags: review?(hholmes) → review+
Oops, forgot I need to update test_animation_performance_warning.html too
Attachment #8744201 - Flags: review?(hiikezoe)
Attachment #8741659 - Attachment is obsolete: true
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+
(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
https://hg.mozilla.org/mozilla-central/rev/543eb9757c11
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Depends on: 1267251
You need to log in before you can comment on or make changes to this bug.