[New Multicolumn] Make layer bounding boxes visual and relative to "root".

Source/core/rendering/RenderLayer.cpp

Index: Source/core/rendering/RenderLayer.cpp
diff --git a/Source/core/rendering/RenderLayer.cpp b/Source/core/rendering/RenderLayer.cpp
index 719dcefebe6bcf23d038fe433ca6d52844b70ae0..5fb7a52daea24eb356ae4189f9b5e91d20bbea87 100644
--- a/Source/core/rendering/RenderLayer.cpp
+++ b/Source/core/rendering/RenderLayer.cpp
@@ -415,6 +415,38 @@ static bool checkContainingBlockChainForPagination(RenderLayerModelObject* rende
     return true;
 }
 
+// Convert a bounding box from flow thread coordinates, relative to |layer|, to visual coordinates, relative to |ancestorLayer|.
+static void convertFromFlowThreadToVisualBoundingBoxInAncestor(const RenderLayer* layer, const RenderLayer* ancestorLayer, LayoutRect& rect)
+{
+    RenderLayer* paginationLayer = layer->enclosingPaginationLayer();
+    ASSERT(paginationLayer);
+    RenderFlowThread* flowThread = toRenderFlowThread(paginationLayer->renderer());
+    LayoutPoint offsetWithinPaginationLayer;
+    layer->convertToLayerCoords(paginationLayer, offsetWithinPaginationLayer);
+
+    // First make the flow thread rectangle relative to the flow thread, not to |layer|.
+    rect.moveBy(offsetWithinPaginationLayer);

[chrishtr, 2014/10/13 16:03:34]

Why is this just an offset in all cases? What if there is a transform between?

[Julien - ping for review, 2014/10/13 19:25:03]

convertToLayerCoords is expected not to call across transformed layers as it
wouldn't work (which is what you're pointing out). In practice however this
invariant doesn't hold (I tried enforcing it a year or 2 ago and it's violated
all the time) so I don't know how our transform code even works.

[chrishtr, 2014/10/14 17:10:20]

Yuck. Yes you're right. convertToLayerCoords does not understand transforms. The
sad thing is that there is code in RenderObject that does all of this, and
convertToLayerCoords appears to be duplicating it.

+
+    // Then make the rectangle visual, relative to the fragmentation context. Split our box up into
+    // the actual fragment boxes that render in the columns/pages and unite those together to get
+    // our true bounding box.
+    rect = flowThread->fragmentsBoundingBox(rect);
+
+    // Finally, make the visual rectangle relative to |ancestorLayer|.
+    // FIXME: nested fragmentation contexts. For now just give up if there are different pagination layers involved.

[Julien - ping for review, 2014/10/13 19:25:03]

Let's put the bug number for that as it will make it easier to spot this FIXME.

[mstensho (USE GERRIT), 2014/10/13 20:47:25]

Done.

+    if (!ancestorLayer->enclosingPaginationLayer() || ancestorLayer->enclosingPaginationLayer() != paginationLayer) {
+        // The easy case. The ancestor layer is not within the pagination layer.
+        paginationLayer->convertToLayerCoords(ancestorLayer, rect);
+        return;
+    }
+    // The ancestor layer is also inside the pagination layer, so we need to subtract the visual
+    // distance from the ancestor layer to the pagination layer.
+    LayoutPoint offsetFromPaginationLayerToAncestor;
+    ancestorLayer->convertToLayerCoords(paginationLayer, offsetFromPaginationLayerToAncestor);
+    offsetFromPaginationLayerToAncestor = flowThread->flowThreadPointToVisualPoint(offsetFromPaginationLayerToAncestor);
+    rect.moveBy(-offsetFromPaginationLayerToAncestor);

[chrishtr, 2014/10/13 16:03:34]

Same question here. Why not use something like
RenderLayer::mapRectToPaintInvalidationBacking? (but suitably generalized)

[Julien - ping for review, 2014/10/13 19:25:02]

In general, RenderLayer::mapRectToPaintInvalidationBacking doesn't work because
localToContainerQuad assumes that the 'ancestorLayer' is inside your containing
blocks' chain, which doesn't hold for RenderLayers.

[chrishtr, 2014/10/14 17:10:20]

Why doesn't that hold?

[Julien - ping for review, 2014/10/14 17:52:58]

|ancestorLayer| is one of your parents but it can be a layer that is not in your
containing block chain. Here is an example:

<div style="position: relative;">
    <div style="opacity: .5;">
        <div style="position: absolute;"></div>
    </div>
</div>

The absolutely positioned layer and the half-transparent layer have a child <->
parent relationship in the RenderLayer tree but the opacity layer doesn't belong
to the absolutely positioned layer's containing block chain. If you look at
accumulateOffsetTowardsAncestor, it explicitly handles such a case (which
accounts for a lot of the complexity in the function).

+}
+
 bool RenderLayer::useRegionBasedColumns() const
 {
     return renderer()->document().regionBasedColumnsEnabled();
@@ -2242,16 +2274,7 @@ LayoutRect RenderLayer::fragmentsBoundingBox(const RenderLayer* ancestorLayer) c
         return physicalBoundingBox(ancestorLayer);
 
     LayoutRect result = flippedLogicalBoundingBox();
-
-    // Split our box up into the actual fragment boxes that render in the columns/pages and unite those together to
-    // get our true bounding box.
-    LayoutPoint offsetWithinPaginationLayer;
-    convertToLayerCoords(enclosingPaginationLayer(), offsetWithinPaginationLayer);
-    result.moveBy(offsetWithinPaginationLayer);
-
-    RenderFlowThread* enclosingFlowThread = toRenderFlowThread(enclosingPaginationLayer()->renderer());
-    result = enclosingFlowThread->fragmentsBoundingBox(result);
-    enclosingPaginationLayer()->convertToLayerCoords(ancestorLayer, result);
+    convertFromFlowThreadToVisualBoundingBoxInAncestor(this, ancestorLayer, result);
     return result;
 }
 
@@ -2316,40 +2339,36 @@ LayoutRect RenderLayer::boundingBoxForCompositing(const RenderLayer* ancestorLay
 
     const bool shouldIncludeTransform = paintsWithTransform(PaintBehaviorNormal) || (options == ApplyBoundsChickenEggHacks && transform());

[Julien - ping for review, 2014/10/13 19:25:02]

This looks like it should be moved when needed below.

[mstensho (USE GERRIT), 2014/10/13 20:47:25]

Done.

 
-    LayoutRect localClipRect = clipper().localClipRect();
-    if (localClipRect != PaintInfo::infiniteRect()) {
-        if (shouldIncludeTransform)
-            localClipRect = transform()->mapRect(localClipRect);
+    LayoutRect result = clipper().localClipRect();
+    if (result == PaintInfo::infiniteRect()) {
+        LayoutPoint origin;
+        result = physicalBoundingBox(ancestorLayer, &origin);
 
-        LayoutPoint delta;
-        convertToLayerCoords(ancestorLayer, delta);
-        localClipRect.moveBy(delta);
-        return localClipRect;
-    }
-
-    LayoutPoint origin;
-    LayoutRect result = physicalBoundingBox(ancestorLayer, &origin);
-
-    const_cast<RenderLayer*>(this)->stackingNode()->updateLayerListsIfNeeded();
+        const_cast<RenderLayer*>(this)->stackingNode()->updateLayerListsIfNeeded();
 
-    // Reflections are implemented with RenderLayers that hang off of the reflected layer. However,
-    // the reflection layer subtree does not include the subtree of the parent RenderLayer, so
-    // a recursive computation of stacking children yields no results. This breaks cases when there are stacking
-    // children of the parent, that need to be included in reflected composited bounds.
-    // Fix this by including composited bounds of stacking children of the reflected RenderLayer.
-    if (hasCompositedLayerMapping() && parent() && parent()->reflectionInfo() && parent()->reflectionInfo()->reflectionLayer() == this)
-        expandRectForReflectionAndStackingChildren(parent(), options, result);
-    else
-        expandRectForReflectionAndStackingChildren(this, options, result);
+        // Reflections are implemented with RenderLayers that hang off of the reflected layer. However,
+        // the reflection layer subtree does not include the subtree of the parent RenderLayer, so
+        // a recursive computation of stacking children yields no results. This breaks cases when there are stacking
+        // children of the parent, that need to be included in reflected composited bounds.
+        // Fix this by including composited bounds of stacking children of the reflected RenderLayer.
+        if (hasCompositedLayerMapping() && parent() && parent()->reflectionInfo() && parent()->reflectionInfo()->reflectionLayer() == this)
+            expandRectForReflectionAndStackingChildren(parent(), options, result);
+        else
+            expandRectForReflectionAndStackingChildren(this, options, result);
 
-    // FIXME: We can optimize the size of the composited layers, by not enlarging
-    // filtered areas with the outsets if we know that the filter is going to render in hardware.
-    // https://bugs.webkit.org/show_bug.cgi?id=81239
-    m_renderer->style()->filterOutsets().expandRect(result);
+        // FIXME: We can optimize the size of the composited layers, by not enlarging
+        // filtered areas with the outsets if we know that the filter is going to render in hardware.
+        // https://bugs.webkit.org/show_bug.cgi?id=81239
+        m_renderer->style()->filterOutsets().expandRect(result);
+    }
 
     if (shouldIncludeTransform)
         result = transform()->mapRect(result);
 
+    if (enclosingPaginationLayer()) {
+        convertFromFlowThreadToVisualBoundingBoxInAncestor(this, ancestorLayer, result);
+        return result;
+    }
     LayoutPoint delta;
     convertToLayerCoords(ancestorLayer, delta);
     result.moveBy(delta);