Closed Bug 936503 Opened 11 years ago Closed 9 years ago

Animation clock hijacking

Categories

(DevTools :: General, defect)

x86
All
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: paul, Assigned: pbro)

References

Details

Attachments

(1 file)

It would be fun (useful?) if the user could control the clock that schedules CSS animations.
Attached patch v1 (no tests)Splinter Review
Assignee: nobody → paul
Status: NEW → ASSIGNED
Summary: Animation clock hikacking → Animation clock hijacking
Once, you did ask for ideas about the FireFox devtool and I must say I enjoy a lot the result :)
I think it could be nice to also control the css animation playhead (kinda) using the mouse 
Click, hold and drag to the left to go reverve 
Click, hold and drag to the right to go forward

Lionel
Comment on attachment 829320 [details] [diff] [review]
v1 (no tests)

Do we want that? (I want)
Is the approach sensible? (especially the sync with another window)

Demo: http://paulrouget.com/e/animationclock2/

Let me know if you have any question…
Attachment #829320 - Flags: feedback?(rcampbell)
Attachment #829320 - Flags: feedback?(jwalker)
Comment on attachment 829320 [details] [diff] [review]
v1 (no tests)

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

I like it. Very simple, but I can imagine it being very useful to people wanting to fix their animations. I don't have any data as to how many of these people there are, and if it's worth occupying screen real estate by default. We've got several options for hiding it by default if we need to.
I'd like to get it working using commands properly. Once the gcli split-apart patch has landed I'm happy to take a look.
Attachment #829320 - Flags: feedback?(jwalker) → feedback+
(In reply to Joe Walker [:jwalker] from comment #4)
> We've got several options for hiding it by default if we need to.

Let's do that. Off by default. With a checkbox in the option panel.
(In reply to Paul Rouget [:paul] from comment #5)
> (In reply to Joe Walker [:jwalker] from comment #4)
> > We've got several options for hiding it by default if we need to.
> 
> Let's do that. Off by default. With a checkbox in the option panel.

Note: GCLI buttons are not supported by Options panel.
(In reply to Girish Sharma [:Optimizer] from comment #6)
> (In reply to Paul Rouget [:paul] from comment #5)
> > (In reply to Joe Walker [:jwalker] from comment #4)
> > > We've got several options for hiding it by default if we need to.
> > 
> > Let's do that. Off by default. With a checkbox in the option panel.
> 
> Note: GCLI buttons are not supported by Options panel.

This is not a gcli button.
Going a step further, I think it would be useful if it were possible to also hijack the clock for a website's setTimeout() calls. Not only slowing down, but speeding up to avoid waiting on long timeouts. Can this be done as a bonus for this bug or should a new bug be created?
Hi :-),

What about a timeline? Is it possible to compute a whole timeline for (i) all DOM elements, (ii) selected ones, (iii) one selected with its children, or (iv) a specific animation based on its name?

The hard part is to compute the timeline: detect the beginning or the end of an animation for a specific element, according to a parent element that offers a range of time (if and only if it's himself animated?). Is it easier to compute the timeline when the pause button has been clicked? We then know the current animation, or we have a clock available? I don't know the API, but it would be a great feature :-].

Note: the UI could look like the network panel with a slider to control the time.
(In reply to Philipp Kewisch [:Fallen] from comment #8)
> Going a step further, I think it would be useful if it were possible to also
> hijack the clock for a website's setTimeout() calls. Not only slowing down,
> but speeding up to avoid waiting on long timeouts. Can this be done as a
> bonus for this bug or should a new bug be created?

Maybe it's possible… we're not talking about pausing the JS right? Just not triggering the setTimeout. But that would have an impact on non graphical operations.
(In reply to Ivan Enderlin from comment #9)
> Hi :-),
> 
> What about a timeline? Is it possible to compute a whole timeline for (i)
> all DOM elements, (ii) selected ones, (iii) one selected with its children,
> or (iv) a specific animation based on its name?

This tool control only the main clock. Being able to control the clock just for one element is a whole different story.

> The hard part is to compute the timeline: detect the beginning or the end of
> an animation for a specific element, according to a parent element that
> offers a range of time (if and only if it's himself animated?). Is it easier
> to compute the timeline when the pause button has been clicked? We then know
> the current animation, or we have a clock available? I don't know the API,
> but it would be a great feature :-].
> 
> Note: the UI could look like the network panel with a slider to control the
> time.

That's a whole different tool, with a much larger scope. And the hard part is not computing the timeline, but building different clocks for different async frames.
Comment on attachment 829320 [details] [diff] [review]
v1 (no tests)

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

interesting. Kinda fun.

A little hard to figure out how to use it at first. The changing states on the icon aren't really intuitive. You should get Darrin's opinion.

::: browser/locales/en-US/chrome/browser/devtools/toolbox.properties
@@ +63,5 @@
> +# %1$S alt or option key
> +# %2$S shift
> +animclock.forwardKeycode=VK_X
> +animclock.backwardKeycode=VK_Z
> +animclock.tooltip=Control animation clock from keyboard\n<x>: one tick forward\n<z>: one tick backward\nhold <x>: play foward\nhold <z>: play backward\nhold <shit-x>: fast foward\nhold <shift-z>: fast backward\nhold <%1$S-x>: slow foward\nhold <%1$S-z>: slow backward

that is an unfortunate typo.

::: toolkit/devtools/server/actors/animation-clock.js
@@ +156,5 @@
> +    this.actorID = form.animationclock;
> +    client.addActorPool(this);
> +    this.manage(this);
> +  },
> +});

neat.
Attachment #829320 - Flags: feedback?(rcampbell)
(In reply to Paul Rouget [:paul] from comment #10)
> (In reply to Philipp Kewisch [:Fallen] from comment #8)
> > Going a step further, I think it would be useful if it were possible to also
> > hijack the clock for a website's setTimeout() calls. Not only slowing down,
> > but speeding up to avoid waiting on long timeouts. Can this be done as a
> > bonus for this bug or should a new bug be created?
> 
> Maybe it's possible… we're not talking about pausing the JS right? Just not
> triggering the setTimeout. But that would have an impact on non graphical
> operations.

Not pausing, but changing the speed of the timeout. For example, if the site calls setTimeout(..., 1000), and the devtools have set a factor of x0.5, then it would fire after 2000ms instead. In connection with that, the Date() object would probably also have to be manipulated. Something like http://sinonjs.org/docs/#clock but integrated in devtools.
See Also: → 985861
Using nsDOMWindowUtils.advanceTimeAndRefresh/restoreNormalRefresh is probably not a good idea. In the future these methods will most likely be fixed so that they can never cause time to go backwards. This is because otherwise all Gecko code that uses the refresh driver needs to handle the backwards case which adds complexity and leads to bugs (e.g. bug 972199).

A better approach for this is to wait for the Web Animations API (bug 875219) and use that. Javascript animations can be defined as effect callbacks and then be rewound/seeked/sped up etc.
I think this would be a perfect fit for a GCLI command, at least to start with.

On the long run, a more advanced animation inspector tool makes a lot more sense. People creating/debugging web animations need a lot more than just play/pause buttons. They need a whole timeline, with keyframes and being able to jump to any of these frames and adjust the animation properties there.

However, in the meantime, I think it's definitely worth exposing these simple nsIDOMWindowUtils functions via a GCLI command, maybe even create a protocol actor out of it if we have a good place to add buttons in our toolbox.

I also think exposing only play/pause/moveTo(time) with time being a positive integer is enough. As Brian said, calling advanceTimeAndRefresh with negative numbers may not work in the future, and it leads to weird results today anyway when interacting with the page (scrolling for example).

I'm willing to work on this.
Paul, can I steal this bug from you?
Flags: needinfo?(paul)
(In reply to Patrick Brosset [:pbrosset] [:patrick] from comment #15)
> I think this would be a perfect fit for a GCLI command, at least to start
> with.
> 
> On the long run, a more advanced animation inspector tool makes a lot more
> sense. People creating/debugging web animations need a lot more than just
> play/pause buttons. They need a whole timeline, with keyframes and being
> able to jump to any of these frames and adjust the animation properties
> there.

I think so. I'm trying to get that API ready as quickly as possible! (See bug 1032573 for today's contribution)

> However, in the meantime, I think it's definitely worth exposing these
> simple nsIDOMWindowUtils functions via a GCLI command, maybe even create a
> protocol actor out of it if we have a good place to add buttons in our
> toolbox.
> 
> I also think exposing only play/pause/moveTo(time) with time being a
> positive integer is enough. As Brian said, calling advanceTimeAndRefresh
> with negative numbers may not work in the future, and it leads to weird
> results today anyway when interacting with the page (scrolling for example).

The trouble is when you restore the refresh driver from test control you end up sending it backwards which could trigger all sorts of bugs (or at least assertions). That's potentially fixable by making the refresh driver time not go backwards on restore, but it's non-trivial since we have to coordinate the same accumulated offset on both the main thread and compositor so we haven't done that yet.

So I'd really caution against using advanceTimeAndRefresh at all, or if you do, using it as a stepping-stone that you can later replace with something else like the Web Animations API.
(In reply to Patrick Brosset [:pbrosset] [:patrick] from comment #15)
> I'm willing to work on this.
> Paul, can I steal this bug from you?

Yes.
Flags: needinfo?(paul)
Thanks Brian for your reply.
It doesn't make that much sense anymore then to expose such global play/pause/move actions through our UI if it's not a good idea to rely on advanceTimeAndRefresh.
Unless if, when the Web Animations API lands, we can use it to pause all currently running animations on the page. Paul's original idea still makes sense as a helpful tool beside a more complex animation inspector.
Assignee: paul → pbrosset
(In reply to Patrick Brosset [:pbrosset] [:patrick] from comment #18)
> Unless if, when the Web Animations API lands, we can use it to pause all
> currently running animations on the page. Paul's original idea still makes
> sense as a helpful tool beside a more complex animation inspector.

Pausing *all* the animations is not currently planned. For Web content it's generally undesirable since it would mean *all* transitions etc. on the page would not run and could lead to all sorts of breakage. For example, a pop-up box that only initializes once it got the 'transitionend' event would never initialize etc.

We could add this, however, if you think it's desirable from a devtools point of view. Short of that, it should still be possible to get all animations and pause them one by one.

Longer-term we hope to introduce grouping and sequencing primitives to the Web platform and allow playback control on groups/sequences as a whole so perhaps that would cover the case where you want to pause/seek multiple animations at once.
Patrick, has your animations panel covered this feature already, or are there still things here that aren't done?

It seems we have a way to pause all animations now...
Flags: needinfo?(pbrosset)
Yeah, let's close this.
Anyway hijacking the refresh driver is not advised and we now have all we need thanks to the Web Animations API.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: needinfo?(pbrosset)
Resolution: --- → WONTFIX
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: