Refactor CCLayerTreeHostCommon - merge visible rect computation into calc draw transforms.

cc/CCLayerTreeHostCommon.cpp

 // 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 "config.h"
 
 #include "CCLayerTreeHostCommon.h"
 
 #include "CCLayerImpl.h"
@@ skipping 96 matching lines @@
 static inline bool layerClipsSubtree(LayerType* layer)
 {
     return layer->masksToBounds() || layer->maskLayer();
 }
 
 template<typename LayerType>
 static IntRect calculateVisibleContentRect(LayerType* layer)
 {
     ASSERT(layer->renderTarget());
 
-    IntRect targetSurfaceRect = layer->renderTarget()->renderSurface()->contentRect();
-
-    targetSurfaceRect.intersect(layer->drawableContentRect());
-
-    if (targetSurfaceRect.isEmpty() || layer->contentBounds().isEmpty())
+    // Nothing is visible if the layer bounds are empty.
+    if (!layer->drawsContent() || layer->contentBounds().isEmpty() || layer->drawableContentRect().isEmpty())
         return IntRect();
 
-    const IntRect contentRect = IntRect(IntPoint(), layer->contentBounds());
-    IntRect visibleContentRect = CCLayerTreeHostCommon::calculateVisibleRect(targetSurfaceRect, contentRect, layer->drawTransform());
-    return visibleContentRect;
+    IntRect targetSurfaceClipRect;
+
+    // First, compute visible bounds in target surface space.
+    if (layer->renderTarget()->renderSurface()->clipRect().isEmpty())
+        targetSurfaceClipRect = layer->drawableContentRect();

[epenner, 2012/10/10 18:35:59]

I don't understand how this doesn't allow for possible out-of-memory cases.
Can't the drawableContentRect be an arbitrary size in this case?

[shawnsingh, 2012/10/10 19:54:56]

Aside from the max texture size safety, you're right this doesn't address that
issue.  I think the intent of this patch was to be a much needed cleanup anyway,
even though we didn't address this yet.  Let's definitely discuss with everyone
how we would want to address this. =)

[epenner, 2012/10/10 21:13:59]

Done.

[epenner, 2012/10/10 21:13:59]

IIUC The max texture size will limit the size of a render surface, but it will
not limit the size of content that renders into that surface. So I'm worried
that the visible content could be arbitrarily large.

[shawnsingh, 2012/10/10 22:35:42]

Looks like the root of the problem is that we are not propagating clipRects
across surfaces.  However, originally Enne had a very strong reason not to do so
- because it will regularly change renderSurface sizes, causing things to be
less cacheable.  I'm guessing cache-ability (i.e. avoiding work entirely) takes
priority over intelligently being able to pre-paint (i.e. doing more work in
idle time)... so maybe we need to find another way to get the visibility
information here?

One option I can think of at this spontaneous moment - make another visibleRect
that does include propagating the clipRect across surfaces.  But I think you've
already considered that and we didn't go for that, right?

+    else {
+        // In this case the target surface does clip layers that contribute to it. So, we
+        // have convert the current surface's clipRect from its ancestor surface space to
+        // the current surface space.
+        targetSurfaceClipRect = enclosingIntRect(CCMathUtil::projectClippedRect(layer->renderTarget()->renderSurface()->drawTransform().inverse(), layer->renderTarget()->renderSurface()->clipRect()));
+        targetSurfaceClipRect.intersect(layer->drawableContentRect());
+    }
+
+    if (targetSurfaceClipRect.isEmpty())
+        return IntRect();
+
+    return CCLayerTreeHostCommon::calculateVisibleRect(targetSurfaceClipRect, IntRect(IntPoint(), layer->contentBounds()), layer->drawTransform());
 }
 
 static bool isScaleOrTranslation(const WebTransformationMatrix& m)
 {
     return !m.m12() && !m.m13() && !m.m14()
            && !m.m21() && !m.m23() && !m.m24()
            && !m.m31() && !m.m32() && !m.m43()
            && m.m44();
 }
 
@@ skipping 424 matching lines @@
         // Update the aggregate hierarchy matrix to include the transform of the
         // newly created RenderSurface.
         nextHierarchyMatrix.multiply(renderSurface->drawTransform());
 
         // The new renderSurface here will correctly clip the entire subtree. So, we do
         // not need to continue propagating the clipping state further down the tree. This
         // way, we can avoid transforming clipRects from ancestor target surface space to
         // current target surface space that could cause more w < 0 headaches.
         subtreeShouldBeClipped = false;
 
-        if (layer->maskLayer())
+        if (layer->maskLayer()) {
             layer->maskLayer()->setRenderTarget(layer);
+            layer->maskLayer()->setVisibleContentRect(IntRect(IntPoint(), layer->contentBounds()));
+        }
 
-        if (layer->replicaLayer() && layer->replicaLayer()->maskLayer())
+        if (layer->replicaLayer() && layer->replicaLayer()->maskLayer()) {
             layer->replicaLayer()->maskLayer()->setRenderTarget(layer);
+            layer->replicaLayer()->maskLayer()->setVisibleContentRect(IntRect(IntPoint(), layer->contentBounds()));
+        }
 
         if (layer->filters().hasFilterThatMovesPixels())
             nearestAncestorThatMovesPixels = renderSurface;
 
+        // The render surface clipRect is expressed in the space where this surface draws, i.e. the same space as clipRectFromAncestor.
+        if (ancestorClipsSubtree)
+            renderSurface->setClipRect(clipRectFromAncestor);
+        else
+            renderSurface->setClipRect(IntRect());
+
         renderSurface->setNearestAncestorThatMovesPixels(nearestAncestorThatMovesPixels);
 
         renderSurfaceLayerList.append(layer);
     } else {
         layer->setDrawTransform(drawTransform);
         layer->setDrawTransformIsAnimating(animatingTransformToTarget);
         layer->setScreenSpaceTransformIsAnimating(animatingTransformToScreen);
         sublayerMatrix = combinedTransform;
 
         layer->setDrawOpacity(drawOpacity);
@@ skipping 71 matching lines @@
         localDrawableContentRectOfSubtree.unite(rectInTargetSpace);
     if (subtreeShouldBeClipped)
         localDrawableContentRectOfSubtree.intersect(clipRectForSubtree);
 
     // Compute the layer's drawable content rect (the rect is in targetSurface space)
     IntRect drawableContentRectOfLayer = rectInTargetSpace;
     if (subtreeShouldBeClipped)
         drawableContentRectOfLayer.intersect(clipRectForSubtree);
     layer->setDrawableContentRect(drawableContentRectOfLayer);
 
+    // Compute the layer's visible content rect (the rect is in content space)
+    IntRect visibleContentRectOfLayer = calculateVisibleContentRect(layer);
+    layer->setVisibleContentRect(visibleContentRectOfLayer);
+
     // Compute the remaining properties for the render surface, if the layer has one.
     if (layer->renderSurface() && layer != rootLayer) {
         RenderSurfaceType* renderSurface = layer->renderSurface();
         IntRect clippedContentRect = localDrawableContentRectOfSubtree;
 
-        // The render surface clipRect is expressed in the space where this surface draws, i.e. the same space as clipRectFromAncestor.
-        if (ancestorClipsSubtree)

[epenner, 2012/10/10 18:35:59]

It looks like this code intends to only set an empty clipRect if
ancestorClipsSubtree is false. But couldn't it also set an empty clip rect if
ancestorClipsSubtree is true but the clipRect is now empty from two disjoint
clips?

[shawnsingh, 2012/10/10 19:54:56]

Yes... this is how we're encoding two different possibilities:
  (1) ancestorClipsSubtree == false implies that the clipRect is infinite and
its value should not be used
  (2) ancestorClipsSubtree == true implies that the clipRect value is finite,
meaningful, and should be used... even if it is empty.

I believe the same still holds even though this chunk of code was removed. 
technically it's not removed, but just moved up to inside the
subtreeShouldRenderToSeparateSurface() case.

Is there something specific that seems incorrect that you're concerned about?

[epenner, 2012/10/10 21:13:59]

It looks like line 124 checks clipRect().isEmpty() but there are two conditions
when it could be empty.

It seems to me like it should only ignore clipping if !ancestorClipsSubtree, but
return an empty visible rect if ancestorClipsSubtree.

[shawnsingh, 2012/10/10 22:35:42]

I see what you are saying now - thanks for catching this!  By fluke, this would
still be technically "correct" because an empty rendersurface would have been
seen and accounted for later in calcDrawTransforms, but clearly this is a kink
that needs to be ironed out somehow.  Created crbug.com/155151 to continue with
this issue.

-            renderSurface->setClipRect(clipRectFromAncestor);
-        else
-            renderSurface->setClipRect(IntRect());
-
         // Don't clip if the layer is reflected as the reflection shouldn't be
         // clipped. If the layer is animating, then the surface's transform to
         // its target is not known on the main thread, and we should not use it
         // to clip.
         if (!layer->replicaLayer() && transformToParentIsKnown(layer)) {
             // Note, it is correct to use ancestorClipsSubtree here, because we are looking at this layer's renderSurface, not the layer itself.
             if (ancestorClipsSubtree && !clippedContentRect.isEmpty()) {
                 IntRect surfaceClipRect = CCLayerTreeHostCommon::calculateVisibleRect(renderSurface->clipRect(), clippedContentRect, renderSurface->drawTransform());
                 clippedContentRect.intersect(surfaceClipRect);
             }
@@ skipping 58 matching lines @@
 
     if (layer->renderSurface())
         drawableContentRectOfSubtree = enclosingIntRect(layer->renderSurface()->drawableContentRect());
     else
         drawableContentRectOfSubtree = localDrawableContentRectOfSubtree;
 
     if (layer->hasContributingDelegatedRenderPasses())
         layer->renderTarget()->renderSurface()->addContributingDelegatedRenderPassLayer(layer);
 }
 
-// FIXME: Instead of using the following function to set visibility rects on a second
-// tree pass, revise calculateVisibleContentRect() so that this can be done in a single
-// pass inside calculateDrawTransformsInternal<>().
-template<typename LayerType, typename LayerList, typename RenderSurfaceType>
-static void calculateVisibleRectsInternal(const LayerList& renderSurfaceLayerList)
-{
-    // Use BackToFront since it's cheap and this isn't order-dependent.
-    typedef CCLayerIterator<LayerType, LayerList, RenderSurfaceType, CCLayerIteratorActions::BackToFront> CCLayerIteratorType;
-
-    CCLayerIteratorType end = CCLayerIteratorType::end(&renderSurfaceLayerList);
-    for (CCLayerIteratorType it = CCLayerIteratorType::begin(&renderSurfaceLayerList); it != end; ++it) {
-        if (it.representsTargetRenderSurface()) {
-            LayerType* maskLayer = it->maskLayer();
-            if (maskLayer)
-                maskLayer->setVisibleContentRect(IntRect(IntPoint(), it->contentBounds()));
-            LayerType* replicaMaskLayer = it->replicaLayer() ? it->replicaLayer()->maskLayer() : 0;
-            if (replicaMaskLayer)
-                replicaMaskLayer->setVisibleContentRect(IntRect(IntPoint(), it->contentBounds()));
-        } else if (it.representsItself()) {
-            IntRect visibleContentRect = calculateVisibleContentRect(*it);
-            it->setVisibleContentRect(visibleContentRect);
-        }
-    }
-}
-
 void CCLayerTreeHostCommon::calculateDrawTransforms(LayerChromium* rootLayer, const IntSize& deviceViewportSize, float deviceScaleFactor, int maxTextureSize, Vector<RefPtr<LayerChromium> >& renderSurfaceLayerList)
 {
     IntRect totalDrawableContentRect;
     WebTransformationMatrix identityMatrix;
     WebTransformationMatrix deviceScaleTransform;
     deviceScaleTransform.scale(deviceScaleFactor);
 
     setupRootLayerAndSurfaceForRecursion<LayerChromium, Vector<RefPtr<LayerChromium> > >(rootLayer, renderSurfaceLayerList, deviceViewportSize);
 
     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);
 }
 
 void CCLayerTreeHostCommon::calculateDrawTransforms(CCLayerImpl* rootLayer, const IntSize& deviceViewportSize, float deviceScaleFactor, CCLayerSorter* layerSorter, int maxTextureSize, Vector<CCLayerImpl*>& renderSurfaceLayerList)
 {
     IntRect totalDrawableContentRect;
     WebTransformationMatrix identityMatrix;
     WebTransformationMatrix deviceScaleTransform;
     deviceScaleTransform.scale(deviceScaleFactor);
 
     setupRootLayerAndSurfaceForRecursion<CCLayerImpl, Vector<CCLayerImpl*> >(rootLayer, renderSurfaceLayerList, deviceViewportSize);
 
     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);
 }
 
-void CCLayerTreeHostCommon::calculateVisibleRects(Vector<RefPtr<LayerChromium> >& renderSurfaceLayerList)
-{
-    calculateVisibleRectsInternal<LayerChromium, Vector<RefPtr<LayerChromium> >, RenderSurfaceChromium>(renderSurfaceLayerList);
-}
-
-void CCLayerTreeHostCommon::calculateVisibleRects(Vector<CCLayerImpl*>& renderSurfaceLayerList)
-{
-    calculateVisibleRectsInternal<CCLayerImpl, Vector<CCLayerImpl*>, CCRenderSurface>(renderSurfaceLayerList);
-}
-
 static bool pointHitsRect(const IntPoint& viewportPoint, const WebTransformationMatrix& localSpaceToScreenSpaceTransform, FloatRect localSpaceRect)
 {
     // If the transform is not invertible, then assume that this point doesn't hit this rect.
     if (!localSpaceToScreenSpaceTransform.isInvertible())
         return false;
 
     // Transform the hit test point from screen space to the local space of the given rect.
     bool clipped = false;
     FloatPoint hitTestPointInLocalSpace = CCMathUtil::projectPoint(localSpaceToScreenSpaceTransform.inverse(), FloatPoint(viewportPoint), clipped);
 
@@ skipping 52 matching lines @@
 
         foundLayer = currentLayer;
         break;
     }
 
     // This can potentially return 0, which means the viewportPoint did not successfully hit test any layers, not even the root layer.
     return foundLayer;
 }
 
 } // namespace cc