[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


Dirk Schulze <krit at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #101117|review?                     |review-
               Flag|                            |




--- Comment #15 from Dirk Schulze <krit at webkit.org>  2011-07-20 04:14:56 PST ---
(From update of attachment 101117)
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.

> 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.

> 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?

> 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.

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