cc : Seperate adding clip node and setting render surface is_clipped

cc/trees/property_tree_builder.cc

Index: cc/trees/property_tree_builder.cc
diff --git a/cc/trees/property_tree_builder.cc b/cc/trees/property_tree_builder.cc
index 521d1bfe23d40471436d9054b80de279db057b8e..261b90334f01f162bc89dcdaedf28905fabd414f 100644
--- a/cc/trees/property_tree_builder.cc
+++ b/cc/trees/property_tree_builder.cc
@@ -53,6 +53,7 @@ struct DataForRecursion {
   bool affected_by_outer_viewport_bounds_delta;
   bool should_flatten;
   bool is_hidden;
+  bool apply_ancestor_clip;
   uint32_t main_thread_scrolling_reasons;
   bool scroll_tree_parent_created_by_uninheritable_criteria;
   const gfx::Transform* device_transform;
@@ -338,6 +339,24 @@ static LayerImpl* Parent(LayerImpl* layer) {
 }
 
 template <typename LayerType>
+static void SetSurfaceIsClipped(DataForRecursion<LayerType>* data_for_children,
+                                bool apply_ancestor_clip,
+                                LayerType* layer) {
+  // A surface with unclipped descendants cannot be clipped by its ancestor
+  // clip at draw time since the unclipped descendants aren't affected by the
+  // ancestor clip.
+  EffectNode* effect_node = data_for_children->property_trees->effect_tree.Node(
+      data_for_children->render_target);
+  DCHECK_EQ(effect_node->owner_id, layer->id());
+  effect_node->surface_is_clipped =
+      apply_ancestor_clip && !NumUnclippedDescendants(layer);
+  // The ancestor clip should propagate to children only if the surface doesn't
+  // apply the clip.
+  data_for_children->apply_ancestor_clip =
+      apply_ancestor_clip && !effect_node->surface_is_clipped;
+}
+
+template <typename LayerType>
 void AddClipNodeIfNeeded(const DataForRecursion<LayerType>& data_from_ancestor,
                          LayerType* layer,
                          bool created_render_surface,
@@ -346,23 +365,25 @@ void AddClipNodeIfNeeded(const DataForRecursion<LayerType>& data_from_ancestor,
   ClipNode* parent = GetClipParent(data_from_ancestor, layer);

[weiliangc, 2016/12/01 20:09:26]

Since GetClipParent() also has two code path depends on ClipParent(), maybe
remove this helper function and merge with apply_ancestor_clip calculation?

[jaydasika, 2016/12/02 05:20:56]

Done.

   int parent_id = parent->id;
 
-  bool is_root = !Parent(layer);
+  bool inherits_clip = !ClipParent(layer);
 
-  // Whether we have an ancestor clip that we might need to apply.
-  bool ancestor_clips_subtree = is_root || parent->layers_are_clipped;
+  bool apply_ancestor_clip = inherits_clip
+                                 ? data_from_ancestor.apply_ancestor_clip
+                                 : parent->layers_are_clipped;

[jaydasika, 2016/12/01 09:12:54]

apply_ancestor_clip here still depends on having a clip node for render surfaces
as it uses parent->layers_are_clipped. But, in the code where we do not create
clip nodes for render surfaces, it will not depend on it.

[weiliangc, 2016/12/01 20:09:26]

Could you clarify when apply_ancestor_clip would be different from old
ancestor_clips_subtree?

[jaydasika, 2016/12/02 05:20:56]

Its the same. Its just computed differently now.
This is required because when we stop creating clip nodes for render surfaces,
there won't be layers_are_clipped anymore (as it will always be true) and so we
have to track whether to apply ancestor clip using a recursion variable (which
this CL does).
When we have clip parent we still use parent's layers_are_clipped value here,
but it won't be the case in the new code path. 

 
   bool layers_are_clipped = false;
   bool has_unclipped_surface = false;
 
   if (created_render_surface) {
+    SetSurfaceIsClipped(data_for_children, apply_ancestor_clip, layer);
     // Clips can usually be applied to a surface's descendants simply by
     // clipping the surface (or applied implicitly by the surface's bounds).
     // However, if the surface has unclipped descendants (layers that aren't
     // affected by the ancestor clip), we cannot clip the surface itself, and
     // must instead apply clips to the clipped descendants.
-    if (ancestor_clips_subtree && NumUnclippedDescendants(layer) > 0) {
+    if (apply_ancestor_clip && NumUnclippedDescendants(layer) > 0) {
       layers_are_clipped = true;
-    } else if (!ancestor_clips_subtree) {
+    } else if (!apply_ancestor_clip) {
       // When there are no ancestor clips that need to be applied to a render
       // surface, we reset clipping state. The surface might contribute a clip
       // of its own, but clips from ancestor nodes don't need to be considered
@@ -370,24 +391,17 @@ void AddClipNodeIfNeeded(const DataForRecursion<LayerType>& data_from_ancestor,
       has_unclipped_surface = true;
       DCHECK_NE(parent->clip_type, ClipNode::ClipType::APPLIES_LOCAL_CLIP);
     }
-    // A surface with unclipped descendants cannot be clipped by its ancestor
-    // clip at draw time since the unclipped descendants aren't affected by the
-    // ancestor clip.
-    EffectNode* effect_node =
-        data_for_children->property_trees->effect_tree.Node(
-            data_for_children->render_target);
-    DCHECK(effect_node->owner_id == layer->id());
-    effect_node->surface_is_clipped =
-        ancestor_clips_subtree && !NumUnclippedDescendants(layer);
   } else {
     // Without a new render surface, layer clipping state from ancestors needs
     // to continue to propagate.
-    layers_are_clipped = ancestor_clips_subtree;
+    layers_are_clipped = apply_ancestor_clip;
   }
 
   bool layer_clips_subtree = LayerClipsSubtree(layer);
-  if (layer_clips_subtree)
+  if (layer_clips_subtree) {
     layers_are_clipped = true;
+    data_for_children->apply_ancestor_clip = true;
+  }
 
   // Without surfaces, all non-viewport clips have to be applied using layer
   // clipping.
@@ -419,12 +433,12 @@ void AddClipNodeIfNeeded(const DataForRecursion<LayerType>& data_from_ancestor,
                                    ->transform_id;
     node.owner_id = layer->id();
 
-    if (ancestor_clips_subtree || layer_clips_subtree) {
+    if (apply_ancestor_clip || layer_clips_subtree) {
       // Surfaces reset the rect used for layer clipping. At other nodes, layer
       // clipping state from ancestors must continue to get propagated.
       node.layer_clipping_uses_only_local_clip =
           (created_render_surface && NumUnclippedDescendants(layer) == 0) ||
-          !ancestor_clips_subtree;
+          !apply_ancestor_clip;
     } else {
       // Otherwise, we're either unclipped, or exist only in order to apply our
       // parent's clips in our space.
@@ -1391,6 +1405,8 @@ void BuildPropertyTreesTopLevelInternal(
   data_for_recursion.affected_by_outer_viewport_bounds_delta = false;
   data_for_recursion.should_flatten = false;
   data_for_recursion.is_hidden = false;
+  // The root clip is always applied.
+  data_for_recursion.apply_ancestor_clip = true;
   data_for_recursion.main_thread_scrolling_reasons =
       MainThreadScrollingReason::kNotScrollingOnMain;
   data_for_recursion.scroll_tree_parent_created_by_uninheritable_criteria =