Bug 63230

Summary: SVGAnimation - keyTime value 1 never get animated
Product: WebKit Reporter: Dirk Schulze <krit>
Component: SVGAssignee: Nobody <webkit-unassigned>
Severity: Normal CC: joybro201, rwlbuis, webkit.review.bot, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: http://dev.w3.org/SVG/profiles/1.1F2/test/svg/animate-elem-31-t.svg
Bug Depends on:    
Bug Blocks: 41761    
Description Flags
SVG values animation with keytimes
Patch none

Description Dirk Schulze 2011-06-22 23:58:12 PDT
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"/>

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
Comment 1 Young Han Lee 2011-06-26 10:49:44 PDT
Created attachment 98633 [details]
Comment 2 Young Han Lee 2011-06-26 11:03:24 PDT
SVGAnimationElement::calculateKeyTimesIndex has to return the index n if the m_keyTimes[n] is the same with the given percent.
Comment 3 Dirk Schulze 2011-06-26 13:08:00 PDT
Comment on attachment 98633 [details]

Good catch! r=me
Comment 4 WebKit Review Bot 2011-06-26 14:26:42 PDT
Comment on attachment 98633 [details]

Clearing flags on attachment: 98633

Committed r89774: <http://trac.webkit.org/changeset/89774>
Comment 5 WebKit Review Bot 2011-06-26 14:26:46 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Dirk Schulze 2011-07-03 09:35:56 PDT
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?
Comment 7 Dirk Schulze 2011-07-03 09:43:08 PDT
/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()
Comment 8 Young Han Lee 2011-07-03 18:22:10 PDT
(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.
Comment 9 Young Han Lee 2011-07-04 08:25:17 PDT
Bug 63911 was opened for this regression.