Bug 1216843 - Part 15: Update styles when current iteration changed. r?birtles draft
authorHiroyuki Ikezoe <hiikezoe@mozilla-japan.org>
Mon, 12 Sep 2016 14:34:29 +0900
changeset 412528 bb61d30a27eadfd4592af2f5e1220b1e20c3e7e3
parent 412527 26c33ec0e44dff8b272ccc5040d0d259333ce714
child 412529 c45b76ddde7993de9bb93502f56aa74087e69fcf
push id29192
push userbmo:hiikezoe@mozilla-japan.org
push dateMon, 12 Sep 2016 05:39:50 +0000
reviewersbirtles
bugs1216843
milestone51.0a1
Bug 1216843 - Part 15: Update styles when current iteration changed. r?birtles MozReview-Commit-ID: 33JtZplxiAz
dom/animation/KeyframeEffectReadOnly.cpp
dom/animation/KeyframeEffectReadOnly.h
layout/reftests/web-animations/reftest.list
layout/reftests/web-animations/style-updates-on-current-iteration-changed.html
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -106,35 +106,32 @@ KeyframeEffectReadOnly::NotifyAnimationT
   // animation cascade for this element whenever that changes.
   bool inEffect = IsInEffect();
   if (inEffect != mInEffectOnLastAnimationTimingUpdate) {
     MarkCascadeNeedsUpdate();
     mInEffectOnLastAnimationTimingUpdate = inEffect;
   }
 
   // Request restyle if necessary.
-  //
-  // Bug 1216843: When we implement iteration composite modes, we need to
-  // also detect if the current iteration has changed.
-  if (mAnimation &&
-      !mProperties.IsEmpty() &&
-      GetComputedTiming().mProgress != mProgressOnLastCompose) {
+  if (mAnimation && !mProperties.IsEmpty() && HasComputedTimingChanged()) {
     EffectCompositor::RestyleType restyleType =
       CanThrottle() ?
       EffectCompositor::RestyleType::Throttled :
       EffectCompositor::RestyleType::Standard;
     RequestRestyle(restyleType);
   }
 
   // If we're no longer "in effect", our ComposeStyle method will never be
-  // called and we will never have a chance to update mProgressOnLastCompose.
-  // We clear mProgressOnLastCompose here to ensure that if we later become
-  // "in effect" we will request a restyle (above).
+  // called and we will never have a chance to update mProgressOnLastCompose
+  // and mCurrentIterationOnLastCompose.
+  // We clear them here to ensure that if we later become "in effect" we will
+  // request a restyle (above).
   if (!inEffect) {
      mProgressOnLastCompose.SetNull();
+     mCurrentIterationOnLastCompose = 0;
   }
 }
 
 static bool
 KeyframesEqualIgnoringComputedOffsets(const nsTArray<Keyframe>& aLhs,
                                       const nsTArray<Keyframe>& aRhs)
 {
   if (aLhs.Length() != aRhs.Length()) {
@@ -307,16 +304,17 @@ KeyframeEffectReadOnly::UpdateProperties
 }
 
 void
 KeyframeEffectReadOnly::ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule,
                                      nsCSSPropertyIDSet& aSetProperties)
 {
   ComputedTiming computedTiming = GetComputedTiming();
   mProgressOnLastCompose = computedTiming.mProgress;
+  mCurrentIterationOnLastCompose = computedTiming.mCurrentIteration;
 
   // If the progress is null, we don't have fill data for the current
   // time so we shouldn't animate.
   if (computedTiming.mProgress.IsNull()) {
     return;
   }
 
   for (size_t propIdx = 0, propEnd = mProperties.Length();
@@ -1306,10 +1304,25 @@ KeyframeEffectReadOnly::MarkCascadeNeeds
   EffectSet* effectSet = EffectSet::GetEffectSet(mTarget->mElement,
                                                  mTarget->mPseudoType);
   if (!effectSet) {
     return;
   }
   effectSet->MarkCascadeNeedsUpdate();
 }
 
+bool
+KeyframeEffectReadOnly::HasComputedTimingChanged() const
+{
+  // Typically we don't need to request a restyle if the progress hasn't
+  // changed since the last call to ComposeStyle. The one exception is if the
+  // iteration composite mode is 'accumulate' and the current iteration has
+  // changed, since that will often produce a different result.
+  ComputedTiming computedTiming = GetComputedTiming();
+  return computedTiming.mProgress != mProgressOnLastCompose ||
+         (mEffectOptions.mIterationComposite ==
+            IterationCompositeOperation::Accumulate &&
+         computedTiming.mCurrentIteration !=
+          mCurrentIterationOnLastCompose);
+}
+
 } // namespace dom
 } // namespace mozilla
--- a/dom/animation/KeyframeEffectReadOnly.h
+++ b/dom/animation/KeyframeEffectReadOnly.h
@@ -370,28 +370,37 @@ protected:
   // A set of per-property value arrays, derived from |mKeyframes|.
   nsTArray<AnimationProperty> mProperties;
 
   // The computed progress last time we composed the style rule. This is
   // used to detect when the progress is not changing (e.g. due to a step
   // timing function) so we can avoid unnecessary style updates.
   Nullable<double> mProgressOnLastCompose;
 
+  // The purpose of this value is the same as mProgressOnLastCompose but
+  // this is used to detect when the current iteration is not changing
+  // in the case when iterationComposite is accumulate.
+  uint64_t mCurrentIterationOnLastCompose = 0;
+
   // We need to track when we go to or from being "in effect" since
   // we need to re-evaluate the cascade of animations when that changes.
   bool mInEffectOnLastAnimationTimingUpdate;
 
 private:
   nsChangeHint mCumulativeChangeHint;
 
   nsIFrame* GetAnimationFrame() const;
 
   bool CanThrottle() const;
   bool CanThrottleTransformChanges(nsIFrame& aFrame) const;
 
+  // Returns true if the computedTiming has changed since the last
+  // composition.
+  bool HasComputedTimingChanged() const;
+
   // Returns true unless Gecko limitations prevent performing transform
   // animations for |aFrame|. When returning true, the reason for the
   // limitation is stored in |aOutPerformanceWarning|.
   static bool CanAnimateTransformOnCompositor(
     const nsIFrame* aFrame,
     AnimationPerformanceWarning::Type& aPerformanceWarning);
   static bool IsGeometricProperty(const nsCSSPropertyID aProperty);
 
--- a/layout/reftests/web-animations/reftest.list
+++ b/layout/reftests/web-animations/reftest.list
@@ -6,8 +6,9 @@ test-pref(dom.animations-api.core.enable
 test-pref(dom.animations-api.core.enabled,true) == stacking-context-opacity-changing-target.html stacking-context-animation-changing-target-ref.html
 test-pref(dom.animations-api.core.enabled,true) == stacking-context-opacity-changing-effect.html stacking-context-animation-ref.html
 test-pref(dom.animations-api.core.enabled,true) == stacking-context-transform-changing-keyframe.html stacking-context-animation-ref.html
 test-pref(dom.animations-api.core.enabled,true) == stacking-context-transform-changing-target.html stacking-context-animation-changing-target-ref.html
 test-pref(dom.animations-api.core.enabled,true) == stacking-context-transform-changing-effect.html stacking-context-animation-ref.html
 test-pref(dom.animations-api.core.enabled,true) == stacking-context-transform-changing-display-property.html stacking-context-animation-ref.html
 test-pref(dom.animations-api.core.enabled,true) == style-updates-on-iteration-composition-changed-from-accumulate-to-replace.html style-updates-for-iteration-composite-ref.html
 test-pref(dom.animations-api.core.enabled,true) == style-updates-on-iteration-composition-changed-from-replace-to-accumulate.html style-updates-for-iteration-composite-ref.html
+test-pref(dom.animations-api.core.enabled,true) == style-updates-on-current-iteration-changed.html style-updates-for-iteration-composite-ref.html
new file mode 100644
--- /dev/null
+++ b/layout/reftests/web-animations/style-updates-on-current-iteration-changed.html
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<title>Update styles when current iteration changed</title>
+<script src="animation-utils.js"></script>
+<style>
+#test {
+  width: 100px; height: 100px;
+  background: blue;
+}
+</style>
+<div id="test"></div>
+<script>
+  var anim = document.getElementById("test")
+    .animate({ marginLeft: [ "0px", "100px" ] },
+             { duration: 100000,
+               delay: -99999, // For starting right before second iteration.
+               easing: "steps(1, start)",
+               iterations: 2,
+               iterationComposite: "accumulate" });
+
+  waitForIterationChange(anim).then(() => {
+    // Wait for painting the result of the second iteration.
+    requestAnimationFrame(() => {
+      document.documentElement.classList.remove("reftest-wait");
+    });
+  });
+</script>