Closed
Bug 1255683
Opened 9 years ago
Closed 9 years ago
'Async' in animation performance messages should be rewritten with 'Compositor'
Categories
(Core :: DOM: Animation, defect)
Core
DOM: Animation
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: hiro, Assigned: birtles)
References
Details
Attachments
(2 files, 2 obsolete files)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
hiro
:
review+
|
Details | Diff | Splinter Review |
Assignee | ||
Comment 1•9 years 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•9 years 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•9 years 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•9 years 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•9 years 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•9 years ago
|
||
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)
Comment 7•9 years ago
|
||
(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•9 years ago
|
||
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•9 years ago
|
Assignee: nobody → bbirtles
Status: NEW → ASSIGNED
Assignee | ||
Updated•9 years ago
|
Attachment #8741658 -
Attachment is obsolete: true
Attachment #8741658 -
Flags: review?(hholmes)
Updated•9 years ago
|
Attachment #8741659 -
Flags: review?(hholmes) → review+
Assignee | ||
Comment 10•9 years ago
|
||
Oops, forgot I need to update test_animation_performance_warning.html too
Attachment #8744201 -
Flags: review?(hiikezoe)
Assignee | ||
Updated•9 years ago
|
Attachment #8741659 -
Attachment is obsolete: true
Reporter | ||
Comment 11•9 years 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•9 years ago
|
||
Reporter | ||
Comment 13•9 years 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•9 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•