Bug 63230 - SVGAnimation - keyTime value 1 never get animated
Summary: SVGAnimation - keyTime value 1 never get animated
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL: http://dev.w3.org/SVG/profiles/1.1F2/...
Keywords:
Depends on:
Blocks: 41761
  Show dependency treegraph
 
Reported: 2011-06-22 23:58 PDT by Dirk Schulze
Modified: 2011-07-04 08:25 PDT (History)
4 users (show)

See Also:


Attachments
SVG values animation with keytimes (224 bytes, image/svg+xml)
2011-06-22 23:58 PDT, Dirk Schulze
no flags Details
Patch (5.95 KB, patch)
2011-06-26 10:49 PDT, Young Han Lee
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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"/>
</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
Comment 1 Young Han Lee 2011-06-26 10:49:44 PDT
Created attachment 98633 [details]
Patch
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]
Patch

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

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