Clean up PointIsClippedBySurfaceOrClipRect

This function made unnecessary use of the render
target and didn't walk its clipping ancestor
chain correctly. This CL refactors the function
and fixes that walk (and adds test to show that
the walk is correct).


R=danakj@chromium.org

Review URL: https://codereview.chromium.org/328753002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@276799 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 157 insertions, 21 deletions

diff --git a/cc/trees/layer_tree_impl.cc b/cc/trees/layer_tree_impl.cc
index 6a35981..2cc9c9f 100644
--- a/cc/trees/layer_tree_impl.cc
+++ b/cc/trees/layer_tree_impl.cc
@@ -1095,35 +1095,33 @@ static bool PointHitsRegion(const gfx::PointF& screen_space_point,
       gfx::ToRoundedPoint(hit_test_point_in_layer_space));
 }
 
+static LayerImpl* GetNextClippingLayer(LayerImpl* layer) {
+  if (layer->scroll_parent())
+    return layer->scroll_parent();
+  if (layer->clip_parent())
+    return layer->clip_parent();
+  return layer->parent();
+}
+
 static bool PointIsClippedBySurfaceOrClipRect(
     const gfx::PointF& screen_space_point,
     LayerImpl* layer) {
-  LayerImpl* current_layer = layer;
-
   // Walk up the layer tree and hit-test any render_surfaces and any layer
   // clip rects that are active.
-  while (current_layer) {
-    if (current_layer->render_surface() &&
-        !PointHitsRect(
-            screen_space_point,
-            current_layer->render_surface()->screen_space_transform(),
-            current_layer->render_surface()->content_rect(),
-            NULL))
+  for (; layer; layer = GetNextClippingLayer(layer)) {
+    if (layer->render_surface() &&
+        !PointHitsRect(screen_space_point,
+                       layer->render_surface()->screen_space_transform(),
+                       layer->render_surface()->content_rect(),
+                       NULL))
       return true;
 
-    // Note that drawable content rects are actually in target surface space, so
-    // the transform we have to provide is the target surface's
-    // screen_space_transform.
-    LayerImpl* render_target = current_layer->render_target();
-    if (LayerClipsSubtree(current_layer) &&
-        !PointHitsRect(
-            screen_space_point,
-            render_target->render_surface()->screen_space_transform(),
-            current_layer->drawable_content_rect(),
-            NULL))
+    if (LayerClipsSubtree(layer) &&
+        !PointHitsRect(screen_space_point,
+                       layer->screen_space_transform(),
+                       gfx::Rect(layer->content_bounds()),
+                       NULL))
       return true;
-
-    current_layer = current_layer->parent();
   }
 
   // If we have finished walking all ancestors without having already exited,
diff --git a/cc/trees/layer_tree_impl_unittest.cc b/cc/trees/layer_tree_impl_unittest.cc
index 8db2b95..91bad3e 100644
--- a/cc/trees/layer_tree_impl_unittest.cc
+++ b/cc/trees/layer_tree_impl_unittest.cc
@@ -1117,6 +1117,144 @@ TEST_F(LayerTreeImplTest, HitTestingForMultipleLayersAtVaryingDepths) {
   EXPECT_EQ(4, result_layer->id());
 }
 
+TEST_F(LayerTreeImplTest, HitTestingRespectsClipParents) {
+  scoped_ptr<LayerImpl> root = LayerImpl::Create(host_impl().active_tree(), 1);
+  gfx::Transform identity_matrix;
+  gfx::Point3F transform_origin;
+  gfx::PointF position;
+  gfx::Size bounds(100, 100);
+  SetLayerPropertiesForTesting(root.get(),
+                               identity_matrix,
+                               transform_origin,
+                               position,
+                               bounds,
+                               true,
+                               false);
+  root->SetDrawsContent(true);
+  {
+    scoped_ptr<LayerImpl> child =
+        LayerImpl::Create(host_impl().active_tree(), 2);
+    scoped_ptr<LayerImpl> grand_child =
+        LayerImpl::Create(host_impl().active_tree(), 4);
+
+    position = gfx::PointF(10.f, 10.f);
+    bounds = gfx::Size(1, 1);
+    SetLayerPropertiesForTesting(child.get(),
+                                 identity_matrix,
+                                 transform_origin,
+                                 position,
+                                 bounds,
+                                 true,
+                                 false);
+    child->SetDrawsContent(true);
+    child->SetMasksToBounds(true);
+
+    position = gfx::PointF(0.f, 40.f);
+    bounds = gfx::Size(100, 50);
+    SetLayerPropertiesForTesting(grand_child.get(),
+                                 identity_matrix,
+                                 transform_origin,
+                                 position,
+                                 bounds,
+                                 true,
+                                 false);
+    grand_child->SetDrawsContent(true);
+    grand_child->SetForceRenderSurface(true);
+
+    // This should let |grand_child| "escape" |child|'s clip.
+    grand_child->SetClipParent(root.get());
+
+    child->AddChild(grand_child.Pass());
+    root->AddChild(child.Pass());
+  }
+
+  host_impl().SetViewportSize(root->bounds());
+  host_impl().active_tree()->SetRootLayer(root.Pass());
+  host_impl().active_tree()->UpdateDrawProperties();
+
+  gfx::Point test_point = gfx::Point(12, 52);
+  LayerImpl* result_layer =
+      host_impl().active_tree()->FindLayerThatIsHitByPoint(test_point);
+  ASSERT_TRUE(result_layer);
+  EXPECT_EQ(4, result_layer->id());
+}
+
+TEST_F(LayerTreeImplTest, HitTestingRespectsScrollParents) {
+  scoped_ptr<LayerImpl> root = LayerImpl::Create(host_impl().active_tree(), 1);
+  gfx::Transform identity_matrix;
+  gfx::Point3F transform_origin;
+  gfx::PointF position;
+  gfx::Size bounds(100, 100);
+  SetLayerPropertiesForTesting(root.get(),
+                               identity_matrix,
+                               transform_origin,
+                               position,
+                               bounds,
+                               true,
+                               false);
+  root->SetDrawsContent(true);
+  {
+    scoped_ptr<LayerImpl> child =
+        LayerImpl::Create(host_impl().active_tree(), 2);
+    scoped_ptr<LayerImpl> scroll_child =
+        LayerImpl::Create(host_impl().active_tree(), 3);
+    scoped_ptr<LayerImpl> grand_child =
+        LayerImpl::Create(host_impl().active_tree(), 4);
+
+    position = gfx::PointF(10.f, 10.f);
+    bounds = gfx::Size(1, 1);
+    SetLayerPropertiesForTesting(child.get(),
+                                 identity_matrix,
+                                 transform_origin,
+                                 position,
+                                 bounds,
+                                 true,
+                                 false);
+    child->SetDrawsContent(true);
+    child->SetMasksToBounds(true);
+
+    position = gfx::PointF();
+    bounds = gfx::Size(200, 200);
+    SetLayerPropertiesForTesting(scroll_child.get(),
+                                 identity_matrix,
+                                 transform_origin,
+                                 position,
+                                 bounds,
+                                 true,
+                                 false);
+    scroll_child->SetDrawsContent(true);
+
+    // This should cause scroll child and its descendants to be affected by
+    // |child|'s clip.
+    scroll_child->SetScrollParent(child.get());
+
+    SetLayerPropertiesForTesting(grand_child.get(),
+                                 identity_matrix,
+                                 transform_origin,
+                                 position,
+                                 bounds,
+                                 true,
+                                 false);
+    grand_child->SetDrawsContent(true);
+    grand_child->SetForceRenderSurface(true);
+
+    scroll_child->AddChild(grand_child.Pass());
+    root->AddChild(scroll_child.Pass());
+    root->AddChild(child.Pass());
+  }
+
+  host_impl().SetViewportSize(root->bounds());
+  host_impl().active_tree()->SetRootLayer(root.Pass());
+  host_impl().active_tree()->UpdateDrawProperties();
+
+  gfx::Point test_point = gfx::Point(12, 52);
+  LayerImpl* result_layer =
+      host_impl().active_tree()->FindLayerThatIsHitByPoint(test_point);
+  // The |test_point| should have been clipped away by |child|, the scroll
+  // parent, so the only thing that should be hit is |root|.
+  ASSERT_TRUE(result_layer);
+  ASSERT_EQ(1, result_layer->id());
+}
 TEST_F(LayerTreeImplTest, HitTestingForMultipleLayerLists) {
   //
   // The geometry is set up similarly to the previous case, but