[Webkit-unassigned] [Bug 63553] SVG animation fill="freeze" doesn't set baseVal to current animVal if animation stops before reaching the end

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=63553





--- Comment #17 from Young Han Lee <joybro at company100.net>  2011-07-20 12:17:20 PST ---
(In reply to comment #15)
> (From update of attachment 101117 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=101117&action=review
> 
> Sorry for the delay. I'm a bit busy at the moment. r- to get it from pending review list. Needs more discussion.

No problem. I appreciate for the detailed comments :)

> 
> > Source/WebCore/ChangeLog:10
> > +        elapsed >= m_intervalEnd, but this is wrong because m_intervalEnd can be before
> > +        the end of the animation. (e.g. begin="0s" end="2s" dur="3s")
> 
> I'm confused about this interpretation. For me the ending of the interval is at 3s, even if we end the animation before that time. What about this example:
> <rect width="0" height="100">
>     <animate attributeName="width" from="0" to="300" begin="0s" dur="3s" end="2s" repeat="3"/>
> </rect>
> 
> With your interpretation of interval end we would repeat the animation after 2s, not after 3s. I'd interprete the animation with start every 3 seconds but animate just 2 seconds. Maybe I just misunderstand your intention.

The interval indicates a time from the beginning to end of the animation, not one iteration. The important thing is when the animation ends.

Let's see some examples.

<animate attributeName="width" from="0" to="300" begin="0s" dur="3s" end="2s" repeat="3"/>

You misunderstood the end attribute at this example. This animation doesn't repeat. The end attribute means absolute end time of the animation, not the end of the iteration. (You can find similar example with an explanation in here[1].)

So, the end of the interval is at 2s in this case.

<animate attributeName="width" from="0" to="300" begin="0s" dur="3s" repeat="3"/>

In this example, the end of the interval is at 9s because the animation ends at 9s.

<animate attributeName="width" from="0" to="300" begin="0s;10s" dur="3s" repeat="3"/>

Here is the example have more than one interval. As the animation has two begin values, it restarts after the first end. The end of the first interval is at 9s and the end of the second interval is at 19s. Here[2] is an explanation of the begin-time list and interval.

Now, looking my comments. 

+        calculateAnimationPercentAndRepeat() is returning 1, which means 100%, whenever
+        elapsed >= m_intervalEnd, but this is wrong because m_intervalEnd can be before
+        the end of the animation. (e.g. begin="0s" end="2s" dur="3s")

Like examples given above, m_intervalEnd is 2s in this case, so when the elapsed time becomes bigger than 2, the function returns 1, but it should be 0.66.

And the comment has fixed, as m_intervalEnd is just the end of the animation.

[1] http://www.w3.org/TR/smil-animation/#EndActive
[2] http://www.w3.org/TR/smil-animation/#Timing-EvaluationOfBeginEndTimeLists

> 
> > Source/WebCore/ChangeLog:14
> > +        This change makes the function return 100% only when
> > +        m_intervalEnd - m_intervalBegin == simpleDuration, which means the animation stops
> > +        at the end of the animation. This also does some refactoring to remove unnecessary
> 
> this sounds more reasonable.
> 
> > Source/WebCore/ChangeLog:17
> > +        m_intervalEnd - m_intervalBegin. I believe this is more intuitive because the animation
> > +        cannot progress beyond it.
> 
> Can you add an ASSERT if the animation shouldn't proceed beyond that time?

This change has disappeared.

> 
> > Source/WebCore/svg/animation/SVGSMILElement.cpp:826
> > +    SMILTime activeTime = min(elapsed, m_intervalEnd) - m_intervalBegin;
> 
> Still unsure about this.The variable is called activeTime. Is intervalBegin the relative time from the current repeat level or the absolute time like elapsedTime? It must be the abs time, otherwise it wouldn't make sense anyway. And then activeTime would be the relative time from the current repeated animation. Anyway why do you take intervalEnd if elapsedTime is bigger than elapsedTime? We should differ between the end of animation process (when end=.. is set) and the end of the current interval. Otherwise the example above would repeat every 2s instead of every 3s.

Ditto.


I removed the refactoring parts from the patch as it breaks some concepts you told and caused confusion. I should have done this earlier :)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.