Changes subtreeShouldRenderToSeparateSurface logic to account for explicit clipping

cc/layer_tree_host_common_unittest.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 "cc/content_layer.h"
 #include "cc/content_layer_client.h"
 #include "cc/layer.h"
 #include "cc/layer_animation_controller.h"
@@ skipping 95 matching lines @@
     }
 
     virtual bool drawsContent() const OVERRIDE { return true; }
 
 private:
     virtual ~LayerWithForcedDrawsContent()
     {
     }
 };
 
+class LayerCanClipSelf : public Layer {
+public:
+    LayerCanClipSelf()
+        : Layer()
+    {
+    }
+
+    virtual bool drawsContent() const OVERRIDE { return true; }
+    virtual bool canClipSelf() const OVERRIDE { return true; }
+
+private:
+    virtual ~LayerCanClipSelf()
+    {
+    }
+};
+
 class MockContentLayerClient : public ContentLayerClient {
 public:
     MockContentLayerClient() { }
     virtual ~MockContentLayerClient() { }
     virtual void paintContents(SkCanvas*, const gfx::Rect& clip, gfx::RectF& opaque) OVERRIDE { }
 };
 
 scoped_refptr<ContentLayer> createDrawableContentLayer(ContentLayerClient* delegate)
 {
     scoped_refptr<ContentLayer> toReturn = ContentLayer::create(delegate);
@@ skipping 202 matching lines @@
     // Child is set up so that a new render surface should be created.
     child->setOpacity(0.5);
     child->setForceRenderSurface(true);
 
     gfx::Transform parentLayerTransform;
     parentLayerTransform.Scale3d(1, 0.9, 1);
     gfx::Transform parentTranslationToAnchor;
     parentTranslationToAnchor.Translate(25, 30);
     gfx::Transform parentSublayerMatrix;
     parentSublayerMatrix.Scale3d(0.9, 1, 3.3);
+

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

Did you add these empty lines here with the intent of landing, or was it just
vestigal stuff from local edits?   Either way OK with me, but just FYI

[whunt, 2012/12/18 19:48:15]

Just trying to read the code, I'll remove them.

On 2012/12/18 19:11:00, shawnsingh wrote:

     gfx::Transform parentTranslationToCenter;
     parentTranslationToCenter.Translate(50, 60);
+
     gfx::Transform parentCompositeTransform = parentTranslationToAnchor * parentLayerTransform * MathUtil::inverse(parentTranslationToAnchor)
             * parentTranslationToCenter * parentSublayerMatrix * MathUtil::inverse(parentTranslationToCenter);
+
     gfx::Vector2dF parentCompositeScale = MathUtil::computeTransform2dScaleComponents(parentCompositeTransform);
+
     gfx::Transform surfaceSublayerTransform;
     surfaceSublayerTransform.Scale(parentCompositeScale.x(), parentCompositeScale.y());
+
     gfx::Transform surfaceSublayerCompositeTransform = parentCompositeTransform * MathUtil::inverse(surfaceSublayerTransform);
 
     // Child's render surface should not exist yet.
     ASSERT_FALSE(child->renderSurface());
 
     setLayerPropertiesForTesting(parent.get(), parentLayerTransform, parentSublayerMatrix, gfx::PointF(0.25, 0.25), gfx::PointF(0, 0), gfx::Size(100, 120), false);
     setLayerPropertiesForTesting(child.get(), identityMatrix, identityMatrix, gfx::PointF(0, 0), gfx::PointF(0, 0), gfx::Size(16, 18), false);
     setLayerPropertiesForTesting(grandChild.get(), identityMatrix, identityMatrix, gfx::PointF(0, 0), gfx::PointF(0, 0), gfx::Size(8, 10), false);
     executeCalculateDrawProperties(root.get());
 
     // Render surface should have been created now.
     ASSERT_TRUE(child->renderSurface());
     ASSERT_EQ(child, child->renderTarget());
 
     // The child layer's draw transform should refer to its new render surface.
     // The screen-space transform, however, should still refer to the root.
     EXPECT_TRANSFORMATION_MATRIX_EQ(surfaceSublayerTransform, child->drawTransform());
     EXPECT_TRANSFORMATION_MATRIX_EQ(parentCompositeTransform, child->screenSpaceTransform());
 
     // Because the grandChild is the only drawable content, the child's renderSurface will tighten its bounds to the grandChild.
     // The scale at which the surface's subtree is drawn must be removed from the composite transform.
     EXPECT_TRANSFORMATION_MATRIX_EQ(surfaceSublayerCompositeTransform, child->renderTarget()->renderSurface()->drawTransform());
 
     // The screen space is the same as the target since the child surface draws into the root.
     EXPECT_TRANSFORMATION_MATRIX_EQ(surfaceSublayerCompositeTransform, child->renderTarget()->renderSurface()->screenSpaceTransform());
 }
 
+TEST(LayerTreeHostCommonTest, verifySeparateRenderTargetRequirement)
+{
+  {
+      scoped_refptr<Layer> root = Layer::create();
+      scoped_refptr<Layer> parent = Layer::create();
+      scoped_refptr<Layer> child = Layer::create();
+      scoped_refptr<Layer> grandChild = make_scoped_refptr(new LayerCanClipSelf());
+      root->addChild(parent);
+      parent->addChild(child);
+      child->addChild(grandChild);
+      parent->setMasksToBounds(true);
+      child->setMasksToBounds(true);
+
+      gfx::Transform identityMatrix;
+      gfx::Transform parentLayerTransform;
+      gfx::Transform parentSublayerMatrix;

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

This is unmodified; how about we just use identityMatrix below, and remove this
declaration?

+      gfx::Transform childLayerMatrix;
+
+      // No render surface should exist yet.
+      ASSERT_FALSE(root->renderSurface());
+      ASSERT_FALSE(parent->renderSurface());
+      ASSERT_FALSE(child->renderSurface());
+      ASSERT_FALSE(grandChild->renderSurface());
+
+      // One-time setup of root layer
+      parentLayerTransform.Scale3d(1, 0.9, 1);
+      childLayerMatrix.Scale3d(0.9, 1, 3.3);
+      childLayerMatrix.Rotate(20);
+
+      setLayerPropertiesForTesting(root      .get(), identityMatrix      , identityMatrix      , gfx::PointF(   0,    0), gfx::PointF(0, 0), gfx::Size(  1,   2), false);
+      setLayerPropertiesForTesting(parent    .get(), parentLayerTransform, parentSublayerMatrix, gfx::PointF(0.25, 0.25), gfx::PointF(0, 0), gfx::Size(100, 120), false);
+      setLayerPropertiesForTesting(child     .get(), childLayerMatrix    , identityMatrix      , gfx::PointF(   0,    0), gfx::PointF(0, 0), gfx::Size( 16,  18), false);
+      setLayerPropertiesForTesting(grandChild.get(), identityMatrix      , identityMatrix      , gfx::PointF(   0,    0), gfx::PointF(0, 0), gfx::Size(  8,  10), false);
+
+      executeCalculateDrawProperties(root.get());
+
+      // Render surface should have been created now.

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

We probably want to update this comment, should probably say something like "We
expect that child should not create a render surface"

+      ASSERT_TRUE(root->renderSurface());
+      ASSERT_FALSE(parent->renderSurface());
+      ASSERT_FALSE(child->renderSurface());
+      ASSERT_FALSE(grandChild->renderSurface());
+  }
+  {
+      scoped_refptr<Layer> root = Layer::create();
+      scoped_refptr<Layer> parent = Layer::create();
+      scoped_refptr<Layer> child = Layer::create();
+      scoped_refptr<Layer> grandChild = make_scoped_refptr(new LayerWithForcedDrawsContent());
+      root->addChild(parent);
+      parent->addChild(child);
+      child->addChild(grandChild);
+      parent->setMasksToBounds(true);
+      child->setMasksToBounds(true);
+
+      gfx::Transform identityMatrix;
+      gfx::Transform parentLayerTransform;
+      gfx::Transform parentSublayerMatrix;

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

This is unmodified; how about we just use identityMatrix below, and remove this
declaration?

+      gfx::Transform childLayerMatrix;
+
+      // No render surface should exist yet.
+      ASSERT_FALSE(root->renderSurface());
+      ASSERT_FALSE(parent->renderSurface());
+      ASSERT_FALSE(child->renderSurface());
+      ASSERT_FALSE(grandChild->renderSurface());
+
+      // One-time setup of root layer
+      parentLayerTransform.Scale3d(1, 0.9, 1);
+      childLayerMatrix.Scale3d(0.9, 1, 3.3);
+      childLayerMatrix.Rotate(20);
+
+      setLayerPropertiesForTesting(root      .get(), identityMatrix      , identityMatrix      , gfx::PointF(   0,    0), gfx::PointF(0, 0), gfx::Size(  1,   2), false);
+      setLayerPropertiesForTesting(parent    .get(), parentLayerTransform, parentSublayerMatrix, gfx::PointF(0.25, 0.25), gfx::PointF(0, 0), gfx::Size(100, 120), false);
+      setLayerPropertiesForTesting(child     .get(), childLayerMatrix    , identityMatrix      , gfx::PointF(   0,    0), gfx::PointF(0, 0), gfx::Size( 16,  18), false);
+      setLayerPropertiesForTesting(grandChild.get(), identityMatrix      , identityMatrix      , gfx::PointF(   0,    0), gfx::PointF(0, 0), gfx::Size(  8,  10), false);
+
+      executeCalculateDrawProperties(root.get());
+
+      // Render surface should have been created now.

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

We probably want to update this comment, something like "This time we do expect
a child surface to be created"

+      ASSERT_TRUE(root->renderSurface());
+      ASSERT_FALSE(parent->renderSurface());
+      ASSERT_TRUE(child->renderSurface());
+      ASSERT_FALSE(grandChild->renderSurface());
+  }
+}
+
 TEST(LayerTreeHostCommonTest, verifyTransformsForReplica)
 {
     scoped_refptr<Layer> root = Layer::create();
     scoped_refptr<Layer> parent = Layer::create();
     scoped_refptr<Layer> child = Layer::create();
     scoped_refptr<Layer> childReplica = Layer::create();
     scoped_refptr<LayerWithForcedDrawsContent> grandChild = make_scoped_refptr(new LayerWithForcedDrawsContent());
     root->addChild(parent);
     parent->addChild(child);
     child->addChild(grandChild);
@@ skipping 483 matching lines @@
     FakeLayerTreeHostImpl hostImpl(&proxy);
     scoped_ptr<LayerImpl> root = createTreeForFixedPositionTests(&hostImpl);
     LayerImpl* child = root->children()[0];
     LayerImpl* grandChild = child->children()[0];
     LayerImpl* greatGrandChild = grandChild->children()[0];
 
     child->setIsContainerForFixedPositionLayers(true);
     grandChild->setPosition(gfx::PointF(8, 6));
     greatGrandChild->setFixedToContainerLayer(true);
 
+

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

another empty line

     // Case 1: scrollDelta of 0, 0
     child->setScrollDelta(gfx::Vector2d(0, 0));
     executeCalculateDrawProperties(root.get());
 
     gfx::Transform expectedChildTransform;
     gfx::Transform expectedGrandChildTransform;
     expectedGrandChildTransform.Translate(8, 6);
 
     gfx::Transform expectedGreatGrandChildTransform = expectedGrandChildTransform;
 
@@ skipping 3927 matching lines @@
     EXPECT_EQ(m_canUseLCDText, m_grandChild->canUseLCDText());
 }
 
 INSTANTIATE_TEST_CASE_P(LayerTreeHostCommonTest,
                         LCDTextTest,
                         testing::Combine(testing::Bool(),
                                          testing::Bool()));
 
 }  // namespace
 }  // namespace cc