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 id17389
push userjwatt@jwatt.org
push dateSun, 05 Dec 2010 14:31:57 +0000
treeherdermozilla-central@20441229d0a7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, roc
bugs614879
milestone2.0b8pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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