Changes subtreeShouldRenderToSeparateSurface logic to account for explicit clipping

cc/layer_tree_host_common.cc

 // Copyright 2011 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/layer_tree_host_common.h"
 
 #include <algorithm>
 
 #include "base/debug/trace_event.h"
 #include "cc/layer.h"
@@ skipping 221 matching lines @@
 // positives (e.g. layers get this called on them but don't actually get drawn).
 static inline void markLayerAsUpdated(LayerImpl* layer)
 {
     layer->didUpdateTransforms();
 }
 
 static inline void markLayerAsUpdated(Layer* layer)
 {
 }
 
+

[shawnsingh, 2012/12/18 19:11:00]

nit: no need to add extra line here

 template<typename LayerType>
 static bool subtreeShouldRenderToSeparateSurface(LayerType* layer, bool axisAlignedWithRespectToParent)
 {
     //
     // A layer and its descendants should render onto a new RenderSurfaceImpl if any of these rules hold:
     //
 
     // The root layer should always have a renderSurface.
     if (isRootLayer(layer))
         return true;
@@ skipping 15 matching lines @@
         return true;
 
     int numDescendantsThatDrawContent = layer->drawProperties().num_descendants_that_draw_content;
 
     // If the layer flattens its subtree (i.e. the layer doesn't preserve-3d), but it is
     // treated as a 3D object by its parent (i.e. parent does preserve-3d).
     if (layerIsInExisting3DRenderingContext(layer) && !layer->preserves3D() && numDescendantsThatDrawContent > 0)
         return true;
 
     // If the layer clips its descendants but it is not axis-aligned with respect to its parent.
-    if (layerClipsSubtree(layer) && !axisAlignedWithRespectToParent && numDescendantsThatDrawContent > 0)
+    if (layerClipsSubtree(layer) && !axisAlignedWithRespectToParent && !layer->drawProperties().does_not_require_scissoring)
         return true;
 
     // If the layer has some translucency and does not have a preserves-3d transform style.
     // This condition only needs a render surface if two or more layers in the
     // subtree overlap. But checking layer overlaps is unnecessarily costly so
     // instead we conservatively create a surface whenever at least two layers
     // draw content for this subtree.
     bool atLeastTwoLayersInSubtreeDrawContent = layer->hasDelegatedContent() ||
         (numDescendantsThatDrawContent > 0 && (layer->drawsContent() || numDescendantsThatDrawContent > 1));
 
@@ skipping 140 matching lines @@
     renderSurfaceLayerList.pop_back();
     layerToRemove->clearRenderSurface();
 }
 
 // Recursively walks the layer tree to compute any information that is needed
 // before doing the main recursion.
 template<typename LayerType>
 static void preCalculateMetaInformation(LayerType* layer)
 {
     int numDescendantsThatDrawContent = 0;
+    bool doesNotRequireScissoring = true;
+    bool sublayerXformPreventsClip = !layer->sublayerTransform().IsPositiveScaleOrTranslation();

[shawnsingh, 2012/12/18 19:11:00]

like Dana, I also prefer Transform instead of Xform.  I know we used it in
gfx/transform_unittests but I think everywhere else our convention has been to
spell out this word

 
     for (size_t i = 0; i < layer->children().size(); ++i) {
         LayerType* childLayer = layer->children()[i];
         preCalculateMetaInformation<LayerType>(childLayer);
+
         numDescendantsThatDrawContent += childLayer->drawsContent() ? 1 : 0;
         numDescendantsThatDrawContent += childLayer->drawProperties().num_descendants_that_draw_content;
+
+        if ((childLayer->drawsContent() && !childLayer->canClipSelf()) ||
+            !childLayer->drawProperties().does_not_require_scissoring ||

[shawnsingh, 2012/12/18 19:11:00]

I feel like my proposed variable name in earlier comments got overlooked, but I
really think it will help readability here.  The point we want readers to
realize is that this boolean tracks that information for all a layer's
descendants, but not the layer itself.  That's the key hint that makes this
if-statement more readable, IMHO.  Something like
"descendants_can_clip_themselves".  With that, the if-statement reads like:
"check the layer, check it's descendants, check the transforms related to the
childLayer"

+            sublayerXformPreventsClip ||
+            !childLayer->transform().IsPositiveScaleOrTranslation())
+            doesNotRequireScissoring = false;
     }
 
     layer->drawProperties().num_descendants_that_draw_content = numDescendantsThatDrawContent;
+    layer->drawProperties().does_not_require_scissoring = doesNotRequireScissoring;
 }
 
 // Recursively walks the layer tree starting at the given node and computes all the
 // necessary transformations, clipRects, render surfaces, etc.
 template<typename LayerType, typename LayerList, typename RenderSurfaceType>
 static void calculateDrawPropertiesInternal(LayerType* layer, const gfx::Transform& parentMatrix,
     const gfx::Transform& fullHierarchyMatrix, const gfx::Transform& currentScrollCompensationMatrix,
     const gfx::Rect& clipRectFromAncestor, const gfx::Rect& clipRectFromAncestorInDescendantSpace, bool ancestorClipsSubtree,
     RenderSurfaceType* nearestAncestorThatMovesPixels, LayerList& renderSurfaceLayerList, LayerList& layerList,
     LayerSorter* layerSorter, int maxTextureSize, float deviceScaleFactor, float pageScaleFactor, bool subtreeCanUseLCDText,
@@ skipping 638 matching lines @@
 
   // At this point, we think the point does hit the touch event handler region on the layer, but we need to walk up
   // the parents to ensure that the layer was not clipped in such a way that the
   // hit point actually should not hit the layer.
   if (pointIsClippedBySurfaceOrClipRect(screenSpacePoint, layerImpl))
      return false;
 
   return true;
 }
 }  // namespace cc