Animate animatedPoints instead of points for SVGPoly*Elements

https://bugs.webkit.org/show_bug.cgi?id=82844

Reviewed by Dirk Schulze.

Source/WebCore:

When the points attribute of a SVGPoly*Element is animated, we should animate
the 'animatedPoints' SVG DOM property, not the 'points' property, which corresponds
to the "baseVal". Fix that, now only the 'd' attribute for SVGPathElement is incorrect,
everything else is moved to the new animVal concept!

* svg/SVGAnimatedPointList.cpp:
(WebCore::SVGAnimatedPointListAnimator::startAnimValAnimation):
(WebCore::SVGAnimatedPointListAnimator::stopAnimValAnimation):
(WebCore::SVGAnimatedPointListAnimator::resetAnimValToBaseVal):
(WebCore::SVGAnimatedPointListAnimator::animValWillChange):
(WebCore::SVGAnimatedPointListAnimator::animValDidChange):
* svg/SVGAnimatedPointList.h:
(SVGAnimatedPointListAnimator):
* svg/SVGAnimatedType.cpp:
(WebCore::SVGAnimatedType::valueAsString):
(WebCore::SVGAnimatedType::setValueAsString):
(WebCore::SVGAnimatedType::supportsAnimVal):
* svg/SVGPolyElement.cpp:
(WebCore::SVGPolyElement::parseAttribute):
(WebCore::SVGPolyElement::lookupOrCreatePointsWrapper):
(WebCore::SVGPolyElement::points):
(WebCore::SVGPolyElement::animatedPoints):

LayoutTests:

Update SVGPointList animation tests, verifying that animatedPoints is animated, and not points.

* svg/animations/script-tests/svgpointlist-animation-1.js:
(sample1):
(sample2):
(sample3):
* svg/animations/script-tests/svgpointlist-animation-2.js:
(sample1):
(sample2):
(sample3):
* svg/animations/svgpointlist-animation-1-expected.txt:
* svg/animations/svgpointlist-animation-2-expected.txt:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@113008 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 6de89b5..0fc94bd 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,23 @@
+2012-04-01  Nikolas Zimmermann  <nzimmermann@rim.com>
+
+        Animate animatedPoints instead of points for SVGPoly*Elements
+        https://bugs.webkit.org/show_bug.cgi?id=82844
+
+        Reviewed by Dirk Schulze.
+
+        Update SVGPointList animation tests, verifying that animatedPoints is animated, and not points.
+
+        * svg/animations/script-tests/svgpointlist-animation-1.js:
+        (sample1):
+        (sample2):
+        (sample3):
+        * svg/animations/script-tests/svgpointlist-animation-2.js:
+        (sample1):
+        (sample2):
+        (sample3):
+        * svg/animations/svgpointlist-animation-1-expected.txt:
+        * svg/animations/svgpointlist-animation-2-expected.txt:
+
 2012-04-03  Philippe Normand  <pnormand@igalia.com>
 
         [GTK] media/video-volume.html fails on 32-bits debug

diff --git a/LayoutTests/svg/animations/script-tests/svgpointlist-animation-1.js b/LayoutTests/svg/animations/script-tests/svgpointlist-animation-1.js
index e672fe8..b34a65a 100644 (file)
--- a/LayoutTests/svg/animations/script-tests/svgpointlist-animation-1.js
+++ b/LayoutTests/svg/animations/script-tests/svgpointlist-animation-1.js
@@ -21,26 +21,29 @@ rootSVGElement.appendChild(poly);
 // Setup animation test
 function sample1() {
     // Check initial/end conditions
-    shouldBeCloseEnough("poly.points.getItem(2).x", "200");
-    shouldBeCloseEnough("poly.points.getItem(2).y", "200");
-    // shouldBeCloseEnough("poly.animatedPoints.getItem(2).x", "200");
-    // shouldBeCloseEnough("poly.animatedPoints.getItem(2).y", "200");
+    shouldBeCloseEnough("poly.animatedPoints.getItem(2).x", "200");
+    shouldBeCloseEnough("poly.animatedPoints.getItem(2).y", "200");
+
+    shouldBe("poly.points.getItem(2).x", "200");
+    shouldBe("poly.points.getItem(2).y", "200");
 }
 
 function sample2() {
     // Check half-time conditions
-    shouldBeCloseEnough("poly.points.getItem(2).x", "150");
-    shouldBeCloseEnough("poly.points.getItem(2).y", "150");
-    // shouldBeCloseEnough("poly.animatedPoints.getItem(2).x", "250");
-    // shouldBeCloseEnough("poly.animatedPoints.getItem(2).y", "250");
+    shouldBeCloseEnough("poly.animatedPoints.getItem(2).x", "150");
+    shouldBeCloseEnough("poly.animatedPoints.getItem(2).y", "150");
+
+    shouldBe("poly.points.getItem(2).x", "200");
+    shouldBe("poly.points.getItem(2).y", "200");
 }
 
 function sample3() {
     // Check just before-end conditions
-    shouldBeCloseEnough("poly.points.getItem(2).x", "100");
-    shouldBeCloseEnough("poly.points.getItem(2).y", "100");
-    // shouldBeCloseEnough("poly.animatedPoints.getItem(2).x", "300");
-    // shouldBeCloseEnough("poly.animatedPoints.getItem(2).y", "300");
+    shouldBeCloseEnough("poly.animatedPoints.getItem(2).x", "100");
+    shouldBeCloseEnough("poly.animatedPoints.getItem(2).y", "100");
+
+    shouldBe("poly.points.getItem(2).x", "200");
+    shouldBe("poly.points.getItem(2).y", "200");
 }
 
 function executeTest() {

diff --git a/LayoutTests/svg/animations/script-tests/svgpointlist-animation-2.js b/LayoutTests/svg/animations/script-tests/svgpointlist-animation-2.js
index a67eb13..4b1667e 100644 (file)
--- a/LayoutTests/svg/animations/script-tests/svgpointlist-animation-2.js
+++ b/LayoutTests/svg/animations/script-tests/svgpointlist-animation-2.js
@@ -21,26 +21,29 @@ rootSVGElement.appendChild(poly);
 // Setup animation test
 function sample1() {
     // Check initial/end conditions
-    shouldBeCloseEnough("poly.points.getItem(2).x", "200");
-    shouldBeCloseEnough("poly.points.getItem(2).y", "200");
-    // shouldBeCloseEnough("poly.animatedPoints.getItem(2).x", "200");
-    // shouldBeCloseEnough("poly.animatedPoints.getItem(2).y", "200");
+    shouldBeCloseEnough("poly.animatedPoints.getItem(2).x", "200");
+    shouldBeCloseEnough("poly.animatedPoints.getItem(2).y", "200");
+
+    shouldBe("poly.points.getItem(2).x", "200");
+    shouldBe("poly.points.getItem(2).y", "200");
 }
 
 function sample2() {
     // Check half-time conditions
-    shouldBeCloseEnough("poly.points.getItem(2).x", "250");
-    shouldBeCloseEnough("poly.points.getItem(2).y", "250");
-    // shouldBeCloseEnough("poly.animatedPoints.getItem(2).x", "250");
-    // shouldBeCloseEnough("poly.animatedPoints.getItem(2).y", "250");
+    shouldBeCloseEnough("poly.animatedPoints.getItem(2).x", "250");
+    shouldBeCloseEnough("poly.animatedPoints.getItem(2).y", "250");
+
+    shouldBe("poly.points.getItem(2).x", "200");
+    shouldBe("poly.points.getItem(2).y", "200");
 }
 
 function sample3() {
     // Check just before-end conditions
-    shouldBeCloseEnough("poly.points.getItem(2).x", "300");
-    shouldBeCloseEnough("poly.points.getItem(2).y", "300");
-    // shouldBeCloseEnough("poly.animatedPoints.getItem(2).x", "300");
-    // shouldBeCloseEnough("poly.animatedPoints.getItem(2).y", "300");
+    shouldBeCloseEnough("poly.animatedPoints.getItem(2).x", "300");
+    shouldBeCloseEnough("poly.animatedPoints.getItem(2).y", "300");
+
+    shouldBe("poly.points.getItem(2).x", "200");
+    shouldBe("poly.points.getItem(2).y", "200");
 }
 
 function executeTest() {

diff --git a/LayoutTests/svg/animations/svgpointlist-animation-1-expected.txt b/LayoutTests/svg/animations/svgpointlist-animation-1-expected.txt
index 1c11bd2..b3eb9e4 100644 (file)
--- a/LayoutTests/svg/animations/svgpointlist-animation-1-expected.txt
+++ b/LayoutTests/svg/animations/svgpointlist-animation-1-expected.txt
@@ -5,12 +5,20 @@ Tests from-to animation of points on polygons.
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
+PASS poly.animatedPoints.getItem(2).x is 200
+PASS poly.animatedPoints.getItem(2).y is 200
 PASS poly.points.getItem(2).x is 200
 PASS poly.points.getItem(2).y is 200
-PASS poly.points.getItem(2).x is 150
-PASS poly.points.getItem(2).y is 150
-PASS poly.points.getItem(2).x is 100
-PASS poly.points.getItem(2).y is 100
+PASS poly.animatedPoints.getItem(2).x is 150
+PASS poly.animatedPoints.getItem(2).y is 150
+PASS poly.points.getItem(2).x is 200
+PASS poly.points.getItem(2).y is 200
+PASS poly.animatedPoints.getItem(2).x is 100
+PASS poly.animatedPoints.getItem(2).y is 100
+PASS poly.points.getItem(2).x is 200
+PASS poly.points.getItem(2).y is 200
+PASS poly.animatedPoints.getItem(2).x is 200
+PASS poly.animatedPoints.getItem(2).y is 200
 PASS poly.points.getItem(2).x is 200
 PASS poly.points.getItem(2).y is 200
 PASS successfullyParsed is true

diff --git a/LayoutTests/svg/animations/svgpointlist-animation-2-expected.txt b/LayoutTests/svg/animations/svgpointlist-animation-2-expected.txt
index 49f72f1..588a5ef 100644 (file)
--- a/LayoutTests/svg/animations/svgpointlist-animation-2-expected.txt
+++ b/LayoutTests/svg/animations/svgpointlist-animation-2-expected.txt
@@ -5,12 +5,20 @@ Tests from-by animation of points on polygons.
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
+PASS poly.animatedPoints.getItem(2).x is 200
+PASS poly.animatedPoints.getItem(2).y is 200
 PASS poly.points.getItem(2).x is 200
 PASS poly.points.getItem(2).y is 200
-PASS poly.points.getItem(2).x is 250
-PASS poly.points.getItem(2).y is 250
-PASS poly.points.getItem(2).x is 300
-PASS poly.points.getItem(2).y is 300
+PASS poly.animatedPoints.getItem(2).x is 250
+PASS poly.animatedPoints.getItem(2).y is 250
+PASS poly.points.getItem(2).x is 200
+PASS poly.points.getItem(2).y is 200
+PASS poly.animatedPoints.getItem(2).x is 300
+PASS poly.animatedPoints.getItem(2).y is 300
+PASS poly.points.getItem(2).x is 200
+PASS poly.points.getItem(2).y is 200
+PASS poly.animatedPoints.getItem(2).x is 200
+PASS poly.animatedPoints.getItem(2).y is 200
 PASS poly.points.getItem(2).x is 200
 PASS poly.points.getItem(2).y is 200
 PASS successfullyParsed is true

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 4be81db..d448ba0 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,33 @@
+2012-04-01  Nikolas Zimmermann  <nzimmermann@rim.com>
+
+        Animate animatedPoints instead of points for SVGPoly*Elements
+        https://bugs.webkit.org/show_bug.cgi?id=82844
+
+        Reviewed by Dirk Schulze.
+
+        When the points attribute of a SVGPoly*Element is animated, we should animate
+        the 'animatedPoints' SVG DOM property, not the 'points' property, which corresponds
+        to the "baseVal". Fix that, now only the 'd' attribute for SVGPathElement is incorrect,
+        everything else is moved to the new animVal concept!
+
+        * svg/SVGAnimatedPointList.cpp:
+        (WebCore::SVGAnimatedPointListAnimator::startAnimValAnimation):
+        (WebCore::SVGAnimatedPointListAnimator::stopAnimValAnimation):
+        (WebCore::SVGAnimatedPointListAnimator::resetAnimValToBaseVal):
+        (WebCore::SVGAnimatedPointListAnimator::animValWillChange):
+        (WebCore::SVGAnimatedPointListAnimator::animValDidChange):
+        * svg/SVGAnimatedPointList.h:
+        (SVGAnimatedPointListAnimator):
+        * svg/SVGAnimatedType.cpp:
+        (WebCore::SVGAnimatedType::valueAsString):
+        (WebCore::SVGAnimatedType::setValueAsString):
+        (WebCore::SVGAnimatedType::supportsAnimVal):
+        * svg/SVGPolyElement.cpp:
+        (WebCore::SVGPolyElement::parseAttribute):
+        (WebCore::SVGPolyElement::lookupOrCreatePointsWrapper):
+        (WebCore::SVGPolyElement::points):
+        (WebCore::SVGPolyElement::animatedPoints):
+
 2012-04-03  Nikolas Zimmermann  <nzimmermann@rim.com>
 
         Not reviewed. Next chromium build fix, this time for real :-)

diff --git a/Source/WebCore/svg/SVGAnimatedPointList.cpp b/Source/WebCore/svg/SVGAnimatedPointList.cpp
index 7325b5b..8acabc4 100644 (file)
--- a/Source/WebCore/svg/SVGAnimatedPointList.cpp
+++ b/Source/WebCore/svg/SVGAnimatedPointList.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) Research In Motion Limited 2011. All rights reserved.
+ * Copyright (C) Research In Motion Limited 2011, 2012. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -40,6 +40,31 @@ PassOwnPtr<SVGAnimatedType> SVGAnimatedPointListAnimator::constructFromString(co
     return animtedType.release();
 }
 
+PassOwnPtr<SVGAnimatedType> SVGAnimatedPointListAnimator::startAnimValAnimation(const Vector<SVGAnimatedProperty*>& properties)
+{
+    return SVGAnimatedType::createPointList(constructFromBaseValue<SVGAnimatedPointList>(properties));
+}
+
+void SVGAnimatedPointListAnimator::stopAnimValAnimation(const Vector<SVGAnimatedProperty*>& properties)
+{
+    stopAnimValAnimationForType<SVGAnimatedPointList>(properties);
+}
+
+void SVGAnimatedPointListAnimator::resetAnimValToBaseVal(const Vector<SVGAnimatedProperty*>& properties, SVGAnimatedType* type)
+{
+    resetFromBaseValue<SVGAnimatedPointList>(properties, type, &SVGAnimatedType::pointList);
+}
+
+void SVGAnimatedPointListAnimator::animValWillChange(const Vector<SVGAnimatedProperty*>& properties)
+{
+    animValWillChangeForType<SVGAnimatedPointList>(properties);
+}
+
+void SVGAnimatedPointListAnimator::animValDidChange(const Vector<SVGAnimatedProperty*>& properties)
+{
+    animValDidChangeForType<SVGAnimatedPointList>(properties);
+}
+
 void SVGAnimatedPointListAnimator::calculateFromAndToValues(OwnPtr<SVGAnimatedType>& from, OwnPtr<SVGAnimatedType>& to, const String& fromString, const String& toString)
 {
     from = constructFromString(fromString);

diff --git a/Source/WebCore/svg/SVGAnimatedPointList.h b/Source/WebCore/svg/SVGAnimatedPointList.h
index 2e5b55e..a951df5 100644 (file)
--- a/Source/WebCore/svg/SVGAnimatedPointList.h
+++ b/Source/WebCore/svg/SVGAnimatedPointList.h
@@ -21,19 +21,28 @@
 #define SVGAnimatedPointList_h
 
 #if ENABLE(SVG)
+#include "SVGAnimatedListPropertyTearOff.h"
 #include "SVGAnimatedTypeAnimator.h"
+#include "SVGPointList.h"
 
 namespace WebCore {
+
+typedef SVGAnimatedListPropertyTearOff<SVGPointList> SVGAnimatedPointList;
+
 class SVGAnimationElement;
 
 class SVGAnimatedPointListAnimator : public SVGAnimatedTypeAnimator {
-    
 public:
     SVGAnimatedPointListAnimator(SVGAnimationElement*, SVGElement*);
     virtual ~SVGAnimatedPointListAnimator() { }
     
     virtual PassOwnPtr<SVGAnimatedType> constructFromString(const String&);
-    
+    virtual PassOwnPtr<SVGAnimatedType> startAnimValAnimation(const Vector<SVGAnimatedProperty*>&);
+    virtual void stopAnimValAnimation(const Vector<SVGAnimatedProperty*>&);
+    virtual void resetAnimValToBaseVal(const Vector<SVGAnimatedProperty*>&, SVGAnimatedType*);
+    virtual void animValWillChange(const Vector<SVGAnimatedProperty*>&);
+    virtual void animValDidChange(const Vector<SVGAnimatedProperty*>&);
+
     virtual void calculateFromAndToValues(OwnPtr<SVGAnimatedType>& fromValue, OwnPtr<SVGAnimatedType>& toValue, const String& fromString, const String& toString);
     virtual void calculateFromAndByValues(OwnPtr<SVGAnimatedType>& fromValue, OwnPtr<SVGAnimatedType>& toValue, const String& fromString, const String& byString);
     virtual void calculateAnimatedValue(float percentage, unsigned repeatCount,

diff --git a/Source/WebCore/svg/SVGAnimatedType.cpp b/Source/WebCore/svg/SVGAnimatedType.cpp
index 8a14861..24e6da1 100644 (file)
--- a/Source/WebCore/svg/SVGAnimatedType.cpp
+++ b/Source/WebCore/svg/SVGAnimatedType.cpp
@@ -360,9 +360,6 @@ String SVGAnimatedType::valueAsString()
         SVGPathParserFactory::self()->buildStringFromByteStream(m_data.path, result, UnalteredParsing);
         return result;
     }
-    case AnimatedPoints:
-        ASSERT(m_data.pointList);
-        return m_data.pointList->valueAsString();
     case AnimatedRect:
         ASSERT(m_data.rect);
         return String::number(m_data.rect->x()) + ' ' + String::number(m_data.rect->y()) + ' '
@@ -379,6 +376,7 @@ String SVGAnimatedType::valueAsString()
     case AnimatedIntegerOptionalInteger:
     case AnimatedNumberList:
     case AnimatedNumberOptionalNumber:
+    case AnimatedPoints:
     case AnimatedPreserveAspectRatio:
     case AnimatedTransformList:
     case AnimatedUnknown:
@@ -418,11 +416,6 @@ bool SVGAnimatedType::setValueAsString(const QualifiedName& attrName, const Stri
         m_data.path = pathByteStream.leakPtr();
         break;
     }
-    case AnimatedPoints:
-        ASSERT(m_data.pointList);
-        m_data.pointList->clear();
-        pointsListFromSVGData(*m_data.pointList, value);
-        break;
     case AnimatedRect:
         ASSERT(m_data.rect);
         parseRect(value, *m_data.rect);
@@ -440,6 +433,7 @@ bool SVGAnimatedType::setValueAsString(const QualifiedName& attrName, const Stri
     case AnimatedIntegerOptionalInteger:
     case AnimatedNumberList:
     case AnimatedNumberOptionalNumber:
+    case AnimatedPoints:
     case AnimatedPreserveAspectRatio:
     case AnimatedTransformList:
     case AnimatedUnknown:
@@ -463,6 +457,7 @@ bool SVGAnimatedType::supportsAnimVal(AnimatedPropertyType type)
     case AnimatedNumber:
     case AnimatedNumberList:
     case AnimatedNumberOptionalNumber:
+    case AnimatedPoints:
     case AnimatedPreserveAspectRatio:
     case AnimatedRect:
     case AnimatedString:
@@ -475,7 +470,6 @@ bool SVGAnimatedType::supportsAnimVal(AnimatedPropertyType type)
 
     // FIXME: Handle the remaining types in animVal concept.
     case AnimatedPath:
-    case AnimatedPoints:
     case AnimatedUnknown:
         return false;
     }

diff --git a/Source/WebCore/svg/SVGPolyElement.cpp b/Source/WebCore/svg/SVGPolyElement.cpp
index cae6251..2bb7cb2 100644 (file)
--- a/Source/WebCore/svg/SVGPolyElement.cpp
+++ b/Source/WebCore/svg/SVGPolyElement.cpp
@@ -28,10 +28,10 @@
 #include "FloatPoint.h"
 #include "RenderSVGPath.h"
 #include "RenderSVGResource.h"
+#include "SVGAnimatedPointList.h"
 #include "SVGElementInstance.h"
 #include "SVGNames.h"
 #include "SVGParserUtilities.h"
-#include "SVGPointList.h"
 
 namespace WebCore {
 
@@ -90,8 +90,8 @@ void SVGPolyElement::parseAttribute(Attribute* attr)
         if (!pointsListFromSVGData(newList, value))
             document()->accessSVGExtensions()->reportError("Problem parsing points=\"" + value + "\"");
 
-        if (SVGAnimatedProperty* wrapper = SVGAnimatedProperty::lookupWrapper<SVGPolyElement, SVGAnimatedListPropertyTearOff<SVGPointList>, true>(this, pointsPropertyInfo()))
-            static_cast<SVGAnimatedListPropertyTearOff<SVGPointList>*>(wrapper)->detachListWrappers(newList.size());
+        if (SVGAnimatedProperty* wrapper = SVGAnimatedProperty::lookupWrapper<SVGPolyElement, SVGAnimatedPointList, true>(this, pointsPropertyInfo()))
+            static_cast<SVGAnimatedPointList*>(wrapper)->detachListWrappers(newList.size());
 
         m_points.value = newList;
         return;
@@ -150,20 +150,20 @@ PassRefPtr<SVGAnimatedProperty> SVGPolyElement::lookupOrCreatePointsWrapper(void
 {
     ASSERT(contextElement);
     SVGPolyElement* ownerType = static_cast<SVGPolyElement*>(contextElement);
-    return SVGAnimatedProperty::lookupOrCreateWrapper<SVGPolyElement, SVGAnimatedListPropertyTearOff<SVGPointList>, SVGPointList, true>
+    return SVGAnimatedProperty::lookupOrCreateWrapper<SVGPolyElement, SVGAnimatedPointList, SVGPointList, true>
            (ownerType, pointsPropertyInfo(), ownerType->m_points.value);
 }
 
 SVGListPropertyTearOff<SVGPointList>* SVGPolyElement::points()
 {
     m_points.shouldSynchronize = true;
-    return static_cast<SVGListPropertyTearOff<SVGPointList>*>(static_pointer_cast<SVGAnimatedListPropertyTearOff<SVGPointList> >(lookupOrCreatePointsWrapper(this))->baseVal());
+    return static_cast<SVGListPropertyTearOff<SVGPointList>*>(static_pointer_cast<SVGAnimatedPointList>(lookupOrCreatePointsWrapper(this))->baseVal());
 }
 
 SVGListPropertyTearOff<SVGPointList>* SVGPolyElement::animatedPoints()
 {
     m_points.shouldSynchronize = true;
-    return static_cast<SVGListPropertyTearOff<SVGPointList>*>(static_pointer_cast<SVGAnimatedListPropertyTearOff<SVGPointList> >(lookupOrCreatePointsWrapper(this))->animVal());
+    return static_cast<SVGListPropertyTearOff<SVGPointList>*>(static_pointer_cast<SVGAnimatedPointList>(lookupOrCreatePointsWrapper(this))->animVal());
 }
 
 }