Bug 1488122 - Update additive filling animations when the base style changes; r=hiro
authorBrian Birtles <birtles@gmail.com>
Thu, 06 Dec 2018 20:59:50 +0000
changeset 508791 174486c8cedba1a86ac4c7f643c7410e4aedd60f
parent 508790 716bbe3d3a1503de28839b88b6b393098158bc1e
child 508792 3c61fb185d513883a3a9c8eb510e102b0e7f5158
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershiro
bugs1488122
milestone65.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 1488122 - Update additive filling animations when the base style changes; r=hiro Differential Revision: https://phabricator.services.mozilla.com/D13934
dom/animation/KeyframeEffect.cpp
dom/animation/KeyframeEffect.h
testing/web-platform/meta/web-animations/animation-model/keyframe-effects/effect-value-context-filling.html.ini
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -319,21 +319,28 @@ bool SpecifiedKeyframeArraysAreEqual(con
 }
 #endif
 
 void KeyframeEffect::UpdateProperties(const ComputedStyle* aStyle) {
   MOZ_ASSERT(aStyle);
 
   nsTArray<AnimationProperty> properties = BuildProperties(aStyle);
 
+  bool propertiesChanged = mProperties != properties;
+
   // We need to update base styles even if any properties are not changed at all
   // since base styles might have been changed due to parent style changes, etc.
-  EnsureBaseStyles(aStyle, properties);
+  bool baseStylesChanged = false;
+  EnsureBaseStyles(aStyle, properties,
+                   !propertiesChanged ? &baseStylesChanged : nullptr);
 
-  if (mProperties == properties) {
+  if (!propertiesChanged) {
+    if (baseStylesChanged) {
+      RequestRestyle(EffectCompositor::RestyleType::Layer);
+    }
     return;
   }
 
   // Preserve the state of the mIsRunningOnCompositor flag.
   nsCSSPropertyIDSet runningOnCompositorProperties;
 
   for (const AnimationProperty& property : mProperties) {
     if (property.mIsRunningOnCompositor) {
@@ -353,21 +360,31 @@ void KeyframeEffect::UpdateProperties(co
 
   MarkCascadeNeedsUpdate();
 
   RequestRestyle(EffectCompositor::RestyleType::Layer);
 }
 
 void KeyframeEffect::EnsureBaseStyles(
     const ComputedStyle* aComputedValues,
-    const nsTArray<AnimationProperty>& aProperties) {
+    const nsTArray<AnimationProperty>& aProperties, bool* aBaseStylesChanged) {
+  if (aBaseStylesChanged != nullptr) {
+    *aBaseStylesChanged = false;
+  }
+
   if (!mTarget) {
     return;
   }
 
+  Maybe<nsRefPtrHashtable<nsUint32HashKey, RawServoAnimationValue>>
+      previousBaseStyles;
+  if (aBaseStylesChanged != nullptr) {
+    previousBaseStyles.emplace(std::move(mBaseValues));
+  }
+
   mBaseValues.Clear();
 
   nsPresContext* presContext =
       nsContentUtils::GetContextForContent(mTarget->mElement);
   // If |aProperties| is empty we're not going to dereference |presContext| so
   // we don't care if it is nullptr.
   //
   // We could just return early when |aProperties| is empty and save looking up
@@ -380,16 +397,26 @@ void KeyframeEffect::EnsureBaseStyles(
              "Typically presContext should not be nullptr but if it is"
              " we should have also failed to calculate the computed values"
              " passed-in as aProperties");
 
   RefPtr<ComputedStyle> baseComputedStyle;
   for (const AnimationProperty& property : aProperties) {
     EnsureBaseStyle(property, presContext, aComputedValues, baseComputedStyle);
   }
+
+  if (aBaseStylesChanged != nullptr) {
+    for (auto iter = mBaseValues.Iter(); !iter.Done(); iter.Next()) {
+      if (AnimationValue(iter.Data()) !=
+          AnimationValue(previousBaseStyles->Get(iter.Key()))) {
+        *aBaseStylesChanged = true;
+        break;
+      }
+    }
+  }
 }
 
 void KeyframeEffect::EnsureBaseStyle(
     const AnimationProperty& aProperty, nsPresContext* aPresContext,
     const ComputedStyle* aComputedStyle,
     RefPtr<ComputedStyle>& aBaseComputedStyle) {
   bool hasAdditiveValues = false;
 
--- a/dom/animation/KeyframeEffect.h
+++ b/dom/animation/KeyframeEffect.h
@@ -363,20 +363,18 @@ class KeyframeEffect : public AnimationE
   // recursion.
   already_AddRefed<ComputedStyle> GetTargetComputedStyle();
 
   // A wrapper for marking cascade update according to the current
   // target and its effectSet.
   void MarkCascadeNeedsUpdate();
 
   void EnsureBaseStyles(const ComputedStyle* aComputedValues,
-                        const nsTArray<AnimationProperty>& aProperties);
-
-  // Stylo version of the above function that also first checks for an additive
-  // value in |aProperty|'s list of segments.
+                        const nsTArray<AnimationProperty>& aProperties,
+                        bool* aBaseStylesChanged);
   void EnsureBaseStyle(const AnimationProperty& aProperty,
                        nsPresContext* aPresContext,
                        const ComputedStyle* aComputedValues,
                        RefPtr<ComputedStyle>& aBaseComputedValues);
 
   Maybe<OwningAnimationTarget> mTarget;
 
   KeyframeEffectParams mEffectOptions;
deleted file mode 100644
--- a/testing/web-platform/meta/web-animations/animation-model/keyframe-effects/effect-value-context-filling.html.ini
+++ /dev/null
@@ -1,13 +0,0 @@
-[effect-value-context-filling.html]
-  [Filling effect values reflect changes to the base value when using additive animation]
-    expected: FAIL
-
-  [Filling effect values reflect changes to the base value when using additive animation on a single keyframe]
-    expected: FAIL
-
-  [Filling effect values reflect changes to the base value when using the fill value is an implicit keyframe]
-    expected: FAIL
-
-  [Filling effect values reflect changes to the base value via a parent element]
-    expected: FAIL
-