Remove root layer specialness in calculateDrawTransforms

cc/CCLayerTreeHostCommon.cpp

Index: cc/CCLayerTreeHostCommon.cpp
diff --git a/cc/CCLayerTreeHostCommon.cpp b/cc/CCLayerTreeHostCommon.cpp
index d5a64ecbeb8bf2e58bbd09fd3df519384c3790d3..cb44ba38f34834a786ea019b5e3dcf26c456c746 100644
--- a/cc/CCLayerTreeHostCommon.cpp
+++ b/cc/CCLayerTreeHostCommon.cpp
@@ -218,18 +218,14 @@ static inline bool subtreeShouldBeSkipped(LayerChromium* layer)
 template<typename LayerType>
 static bool subtreeShouldRenderToSeparateSurface(LayerType* layer, bool axisAlignedWithRespectToParent)
 {
-    // The root layer has a special render surface that is set up externally, so
-    // it shouldn't be treated as a surface in this code.
-    if (!layer->parent())
-        return false;
-
-    // Cache this value, because otherwise it walks the entire subtree several times.
-    bool descendantDrawsContent = layer->descendantDrawsContent();
-
     //
     // A layer and its descendants should render onto a new RenderSurface if any of these rules hold:
     //
 
+    // The root layer should always have a renderSurface.
+    if (!layer->parent())

[danakj, 2012/10/09 22:21:54]

<3 this

[enne (OOO), 2012/10/15 18:29:45]

Can you remove the inconsistent use of layer == rootLayer vs !layer->parent()?
Do we even need to pass the root layer down this function recursively?

+        return true;
+
     // If we force it.
     if (layer->forceRenderSurface())
         return true;
@@ -246,6 +242,9 @@ static bool subtreeShouldRenderToSeparateSurface(LayerType* layer, bool axisAlig
     if (!layer->filters().isEmpty() || !layer->backgroundFilters().isEmpty())
         return true;
 
+    // Cache this value, because otherwise it walks the entire subtree several times.
+    bool descendantDrawsContent = layer->descendantDrawsContent();
+
     // 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() && descendantDrawsContent)
@@ -344,20 +343,6 @@ WebTransformationMatrix computeScrollCompensationMatrixForChildren(CCLayerImpl*
     return nextScrollCompensationMatrix;
 }
 
-// Should be called just before the recursive calculateDrawTransformsInternal().
-template<typename LayerType, typename LayerList>
-void setupRootLayerAndSurfaceForRecursion(LayerType* rootLayer, LayerList& renderSurfaceLayerList, const IntSize& deviceViewportSize)
-{
-    if (!rootLayer->renderSurface())
-        rootLayer->createRenderSurface();
-
-    rootLayer->renderSurface()->setContentRect(IntRect(IntPoint::zero(), deviceViewportSize));
-    rootLayer->renderSurface()->clearLayerLists();
-
-    ASSERT(renderSurfaceLayerList.isEmpty());
-    renderSurfaceLayerList.append(rootLayer);
-}
-
 // 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, typename LayerSorter>
@@ -452,7 +437,8 @@ static void calculateDrawTransformsInternal(LayerType* layer, LayerType* rootLay
     // If we early-exit anywhere in this function, the drawableContentRect of this subtree should be considered empty.
     drawableContentRectOfSubtree = IntRect();
 
-    if (subtreeShouldBeSkipped(layer))
+    // The root layer cannot skip calcDrawTransforms.
+    if (layer != rootLayer && subtreeShouldBeSkipped(layer))
         return;
 
     IntRect clipRectForSubtree;
@@ -590,6 +576,9 @@ static void calculateDrawTransformsInternal(LayerType* layer, LayerType* rootLay
 
         renderSurfaceLayerList.append(layer);
     } else {
+        ASSERT(layer != rootLayer);
+        ASSERT(layer->parent());
+
         layer->setDrawTransform(drawTransform);
         layer->setDrawTransformIsAnimating(animatingTransformToTarget);
         layer->setScreenSpaceTransformIsAnimating(animatingTransformToScreen);
@@ -598,25 +587,15 @@ static void calculateDrawTransformsInternal(LayerType* layer, LayerType* rootLay
         layer->setDrawOpacity(drawOpacity);
         layer->setDrawOpacityIsAnimating(drawOpacityIsAnimating);
 
-        if (layer != rootLayer) {
-            ASSERT(layer->parent());
-            layer->clearRenderSurface();
+        layer->clearRenderSurface();
 
-            // Layers without renderSurfaces directly inherit the ancestor's clip status.
-            subtreeShouldBeClipped = ancestorClipsSubtree;
-            if (ancestorClipsSubtree)
-                clipRectForSubtree = clipRectFromAncestor;
-
-            // Layers that are not their own renderTarget will render into the target of their nearest ancestor.
-            layer->setRenderTarget(layer->parent()->renderTarget());
-        } else {
-            // FIXME: This root layer special case code should eventually go away. https://bugs.webkit.org/show_bug.cgi?id=92290
-            ASSERT(!layer->parent());
-            ASSERT(layer->renderSurface());
-            ASSERT(ancestorClipsSubtree);
-            layer->renderSurface()->setClipRect(clipRectFromAncestor);
-            subtreeShouldBeClipped = false;
-        }
+        // Layers without renderSurfaces directly inherit the ancestor's clip status.
+        subtreeShouldBeClipped = ancestorClipsSubtree;
+        if (ancestorClipsSubtree)
+            clipRectForSubtree = clipRectFromAncestor;
+
+        // Layers that are not their own renderTarget will render into the target of their nearest ancestor.
+        layer->setRenderTarget(layer->parent()->renderTarget());
     }
 
     IntRect rectInTargetSpace = enclosingIntRect(CCMathUtil::mapClippedRect(layer->drawTransform(), contentRect));
@@ -681,7 +660,11 @@ static void calculateDrawTransformsInternal(LayerType* layer, LayerType* rootLay
     layer->setVisibleContentRect(visibleContentRectOfLayer);
 
     // Compute the remaining properties for the render surface, if the layer has one.
-    if (layer->renderSurface() && layer != rootLayer) {
+    if (layer == rootLayer) {
+        // The root layer's surface's contentRect is always the entire viewport.
+        ASSERT(layer->renderSurface());
+        layer->renderSurface()->setContentRect(clipRectFromAncestor);
+    } else if (layer->renderSurface() && layer != rootLayer) {
         RenderSurfaceType* renderSurface = layer->renderSurface();
         IntRect clippedContentRect = localDrawableContentRectOfSubtree;
 
@@ -766,29 +749,39 @@ static void calculateDrawTransformsInternal(LayerType* layer, LayerType* rootLay
 void CCLayerTreeHostCommon::calculateDrawTransforms(LayerChromium* rootLayer, const IntSize& deviceViewportSize, float deviceScaleFactor, int maxTextureSize, Vector<RefPtr<LayerChromium> >& renderSurfaceLayerList)
 {
     IntRect totalDrawableContentRect;
+    IntRect deviceViewportRect(IntPoint::zero(), deviceViewportSize);
     WebTransformationMatrix identityMatrix;
     WebTransformationMatrix deviceScaleTransform;
     deviceScaleTransform.scale(deviceScaleFactor);
-
-    setupRootLayerAndSurfaceForRecursion<LayerChromium, Vector<RefPtr<LayerChromium> > >(rootLayer, renderSurfaceLayerList, deviceViewportSize);
+    Vector<RefPtr<LayerChromium> > dummyLayerList;
 
     cc::calculateDrawTransformsInternal<LayerChromium, Vector<RefPtr<LayerChromium> >, RenderSurfaceChromium, void>(rootLayer, rootLayer, deviceScaleTransform, identityMatrix, identityMatrix,
-                                                                                                                         rootLayer->renderSurface()->contentRect(), true, 0, renderSurfaceLayerList,
-                                                                                                                         rootLayer->renderSurface()->layerList(), 0, maxTextureSize, deviceScaleFactor, totalDrawableContentRect);
+                                                                                                                         deviceViewportRect, true, 0, renderSurfaceLayerList,
+                                                                                                                         dummyLayerList, 0, maxTextureSize, deviceScaleFactor, totalDrawableContentRect);
+
+    // The dummy layer list should not have been used.
+    ASSERT(dummyLayerList.size() == 0);
+    // A root layer renderSurface should always exist after calcDrawTransforms.
+    ASSERT(rootLayer->renderSurface());
 }
 
 void CCLayerTreeHostCommon::calculateDrawTransforms(CCLayerImpl* rootLayer, const IntSize& deviceViewportSize, float deviceScaleFactor, CCLayerSorter* layerSorter, int maxTextureSize, Vector<CCLayerImpl*>& renderSurfaceLayerList)
 {
     IntRect totalDrawableContentRect;
+    IntRect deviceViewportRect(IntPoint::zero(), deviceViewportSize);
     WebTransformationMatrix identityMatrix;
     WebTransformationMatrix deviceScaleTransform;
     deviceScaleTransform.scale(deviceScaleFactor);
-
-    setupRootLayerAndSurfaceForRecursion<CCLayerImpl, Vector<CCLayerImpl*> >(rootLayer, renderSurfaceLayerList, deviceViewportSize);
+    Vector<CCLayerImpl*> dummyLayerList;
 
     cc::calculateDrawTransformsInternal<CCLayerImpl, Vector<CCLayerImpl*>, CCRenderSurface, CCLayerSorter>(rootLayer, rootLayer, deviceScaleTransform, identityMatrix, identityMatrix,
-                                                                                                                rootLayer->renderSurface()->contentRect(), true, 0, renderSurfaceLayerList,
-                                                                                                                rootLayer->renderSurface()->layerList(), layerSorter, maxTextureSize, deviceScaleFactor, totalDrawableContentRect);
+                                                                                                                deviceViewportRect, true, 0, renderSurfaceLayerList,

[danakj, 2012/10/09 22:21:54]

Can we remove the magic true bool here and give it a variable while we're here?

+                                                                                                                dummyLayerList, layerSorter, maxTextureSize, deviceScaleFactor, totalDrawableContentRect);
+
+    // The dummy layer list should not have been used.
+    ASSERT(dummyLayerList.size() == 0);
+    // A root layer renderSurface should always exist after calcDrawTransforms.
+    ASSERT(rootLayer->renderSurface());
 }
 
 static bool pointHitsRect(const IntPoint& viewportPoint, const WebTransformationMatrix& localSpaceToScreenSpaceTransform, FloatRect localSpaceRect)