Closed Bug 1205704 Opened 9 years ago Closed 9 years ago

Show when animations are running on the compositor thread in the new timeline UI

Categories

(DevTools :: Inspector: Animations, defect)

defect
Not set
normal

Tracking

(firefox44 fixed, firefox45 fixed, relnote-firefox 44+)

RESOLVED FIXED
Firefox 45
Tracking Status
firefox44 --- fixed
firefox45 --- fixed
relnote-firefox --- 44+

People

(Reporter: pbro, Assigned: pbro)

References

Details

(Keywords: dev-doc-complete)

Attachments

(2 files, 2 obsolete files)

In bug 1121896, we added an icon to animations that run on the compositor thread.
With bug 1153271, the UI is changing, so we need to port the icon and logic to show it in the new UI too.

Also, more animations are going to run on the compositor now, so it's important that we do this (see http://dbaron.org/log/20150916-compositor-animations ).

The challenge here is to show the icon at the right times. With the previous UI it was simple because the UI was constantly polling the state of animations on the devtools server, so it was easy to know when animations where running on the compositor and when they were not.
The new UI is server-event driven, no polling there.
When the UI receives a new animation for display, it can know its state, but it won't receive events after that telling it when the animation is or isn't running on the compositor.

I think we can assume that it runs there as long as it is playing. When animations are paused, they are taken off the compositor.
Add a label to the tooltip that's displayed when hovering over animations when animations are running on the compositor.
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Add an icon on the timeline for animations that run on the compositor.
This screenshot shows what the patches add to the UI: an icon to the timeline, and an extra line in the tooltip.
Cleaned up the patches, folded them together, added a test, and fixed all sorts of icon alignment problems in these edge cases: negative delay, multiple iterations, infinite animations, different playback rate.

You can test with these STR:
- http://jsbin.com/tugikotiyo/edit?css,js,output
- while the animations are running, select the <body> in the jsbin's output iframe
- in the animation inspector you should see 4 animations: 2 css animations and 2 css transitions. 2 of them use margin-left (causes layouts, no compositor thread for this), and 2 others use transform (runs on the compositor).

You should see the icons and the extra info in the tooltip.
Attachment #8672307 - Attachment is obsolete: true
Attachment #8672308 - Attachment is obsolete: true
Attachment #8680601 - Flags: review?(ttromey)
Comment on attachment 8680601 [details] [diff] [review]
Bug_1205704_-_Show_an_icon_and_tooltip_when_animat.diff

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

Looks good, thanks.
Attachment #8680601 - Flags: review?(ttromey) → review+
Any chance this might be uplifted to v44? (Is it v44 we were targeting for the new timeline UI?)

(I'm hoping to plug these animation tools at our Firefox DevCon in Tokyo on Nov 14 and I'd especially like to talk about compositor animations. It would be nice to use v44 if possible.)
(In reply to Brian Birtles (:birtles) from comment #7)
> Any chance this might be uplifted to v44? (Is it v44 we were targeting for
> the new timeline UI?)
Yes, but a few things didn't make it in time, like this bug.
> (I'm hoping to plug these animation tools at our Firefox DevCon in Tokyo on
> Nov 14 and I'd especially like to talk about compositor animations. It would
> be nice to use v44 if possible.)
I think we can uplift this as it really low risk. I'll first land it to m-c and ask for approval next.
https://hg.mozilla.org/mozilla-central/rev/89a683d15226
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
(In reply to Patrick Brosset [:pbrosset] [:pbro] from comment #8)
> > (I'm hoping to plug these animation tools at our Firefox DevCon in Tokyo on
> > Nov 14 and I'd especially like to talk about compositor animations. It would
> > be nice to use v44 if possible.)
> 
> I think we can uplift this as it really low risk. I'll first land it to m-c
> and ask for approval next.

Thanks Patrick! That would be great.
Comment on attachment 8680601 [details] [diff] [review]
Bug_1205704_-_Show_an_icon_and_tooltip_when_animat.diff

Approval Request Comment
[Feature/regressing bug #]: This isn't a regression or a bug, it's a new minor feature that we wanted to ship in 44 but missed the merge date by a day.

[User impact if declined]: If declined, users won't know when animations are running on the compositor thread in devtools. This is quite an important information as it helps people optimize their animations and therefore have less janky websites.
FF44 is a major hit in terms of animation tooling, and it'd be great if this feature made it to 44 too.

[Describe test coverage new/current, TreeHerder]: There are automated browser mochitests running for this, all passed fine on fx-team and are now running on m-c.

[Risks and why]: Given the type of code, the risks are very limited. This really just displays an icon on animations. I don't any chance for regression here.
The only risk is related to how the icon is displayed, there could be bugs around where the icon is displayed, if it's visible correctly. But so far, all tests were successful.

[String/UUID change made/needed]: None
Attachment #8680601 - Flags: approval-mozilla-aurora?
Comment on attachment 8680601 [details] [diff] [review]
Bug_1205704_-_Show_an_icon_and_tooltip_when_animat.diff

This change has been on Nightly for a few days so feels safe. Let's uplift to Aurora44.
Attachment #8680601 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Patrick, is this worth relnoting in 44 release notes? We normally do not update Aurora release notes once published, but I could add this when 44 moves from Aurora to Beta channel. Please let me know.
Flags: needinfo?(pbrosset)
(In reply to Ritu Kothari (:ritu) from comment #15)
> Patrick, is this worth relnoting in 44 release notes? We normally do not
> update Aurora release notes once published, but I could add this when 44
> moves from Aurora to Beta channel. Please let me know.
Adding a relnote in Aurora is too late as you said, and there are already several notes for the animation tool there. Plus the compositor icon is not the most important feature anyway, so no problem. Not sure it's worth mentioning when moving to Beta either.

I think we ought to mention this feature a little more once bug 1223204 will be fixed.
Flags: needinfo?(pbrosset)
I've added a section on Firefox 44: https://developer.mozilla.org/en-US/docs/Tools/Page_Inspector/How_to/Work_with_animations#Firefox_44

Also see bug 1205681, comment 15 for more on why I've chosen to present it like this.
Flags: needinfo?(pbrosset)
(In reply to Will Bamberg [:wbamberg] from comment #18)
> I've added a section on Firefox 44:
> https://developer.mozilla.org/en-US/docs/Tools/Page_Inspector/How_to/
> Work_with_animations#Firefox_44
> 
> Also see bug 1205681, comment 15 for more on why I've chosen to present it
> like this.
See my reply in bug 1205681.
Other than this, the doc you've added for this looks good to me. Thanks!
Flags: needinfo?(pbrosset)
Component: Developer Tools: Inspector → Developer Tools: Animation Inspector
Release Note Request (optional, but appreciated)
[Why is this notable]: This provides performance insights to users of the animation-inspector in firefoxdevtools
[Suggested wording]: In the animation-inspector timeline, the animations that are running on the compositor thread now have a lightning bolt icon to identify them. This is useful to know when animations are guaranteed to run smoothly.
[Links (documentation, blog post, etc)]:
relnote-firefox: --- → ?
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: