Move cc::ClipNode::target_is_clipped to cc::EffectNode

The flag means whether the clip node's target surface will be subject to some ancestor clip. Move it to the effect node and access it through ClipNode::target_effect_id instead. This lifts the restriction that every effect node needs to associate with a clip node created by the same owner. Thus will unblock https://codereview.chromium.org/2495973002/ CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2490273004 Cr-Commit-Position: refs/heads/master@{#433333}
qweri0p · Nov 18, 2016 · c3bd281 · c3bd281
1 parent 32391c1
commit c3bd281
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 26 deletions.
diff --git a/cc/trees/clip_node.cc b/cc/trees/clip_node.cc
@@ -19,7 +19,6 @@ ClipNode::ClipNode()
       target_transform_id(-1),
       target_effect_id(-1),
       layer_clipping_uses_only_local_clip(false),
-      target_is_clipped(false),
       layers_are_clipped(false),
       layers_are_clipped_when_surfaces_disabled(false),
       resets_clip(false) {}
@@ -37,7 +36,6 @@ bool ClipNode::operator==(const ClipNode& other) const {
          target_effect_id == other.target_effect_id &&
          layer_clipping_uses_only_local_clip ==
              other.layer_clipping_uses_only_local_clip &&
-         target_is_clipped == other.target_is_clipped &&
          layers_are_clipped == other.layers_are_clipped &&
          layers_are_clipped_when_surfaces_disabled ==
              other.layers_are_clipped_when_surfaces_disabled &&
@@ -55,7 +53,6 @@ void ClipNode::AsValueInto(base::trace_event::TracedValue* value) const {
   value->SetInteger("target_effect_id", target_effect_id);
   value->SetBoolean("layer_clipping_uses_only_local_clip",
                     layer_clipping_uses_only_local_clip);
-  value->SetBoolean("target_is_clipped", target_is_clipped);
   value->SetBoolean("layers_are_clipped", layers_are_clipped);
   value->SetBoolean("layers_are_clipped_when_surfaces_disabled",
                     layers_are_clipped_when_surfaces_disabled);

diff --git a/cc/trees/clip_node.h b/cc/trees/clip_node.h
@@ -59,15 +59,15 @@ struct CC_EXPORT ClipNode {
   int target_transform_id;
 
   // The id of the effect node that defines the clip node's target space.
+  // TODO(crbug.com/642581 crbug.com/642584): As we progress toward SPv2 and
+  // layer list mode, there may be layers having the same clip but draw onto
+  // different target. Target information shall be removed from here.
   int target_effect_id;
 
   // When true, |clip_in_target_space| does not include clips from ancestor
   // nodes.
   bool layer_clipping_uses_only_local_clip : 1;
 
-  // True if target surface needs to be drawn with a clip applied.
-  bool target_is_clipped : 1;
-
   // True if layers with this clip tree node need to be drawn with a clip
   // applied.
   bool layers_are_clipped : 1;

diff --git a/cc/trees/draw_property_utils.cc b/cc/trees/draw_property_utils.cc
@@ -397,7 +397,8 @@ void CalculateVisibleRects(const LayerImplList& visible_layer_list,
     // When both the layer and the target are unclipped, we only have to apply
     // the viewport clip.
     const bool fully_visible =
-        !clip_node->layers_are_clipped && !clip_node->target_is_clipped;
+        !clip_node->layers_are_clipped &&
+        !effect_tree.Node(clip_node->target_effect_id)->surface_is_clipped;
 
     if (fully_visible) {
       if (!transform_node->ancestors_are_invertible) {
@@ -748,6 +749,9 @@ void ComputeClips(PropertyTrees* property_trees,
         transform_tree.Node(clip_node->transform_id);
     ClipNode* parent_clip_node = clip_tree->parent(clip_node);
 
+    bool target_is_clipped =
+        effect_tree.Node(clip_node->target_effect_id)->surface_is_clipped;
+
     gfx::Transform parent_to_current;
     const TransformNode* parent_target_transform_node =
         transform_tree.Node(parent_clip_node->target_transform_id);
@@ -807,7 +811,7 @@ void ComputeClips(PropertyTrees* property_trees,
             gfx::IntersectRects(clip_node->clip_in_target_space,
                                 parent_combined_clip_in_target_space);
       } else {
-        DCHECK(!clip_node->target_is_clipped);
+        DCHECK(!target_is_clipped);
         DCHECK(!clip_node->layers_are_clipped);
         clip_node->combined_clip_in_target_space =
             parent_combined_clip_in_target_space;
@@ -823,7 +827,7 @@ void ComputeClips(PropertyTrees* property_trees,
       if (!non_root_surfaces_enabled) {
         clip_node->clip_in_target_space =
             parent_clip_node->clip_in_target_space;
-      } else if (!clip_node->target_is_clipped) {
+      } else if (!target_is_clipped) {
         clip_node->clip_in_target_space = parent_clip_in_target_space;
       } else {
         // Render Surface applies clip and the owning layer itself applies
@@ -1190,14 +1194,6 @@ static void SetSurfaceDrawTransform(const PropertyTrees* property_trees,
   render_surface->SetDrawTransform(render_surface_transform);
 }
 
-static void SetSurfaceIsClipped(const ClipNode* clip_node,
-                                RenderSurfaceImpl* render_surface) {
-  DCHECK(render_surface->OwningLayerId() == clip_node->owner_id)
-      << "we now create clip node for every render surface";
-
-  render_surface->SetIsClipped(clip_node->target_is_clipped);
-}
-
 static void SetSurfaceClipRect(const ClipNode* parent_clip_node,
                                const PropertyTrees* property_trees,
                                RenderSurfaceImpl* render_surface) {
@@ -1345,17 +1341,18 @@ void ComputeMaskDrawProperties(LayerImpl* mask_layer,
 
 void ComputeSurfaceDrawProperties(const PropertyTrees* property_trees,
                                   RenderSurfaceImpl* render_surface) {
-  const ClipNode* clip_node =
-      property_trees->clip_tree.Node(render_surface->ClipTreeIndex());
-
-  SetSurfaceIsClipped(clip_node, render_surface);
+  const EffectNode* effect_node =
+      property_trees->effect_tree.Node(render_surface->EffectTreeIndex());
+  render_surface->SetIsClipped(effect_node->surface_is_clipped);
   SetSurfaceDrawOpacity(property_trees->effect_tree, render_surface);
   SetSurfaceDrawTransform(property_trees, render_surface);
   render_surface->SetScreenSpaceTransform(
       property_trees->ToScreenSpaceTransformWithoutSurfaceContentsScale(
           render_surface->TransformTreeIndex(),
           render_surface->EffectTreeIndex()));
 
+  const ClipNode* clip_node =
+      property_trees->clip_tree.Node(render_surface->ClipTreeIndex());
   SetSurfaceClipRect(property_trees->clip_tree.parent(clip_node),
                      property_trees, render_surface);
 }

diff --git a/cc/trees/effect_node.cc b/cc/trees/effect_node.cc
@@ -18,6 +18,7 @@ EffectNode::EffectNode()
       blend_mode(SkXfermode::kSrcOver_Mode),
       has_render_surface(false),
       render_surface(nullptr),
+      surface_is_clipped(false),
       has_copy_request(false),
       hidden_by_backface_visibility(false),
       double_sided(false),
@@ -42,6 +43,7 @@ bool EffectNode::operator==(const EffectNode& other) const {
          owner_id == other.owner_id && opacity == other.opacity &&
          screen_space_opacity == other.screen_space_opacity &&
          has_render_surface == other.has_render_surface &&
+         surface_is_clipped == other.surface_is_clipped &&
          has_copy_request == other.has_copy_request &&
          filters == other.filters &&
          background_filters == other.background_filters &&
@@ -71,6 +73,7 @@ void EffectNode::AsValueInto(base::trace_event::TracedValue* value) const {
   value->SetInteger("owner_id", owner_id);
   value->SetDouble("opacity", opacity);
   value->SetBoolean("has_render_surface", has_render_surface);
+  value->SetBoolean("surface_is_clipped", surface_is_clipped);
   value->SetBoolean("has_copy_request", has_copy_request);
   value->SetBoolean("double_sided", double_sided);
   value->SetBoolean("is_drawn", is_drawn);

diff --git a/cc/trees/effect_node.h b/cc/trees/effect_node.h
@@ -44,6 +44,13 @@ struct CC_EXPORT EffectNode {
 
   bool has_render_surface;
   RenderSurfaceImpl* render_surface;
+  // Only applicable if has render surface. A true value means a clip needs to
+  // be applied to the output of the surface when it is drawn onto its parent
+  // surface.
+  // TODO(crbug.com/504464): There is ongoing work to delay render surface
+  // decision to later phase of the pipeline. This flag shall be removed and
+  // computed during render surface decision.
+  bool surface_is_clipped;
   bool has_copy_request;
   bool hidden_by_backface_visibility;
   bool double_sided;

diff --git a/cc/trees/property_tree_builder.cc b/cc/trees/property_tree_builder.cc
@@ -52,7 +52,6 @@ struct DataForRecursion {
   bool affected_by_inner_viewport_bounds_delta;
   bool affected_by_outer_viewport_bounds_delta;
   bool should_flatten;
-  bool target_is_clipped;
   bool is_hidden;
   uint32_t main_thread_scrolling_reasons;
   bool scroll_tree_parent_created_by_uninheritable_criteria;
@@ -374,12 +373,15 @@ void AddClipNodeIfNeeded(const DataForRecursion<LayerType>& data_from_ancestor,
     // 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.
-    data_for_children->target_is_clipped =
+    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.
-    data_for_children->target_is_clipped = data_from_ancestor.target_is_clipped;
     layers_are_clipped = ancestor_clips_subtree;
   }
 
@@ -434,7 +436,6 @@ void AddClipNodeIfNeeded(const DataForRecursion<LayerType>& data_from_ancestor,
     else
       node.clip_type = ClipNode::ClipType::NONE;
     node.resets_clip = has_unclipped_surface;
-    node.target_is_clipped = data_for_children->target_is_clipped;
     node.layers_are_clipped = layers_are_clipped;
     node.layers_are_clipped_when_surfaces_disabled =
         layers_are_clipped_when_surfaces_disabled;
@@ -1389,7 +1390,6 @@ void BuildPropertyTreesTopLevelInternal(
   data_for_recursion.affected_by_inner_viewport_bounds_delta = false;
   data_for_recursion.affected_by_outer_viewport_bounds_delta = false;
   data_for_recursion.should_flatten = false;
-  data_for_recursion.target_is_clipped = false;
   data_for_recursion.is_hidden = false;
   data_for_recursion.main_thread_scrolling_reasons =
       MainThreadScrollingReason::kNotScrollingOnMain;