cc: Set clip rect to empty rect when layer's is_clipped is false

cc/trees/draw_property_utils.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/trees/draw_property_utils.h"
 
 #include <stddef.h>
 
 #include <vector>
 
@@ skipping 256 matching lines @@
   DCHECK(accumulated_clip.is_clipped);
   return accumulated_clip.clip_rect;
 }
 
 void CalculateClipRects(const std::vector<LayerImpl*>& visible_layer_list,
                         const PropertyTrees* property_trees,
                         bool non_root_surfaces_enabled) {
   const ClipTree& clip_tree = property_trees->clip_tree;
   for (auto& layer : visible_layer_list) {
     const ClipNode* clip_node = clip_tree.Node(layer->clip_tree_index());
+    bool layer_needs_clip_rect =
+        non_root_surfaces_enabled
+            ? clip_node->layers_are_clipped
+            : clip_node->layers_are_clipped_when_surfaces_disabled;
+    if (!layer_needs_clip_rect) {
+      layer->set_clip_rect(gfx::Rect());
+      continue;
+    }
     if (!non_root_surfaces_enabled) {
       layer->set_clip_rect(
           gfx::ToEnclosingRect(clip_node->clip_in_target_space));
       continue;
     }
 
-    // When both the layer and the target are unclipped, the entire layer
-    // content rect is visible.
-    const bool fully_visible =
-        !clip_node->layers_are_clipped && !clip_node->target_is_clipped;

[weiliangc, 2016/11/02 21:08:07]

Could you explain why target_is_clipped is ok to be removed here?

[jaydasika, 2016/11/02 21:21:56]

if !non_root_surfaces_enabled = true, we wont reach here(line 288 in new code)

When non_root_surface_enabled,
if clip_node->layers_are_clipped = false, we wont reach here (line 283 in new
code).

So, here clip_node->layers_are_clipped = true always => fully_visible = false
always (irrespective of what target_is_clipped is)  

[weiliangc, 2016/11/02 21:34:28]

For !non_root_surface_enabled, old cold will set the root layer to
clip_in_target_space, and new code would set it to empty. This is expected?

For non_root_surface_enabled, and layers_are_clipped false, new code will set
clip rect to empty, and old code would set to empty or calculate the clip rect
depends on whether target_is_clipped? For target_is_clipped true case, does it
always ends being empty clip?

[jaydasika, 2016/11/02 21:46:40]

When layer is not clipped, clip rect is never used, so it doesn't matter what
its set to

[weiliangc, 2016/11/03 19:21:54]

Ah I see because !non_root_surface_enabled's root layer is not clipped, and
non_root_surface_enabled we use layers_are_clipped as signal to whether layer is
clipped.

Thanks for patient replies. :)

+    const TransformTree& transform_tree = property_trees->transform_tree;
+    const TransformNode* transform_node =
+        transform_tree.Node(layer->transform_tree_index());
+    int target_node_id = transform_tree.ContentTargetId(transform_node->id);
 
-    if (!fully_visible) {
-      const TransformTree& transform_tree = property_trees->transform_tree;
-      const TransformNode* transform_node =
-          transform_tree.Node(layer->transform_tree_index());
-      int target_node_id = transform_tree.ContentTargetId(transform_node->id);
+    // The clip node stores clip rect in its target space.
+    gfx::RectF clip_rect_in_target_space = clip_node->clip_in_target_space;
 
-      // The clip node stores clip rect in its target space.
-      gfx::RectF clip_rect_in_target_space = clip_node->clip_in_target_space;
+    // If required, this clip rect should be mapped to the current layer's
+    // target space.
+    if (clip_node->target_transform_id != target_node_id) {
+      // In this case, layer has a clip parent or scroll parent (or shares the
+      // target with an ancestor layer that has clip parent) and the clip
+      // parent's target is different from the layer's target. As the layer's
+      // target has unclippped descendants, it is unclippped.
+      if (!clip_node->layers_are_clipped)
+        continue;
 
-      // If required, this clip rect should be mapped to the current layer's
-      // target space.
-      if (clip_node->target_transform_id != target_node_id) {
-        // In this case, layer has a clip parent or scroll parent (or shares the
-        // target with an ancestor layer that has clip parent) and the clip
-        // parent's target is different from the layer's target. As the layer's
-        // target has unclippped descendants, it is unclippped.
-        if (!clip_node->layers_are_clipped)
-          continue;
+      // Compute the clip rect in target space and store it.
+      bool for_visible_rect_calculation = false;
+      if (!ComputeClipRectInTargetSpace(
+              layer, clip_node, property_trees, target_node_id,
+              for_visible_rect_calculation, &clip_rect_in_target_space))
+        continue;
+    }
 
-        // Compute the clip rect in target space and store it.
-        bool for_visible_rect_calculation = false;
-        if (!ComputeClipRectInTargetSpace(
-                layer, clip_node, property_trees, target_node_id,
-                for_visible_rect_calculation, &clip_rect_in_target_space))
-          continue;
-      }
-
-      if (!clip_rect_in_target_space.IsEmpty()) {
-        layer->set_clip_rect(gfx::ToEnclosingRect(clip_rect_in_target_space));
-      } else {
-        layer->set_clip_rect(gfx::Rect());
-      }
+    if (!clip_rect_in_target_space.IsEmpty()) {
+      layer->set_clip_rect(gfx::ToEnclosingRect(clip_rect_in_target_space));
+    } else {
+      layer->set_clip_rect(gfx::Rect());
     }
   }
 }
 
 void CalculateVisibleRects(const LayerImplList& visible_layer_list,
                            const PropertyTrees* property_trees,
                            bool non_root_surfaces_enabled) {
   const EffectTree& effect_tree = property_trees->effect_tree;
   const TransformTree& transform_tree = property_trees->transform_tree;
   const ClipTree& clip_tree = property_trees->clip_tree;
@@ skipping 619 matching lines @@
           << " v.s. accumulated clip: "
           << gfx::ToEnclosingRect(accumulated_clip).ToString();
     }
   }
 }
 
 static void ComputeLayerClipRect(const PropertyTrees* property_trees,
                                  const LayerImpl* layer) {
   const EffectTree* effect_tree = &property_trees->effect_tree;
   const ClipTree* clip_tree = &property_trees->clip_tree;
+  const ClipNode* clip_node = clip_tree->Node(layer->clip_tree_index());
   const EffectNode* effect_node = effect_tree->Node(layer->effect_tree_index());
   const EffectNode* target_node =
       effect_node->has_render_surface
           ? effect_node
           : effect_tree->Node(effect_node->target_id);
   // TODO(weiliangc): When effect node has up to date render surface info on
   // compositor thread, no need to check for resourceless draw mode
   if (!property_trees->non_root_surfaces_enabled) {
     target_node = effect_tree->Node(1);
   }
 
   bool include_viewport_clip = false;
   ConditionalClip accumulated_clip_rect =
       ComputeAccumulatedClip(property_trees, include_viewport_clip,
                              layer->clip_tree_index(), target_node->id);
 
+  bool is_clipped_from_clip_tree =
+      property_trees->non_root_surfaces_enabled
+          ? clip_node->layers_are_clipped
+          : clip_node->layers_are_clipped_when_surfaces_disabled;
+  DCHECK_EQ(is_clipped_from_clip_tree, accumulated_clip_rect.is_clipped);
+
   gfx::RectF accumulated_clip = accumulated_clip_rect.clip_rect;
 
-  if ((!property_trees->non_root_surfaces_enabled &&
-       clip_tree->Node(layer->clip_tree_index())
-           ->layers_are_clipped_when_surfaces_disabled) ||
-      clip_tree->Node(layer->clip_tree_index())->layers_are_clipped) {
-    DCHECK(layer->clip_rect() == gfx::ToEnclosingRect(accumulated_clip))
-        << " layer: " << layer->id() << " clip id: " << layer->clip_tree_index()
-        << " layer clip: " << layer->clip_rect().ToString() << " v.s. "
-        << gfx::ToEnclosingRect(accumulated_clip).ToString()
-        << " and clip node clip: "
-        << gfx::ToEnclosingRect(
-               clip_tree->Node(layer->clip_tree_index())->clip_in_target_space)
-               .ToString();
-  }
+  DCHECK(layer->clip_rect() == gfx::ToEnclosingRect(accumulated_clip))
+      << " layer: " << layer->id() << " clip id: " << layer->clip_tree_index()
+      << " layer clip: " << layer->clip_rect().ToString() << " v.s. "
+      << gfx::ToEnclosingRect(accumulated_clip).ToString()
+      << " and clip node clip: "
+      << gfx::ToEnclosingRect(clip_node->clip_in_target_space).ToString();
 }
 
 static void ComputeVisibleRectsInternal(
     LayerImpl* root_layer,
     PropertyTrees* property_trees,
     bool can_render_to_separate_surface,
     std::vector<LayerImpl*>* visible_layer_list) {
   if (property_trees->non_root_surfaces_enabled !=
       can_render_to_separate_surface) {
     property_trees->non_root_surfaces_enabled = can_render_to_separate_surface;
@@ skipping 467 matching lines @@
 void UpdateElasticOverscroll(PropertyTrees* property_trees,
                              const Layer* overscroll_elasticity_layer,
                              const gfx::Vector2dF& elastic_overscroll) {
   UpdateElasticOverscrollInternal(property_trees, overscroll_elasticity_layer,
                                   elastic_overscroll);
 }
 
 }  // namespace draw_property_utils
 
 }  // namespace cc