Last Comment Bug 755603 - Frozen to-animation is broken
: Frozen to-animation is broken
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: All All
-- normal (vote)
: mozilla15
Assigned To: Brian Birtles (:birtles)
: Jet Villegas (:jet)
Depends on:
  Show dependency treegraph
Reported: 2012-05-15 18:52 PDT by Brian Birtles (:birtles)
Modified: 2012-05-20 23:40 PDT (History)
2 users (show)
bbirtles: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Proposed patch v1a (9.40 KB, patch)
2012-05-15 18:53 PDT, Brian Birtles (:birtles)
dholbert: review+
Details | Diff | Splinter Review
Proposed patch v1b; r=dholbert (9.41 KB, patch)
2012-05-18 01:26 PDT, Brian Birtles (:birtles)
no flags Details | Diff | Splinter Review

Description User image Brian Birtles (:birtles) 2012-05-15 18:52:00 PDT
SMIL defines some pretty counter-intuitive behaviour for frozen to-animations. I've documented it here:

I tried to get the spec changed but got pushback from one individual and didn't have the perseverance to see it through:

Currently out implementation /tries/ to follow the spec but:
* it is broken (if you seek or the frame rate is slow the result will be incorrect)
* it is not easily fixable (you need to increase coupling between the timing and animation models)
* is not interoperable (no-one else bothers to try and do what the spec says here)

I'd like to fix this to match what Opera does which I think is most intuitive. Then at least we'd have something:
* interoperable
* not-broken
* intuitive

I'll try to get the spec fixed in SVG 2.
Comment 1 User image Brian Birtles (:birtles) 2012-05-15 18:53:23 PDT
Created attachment 624270 [details] [diff] [review]
Proposed patch v1a
Comment 2 User image Daniel Holbert [:dholbert] 2012-05-16 15:37:09 PDT
Comment on attachment 624270 [details] [diff] [review]
Proposed patch v1a

Looks great! I like this much better without the frozen-to special case.  Your www-svg post outlines the situation quite well.

>+    <!-- From time t=1s onwards this should apply 50% of its effect to the base
>+         value (since it is frozen at repeatCount=0.5).
>+         So, if the base value is 50%, and the to-value is 150%, it will produce
>+         100%. -->
>+    <animate attributeName="width" to="150%" fill="freeze"
>+      dur="1s" repeatCount="0.5"/>

Nit: shouldn't this say "from time t=0.5s onwards" (instead of "t=1s onwards")?

That is to say -- won't the animation stop halfway through its 1s simple duration, at 0.5s, and freeze at that point?

Assuming I'm understanding correctly: It's probably simplest to just change "dur=1s" to "dur=2s" so that the existing comment will magically become correct, and so we won't be dealing with both fractional-second and 100-second ranges in the same test.

>+    <!-- From time t=1s onwards this should apply 50% of its effect to the
>+         value.
>+         So, if the base value is 150%, and the to-value is 50%, it will produce
>+         100%. -->
>+    <animate attributeName="height" to="50%" fill="freeze"
>+      dur="1s" repeatCount="0.5"/>

(the above applies here as well)

r=me with that
Comment 3 User image Brian Birtles (:birtles) 2012-05-18 01:26:23 PDT
Created attachment 625034 [details] [diff] [review]
Proposed patch v1b; r=dholbert

Address review feedback. Thanks Daniel!
Comment 4 User image Brian Birtles (:birtles) 2012-05-20 16:53:59 PDT
Pushed to m-i:
Comment 5 User image Ed Morley [:emorley] 2012-05-20 23:39:03 PDT

Note You need to log in before you can comment on or make changes to this bug.