Created attachment 102192 [details] use one consistent elapsed time for animation update

I wonder if we should plug in the same timebase that requestAnimationFrame etc use? Maybe not in this patch, but soon.

Yeah, I'd really like to tie in the timebase and the scheduling for this in with requestAnimationFrame. That would mean changing the 'current time' notion from something internal to SMILTimeContainer to something passed in from the outside on each animation update. I also think it's a good thing for a different patch than this one. I'm not super familiar with SMIL but doesn't it allow applying transformations in time space (for example, making a container in which time travels twice as fast)? Is that handled at this level or somewhere else?

I've only just started cozying up with the spec, so I'm not entirely sure if there's sub time warping requirements or not. FWIW, there's a comment in the time interval calculations claiming: // No nested time containers in SVG, no need for crazy time space conversions. Phew! but will need to investigate more for a future requestAnimationFrame patch. Is there anyone else who should review this patch as is?

(In reply to comment #3) > Yeah, I'd really like to tie in the timebase and the scheduling for this in with requestAnimationFrame. That would mean changing the 'current time' notion from something internal to SMILTimeContainer to something passed in from the outside on each animation update. I also think it's a good thing for a different patch than this one. SVG has a time container per SVGSVGElement. And every time container can be paused and continued independent of the other time container. You can set the time of every container independent of the other containers. So we can end up with different time containers with different status per document. We could still use a global timer. But this needs to get considered. > > I'm not super familiar with SMIL but doesn't it allow applying transformations in time space (for example, making a container in which time travels twice as fast)? Is that handled at this level or somewhere else? Not sure if SMIL supports that, but SVG Animation doesn't. And we do not need a complete SMIL implementation.

Comment on attachment 102192 [details] use one consistent elapsed time for animation update View in context: https://bugs.webkit.org/attachment.cgi?id=102192&action=review > Source/WebCore/ChangeLog:13 > + No new tests, but existing svg animations should continue to work > + correctly, and elapsed() is stable when stepping in debugger. Is this observable from the web? Can we test this?

(In reply to comment #6) > (From update of attachment 102192 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=102192&action=review > > > Source/WebCore/ChangeLog:13 > > + No new tests, but existing svg animations should continue to work > > + correctly, and elapsed() is stable when stepping in debugger. > > Is this observable from the web? Can we test this? I guess it would be on a heavily loaded machine. I can't think of good way to test automatically in a layout test though. My primary concern was really debugger single-stepping, but it just seemed wrong to update parts of the same animation with different times.

Comment on attachment 102192 [details] use one consistent elapsed time for animation update Without some way to tst this, I'm sure this will break again. :( The change seems OK to me though. We do this all on the main thread (including running JavaScript) so there is no way this is observable from the web. Unless there are JS events which are fired synchronously from animations? But I doubt that. I'm not sure this change is worth it, for just debugging.

This is fixed since r158405, by properly reporting a fixed time, if the animation is paused on a SMILTimeContainer.