Changes subtreeShouldRenderToSeparateSurface logic to account for explicit clipping

cc/layer_tree_host_common.cc

Index: cc/layer_tree_host_common.cc
diff --git a/cc/layer_tree_host_common.cc b/cc/layer_tree_host_common.cc
index 0a204ceb6914df96a5bdede2b6c8c9ee7e1c6915..af489b0c70b6fce775e24923afb4928b1da7de90 100644
--- a/cc/layer_tree_host_common.cc
+++ b/cc/layer_tree_host_common.cc
@@ -274,7 +274,7 @@ static bool subtreeShouldRenderToSeparateSurface(LayerType* layer, bool axisAlig
         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().can_clip_self)
         return true;
 
     // If the layer has some translucency and does not have a preserves-3d transform style.
@@ -429,21 +429,45 @@ static inline void removeSurfaceForEarlyExit(LayerType* layerToRemove, LayerList
     layerToRemove->clearRenderSurface();
 }
 
+inline bool isPositiveScaleOrTranslation(const gfx::Transform& xform)

[shawnsingh, 2012/12/14 23:59:11]

Re: James' comment; I think a good place for this, for now, is MathUtil

[danakj, 2012/12/15 00:02:17]

nit: call the variable "transform"? we don't do this elsewhere.

+{
+    if (!xform.IsScaleOrTranslation())
+        return false;
+
+    // Grab our scale and make sure it's positive.
+    double x_scale = xform.matrix().getDouble(0, 0);
+    double y_scale = xform.matrix().getDouble(1, 1);

[danakj, 2012/12/15 00:02:17]

What about z_scale?

+    if (x_scale <= 0.0 || y_scale <= 0.0)
+        return false;
+
+    return true;
+}
+
 // 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 canClipSelf = true;
+    bool sublayerXformPreventsClip = !isPositiveScaleOrTranslation(layer->sublayerTransform());

[danakj, 2012/12/15 00:02:17]

nit: "sublayerTransform..."

 
     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() ||

[jamesr, 2012/12/14 22:51:16]

multiple invocations of && and || merit some grouping parenthesis - compilers
can follow precedence rules correctly, but it's harder for humans to do so
quickly

+            !childLayer->drawProperties().can_clip_self ||
+            sublayerXformPreventsClip ||
+            !isPositiveScaleOrTranslation(childLayer->transform()))
+            canClipSelf = false;

[jamesr, 2012/12/14 22:51:16]

i would probably use braces around this block since it's a bit hard to tell the
end of the if and the start of the actual block

[shawnsingh, 2012/12/14 23:59:11]

looking at this if-statement, there is a triple-meaning to "can clip self" that
needs to be disambiguated.
(1) The layer's accessor "CanClipSelf" perhaps should say
"supportsSelfClipping".
(2) The drawProperties can_clip_self actually seems like it should say
"all_children_can_clip_themselves"
(3) the actual "layer can clip itself" canClipSelf is implicit in this
if-statement. I wonder if it would help readability to make that boolean
explicit, just so the variable name exists to help people see the flow of logic.

for example, something like this in code:

bool thisLayerCanClipItself = <opposite of stuff currently inside your if
statement>
if (!thisLayerCanClipItself)
   allChildrenCanClipThemselves = false;

     }
 
     layer->drawProperties().num_descendants_that_draw_content = numDescendantsThatDrawContent;
+    layer->drawProperties().can_clip_self = canClipSelf;
 }
 
 // Recursively walks the layer tree starting at the given node and computes all the