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..482dc577a7b0b2852d402e5fff7d52d6a4802c96 100644
--- a/cc/trees/draw_property_utils.cc
+++ b/cc/trees/draw_property_utils.cc
@@ -388,6 +388,7 @@ void ComputeClips(ClipTree* clip_tree, const TransformTree& transform_tree) {
     // clip adjusted due to intersecting with an ancestor clip.
     const bool is_clipped = clip_node->parent_id > 0;
     if (!is_clipped) {
+      DCHECK(!clip_node->data.render_surface_applies_clip);
       clip_node->data.combined_clip = clip_node->data.clip;
       continue;
     }
@@ -409,6 +410,8 @@ void ComputeClips(ClipTree* clip_tree, const TransformTree& transform_tree) {
     gfx::Transform parent_to_target;
     gfx::Transform clip_to_target;
     gfx::Transform target_to_clip;
+    gfx::Transform parent_to_transform_target;
+    gfx::Transform transform_target_to_target;
 
     const bool target_is_root_surface = clip_node->data.target_id == 1;
     // When the target is the root surface, we need to include the root
@@ -417,8 +420,19 @@ 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) {
+    // When render surface applies clip, we need the clip from the target's

[ajuma, 2015/07/30 20:56:50]

Is it that we need a clip "from" that space, or that we actually need to perform
the clip "in" that space?

[jaydasika, 2015/07/31 00:46:29]

We need the clip in "that" space projected into current target space.

+    // target space. But, as the combined clip is in parent clip's target
+    // space, we need to first transform it from parent's target space to
+    // target's target space.
+    if (clip_node->data.render_surface_applies_clip) {
+      success &= transform_tree.ComputeTransformWithDestinationSublayerScale(
+          parent_transform_node->id, transform_node->data.target_id,
+          &parent_to_transform_target);
+      success &= transform_tree.ComputeTransformWithDestinationSublayerScale(
+          transform_node->data.target_id, target_id,
+          &transform_target_to_target);
+    } 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,19 +462,40 @@ 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 (clip_node->data.render_surface_applies_clip) {
+      gfx::RectF combined_clip_in_transform_target_space =
+          MathUtil::ProjectClippedRect(parent_to_transform_target,
+                                       parent_clip_node->data.combined_clip);

[ajuma, 2015/07/30 20:56:50]

Will it always be true that in this case, the parent node will be an ancestor of
the transform target node (since we're using Project rather than Map)? I think
that makes sense, but please add a DCHECK.

[jaydasika, 2015/07/31 00:46:29]

It is not always true that parent node will be ancestor of the transform target
node as transform target is target of the render surface and not the surface
itself.

+      inherited_clip_in_target_space = MathUtil::ProjectClippedRect(
+          transform_target_to_target, combined_clip_in_transform_target_space);

[ajuma, 2015/07/30 20:56:50]

The combined effect of this line and the line above is to project
parent_clip_node->data.combined_clip from parent space to target space. What
goes wrong (geometrically) if we just do this projection in a single step?

[jaydasika, 2015/07/31 00:46:29]

Since we might map/project(next patch), I don't think they can be combined?

+    } else if (parent_transform_node->id > target_id) {
+      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);
+    // When render surface applies clip, the layer that created the clip node
+    // doesn't apply any clip. So, we should clip using the clip value
+    // stored in the clip node.

[ajuma, 2015/07/30 20:56:50]

Just to see if I understand: the "layer" here is the owner of the render
surface, and instead of clipping to its own bounds, it ignores its own bounds
and applies the clip inherited from its parent?

If that's the case, would it be equivalent to simply not create a clip node for
this layer, and allow the clip tree to propagate the clip?

[jaydasika, 2015/07/31 00:46:29]

Yes, it ignores its own bounds. It uses clip inherited from ancestor
"intersected" with projection of clip rect of its render surface's target into
the render surface's space. We need the clip node for the second rect.

+    gfx::RectF intersected_in_target_space;
+    if (!clip_node->data.render_surface_applies_clip) {
+      gfx::RectF clip_in_target_space =
+          MathUtil::MapClippedRect(clip_to_target, clip_node->data.clip);
 
-    gfx::RectF intersected_in_target_space = gfx::IntersectRects(
-        inherited_clip_in_target_space, clip_in_target_space);
+      intersected_in_target_space = gfx::IntersectRects(
+          inherited_clip_in_target_space, clip_in_target_space);
+    } else {
+      intersected_in_target_space = inherited_clip_in_target_space;
+    }
 
     clip_node->data.combined_clip = MathUtil::ProjectClippedRect(
         target_to_clip, intersected_in_target_space);
 
-    clip_node->data.combined_clip.Intersect(clip_node->data.clip);
+    if (!clip_node->data.render_surface_applies_clip)
+      clip_node->data.combined_clip.Intersect(clip_node->data.clip);
   }
   clip_tree->set_needs_update(false);
 }