cc: Include viewport clip in visible rect of clip children

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 71 matching lines @@
     const EffectTree& effect_tree) {
   const EffectNode* effect_node = effect_tree.Node(effect_tree_index);
   return effect_node->render_surface ? effect_node
                                      : effect_tree.Node(effect_node->target_id);
 }
 
 bool ComputeClipRectInTargetSpace(const LayerImpl* layer,
                                   const ClipNode* clip_node,
                                   const PropertyTrees* property_trees,
                                   int target_node_id,
-                                  gfx::RectF* clip_rect_in_target_space) {
+                                  gfx::RectF* clip_rect_in_target_space,
+                                  bool use_combined_clip) {

[weiliangc, 2016/09/13 15:44:14]

clip_rect_in_target_space is output of this function. We should put output as
the last parameter, and use_combined_clip goes before that.

[jaydasika, 2016/09/13 17:17:56]

Done.

   DCHECK(layer->clip_tree_index() == clip_node->id);
   DCHECK(clip_node->target_transform_id != target_node_id);
 
   const EffectTree& effect_tree = property_trees->effect_tree;
   const EffectNode* target_effect_node =
       ContentsTargetEffectNode(layer->effect_tree_index(), effect_tree);
   gfx::Transform clip_to_target;
+  gfx::RectF clip_from_clip_node =
+      use_combined_clip ? clip_node->combined_clip_in_target_space
+                        : clip_node->clip_in_target_space;
+
   if (clip_node->target_transform_id > target_node_id) {
     // In this case, layer has a scroll parent. We need to keep the scale
     // at the layer's target but remove the scale at the scroll parent's
     // target.
     if (property_trees->ComputeTransformToTarget(clip_node->target_transform_id,
                                                  target_effect_node->id,
                                                  &clip_to_target)) {
       // We don't have to apply surface contents scale when target is root.
       if (target_effect_node->id != EffectTree::kContentsRootNodeId) {
         PostConcatSurfaceContentsScale(target_effect_node, &clip_to_target);
 #if DCHECK_IS_ON()
         const TransformTree& transform_tree = property_trees->transform_tree;
         VerifySurfaceContentsScalesMatch(layer->effect_tree_index(),
                                          target_node_id, effect_tree,
                                          transform_tree);
 #endif
       }
 
       const EffectNode* source_node =
           effect_tree.Node(clip_node->target_effect_id);
       ConcatInverseSurfaceContentsScale(source_node, &clip_to_target);
 #if DCHECK_IS_ON()
       const TransformTree& transform_tree = property_trees->transform_tree;
       VerifySurfaceContentsScalesMatch(clip_node->target_effect_id,
                                        clip_node->target_transform_id,
                                        effect_tree, transform_tree);
 #endif
-      *clip_rect_in_target_space = MathUtil::MapClippedRect(
-          clip_to_target, clip_node->clip_in_target_space);
+      *clip_rect_in_target_space =
+          MathUtil::MapClippedRect(clip_to_target, clip_from_clip_node);
     } else {
       return false;
     }
   } else {
     if (property_trees->ComputeTransformFromTarget(
             target_node_id, clip_node->target_effect_id, &clip_to_target)) {
-      *clip_rect_in_target_space = MathUtil::ProjectClippedRect(
-          clip_to_target, clip_node->clip_in_target_space);
+      *clip_rect_in_target_space =
+          MathUtil::ProjectClippedRect(clip_to_target, clip_from_clip_node);
     } else {
       return false;
     }
   }
   return true;
 }
 
 struct ConditionalClip {
   bool is_clipped;
   gfx::RectF clip_rect;
@@ skipping 180 matching lines @@
       // 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.
-        if (!ComputeClipRectInTargetSpace(layer, clip_node, property_trees,
-                                          target_node_id,
-                                          &clip_rect_in_target_space))
+        // Compute the clip rect in target space and store it. We don't use the
+        // combined clip for calculating clip rects as we don't want to reapply
+        // clips.
+        bool use_combined_clip = false;

[weiliangc, 2016/09/13 15:44:14]

nit: I think this name might not be easy to understand for people who are not
familiar with the code. How about renaming it to for_visible_rect_calculation or
something like that, since it is used for saying whether we are using the helper
function to calculate visible rect or clip rect. And inside the
ComputeClipRectInTargetSpace function, we can say for visible rect calculation
we use combined_clip?

[jaydasika, 2016/09/13 17:17:56]

Done.

+        if (!ComputeClipRectInTargetSpace(
+                layer, clip_node, property_trees, target_node_id,
+                &clip_rect_in_target_space, use_combined_clip))
           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());
       }
     }
   }
 }
 
-bool GetLayerClipRect(const LayerImpl* layer,
-                      const ClipNode* clip_node,
-                      const PropertyTrees* property_trees,
-                      int target_node_id,
-                      gfx::RectF* clip_rect_in_target_space) {
-  // This is equivalent of calling ComputeClipRectInTargetSpace.
-  *clip_rect_in_target_space = gfx::RectF(layer->clip_rect());
-  return property_trees->transform_tree.Node(target_node_id)
-      ->ancestors_are_invertible;
-}
-
 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;
   for (auto& layer : visible_layer_list) {
     gfx::Size layer_bounds = layer->bounds();
 
     int effect_ancestor_with_copy_request =
@@ skipping 75 matching lines @@
     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) {
         layer->set_visible_layer_rect(gfx::Rect(layer_bounds));
         continue;
       }
 
-      // We use the clip node's clip_in_target_space (and not
-      // combined_clip_in_target_space) here because we want to clip
-      // with respect to clip parent's local clip and not its combined clip as
-      // the combined clip has even the clip parent's target's clip baked into
-      // it and as our target is different, we don't want to use it in our
-      // visible rect computation.
-      if (!GetLayerClipRect(layer, clip_node, property_trees, target_node_id,
-                            &combined_clip_rect_in_target_space)) {
+      bool use_combined_clip = true;
+      if (!ComputeClipRectInTargetSpace(
+              layer, clip_node, property_trees, target_node_id,
+              &combined_clip_rect_in_target_space, use_combined_clip)) {
         layer->set_visible_layer_rect(gfx::Rect(layer_bounds));
         continue;
       }
     } else {
       combined_clip_rect_in_target_space =
           clip_node->combined_clip_in_target_space;
     }
 
     // The clip rect should be intersected with layer rect in target space.
     gfx::Transform content_to_target = transform_tree.ToTarget(
@@ skipping 1114 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