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
Created attachment 98633 [details] Patch
SVGAnimationElement::calculateKeyTimesIndex has to return the index n if the m_keyTimes[n] is the same with the given percent.
Comment on attachment 98633 [details] Patch Good catch! r=me
Comment on attachment 98633 [details] Patch Clearing flags on attachment: 98633 Committed r89774: <http://trac.webkit.org/changeset/89774>
All reviewed patches have been landed. Closing bug.
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?
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()
(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.
Bug 63911 was opened for this regression.