Bug 1181392 part 5 - Remove use of IsFinishedTransition from AnimationCollection::HasAnimationOfProperty; r=dbaron
authorBrian Birtles <birtles@gmail.com>
Fri, 07 Aug 2015 12:29:36 +0900
changeset 288401 c9b2670f5c1a1fed0cef6b7fa2b98b1f3fa789be
parent 288400 f8b8e1084b4080b39a8278112aa4519cd0db0eba
child 288402 5448e9a763c47f2460ffa414ebd3e856c7e3a549
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs1181392
milestone42.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 1181392 part 5 - Remove use of IsFinishedTransition from AnimationCollection::HasAnimationOfProperty; r=dbaron AnimationCollection::HasAnimationOfProperty uses IsFinishedTransition to filter out transitions that should otherwise be ignored. This is used in the following places: 1. nsLayoutUtils::HasAnimations The is only used by nsIFrame::BuildDisplayListForStackingContext to see if there are any opacity animations For this case, simply returning *current* animations would be sufficient (since finished but filling animations should have already filled in the display opacity) 2. CommonAnimationManager::GetAnimationsForCompositor This should really only return *current* animations--that is, animations that are running or scheduled to run. Finished animations never run on the compositor. Indeed, only *playing* animations run on the compositor but, as we will see in some of the cases below, it is sometimes useful to know that an animation *will* run on the compositor in the near future (e.g. so we can pre-render content). The places where GetAnimationsForCompositor is used are: - When building layers to add animations to layers in nsDisplayList--in this case we skip any animations that aren't playing so if GetAnimationsForCompositor only returned current animations that would be more than sufficient. - In nsLayoutUtils::HasAnimationsForCompositor. This in turn is used: - In ChooseScaleAndSetTransform to see if the transform is being animated on the compositor. If so, it calls nsLayoutUtils::ComputeSuitableScaleForAnimation (which also calls GetAnimationsForCompositor) and passes the result to GetMinAndMaxScaleForAnimationProperty which we have already adjusted in part 4 of this patch series to only deal with *relevant* animations Relevant animations include both current animations and in effect animations but we don't run forwards-filling animations on the compositor so GetAnimationsForCompositor should NOT return them. Current animations should be enough. In fact, playing animations should be enough but we might want to pre-render layers at a suitable size during their delay phase so returning current animations is probably ok. - In nsDisplayListBuilder::MarkOutOfFlowFrameForDisplay to add a fuzz factor to the overflow rect for frames undergoing a transform animation on the compositor. In this case too current animations should be sufficient. - In nsDisplayOpacity::NeedsActiveLayer to say "yes" if we are animating opacity on the compositor. Presumably in this case it would be good to say "yes" if the animation is in the delay phase too (as it currently does). After the animation is finished, we should drop the layer, i.e. current animations should be sufficient. - In nsDisplayTransform::ShouldPrerenderTransformedContent. As with nsDisplayOpacity::NeedsActiveLayer, we only need to pre-render transformed content for animations that are current. - In nsDisplayTransform::GetLayerState. As with nsDisplayOpacity::NeedsActiveLayer, we only need to return active here for current animations. - In nsIFrame::IsTransformed. Here we test the display style to see if there is a transform and also check if transform is being animated on the compositor. As a result, we really only need HasAnimationsForCompositor to return true for animations that are playing--otherwise the display style will tell us if we're transformed or not. Returning true for all current compositor animations (which is a superset of playing), however, should not cause problems (we already return true for even more than that). - In nsIFrame::HasOpacityInternal which is much the same as nsIFrame::IsTransformed and hence current should be fine. 3. AnimationCollection::CanThrottleAnimation Here, HasAnimationOfProperty is used when looking for animations that would disqualify us from throttling the animation by having an out-of-date layer generation or being a transform animation that affects scroll and so requires that we do the occasional main thread sample to update scrollbars. It would seem like current animations are enough here too. One interesting case is where we *had* a compositor animation but it has finished or been cancelled. In that case, the animation won't be current and we should not throttle the animation since we need to take it off its layer. It turns out checking for current animations is still ok in this case too. The reasoning is as follows: - If the animation is newly-finished, we'll pick that up in Animation::CanThrottle and return false then. - If the animation is newly-idle then there are two cases: If the cancelled animation was the only compositor animation then AnimationCollection::CanPerformOnCompositorThread will notice that there are no playing compositor animations and return false and AnimationCollection::CanThrottleAnimation will never be called. If there are other compositor animations running, then AnimationCollection::CanThrottleAnimation will still return false because whatever cancelled the animation will update the animation generation and we'll notice the mismatch between the layer animation generation and the animation generation on the collection. Based on the above analysis it appears that making AnimationCollection::HasAnimationOfProperty return only current animations (and simulatneously renaming it to HasCurrentAnimationOfProperty) is safe. Indeed, in effect, we already do this for transitions but not for animations. This patch generalizes this behavior to all animations. This patch also updates test_animations_omta.html since it was incorrectly testing that a finished opacity animation was still running on the compositor. Finished animations should not run on the compositor and the changes in this patch cause that to happen. The reason we don't just update this test to check for RunningOn.MainThread is that for opacity animations, unlike transform animations, we can't detect if an opacity on a layer was set by animation or not. As a result, for opacity animations we typically test the opacity on either the main thread or compositor in order to allow for the case where an animation-set opacity is still lingering on the compositor.
layout/base/nsLayoutUtils.cpp
layout/base/nsLayoutUtils.h
layout/generic/nsFrame.cpp
layout/style/AnimationCommon.cpp
layout/style/AnimationCommon.h
layout/style/test/test_animations_omta.html
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -376,30 +376,30 @@ nsLayoutUtils::HasAnimationsForComposito
                                           nsCSSProperty aProperty)
 {
   nsPresContext* presContext = aFrame->PresContext();
   return presContext->AnimationManager()->GetAnimationsForCompositor(aFrame, aProperty) ||
          presContext->TransitionManager()->GetAnimationsForCompositor(aFrame, aProperty);
 }
 
 bool
-nsLayoutUtils::HasAnimations(const nsIFrame* aFrame,
-                             nsCSSProperty aProperty)
+nsLayoutUtils::HasCurrentAnimationOfProperty(const nsIFrame* aFrame,
+                                             nsCSSProperty aProperty)
 {
   nsPresContext* presContext = aFrame->PresContext();
   AnimationCollection* collection =
     presContext->AnimationManager()->GetAnimationCollection(aFrame);
   if (collection &&
-      collection->HasAnimationOfProperty(aProperty)) {
+      collection->HasCurrentAnimationOfProperty(aProperty)) {
     return true;
   }
   collection =
     presContext->TransitionManager()->GetAnimationCollection(aFrame);
   if (collection &&
-      collection->HasAnimationOfProperty(aProperty)) {
+      collection->HasCurrentAnimationOfProperty(aProperty)) {
     return true;
   }
   return false;
 }
 
 bool
 nsLayoutUtils::HasCurrentAnimations(const nsIFrame* aFrame)
 {
--- a/layout/base/nsLayoutUtils.h
+++ b/layout/base/nsLayoutUtils.h
@@ -2155,20 +2155,21 @@ public:
   /**
    * Returns true if the frame has animations or transitions that can be
    * performed on the compositor.
    */
   static bool HasAnimationsForCompositor(const nsIFrame* aFrame,
                                          nsCSSProperty aProperty);
 
   /**
-   * Returns true if the frame has animations or transitions for the
-   * property.
+   * Returns true if the frame has current (i.e. running or scheduled-to-run)
+   * animations or transitions for the property.
    */
-  static bool HasAnimations(const nsIFrame* aFrame, nsCSSProperty aProperty);
+  static bool HasCurrentAnimationOfProperty(const nsIFrame* aFrame,
+                                            nsCSSProperty aProperty);
 
   /**
    * Returns true if the frame has any current animations.
    * A current animation is any animation that has not yet finished playing
    * including paused animations.
    */
   static bool HasCurrentAnimations(const nsIFrame* aFrame);
 
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -1941,17 +1941,18 @@ nsIFrame::BuildDisplayListForStackingCon
   // we're painting, and we're not animating opacity. Don't do this
   // if we're going to compute plugin geometry, since opacity-0 plugins
   // need to have display items built for them.
   bool needEventRegions = aBuilder->IsBuildingLayerEventRegions() &&
       StyleVisibility()->GetEffectivePointerEvents(this) != NS_STYLE_POINTER_EVENTS_NONE;
   if (disp->mOpacity == 0.0 && aBuilder->IsForPainting() &&
       !aBuilder->WillComputePluginGeometry() &&
       !(disp->mWillChangeBitField & NS_STYLE_WILL_CHANGE_OPACITY) &&
-      !nsLayoutUtils::HasAnimations(this, eCSSProperty_opacity) &&
+      !nsLayoutUtils::HasCurrentAnimationOfProperty(this,
+                                                    eCSSProperty_opacity) &&
       !needEventRegions) {
     return;
   }
 
   if (disp->mWillChangeBitField != 0) {
     aBuilder->AddToWillChangeBudget(this, GetSize());
   }
 
--- a/layout/style/AnimationCommon.cpp
+++ b/layout/style/AnimationCommon.cpp
@@ -168,17 +168,17 @@ CommonAnimationManager::GetAnimationColl
 }
 
 AnimationCollection*
 CommonAnimationManager::GetAnimationsForCompositor(const nsIFrame* aFrame,
                                                    nsCSSProperty aProperty)
 {
   AnimationCollection* collection = GetAnimationCollection(aFrame);
   if (!collection ||
-      !collection->HasAnimationOfProperty(aProperty) ||
+      !collection->HasCurrentAnimationOfProperty(aProperty) ||
       !collection->CanPerformOnCompositorThread(
         AnimationCollection::CanAnimate_AllowPartial)) {
     return nullptr;
   }
 
   // This animation can be done on the compositor.
   return collection;
 }
@@ -703,22 +703,22 @@ AnimationCollection::PostUpdateLayerAnim
             PostRestyleEvent(element, nsRestyleHint(0), changeHint);
         }
       }
     }
   }
 }
 
 bool
-AnimationCollection::HasAnimationOfProperty(nsCSSProperty aProperty) const
+AnimationCollection::HasCurrentAnimationOfProperty(nsCSSProperty
+                                                     aProperty) const
 {
-  for (size_t animIdx = mAnimations.Length(); animIdx-- != 0; ) {
-    const KeyframeEffectReadOnly* effect = mAnimations[animIdx]->GetEffect();
-    if (effect && effect->HasAnimationOfProperty(aProperty) &&
-        !effect->IsFinishedTransition()) {
+  for (Animation* animation : mAnimations) {
+    if (animation->HasCurrentEffect() &&
+        animation->GetEffect()->HasAnimationOfProperty(aProperty)) {
       return true;
     }
   }
   return false;
 }
 
 /*static*/ nsString
 AnimationCollection::PseudoTypeAsString(nsCSSPseudoElements::Type aPseudoType)
@@ -929,21 +929,28 @@ AnimationCollection::CanThrottleAnimatio
   if (!element) {
     return false;
   }
   nsIFrame* frame = nsLayoutUtils::GetStyleFrame(element);
   if (!frame) {
     return false;
   }
 
-
   const auto& info = CommonAnimationManager::sLayerAnimationInfo;
   for (size_t i = 0; i < ArrayLength(info); i++) {
     auto record = info[i];
-    if (!HasAnimationOfProperty(record.mProperty)) {
+    // We only need to worry about *current* animations here.
+    // - If we have a newly-finished animation, Animation::CanThrottle will
+    //   detect that and force an unthrottled sample.
+    // - If we have a newly-idle animation, then whatever caused the animation
+    //   to be idle will update the animation generation so we'll return false
+    //   from the layer generation check below for any other running compositor
+    //   animations (and if no other compositor animations exist we won't get
+    //   this far).
+    if (!HasCurrentAnimationOfProperty(record.mProperty)) {
       continue;
     }
 
     Layer* layer = FrameLayerBuilder::GetDedicatedLayer(
                      frame, record.mLayerType);
     if (!layer || mAnimationGeneration > layer->GetAnimationGeneration()) {
       return false;
     }
--- a/layout/style/AnimationCommon.h
+++ b/layout/style/AnimationCommon.h
@@ -315,17 +315,17 @@ public:
   //
   // Note that this does not test whether the element's layer uses
   // off-main-thread compositing, although it does check whether
   // off-main-thread compositing is enabled as a whole.
   bool CanPerformOnCompositorThread(CanAnimateFlags aFlags) const;
 
   void PostUpdateLayerAnimations();
 
-  bool HasAnimationOfProperty(nsCSSProperty aProperty) const;
+  bool HasCurrentAnimationOfProperty(nsCSSProperty aProperty) const;
 
   bool IsForElement() const { // rather than for a pseudo-element
     return mElementProperty == nsGkAtoms::animationsProperty ||
            mElementProperty == nsGkAtoms::transitionsProperty;
   }
 
   bool IsForBeforePseudo() const {
     return mElementProperty == nsGkAtoms::animationsOfBeforeProperty ||
--- a/layout/style/test/test_animations_omta.html
+++ b/layout/style/test/test_animations_omta.html
@@ -946,17 +946,17 @@ addAsyncAnimTest(function *() {
   omta_is_approx("transform", { ty: 100 * gTF.ease_out(0.81) }, 0.01,
                  RunningOn.Compositor,
                  "animation-iteration-count test 3 at 23.8s");
   advance_clock(200);
   omta_is_approx("transform", { ty: 100 * gTF.ease_out(0.8) }, 0.01,
                  RunningOn.Compositor,
                  "animation-iteration-count test 3 at 24s");
   advance_clock(200);
-  omta_is("opacity", 1, RunningOn.Compositor,
+  omta_is("opacity", 1, RunningOn.Either,
           "animation-iteration-count test 2 at 25s");
   omta_is_approx("transform", { ty: 100 * gTF.ease_out(0.8) }, 0.01,
                  RunningOn.Compositor,
                  "animation-iteration-count test 3 at 25s");
   done_div();
 
   new_div("animation: anim4 ease-in-out 5s 1.6 forwards");
   yield waitForPaintsFlushed();