Add DeferredPaintObserver to ScopedLayerAnimationSettings.

We want to defer layer painting during some UI animations so that we can avoid resterization at the same time and we can cache the render suface of the animating layer. Adding an observer to ScopedLayerAnimationSettings can AddDeferredPaintRequest and RemoveDeferredPaintRequest at the beginning and end of the animation. Bug: 761109 Test: LayerAnimatorTest.DeferredPaint* Change-Id: If5e7afb7d2564f5ff480438447eabd10fca799ab Reviewed-on: https://chromium-review.googlesource.com/685493 Commit-Queue: Tao Wu <wutao@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Reviewed-by: David Reveman <reveman@chromium.org> Cr-Commit-Position: refs/heads/master@{#506253}
hylschwy · Oct 4, 2017 · 7ba7cc8 · 7ba7cc8
1 parent 5d72d54
commit 7ba7cc8
Show file tree

Hide file tree

Showing 6 changed files with 308 additions and 7 deletions.
diff --git a/ui/compositor/layer.h b/ui/compositor/layer.h
@@ -435,6 +435,8 @@ class COMPOSITOR_EXPORT Layer : public LayerAnimationDelegate,
   void AddDeferredPaintRequest();
   void RemoveDeferredPaintRequest();
 
+  bool IsPaintDeferredForTesting() const { return deferred_paint_requests_; }
+
   // Request trilinear filtering for layer.
   void SetTrilinearFiltering(bool trilinear_filtering);
 

diff --git a/ui/compositor/layer_animator.cc b/ui/compositor/layer_animator.cc
@@ -370,6 +370,23 @@ void LayerAnimator::RemoveObserver(LayerAnimationObserver* observer) {
   }
 }
 
+void LayerAnimator::AddOwnedObserver(
+    std::unique_ptr<ImplicitAnimationObserver> animation_observer) {
+  owned_observer_list_.push_back(std::move(animation_observer));
+}
+
+void LayerAnimator::RemoveAndDestroyOwnedObserver(
+    ImplicitAnimationObserver* animation_observer) {
+  owned_observer_list_.erase(
+      std::remove_if(
+          owned_observer_list_.begin(), owned_observer_list_.end(),
+          [animation_observer](
+              const std::unique_ptr<ImplicitAnimationObserver>& other) {
+            return other.get() == animation_observer;
+          }),
+      owned_observer_list_.end());
+}
+
 void LayerAnimator::OnThreadedAnimationStarted(
     base::TimeTicks monotonic_time,
     cc::TargetProperty::Type target_property,

diff --git a/ui/compositor/layer_animator.h b/ui/compositor/layer_animator.h
@@ -37,6 +37,7 @@ class Transform;
 
 namespace ui {
 class Compositor;
+class ImplicitAnimationObserver;
 class Layer;
 class LayerAnimationSequence;
 class LayerAnimationDelegate;
@@ -196,6 +197,11 @@ class COMPOSITOR_EXPORT LayerAnimator : public base::RefCounted<LayerAnimator>,
   void AddObserver(LayerAnimationObserver* observer);
   void RemoveObserver(LayerAnimationObserver* observer);
 
+  void AddOwnedObserver(
+      std::unique_ptr<ImplicitAnimationObserver> animation_observer);
+  void RemoveAndDestroyOwnedObserver(
+      ImplicitAnimationObserver* animation_observer);
+
   // Called when a threaded animation is actually started.
   void OnThreadedAnimationStarted(base::TimeTicks monotonic_time,
                                   cc::TargetProperty::Type target_property,
@@ -416,6 +422,8 @@ class COMPOSITOR_EXPORT LayerAnimator : public base::RefCounted<LayerAnimator>,
   // aborted.
   base::ObserverList<LayerAnimationObserver> observers_;
 
+  std::vector<std::unique_ptr<ImplicitAnimationObserver>> owned_observer_list_;
+
   DISALLOW_COPY_AND_ASSIGN(LayerAnimator);
 };
 

diff --git a/ui/compositor/layer_animator_unittest.cc b/ui/compositor/layer_animator_unittest.cc
@@ -1727,7 +1727,7 @@ TEST(LayerAnimatorTest, CacheRenderSurface) {
 // crash when the layer was destroyed.
 TEST(LayerAnimatorTest, CacheRenderSurfaceOnWillBeDestroyedLayer) {
   // Case 1: layer is a pointer.
-  ui::Layer* layer1 = new Layer();
+  auto layer1 = base::MakeUnique<Layer>();
   scoped_refptr<LayerAnimator> animator(layer1->GetAnimator());
   animator->set_disable_timer_for_test(true);
   TestImplicitAnimationObserver observer1(false);
@@ -1743,8 +1743,7 @@ TEST(LayerAnimatorTest, CacheRenderSurfaceOnWillBeDestroyedLayer) {
   }
 
   EXPECT_FALSE(observer1.animations_completed());
-  delete layer1;
-  layer1 = nullptr;
+  layer1.release();
   animator->StopAnimatingProperty(LayerAnimationElement::OPACITY);
   EXPECT_TRUE(observer1.animations_completed());
 
@@ -1866,6 +1865,223 @@ TEST(LayerAnimatorTest, CacheRenderSurfaceInTwoAnimations) {
   }
 }
 
+// Tests that deferred painting request added to a scoped settings object is
+// still reset when the object goes out of scope.
+TEST(LayerAnimatorTest, DeferredPaint) {
+  ui::Layer layer;
+  scoped_refptr<LayerAnimator> animator(layer.GetAnimator());
+  animator->set_disable_timer_for_test(true);
+  TestImplicitAnimationObserver observer(false);
+
+  EXPECT_FALSE(observer.animations_completed());
+  EXPECT_FALSE(layer.IsPaintDeferredForTesting());
+  animator->SetOpacity(1.0f);
+
+  {
+    ScopedLayerAnimationSettings settings(animator.get());
+    settings.DeferPaint();
+    settings.AddObserver(&observer);
+    animator->SetOpacity(0.0f);
+  }
+
+  EXPECT_FALSE(observer.animations_completed());
+  EXPECT_TRUE(layer.IsPaintDeferredForTesting());
+  animator->StopAnimatingProperty(LayerAnimationElement::OPACITY);
+  EXPECT_TRUE(observer.animations_completed());
+  EXPECT_TRUE(observer.WasAnimationCompletedForProperty(
+      LayerAnimationElement::OPACITY));
+  EXPECT_FLOAT_EQ(0.0f, layer.opacity());
+  EXPECT_FALSE(layer.IsPaintDeferredForTesting());
+}
+
+// Tests that deferred painting request is added to child layers and will be
+// removed even the child layer was reparented.
+TEST(LayerAnimatorTest, DeferredPaintOnChildLayer) {
+  ui::Layer layer;
+  ui::Layer child_layer1;
+  ui::Layer child_layer2;
+  layer.Add(&child_layer1);
+  layer.Add(&child_layer2);
+
+  scoped_refptr<LayerAnimator> animator(layer.GetAnimator());
+  animator->set_disable_timer_for_test(true);
+  TestImplicitAnimationObserver observer(false);
+
+  EXPECT_FALSE(observer.animations_completed());
+  EXPECT_FALSE(layer.IsPaintDeferredForTesting());
+  EXPECT_FALSE(child_layer1.IsPaintDeferredForTesting());
+  EXPECT_FALSE(child_layer2.IsPaintDeferredForTesting());
+  animator->SetOpacity(1.0f);
+
+  {
+    ScopedLayerAnimationSettings settings(animator.get());
+    settings.DeferPaint();
+    settings.AddObserver(&observer);
+    animator->SetOpacity(0.0f);
+  }
+
+  EXPECT_FALSE(observer.animations_completed());
+  EXPECT_TRUE(layer.IsPaintDeferredForTesting());
+  EXPECT_TRUE(child_layer1.IsPaintDeferredForTesting());
+  EXPECT_TRUE(child_layer2.IsPaintDeferredForTesting());
+
+  // Reparent child_layer2.
+  ui::Layer new_parent_layer;
+  new_parent_layer.Add(&child_layer2);
+  EXPECT_TRUE(child_layer2.IsPaintDeferredForTesting());
+
+  animator->StopAnimatingProperty(LayerAnimationElement::OPACITY);
+  EXPECT_TRUE(observer.animations_completed());
+  EXPECT_TRUE(observer.WasAnimationCompletedForProperty(
+      LayerAnimationElement::OPACITY));
+  EXPECT_FLOAT_EQ(0.0f, layer.opacity());
+  EXPECT_FALSE(layer.IsPaintDeferredForTesting());
+  EXPECT_FALSE(child_layer1.IsPaintDeferredForTesting());
+  EXPECT_FALSE(child_layer2.IsPaintDeferredForTesting());
+}
+
+// Tests that deffered paint request added to two scoped settings objects is
+// still reset when animation finishes.
+TEST(LayerAnimatorTest, DeferredPaintInTwoAnimations) {
+  ui::Layer layer;
+  scoped_refptr<LayerAnimator> animator(layer.GetAnimator());
+  animator->set_disable_timer_for_test(true);
+
+  // Case 1: the original deferred paint status if false.
+  EXPECT_FALSE(layer.IsPaintDeferredForTesting());
+  animator->SetBrightness(1.0f);
+  animator->SetOpacity(1.0f);
+
+  // Start the brightness animation.
+  {
+    ScopedLayerAnimationSettings settings(animator.get());
+    settings.DeferPaint();
+    animator->SetBrightness(0.0f);
+  }
+  EXPECT_TRUE(layer.IsPaintDeferredForTesting());
+
+  // Start the opacity animation.
+  {
+    ScopedLayerAnimationSettings settings(animator.get());
+    settings.DeferPaint();
+    animator->SetOpacity(0.0f);
+  }
+  EXPECT_TRUE(layer.IsPaintDeferredForTesting());
+
+  // Finish the brightness animation.
+  {
+    animator->StopAnimatingProperty(LayerAnimationElement::BRIGHTNESS);
+    EXPECT_FLOAT_EQ(0.0f, layer.layer_brightness());
+    EXPECT_TRUE(layer.IsPaintDeferredForTesting());
+  }
+
+  // Finish the opacity animation.
+  {
+    animator->StopAnimatingProperty(LayerAnimationElement::OPACITY);
+    EXPECT_FLOAT_EQ(0.0f, layer.opacity());
+    EXPECT_FALSE(layer.IsPaintDeferredForTesting());
+  }
+
+  // Case 2: the original cache status if true.
+  layer.AddDeferredPaintRequest();
+  EXPECT_TRUE(layer.IsPaintDeferredForTesting());
+  animator->SetBrightness(1.0f);
+  animator->SetOpacity(1.0f);
+
+  // Start the brightness animation.
+  {
+    ScopedLayerAnimationSettings settings(animator.get());
+    settings.DeferPaint();
+    animator->SetBrightness(0.0f);
+  }
+  EXPECT_TRUE(layer.IsPaintDeferredForTesting());
+
+  // Start the opacity animation.
+  {
+    ScopedLayerAnimationSettings settings(animator.get());
+    settings.DeferPaint();
+    animator->SetOpacity(0.0f);
+  }
+  EXPECT_TRUE(layer.IsPaintDeferredForTesting());
+
+  // Finish the brightness animation.
+  {
+    animator->StopAnimatingProperty(LayerAnimationElement::BRIGHTNESS);
+    EXPECT_FLOAT_EQ(0.0f, layer.layer_brightness());
+    EXPECT_TRUE(layer.IsPaintDeferredForTesting());
+  }
+
+  // Finish the opacity animation.
+  {
+    animator->StopAnimatingProperty(LayerAnimationElement::OPACITY);
+    EXPECT_FLOAT_EQ(0.0f, layer.opacity());
+    EXPECT_TRUE(layer.IsPaintDeferredForTesting());
+  }
+}
+
+// Tests that deferred paint request added to a scoped settings object will
+// not crash when the layer was destroyed.
+TEST(LayerAnimatorTest, DeferredPaintOnWillBeDestroyedLayer) {
+  // Case 1: layer is a pointer.
+  auto layer1 = base::MakeUnique<Layer>();
+  scoped_refptr<LayerAnimator> animator(layer1->GetAnimator());
+  animator->set_disable_timer_for_test(true);
+  TestImplicitAnimationObserver observer1(false);
+
+  EXPECT_FALSE(observer1.animations_completed());
+  animator->SetOpacity(1.0f);
+
+  {
+    ScopedLayerAnimationSettings settings(animator.get());
+    settings.DeferPaint();
+    settings.AddObserver(&observer1);
+    animator->SetOpacity(0.0f);
+  }
+
+  EXPECT_FALSE(observer1.animations_completed());
+  layer1.release();
+  animator->StopAnimatingProperty(LayerAnimationElement::OPACITY);
+  EXPECT_TRUE(observer1.animations_completed());
+
+  // Case 2: layer is a local variable.
+  TestImplicitAnimationObserver observer2(false);
+  {
+    ui::Layer layer2;
+    animator = layer2.GetAnimator();
+
+    EXPECT_FALSE(observer2.animations_completed());
+    animator->SetBrightness(1.0f);
+
+    ScopedLayerAnimationSettings settings(animator.get());
+    settings.DeferPaint();
+    settings.AddObserver(&observer2);
+    animator->SetBrightness(0.0f);
+  }
+
+  EXPECT_FALSE(observer2.animations_completed());
+  animator->StopAnimatingProperty(LayerAnimationElement::BRIGHTNESS);
+  EXPECT_TRUE(observer2.animations_completed());
+
+  // Case 3: Animation finishes before layer is destroyed.
+  ui::Layer layer3;
+  animator = layer3.GetAnimator();
+  TestImplicitAnimationObserver observer3(false);
+
+  EXPECT_FALSE(observer3.animations_completed());
+  animator->SetGrayscale(1.0f);
+
+  {
+    ScopedLayerAnimationSettings settings(animator.get());
+    settings.DeferPaint();
+    settings.AddObserver(&observer3);
+    animator->SetGrayscale(0.0f);
+  }
+
+  EXPECT_FALSE(observer3.animations_completed());
+  animator->StopAnimatingProperty(LayerAnimationElement::GRAYSCALE);
+  EXPECT_TRUE(observer3.animations_completed());
+}
+
 // Tests that an observer added to a scoped settings object is still notified
 // when the object goes out of scope due to the animation being interrupted.
 TEST(LayerAnimatorTest, InterruptedImplicitAnimationObservers) {

diff --git a/ui/compositor/scoped_layer_animation_settings.cc b/ui/compositor/scoped_layer_animation_settings.cc
@@ -33,9 +33,10 @@ class CacheRenderSurfaceObserver : public ui::ImplicitAnimationObserver,
     // If animation finishes before |layer_| is destoyed, we will reset the
     // cache and remove |this| from the |layer_| observer list when deleting
     // |this|.
-    if (layer_)
+    if (layer_) {
       layer_->RemoveCacheRenderSurfaceRequest();
-    delete this;
+      layer_->GetAnimator()->RemoveAndDestroyOwnedObserver(this);
+    }
   }
 
   // ui::LayerObserver overrides:
@@ -53,6 +54,54 @@ class CacheRenderSurfaceObserver : public ui::ImplicitAnimationObserver,
   DISALLOW_COPY_AND_ASSIGN(CacheRenderSurfaceObserver);
 };
 
+class DeferredPaintObserver : public ui::ImplicitAnimationObserver,
+                              public ui::LayerObserver {
+ public:
+  DeferredPaintObserver(ui::Layer* layer) : layer_(layer) {
+    layer_->AddObserver(this);
+    layer_->AddDeferredPaintRequest();
+  }
+  ~DeferredPaintObserver() override {
+    if (layer_)
+      layer_->RemoveObserver(this);
+  }
+
+  // ui::ImplicitAnimationObserver overrides:
+  void OnImplicitAnimationsCompleted() override {
+    // If animation finishes before |layer_| is destoyed, we will remove the
+    // request of deferred painting and remove |this| from the |layer_|
+    // observer list when deleting |this|.
+    if (layer_) {
+      layer_->RemoveDeferredPaintRequest();
+      layer_->GetAnimator()->RemoveAndDestroyOwnedObserver(this);
+    }
+  }
+
+  // ui::LayerObserver overrides:
+  void LayerDestroyed(ui::Layer* layer) override {
+    // If the animation is still going past layer destruction then we want the
+    // layer too keep being deferred paint until the animation has finished. We
+    // will defer deleting |this| until the animation finishes.
+    layer_->RemoveObserver(this);
+    layer_ = nullptr;
+  }
+
+ private:
+  ui::Layer* layer_;
+
+  DISALLOW_COPY_AND_ASSIGN(DeferredPaintObserver);
+};
+
+void AddDeferredPaintObserverRecursive(
+    ui::Layer* layer,
+    ui::ScopedLayerAnimationSettings* settings) {
+  auto observer = base::MakeUnique<DeferredPaintObserver>(layer);
+  settings->AddObserver(observer.get());
+  layer->GetAnimator()->AddOwnedObserver(std::move(observer));
+  for (auto* child : layer->children())
+    AddDeferredPaintObserverRecursive(child, settings);
+}
+
 }  // namespace
 
 namespace ui {
@@ -102,8 +151,14 @@ void ScopedLayerAnimationSettings::SetTransitionDuration(
 }
 
 void ScopedLayerAnimationSettings::CacheRenderSurface() {
-  AddObserver(
-      new CacheRenderSurfaceObserver(animator_->delegate()->GetLayer()));
+  auto observer = base::MakeUnique<CacheRenderSurfaceObserver>(
+      animator_->delegate()->GetLayer());
+  AddObserver(observer.get());
+  animator_->AddOwnedObserver(std::move(observer));
+}
+
+void ScopedLayerAnimationSettings::DeferPaint() {
+  AddDeferredPaintObserverRecursive(animator_->delegate()->GetLayer(), this);
 }
 
 void ScopedLayerAnimationSettings::LockTransitionDuration() {

diff --git a/ui/compositor/scoped_layer_animation_settings.h b/ui/compositor/scoped_layer_animation_settings.h
@@ -34,6 +34,9 @@ class COMPOSITOR_EXPORT ScopedLayerAnimationSettings {
   // This will request render surface caching on the animating layer. The cache
   // request will be removed at the end of the animation.
   void CacheRenderSurface();
+  // This will defer painting on the animating layer. The deferred paint
+  // request will be removed at the end of the animation.
+  void DeferPaint();
   base::TimeDelta GetTransitionDuration() const;
 
   // Locks transition duration in |animator_|. When transition duration