2011-02-10 Dirk Schulze <krit@webkit.org>
authorkrit@webkit.org <krit@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 10 Feb 2011 22:45:04 +0000 (22:45 +0000)
committerkrit@webkit.org <krit@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 10 Feb 2011 22:45:04 +0000 (22:45 +0000)
        Reviewed by Andreas Kling.

        SVG animation avoid unnecessary adjust for currentColor
        https://bugs.webkit.org/show_bug.cgi?id=54243

        At the moment we try to adjust every animation value for currentColor - independent of the animation type.
        Since the value is a string and the target element may needs to get called by getElementById, this could
        be an expensive and unnecessary operation. Also after we adjust for currentColor, we save the result back
        as a string and parse it to Color afterwards again.
        With the patch we just adjust an animation value, if we use color animation. The color won't get saved and
        parsed as a string again.

        No change of functionality, no new tests.

        * svg/SVGAnimateElement.cpp:
        (WebCore::adjustForCurrentColor):
        (WebCore::SVGAnimateElement::calculateFromAndToValues):
        (WebCore::SVGAnimateElement::calculateFromAndByValues):
        * svg/SVGAnimationElement.cpp:
        (WebCore::SVGAnimationElement::currentValuesForValuesAnimation):
        (WebCore::SVGAnimationElement::startedActiveInterval):

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

Source/WebCore/ChangeLog
Source/WebCore/svg/SVGAnimateElement.cpp
Source/WebCore/svg/SVGAnimationElement.cpp

index 60ac5f8..bb680c9 100644 (file)
@@ -1,3 +1,27 @@
+2011-02-10  Dirk Schulze  <krit@webkit.org>
+
+        Reviewed by Andreas Kling.
+
+        SVG animation avoid unnecessary adjust for currentColor
+        https://bugs.webkit.org/show_bug.cgi?id=54243
+
+        At the moment we try to adjust every animation value for currentColor - independent of the animation type.
+        Since the value is a string and the target element may needs to get called by getElementById, this could
+        be an expensive and unnecessary operation. Also after we adjust for currentColor, we save the result back
+        as a string and parse it to Color afterwards again.
+        With the patch we just adjust an animation value, if we use color animation. The color won't get saved and
+        parsed as a string again.
+
+        No change of functionality, no new tests.
+
+        * svg/SVGAnimateElement.cpp:
+        (WebCore::adjustForCurrentColor):
+        (WebCore::SVGAnimateElement::calculateFromAndToValues):
+        (WebCore::SVGAnimateElement::calculateFromAndByValues):
+        * svg/SVGAnimationElement.cpp:
+        (WebCore::SVGAnimationElement::currentValuesForValuesAnimation):
+        (WebCore::SVGAnimationElement::startedActiveInterval):
+
 2011-02-10  Ademar de Souza Reis Jr  <ademar.reis@openbossa.org>
 
         Reviewed by Andreas Kling.
index b18d8bd..0dfe086 100644 (file)
 #if ENABLE(SVG) && ENABLE(SVG_ANIMATION)
 #include "SVGAnimateElement.h"
 
+#include "CSSPropertyNames.h"
 #include "ColorDistance.h"
 #include "FloatConversion.h"
 #include "QualifiedName.h"
+#include "RenderObject.h"
 #include "SVGColor.h"
 #include "SVGNames.h"
 #include "SVGParserUtilities.h"
@@ -83,6 +85,17 @@ static bool parseNumberValueAndUnit(const String& in, double& value, String& uni
     return ok;
 }
 
+static inline bool adjustForCurrentColor(Color& color, const String& value, SVGElement* target)
+{
+    if (!target || !target->isStyled() || value != "currentColor")
+        return false;
+    
+    if (RenderObject* targetRenderer = target->renderer())
+        color = targetRenderer->style()->visitedDependentColor(CSSPropertyColor);
+
+    return true;
+}
+
 SVGAnimateElement::PropertyType SVGAnimateElement::determinePropertyType(const String& attribute) const
 {
     // FIXME: We should not allow animation of attribute types other than AnimatedColor for <animateColor>.
@@ -216,8 +229,11 @@ bool SVGAnimateElement::calculateFromAndToValues(const String& fromString, const
     // FIXME: Needs more solid way determine target attribute type.
     m_propertyType = determinePropertyType(attributeName());
     if (m_propertyType == ColorProperty) {
-        m_fromColor = SVGColor::colorFromRGBColorString(fromString);
-        m_toColor = SVGColor::colorFromRGBColorString(toString);
+        SVGElement* targetElement = this->targetElement();
+        if (!adjustForCurrentColor(m_fromColor, fromString, targetElement))
+            m_fromColor = SVGColor::colorFromRGBColorString(fromString);
+        if (!adjustForCurrentColor(m_toColor, toString, targetElement))
+            m_toColor = SVGColor::colorFromRGBColorString(toString);
         if ((m_fromColor.isValid() && m_toColor.isValid()) || (m_toColor.isValid() && animationMode() == ToAnimation))
             return true;
     } else if (m_propertyType == NumberProperty) {
@@ -255,8 +271,12 @@ bool SVGAnimateElement::calculateFromAndByValues(const String& fromString, const
     ASSERT(!hasTagName(SVGNames::setTag));
     m_propertyType = determinePropertyType(attributeName());
     if (m_propertyType == ColorProperty) {
-        m_fromColor = fromString.isEmpty() ? Color() : SVGColor::colorFromRGBColorString(fromString);
-        m_toColor = ColorDistance::addColorsAndClamp(m_fromColor, SVGColor::colorFromRGBColorString(byString));
+        SVGElement* targetElement = this->targetElement();
+        if (!adjustForCurrentColor(m_fromColor, fromString, targetElement))
+            m_fromColor = fromString.isEmpty() ? Color() : SVGColor::colorFromRGBColorString(fromString);
+        if (!adjustForCurrentColor(m_toColor, byString, targetElement))
+            m_toColor = SVGColor::colorFromRGBColorString(byString);
+        m_toColor = ColorDistance::addColorsAndClamp(m_fromColor, m_toColor);
         if (!m_fromColor.isValid() || !m_toColor.isValid())
             return false;
     } else {
index 0fcda6d..c56e3b6 100644 (file)
@@ -38,7 +38,6 @@
 #include "FloatConversion.h"
 #include "HTMLNames.h"
 #include "PlatformString.h"
-#include "RenderObject.h"
 #include "SVGElementInstance.h"
 #include "SVGNames.h"
 #include "SVGParserUtilities.h"
@@ -499,14 +498,6 @@ void SVGAnimationElement::currentValuesForValuesAnimation(float percent, float&
         effectivePercent = calculatePercentForSpline(effectivePercent, index);
     }
 }
-static inline void adjustForCurrentColor(String& value, SVGElement* target)
-{
-    if (!target || !target->isStyled() || value != "currentColor")
-        return;
-
-    if (RenderObject* targetRenderer = target->renderer())
-        value = targetRenderer->style()->visitedDependentColor(CSSPropertyColor).name();
-}
     
 void SVGAnimationElement::startedActiveInterval()
 {
@@ -531,29 +522,22 @@ void SVGAnimationElement::startedActiveInterval()
     String from = fromValue();
     String to = toValue();
     String by = byValue();
-    SVGElement* target = targetElement();
     if (animationMode == NoAnimation)
         return;
-    if (animationMode == FromToAnimation) {
-        adjustForCurrentColor(from, target);
-        adjustForCurrentColor(to, target);
+    if (animationMode == FromToAnimation)
         m_animationValid = calculateFromAndToValues(from, to);
-    else if (animationMode == ToAnimation) {
+    else if (animationMode == ToAnimation) {
         // For to-animations the from value is the current accumulated value from lower priority animations.
         // The value is not static and is determined during the animation.
-        adjustForCurrentColor(to, target);
         m_animationValid = calculateFromAndToValues(String(), to);
-    } else if (animationMode == FromByAnimation) {
-        adjustForCurrentColor(from, target);
-        adjustForCurrentColor(by, target);
+    } else if (animationMode == FromByAnimation)
         m_animationValid = calculateFromAndByValues(from, by);
-    } else if (animationMode == ByAnimation) {
-        adjustForCurrentColor(by, target);
+    else if (animationMode == ByAnimation)
         m_animationValid = calculateFromAndByValues(String(), by);
-    else if (animationMode == ValuesAnimation) {
+    else if (animationMode == ValuesAnimation) {
         m_animationValid = m_values.size() > 1
             && (calcMode == CalcModePaced || !hasAttribute(SVGNames::keyTimesAttr) || hasAttribute(SVGNames::keyPointsAttr) || (m_values.size() == m_keyTimes.size()))
-            && (calcMode == CalcModeDiscrete || !m_keyTimes.size() || m_keyTimes.last() == 1.0)
+            && (calcMode == CalcModeDiscrete || !m_keyTimes.size() || m_keyTimes.last() == 1)
             && (calcMode != CalcModeSpline || ((m_keySplines.size() && (m_keySplines.size() == m_values.size() - 1)) || m_keySplines.size() == m_keyPoints.size() - 1))
             && (!hasAttribute(SVGNames::keyPointsAttr) || (m_keyTimes.size() > 1 && m_keyTimes.size() == m_keyPoints.size()));
         if (calcMode == CalcModePaced && m_animationValid)