Bug 12350 – Add calcMode support to SVG Animation

• Component: SVG
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

Add calcMode support to SVG Animation

The attached patch actually does a lot of surgery to the animation support to make it support calcValues as well as non-relative value computation (to eventually allow changing of the animation timeline).

[Eric Seidel (no email)]

Created attachment 12580 [details]
incomplete patch

This patch is not quite complete.  Although the base class SVGAnimationElement has been re-worked to support calcMode, the subclasses are not yet ready to handle the new parameters correctly.  (The subclasses also need further cleanup to allow abstracting concepts like "frozen" and generalize from-to animations to be simple 2-value animations.)

[Eric Seidel (no email)]

Created attachment 12602 [details]
Another step closer (still not complete, doesn't compile)

[Eric Seidel (no email)]

Created attachment 12650 [details]
yet another step closer (doesn't compile, now with SVGTransformDistance)

[Eric Seidel (no email)]

Created attachment 12656 [details]
a version which compiles but does not seem to work

[Eric Seidel (no email)]

Created attachment 12657 [details]
A version which works about as well as pre-patch

This one is actually reviewable.  WildFox should take a peak.  Things are still not fully functioning (like animateTransform seems broken), but animation works about as well as it did pre-patch.  With a bit more tweaking it can now be made much much better than it was pre-patch.

[Eric Seidel (no email)]

Created attachment 12660 [details]
Now better than TOT

calcMode now works (which is really cool).  keyTimes also works.  I expect keySplines may work as well (but I have not tested).  There are issues with rotational transforms (slightly different than existing issues on TOT), there are also issues in distance calculations between colors.  I expect to fix these issues and more before landing.  WildFox or rwlbuis should at least take a peak at this.

[Eric Seidel (no email)]

Created attachment 12677 [details]
final patch, with test cases

I'm not sure if anyone in particular is particularly qualified to review this.  WildFox originally wrote this code.  This is a total re-write, so mostly I expect folks to sanity check the individual *Distance classes, and give a rubber stamp to the "oh, that architecture more-or-less looks sane".  I'm happy to talk about any of these changes over IRC.  The resulting state of animation is certainly better than TOT.  calcMode, keyTimes work (which they didn't before).

Some thoughts on what remains broken:
keySplines is a few lines of code away from working (code to find a point on a line given an x value).  freeze and repeat accumulations do not work (but I'm not sure they did before either).  The underlying animVal/baseVal system is still broken.  <animate> can't really be wired in correctly until a system for handling animated mapped attributes, and animated styles is devised.

[Eric Seidel (no email)]

Created attachment 12679 [details]
better patch, now includes animateMotion support too

animateMotion was so easy to add under the new architecture, I figured I would just do it. :)  A few architectural changes will be needed before adding keyPoints and real path following, but simple animateMotion from, to, by and values animations work now.

[Oliver Hunt]

Comment on attachment 12679 [details]
better patch, now includes animateMotion support too

Assuming the changes we discussed on irc, this should be fine

We eneed a way to test the animation.. the drag layout tests do magical stuff with the event controller.  We may need to do similar for animation test cases in future

[Eric Seidel (no email)]

(In reply to comment #9)
> (From update of attachment 12679 [details] [edit])
> Assuming the changes we discussed on irc, this should be fine
> 
> We eneed a way to test the animation.. the drag layout tests do magical stuff
> with the event controller.  We may need to do similar for animation test cases
> in future

Thanks.

I totally agree!  That's why I filed:
http://bugs.webkit.org/show_bug.cgi?id=12074
a while back.

[Sam Weinig]

Landed in r19168.