flex-pack:center and flex-item-align:center should do true centering
authorojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Feb 2012 23:01:40 +0000 (23:01 +0000)
committerojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Feb 2012 23:01:40 +0000 (23:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=77385

Reviewed by Tony Chang.

Source/WebCore:

Also, removed passing totalPositiveFlexibility around. We don't
need to know about positive/negative flex once we run the flexing algorithm.
We used to need to know this in order to flex margins, but margins can
no longer be flexed.

Test: css3/flexbox/true-centering.html

* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::layoutFlexItems):
(WebCore::initialPackingOffset):
(WebCore::packingSpaceBetweenChildren):
(WebCore::RenderFlexibleBox::layoutAndPlaceChildren):
(WebCore::RenderFlexibleBox::layoutColumnReverse):
* rendering/RenderFlexibleBox.h:
(RenderFlexibleBox):

LayoutTests:

* css3/flexbox/true-centering-expected.txt: Added.
* css3/flexbox/true-centering.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/css3/flexbox/true-centering-expected.txt [new file with mode: 0644]
LayoutTests/css3/flexbox/true-centering.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderFlexibleBox.cpp
Source/WebCore/rendering/RenderFlexibleBox.h

index 480fc02..f9738cb 100644 (file)
@@ -1,3 +1,13 @@
+2012-02-10  Ojan Vafai  <ojan@chromium.org>
+
+        flex-pack:center and flex-item-align:center should do true centering
+        https://bugs.webkit.org/show_bug.cgi?id=77385
+
+        Reviewed by Tony Chang.
+
+        * css3/flexbox/true-centering-expected.txt: Added.
+        * css3/flexbox/true-centering.html: Added.
+
 2012-02-10  Tony Chang  <tony@chromium.org>
 
         [chromium] Unreviewed, adding Leopard pixel baselines for
diff --git a/LayoutTests/css3/flexbox/true-centering-expected.txt b/LayoutTests/css3/flexbox/true-centering-expected.txt
new file mode 100644 (file)
index 0000000..138cc04
--- /dev/null
@@ -0,0 +1,127 @@
+PASS
+PASS
+PASS
+PASS
+PASS
+PASS
+PASS
+horizontal-tb ltr row
+PASS
+horizontal-tb rtl row
+PASS
+horizontal-tb ltr column
+PASS
+horizontal-tb rtl column
+FAIL:
+Expected -5 for offsetLeft, but got -15. 
+Expected -5 for offsetLeft, but got -15. 
+
+<div class="flexbox horizontal-tb rtl column vertical" style="-webkit-flex-align: center; -webkit-flex-pack: center;">
+<div data-offset-x="-5" data-offset-y="-10"></div>
+<div data-offset-x="-5" data-offset-y="50"></div>
+</div>
+horizontal-tb ltr row-reverse
+PASS
+horizontal-tb rtl row-reverse
+PASS
+horizontal-tb ltr column-reverse
+PASS
+horizontal-tb rtl column-reverse
+FAIL:
+Expected -5 for offsetLeft, but got -15. 
+Expected -5 for offsetLeft, but got -15. 
+
+<div class="flexbox horizontal-tb rtl column-reverse vertical" style="-webkit-flex-align: center; -webkit-flex-pack: center;">
+<div data-offset-x="-5" data-offset-y="50"></div>
+<div data-offset-x="-5" data-offset-y="-10"></div>
+</div>
+horizontal-bt ltr row
+PASS
+horizontal-bt rtl row
+PASS
+horizontal-bt ltr column
+PASS
+horizontal-bt rtl column
+FAIL:
+Expected -5 for offsetLeft, but got -15. 
+Expected -5 for offsetLeft, but got -15. 
+
+<div class="flexbox horizontal-bt rtl column vertical" style="-webkit-flex-align: center; -webkit-flex-pack: center;">
+<div data-offset-x="-5" data-offset-y="50"></div>
+<div data-offset-x="-5" data-offset-y="-10"></div>
+</div>
+horizontal-bt ltr row-reverse
+PASS
+horizontal-bt rtl row-reverse
+PASS
+horizontal-bt ltr column-reverse
+PASS
+horizontal-bt rtl column-reverse
+FAIL:
+Expected -5 for offsetLeft, but got -15. 
+Expected -5 for offsetLeft, but got -15. 
+
+<div class="flexbox horizontal-bt rtl column-reverse vertical" style="-webkit-flex-align: center; -webkit-flex-pack: center;">
+<div data-offset-x="-5" data-offset-y="-10"></div>
+<div data-offset-x="-5" data-offset-y="50"></div>
+</div>
+vertical-lr ltr row
+PASS
+vertical-lr rtl row
+PASS
+vertical-lr ltr column
+PASS
+vertical-lr rtl column
+FAIL:
+Expected -5 for offsetTop, but got -15. 
+Expected -5 for offsetTop, but got -15. 
+
+<div class="flexbox vertical-lr rtl column" style="-webkit-flex-align: center; -webkit-flex-pack: center;">
+<div data-offset-x="-10" data-offset-y="-5"></div>
+<div data-offset-x="50" data-offset-y="-5"></div>
+</div>
+vertical-lr ltr row-reverse
+PASS
+vertical-lr rtl row-reverse
+PASS
+vertical-lr ltr column-reverse
+PASS
+vertical-lr rtl column-reverse
+FAIL:
+Expected -5 for offsetTop, but got -15. 
+Expected -5 for offsetTop, but got -15. 
+
+<div class="flexbox vertical-lr rtl column-reverse" style="-webkit-flex-align: center; -webkit-flex-pack: center;">
+<div data-offset-x="50" data-offset-y="-5"></div>
+<div data-offset-x="-10" data-offset-y="-5"></div>
+</div>
+vertical-rl ltr row
+PASS
+vertical-rl rtl row
+PASS
+vertical-rl ltr column
+PASS
+vertical-rl rtl column
+FAIL:
+Expected -5 for offsetTop, but got -15. 
+Expected -5 for offsetTop, but got -15. 
+
+<div class="flexbox vertical-rl rtl column" style="-webkit-flex-align: center; -webkit-flex-pack: center;">
+<div data-offset-x="50" data-offset-y="-5"></div>
+<div data-offset-x="-10" data-offset-y="-5"></div>
+</div>
+vertical-rl ltr row-reverse
+PASS
+vertical-rl rtl row-reverse
+PASS
+vertical-rl ltr column-reverse
+PASS
+vertical-rl rtl column-reverse
+FAIL:
+Expected -5 for offsetTop, but got -15. 
+Expected -5 for offsetTop, but got -15. 
+
+<div class="flexbox vertical-rl rtl column-reverse" style="-webkit-flex-align: center; -webkit-flex-pack: center;">
+<div data-offset-x="-10" data-offset-y="-5"></div>
+<div data-offset-x="50" data-offset-y="-5"></div>
+</div>
diff --git a/LayoutTests/css3/flexbox/true-centering.html b/LayoutTests/css3/flexbox/true-centering.html
new file mode 100644 (file)
index 0000000..dd5005d
--- /dev/null
@@ -0,0 +1,158 @@
+<!DOCTYPE html>
+<html>
+<style>
+.flexbox {
+    width: 100px;
+    height: 100px;
+    display: -webkit-flexbox;
+    background-color: #aaa;
+    position: relative;
+}
+.flexbox > div {
+    height: 110px;
+    width: 60px;
+}
+.vertical > div {
+    width: 110px;
+    height: 60px;
+}
+.flexbox :nth-child(1) {
+    background-color: blue;
+}
+.flexbox :nth-child(2) {
+    background-color: green;
+}
+.ltr {
+    direction: ltr;
+}
+.rtl {
+    direction: rtl;
+}
+.horizontal-tb {
+    -webkit-writing-mode: horizontal-tb;
+}
+.horizontal-bt {
+    -webkit-writing-mode: horizontal-bt;
+}
+.vertical-rl {
+    -webkit-writing-mode: vertical-rl;
+}
+.vertical-lr {
+    -webkit-writing-mode: vertical-lr;
+}
+.row-reverse {
+    -webkit-flex-flow: row-reverse;
+}
+.column {
+    -webkit-flex-flow: column;
+}
+.column-reverse {
+    -webkit-flex-flow: column-reverse;
+}
+</style>
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+</script>
+<script src="resources/flexbox.js"></script>
+<body onload="checkFlexBoxen()">
+
+<div class="flexbox" style="-webkit-flex-align: center; -webkit-flex-pack: center;">
+    <div data-offset-x="-10" data-offset-y="-5"></div>
+    <div data-offset-x="50" data-offset-y="-5"></div>
+</div>
+
+<div class="flexbox row-reverse" style="-webkit-flex-align: center; -webkit-flex-pack: center;">
+    <div data-offset-x="50" data-offset-y="-5"></div>
+    <div data-offset-x="-10" data-offset-y="-5"></div>
+</div>
+
+<div class="flexbox column vertical" style="-webkit-flex-align: center; -webkit-flex-pack: center;">
+    <div data-offset-x="-5" data-offset-y="-10"></div>
+    <div data-offset-x="-5" data-offset-y="50"></div>
+</div>
+
+<div class="flexbox column-reverse vertical" style="-webkit-flex-align: center; -webkit-flex-pack: center;">
+    <div data-offset-x="-5" data-offset-y="50"></div>
+    <div data-offset-x="-5" data-offset-y="-10"></div>
+</div>
+
+<div class="flexbox" style="-webkit-flex-align: center; -webkit-flex-pack: distribute;">
+    <div data-offset-x="-10" data-offset-y="-5"></div>
+    <div data-offset-x="50" data-offset-y="-5"></div>
+</div>
+
+<div class="flexbox" style="-webkit-flex-align: start; -webkit-flex-pack: start;">
+    <div data-offset-x=0 data-offset-y=0></div>
+    <div data-offset-x=60 data-offset-y=0></div>
+</div>
+
+<div class="flexbox" style="-webkit-flex-align: start; -webkit-flex-pack: justify;">
+    <div data-offset-x=0 data-offset-y=0></div>
+    <div data-offset-x=60 data-offset-y=0></div>
+</div>
+
+<script>
+var writingModes = ['horizontal-tb', 'horizontal-bt', 'vertical-lr', 'vertical-rl'];
+var flexFlows = ['row', 'column', 'row-reverse', 'column-reverse'];
+var directions = ['ltr', 'rtl'];
+
+var verticalFirstChildExpected = 'data-offset-x="-5" data-offset-y="-10"';
+var verticalLastChildExpected = 'data-offset-x="-5" data-offset-y="50"';
+var horizontalFirstChildExpected = 'data-offset-x="-10" data-offset-y="-5"';
+var horizontalLastChildExpected = 'data-offset-x="50" data-offset-y="-5"';
+
+writingModes.forEach(function(writingMode) {
+    flexFlows.forEach(function(flexFlow) {
+        directions.forEach(function(direction) {
+            var flexboxClassName = writingMode + ' ' + direction + ' ' + flexFlow;
+
+            var title = document.createElement('div');
+            title.className = 'title';
+            title.innerHTML = flexboxClassName;
+            document.body.appendChild(title);
+
+            var isColumn = flexFlow.indexOf('column') != -1;
+            var isHorizontal = (writingMode.indexOf('horizontal') != -1) ? !isColumn : isColumn;
+            if (!isHorizontal)
+                flexboxClassName += ' vertical';
+
+            var isReverse;
+            switch (flexFlow) {
+                case 'row':
+                    isReverse = direction == 'rtl';
+                    break;
+                case 'row-reverse':
+                    isReverse = direction == 'ltr';
+                    break;
+                case 'column':
+                    isReverse = writingMode == 'vertical-rl' || writingMode == 'horizontal-bt';
+                    break;
+                case 'column-reverse':
+                    isReverse = writingMode == 'vertical-lr' || writingMode == 'horizontal-tb';
+                    break;
+            }
+
+            var firstChildExpected, lastChildExpected;
+            if (isHorizontal) {
+                firstChildExpected = isReverse ? horizontalLastChildExpected : horizontalFirstChildExpected;
+                lastChildExpected = isReverse ? horizontalFirstChildExpected : horizontalLastChildExpected;
+            } else {
+                firstChildExpected = isReverse ? verticalLastChildExpected : verticalFirstChildExpected;
+                lastChildExpected = isReverse ? verticalFirstChildExpected : verticalLastChildExpected;                
+            }
+
+            var container = document.createElement('div');
+            container.innerHTML = '<div class="flexbox ' + flexboxClassName + '" style="-webkit-flex-align: center; -webkit-flex-pack: center;">\n' +
+                '<div ' + firstChildExpected + '></div>\n' +
+                '<div ' + lastChildExpected + '></div>\n' +
+            '</div>';
+
+            document.body.appendChild(container);
+        })
+    })
+})
+</script>
+
+</body>
+</html>
\ No newline at end of file
index fdd5b6f..fb8a833 100644 (file)
@@ -1,3 +1,26 @@
+2012-02-10  Ojan Vafai  <ojan@chromium.org>
+
+        flex-pack:center and flex-item-align:center should do true centering
+        https://bugs.webkit.org/show_bug.cgi?id=77385
+
+        Reviewed by Tony Chang.
+
+        Also, removed passing totalPositiveFlexibility around. We don't
+        need to know about positive/negative flex once we run the flexing algorithm.
+        We used to need to know this in order to flex margins, but margins can
+        no longer be flexed.
+
+        Test: css3/flexbox/true-centering.html
+
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::layoutFlexItems):
+        (WebCore::initialPackingOffset):
+        (WebCore::packingSpaceBetweenChildren):
+        (WebCore::RenderFlexibleBox::layoutAndPlaceChildren):
+        (WebCore::RenderFlexibleBox::layoutColumnReverse):
+        * rendering/RenderFlexibleBox.h:
+        (RenderFlexibleBox):
+
 2012-02-10  Mark Hahnenberg  <mhahnenberg@apple.com>
 
         Split MarkedSpace into destructor and destructor-free subspaces
index 9fa5660..36fefb3 100644 (file)
@@ -441,7 +441,7 @@ void RenderFlexibleBox::layoutFlexItems(bool relayoutChildren)
         ASSERT(inflexibleItems.size() > 0);
     }
 
-    layoutAndPlaceChildren(orderedChildren, childSizes, availableFreeSpace, totalPositiveFlexibility);
+    layoutAndPlaceChildren(orderedChildren, childSizes, availableFreeSpace);
 }
 
 float RenderFlexibleBox::positiveFlexForChild(RenderBox* child) const
@@ -571,27 +571,25 @@ bool RenderFlexibleBox::runFreeSpaceAllocationAlgorithm(const OrderedFlexItemLis
     return true;
 }
 
-static bool hasPackingSpace(LayoutUnit availableFreeSpace, float totalPositiveFlexibility)
+static LayoutUnit initialPackingOffset(LayoutUnit availableFreeSpace, EFlexPack flexPack, size_t numberOfChildren)
 {
-    return availableFreeSpace > 0 && !totalPositiveFlexibility;
-}
-
-static LayoutUnit initialPackingOffset(LayoutUnit availableFreeSpace, float totalPositiveFlexibility, EFlexPack flexPack, size_t numberOfChildren)
-{
-    if (hasPackingSpace(availableFreeSpace, totalPositiveFlexibility)) {
+    if (availableFreeSpace > 0) {
         if (flexPack == PackEnd)
             return availableFreeSpace;
         if (flexPack == PackCenter)
             return availableFreeSpace / 2;
         if (flexPack == PackDistribute && numberOfChildren)
             return availableFreeSpace / (2 * numberOfChildren);
+    } else if (availableFreeSpace < 0) {
+        if (flexPack == PackCenter || flexPack == PackDistribute)
+            return availableFreeSpace / 2;
     }
     return 0;
 }
 
-static LayoutUnit packingSpaceBetweenChildren(LayoutUnit availableFreeSpace, float totalPositiveFlexibility, EFlexPack flexPack, size_t numberOfChildren)
+static LayoutUnit packingSpaceBetweenChildren(LayoutUnit availableFreeSpace, EFlexPack flexPack, size_t numberOfChildren)
 {
-    if (hasPackingSpace(availableFreeSpace, totalPositiveFlexibility) && numberOfChildren > 1) {
+    if (availableFreeSpace > 0 && numberOfChildren > 1) {
         if (flexPack == PackJustify)
             return availableFreeSpace / (numberOfChildren - 1);
         if (flexPack == PackDistribute)
@@ -635,10 +633,10 @@ static EFlexAlign flexAlignForChild(RenderBox* child)
     return align;
 }
 
-void RenderFlexibleBox::layoutAndPlaceChildren(const OrderedFlexItemList& children, const WTF::Vector<LayoutUnit>& childSizes, LayoutUnit availableFreeSpace, float totalPositiveFlexibility)
+void RenderFlexibleBox::layoutAndPlaceChildren(const OrderedFlexItemList& children, const WTF::Vector<LayoutUnit>& childSizes, LayoutUnit availableFreeSpace)
 {
     LayoutUnit mainAxisOffset = flowAwareBorderStart() + flowAwarePaddingStart();
-    mainAxisOffset += initialPackingOffset(availableFreeSpace, totalPositiveFlexibility, style()->flexPack(), childSizes.size());
+    mainAxisOffset += initialPackingOffset(availableFreeSpace, style()->flexPack(), childSizes.size());
     if (style()->flexDirection() == FlowRowReverse)
         mainAxisOffset += isHorizontalFlow() ? verticalScrollbarWidth() : horizontalScrollbarHeight();
 
@@ -650,7 +648,7 @@ void RenderFlexibleBox::layoutAndPlaceChildren(const OrderedFlexItemList& childr
         RenderBox* child = children[i];
         if (child->isPositioned()) {
             prepareChildForPositionedLayout(child, mainAxisOffset, crossAxisOffset);
-            mainAxisOffset += packingSpaceBetweenChildren(availableFreeSpace, totalPositiveFlexibility, style()->flexPack(), childSizes.size());
+            mainAxisOffset += packingSpaceBetweenChildren(availableFreeSpace, style()->flexPack(), childSizes.size());
             continue;
         }
         LayoutUnit childPreferredSize = childSizes[i] + mainAxisBorderAndPaddingExtentForChild(child);
@@ -680,7 +678,7 @@ void RenderFlexibleBox::layoutAndPlaceChildren(const OrderedFlexItemList& childr
         setFlowAwareLocationForChild(child, childLocation);
         mainAxisOffset += childMainExtent + flowAwareMarginEndForChild(child);
 
-        mainAxisOffset += packingSpaceBetweenChildren(availableFreeSpace, totalPositiveFlexibility, style()->flexPack(), childSizes.size());
+        mainAxisOffset += packingSpaceBetweenChildren(availableFreeSpace, style()->flexPack(), childSizes.size());
 
         if (isColumnFlow())
             setLogicalHeight(mainAxisOffset + flowAwareBorderEnd() + flowAwarePaddingEnd() + scrollbarLogicalHeight());
@@ -690,19 +688,19 @@ void RenderFlexibleBox::layoutAndPlaceChildren(const OrderedFlexItemList& childr
         // We have to do an extra pass for column-reverse to reposition the flex items since the start depends
         // on the height of the flexbox, which we only know after we've positioned all the flex items.
         computeLogicalHeight();
-        layoutColumnReverse(children, childSizes, availableFreeSpace, totalPositiveFlexibility);
+        layoutColumnReverse(children, childSizes, availableFreeSpace);
     }
 
     alignChildren(children, maxAscent);
 }
 
-void RenderFlexibleBox::layoutColumnReverse(const OrderedFlexItemList& children, const WTF::Vector<LayoutUnit>& childSizes, LayoutUnit availableFreeSpace, float totalPositiveFlexibility)
+void RenderFlexibleBox::layoutColumnReverse(const OrderedFlexItemList& children, const WTF::Vector<LayoutUnit>& childSizes, LayoutUnit availableFreeSpace)
 {
     // This is similar to the logic in layoutAndPlaceChildren, except we place the children
     // starting from the end of the flexbox. We also don't need to layout anything since we're
     // just moving the children to a new position.
     LayoutUnit mainAxisOffset = logicalHeight() - flowAwareBorderEnd() - flowAwarePaddingEnd();
-    mainAxisOffset -= initialPackingOffset(availableFreeSpace, totalPositiveFlexibility, style()->flexPack(), childSizes.size());
+    mainAxisOffset -= initialPackingOffset(availableFreeSpace, style()->flexPack(), childSizes.size());
     mainAxisOffset -= isHorizontalFlow() ? verticalScrollbarWidth() : horizontalScrollbarHeight();
 
     LayoutUnit crossAxisOffset = flowAwareBorderBefore() + flowAwarePaddingBefore();
@@ -710,7 +708,7 @@ void RenderFlexibleBox::layoutColumnReverse(const OrderedFlexItemList& children,
         RenderBox* child = children[i];
         if (child->isPositioned()) {
             child->layer()->setStaticBlockPosition(mainAxisOffset);
-            mainAxisOffset -= packingSpaceBetweenChildren(availableFreeSpace, totalPositiveFlexibility, style()->flexPack(), childSizes.size());
+            mainAxisOffset -= packingSpaceBetweenChildren(availableFreeSpace, style()->flexPack(), childSizes.size());
             continue;
         }
         mainAxisOffset -= mainAxisExtentForChild(child) + flowAwareMarginEndForChild(child);
@@ -721,7 +719,7 @@ void RenderFlexibleBox::layoutColumnReverse(const OrderedFlexItemList& children,
             child->repaintDuringLayoutIfMoved(oldRect);
 
         mainAxisOffset -= flowAwareMarginStartForChild(child);
-        mainAxisOffset -= packingSpaceBetweenChildren(availableFreeSpace, totalPositiveFlexibility, style()->flexPack(), childSizes.size());
+        mainAxisOffset -= packingSpaceBetweenChildren(availableFreeSpace, style()->flexPack(), childSizes.size());
     }
 }
 
index 48e9fb5..0ca255c 100644 (file)
@@ -103,8 +103,8 @@ private:
     bool runFreeSpaceAllocationAlgorithm(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(const OrderedFlexItemList&, const WTF::Vector<LayoutUnit>& childSizes, LayoutUnit availableFreeSpace, float totalPositiveFlexibility);
-    void layoutColumnReverse(const OrderedFlexItemList&, const WTF::Vector<LayoutUnit>& childSizes, LayoutUnit availableFreeSpace, float totalPositiveFlexibility);
+    void layoutAndPlaceChildren(const OrderedFlexItemList&, const WTF::Vector<LayoutUnit>& childSizes, LayoutUnit availableFreeSpace);
+    void layoutColumnReverse(const OrderedFlexItemList&, const WTF::Vector<LayoutUnit>& childSizes, LayoutUnit availableFreeSpace);
     void alignChildren(const OrderedFlexItemList&, LayoutUnit maxAscent);
 };