refactor flexbox in preparation for flex-line-pack
authortony@chromium.org <tony@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Mar 2012 17:16:30 +0000 (17:16 +0000)
committertony@chromium.org <tony@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Mar 2012 17:16:30 +0000 (17:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=81843

Reviewed by Ojan Vafai.

Replace WrapReverseContext with a vector of LineContexts that contain
the same information, plus values needed for flex-align.

alignChildren has been moved to after all the lines have been
positioned. We want to align children after flex-line-pack has changed
the size of each line to avoid unnecessary layouts.

Take 2: Remove the assert. If there are no children, then there are no
flex lines. Instead, assert that child is not null.

No new tests, just refactoring.

* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::LineContext::LineContext): New struct,
holds information needed for wrap-reverse and aligning children.
(RenderFlexibleBox::LineContext):
(WebCore::RenderFlexibleBox::layoutFlexItems): alignChildren after layout out all the lines rather than after each line.
(WebCore::RenderFlexibleBox::layoutAndPlaceChildren): don't alignChildren
(WebCore::RenderFlexibleBox::alignChildren): align all flex items, not just a line at a time.
(WebCore::RenderFlexibleBox::flipForWrapReverse): Update to use LineContext
* rendering/RenderFlexibleBox.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@111720 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderFlexibleBox.cpp
Source/WebCore/rendering/RenderFlexibleBox.h

index a7a905e..45d7a45 100644 (file)
@@ -1,3 +1,32 @@
+2012-03-22  Tony Chang  <tony@chromium.org>
+
+        refactor flexbox in preparation for flex-line-pack
+        https://bugs.webkit.org/show_bug.cgi?id=81843
+
+        Reviewed by Ojan Vafai.
+
+        Replace WrapReverseContext with a vector of LineContexts that contain
+        the same information, plus values needed for flex-align.
+
+        alignChildren has been moved to after all the lines have been
+        positioned. We want to align children after flex-line-pack has changed
+        the size of each line to avoid unnecessary layouts.
+
+        Take 2: Remove the assert. If there are no children, then there are no
+        flex lines. Instead, assert that child is not null.
+
+        No new tests, just refactoring.
+
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::LineContext::LineContext): New struct,
+        holds information needed for wrap-reverse and aligning children.
+        (RenderFlexibleBox::LineContext):
+        (WebCore::RenderFlexibleBox::layoutFlexItems): alignChildren after layout out all the lines rather than after each line.
+        (WebCore::RenderFlexibleBox::layoutAndPlaceChildren): don't alignChildren
+        (WebCore::RenderFlexibleBox::alignChildren): align all flex items, not just a line at a time.
+        (WebCore::RenderFlexibleBox::flipForWrapReverse): Update to use LineContext
+        * rendering/RenderFlexibleBox.h:
+
 2012-03-22  Allan Sandfeld Jensen  <allan.jensen@nokia.com>
 
         Event dispatching in XMLHttpRequestProgressEventThrottle should go through XMLHttpRequestProgressEventThrottle::dispatchEvent
index aa53c14..f877985 100644 (file)
@@ -102,38 +102,19 @@ private:
     Vector<int>::const_iterator m_orderValuesIterator;
 };
 
-struct RenderFlexibleBox::WrapReverseContext {
-    explicit WrapReverseContext(EFlexWrap flexWrap)
-        : isWrapReverse(flexWrap == FlexWrapReverse)
+struct RenderFlexibleBox::LineContext {
+    LineContext(LayoutUnit crossAxisOffset, LayoutUnit crossAxisExtent, size_t numberOfChildren, LayoutUnit maxAscent)
+        : crossAxisOffset(crossAxisOffset)
+        , crossAxisExtent(crossAxisExtent)
+        , numberOfChildren(numberOfChildren)
+        , maxAscent(maxAscent)
     {
     }
 
-    void addCrossAxisOffset(LayoutUnit offset)
-    {
-        if (!isWrapReverse)
-            return;
-        crossAxisOffsets.append(offset);
-    }
-
-    void addNumberOfChildrenOnLine(size_t numberOfChildren)
-    {
-        if (!isWrapReverse)
-            return;
-        childrenPerLine.append(numberOfChildren);
-    }
-
-    LayoutUnit lineCrossAxisDelta(size_t line, LayoutUnit crossAxisContentExtent) const
-    {
-        ASSERT(line + 1 < crossAxisOffsets.size());
-        LayoutUnit lineHeight = crossAxisOffsets[line + 1] - crossAxisOffsets[line];
-        LayoutUnit originalOffset = crossAxisOffsets[line] - crossAxisOffsets[0];
-        LayoutUnit newOffset = crossAxisContentExtent - originalOffset - lineHeight;
-        return newOffset - originalOffset;
-    }
-
-    WTF::Vector<LayoutUnit> crossAxisOffsets;
-    WTF::Vector<size_t> childrenPerLine;
-    bool isWrapReverse;
+    LayoutUnit crossAxisOffset;
+    LayoutUnit crossAxisExtent;
+    size_t numberOfChildren;
+    LayoutUnit maxAscent;
 };
 
 
@@ -607,12 +588,9 @@ void RenderFlexibleBox::layoutFlexItems(bool relayoutChildren)
     float totalPositiveFlexibility;
     float totalNegativeFlexibility;
     LayoutUnit minMaxAppliedMainAxisExtent;
+    WTF::Vector<LineContext> lineContexts;
     FlexOrderIterator flexIterator(this, flexOrderValues);
 
-    // For wrap-reverse, we need to layout as wrap, then reverse the lines. The next two arrays
-    // are some extra information so it's possible to reverse the lines.
-    WrapReverseContext wrapReverseContext(style()->flexWrap());
-
     LayoutUnit crossAxisOffset = flowAwareBorderBefore() + flowAwarePaddingBefore();
     while (computeNextFlexLine(flexIterator, orderedChildren, preferredMainAxisExtent, totalPositiveFlexibility, totalNegativeFlexibility, minMaxAppliedMainAxisExtent)) {
         LayoutUnit availableFreeSpace = computeAvailableFreeSpace(preferredMainAxisExtent);
@@ -624,15 +602,13 @@ void RenderFlexibleBox::layoutFlexItems(bool relayoutChildren)
             ASSERT(inflexibleItems.size() > 0);
         }
 
-        wrapReverseContext.addNumberOfChildrenOnLine(orderedChildren.size());
-        wrapReverseContext.addCrossAxisOffset(crossAxisOffset);
-        layoutAndPlaceChildren(crossAxisOffset, orderedChildren, childSizes, availableFreeSpace);
+        layoutAndPlaceChildren(crossAxisOffset, orderedChildren, childSizes, availableFreeSpace, lineContexts);
     }
 
-    if (wrapReverseContext.isWrapReverse) {
-        wrapReverseContext.addCrossAxisOffset(crossAxisOffset);
-        flipForWrapReverse(flexIterator, wrapReverseContext);
-    }
+    alignChildren(flexIterator, lineContexts);
+
+    if (style()->flexWrap() == FlexWrapReverse)
+        flipForWrapReverse(flexIterator, lineContexts);
 
     // direction:rtl + flex-direction:column means the cross-axis direction is flipped.
     flipForRightToLeftColumn(flexIterator);
@@ -868,7 +844,7 @@ static EFlexAlign flexAlignForChild(RenderBox* child)
     return align;
 }
 
-void RenderFlexibleBox::layoutAndPlaceChildren(LayoutUnit& crossAxisOffset, const OrderedFlexItemList& children, const WTF::Vector<LayoutUnit>& childSizes, LayoutUnit availableFreeSpace)
+void RenderFlexibleBox::layoutAndPlaceChildren(LayoutUnit& crossAxisOffset, const OrderedFlexItemList& children, const WTF::Vector<LayoutUnit>& childSizes, LayoutUnit availableFreeSpace, WTF::Vector<LineContext>& lineContexts)
 {
     LayoutUnit mainAxisOffset = flowAwareBorderStart() + flowAwarePaddingStart();
     mainAxisOffset += initialPackingOffset(availableFreeSpace, style()->flexPack(), childSizes.size());
@@ -930,8 +906,7 @@ void RenderFlexibleBox::layoutAndPlaceChildren(LayoutUnit& crossAxisOffset, cons
     }
 
     LayoutUnit lineCrossAxisExtent = isMultiline() ? maxChildCrossAxisExtent : crossAxisContentExtent();
-    alignChildren(children, lineCrossAxisExtent, maxAscent);
-
+    lineContexts.append(LineContext(crossAxisOffset, lineCrossAxisExtent, children.size(), maxAscent));
     crossAxisOffset += lineCrossAxisExtent;
 }
 
@@ -976,49 +951,63 @@ void RenderFlexibleBox::adjustAlignmentForChild(RenderBox* child, LayoutUnit del
         child->repaintDuringLayoutIfMoved(oldRect);
 }
 
-void RenderFlexibleBox::alignChildren(const OrderedFlexItemList& children, LayoutUnit lineCrossAxisExtent, LayoutUnit maxAscent)
+void RenderFlexibleBox::alignChildren(FlexOrderIterator& iterator, const WTF::Vector<LineContext>& lineContexts)
 {
-    LayoutUnit minMarginAfterBaseline = std::numeric_limits<LayoutUnit>::max();
-
-    for (size_t i = 0; i < children.size(); ++i) {
-        RenderBox* child = children[i];
-        switch (flexAlignForChild(child)) {
-        case AlignAuto:
-            ASSERT_NOT_REACHED();
-            break;
-        case AlignStretch: {
-            applyStretchAlignmentToChild(child, lineCrossAxisExtent);
-            // Since wrap-reverse flips cross start and cross end, strech children should be aligned with the cross end.
-            if (style()->flexWrap() == FlexWrapReverse)
+    // Keep track of the space between the baseline edge and the after edge of the box for each line.
+    WTF::Vector<LayoutUnit> minMarginAfterBaselines;
+
+    RenderBox* child = iterator.first();
+    for (size_t lineNumber = 0; lineNumber < lineContexts.size(); ++lineNumber) {
+        LayoutUnit minMarginAfterBaseline = std::numeric_limits<LayoutUnit>::max();
+        LayoutUnit lineCrossAxisExtent = lineContexts[lineNumber].crossAxisExtent;
+        LayoutUnit maxAscent = lineContexts[lineNumber].maxAscent;
+
+        for (size_t childNumber = 0; childNumber < lineContexts[lineNumber].numberOfChildren; ++childNumber, child = iterator.next()) {
+            ASSERT(child);
+            switch (flexAlignForChild(child)) {
+            case AlignAuto:
+                ASSERT_NOT_REACHED();
+                break;
+            case AlignStretch: {
+                applyStretchAlignmentToChild(child, lineCrossAxisExtent);
+                // Since wrap-reverse flips cross start and cross end, strech children should be aligned with the cross end.
+                if (style()->flexWrap() == FlexWrapReverse)
+                    adjustAlignmentForChild(child, availableAlignmentSpaceForChild(lineCrossAxisExtent, child));
+                break;
+            }
+            case AlignStart:
+                break;
+            case AlignEnd:
                 adjustAlignmentForChild(child, availableAlignmentSpaceForChild(lineCrossAxisExtent, child));
-            break;
-        }
-        case AlignStart:
-            break;
-        case AlignEnd:
-            adjustAlignmentForChild(child, availableAlignmentSpaceForChild(lineCrossAxisExtent, child));
-            break;
-        case AlignCenter:
-            adjustAlignmentForChild(child, availableAlignmentSpaceForChild(lineCrossAxisExtent, child) / 2);
-            break;
-        case AlignBaseline: {
-            LayoutUnit ascent = marginBoxAscentForChild(child);
-            LayoutUnit startOffset = maxAscent - ascent;
-            adjustAlignmentForChild(child, startOffset);
-
-            if (style()->flexWrap() == FlexWrapReverse)
-                minMarginAfterBaseline = std::min(minMarginAfterBaseline, availableAlignmentSpaceForChild(lineCrossAxisExtent, child) - startOffset);
-            break;
-        }
+                break;
+            case AlignCenter:
+                adjustAlignmentForChild(child, availableAlignmentSpaceForChild(lineCrossAxisExtent, child) / 2);
+                break;
+            case AlignBaseline: {
+                LayoutUnit ascent = marginBoxAscentForChild(child);
+                LayoutUnit startOffset = maxAscent - ascent;
+                adjustAlignmentForChild(child, startOffset);
+
+                if (style()->flexWrap() == FlexWrapReverse)
+                    minMarginAfterBaseline = std::min(minMarginAfterBaseline, availableAlignmentSpaceForChild(lineCrossAxisExtent, child) - startOffset);
+                break;
+            }
+            }
         }
+        minMarginAfterBaselines.append(minMarginAfterBaseline);
     }
 
+    if (style()->flexWrap() != FlexWrapReverse)
+        return;
+
     // wrap-reverse flips the cross axis start and end. For baseline alignment, this means we
     // need to align the after edge of baseline elements with the after edge of the flex line.
-    if (style()->flexWrap() == FlexWrapReverse && minMarginAfterBaseline) {
-        for (size_t i = 0; i < children.size(); ++i) {
-            RenderBox* child = children[i];
-            if (flexAlignForChild(child) == AlignBaseline)
+    child = iterator.first();
+    for (size_t lineNumber = 0; lineNumber < lineContexts.size(); ++lineNumber) {
+        LayoutUnit minMarginAfterBaseline = minMarginAfterBaselines[lineNumber];
+        for (size_t childNumber = 0; childNumber < lineContexts[lineNumber].numberOfChildren; ++childNumber, child = iterator.next()) {
+            ASSERT(child);
+            if (flexAlignForChild(child) == AlignBaseline && minMarginAfterBaseline)
                 adjustAlignmentForChild(child, minMarginAfterBaseline);
         }
     }
@@ -1063,26 +1052,26 @@ void RenderFlexibleBox::flipForRightToLeftColumn(FlexOrderIterator& iterator)
     }
 }
 
-void RenderFlexibleBox::flipForWrapReverse(FlexOrderIterator& iterator, const WrapReverseContext& wrapReverseContext)
+void RenderFlexibleBox::flipForWrapReverse(FlexOrderIterator& iterator, const WTF::Vector<LineContext>& lineContexts)
 {
     if (!isColumnFlow())
         computeLogicalHeight();
 
-    size_t currentChild = 0;
-    size_t lineNumber = 0;
     LayoutUnit contentExtent = crossAxisContentExtent();
-    for (RenderBox* child = iterator.first(); child; child = iterator.next()) {
-        LayoutPoint location = flowAwareLocationForChild(child);
-        location.setY(location.y() + wrapReverseContext.lineCrossAxisDelta(lineNumber, contentExtent));
-
-        LayoutRect oldRect = child->frameRect();
-        setFlowAwareLocationForChild(child, location);
-        if (!selfNeedsLayout() && child->checkForRepaintDuringLayout())
-            child->repaintDuringLayoutIfMoved(oldRect);
-
-        if (++currentChild == wrapReverseContext.childrenPerLine[lineNumber]) {
-            ++lineNumber;
-            currentChild = 0;
+    RenderBox* child = iterator.first();
+    for (size_t lineNumber = 0; lineNumber < lineContexts.size(); ++lineNumber) {
+        for (size_t childNumber = 0; childNumber < lineContexts[lineNumber].numberOfChildren; ++childNumber, child = iterator.next()) {
+            ASSERT(child);
+            LayoutPoint location = flowAwareLocationForChild(child);
+            LayoutUnit lineCrossAxisExtent = lineContexts[lineNumber].crossAxisExtent;
+            LayoutUnit originalOffset = lineContexts[lineNumber].crossAxisOffset - lineContexts[0].crossAxisOffset;
+            LayoutUnit newOffset = contentExtent - originalOffset - lineCrossAxisExtent;
+            location.setY(location.y() + newOffset - originalOffset);
+
+            LayoutRect oldRect = child->frameRect();
+            setFlowAwareLocationForChild(child, location);
+            if (!selfNeedsLayout() && child->checkForRepaintDuringLayout())
+                child->repaintDuringLayoutIfMoved(oldRect);
         }
     }
 }
index 2a63021..d9b76e5 100644 (file)
@@ -61,7 +61,7 @@ private:
     typedef WTF::HashMap<const RenderBox*, LayoutUnit> InflexibleFlexItemSize;
     typedef WTF::Vector<RenderBox*> OrderedFlexItemList;
 
-    struct WrapReverseContext;
+    struct LineContext;
 
     bool hasOrthogonalFlow(RenderBox* child) const;
     bool isColumnFlow() const;
@@ -114,12 +114,12 @@ private:
     bool resolveFlexibleLengths(FlexSign, const OrderedFlexItemList&, LayoutUnit& availableFreeSpace, float& totalPositiveFlexibility, float& totalNegativeFlexibility, InflexibleFlexItemSize&, WTF::Vector<LayoutUnit>& childSizes);
     void setLogicalOverrideSize(RenderBox* child, LayoutUnit childPreferredSize);
     void prepareChildForPositionedLayout(RenderBox* child, LayoutUnit mainAxisOffset, LayoutUnit crossAxisOffset);
-    void layoutAndPlaceChildren(LayoutUnit& crossAxisOffset, const OrderedFlexItemList&, const WTF::Vector<LayoutUnit>& childSizes, LayoutUnit availableFreeSpace);
+    void layoutAndPlaceChildren(LayoutUnit& crossAxisOffset, const OrderedFlexItemList&, const WTF::Vector<LayoutUnit>& childSizes, LayoutUnit availableFreeSpace, WTF::Vector<LineContext>& lineContexts);
     void layoutColumnReverse(const OrderedFlexItemList&, const WTF::Vector<LayoutUnit>& childSizes, LayoutUnit crossAxisOffset, LayoutUnit availableFreeSpace);
-    void alignChildren(const OrderedFlexItemList&, LayoutUnit lineCrossAxisExtent, LayoutUnit maxAscent);
+    void alignChildren(FlexOrderIterator&, const WTF::Vector<LineContext>&);
     void applyStretchAlignmentToChild(RenderBox*, LayoutUnit lineCrossAxisExtent);
     void flipForRightToLeftColumn(FlexOrderIterator&);
-    void flipForWrapReverse(FlexOrderIterator&, const WrapReverseContext&);
+    void flipForWrapReverse(FlexOrderIterator&, const WTF::Vector<LineContext>&);
 };
 
 } // namespace WebCore