cc : Clean up cc clip tree

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 a591845f38b5c02e7e5bde6a6c134e076acbc110..e53859720daca7e7a7019de0fb6f5a002f691834 100644
--- a/cc/trees/draw_property_utils.cc
+++ b/cc/trees/draw_property_utils.cc
@@ -34,15 +34,6 @@ static bool IsRootLayer(const LayerImpl* layer) {
   return layer->layer_tree_impl()->IsRootLayer(layer);
 }
 
-static const EffectNode* ContentsTargetEffectNode(
-    const int effect_tree_index,
-    const EffectTree& effect_tree) {
-  const EffectNode* effect_node = effect_tree.Node(effect_tree_index);
-  return effect_tree.GetRenderSurface(effect_tree_index)
-             ? effect_node
-             : effect_tree.Node(effect_node->target_id);
-}
-
 static bool ConvertRectBetweenSurfaceSpaces(const PropertyTrees* property_trees,
                                             int source_effect_id,
                                             int dest_effect_id,
@@ -77,35 +68,6 @@ static bool ConvertRectBetweenSurfaceSpaces(const PropertyTrees* property_trees,
   return true;
 }
 
-bool ComputeClipRectInTargetSpace(const LayerImpl* layer,
-                                  const ClipNode* clip_node,
-                                  const PropertyTrees* property_trees,
-                                  int target_node_id,
-                                  bool for_visible_rect_calculation,
-                                  gfx::RectF* clip_rect_in_target_space) {
-  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;
-  // We use the local clip for clip rect calculation and combined clip for
-  // visible rect calculation.
-  gfx::RectF clip_from_clip_node =
-      for_visible_rect_calculation ? clip_node->combined_clip_in_target_space
-                                   : clip_node->clip_in_target_space;
-
-  return ConvertRectBetweenSurfaceSpaces(
-      property_trees, clip_node->target_effect_id, target_effect_node->id,
-      clip_from_clip_node, clip_rect_in_target_space);
-}
-
-struct ConditionalClip {
-  bool is_clipped;
-  gfx::RectF clip_rect;
-};
-
 static ConditionalClip ComputeTargetRectInLocalSpace(
     gfx::RectF rect,
     const PropertyTrees* property_trees,
@@ -113,11 +75,18 @@ static ConditionalClip ComputeTargetRectInLocalSpace(
     int local_transform_id,
     const int target_effect_id) {
   gfx::Transform target_to_local;
-  bool success = property_trees->GetFromTarget(
-      local_transform_id, target_effect_id, &target_to_local);
-  if (!success)
-    // If transform is not invertible, cannot apply clip.
-    return ConditionalClip{false, gfx::RectF()};
+  const TransformNode* node =
+      property_trees->transform_tree.Node(local_transform_id);
+  if (target_transform_id == TransformTree::kRootNodeId &&
+      node->ancestors_are_invertible) {
+    target_to_local = property_trees->transform_tree.FromScreen(node->id);
+  } else {
+    bool success = property_trees->GetFromTarget(
+        local_transform_id, target_effect_id, &target_to_local);
+    if (!success)
+      // If transform is not invertible, cannot apply clip.
+      return ConditionalClip{false, gfx::RectF()};
+  }
 
   if (target_transform_id > local_transform_id)
     return ConditionalClip{true,  // is_clipped.
@@ -134,8 +103,11 @@ static ConditionalClip ComputeLocalRectInTargetSpace(
     int target_transform_id,
     int target_effect_id) {
   gfx::Transform current_to_target;
-  if (!property_trees->GetToTarget(current_transform_id, target_effect_id,
-                                   &current_to_target)) {
+  if (target_transform_id == TransformTree::kRootNodeId) {

[weiliangc, 2017/03/14 22:04:30]

Is it possible to make this logic part of GetToTarget(...)?

[jaydasika, 2017/03/16 20:33:31]

Ah, that makes sense, I added this to speed up transform computation. Adding
this it GetTo(From)Target should speed up other places where we calculate target
transforms to screen.

+    current_to_target =
+        property_trees->transform_tree.ToScreen(current_transform_id);
+  } else if (!property_trees->GetToTarget(
+                 current_transform_id, target_effect_id, &current_to_target)) {
     // If transform is not invertible, cannot apply clip.
     return ConditionalClip{false, gfx::RectF()};
   }
@@ -218,30 +190,35 @@ static bool ApplyClipNodeToAccumulatedClip(const PropertyTrees* property_trees,
         return false;
       return true;
     }
-    case ClipNode::ClipType::NONE:
-      return true;
   }
   NOTREACHED();
   return true;
 }
 
-static ConditionalClip ComputeAccumulatedClip(
-    const PropertyTrees* property_trees,
-    bool include_viewport_clip,
-    bool include_expanding_clips,
-    int local_clip_id,
-    int target_id) {
-  DCHECK(!include_viewport_clip ||
-         target_id == EffectTree::kContentsRootNodeId);
-  const ClipTree& clip_tree = property_trees->clip_tree;
-  const EffectTree& effect_tree = property_trees->effect_tree;
+static ConditionalClip ComputeAccumulatedClip(PropertyTrees* property_trees,
+                                              bool include_expanding_clips,
+                                              int local_clip_id,
+                                              int target_id) {
+  ClipRectData& cached_data =
+      property_trees->FetchClipRectFromCache(local_clip_id, target_id);
+  if (cached_data.target_id != EffectTree::kInvalidNodeId) {
+    // Cache hit
+    return cached_data.clip;
+  }
+  cached_data.target_id = target_id;
 
+  const ClipTree& clip_tree = property_trees->clip_tree;
   const ClipNode* clip_node = clip_tree.Node(local_clip_id);
+  const EffectTree& effect_tree = property_trees->effect_tree;
   const EffectNode* target_node = effect_tree.Node(target_id);
   int target_transform_id = target_node->transform_id;
 
+  bool cache_hit = false;
+  ConditionalClip cached_clip = ConditionalClip{false, gfx::RectF()};
+  ConditionalClip unclipped = ConditionalClip{false, gfx::RectF()};
+
   // Collect all the clips that need to be accumulated.
-  std::stack<const ClipNode*> parent_chain;
+  std::stack<const ClipNode*, std::vector<const ClipNode*>> parent_chain;
 
   // If target is not direct ancestor of clip, this will find least common
   // ancestor between the target and the clip.
@@ -250,303 +227,86 @@ static ConditionalClip ComputeAccumulatedClip(
     target_node = effect_tree.Node(target_node->target_id);
   }
 
-  // Collect clip nodes up to the least common ancestor.
+  // Collect clip nodes up to the least common ancestor or till we get a cache
+  // hit.
   while (target_node->clip_id < clip_node->id) {
+    if (parent_chain.size() > 0) {
+      // Search the cache.
+      for (auto& data : clip_node->cached_clip_rects) {
+        if (data.target_id == target_id) {
+          cache_hit = true;
+          cached_clip = data.clip;
+        }
+      }
+    }
     parent_chain.push(clip_node);
     clip_node = clip_tree.parent(clip_node);
   }
-  DCHECK_EQ(target_node->clip_id, clip_node->id);
-
-  if (!include_viewport_clip && parent_chain.size() == 0) {
-    // There aren't any clips to apply.
-    return ConditionalClip{false, gfx::RectF()};
-  }
 
-  if (!include_viewport_clip) {
-    clip_node = parent_chain.top();
-    parent_chain.pop();
+  if (parent_chain.size() == 0) {
+    // No accumulated clip nodes.
+    cached_data.clip = unclipped;
+    return unclipped;
   }
 
-  // Find the first clip in the chain that we need to apply.
-  while (clip_node->clip_type != ClipNode::ClipType::APPLIES_LOCAL_CLIP &&
-         parent_chain.size() > 0) {
-    clip_node = parent_chain.top();
-    parent_chain.pop();
-  }
+  clip_node = parent_chain.top();
+  parent_chain.pop();
 
-  if (clip_node->clip_type != ClipNode::ClipType::APPLIES_LOCAL_CLIP) {
-    // No clip node applying clip in between.
-    return ConditionalClip{false, gfx::RectF()};
-  }
-
-  ConditionalClip current_clip = ComputeCurrentClip(
-      clip_node, property_trees, target_transform_id, target_id);
-
-  // If transform is not invertible, no clip will be applied.
-  if (!current_clip.is_clipped)
-    return ConditionalClip{false, gfx::RectF()};
-  gfx::RectF accumulated_clip = current_clip.clip_rect;
-
-  while (parent_chain.size() > 0) {
-    clip_node = parent_chain.top();
-    parent_chain.pop();
+  gfx::RectF accumulated_clip;
+  if (cache_hit && cached_clip.is_clipped) {
+    // Apply the first clip in parent_chain to the cached clip.
+    accumulated_clip = cached_clip.clip_rect;
     bool success = ApplyClipNodeToAccumulatedClip(
         property_trees, include_expanding_clips, target_id, target_transform_id,
         clip_node, &accumulated_clip);
-
-    // Failure to apply the clip means we encountered an uninvertible transform,
-    // so no clip will be applied.
-    if (!success)
-      return ConditionalClip{false /* is_clipped */, gfx::RectF()};
-  }
-
-  return ConditionalClip{true /* is_clipped */, accumulated_clip.IsEmpty()
-                                                    ? gfx::RectF()
-                                                    : accumulated_clip};
-}
-
-static gfx::RectF ComputeAccumulatedClipInRootSpaceForVisibleRect(
-    const PropertyTrees* property_trees,
-    int local_clip_id) {
-  const int root_effect_id = EffectTree::kContentsRootNodeId;
-  bool include_viewport_clip = true;
-  bool include_expanding_clips = true;
-  ConditionalClip accumulated_clip = ComputeAccumulatedClip(
-      property_trees, include_viewport_clip, include_expanding_clips,
-      local_clip_id, root_effect_id);
-  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 (!success) {
+      // Singular transform
+      cached_data.clip = unclipped;
+      return unclipped;
     }
-    if (!non_root_surfaces_enabled) {
-      layer->set_clip_rect(
-          gfx::ToEnclosingRect(clip_node->clip_in_target_space));
-      continue;
+  } else {
+    // No cache hit or the cached clip has no clip to apply. We need to find
+    // the first clip that applies clip as there is no clip to expand.
+    while (clip_node->clip_type != ClipNode::ClipType::APPLIES_LOCAL_CLIP &&
+           parent_chain.size() > 0) {
+      clip_node = parent_chain.top();
+      parent_chain.pop();
     }
 
-    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;
-
-    // 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;
+    if (clip_node->clip_type != ClipNode::ClipType::APPLIES_LOCAL_CLIP) {
+      // No clip to apply.
+      cached_data.clip = unclipped;
+      return unclipped;
     }
-
-    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());
+    ConditionalClip current_clip = ComputeCurrentClip(
+        clip_node, property_trees, target_transform_id, target_id);
+    if (!current_clip.is_clipped) {
+      // Singular transform
+      cached_data.clip = unclipped;
+      return unclipped;
     }
+    accumulated_clip = current_clip.clip_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;
-  for (auto* layer : visible_layer_list) {
-    gfx::Size layer_bounds = layer->bounds();
-
-    int effect_ancestor_with_copy_request =
-        effect_tree.ClosestAncestorWithCopyRequest(layer->effect_tree_index());
-    if (effect_ancestor_with_copy_request > EffectTree::kContentsRootNodeId) {
-      // Non root copy request.
-      bool include_viewport_clip = false;
-      bool include_expanding_clips = true;
-      ConditionalClip accumulated_clip_rect = ComputeAccumulatedClip(
-          property_trees, include_viewport_clip, include_expanding_clips,
-          layer->clip_tree_index(), effect_ancestor_with_copy_request);
-      if (!accumulated_clip_rect.is_clipped) {
-        layer->set_visible_layer_rect(gfx::Rect(layer_bounds));
-        continue;
-      }
-
-      gfx::RectF accumulated_clip_in_copy_request_space =
-          accumulated_clip_rect.clip_rect;
-
-      const EffectNode* copy_request_effect_node =
-          effect_tree.Node(effect_ancestor_with_copy_request);
-      ConditionalClip clip_in_layer_space = ComputeTargetRectInLocalSpace(
-          accumulated_clip_in_copy_request_space, property_trees,
-          copy_request_effect_node->transform_id, layer->transform_tree_index(),
-          copy_request_effect_node->id);
-
-      if (clip_in_layer_space.is_clipped) {
-        gfx::RectF clip_rect = clip_in_layer_space.clip_rect;
-        clip_rect.Offset(-layer->offset_to_transform_parent());
-        gfx::Rect visible_rect = gfx::ToEnclosingRect(clip_rect);
-        visible_rect.Intersect(gfx::Rect(layer_bounds));
-        layer->set_visible_layer_rect(visible_rect);
-      } else {
-        layer->set_visible_layer_rect(gfx::Rect(layer_bounds));
-      }
-      continue;
-    }
-
-    const ClipNode* clip_node = clip_tree.Node(layer->clip_tree_index());
-    const TransformNode* transform_node =
-        transform_tree.Node(layer->transform_tree_index());
-    if (!non_root_surfaces_enabled) {
-      // When we only have a root surface, the clip node and the layer must
-      // necessarily have the same target (the root).
-      if (transform_node->ancestors_are_invertible) {
-        gfx::RectF combined_clip_rect_in_target_space =
-            clip_node->combined_clip_in_target_space;
-        gfx::Transform target_to_content;
-        target_to_content.Translate(-layer->offset_to_transform_parent().x(),
-                                    -layer->offset_to_transform_parent().y());
-        target_to_content.PreconcatTransform(
-            transform_tree.FromScreen(transform_node->id));
-
-        gfx::Rect visible_rect =
-            gfx::ToEnclosingRect(MathUtil::ProjectClippedRect(
-                target_to_content, combined_clip_rect_in_target_space));
-        visible_rect.Intersect(gfx::Rect(layer_bounds));
-        layer->set_visible_layer_rect(visible_rect);
-      } else {
-        layer->set_visible_layer_rect(gfx::Rect(layer_bounds));
-      }
-      continue;
-    }
-
-    // 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 &&
-        !effect_tree.Node(clip_node->target_effect_id)->surface_is_clipped;
-
-    if (fully_visible) {
-      if (!transform_node->ancestors_are_invertible) {
-        // An animated singular transform may become non-singular during the
-        // animation, so we still need to compute a visible rect. In this
-        // situation, we treat the entire layer as visible.
-        layer->set_visible_layer_rect(gfx::Rect(layer_bounds));
-      } else {
-        gfx::Transform from_screen;
-        from_screen.Translate(-layer->offset_to_transform_parent().x(),
-                              -layer->offset_to_transform_parent().y());
-        from_screen.PreconcatTransform(
-            property_trees->transform_tree.FromScreen(transform_node->id));
-        gfx::Rect visible_rect =
-            gfx::ToEnclosingRect(MathUtil::ProjectClippedRect(
-                from_screen, property_trees->clip_tree.ViewportClip()));
-        visible_rect.Intersect(gfx::Rect(layer_bounds));
-        layer->set_visible_layer_rect(visible_rect);
-      }
-      continue;
-    }
-
-    int target_node_id = transform_tree.ContentTargetId(transform_node->id);
-
-    // The clip node stores clip rect in its target space. If required,
-    // this clip rect should be mapped to the current layer's target space.
-    gfx::RectF combined_clip_rect_in_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) {
-        layer->set_visible_layer_rect(gfx::Rect(layer_bounds));
-        continue;
-      }
-
-      bool for_visible_rect_calculation = true;
-      if (!ComputeClipRectInTargetSpace(layer, clip_node, property_trees,
-                                        target_node_id,
-                                        for_visible_rect_calculation,
-                                        &combined_clip_rect_in_target_space)) {
-        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;
-    property_trees->GetToTarget(transform_node->id,
-                                layer->render_target_effect_tree_index(),
-                                &content_to_target);
-    content_to_target.Translate(layer->offset_to_transform_parent().x(),
-                                layer->offset_to_transform_parent().y());
-    gfx::Rect layer_content_rect = gfx::Rect(layer_bounds);
-    gfx::RectF layer_content_bounds_in_target_space = MathUtil::MapClippedRect(
-        content_to_target, gfx::RectF(layer_content_rect));
-    // If the layer is fully contained within the clip, treat it as fully
-    // visible.
-    if (!layer_content_bounds_in_target_space.IsEmpty() &&
-        combined_clip_rect_in_target_space.Contains(
-            layer_content_bounds_in_target_space)) {
-      layer->set_visible_layer_rect(gfx::Rect(layer_bounds));
-      continue;
-    }
-
-    combined_clip_rect_in_target_space.Intersect(
-        layer_content_bounds_in_target_space);
-    if (combined_clip_rect_in_target_space.IsEmpty()) {
-      layer->set_visible_layer_rect(gfx::Rect());
-      continue;
-    }
 
-    gfx::Transform target_to_layer;
-    const EffectNode* target_effect_node =
-        ContentsTargetEffectNode(layer->effect_tree_index(), effect_tree);
-    bool success = property_trees->GetFromTarget(
-        transform_node->id, target_effect_node->id, &target_to_layer);
+  // Apply remaining clips
+  while (parent_chain.size() > 0) {
+    clip_node = parent_chain.top();
+    parent_chain.pop();
+    bool success = ApplyClipNodeToAccumulatedClip(
+        property_trees, include_expanding_clips, target_id, target_transform_id,
+        clip_node, &accumulated_clip);
     if (!success) {
-      // An animated singular transform may become non-singular during the
-      // animation, so we still need to compute a visible rect. In this
-      // situation, we treat the entire layer as visible.
-      layer->set_visible_layer_rect(gfx::Rect(layer_bounds));
-      continue;
+      // Singular transform
+      cached_data.clip = unclipped;
+      return unclipped;
     }
-    gfx::Transform target_to_content;
-    target_to_content.Translate(-layer->offset_to_transform_parent().x(),
-                                -layer->offset_to_transform_parent().y());
-    target_to_content.PreconcatTransform(target_to_layer);
-
-    gfx::Rect visible_rect = gfx::ToEnclosingRect(MathUtil::ProjectClippedRect(
-        target_to_content, combined_clip_rect_in_target_space));
-    visible_rect.Intersect(gfx::Rect(layer_bounds));
-    layer->set_visible_layer_rect(visible_rect);
   }
+
+  ConditionalClip clip = ConditionalClip{
+      true /* is_clipped */,
+      accumulated_clip.IsEmpty() ? gfx::RectF() : accumulated_clip};
+  cached_data.clip = clip;
+  return clip;
 }
 
 static bool HasSingularTransform(int transform_tree_index,
@@ -653,24 +413,7 @@ static bool LayerNeedsUpdateInternal(LayerType* layer,
   return true;
 }
 
-void FindLayersThatNeedUpdates(LayerTreeImpl* layer_tree_impl,
-                               const PropertyTrees* property_trees,
-                               std::vector<LayerImpl*>* visible_layer_list) {
-  const TransformTree& transform_tree = property_trees->transform_tree;
-  const EffectTree& effect_tree = property_trees->effect_tree;
 
-  for (auto* layer_impl : *layer_tree_impl) {
-    if (!IsRootLayer(layer_impl) &&
-        LayerShouldBeSkipped(layer_impl, transform_tree, effect_tree))
-      continue;
-
-    bool layer_is_drawn =
-        effect_tree.Node(layer_impl->effect_tree_index())->is_drawn;
-
-    if (LayerNeedsUpdate(layer_impl, layer_is_drawn, property_trees))
-      visible_layer_list->push_back(layer_impl);
-  }
-}
 
 }  // namespace
 
@@ -730,10 +473,23 @@ void FindLayersThatNeedUpdates(LayerTreeHost* layer_tree_host,
   }
 }
 
-static void ResetIfHasNanCoordinate(gfx::RectF* rect) {
-  if (std::isnan(rect->x()) || std::isnan(rect->y()) ||
-      std::isnan(rect->right()) || std::isnan(rect->bottom()))
-    *rect = gfx::RectF();
+void FindLayersThatNeedUpdates(LayerTreeImpl* layer_tree_impl,
+                               const PropertyTrees* property_trees,
+                               std::vector<LayerImpl*>* visible_layer_list) {
+  const TransformTree& transform_tree = property_trees->transform_tree;
+  const EffectTree& effect_tree = property_trees->effect_tree;
+
+  for (auto* layer_impl : *layer_tree_impl) {
+    if (!IsRootLayer(layer_impl) &&
+        LayerShouldBeSkipped(layer_impl, transform_tree, effect_tree))
+      continue;
+
+    bool layer_is_drawn =
+        effect_tree.Node(layer_impl->effect_tree_index())->is_drawn;
+
+    if (LayerNeedsUpdate(layer_impl, layer_is_drawn, property_trees))
+      visible_layer_list->push_back(layer_impl);
+  }
 }
 
 void PostConcatSurfaceContentsScale(const EffectNode* effect_node,
@@ -757,158 +513,6 @@ void ConcatInverseSurfaceContentsScale(const EffectNode* effect_node,
                      1.0 / effect_node->surface_contents_scale.y());
 }
 
-void ComputeClips(PropertyTrees* property_trees,
-                  bool non_root_surfaces_enabled) {
-  ClipTree* clip_tree = &property_trees->clip_tree;
-  if (!clip_tree->needs_update())
-    return;
-  for (int i = ClipTree::kViewportNodeId;
-       i < static_cast<int>(clip_tree->size()); ++i) {
-    ClipNode* clip_node = clip_tree->Node(i);
-
-    if (clip_node->id == ClipTree::kViewportNodeId) {
-      ResetIfHasNanCoordinate(&clip_node->clip);
-      clip_node->clip_in_target_space = clip_node->clip;
-      clip_node->combined_clip_in_target_space = clip_node->clip;
-      continue;
-    }
-    const TransformTree& transform_tree = property_trees->transform_tree;
-    const EffectTree& effect_tree = property_trees->effect_tree;
-    const TransformNode* transform_node =
-        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);
-    bool success = true;
-
-    // Clips must be combined in target space. We cannot, for example, combine
-    // clips in the space of the child clip. The reason is non-affine
-    // transforms. Say we have the following tree T->A->B->C, and B clips C, but
-    // draw into target T. It may be the case that A applies a perspective
-    // transform, and B and C are at different z positions. When projected into
-    // target space, the relative sizes and positions of B and C can shift.
-    // Since it's the relationship in target space that matters, that's where we
-    // must combine clips. For each clip node, we save the clip rects in its
-    // target space. So, we need to get the ancestor clip rect in the current
-    // clip node's target space.
-    gfx::RectF parent_combined_clip_in_target_space =
-        parent_clip_node->combined_clip_in_target_space;
-    gfx::RectF parent_clip_in_target_space =
-        parent_clip_node->clip_in_target_space;
-    if (parent_target_transform_node &&
-        parent_target_transform_node->id != clip_node->target_transform_id &&
-        non_root_surfaces_enabled) {
-      success &= ConvertRectBetweenSurfaceSpaces(
-          property_trees, parent_clip_node->target_effect_id,
-          clip_node->target_effect_id,
-          parent_clip_node->combined_clip_in_target_space,
-          &parent_combined_clip_in_target_space);
-      // If we can't compute a transform, it's because we had to use the inverse
-      // of a singular transform. We won't draw in this case, so there's no need
-      // to compute clips.
-      if (!success)
-        continue;
-      if (clip_node->clip_type == ClipNode::ClipType::EXPANDS_CLIP) {
-        parent_combined_clip_in_target_space =
-            gfx::RectF(clip_node->clip_expander->MapRectReverse(
-                gfx::ToEnclosingRect(parent_combined_clip_in_target_space),
-                property_trees));
-      }
-      ConvertRectBetweenSurfaceSpaces(
-          property_trees, parent_clip_node->target_effect_id,
-          clip_node->target_effect_id, parent_clip_node->clip_in_target_space,
-          &parent_clip_in_target_space);
-    }
-    // Only nodes affected by ancestor clips will have their clip adjusted due
-    // to intersecting with an ancestor clip. But, we still need to propagate
-    // the combined clip to our children because if they are clipped, they may
-    // need to clip using our parent clip and if we don't propagate it here,
-    // it will be lost.
-    if (clip_node->resets_clip && non_root_surfaces_enabled) {
-      if (clip_node->clip_type == ClipNode::ClipType::APPLIES_LOCAL_CLIP) {
-        gfx::Transform to_target;
-        property_trees->GetToTarget(clip_node->transform_id,
-                                    clip_node->target_effect_id, &to_target);
-        clip_node->clip_in_target_space =
-            MathUtil::MapClippedRect(to_target, clip_node->clip);
-        ResetIfHasNanCoordinate(&clip_node->clip_in_target_space);
-        clip_node->combined_clip_in_target_space =
-            gfx::IntersectRects(clip_node->clip_in_target_space,
-                                parent_combined_clip_in_target_space);
-      } else {
-        DCHECK(!target_is_clipped);
-        DCHECK(!clip_node->layers_are_clipped);
-        clip_node->combined_clip_in_target_space =
-            parent_combined_clip_in_target_space;
-      }
-      ResetIfHasNanCoordinate(&clip_node->combined_clip_in_target_space);
-      continue;
-    }
-    bool use_only_parent_clip =
-        clip_node->clip_type != ClipNode::ClipType::APPLIES_LOCAL_CLIP;
-    if (use_only_parent_clip) {
-      clip_node->combined_clip_in_target_space =
-          parent_combined_clip_in_target_space;
-      if (!non_root_surfaces_enabled) {
-        clip_node->clip_in_target_space =
-            parent_clip_node->clip_in_target_space;
-      } 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
-        // no clip. So, clip_in_target_space is not used and hence we can set
-        // it to an empty rect.
-        clip_node->clip_in_target_space = gfx::RectF();
-      }
-    } else {
-      gfx::Transform source_to_target;
-
-      if (!non_root_surfaces_enabled) {
-        source_to_target = transform_tree.ToScreen(clip_node->transform_id);
-      } else if (transform_tree.ContentTargetId(transform_node->id) ==
-                 clip_node->target_transform_id) {
-        property_trees->GetToTarget(clip_node->transform_id,
-                                    clip_node->target_effect_id,
-                                    &source_to_target);
-      } else {
-        success = property_trees->GetToTarget(
-            transform_node->id, clip_node->target_effect_id, &source_to_target);
-        // source_to_target computation should be successful as target is an
-        // ancestor of the transform node.
-        DCHECK(success);
-      }
-
-      gfx::RectF source_clip_in_target_space =
-          MathUtil::MapClippedRect(source_to_target, clip_node->clip);
-
-      // With surfaces disabled, the only case where we use only the local clip
-      // for layer clipping is the case where no non-viewport ancestor node
-      // applies a local clip.
-      bool layer_clipping_uses_only_local_clip =
-          non_root_surfaces_enabled
-              ? clip_node->layer_clipping_uses_only_local_clip
-              : !parent_clip_node->layers_are_clipped_when_surfaces_disabled;
-      if (!layer_clipping_uses_only_local_clip) {
-        clip_node->clip_in_target_space = gfx::IntersectRects(
-            parent_clip_in_target_space, source_clip_in_target_space);
-      } else {
-        clip_node->clip_in_target_space = source_clip_in_target_space;
-      }
-
-      clip_node->combined_clip_in_target_space = gfx::IntersectRects(
-          parent_combined_clip_in_target_space, source_clip_in_target_space);
-    }
-    ResetIfHasNanCoordinate(&clip_node->clip_in_target_space);
-    ResetIfHasNanCoordinate(&clip_node->combined_clip_in_target_space);
-  }
-  clip_tree->set_needs_update(false);
-}
-
 void ComputeTransforms(TransformTree* transform_tree) {
   if (!transform_tree->needs_update())
     return;
@@ -945,46 +549,38 @@ void ComputeEffects(EffectTree* effect_tree) {
   effect_tree->set_needs_update(false);
 }
 
-static void ComputeClipsWithEffectTree(PropertyTrees* property_trees) {
-  EffectTree* effect_tree = &property_trees->effect_tree;
-  const ClipTree* clip_tree = &property_trees->clip_tree;
-  EffectNode* root_effect_node =
-      effect_tree->Node(EffectTree::kContentsRootNodeId);
-  const RenderSurfaceImpl* root_render_surface =
-      effect_tree->GetRenderSurface(EffectTree::kContentsRootNodeId);
-  gfx::Rect root_clip =
-      gfx::ToEnclosingRect(clip_tree->Node(root_effect_node->clip_id)->clip);
-  if (root_render_surface->is_clipped())
-    DCHECK(root_clip == root_render_surface->clip_rect())
-        << "clip on root render surface: "
-        << root_render_surface->clip_rect().ToString()
-        << " v.s. root effect node's clip: " << root_clip.ToString();
-  for (int i = 2; i < static_cast<int>(effect_tree->size()); ++i) {
-    EffectNode* effect_node = effect_tree->Node(i);
-    const EffectNode* target_node = effect_tree->Node(effect_node->target_id);
-    bool include_viewport_clip = false;
-    bool include_expanding_clips = false;
-    ConditionalClip accumulated_clip_rect = ComputeAccumulatedClip(
-        property_trees, include_viewport_clip, include_expanding_clips,
-        effect_node->clip_id, target_node->id);
-    gfx::RectF accumulated_clip = accumulated_clip_rect.clip_rect;
-    const RenderSurfaceImpl* render_surface = effect_tree->GetRenderSurface(i);
-    if (render_surface && render_surface->is_clipped()) {
-      DCHECK(gfx::ToEnclosingRect(accumulated_clip) ==
-             render_surface->clip_rect())
-          << " render surface's clip rect: "
-          << render_surface->clip_rect().ToString()
-          << " v.s. accumulated clip: "
-          << gfx::ToEnclosingRect(accumulated_clip).ToString();
+void ComputeClips(PropertyTrees* property_trees) {
+  ClipTree* clip_tree = &property_trees->clip_tree;
+  if (!clip_tree->needs_update())
+    return;
+  const int target_effect_id = EffectTree::kContentsRootNodeId;
+  const int target_transform_id = TransformTree::kRootNodeId;
+  const bool include_expanding_clips = true;
+  for (int i = ClipTree::kViewportNodeId;
+       i < static_cast<int>(clip_tree->size()); ++i) {
+    ClipNode* clip_node = clip_tree->Node(i);
+    // Clear the clip rect cache
+    clip_node->cached_clip_rects = std::vector<ClipRectData>(1);
+    if (clip_node->id == ClipTree::kViewportNodeId) {
+      clip_node->accumulated_rect_in_screen_space = clip_node->clip;
+      continue;
     }
+    ClipNode* parent_clip_node = clip_tree->parent(clip_node);
+    DCHECK(parent_clip_node);
+    gfx::RectF accumulated_clip =
+        parent_clip_node->accumulated_rect_in_screen_space;
+    bool success = ApplyClipNodeToAccumulatedClip(
+        property_trees, include_expanding_clips, target_effect_id,
+        target_transform_id, clip_node, &accumulated_clip);
+    DCHECK(success);
+    clip_node->accumulated_rect_in_screen_space = accumulated_clip;

[weiliangc, 2017/03/14 22:04:30]

Have you tried not calculating acculated_rect_in_screen_space all the time, but
update it only when needed like cache? What is the performance like?

[jaydasika, 2017/03/16 20:33:31]

I tried that. The performance was almost same. I decided to go with this as the
code is much simpler here than in that approach (the other approach has a lot of
state maintenance for accumulating clips, very similar to how we accumulate
clips for clip rects).  

   }
+  clip_tree->set_needs_update(false);
 }
 
-static void ComputeLayerClipRect(const PropertyTrees* property_trees,
-                                 const LayerImpl* layer) {
+static ConditionalClip LayerClipRect(PropertyTrees* property_trees,
+                                     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
@@ -996,117 +592,13 @@ static void ComputeLayerClipRect(const PropertyTrees* property_trees,
     target_node = effect_tree->Node(1);
   }
 
-  bool include_viewport_clip = false;
   bool include_expanding_clips = false;
-  ConditionalClip accumulated_clip_rect = ComputeAccumulatedClip(
-      property_trees, include_viewport_clip, include_expanding_clips,
-      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;
-
-  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();
+  return ComputeAccumulatedClip(property_trees, include_expanding_clips,
+                                layer->clip_tree_index(), target_node->id);
 }
 
-void ComputeVisibleRects(LayerImpl* root_layer,
-                         PropertyTrees* property_trees,
-                         bool can_render_to_separate_surface,
-                         LayerImplList* visible_layer_list) {
-  bool render_surfaces_need_update = false;
-  if (property_trees->non_root_surfaces_enabled !=
-      can_render_to_separate_surface) {
-    property_trees->non_root_surfaces_enabled = can_render_to_separate_surface;
-    property_trees->transform_tree.set_needs_update(true);
-    render_surfaces_need_update = true;
-  }
-  if (property_trees->transform_tree.needs_update()) {
-    property_trees->clip_tree.set_needs_update(true);
-    property_trees->effect_tree.set_needs_update(true);
-  }
-
-  if (render_surfaces_need_update) {
-    property_trees->effect_tree.UpdateRenderSurfaces(
-        root_layer->layer_tree_impl(),
-        property_trees->non_root_surfaces_enabled);
-  }
-  UpdateRenderTarget(&property_trees->effect_tree,
-                     property_trees->non_root_surfaces_enabled);
-  ComputeTransforms(&property_trees->transform_tree);
-  // Computation of clips uses surface contents scale which is updated while
-  // computing effects. So, ComputeEffects should be before ComputeClips.
-  ComputeEffects(&property_trees->effect_tree);
-  ComputeClips(property_trees, can_render_to_separate_surface);
-
-  FindLayersThatNeedUpdates(root_layer->layer_tree_impl(), property_trees,
-                            visible_layer_list);
-  CalculateClipRects(*visible_layer_list, property_trees,
-                     can_render_to_separate_surface);
-  CalculateVisibleRects(*visible_layer_list, property_trees,
-                        can_render_to_separate_surface);
-}
-
-void UpdatePropertyTrees(PropertyTrees* property_trees,
-                         bool can_render_to_separate_surface) {
-  if (property_trees->non_root_surfaces_enabled !=
-      can_render_to_separate_surface) {
-    property_trees->non_root_surfaces_enabled = can_render_to_separate_surface;
-    property_trees->transform_tree.set_needs_update(true);
-  }
-  if (property_trees->transform_tree.needs_update()) {
-    property_trees->clip_tree.set_needs_update(true);
-    property_trees->effect_tree.set_needs_update(true);
-  }
-  ComputeTransforms(&property_trees->transform_tree);
-  // Computation of clips uses surface contents scale which is updated while
-  // computing effects. So, ComputeEffects should be before ComputeClips.
-  ComputeEffects(&property_trees->effect_tree);
-  ComputeClips(property_trees, can_render_to_separate_surface);
-}
-
-void BuildPropertyTreesAndComputeVisibleRects(
-    LayerImpl* root_layer,
-    const LayerImpl* page_scale_layer,
-    const LayerImpl* inner_viewport_scroll_layer,
-    const LayerImpl* outer_viewport_scroll_layer,
-    const LayerImpl* overscroll_elasticity_layer,
-    const gfx::Vector2dF& elastic_overscroll,
-    float page_scale_factor,
-    float device_scale_factor,
-    const gfx::Rect& viewport,
-    const gfx::Transform& device_transform,
-    bool can_render_to_separate_surface,
-    PropertyTrees* property_trees,
-    LayerImplList* visible_layer_list) {
-  PropertyTreeBuilder::BuildPropertyTrees(
-      root_layer, page_scale_layer, inner_viewport_scroll_layer,
-      outer_viewport_scroll_layer, overscroll_elasticity_layer,
-      elastic_overscroll, page_scale_factor, device_scale_factor, viewport,
-      device_transform, property_trees);
-  ComputeVisibleRects(root_layer, property_trees,
-                      can_render_to_separate_surface, visible_layer_list);
-}
-
-void VerifyClipTreeCalculations(const LayerImplList& layer_list,
-                                PropertyTrees* property_trees) {
-  if (property_trees->non_root_surfaces_enabled) {
-    ComputeClipsWithEffectTree(property_trees);
-  }
-  for (auto* layer : layer_list)
-    ComputeLayerClipRect(property_trees, layer);
-}
-
-gfx::Rect ComputeLayerVisibleRectDynamic(const PropertyTrees* property_trees,
-                                         const LayerImpl* layer) {
+static gfx::Rect LayerVisibleRect(PropertyTrees* property_trees,
+                                  LayerImpl* layer) {
   int effect_ancestor_with_copy_request =
       property_trees->effect_tree.ClosestAncestorWithCopyRequest(
           layer->effect_tree_index());
@@ -1115,18 +607,18 @@ gfx::Rect ComputeLayerVisibleRectDynamic(const PropertyTrees* property_trees,
   gfx::Rect layer_content_rect = gfx::Rect(layer->bounds());
   gfx::RectF accumulated_clip_in_root_space;
   if (non_root_copy_request) {
-    bool include_viewport_clip = false;
     bool include_expanding_clips = true;
     ConditionalClip accumulated_clip = ComputeAccumulatedClip(
-        property_trees, include_viewport_clip, include_expanding_clips,
-        layer->clip_tree_index(), effect_ancestor_with_copy_request);
+        property_trees, include_expanding_clips, layer->clip_tree_index(),
+        effect_ancestor_with_copy_request);
     if (!accumulated_clip.is_clipped)
       return layer_content_rect;
     accumulated_clip_in_root_space = accumulated_clip.clip_rect;
   } else {
+    const ClipNode* clip_node =
+        property_trees->clip_tree.Node(layer->clip_tree_index());
     accumulated_clip_in_root_space =
-        ComputeAccumulatedClipInRootSpaceForVisibleRect(
-            property_trees, layer->clip_tree_index());
+        clip_node->accumulated_rect_in_screen_space;
   }
 
   const EffectNode* root_effect_node =
@@ -1138,8 +630,9 @@ gfx::Rect ComputeLayerVisibleRectDynamic(const PropertyTrees* property_trees,
           accumulated_clip_in_root_space, property_trees,
           root_effect_node->transform_id, layer->transform_tree_index(),
           root_effect_node->id);
-  if (!accumulated_clip_in_layer_space.is_clipped)
+  if (!accumulated_clip_in_layer_space.is_clipped) {
     return layer_content_rect;
+  }
   gfx::RectF clip_in_layer_space = accumulated_clip_in_layer_space.clip_rect;
   clip_in_layer_space.Offset(-layer->offset_to_transform_parent());
 
@@ -1148,17 +641,51 @@ gfx::Rect ComputeLayerVisibleRectDynamic(const PropertyTrees* property_trees,
   return visible_rect;
 }
 
-void VerifyVisibleRectsCalculations(const LayerImplList& layer_list,
-                                    const PropertyTrees* property_trees) {
-  for (auto* layer : layer_list) {
-    gfx::Rect visible_rect_dynamic =
-        ComputeLayerVisibleRectDynamic(property_trees, layer);
-    DCHECK(layer->visible_layer_rect() == visible_rect_dynamic)
-        << " layer: " << layer->id() << " clip id: " << layer->clip_tree_index()
-        << " visible rect cached: " << layer->visible_layer_rect().ToString()
-        << " v.s. "
-        << " visible rect dynamic: " << visible_rect_dynamic.ToString();
+void UpdatePropertyTrees(PropertyTrees* property_trees,
+                         bool can_render_to_separate_surface) {
+  if (property_trees->non_root_surfaces_enabled !=
+      can_render_to_separate_surface) {
+    property_trees->non_root_surfaces_enabled = can_render_to_separate_surface;
+    property_trees->transform_tree.set_needs_update(true);
+  }
+  if (property_trees->transform_tree.needs_update()) {
+    property_trees->clip_tree.set_needs_update(true);
+    property_trees->effect_tree.set_needs_update(true);
+  }
+  ComputeTransforms(&property_trees->transform_tree);
+  // Computation of clips uses surface contents scale which is updated while
+  // computing effects. So, ComputeEffects should be before ComputeClips.
+  ComputeEffects(&property_trees->effect_tree);
+  ComputeClips(property_trees);

[weiliangc, 2017/03/14 22:04:29]

Also DCHECK in ComputeClips that effect tree doesn't need update.

[jaydasika, 2017/03/16 20:33:31]

Actually, after creating a fast-path of using ToScreen in GetToTarget, this
comment is no longer true, we only need transform tree updated to have an
updated ToScreen

+}
+
+void UpdatePropertyTreesAndRenderSurfaces(LayerImpl* root_layer,
+                                          PropertyTrees* property_trees,
+                                          bool can_render_to_separate_surface) {
+  bool render_surfaces_need_update = false;
+  if (property_trees->non_root_surfaces_enabled !=
+      can_render_to_separate_surface) {
+    property_trees->non_root_surfaces_enabled = can_render_to_separate_surface;
+    property_trees->transform_tree.set_needs_update(true);
+    render_surfaces_need_update = true;
+  }
+  if (property_trees->transform_tree.needs_update()) {
+    property_trees->clip_tree.set_needs_update(true);
+    property_trees->effect_tree.set_needs_update(true);
   }
+  if (render_surfaces_need_update) {
+    property_trees->effect_tree.UpdateRenderSurfaces(
+        root_layer->layer_tree_impl(),
+        property_trees->non_root_surfaces_enabled);
+  }
+  UpdateRenderTarget(&property_trees->effect_tree,
+                     property_trees->non_root_surfaces_enabled);
+
+  ComputeTransforms(&property_trees->transform_tree);
+  // Computation of clips uses surface contents scale which is updated while
+  // computing effects. So, ComputeEffects should be before ComputeClips.
+  ComputeEffects(&property_trees->effect_tree);
+  ComputeClips(property_trees);

[weiliangc, 2017/03/14 22:04:30]

DCHECK like above.

[jaydasika, 2017/03/16 20:33:30]

Done.

 }
 
 bool LayerNeedsUpdate(Layer* layer,
@@ -1218,7 +745,7 @@ static void SetSurfaceDrawTransform(const PropertyTrees* property_trees,
 }
 
 static void SetSurfaceClipRect(const ClipNode* parent_clip_node,
-                               const PropertyTrees* property_trees,
+                               PropertyTrees* property_trees,
                                RenderSurfaceImpl* render_surface) {
   if (!render_surface->is_clipped()) {
     render_surface->SetClipRect(gfx::Rect());
@@ -1226,32 +753,21 @@ static void SetSurfaceClipRect(const ClipNode* parent_clip_node,
   }
 
   const EffectTree& effect_tree = property_trees->effect_tree;
-  const TransformTree& transform_tree = property_trees->transform_tree;
-  const TransformNode* transform_node =
-      transform_tree.Node(render_surface->TransformTreeIndex());
-  if (transform_tree.TargetId(transform_node->id) ==
-      parent_clip_node->target_transform_id) {
-    render_surface->SetClipRect(
-        gfx::ToEnclosingRect(parent_clip_node->clip_in_target_space));
-    return;
-  }
-
-  // In this case, the clip child has reset the clip node for subtree and hence
-  // the parent clip node's clip rect is in clip parent's target space and not
-  // our target space. We need to transform it to our target space.
+  const ClipTree& clip_tree = property_trees->clip_tree;
   const EffectNode* effect_node =
       effect_tree.Node(render_surface->EffectTreeIndex());
-  int target_effect_id = effect_node->target_id;
-  gfx::RectF clip_rect;
-  const bool success = ConvertRectBetweenSurfaceSpaces(
-      property_trees, parent_clip_node->target_effect_id, target_effect_id,
-      parent_clip_node->clip_in_target_space, &clip_rect);
-
-  if (!success) {
-    render_surface->SetClipRect(gfx::Rect());
-    return;
+  const EffectNode* target_node = effect_tree.Node(effect_node->target_id);
+  bool include_expanding_clips = false;
+  if (render_surface->EffectTreeIndex() == EffectTree::kContentsRootNodeId) {
+    render_surface->SetClipRect(
+        gfx::ToEnclosingRect(clip_tree.Node(effect_node->clip_id)->clip));
+  } else {
+    ConditionalClip accumulated_clip_rect =
+        ComputeAccumulatedClip(property_trees, include_expanding_clips,
+                               effect_node->clip_id, target_node->id);
+    render_surface->SetClipRect(
+        gfx::ToEnclosingRect(accumulated_clip_rect.clip_rect));
   }
-  render_surface->SetClipRect(gfx::ToEnclosingRect(clip_rect));
 }
 
 template <typename LayerType>
@@ -1318,11 +834,9 @@ static gfx::Rect LayerDrawableContentRect(
 }
 
 void ComputeLayerDrawProperties(LayerImpl* layer,
-                                const PropertyTrees* property_trees) {
+                                PropertyTrees* property_trees) {
   const TransformNode* transform_node =
       property_trees->transform_tree.Node(layer->transform_tree_index());
-  const ClipNode* clip_node =
-      property_trees->clip_tree.Node(layer->clip_tree_index());
 
   layer->draw_properties().screen_space_transform =
       ScreenSpaceTransformInternal(layer, property_trees->transform_tree);
@@ -1333,16 +847,15 @@ void ComputeLayerDrawProperties(LayerImpl* layer,
 
   layer->draw_properties().opacity =
       LayerDrawOpacity(layer, property_trees->effect_tree);
-  if (property_trees->non_root_surfaces_enabled) {
-    layer->draw_properties().is_clipped = clip_node->layers_are_clipped;
-  } else {
-    layer->draw_properties().is_clipped =
-        clip_node->layers_are_clipped_when_surfaces_disabled;
-  }
 
   gfx::Rect bounds_in_target_space = MathUtil::MapEnclosingClippedRect(
       layer->draw_properties().target_space_transform,
       gfx::Rect(layer->bounds()));
+  ConditionalClip clip = LayerClipRect(property_trees, layer);
+  layer->draw_properties().is_clipped = clip.is_clipped;
+  layer->draw_properties().clip_rect = gfx::ToEnclosingRect(clip.clip_rect);
+  layer->draw_properties().visible_layer_rect =
+      LayerVisibleRect(property_trees, layer);
   layer->draw_properties().drawable_content_rect = LayerDrawableContentRect(
       layer, bounds_in_target_space, layer->draw_properties().clip_rect);
 }
@@ -1358,7 +871,7 @@ void ComputeMaskDrawProperties(LayerImpl* mask_layer,
       gfx::Rect(mask_layer->bounds());
 }
 
-void ComputeSurfaceDrawProperties(const PropertyTrees* property_trees,
+void ComputeSurfaceDrawProperties(PropertyTrees* property_trees,
                                   RenderSurfaceImpl* render_surface) {
   const EffectNode* effect_node =
       property_trees->effect_tree.Node(render_surface->EffectTreeIndex());