Bug 755603 - Remove broken frozen to-animation behavior; r=dholbert
authorBrian Birtles <birtles@gmail.com>
Mon, 21 May 2012 08:48:38 +0900
changeset 94468 b5352f7337b15c7456cc1eafea7d2b5754360082
parent 94467 022dfc84028c87dda82496c82f471ec059082315
child 94474 1158503601be5ca5f0591732c96dde073b632cc4
child 94483 4cbae0f1285c50e5921bf80a4e47b26bcd5bfde8
push id22719
push useremorley@mozilla.com
push dateMon, 21 May 2012 06:31:45 +0000
treeherdermozilla-central@b5352f7337b1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs755603
milestone15.0a1
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 755603 - Remove broken frozen to-animation behavior; r=dholbert
content/smil/nsSMILAnimationFunction.cpp
content/smil/nsSMILAnimationFunction.h
layout/reftests/svg/smil/frozen-to-anim-1.svg
layout/reftests/svg/smil/reftest.list
--- a/content/smil/nsSMILAnimationFunction.cpp
+++ b/content/smil/nsSMILAnimationFunction.cpp
@@ -210,26 +210,24 @@ nsSMILAnimationFunction::SampleLastValue
 }
 
 void
 nsSMILAnimationFunction::Activate(nsSMILTime aBeginTime)
 {
   mBeginTime = aBeginTime;
   mIsActive = true;
   mIsFrozen = false;
-  mFrozenValue = nsSMILValue();
   mHasChanged = true;
 }
 
 void
 nsSMILAnimationFunction::Inactivate(bool aIsFrozen)
 {
   mIsActive = false;
   mIsFrozen = aIsFrozen;
-  mFrozenValue = nsSMILValue();
   mHasChanged = true;
 }
 
 void
 nsSMILAnimationFunction::ComposeResult(const nsISMILAttr& aSMILAttr,
                                        nsSMILValue& aResult)
 {
   mHasChanged = false;
@@ -281,33 +279,24 @@ nsSMILAnimationFunction::ComposeResult(c
 
     // See comment in AccumulateResult: to-animation does not accumulate
     if (!IsToAnimation() && GetAccumulate() && mRepeatIteration) {
       // If the target attribute type doesn't support addition Add will
       // fail leaving result = last
       result.Add(last, mRepeatIteration);
     }
 
-  } else if (!mFrozenValue.IsNull() && !mHasChanged) {
-
-    // Frozen to animation
-    result = mFrozenValue;
-
   } else {
 
     // Interpolation
     if (NS_FAILED(InterpolateResult(values, result, aResult)))
       return;
 
     if (NS_FAILED(AccumulateResult(values, result)))
       return;
-
-    if (IsToAnimation() && mIsFrozen) {
-      mFrozenValue = result;
-    }
   }
 
   // If additive animation isn't required or isn't supported, set the value.
   if (!isAdditive || NS_FAILED(aResult.SandwichAdd(result))) {
     aResult.Swap(result);
     // Note: The old value of aResult is now in |result|, and it will get
     // cleaned up when |result| goes out of scope, when this function returns.
   }
@@ -354,32 +343,32 @@ nsSMILAnimationFunction::CompareTo(const
           ? -1 : 1;
 }
 
 bool
 nsSMILAnimationFunction::WillReplace() const
 {
   /*
    * In IsAdditive() we don't consider to-animation to be additive as it is
-   * a special case that is dealt with differently in the compositing method but
-   * here we return false for to animation as it builds on the underlying value
-   * unless its a frozen to animation.
+   * a special case that is dealt with differently in the compositing method.
+   * Here, however, we return FALSE for to-animation (i.e. it will NOT replace
+   * the underlying value) as it builds on the underlying value.
    */
-  return !mErrorFlags && (!(IsAdditive() || IsToAnimation()) ||
-                          (IsToAnimation() && mIsFrozen && !mHasChanged));
+  return !mErrorFlags && !(IsAdditive() || IsToAnimation());
 }
 
 bool
 nsSMILAnimationFunction::HasChanged() const
 {
   return mHasChanged || mValueNeedsReparsingEverySample;
 }
 
 bool
-nsSMILAnimationFunction::UpdateCachedTarget(const nsSMILTargetIdentifier& aNewTarget)
+nsSMILAnimationFunction::UpdateCachedTarget(
+  const nsSMILTargetIdentifier& aNewTarget)
 {
   if (!mLastTarget.Equals(aNewTarget)) {
     mLastTarget = aNewTarget;
     return true;
   }
   return false;
 }
 
@@ -509,18 +498,17 @@ nsSMILAnimationFunction::InterpolateResu
   }
   return rv;
 }
 
 nsresult
 nsSMILAnimationFunction::AccumulateResult(const nsSMILValueArray& aValues,
                                           nsSMILValue& aResult)
 {
-  if (!IsToAnimation() && GetAccumulate() && mRepeatIteration)
-  {
+  if (!IsToAnimation() && GetAccumulate() && mRepeatIteration) {
     const nsSMILValue& lastValue = aValues[aValues.Length() - 1];
 
     // If the target attribute type doesn't support addition, Add will
     // fail and we leave aResult untouched.
     aResult.Add(lastValue, mRepeatIteration);
   }
 
   return NS_OK;
@@ -814,17 +802,17 @@ nsSMILAnimationFunction::GetValues(const
     bool parseOk = true;
     nsSMILValue to, from, by;
     parseOk &= ParseAttr(nsGkAtoms::to,   aSMILAttr, to,
                          preventCachingOfSandwich);
     parseOk &= ParseAttr(nsGkAtoms::from, aSMILAttr, from,
                          preventCachingOfSandwich);
     parseOk &= ParseAttr(nsGkAtoms::by,   aSMILAttr, by,
                          preventCachingOfSandwich);
-    
+
     if (preventCachingOfSandwich) {
       mValueNeedsReparsingEverySample = true;
     }
 
     if (!parseOk)
       return NS_ERROR_FAILURE;
 
     result.SetCapacity(2);
--- a/content/smil/nsSMILAnimationFunction.h
+++ b/content/smil/nsSMILAnimationFunction.h
@@ -448,41 +448,16 @@ protected:
   // its owning animation element.
   nsISMILAnimationElement*      mAnimationElement;
 
   // Which attributes have been set but have had errors. This is not used for
   // all attributes but only those which have specified error behaviour
   // associated with them.
   PRUint16                      mErrorFlags;
 
-  // This is for the very specific case where we have a 'to' animation that is
-  // frozen part way through the simple duration and there are other active
-  // lower-priority animations targetting the same attribute. In this case
-  // SMILANIM 3.3.6 says:
-  //
-  //   The value for F(t) when a to-animation is frozen (at the end of the
-  //   simple duration) is just the to value. If a to-animation is frozen
-  //   anywhere within the simple duration (e.g., using a repeatCount of "2.5"),
-  //   the value for F(t) when the animation is frozen is the value computed for
-  //   the end of the active duration. Even if other, lower priority animations
-  //   are active while a to-animation is frozen, the value for F(t) does not
-  //   change.
-  //
-  // To implement this properly we'd need to force a resample of all the lower
-  // priority animations at the active end of this animation--something which
-  // would introduce unwanted coupling between the timing and animation model.
-  // Instead we just save the value calculated when this animation is frozen (in
-  // which case this animation will be sampled at the active end and the lower
-  // priority animations should be sampled at a time pretty close to this,
-  // provided we have a reasonable frame rate and we aren't seeking).
-  //
-  // @see
-  // http://www.w3.org/TR/2001/REC-smil-animation-20010904/#FromToByAndAdditive
-  nsSMILValue                   mFrozenValue;
-
   // Allows us to check whether an animation function has changed target from
   // sample to sample (because if neither target nor animated value have
   // changed, we don't have to do anything).
   nsSMILWeakTargetIdentifier    mLastTarget;
 
   // Boolean flags
   bool mIsActive:1;
   bool mIsFrozen:1;
new file mode 100644
--- /dev/null
+++ b/layout/reftests/svg/smil/frozen-to-anim-1.svg
@@ -0,0 +1,33 @@
+<?xml version="1.0" encoding="utf-8"?>
+<svg xmlns="http://www.w3.org/2000/svg"
+     xmlns:xlink="http://www.w3.org/1999/xlink"
+     class="reftest-wait"
+     onload="setTimeAndSnapshot(50, true)">
+  <script xlink:href="smil-util.js" type="text/javascript"/>
+  <!-- Covered up red area, if we succeed -->
+  <rect width="100%" height="100%" fill="red"/>
+  <rect width="100%" height="100%" fill="lime">
+    <!-- At time t=50s, this will give a base value of 50% -->
+    <animate attributeName="width" from="0%" to="100%"
+      dur="100s"/>
+    <!-- From time t=1s onwards this should apply 50% of its effect to the base
+         value (since it is frozen at repeatCount=0.5).
+         So, if the base value is 50%, and the to-value is 150%, it will produce
+         100%. -->
+    <animate attributeName="width" to="150%" fill="freeze"
+      dur="2s" repeatCount="0.5"/>
+    <!-- The above will check that the resulting width is *at least* 100%. But
+         if it's, say, 120%, we won't see any failure.
+         Therefore, we perform the reverse operation on the height to ensure we
+         don't apply a value that's too large. -->
+    <!-- At time t=50s, this will give a base value of 150% -->
+    <animate attributeName="height" from="200%" to="100%"
+      dur="100s"/>
+    <!-- From time t=1s onwards this should apply 50% of its effect to the
+         value.
+         So, if the base value is 150%, and the to-value is 50%, it will produce
+         100%. -->
+    <animate attributeName="height" to="50%" fill="freeze"
+      dur="2s" repeatCount="0.5"/>
+  </rect>
+</svg>
--- a/layout/reftests/svg/smil/reftest.list
+++ b/layout/reftests/svg/smil/reftest.list
@@ -229,16 +229,17 @@ fails == anim-pattern-attr-presence-02.s
 == anim-gradient-attr-presence-01.svg anim-gradient-attr-presence-01-ref.svg
 
 == api-sanity-1.svg lime.svg
 
 == freeze-applied-late-1.svg anim-standard-ref.svg
 == freeze-applied-late-2.svg anim-standard-ref.svg
 == freeze-applied-late-3.svg anim-standard-ref.svg
 == freeze-applied-late-4.svg anim-standard-ref.svg
+== frozen-to-anim-1.svg lime.svg
 
 == inactivate-with-active-unchanged-1.svg anim-standard-ref.svg
 == inactivate-with-active-unchanged-2.svg anim-standard-ref.svg
 
 # interaction between xml mapped attributes and their css equivalents
 == mapped-attr-vs-css-prop-1.svg lime.svg
 
 == smil-transitions-interaction-1a.svg lime.svg