SVGAnimation - keyTime value 1 never get animated

• Component: SVG
• Status: RESOLVED FIXED

[Dirk Schulze]

Created attachment 98317 [details]
SVG values animation with keytimes

According to http://www.w3.org/TR/SMIL3/smil-animation.html#adef-keyTimes , the keyTime 1 should be the last time value of an animation. Right now it is the value right after that value. Short example:

<rect width="100" height="100" fill="green">
    <animate attributeName="width" values="100 ; 200 ; 300" calcMode="discrete" keyTimes="0;0.5;1" dur="6" fill="freeze"/>
</rect>

According to http://www.w3.org/TR/SMIL3/smil-animation.html#animationNS-InterpolationIllus the values should change at 2s, 4s and right before 6s. At the moment we take the last value at exactly 6s. According to the spec, it should not affect the animation, because 6s would be excluded. We should check for keyTime 1 and should calculate the last time value right before reaching the end.

While FF, Opera and Batik end the animation with the value 300 for width, we end with 200. 

This affects at least one W3C test: http://dev.w3.org/SVG/profiles/1.1F2/test/svg/animate-elem-31-t.svg

[Young Han Lee]

Created attachment 98633 [details]
Patch

[Young Han Lee]

SVGAnimationElement::calculateKeyTimesIndex has to return the index n if the m_keyTimes[n] is the same with the given percent.

[Dirk Schulze]

Comment on attachment 98633 [details]
Patch

Good catch! r=me

[WebKit Review Bot]

Comment on attachment 98633 [details]
Patch

Clearing flags on attachment: 98633

Committed r89774: <http://trac.webkit.org/changeset/89774>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Dirk Schulze]

This seems to cause an ASSERTion on http://dev.w3.org/SVG/profiles/1.1F2/test/svg/animate-elem-82-t.svg

Can you have a look please, Young Han?

[Dirk Schulze]

ASSERTION FAILED: i < size()
/Users/dirk/Downloads/webkit-trunk/WebKitBuild/Debug/JavaScriptCore.framework/PrivateHeaders/Vector.h(542) : const T& WTF::Vector<T, inlineCapacity>::at(size_t) const [with T = float, long unsigned int inlineCapacity = 0ul]
1   WTF::Vector<float, 0ul>::at(unsigned long) const
2   WTF::Vector<float, 0ul>::operator[](unsigned long) const
3   WebCore::SVGAnimationElement::currentValuesForValuesAnimation(float, float&, WTF::String&, WTF::String&) const
4   WebCore::SVGAnimationElement::updateAnimation(float, unsigned int, WebCore::SVGSMILElement*)
5   WebCore::SVGSMILElement::progress(WebCore::SMILTime, WebCore::SVGSMILElement*)
6   WebCore::SMILTimeContainer::updateAnimations(WebCore::SMILTime, double, WTF::String const&)
7   WebCore::SMILTimeContainer::timerFired(WebCore::Timer<WebCore::SMILTimeContainer>*)
8   WebCore::Timer<WebCore::SMILTimeContainer>::fired()

[Young Han Lee]

(In reply to comment #6)
> This seems to cause an ASSERTion on http://dev.w3.org/SVG/profiles/1.1F2/test/svg/animate-elem-82-t.svg
> 
> Can you have a look please, Young Han?

Yes, the assertion failure seems to be caused by my patch.

I will upload a follow-up patch soon.

[Young Han Lee]

Bug 63911 was opened for this regression.