Bug 614879 - SVG SMIL: Fix indefinite to-animation; r=dholbert, a=roc
authorBrian Birtles <birtles@gmail.com>
Sun, 05 Dec 2010 13:13:31 +0000
changeset 58629 566bac81d5febeca7bdf208162aea69cb353ad6d
parent 58628 02b080d3a0e0cfb6a7cdfe20cbdc117d03c56ede
child 58630 3e68a0b2cdc47c59681f11705baa49380757ad6e
push idunknown
push userunknown
push dateunknown
reviewersdholbert, roc
bugs614879
milestone2.0b8pre
Bug 614879 - SVG SMIL: Fix indefinite to-animation; r=dholbert, a=roc
content/smil/nsSMILAnimationFunction.cpp
content/smil/nsSMILAnimationFunction.h
content/smil/nsSMILSetAnimationFunction.h
layout/reftests/svg/smil/anim-indefinite-to-1.svg
layout/reftests/svg/smil/anim-indefinite-to-2.svg
layout/reftests/svg/smil/anim-indefinite-to-3.svg
layout/reftests/svg/smil/anim-indefinite-to-4.svg
layout/reftests/svg/smil/reftest.list
--- a/content/smil/nsSMILAnimationFunction.cpp
+++ b/content/smil/nsSMILAnimationFunction.cpp
@@ -251,19 +251,19 @@ nsSMILAnimationFunction::ComposeResult(c
   // animation function in the sandwich that should replace it but that function
   // failed unexpectedly.
   PRBool isAdditive = IsAdditive();
   if (isAdditive && aResult.IsNull())
     return;
 
   nsSMILValue result;
 
-  if (mSimpleDuration.IsIndefinite() ||
-      (values.Length() == 1 && TreatSingleValueAsStatic())) {
-    // Indefinite duration or only one value set: Always set the first value
+  if (values.Length() == 1 && !IsToAnimation()) {
+
+    // Single-valued animation
     result = values[0];
 
   } else if (mLastValue) {
 
     // Sampling last value
     const nsSMILValue& last = values[values.Length() - 1];
     result = last;
 
@@ -374,38 +374,47 @@ nsSMILAnimationFunction::UpdateCachedTar
 //----------------------------------------------------------------------
 // Implementation helpers
 
 nsresult
 nsSMILAnimationFunction::InterpolateResult(const nsSMILValueArray& aValues,
                                            nsSMILValue& aResult,
                                            nsSMILValue& aBaseValue)
 {
-  nsresult rv = NS_OK;
-  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;
-  }
-
+  // Sanity check animation values
   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.
+  //
+  // 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();
 
-  // Get the normalised progress through the simple duration
-  const double simpleProgress = dur > 0.0 ? (double)mSampleTime / dur : 0.0;
+    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
     const nsSMILValue* from = nsnull;
     const nsSMILValue* to = nsnull;
     // Init to -1 to make sure that if we ever forget to set this, the
     // NS_ABORT_IF_FALSE that tests that intervalProgress is in range will fail.
     double intervalProgress = -1.f;
@@ -419,36 +428,35 @@ nsSMILAnimationFunction::InterpolateResu
           // 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) {
-        rv = ComputePacedPosition(aValues, simpleProgress,
-                                  intervalProgress, from, to);
-        // Note: If the above call fails, we'll skip the "from->Interpolate"
-        // call below, and we'll drop into the CALC_DISCRETE section
-        // instead. (as the spec says we should, because our failure was
-        // presumably due to the values being non-additive)
-      } else { // calcMode == CALC_LINEAR or calcMode == CALC_SPLINE
-        double scaledSimpleProgress =
-          ScaleSimpleProgress(simpleProgress, calcMode);
-        PRUint32 index = (PRUint32)floor(scaledSimpleProgress *
-                                         (aValues.Length() - 1));
-        from = &aValues[index];
-        to = &aValues[index + 1];
-        intervalProgress =
-          scaledSimpleProgress * (aValues.Length() - 1) - index;
-        intervalProgress = ScaleIntervalProgress(intervalProgress, index);
-      }
+    } else if (calcMode == CALC_PACED) {
+      rv = ComputePacedPosition(aValues, simpleProgress,
+                                intervalProgress, from, to);
+      // Note: If the above call fails, we'll skip the "from->Interpolate"
+      // call below, and we'll drop into the CALC_DISCRETE section
+      // instead. (as the spec says we should, because our failure was
+      // presumably due to the values being non-additive)
+    } else { // calcMode == CALC_LINEAR or calcMode == CALC_SPLINE
+      double scaledSimpleProgress =
+        ScaleSimpleProgress(simpleProgress, calcMode);
+      PRUint32 index = (PRUint32)floor(scaledSimpleProgress *
+                                       (aValues.Length() - 1));
+      from = &aValues[index];
+      to = &aValues[index + 1];
+      intervalProgress =
+        scaledSimpleProgress * (aValues.Length() - 1) - 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");
       NS_ABORT_IF_FALSE(0.0f <= intervalProgress && intervalProgress < 1.0f,
                       "Interval progress should be in the range [0, 1)");
       rv = from->Interpolate(*to, intervalProgress, aResult);
     }
   }
--- a/content/smil/nsSMILAnimationFunction.h
+++ b/content/smil/nsSMILAnimationFunction.h
@@ -333,24 +333,17 @@ protected:
 
   virtual nsresult GetValues(const nsISMILAttr& aSMILAttr,
                              nsSMILValueArray& aResult);
 
   virtual void CheckValueListDependentAttrs(PRUint32 aNumValues);
   void         CheckKeyTimes(PRUint32 aNumValues);
   void         CheckKeySplines(PRUint32 aNumValues);
 
-  // When GetValues() returns a single-value array, this method indicates
-  // 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 {
+  virtual PRBool IsToAnimation() const {
     return !HasAttr(nsGkAtoms::values) &&
             HasAttr(nsGkAtoms::to) &&
            !HasAttr(nsGkAtoms::from);
   }
 
   inline PRBool IsAdditive() const {
     /*
      * Animation is additive if:
--- a/content/smil/nsSMILSetAnimationFunction.h
+++ b/content/smil/nsSMILSetAnimationFunction.h
@@ -69,21 +69,22 @@ public:
    * Unsets the given attribute.
    *
    * @returns PR_TRUE if aAttribute is a recognized animation-related
    *          attribute; PR_FALSE otherwise.
    */
   NS_OVERRIDE virtual PRBool UnsetAttr(nsIAtom* aAttribute);
 
 protected:
-  // <set> uses the "to" attribute as its only source of animation values
-  // (which gives us a single value in our values array), and we want to use
-  // that value whenever the animation is active (no interpolation or anything).
-  NS_OVERRIDE virtual PRBool TreatSingleValueAsStatic() const {
-    return PR_TRUE;
+  // Although <set> animation might look like to-animation, unlike to-animation,
+  // it never interpolates values.
+  // Returning PR_FALSE here will mean this animation function gets treated as
+  // a single-valued function and no interpolation will be attempted.
+  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;
   NS_OVERRIDE virtual PRBool             GetAttr(nsIAtom* aAttName,
                                                  nsAString& aResult) const;
   NS_OVERRIDE virtual PRBool WillReplace() const;
 
   PRBool IsDisallowedAttribute(const nsIAtom* aAttribute) const;
new file mode 100644
--- /dev/null
+++ b/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>
new file mode 100644
--- /dev/null
+++ b/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>
new file mode 100644
--- /dev/null
+++ b/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>
new file mode 100644
--- /dev/null
+++ b/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>
--- a/layout/reftests/svg/smil/reftest.list
+++ b/layout/reftests/svg/smil/reftest.list
@@ -59,16 +59,21 @@ include event/reftest.list
 == anim-discrete-sum-none-1.svg    anim-standard-ref.svg
 == anim-discrete-sum-sum-1.svg     anim-standard-ref.svg
 
 == anim-discrete-to-1.svg          anim-standard-ref.svg
 == anim-discrete-to-2.svg          anim-standard-ref.svg
 == 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
 == anim-fillopacity-1xml.svg  anim-standard-ref.svg
 
 == anim-height-done-1a.svg anim-standard-ref.svg
 == anim-height-done-1b.svg anim-standard-ref.svg
 == anim-height-interp-1.svg anim-height-interp-1-ref.svg