Bug 614879 - SVG SMIL: Fix indefinite to-animation; r=dholbert, a=roc

--HG--
extra : rebase_source : 9abf06da6a8e24f814623c370e66d77ec467c0e6

content/smil/nsSMILAnimationFunction.cpp

@@ -256,9 +256,9 @@ nsSMILAnimationFunction::ComposeResult(const nsISMILAttr& aSMILAttr,
 
   nsSMILValue result;
 
-  if (mSimpleDuration.IsIndefinite() ||
+  if (values.Length() == 1 && !IsToAnimation()) {
-      (values.Length() == 1 && TreatSingleValueAsStatic())) {
+
-    // Indefinite duration or only one value set: Always set the first value
+    // Single-valued animation
     result = values[0];
 
   } else if (mLastValue) {
@@ -379,28 +379,37 @@ nsSMILAnimationFunction::InterpolateResult(const nsSMILValueArray& aValues,
                                            nsSMILValue& aResult,
                                            nsSMILValue& aBaseValue)
 {
-  nsresult rv = NS_OK;
+  // Sanity check animation values
-  const nsSMILTime& dur = mSimpleDuration.GetMillis();
-
-  // Sanity Checks
-  NS_ABORT_IF_FALSE(mSampleTime >= 0.0f, "Sample time should not be negative");
-  NS_ABORT_IF_FALSE(dur >= 0.0f, "Simple duration should not be negative");
-
-  if (mSampleTime >= dur || mSampleTime < 0.0f) {
-    NS_ERROR("Animation sampled outside interval");
-    return NS_ERROR_FAILURE;
-  }
-
   if ((!IsToAnimation() && aValues.Length() < 2) ||
       (IsToAnimation()  && aValues.Length() != 1)) {
     NS_ERROR("Unexpected number of values");
     return NS_ERROR_FAILURE;
   }
-  // End Sanity Checks
 
-  // Get the normalised progress through the simple duration
+  // Get the normalised progress through the simple duration.
-  const double simpleProgress = dur > 0.0 ? (double)mSampleTime / dur : 0.0;
+  //
+  // If we have an indefinite simple duration, just set the progress to be
+  // 0 which will give us the expected behaviour of the animation being fixed at
+  // its starting point.
+  double simpleProgress = 0.0;
 
+  if (mSimpleDuration.IsResolved()) {
+    nsSMILTime dur = mSimpleDuration.GetMillis();
+
+    NS_ABORT_IF_FALSE(dur >= 0, "Simple duration should not be negative");
+    NS_ABORT_IF_FALSE(mSampleTime >= 0, "Sample time should not be negative");
+
+    if (mSampleTime >= dur || mSampleTime < 0) {
+      NS_ERROR("Animation sampled outside interval");
+      return NS_ERROR_FAILURE;
+    }
+
+    if (dur > 0) {
+      simpleProgress = (double)mSampleTime / dur;
+    } // else leave simpleProgress at 0.0 (e.g. if mSampleTime == dur == 0)
+  }
+
+  nsresult rv = NS_OK;
   nsSMILCalcMode calcMode = GetCalcMode();
   if (calcMode != CALC_DISCRETE) {
     // Get the normalised progress between adjacent values
@@ -424,26 +433,25 @@ nsSMILAnimationFunction::InterpolateResult(const nsSMILValueArray& aValues,
           intervalProgress = ScaleIntervalProgress(scaledSimpleProgress, 0);
         }
       }
-    } else {
+    } else if (calcMode == CALC_PACED) {
-      if (calcMode == CALC_PACED) {
+      rv = ComputePacedPosition(aValues, simpleProgress,
-        rv = ComputePacedPosition(aValues, simpleProgress,
+                                intervalProgress, from, to);
-                                  intervalProgress, from, to);
+      // Note: If the above call fails, we'll skip the "from->Interpolate"
-        // Note: If the above call fails, we'll skip the "from->Interpolate"
+      // call below, and we'll drop into the CALC_DISCRETE section
-        // call below, and we'll drop into the CALC_DISCRETE section
+      // instead. (as the spec says we should, because our failure was
-        // instead. (as the spec says we should, because our failure was
+      // presumably due to the values being non-additive)
-        // presumably due to the values being non-additive)
+    } else { // calcMode == CALC_LINEAR or calcMode == CALC_SPLINE
-      } else { // calcMode == CALC_LINEAR or calcMode == CALC_SPLINE
+      double scaledSimpleProgress =
-        double scaledSimpleProgress =
+        ScaleSimpleProgress(simpleProgress, calcMode);
-          ScaleSimpleProgress(simpleProgress, calcMode);
+      PRUint32 index = (PRUint32)floor(scaledSimpleProgress *
-        PRUint32 index = (PRUint32)floor(scaledSimpleProgress *
+                                       (aValues.Length() - 1));
-                                         (aValues.Length() - 1));
+      from = &aValues[index];
-        from = &aValues[index];
+      to = &aValues[index + 1];
-        to = &aValues[index + 1];
+      intervalProgress =
-        intervalProgress =
+        scaledSimpleProgress * (aValues.Length() - 1) - index;
-          scaledSimpleProgress * (aValues.Length() - 1) - index;
+      intervalProgress = ScaleIntervalProgress(intervalProgress, index);
-        intervalProgress = ScaleIntervalProgress(intervalProgress, index);
-      }
     }
+
     if (NS_SUCCEEDED(rv)) {
       NS_ABORT_IF_FALSE(from, "NULL from-value during interpolation");
       NS_ABORT_IF_FALSE(to, "NULL to-value during interpolation");

content/smil/nsSMILAnimationFunction.h

@@ -338,14 +338,7 @@ protected:
   void         CheckKeyTimes(PRUint32 aNumValues);
   void         CheckKeySplines(PRUint32 aNumValues);
 
-  // When GetValues() returns a single-value array, this method indicates
+  virtual PRBool IsToAnimation() const {
-  // whether that single value can be understood to be a static value, to be
-  // set for the full animation duration.
-  virtual PRBool TreatSingleValueAsStatic() const {
-    return HasAttr(nsGkAtoms::values);
-  }
-
-  inline PRBool IsToAnimation() const {
     return !HasAttr(nsGkAtoms::values) &&
             HasAttr(nsGkAtoms::to) &&
            !HasAttr(nsGkAtoms::from);

content/smil/nsSMILSetAnimationFunction.h

@@ -74,11 +74,12 @@ public:
   NS_OVERRIDE virtual PRBool UnsetAttr(nsIAtom* aAttribute);
 
 protected:
-  // <set> uses the "to" attribute as its only source of animation values
+  // Although <set> animation might look like to-animation, unlike to-animation,
-  // (which gives us a single value in our values array), and we want to use
+  // it never interpolates values.
-  // that value whenever the animation is active (no interpolation or anything).
+  // Returning PR_FALSE here will mean this animation function gets treated as
-  NS_OVERRIDE virtual PRBool TreatSingleValueAsStatic() const {
+  // a single-valued function and no interpolation will be attempted.
-    return PR_TRUE;
+  NS_OVERRIDE virtual PRBool IsToAnimation() const {
+    return PR_FALSE;
   }
   NS_OVERRIDE virtual PRBool             HasAttr(nsIAtom* aAttName) const;
   NS_OVERRIDE virtual const nsAttrValue* GetAttr(nsIAtom* aAttName) const;

layout/reftests/svg/smil/anim-indefinite-to-1.svg

@@ -0,0 +1,7 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+  <!-- Test that an indefinite to-animation just sticks to the base value for
+       interpolatable attributes. -->
+  <rect x="15" y="15" width="200" height="200" fill="blue">
+    <animate attributeName="height" to="100" dur="indefinite"/>
+  </rect>
+</svg>

layout/reftests/svg/smil/anim-indefinite-to-2.svg

@@ -0,0 +1,8 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+  <!-- Test that an indefinite to-animation with discrete calcMode still applies
+       the to-value for the whole time. -->
+  <rect x="15" y="15" width="200" height="100" fill="blue">
+    <animate attributeName="height" to="200" dur="indefinite"
+      calcMode="discrete"/>
+  </rect>
+</svg>

layout/reftests/svg/smil/anim-indefinite-to-3.svg

@@ -0,0 +1,8 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+  <!-- Test that an indefinite to-animation that falls back to discrete calcMode
+       because the property is not interpolatable, still applies the to-value
+       for the whole time. -->
+  <rect x="15" y="15" width="200" height="200" fill="blue" visibility="hidden">
+    <animate attributeName="visibility" to="visible" dur="indefinite"/>
+  </rect>
+</svg>

layout/reftests/svg/smil/anim-indefinite-to-4.svg

@@ -0,0 +1,14 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+  <!-- This is not really a to-animation, but we want to check that set
+       animation isn't incorrectly treated as to-animation.
+
+       Provided the attribute being animated is interpolatable (as it is in this
+       case) and calcMode != discrete, to-animation will begin from the base
+       value (and never change in this case as the simple duration is
+       indefinite).
+
+       Set animation, however, never sets the base value, only the to value. -->
+  <rect x="15" y="15" width="200" height="100" fill="blue">
+    <set attributeName="height" to="200" dur="indefinite"/>
+  </rect>
+</svg>

layout/reftests/svg/smil/reftest.list

@@ -64,6 +64,11 @@ include event/reftest.list
 == anim-discrete-to-3.svg          anim-standard-ref.svg
 == anim-discrete-to-4.svg          anim-standard-ref.svg
 
+== anim-indefinite-to-1.svg        anim-standard-ref.svg
+== anim-indefinite-to-2.svg        anim-standard-ref.svg
+== anim-indefinite-to-3.svg        anim-standard-ref.svg
+== anim-indefinite-to-4.svg        anim-standard-ref.svg
+
 fails == anim-fillcolor-1.svg      anim-standard-ref.svg # bug 436296
 == anim-fillopacity-1none.svg anim-standard-ref.svg
 == anim-fillopacity-1css.svg  anim-standard-ref.svg