Bug 603917 - SVG SMIL: Allow some forms of animation even if base value is not animatable; r=dholbert; a=blocking-2.0

content/smil/nsSMILAnimationFunction.cpp

@@ -246,7 +246,7 @@ nsSMILAnimationFunction::ComposeResult(const nsISMILAttr& aSMILAttr,
       mSimpleDuration.IsIndefinite() || mLastValue,
       "Unresolved simple duration for active or frozen animation");
 
-  nsSMILValue result(aResult.mType);
+  nsSMILValue result;
 
   if (mSimpleDuration.IsIndefinite() ||
       (values.Length() == 1 && TreatSingleValueAsStatic())) {
@@ -402,15 +402,19 @@ nsSMILAnimationFunction::InterpolateResult(const nsSMILValueArray& aValues,
     // NS_ABORT_IF_FALSE that tests that intervalProgress is in range will fail.
     double intervalProgress = -1.f;
     if (IsToAnimation()) {
-      from = &aBaseValue;
-      to = &aValues[0];
-      if (calcMode == CALC_PACED) {
-        // Note: key[Times/Splines/Points] are ignored for calcMode="paced"
-        intervalProgress = simpleProgress;
+      if (aBaseValue.IsNull()) {
+        rv = NS_ERROR_FAILURE;
       } else {
-        double scaledSimpleProgress =
-          ScaleSimpleProgress(simpleProgress, calcMode);
-        intervalProgress = ScaleIntervalProgress(scaledSimpleProgress, 0);
+        from = &aBaseValue;
+        to = &aValues[0];
+        if (calcMode == CALC_PACED) {
+          // Note: key[Times/Splines/Points] are ignored for calcMode="paced"
+          intervalProgress = simpleProgress;
+        } else {
+          double scaledSimpleProgress =
+            ScaleSimpleProgress(simpleProgress, calcMode);
+          intervalProgress = ScaleIntervalProgress(scaledSimpleProgress, 0);
+        }
       }
     } else {
       if (calcMode == CALC_PACED) {

content/smil/nsSMILAnimationFunction.h

@@ -201,7 +201,7 @@ public:
    * @return  True if the animation will replace, false if it will add or
    *          otherwise build on the passed in value.
    */
-  PRBool WillReplace() const;
+  virtual PRBool WillReplace() const;
 
   /**
    * Indicates if the parameters for this animation have changed since the last

content/smil/nsSMILCSSProperty.cpp

@@ -106,19 +106,17 @@ nsSMILCSSProperty::GetBaseValue() const
   //               (b) 'display'
   if (nsCSSProps::IsShorthand(mPropID) || mPropID == eCSSProperty_display) {
     // We can't look up the base (computed-style) value of shorthand
-    // properties, because they aren't guaranteed to have a consistent computed
-    // value.  However, that's not a problem, because it turns out the caller
-    // isn't going to end up using the value we return anyway. Base values only
-    // get used when there's interpolation or addition, and the shorthand
-    // properties we know about don't support those operations. So, we can just
-    // return a dummy value (initialized with the right type, so as not to
-    // indicate failure).
-    // For 'display' we'd like to avoid clearing and setting this property since
-    // it can cause frames to be recreated, so instead we just return a dummy
-    // value. As with shorthand properties this is ok as we never interpolate or
-    // add display properties.
-    nsSMILValue tmpVal(&nsSMILCSSValueType::sSingleton);
-    baseValue.Swap(tmpVal);
+    // properties because they aren't guaranteed to have a consistent computed
+    // value.
+    //
+    // Also, although we can look up the base value of the display property,
+    // doing so involves clearing and resetting the property which can cause
+    // frames to be recreated which we'd like to avoid.
+    //
+    // In either case, simply returning a null-typed nsSMILValue to indicate
+    // failure is acceptable because the caller only uses base values when
+    // there's interpolation or addition, and for both the shorthand properties
+    // we know about and the display property those operations aren't supported.
     return baseValue;
   }
 

content/smil/nsSMILCompositor.cpp

@@ -108,10 +108,9 @@ nsSMILCompositor::ComposeAttribute()
   PRUint32 firstFuncToCompose = GetFirstFuncToAffectSandwich();
 
   // FOURTH: Get & cache base value
-  nsSMILValue sandwichResultValue = smilAttr->GetBaseValue();
-  if (sandwichResultValue.IsNull()) {
-    NS_WARNING("nsISMILAttr::GetBaseValue failed");
-    return;
+  nsSMILValue sandwichResultValue;
+  if (!mAnimationFunctions[firstFuncToCompose]->WillReplace()) {
+    sandwichResultValue = smilAttr->GetBaseValue();
   }
   UpdateCachedBaseValue(sandwichResultValue);
 
@@ -124,6 +123,10 @@ nsSMILCompositor::ComposeAttribute()
   for (PRUint32 i = firstFuncToCompose; i < length; ++i) {
     mAnimationFunctions[i]->ComposeResult(*smilAttr, sandwichResultValue);
   }
+  if (sandwichResultValue.IsNull()) {
+    smilAttr->ClearAnimValue();
+    return;
+  }
 
   // SIXTH: Set the animated value to the final composited result.
   nsresult rv = smilAttr->SetAnimValue(sandwichResultValue);

content/smil/nsSMILSetAnimationFunction.cpp

@@ -124,3 +124,9 @@ nsSMILSetAnimationFunction::GetAttr(nsIAtom* aAttName,
 
   return nsSMILAnimationFunction::GetAttr(aAttName, aResult);
 }
+
+PRBool
+nsSMILSetAnimationFunction::WillReplace() const
+{
+  return PR_TRUE;
+}

content/smil/nsSMILSetAnimationFunction.h

@@ -84,6 +84,7 @@ protected:
   NS_OVERRIDE virtual const nsAttrValue* GetAttr(nsIAtom* aAttName) const;
   NS_OVERRIDE virtual PRBool             GetAttr(nsIAtom* aAttName,
                                                  nsAString& aResult) const;
+  NS_OVERRIDE virtual PRBool WillReplace() const;
 
   PRBool IsDisallowedAttribute(const nsIAtom* aAttribute) const;
 };

layout/reftests/svg/smil/anim-fill-overpaintserver-1.svg

@@ -0,0 +1,10 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+  <defs>
+    <linearGradient id="red">
+      <stop offset="0.0" stop-color="#f00"/>
+    </linearGradient>
+  </defs>
+  <rect width="100%" height="100%" fill="url(#red)">
+    <set attributeName="fill" to="lime"/>
+  </rect>
+</svg>

layout/reftests/svg/smil/anim-fill-overpaintserver-2.svg

@@ -0,0 +1,14 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+  <!-- In this test we will attempt to interpolate since fill is interpolatable
+       but fail since the base value can't be converted to an RGB color value
+       and hence should fall back to discrete calcMode which, in the case of
+       to-animation, sets the to value for the entire simple duration. -->
+  <defs>
+    <linearGradient id="red">
+      <stop offset="0.0" stop-color="#f00"/>
+    </linearGradient>
+  </defs>
+  <rect width="100%" height="100%" fill="url(#red)">
+    <animate attributeName="fill" to="lime" dur="500s"/>
+  </rect>
+</svg>

layout/reftests/svg/smil/reftest.list

@@ -118,6 +118,10 @@ fails == anim-fillcolor-1.svg      anim-standard-ref.svg # bug 436296
 == anim-pattern-href-01.svg lime.svg
 asserts(6) == anim-use-href-01.svg lime.svg  # the asserts here are bug 563481
 
+# animate where the base value is non-interpolatable but will be replaced anyway
+== anim-fill-overpaintserver-1.svg lime.svg
+== anim-fill-overpaintserver-2.svg lime.svg
+
 == anim-remove-1.svg anim-standard-ref.svg
 == anim-remove-2.svg anim-standard-ref.svg
 == anim-remove-3.svg anim-standard-ref.svg