[New Multicolumn] Actual support for layout of column-span:all.

Source/core/rendering/RenderMultiColumnSet.cpp

 /*
  * Copyright (C) 2012 Apple Inc.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 53 matching lines @@
 
 RenderMultiColumnSet* RenderMultiColumnSet::previousSiblingMultiColumnSet() const
 {
     for (RenderObject* sibling = previousSibling(); sibling; sibling = sibling->previousSibling()) {
         if (sibling->isRenderMultiColumnSet())
             return toRenderMultiColumnSet(sibling);
     }
     return 0;
 }
 
+void RenderMultiColumnSet::setLogicalTopInFlowThread(LayoutUnit logicalTop)
+{
+    LayoutRect rect = flowThreadPortionRect();
+    if (isHorizontalWritingMode())
+        rect.setY(logicalTop);
+    else
+        rect.setX(logicalTop);

[Julien - ping for review, 2014/11/19 17:54:34]

It seems like the flowThreadPortionRect() is in logical coordinate as it is
inferred from the visual overflow. Could you explain the need for this code?

[mstensho (USE GERRIT), 2014/11/21 15:39:24]

flowThreadPortionRect() is in the same coordinate system as
RenderBox::m_frameRect, i.e. rather physical. (but both m_frameRect and the flow
thread portion rect have their block direction coordinate (either x or y)
flipped for reverse block direction mode (vertical-rl and horizontal-bt),
though). Visual overflow is in the same coordinate system, isn't it?

The logicalTop that we have here, on the other hand, is super-logical. It's a
block offset from the start of the flow thread, which starts at 0, and increases
as you add children, regardless of writing-mode.

[Julien - ping for review, 2014/11/26 21:09:00]

Visual overflow is in the same coordinate system indeed (which is what most of
the layout code manipulate).
OK but in this case, shouldn't we also check for isFlippingWritingMode() as this
is the primary determinant of whether the need to flip?

+    setFlowThreadPortionRect(rect);
+}
+
+void RenderMultiColumnSet::setLogicalBottomInFlowThread(LayoutUnit logicalBottom)
+{
+    LayoutRect rect = flowThreadPortionRect();
+    if (isHorizontalWritingMode())
+        rect.shiftMaxYEdgeTo(logicalBottom);
+    else
+        rect.shiftMaxXEdgeTo(logicalBottom);
+    setFlowThreadPortionRect(rect);
+}
+
+bool RenderMultiColumnSet::heightIsAuto() const
+{
+    RenderMultiColumnFlowThread* flowThread = multiColumnFlowThread();
+    if (!flowThread->isRenderPagedFlowThread()) {
+        if (multiColumnBlockFlow()->style()->columnFill() == ColumnFillBalance)
+            return true;
+        if (RenderMultiColumnSet* next = nextSiblingMultiColumnSet()) {
+            if (next->isRenderMultiColumnSpannerSet()) {
+                // If we're followed by a spanner, we need to balance.

[Julien - ping for review, 2014/11/19 17:54:34]

This comment (and the rest of the logic) shows that the name is not well done.
It seems like it should just be called needsBalancing() or something similar.

It's also super weird to have the only difference between this function and
requiresBalancing() is a type check. Can't we just squash them together?

[mstensho (USE GERRIT), 2014/11/21 15:39:24]

Callers of this method should really only want to know if the set height is
auto, and not worry about balancing. I found one call site now that (harmlessly)
assumed that auto height meant balancing. I changed it to calling
requiresBalancing(). That will result in an additional virtual method call, but
I suppose we can afford that for the sake of clarity.
That's a crucial type check. A spanner shouldn't be balanced, but its height may
still be auto.

This can be considered to be a result of letting RenderMultiColumnSpannerSet
inherit from RenderMultiColumnSet (see FIXME in RenderMultiColumnSpannerSet.h).
I think the two classes would be better off inheriting from some common base
class. That would be RenderRegion in the current code, but I'd rather get rid of
/ clean up / rename that one before cleaning up this.

Asking a spanner if it requires balancing is meaningless, and we'd probably be
better off avoiding calling requiresBalancing() on something that may
potentially be a spanner set.

+                return true;
+            }
+        }
+    }
+    return !flowThread->columnHeightAvailable();
+}
+
 LayoutSize RenderMultiColumnSet::flowThreadTranslationAtOffset(LayoutUnit blockOffset) const
 {
     unsigned columnIndex = columnIndexAtOffset(blockOffset);
     LayoutRect portionRect(flowThreadPortionRectAt(columnIndex));
     flipForWritingMode(portionRect);
     LayoutRect columnRect(columnRectAt(columnIndex));
     flipForWritingMode(columnRect);
     return contentBoxRect().location() + columnRect.location() - portionRect.location();
 }
 
@@ skipping 103 matching lines @@
     ASSERT(m_minSpaceShortage > 0); // We should never _shrink_ the height!
     ASSERT(m_minSpaceShortage != RenderFlowThread::maxLogicalHeight()); // If this happens, we probably have a bug.
     if (m_minSpaceShortage == RenderFlowThread::maxLogicalHeight())
         return m_columnHeight; // So bail out rather than looping infinitely.
 
     return m_columnHeight + m_minSpaceShortage;
 }
 
 void RenderMultiColumnSet::addContentRun(LayoutUnit endOffsetFromFirstPage)
 {
-    if (!multiColumnFlowThread()->heightIsAuto())
+    if (!requiresBalancing())
         return;
     if (!m_contentRuns.isEmpty() && endOffsetFromFirstPage <= m_contentRuns.last().breakOffset())
         return;
     // Append another item as long as we haven't exceeded used column count. What ends up in the
     // overflow area shouldn't affect column balancing.
     if (m_contentRuns.size() < usedColumnCount())
         m_contentRuns.append(ContentRun(endOffsetFromFirstPage));
 }
 
 bool RenderMultiColumnSet::recalculateColumnHeight(BalancedHeightCalculation calculationMode)
 {
-    if (previousSiblingMultiColumnSet()) {
-        // FIXME: column spanner layout is not yet implemented. Until it's in place, we only operate
-        // on the first set during layout. We need to ignore the others here, or assertions will
-        // fail.
-        return false;
+    LayoutUnit oldColumnHeight = m_columnHeight;
+
+    m_maxColumnHeight = calculateMaxColumnHeight();
+
+    if (heightIsAuto()) {
+        if (calculationMode == GuessFromFlowThreadPortion) {
+            // Post-process the content runs and find out where the implicit breaks will occur.
+            distributeImplicitBreaks();
+        }
+        LayoutUnit newColumnHeight = calculateColumnHeight(calculationMode);
+        setAndConstrainColumnHeight(newColumnHeight);
+        // After having calculated an initial column height, the multicol container typically needs at
+        // least one more layout pass with a new column height, but if a height was specified, we only
+        // need to do this if we think that we need less space than specified. Conversely, if we
+        // determined that the columns need to be as tall as the specified height of the container, we
+        // have already laid it out correctly, and there's no need for another pass.
+    } else {
+        // The position of the column set may have changed, in which case height available for
+        // columns may have changed as well.
+        setAndConstrainColumnHeight(m_columnHeight);
     }
-    ASSERT(multiColumnFlowThread()->heightIsAuto());
-
-    LayoutUnit oldColumnHeight = m_columnHeight;
-    if (calculationMode == GuessFromFlowThreadPortion) {
-        // Post-process the content runs and find out where the implicit breaks will occur.
-        distributeImplicitBreaks();
-    }
-    LayoutUnit newColumnHeight = calculateColumnHeight(calculationMode);
-    setAndConstrainColumnHeight(newColumnHeight);
-
-    // After having calculated an initial column height, the multicol container typically needs at
-    // least one more layout pass with a new column height, but if a height was specified, we only
-    // need to do this if we think that we need less space than specified. Conversely, if we
-    // determined that the columns need to be as tall as the specified height of the container, we
-    // have already laid it out correctly, and there's no need for another pass.
 
     // We can get rid of the content runs now, if we haven't already done so. They are only needed
     // to calculate the initial balanced column height. In fact, we have to get rid of them before
     // the next layout pass, since each pass will rebuild this.
     m_contentRuns.clear();
 
     if (m_columnHeight == oldColumnHeight)
         return false; // No change. We're done.
 
     m_minSpaceShortage = RenderFlowThread::maxLogicalHeight();
@@ skipping 14 matching lines @@
 
 void RenderMultiColumnSet::resetColumnHeight()
 {
     // Nuke previously stored minimum column height. Contents may have changed for all we know.
     m_minimumColumnHeight = 0;
 
     m_maxColumnHeight = calculateMaxColumnHeight();
 
     LayoutUnit oldColumnHeight = pageLogicalHeight();
 
-    if (multiColumnFlowThread()->heightIsAuto())
+    if (heightIsAuto())
         m_columnHeight = 0;
     else
         setAndConstrainColumnHeight(heightAdjustedForSetOffset(multiColumnFlowThread()->columnHeightAvailable()));
 
     if (pageLogicalHeight() != oldColumnHeight)
         setChildNeedsLayout(MarkOnlyThis);
 
     // Content runs are only needed in the initial layout pass, in order to find an initial column
     // height, and should have been deleted afterwards. We're about to rebuild the content runs, so
     // the list needs to be empty.
@@ skipping 353 matching lines @@
 
 void RenderMultiColumnSet::detachRegion()
 {
     if (m_flowThread) {
         m_flowThread->removeRegionFromThread(this);
         m_flowThread = 0;
     }
 }
 
 }