DaddyFrosty/yoga
1
0
Fork 0
Code Issues 96 Pull Requests 31 Actions Packages Projects Releases 33 Wiki Activity

Fix positioning of items with min/max width/height

Browse Source 
We found a case where a flexible item with a max width that was supposed
to be centered was positioned in the wrong location.

The problem was with our 2 pass approach for sizing flexible items with
a min/max width/height. Items sized in the first pass were being double
counted when calculating the remainingFreeSpace. Specifically, their
sizes were being subtracted from remainingFreeSpace in both the first
and second passes.

I also noticed a second unrelated bug. We weren't correctly calculating
deltaFreeSpace in the first pass. When calculating deltaFreeSpace, we
need to take into account the flex basis like we do in the second pass.
Consequently, in the first pass I changed this:
  deltaFreeSpace -= boundMainSize;

To this:
  deltaFreeSpace -= boundMainSize - childFlexBasis;

The problem there was that we'd end up double counting childFlexBasis
in the remainingFreeSpace.
This commit is contained in:
Adam Comella
2016-04-28 17:07:34 -07:00
committed by Adam Comella
parent f3dd51ab97
commit 8779d942ea
13 changed files with 411 additions and 118 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
src/java/src/com/facebook/csslayout/LayoutEngine.java
View File

@@ -751,7 +751,8 @@ public class LayoutEngine {
remainingFreeSpace = -sizeConsumedOnCurrentLine;
}
float remainingFreeSpaceAfterFlex = remainingFreeSpace;
float originalRemainingFreeSpace = remainingFreeSpace;
float deltaFreeSpace = 0;
if (!canSkipFlex) {
float childFlexBasis;
@@ -774,7 +775,6 @@ public class LayoutEngine {
// concerns because we know exactly how many passes it'll do.
// First pass: detect the flex items whose min/max constraints trigger
float deltaFreeSpace = 0;
float deltaFlexShrinkScaledFactors = 0;
float deltaFlexGrowFactors = 0;
currentRelativeChild = firstRelativeChild;
@@ -793,7 +793,7 @@ public class LayoutEngine {
// By excluding this item's size and flex factor from remaining, this item's
// min/max constraints should also trigger in the second pass resulting in the
// item's size calculation being identical in the first and second passes.
deltaFreeSpace -= boundMainSize;
deltaFreeSpace -= boundMainSize - childFlexBasis;
deltaFlexShrinkScaledFactors -= flexShrinkScaledFactor;
}
}
@@ -809,7 +809,7 @@ public class LayoutEngine {
// By excluding this item's size and flex factor from remaining, this item's
// min/max constraints should also trigger in the second pass resulting in the
// item's size calculation being identical in the first and second passes.
deltaFreeSpace -= boundMainSize;
deltaFreeSpace -= boundMainSize - childFlexBasis;
deltaFlexGrowFactors -= flexGrowFactor;
}
}
@@ -821,9 +821,9 @@ public class LayoutEngine {
totalFlexShrinkScaledFactors += deltaFlexShrinkScaledFactors;
totalFlexGrowFactors += deltaFlexGrowFactors;
remainingFreeSpace += deltaFreeSpace;
remainingFreeSpaceAfterFlex = remainingFreeSpace;
// Second pass: resolve the sizes of the flexible items
deltaFreeSpace = 0;
currentRelativeChild = firstRelativeChild;
while (currentRelativeChild != null) {
childFlexBasis = currentRelativeChild.layout.flexBasis;
@@ -847,7 +847,7 @@ public class LayoutEngine {
}
}
remainingFreeSpaceAfterFlex -= updatedMainSize - childFlexBasis;
deltaFreeSpace -= updatedMainSize - childFlexBasis;
if (isMainAxisRow) {
childWidth = updatedMainSize + (currentRelativeChild.style.margin.getWithFallback(leadingSpacing[CSS_FLEX_DIRECTION_ROW], leading[CSS_FLEX_DIRECTION_ROW]) + currentRelativeChild.style.margin.getWithFallback(trailingSpacing[CSS_FLEX_DIRECTION_ROW], trailing[CSS_FLEX_DIRECTION_ROW]));
@@ -883,7 +883,7 @@ public class LayoutEngine {
}
}
remainingFreeSpace = remainingFreeSpaceAfterFlex;
remainingFreeSpace = originalRemainingFreeSpace + deltaFreeSpace;
// STEP 6: MAIN-AXIS JUSTIFICATION & CROSS-AXIS SIZE DETERMINATION

165
src/java/tests/com/facebook/csslayout/LayoutEngineTest.java
View File

@@ -7365,6 +7365,97 @@ public class LayoutEngineTest {
@Test
public void testCase168()
{
TestCSSNode root_node = new TestCSSNode();
{
TestCSSNode node_0 = root_node;
node_0.style.flexDirection = CSSFlexDirection.ROW;
node_0.style.justifyContent = CSSJustify.CENTER;
node_0.style.dimensions[DIMENSION_WIDTH] = 1000;
node_0.style.dimensions[DIMENSION_HEIGHT] = 1000;
addChildren(node_0, 1);
{
TestCSSNode node_1;
node_1 = node_0.getChildAt(0);
node_1.style.flex = 1;
node_1.style.dimensions[DIMENSION_HEIGHT] = 1000;
node_1.style.maxWidth = 600;
}
}
TestCSSNode root_layout = new TestCSSNode();
{
TestCSSNode node_0 = root_layout;
node_0.layout.position[POSITION_TOP] = 0;
node_0.layout.position[POSITION_LEFT] = 0;
node_0.layout.dimensions[DIMENSION_WIDTH] = 1000;
node_0.layout.dimensions[DIMENSION_HEIGHT] = 1000;
addChildren(node_0, 1);
{
TestCSSNode node_1;
node_1 = node_0.getChildAt(0);
node_1.layout.position[POSITION_TOP] = 0;
node_1.layout.position[POSITION_LEFT] = 200;
node_1.layout.dimensions[DIMENSION_WIDTH] = 600;
node_1.layout.dimensions[DIMENSION_HEIGHT] = 1000;
}
}
test("should center flexible item with max size", root_node, root_layout);
}
@Test
public void testCase169()
{
TestCSSNode root_node = new TestCSSNode();
{
TestCSSNode node_0 = root_node;
node_0.style.flexDirection = CSSFlexDirection.ROW;
node_0.style.dimensions[DIMENSION_WIDTH] = 1000;
node_0.style.dimensions[DIMENSION_HEIGHT] = 1000;
addChildren(node_0, 2);
{
TestCSSNode node_1;
node_1 = node_0.getChildAt(0);
node_1.style.flex = 1;
node_1.style.dimensions[DIMENSION_WIDTH] = 100;
node_1.style.dimensions[DIMENSION_HEIGHT] = 1000;
node_1 = node_0.getChildAt(1);
node_1.style.flex = 1;
node_1.style.dimensions[DIMENSION_WIDTH] = 100;
node_1.style.dimensions[DIMENSION_HEIGHT] = 1000;
node_1.style.maxWidth = 200;
}
}
TestCSSNode root_layout = new TestCSSNode();
{
TestCSSNode node_0 = root_layout;
node_0.layout.position[POSITION_TOP] = 0;
node_0.layout.position[POSITION_LEFT] = 0;
node_0.layout.dimensions[DIMENSION_WIDTH] = 1000;
node_0.layout.dimensions[DIMENSION_HEIGHT] = 1000;
addChildren(node_0, 2);
{
TestCSSNode node_1;
node_1 = node_0.getChildAt(0);
node_1.layout.position[POSITION_TOP] = 0;
node_1.layout.position[POSITION_LEFT] = 0;
node_1.layout.dimensions[DIMENSION_WIDTH] = 800;
node_1.layout.dimensions[DIMENSION_HEIGHT] = 1000;
node_1 = node_0.getChildAt(1);
node_1.layout.position[POSITION_TOP] = 0;
node_1.layout.position[POSITION_LEFT] = 800;
node_1.layout.dimensions[DIMENSION_WIDTH] = 200;
node_1.layout.dimensions[DIMENSION_HEIGHT] = 1000;
}
}
test("should correctly size flexible items with flex basis and a max width", root_node, root_layout);
}
@Test
public void testCase170()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -7430,7 +7521,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase169()
public void testCase171()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -7502,7 +7593,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase170()
public void testCase172()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -7564,7 +7655,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase171()
public void testCase173()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -7658,7 +7749,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase172()
public void testCase174()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -7739,7 +7830,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase173()
public void testCase175()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -7779,7 +7870,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase174()
public void testCase176()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -7819,7 +7910,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase175()
public void testCase177()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -7859,7 +7950,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase176()
public void testCase178()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -7897,7 +7988,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase177()
public void testCase179()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -7936,7 +8027,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase178()
public void testCase180()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -7974,7 +8065,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase179()
public void testCase181()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -8013,7 +8104,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase180()
public void testCase182()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -8051,7 +8142,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase181()
public void testCase183()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -8090,7 +8181,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase182()
public void testCase184()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -8126,7 +8217,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase183()
public void testCase185()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -8194,7 +8285,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase184()
public void testCase186()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -8271,7 +8362,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase185()
public void testCase187()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -8343,7 +8434,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase186()
public void testCase188()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -8396,7 +8487,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase187()
public void testCase189()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -8486,7 +8577,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase188()
public void testCase190()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -8540,7 +8631,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase189()
public void testCase191()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -8594,7 +8685,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase190()
public void testCase192()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -8664,7 +8755,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase191()
public void testCase193()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -8734,7 +8825,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase192()
public void testCase194()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -8790,7 +8881,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase193()
public void testCase195()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -8845,7 +8936,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase194()
public void testCase196()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -8900,7 +8991,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase195()
public void testCase197()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -8971,7 +9062,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase196()
public void testCase198()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -9042,7 +9133,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase197()
public void testCase199()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -9099,7 +9190,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase198()
public void testCase200()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -9155,7 +9246,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase199()
public void testCase201()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -9211,7 +9302,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase200()
public void testCase202()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -9283,7 +9374,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase201()
public void testCase203()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -9355,7 +9446,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase202()
public void testCase204()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -9414,7 +9505,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase203()
public void testCase205()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -9487,7 +9578,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase204()
public void testCase206()
{
TestCSSNode root_node = new TestCSSNode();
{
@@ -9561,7 +9652,7 @@ public class LayoutEngineTest {
}
@Test
public void testCase205()
public void testCase207()
{
TestCSSNode root_node = new TestCSSNode();
{