SubtreeShouldBeSkipped uses information from property trees

cc/trees/layer_tree_host_common_unittest.cc

Index: cc/trees/layer_tree_host_common_unittest.cc
diff --git a/cc/trees/layer_tree_host_common_unittest.cc b/cc/trees/layer_tree_host_common_unittest.cc
index 08e611329374e56d2bb210cfe868e23f476aa270..987fe999bc449006cc7ee152d58f9e0536ea8bc2 100644
--- a/cc/trees/layer_tree_host_common_unittest.cc
+++ b/cc/trees/layer_tree_host_common_unittest.cc
@@ -12,6 +12,7 @@
 
 #include "cc/animation/animation_host.h"
 #include "cc/animation/animation_id_provider.h"
+#include "cc/animation/animation_player.h"
 #include "cc/animation/keyframed_animation_curve.h"
 #include "cc/animation/layer_animation_controller.h"
 #include "cc/animation/transform_operations.h"
@@ -3521,16 +3522,13 @@ TEST_F(LayerTreeHostCommonTest,
 
   // Though all layers have invertible transforms, matrix multiplication using
   // floating-point math makes the draw transform uninvertible.
-  EXPECT_FALSE(grand_child->DrawTransform().IsInvertible());
-
-  // CalcDrawProps only skips a subtree when a layer's own transform is
-  // uninvertible, not when its draw transform is invertible, since CDP makes
-  // skipping decisions before computing a layer's draw transform. Property
-  // trees make skipping decisions after computing draw transforms, so could be
-  // made to skip layers with an uninvertible draw transform (once CDP is
-  // deleted).
-  EXPECT_EQ(gfx::Rect(grand_child->bounds()),
-            grand_child->visible_layer_rect());
+  EXPECT_FALSE(root->layer_tree_impl()
+                   ->property_trees()
+                   ->transform_tree.Node(grand_child->transform_tree_index())
+                   ->data.ancestors_are_invertible);
+
+  // CalcDrawProps skips a subtree when a layer's own transform is uninvertible
+  EXPECT_EQ(gfx::Rect(), grand_child->visible_layer_rect());
 }
 
 TEST_F(LayerTreeHostCommonTest,
@@ -3589,12 +3587,16 @@ TEST_F(LayerTreeHostCommonTest,
 
   // Though all layers have invertible transforms, matrix multiplication using
   // floating-point math makes the draw transform uninvertible.
-  EXPECT_FALSE(grand_child_ptr->DrawTransform().IsInvertible());
+  EXPECT_FALSE(
+      host_impl.active_tree()
+          ->property_trees()
+          ->transform_tree.Node(grand_child_ptr->transform_tree_index())
+          ->data.ancestors_are_invertible);

[ajuma, 2016/03/24 15:11:59]

Hmm. I think what's going wrong here is that we're not computing draw properties
for this layer (since we're now correctly skipping it because it's screen space
transform is non-invertible), but since the subtree-skipping logic used to
compute the render surface layer list hasn't been updated yet, the layer is
still in the RSLL, and therefore *thinks* it has up-to-date draw properties
(otherwise DrawTransform would be correctly computed on demand).

I think this means we need make the layer_tree_host_common and
draw_property_utils skipping logic match as a prerequisite for this CL (so that
layers without up-to-date draw properties don't wind up in the RSLL).

 
   // Since |grand_child| has an uninvertible draw transform, it is treated as
   // unoccluded (even though |occluding_child| comes later in draw order, and
   // hence potentially occludes it).
-  gfx::Rect layer_bounds = gfx::Rect(grand_child_ptr->bounds());
+  gfx::Rect layer_bounds = gfx::Rect();
   EXPECT_EQ(
       layer_bounds,
       grand_child_ptr->draw_properties()
@@ -4496,7 +4498,7 @@ TEST_F(LayerTreeHostCommonTest, BackFaceCullingWithPreserves3d) {
   EXPECT_FALSE(front_facing_child_of_back_facing_surface->has_render_surface());
   EXPECT_FALSE(back_facing_child_of_back_facing_surface->has_render_surface());
 
-  EXPECT_EQ(3u, update_layer_list_impl()->size());
+  EXPECT_EQ(4u, update_layer_list_impl()->size());
 
   EXPECT_TRUE(UpdateLayerListImplContains(front_facing_child->id()));
   EXPECT_TRUE(UpdateLayerListImplContains(front_facing_surface->id()));
@@ -5849,26 +5851,26 @@ TEST_F(LayerTreeHostCommonTest, ClippedOutCopyRequest) {
   copy_layer->AddChild(std::move(copy_child));
   copy_parent->AddChild(std::move(copy_layer));
   root->AddChild(std::move(copy_parent));
-  host_impl.pending_tree()->SetRootLayer(std::move(root));
-  LayerImpl* root_ptr = host_impl.pending_tree()->root_layer();
 
   LayerImplList render_surface_layer_list;
-  root_ptr->layer_tree_impl()->IncrementRenderSurfaceListIdForTesting();
+  root->layer_tree_impl()->IncrementRenderSurfaceListIdForTesting();
+  LayerImpl* root_layer = root.get();
+  root->layer_tree_impl()->SetRootLayer(std::move(root));

[ajuma, 2016/03/24 15:11:59]

This changes (renaming root_ptr and so on) seem correct, but not really
necessary.

[sunxd, 2016/03/29 14:26:34]

Ah, yes, it is a result of rebasing. Because in other tests we use root_layer, I
think it might be better to use the same name here.

   LayerTreeHostCommon::CalcDrawPropsImplInputsForTesting inputs(
-      root_ptr, root_ptr->bounds(), &render_surface_layer_list,
-      root_ptr->layer_tree_impl()->current_render_surface_list_id());
+      root_layer, root_layer->bounds(), &render_surface_layer_list,
+      root_layer->layer_tree_impl()->current_render_surface_list_id());
   inputs.can_adjust_raster_scales = true;
   LayerTreeHostCommon::CalculateDrawProperties(&inputs);
 
   // We should have two render surface, as the others are clipped out.
   ASSERT_EQ(2u, render_surface_layer_list.size());
-  EXPECT_EQ(root_ptr->id(), render_surface_layer_list.at(0)->id());
+  EXPECT_EQ(root_layer->id(), render_surface_layer_list.at(0)->id());
 
   // The root render surface should only have 2 contributing layer, since the
   // other layers are empty/clipped away.
-  ASSERT_EQ(2u, root_ptr->render_surface()->layer_list().size());
-  EXPECT_EQ(root_ptr->id(),
-            root_ptr->render_surface()->layer_list().at(0)->id());
+  ASSERT_EQ(2u, root_layer->render_surface()->layer_list().size());
+  EXPECT_EQ(root_layer->id(),
+            root_layer->render_surface()->layer_list().at(0)->id());
 }
 
 TEST_F(LayerTreeHostCommonTest, VisibleContentRectInsideSurface) {
@@ -6415,16 +6417,16 @@ TEST_F(LayerTreeHostCommonTest, CanRenderToSeparateSurface) {
   child2->AddChild(std::move(child3));
   child1->AddChild(std::move(child2));
   root->AddChild(std::move(child1));
-  host_impl.active_tree()->SetRootLayer(std::move(root));
-  LayerImpl* root_ptr = host_impl.active_tree()->root_layer();
+  LayerImpl* root_layer = root.get();
+  root->layer_tree_impl()->SetRootLayer(std::move(root));
 
   {
     LayerImplList render_surface_layer_list;
-    FakeLayerTreeHostImpl::RecursiveUpdateNumChildren(root_ptr);
-    root_ptr->layer_tree_impl()->IncrementRenderSurfaceListIdForTesting();
+    FakeLayerTreeHostImpl::RecursiveUpdateNumChildren(root_layer);
+    root_layer->layer_tree_impl()->IncrementRenderSurfaceListIdForTesting();
     LayerTreeHostCommon::CalcDrawPropsImplInputsForTesting inputs(
-        root_ptr, root_ptr->bounds(), &render_surface_layer_list,
-        root_ptr->layer_tree_impl()->current_render_surface_list_id());
+        root_layer, root_layer->bounds(), &render_surface_layer_list,
+        root_layer->layer_tree_impl()->current_render_surface_list_id());
     inputs.can_render_to_separate_surface = true;
     LayerTreeHostCommon::CalculateDrawProperties(&inputs);
 
@@ -6454,10 +6456,10 @@ TEST_F(LayerTreeHostCommonTest, CanRenderToSeparateSurface) {
 
   {
     LayerImplList render_surface_layer_list;
-    root_ptr->layer_tree_impl()->IncrementRenderSurfaceListIdForTesting();
+    root_layer->layer_tree_impl()->IncrementRenderSurfaceListIdForTesting();
     LayerTreeHostCommon::CalcDrawPropsImplInputsForTesting inputs(
-        root_ptr, root_ptr->bounds(), &render_surface_layer_list,
-        root_ptr->layer_tree_impl()->current_render_surface_list_id());
+        root_layer, root_layer->bounds(), &render_surface_layer_list,
+        root_layer->layer_tree_impl()->current_render_surface_list_id());
     inputs.can_render_to_separate_surface = false;
     LayerTreeHostCommon::CalculateDrawProperties(&inputs);
 
@@ -7351,7 +7353,6 @@ TEST_F(LayerTreeHostCommonTest, MaximumAnimationScaleFactor) {
   child->AddChild(std::move(grand_child));
   parent->AddChild(std::move(child));
   grand_parent->AddChild(std::move(parent));
-  host_impl.active_tree()->SetRootLayer(std::move(grand_parent));
 
   SetLayerPropertiesForTesting(grand_parent_raw, identity_matrix,
                                gfx::Point3F(), gfx::PointF(), gfx::Size(1, 2),
@@ -7367,6 +7368,8 @@ TEST_F(LayerTreeHostCommonTest, MaximumAnimationScaleFactor) {
                                gfx::PointF(), gfx::Size(1, 2), true, false,
                                false);
 
+  host_impl.active_tree()->SetRootLayer(std::move(grand_parent));
+
   ExecuteCalculateDrawProperties(grand_parent_raw);
 
   // No layers have animations.
@@ -8885,7 +8888,7 @@ TEST_F(LayerTreeHostCommonTest, SkippingSubtreeMain) {
   EXPECT_EQ(gfx::Rect(10, 10), grandchild->visible_layer_rect());
 }
 
-TEST_F(LayerTreeHostCommonTest, SkippingSubtreeImpl) {
+TEST_F(LayerTreeHostCommonTest, SkippingLayerImpl) {
   FakeImplTaskRunnerProvider task_runner_provider;
   TestSharedBitmapManager shared_bitmap_manager;
   TestTaskGraphRunner task_graph_runner;
@@ -8934,8 +8937,10 @@ TEST_F(LayerTreeHostCommonTest, SkippingSubtreeImpl) {
   grandchild_ptr->set_visible_layer_rect(gfx::Rect());
   gfx::Transform singular;
   singular.matrix().set(0, 0, 0);
+  singular.matrix().set(0, 1, 1);

[ajuma, 2016/03/24 15:11:59]

Is this needed? It looks like the matrix is non-invertible either way?

[sunxd, 2016/03/29 14:26:34]

The problem here is that a transform matrix
 0 0 0 0
 0 1 0 0
(0 0 1 0) can make ComputeVisibleRects get a gfx::Rect(0, 0) result for 
 0 0 0 1

grand_child's visible_layer_rect even though the layer isn't skipped.

 
   child_ptr->SetTransform(singular);
+  host_impl.active_tree()->property_trees()->needs_rebuild = true;
   ExecuteCalculateDrawPropertiesWithPropertyTrees(root_ptr);
   EXPECT_EQ(gfx::Rect(0, 0), grandchild_ptr->visible_layer_rect());
   child_ptr->SetTransform(identity);
@@ -8948,7 +8953,24 @@ TEST_F(LayerTreeHostCommonTest, SkippingSubtreeImpl) {
   child_ptr->SetOpacity(0.f);
   ExecuteCalculateDrawPropertiesWithPropertyTrees(root_ptr);
   EXPECT_EQ(gfx::Rect(0, 0), grandchild_ptr->visible_layer_rect());
+  child_ptr->SetOpacity(1.f);
+
+  root_ptr->SetTransform(singular);

[sunxd, 2016/03/30 15:21:34]

Add comment

+  child_ptr->SetScrollClipLayer(root_ptr->id());

[ajuma, 2016/03/24 15:11:59]

This test doesn't seem to involve scrolling at all, so is this needed?

[sunxd, 2016/03/29 14:26:34]

This is for creating a transform_node for child, otherwise we have only one node
in the transform_tree.

+  host_impl.active_tree()->property_trees()->needs_rebuild = true;
+  ExecuteCalculateDrawPropertiesWithPropertyTrees(root_ptr);
+  EXPECT_EQ(gfx::Rect(0, 0), grandchild_ptr->visible_layer_rect());
+  root_ptr->SetTransform(identity);
+  child_ptr->SetScrollClipLayer(-1);
 
+  root_ptr->SetOpacity(0.f);
+  child_ptr->SetOpacity(0.7f);
+  host_impl.active_tree()->property_trees()->needs_rebuild = true;
+  ExecuteCalculateDrawPropertiesWithPropertyTrees(root_ptr);
+  EXPECT_EQ(gfx::Rect(0, 0), grandchild_ptr->visible_layer_rect());
+  root_ptr->SetOpacity(1.f);
+
+  child_ptr->SetOpacity(0.f);
   // Now, even though child has zero opacity, we will configure |grandchild| and
   // |greatgrandchild| in several ways that should force the subtree to be
   // processed anyhow.
@@ -8959,6 +8981,106 @@ TEST_F(LayerTreeHostCommonTest, SkippingSubtreeImpl) {
   root_ptr->layer_tree_impl()->property_trees()->needs_rebuild = true;
   ExecuteCalculateDrawPropertiesWithPropertyTrees(root_ptr);
   EXPECT_EQ(gfx::Rect(10, 10), grandchild_ptr->visible_layer_rect());
+
+  scoped_ptr<KeyframedTransformAnimationCurve> curve(
+      KeyframedTransformAnimationCurve::Create());
+  TransformOperations start;
+  start.AppendTranslate(1.f, 2.f, 3.f);
+  gfx::Transform transform;
+  transform.Scale3d(1.0, 2.0, 3.0);
+  TransformOperations operation;
+  operation.AppendMatrix(transform);
+  curve->AddKeyframe(
+      TransformKeyframe::Create(base::TimeDelta(), start, nullptr));
+  curve->AddKeyframe(TransformKeyframe::Create(
+      base::TimeDelta::FromSecondsD(1.0), operation, nullptr));
+  scoped_ptr<Animation> transform_animation(
+      Animation::Create(std::move(curve), 3, 3, TargetProperty::TRANSFORM));
+  scoped_refptr<AnimationPlayer> player(AnimationPlayer::Create(1));
+  host_impl.active_tree()->animation_host()->RegisterPlayerForLayer(
+      root_ptr->id(), player.get());
+  host_impl.active_tree()
+      ->animation_host()
+      ->GetControllerForLayerId(root_ptr->id())
+      ->AddAnimation(std::move(transform_animation));
+  grandchild_ptr->set_visible_layer_rect(gfx::Rect());
+  child_ptr->SetScrollClipLayer(root_ptr->id());
+  root_ptr->SetTransform(singular);
+  child_ptr->SetTransform(singular);
+  root_ptr->layer_tree_impl()->property_trees()->needs_rebuild = true;
+  ExecuteCalculateDrawPropertiesWithPropertyTrees(root_ptr);
+  EXPECT_EQ(gfx::Rect(10, 10), grandchild_ptr->visible_layer_rect());
+
+  host_impl.active_tree()->animation_host()->UnregisterPlayerForLayer(
+      root_ptr->id(), player.get());
+}
+
+TEST_F(LayerTreeHostCommonTest, SkippingPendingLayerImpl) {
+  FakeImplTaskRunnerProvider task_runner_provider;
+  TestSharedBitmapManager shared_bitmap_manager;
+  TestTaskGraphRunner task_graph_runner;
+  FakeLayerTreeHostImpl host_impl(&task_runner_provider, &shared_bitmap_manager,
+                                  &task_graph_runner);
+
+  gfx::Transform identity;
+  host_impl.CreatePendingTree();
+  scoped_ptr<LayerImpl> root = LayerImpl::Create(host_impl.pending_tree(), 1);
+  scoped_ptr<LayerImpl> child = LayerImpl::Create(host_impl.pending_tree(), 2);
+  scoped_ptr<LayerImpl> grandchild =
+      LayerImpl::Create(host_impl.pending_tree(), 3);
+
+  scoped_ptr<FakePictureLayerImpl> greatgrandchild(
+      FakePictureLayerImpl::Create(host_impl.pending_tree(), 4));
+
+  child->SetDrawsContent(true);
+  grandchild->SetDrawsContent(true);
+  greatgrandchild->SetDrawsContent(true);
+
+  SetLayerPropertiesForTesting(root.get(), identity, gfx::Point3F(),
+                               gfx::PointF(), gfx::Size(100, 100), true, false,
+                               true);
+  SetLayerPropertiesForTesting(child.get(), identity, gfx::Point3F(),
+                               gfx::PointF(), gfx::Size(10, 10), true, false,
+                               false);
+  SetLayerPropertiesForTesting(grandchild.get(), identity, gfx::Point3F(),
+                               gfx::PointF(), gfx::Size(10, 10), true, false,
+                               false);
+
+  LayerImpl* root_ptr = root.get();
+  LayerImpl* grandchild_ptr = grandchild.get();
+
+  child->AddChild(std::move(grandchild));
+  root->AddChild(std::move(child));
+
+  host_impl.pending_tree()->SetRootLayer(std::move(root));
+
+  // Check the non-skipped case.
+  ExecuteCalculateDrawPropertiesWithPropertyTrees(root_ptr);
+  EXPECT_EQ(gfx::Rect(10, 10), grandchild_ptr->visible_layer_rect());
+
+  scoped_ptr<KeyframedFloatAnimationCurve> curve(
+      KeyframedFloatAnimationCurve::Create());
+  scoped_ptr<TimingFunction> func = EaseTimingFunction::Create();
+  curve->AddKeyframe(
+      FloatKeyframe::Create(base::TimeDelta(), 0.9f, std::move(func)));
+  curve->AddKeyframe(
+      FloatKeyframe::Create(base::TimeDelta::FromSecondsD(1.0), 0.3f, nullptr));
+  scoped_ptr<Animation> animation(
+      Animation::Create(std::move(curve), 3, 3, TargetProperty::OPACITY));
+  scoped_refptr<AnimationPlayer> player(AnimationPlayer::Create(1));
+  host_impl.active_tree()->animation_host()->RegisterPlayerForLayer(
+      root_ptr->id(), player.get());
+  host_impl.active_tree()
+      ->animation_host()
+      ->GetControllerForLayerId(root_ptr->id())
+      ->AddAnimation(std::move(animation));
+  grandchild_ptr->set_visible_layer_rect(gfx::Rect());
+  root_ptr->layer_tree_impl()->property_trees()->needs_rebuild = true;
+  ExecuteCalculateDrawPropertiesWithPropertyTrees(root_ptr);
+  EXPECT_EQ(gfx::Rect(10, 10), grandchild_ptr->visible_layer_rect());
+
+  host_impl.active_tree()->animation_host()->UnregisterPlayerForLayer(
+      root_ptr->id(), player.get());
 }
 
 TEST_F(LayerTreeHostCommonTest, SkippingLayer) {