Bug 1479173 - Check animation generation change in the mProperties loop and drop LayerAnimationInfo::sRecords loop. r=birtles
☠☠ backed out by 944be4a5056f ☠ ☠
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Tue, 06 Nov 2018 09:40:39 +0000
changeset 444603 439ac5cfbcedb23c1587c703af3e66cda06ed5cf
parent 444602 73aba16a223faf3c6f7e3ad3b6da27995fd31db0
child 444604 4564c0cbfbc6c14ad1b2f26873a3bcf837602498
push id34998
push userapavel@mozilla.com
push dateTue, 06 Nov 2018 17:04:36 +0000
treeherdermozilla-central@f9affb18c3e1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles
bugs1479173
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 1479173 - Check animation generation change in the mProperties loop and drop LayerAnimationInfo::sRecords loop. r=birtles If mIsRunningOnCompositor is true, the property is effective state because CanThrottle() is called in advance of a restyle for the effect so that we can drop the check and drop skipping in the case of non-effective properties. Depends on D10694 Differential Revision: https://phabricator.services.mozilla.com/D10695
dom/animation/KeyframeEffect.cpp
layout/style/LayerAnimationInfo.cpp
layout/style/LayerAnimationInfo.h
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -1237,44 +1237,43 @@ KeyframeEffect::CanThrottle() const
     // need to update on the main thread.
     return true;
   }
 
   if (CanThrottleIfNotVisible(*frame)) {
     return true;
   }
 
+  EffectSet* effectSet = nullptr;
   for (const AnimationProperty& property : mProperties) {
     if (!property.mIsRunningOnCompositor) {
       return false;
     }
-  }
 
-  EffectSet* effectSet = nullptr;
-  for (const LayerAnimationInfo::Record& record :
-        LayerAnimationInfo::sRecords) {
-    // Skip properties that are overridden by !important rules.
-    // (GetEffectiveAnimationOfProperty, as called by
-    // HasEffectiveAnimationOfProperty, only returns a property which is
-    // neither overridden by !important rules nor overridden by other
-    // animation.)
-    if (!HasEffectiveAnimationOfProperty(record.mProperty)) {
-      continue;
-    }
+    MOZ_ASSERT(nsCSSPropertyIDSet::CompositorAnimatables()
+                 .HasProperty(property.mProperty),
+               "The property should be able to run on the compositor");
+    MOZ_ASSERT(HasEffectiveAnimationOfProperty(property.mProperty),
+               "There should be an effective animation of the property while "
+               "it is marked as being run on the compositor");
 
     if (!effectSet) {
       effectSet = EffectSet::GetEffectSet(mTarget->mElement,
                                           mTarget->mPseudoType);
       MOZ_ASSERT(effectSet, "CanThrottle should be called on an effect "
                             "associated with a target element");
     }
+
+    DisplayItemType displayItemType =
+      LayerAnimationInfo::GetDisplayItemTypeForProperty(property.mProperty);
+
     // Note that AnimationInfo::GetGenarationFromFrame() is supposed to work
     // with the primary frame instead of the style frame.
     Maybe<uint64_t> generation = layers::AnimationInfo::GetGenerationFromFrame(
-        GetPrimaryFrame(), record.mDisplayItemType);
+        GetPrimaryFrame(), displayItemType);
     // Unthrottle if the animation needs to be brought up to date
     if (!generation || effectSet->GetAnimationGeneration() != *generation) {
       return false;
     }
 
     // If this is a transform animation that affects the overflow region,
     // we should unthrottle the animation periodically.
     if (HasPropertiesThatMightAffectOverflow() &&
--- a/layout/style/LayerAnimationInfo.cpp
+++ b/layout/style/LayerAnimationInfo.cpp
@@ -14,16 +14,30 @@ namespace mozilla {
 /* static */ const LayerAnimationInfo::Record LayerAnimationInfo::sRecords[] =
   { { eCSSProperty_transform,
       DisplayItemType::TYPE_TRANSFORM,
       nsChangeHint_UpdateTransformLayer },
     { eCSSProperty_opacity,
       DisplayItemType::TYPE_OPACITY,
       nsChangeHint_UpdateOpacityLayer } };
 
+/* static */ DisplayItemType
+LayerAnimationInfo::GetDisplayItemTypeForProperty(nsCSSPropertyID aProperty)
+{
+  switch (aProperty) {
+    case eCSSProperty_opacity:
+      return DisplayItemType::TYPE_OPACITY;
+    case eCSSProperty_transform:
+      return DisplayItemType::TYPE_TRANSFORM;
+    default:
+      break;
+  }
+  return DisplayItemType::TYPE_ZERO;
+}
+
 #ifdef DEBUG
 /* static */ void
 LayerAnimationInfo::Initialize()
 {
   for (const Record& record : sRecords) {
     MOZ_ASSERT(nsCSSProps::PropHasFlags(record.mProperty,
                                         CSSPropFlags::CanAnimateOnCompositor),
                "CSS property with entry in LayerAnimation::sRecords does not "
@@ -44,15 +58,19 @@ LayerAnimationInfo::Initialize()
           found = true;
           properties.AddProperty(record.mProperty);
           break;
         }
       }
       MOZ_ASSERT(found,
                  "CSS property with the CSSPropFlags::CanAnimateOnCompositor "
                  "flag does not have an entry in LayerAnimationInfo::sRecords");
+      MOZ_ASSERT(GetDisplayItemTypeForProperty(prop) !=
+                   DisplayItemType::TYPE_ZERO,
+                 "GetDisplayItemTypeForProperty should return a valid display "
+                 "item type");
     }
   }
   MOZ_ASSERT(properties.Equals(nsCSSPropertyIDSet::CompositorAnimatables()));
 }
 #endif
 
 } // namespace mozilla
--- a/layout/style/LayerAnimationInfo.h
+++ b/layout/style/LayerAnimationInfo.h
@@ -21,16 +21,23 @@ struct LayerAnimationInfo {
   // For CSS properties that may be animated on a separate layer, represents
   // a record of the corresponding layer type and change hint.
   struct Record {
     nsCSSPropertyID mProperty;
     DisplayItemType mDisplayItemType;
     nsChangeHint mChangeHint;
   };
 
+  // Returns the corresponding display item type for |aProperty| when it is
+  // animated on the compositor.
+  // Returns DisplayItemType::TYPE_ZERO if |aProperty| cannot be animated on the
+  // compositor.
+  static DisplayItemType
+  GetDisplayItemTypeForProperty(nsCSSPropertyID aProperty);
+
   static const size_t kRecords =
     nsCSSPropertyIDSet::CompositorAnimatableCount();
   static const Record sRecords[kRecords];
 };
 
 } // namespace mozilla
 
 #endif /* !defined(mozilla_LayerAnimationInfo_h) */