Bug 12350 - Add calcMode support to SVG Animation
Summary: Add calcMode support to SVG Animation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Eric Seidel (no email)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-01-21 01:02 PST by Eric Seidel (no email)
Modified: 2007-01-26 17:35 PST (History)
1 user (show)

See Also:


Attachments
incomplete patch (19.23 KB, patch)
2007-01-21 01:04 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Another step closer (still not complete, doesn't compile) (36.66 KB, patch)
2007-01-22 01:34 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
yet another step closer (doesn't compile, now with SVGTransformDistance) (60.56 KB, patch)
2007-01-24 14:31 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
a version which compiles but does not seem to work (77.86 KB, patch)
2007-01-24 17:04 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
A version which works about as well as pre-patch (77.97 KB, patch)
2007-01-24 18:07 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Now better than TOT (80.46 KB, patch)
2007-01-24 21:53 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
final patch, with test cases (96.01 KB, patch)
2007-01-26 02:05 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
better patch, now includes animateMotion support too (101.69 KB, patch)
2007-01-26 04:53 PST, Eric Seidel (no email)
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2007-01-21 01:02:09 PST
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).
Comment 1 Eric Seidel (no email) 2007-01-21 01:04:06 PST
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.)
Comment 2 Eric Seidel (no email) 2007-01-22 01:34:21 PST
Created attachment 12602 [details]
Another step closer (still not complete, doesn't compile)
Comment 3 Eric Seidel (no email) 2007-01-24 14:31:35 PST
Created attachment 12650 [details]
yet another step closer (doesn't compile, now with SVGTransformDistance)
Comment 4 Eric Seidel (no email) 2007-01-24 17:04:37 PST
Created attachment 12656 [details]
a version which compiles but does not seem to work
Comment 5 Eric Seidel (no email) 2007-01-24 18:07:40 PST
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.
Comment 6 Eric Seidel (no email) 2007-01-24 21:53:56 PST
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.
Comment 7 Eric Seidel (no email) 2007-01-26 02:05:00 PST
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.
Comment 8 Eric Seidel (no email) 2007-01-26 04:53:41 PST
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.
Comment 9 Oliver Hunt 2007-01-26 16:35:22 PST
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
Comment 10 Eric Seidel (no email) 2007-01-26 16:41:22 PST
(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.

Comment 11 Sam Weinig 2007-01-26 17:35:20 PST
Landed in r19168.