Add ClipNode when Render Surface Inherits Clip

cc/trees/draw_property_utils.cc

Index: cc/trees/draw_property_utils.cc
diff --git a/cc/trees/draw_property_utils.cc b/cc/trees/draw_property_utils.cc
index 175c33415100440aa966860f8d6a576a48d6f30e..30ae9692b6df96d7b924486969de33ae12c6fe96 100644
--- a/cc/trees/draw_property_utils.cc
+++ b/cc/trees/draw_property_utils.cc
@@ -417,8 +417,11 @@ void ComputeClips(ClipTree* clip_tree, const TransformTree& transform_tree) {
         target_is_root_surface ? 0 : clip_node->data.target_id;
 
     bool success = true;
-    if (parent_transform_node->data.content_target_id ==
-        clip_node->data.target_id) {
+    if (clip_node->data.render_surface_applies_clip) {
+      success &= transform_tree.ComputeTransform(transform_node->data.target_id,
+                                                 target_id, &parent_to_target);

[ajuma, 2015/07/29 16:44:29]

I'm having trouble following this. parent_to_target used to be the
transformation from the parent transform node to the target node (so, in the
example given in the comment above, it would be the transform from A to T). But
here it's the transform from the transform node's render surface to the target
(and, depending on the transform tree, the transform node's render surface might
be the same as target, or might be the target's target). I'd have expected this
change to also require a change below (where we use parent_to_target to
transform the parent_clip_node's combined_clip from parent space to target
space).

[jaydasika, 2015/07/30 19:43:10]

Done.

+    } else if (parent_transform_node->data.content_target_id ==
+               clip_node->data.target_id) {
       parent_to_target = parent_transform_node->data.to_target;
     } else {
       success &= transform_tree.ComputeTransformWithDestinationSublayerScale(
@@ -448,8 +451,14 @@ void ComputeClips(ClipTree* clip_tree, const TransformTree& transform_tree) {
     // In order to intersect with as small a rect as possible, we do a
     // preliminary clip in target space so that when we project back, there's
     // less likelihood of intersecting the view plane.
-    gfx::RectF inherited_clip_in_target_space = MathUtil::MapClippedRect(
-        parent_to_target, parent_clip_node->data.combined_clip);
+    gfx::RectF inherited_clip_in_target_space;
+    if (parent_transform_node->id > target_id &&

[ajuma, 2015/07/29 16:44:29]

We weren't previously handling the case where parent_transform_node->id <
target_id, so that was itself probably causing bugs.

Please add (at least) two test cases to layer_tree_host_common_unittest.cc
showing what this change fixes: one where we have parent_transform_node->id <
target_id, and another where we have render_surface_applies_clip.

[jaydasika, 2015/07/30 19:43:10]

Add a unit test where render surface applies clip. I am not sure if the other
case ever occurs.

+        !clip_node->data.render_surface_applies_clip)
+      inherited_clip_in_target_space = MathUtil::MapClippedRect(
+          parent_to_target, parent_clip_node->data.combined_clip);
+    else
+      inherited_clip_in_target_space = MathUtil::ProjectClippedRect(
+          parent_to_target, parent_clip_node->data.combined_clip);
 
     gfx::RectF clip_in_target_space =
         MathUtil::MapClippedRect(clip_to_target, clip_node->data.clip);