flexbox flexing implementation should match the spec
authortony@chromium.org <tony@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Mar 2012 21:45:58 +0000 (21:45 +0000)
committertony@chromium.org <tony@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Mar 2012 21:45:58 +0000 (21:45 +0000)
https://bugs.webkit.org/show_bug.cgi?id=70796

Reviewed by Ojan Vafai.

Source/WebCore:

Match the algorithm in the spec. Handling min/max constraints are slightly improved.
http://dev.w3.org/csswg/css3-flexbox/#resolve-the-flexible-lengths

New test cases in css3/flexbox/flex-algorithm-min-max.html.

* rendering/RenderFlexibleBox.cpp:
(WebCore::adjustFlexSizeForMinAndMax): Step 5 of resolving flexible lengths.
(WebCore):
(WebCore::RenderFlexibleBox::Violation::Violation):
(RenderFlexibleBox::Violation):
(WebCore::RenderFlexibleBox::freezeViolations): Used by step 6.
(WebCore::RenderFlexibleBox::resolveFlexibleLengths):
* rendering/RenderFlexibleBox.h:

LayoutTests:

* css3/flexbox/flex-algorithm-min-max-expected.txt:
* css3/flexbox/flex-algorithm-min-max.html:

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

LayoutTests/ChangeLog
LayoutTests/css3/flexbox/flex-algorithm-min-max-expected.txt
LayoutTests/css3/flexbox/flex-algorithm-min-max.html
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderFlexibleBox.cpp
Source/WebCore/rendering/RenderFlexibleBox.h

index 2061197..268a872 100644 (file)
@@ -1,3 +1,13 @@
+2012-03-22  Tony Chang  <tony@chromium.org>
+
+        flexbox flexing implementation should match the spec
+        https://bugs.webkit.org/show_bug.cgi?id=70796
+
+        Reviewed by Ojan Vafai.
+
+        * css3/flexbox/flex-algorithm-min-max-expected.txt:
+        * css3/flexbox/flex-algorithm-min-max.html:
+
 2012-03-22  Robert Hogan  <robert@webkit.org>
 
         Make reference result added in r111755 compatible with Qt and GTK ports.
index 0b66284..fa575b0 100644 (file)
@@ -75,5 +75,31 @@ if (window.layoutTestController)
   <div data-expected-width="100" style="width: -webkit-flex(1 0 50%); max-width: 100px"></div>
 </div>
 
+<div class="flexbox" style="width: 200px">
+  <div data-expected-width="150" style="width: -webkit-flex(1); min-width: 150px"></div>
+  <div data-expected-width="50" style="width: -webkit-flex(1); max-width: 90px"></div>
+</div>
+
+<div class="flexbox" style="width: 200px">
+  <div data-expected-width="150" style="width: -webkit-flex(1); min-width: 120px"></div>
+  <div data-expected-width="50" style="width: -webkit-flex(1); max-width: 50px"></div>
+</div>
+
+<div class="flexbox" style="width: 200px">
+  <div data-expected-width="100" style="width: -webkit-flex(1); min-width: 100px"></div>
+  <div data-expected-width="100" style="width: -webkit-flex(3);"></div>
+</div>
+
+<div class="flexbox" style="width: 200px">
+  <div data-expected-width="150" style="width: -webkit-flex(1 50px); min-width: 100px"></div>
+  <div data-expected-width="50" style="width: -webkit-flex(1 100px); max-width: 50px"></div>
+</div>
+
+<div class="flexbox">
+  <div data-expected-width="80" style="width: -webkit-flex(1)"></div>
+  <div data-expected-width="160" style="width: -webkit-flex(2)"></div>
+  <div data-expected-width="360" style="width: -webkit-flex(1); min-width: 360px"></div>
+</div>
+
 </body>
 </html>
index 8c28064..68ef05b 100644 (file)
@@ -1,3 +1,24 @@
+2012-03-22  Tony Chang  <tony@chromium.org>
+
+        flexbox flexing implementation should match the spec
+        https://bugs.webkit.org/show_bug.cgi?id=70796
+
+        Reviewed by Ojan Vafai.
+
+        Match the algorithm in the spec. Handling min/max constraints are slightly improved.
+        http://dev.w3.org/csswg/css3-flexbox/#resolve-the-flexible-lengths
+
+        New test cases in css3/flexbox/flex-algorithm-min-max.html.
+
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::adjustFlexSizeForMinAndMax): Step 5 of resolving flexible lengths.
+        (WebCore):
+        (WebCore::RenderFlexibleBox::Violation::Violation):
+        (RenderFlexibleBox::Violation):
+        (WebCore::RenderFlexibleBox::freezeViolations): Used by step 6.
+        (WebCore::RenderFlexibleBox::resolveFlexibleLengths):
+        * rendering/RenderFlexibleBox.h:
+
 2012-03-22  Emil A Eklund  <eae@chromium.org>
 
         Unreviewed, add missing import.
index f877985..71e8cd2 100644 (file)
@@ -117,6 +117,17 @@ struct RenderFlexibleBox::LineContext {
     LayoutUnit maxAscent;
 };
 
+struct RenderFlexibleBox::Violation {
+    Violation(RenderBox* child, LayoutUnit childSize)
+        : child(child)
+        , childSize(childSize)
+    {
+    }
+
+    RenderBox* child;
+    LayoutUnit childSize;
+};
+
 
 RenderFlexibleBox::RenderFlexibleBox(Node* node)
     : RenderBlock(node)
@@ -681,6 +692,19 @@ LayoutUnit RenderFlexibleBox::lineBreakLength()
     return height;
 }
 
+LayoutUnit RenderFlexibleBox::adjustChildSizeForMinAndMax(RenderBox* child, LayoutUnit childSize, LayoutUnit flexboxAvailableContentExtent)
+{
+    Length max = isHorizontalFlow() ? child->style()->maxWidth() : child->style()->maxHeight();
+    Length min = isHorizontalFlow() ? child->style()->minWidth() : child->style()->minHeight();
+    // FIXME: valueForLength isn't quite right in quirks mode: percentage heights should check parents until a value is found.
+    // https://bugs.webkit.org/show_bug.cgi?id=81809
+    if (max.isSpecified() && childSize > valueForLength(max, flexboxAvailableContentExtent))
+        childSize = valueForLength(max, flexboxAvailableContentExtent);
+    if (min.isSpecified() && childSize < valueForLength(min, flexboxAvailableContentExtent))
+        childSize = valueForLength(min, flexboxAvailableContentExtent);
+    return childSize;
+}
+
 bool RenderFlexibleBox::computeNextFlexLine(FlexOrderIterator& iterator, OrderedFlexItemList& orderedChildren, LayoutUnit& preferredMainAxisExtent, float& totalPositiveFlexibility, float& totalNegativeFlexibility, LayoutUnit& minMaxAppliedMainAxisExtent)
 {
     orderedChildren.clear();
@@ -711,26 +735,33 @@ bool RenderFlexibleBox::computeNextFlexLine(FlexOrderIterator& iterator, Ordered
         totalPositiveFlexibility += positiveFlexForChild(child);
         totalNegativeFlexibility += negativeFlexForChild(child);
 
-        LayoutUnit childMinMaxAppliedMainAxisExtent = childMainAxisExtent;
-        // FIXME: valueForLength isn't quite right in quirks mode: percentage heights should check parents until a value is found.
-        // https://bugs.webkit.org/show_bug.cgi?id=81809
-        Length maxLength = isHorizontalFlow() ? child->style()->maxWidth() : child->style()->maxHeight();
-        if (maxLength.isSpecified() && childMinMaxAppliedMainAxisExtent > valueForLength(maxLength, flexboxAvailableContentExtent))
-            childMinMaxAppliedMainAxisExtent = valueForLength(maxLength, flexboxAvailableContentExtent);
-        Length minLength = isHorizontalFlow() ? child->style()->minWidth() : child->style()->minHeight();
-        if (minLength.isSpecified() && childMinMaxAppliedMainAxisExtent < valueForLength(minLength, flexboxAvailableContentExtent))
-            childMinMaxAppliedMainAxisExtent = valueForLength(minLength, flexboxAvailableContentExtent);
+        LayoutUnit childMinMaxAppliedMainAxisExtent = adjustChildSizeForMinAndMax(child, childMainAxisExtent, flexboxAvailableContentExtent);
         minMaxAppliedMainAxisExtent += childMinMaxAppliedMainAxisExtent - childMainAxisExtent + childMainAxisMarginBoxExtent;
     }
     return true;
 }
 
+void RenderFlexibleBox::freezeViolations(const WTF::Vector<Violation>& violations, LayoutUnit& availableFreeSpace, float& totalPositiveFlexibility, float& totalNegativeFlexibility, InflexibleFlexItemSize& inflexibleItems)
+{
+    for (size_t i = 0; i < violations.size(); ++i) {
+        RenderBox* child = violations[i].child;
+        LayoutUnit childSize = violations[i].childSize;
+        availableFreeSpace -= childSize - preferredMainAxisContentExtentForChild(child);
+        totalPositiveFlexibility -= positiveFlexForChild(child);
+        totalNegativeFlexibility -= negativeFlexForChild(child);
+        inflexibleItems.set(child, childSize);
+    }
+}
+
 // Returns true if we successfully ran the algorithm and sized the flex items.
-bool RenderFlexibleBox::resolveFlexibleLengths(FlexSign, const OrderedFlexItemList& children, LayoutUnit& availableFreeSpace, float& totalPositiveFlexibility, float& totalNegativeFlexibility, InflexibleFlexItemSize& inflexibleItems, WTF::Vector<LayoutUnit>& childSizes)
+bool RenderFlexibleBox::resolveFlexibleLengths(FlexSign flexSign, const OrderedFlexItemList& children, LayoutUnit& availableFreeSpace, float& totalPositiveFlexibility, float& totalNegativeFlexibility, InflexibleFlexItemSize& inflexibleItems, WTF::Vector<LayoutUnit>& childSizes)
 {
     childSizes.clear();
 
     LayoutUnit flexboxAvailableContentExtent = mainAxisContentExtent();
+    LayoutUnit totalViolation = 0;
+    WTF::Vector<Violation> minViolations;
+    WTF::Vector<Violation> maxViolations;
     for (size_t i = 0; i < children.size(); ++i) {
         RenderBox* child = children[i];
         if (child->isPositioned()) {
@@ -738,40 +769,30 @@ bool RenderFlexibleBox::resolveFlexibleLengths(FlexSign, const OrderedFlexItemLi
             continue;
         }
 
-        LayoutUnit childPreferredSize;
         if (inflexibleItems.contains(child))
-            childPreferredSize = inflexibleItems.get(child);
+            childSizes.append(inflexibleItems.get(child));
         else {
-            childPreferredSize = preferredMainAxisContentExtentForChild(child);
-            if (availableFreeSpace > 0 && totalPositiveFlexibility > 0) {
-                childPreferredSize += lroundf(availableFreeSpace * positiveFlexForChild(child) / totalPositiveFlexibility);
-
-                Length childLogicalMaxWidth = isHorizontalFlow() ? child->style()->maxWidth() : child->style()->maxHeight();
-                if (childLogicalMaxWidth.isSpecified() && childPreferredSize > valueForLength(childLogicalMaxWidth, flexboxAvailableContentExtent)) {
-                    childPreferredSize = valueForLength(childLogicalMaxWidth, flexboxAvailableContentExtent);
-                    availableFreeSpace -= childPreferredSize - preferredMainAxisContentExtentForChild(child);
-                    totalPositiveFlexibility -= positiveFlexForChild(child);
-
-                    inflexibleItems.set(child, childPreferredSize);
-                    return false;
-                }
-            } else if (availableFreeSpace < 0 && totalNegativeFlexibility > 0) {
-                childPreferredSize += lroundf(availableFreeSpace * negativeFlexForChild(child) / totalNegativeFlexibility);
-
-                Length childLogicalMinWidth = isHorizontalFlow() ? child->style()->minWidth() : child->style()->minHeight();
-                if (childLogicalMinWidth.isSpecified() && childPreferredSize < valueForLength(childLogicalMinWidth, flexboxAvailableContentExtent)) {
-                    childPreferredSize = valueForLength(childLogicalMinWidth, flexboxAvailableContentExtent);
-                    availableFreeSpace += preferredMainAxisContentExtentForChild(child) - childPreferredSize;
-                    totalNegativeFlexibility -= negativeFlexForChild(child);
-
-                    inflexibleItems.set(child, childPreferredSize);
-                    return false;
-                }
-            }
+            LayoutUnit childSize = preferredMainAxisContentExtentForChild(child);
+            if (availableFreeSpace > 0 && totalPositiveFlexibility > 0 && flexSign == PositiveFlexibility)
+                childSize += lroundf(availableFreeSpace * positiveFlexForChild(child) / totalPositiveFlexibility);
+            else if (availableFreeSpace < 0 && totalNegativeFlexibility > 0  && flexSign == NegativeFlexibility)
+                childSize += lroundf(availableFreeSpace * negativeFlexForChild(child) / totalNegativeFlexibility);
+
+            LayoutUnit adjustedChildSize = adjustChildSizeForMinAndMax(child, childSize, flexboxAvailableContentExtent);
+            childSizes.append(adjustedChildSize);
+
+            LayoutUnit violation = adjustedChildSize - childSize;
+            if (violation > 0)
+                minViolations.append(Violation(child, adjustedChildSize));
+            else if (violation < 0)
+                maxViolations.append(Violation(child, adjustedChildSize));
+            totalViolation += violation;
         }
-        childSizes.append(childPreferredSize);
     }
-    return true;
+
+    if (totalViolation)
+        freezeViolations(totalViolation < 0 ? maxViolations : minViolations, availableFreeSpace, totalPositiveFlexibility, totalNegativeFlexibility, inflexibleItems);
+    return !totalViolation;
 }
 
 static LayoutUnit initialPackingOffset(LayoutUnit availableFreeSpace, EFlexPack flexPack, size_t numberOfChildren)
@@ -846,6 +867,7 @@ static EFlexAlign flexAlignForChild(RenderBox* child)
 
 void RenderFlexibleBox::layoutAndPlaceChildren(LayoutUnit& crossAxisOffset, const OrderedFlexItemList& children, const WTF::Vector<LayoutUnit>& childSizes, LayoutUnit availableFreeSpace, WTF::Vector<LineContext>& lineContexts)
 {
+    ASSERT(childSizes.size() == children.size());
     LayoutUnit mainAxisOffset = flowAwareBorderStart() + flowAwarePaddingStart();
     mainAxisOffset += initialPackingOffset(availableFreeSpace, style()->flexPack(), childSizes.size());
     if (style()->flexDirection() == FlowRowReverse)
index d9b76e5..c65e6dc 100644 (file)
@@ -62,6 +62,7 @@ private:
     typedef WTF::Vector<RenderBox*> OrderedFlexItemList;
 
     struct LineContext;
+    struct Violation;
 
     bool hasOrthogonalFlow(RenderBox* child) const;
     bool isColumnFlow() const;
@@ -109,9 +110,13 @@ private:
 
     void computeMainAxisPreferredSizes(bool relayoutChildren, FlexOrderHashSet&);
     LayoutUnit lineBreakLength();
+    LayoutUnit adjustChildSizeForMinAndMax(RenderBox*, LayoutUnit childSize, LayoutUnit flexboxAvailableContentExtent);
     bool computeNextFlexLine(FlexOrderIterator&, OrderedFlexItemList& orderedChildren, LayoutUnit& preferredMainAxisExtent, float& totalPositiveFlexibility, float& totalNegativeFlexibility, LayoutUnit& minMaxAppliedMainAxisExtent);
     LayoutUnit computeAvailableFreeSpace(LayoutUnit preferredMainAxisExtent);
+
     bool resolveFlexibleLengths(FlexSign, const OrderedFlexItemList&, LayoutUnit& availableFreeSpace, float& totalPositiveFlexibility, float& totalNegativeFlexibility, InflexibleFlexItemSize&, WTF::Vector<LayoutUnit>& childSizes);
+    void freezeViolations(const WTF::Vector<Violation>&, LayoutUnit& availableFreeSpace, float& totalPositiveFlexibility, float& totalNegativeFlexibility, InflexibleFlexItemSize&);
+
     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, WTF::Vector<LineContext>& lineContexts);