diff --git a/CSSLayout/CSSLayout.c b/CSSLayout/CSSLayout.c index 25ab962f..7b5c3b4a 100644 --- a/CSSLayout/CSSLayout.c +++ b/CSSLayout/CSSLayout.c @@ -286,7 +286,6 @@ static void _CSSNodeMarkDirty(const CSSNodeRef node) { } void CSSNodeSetMeasureFunc(const CSSNodeRef node, CSSMeasureFunc measureFunc) { - // You can always NULLify the measure function of a node. if (measureFunc == NULL) { node->measure = NULL; } else { @@ -301,7 +300,8 @@ CSSMeasureFunc CSSNodeGetMeasureFunc(const CSSNodeRef node) { void CSSNodeInsertChild(const CSSNodeRef node, const CSSNodeRef child, const uint32_t index) { CSS_ASSERT(child->parent == NULL, "Child already has a parent, it must be removed first."); - CSS_ASSERT(node->measure == NULL, "Cannot add child: Nodes with measure functions cannot have children."); + CSS_ASSERT(node->measure == NULL, + "Cannot add child: Nodes with measure functions cannot have children."); CSSNodeListInsert(&node->children, child, index); child->parent = node; _CSSNodeMarkDirty(node); @@ -1399,6 +1399,26 @@ static void layoutNodeImpl(const CSSNodeRef node, const float availableInnerMainDim = isMainAxisRow ? availableInnerWidth : availableInnerHeight; const float availableInnerCrossDim = isMainAxisRow ? availableInnerHeight : availableInnerWidth; + // If there is only one child with flexGrow + flexShrink it means we can set the + // computedFlexBasis to 0 instead of measuring and shrinking / flexing the child to exactly + // match the remaining space + CSSNodeRef singleFlexChild = NULL; + if ((isMainAxisRow && widthMeasureMode == CSSMeasureModeExactly) || + (!isMainAxisRow && heightMeasureMode == CSSMeasureModeExactly)) { + for (uint32_t i = 0; i < childCount; i++) { + const CSSNodeRef child = CSSNodeGetChild(node, i); + if (singleFlexChild) { + if (isFlex(child)) { + // There is already a flexible child, abort. + singleFlexChild = NULL; + break; + } + } else if (CSSNodeStyleGetFlexGrow(child) > 0 && CSSNodeStyleGetFlexShrink(child) > 0) { + singleFlexChild = child; + } + } + } + // STEP 3: DETERMINE FLEX BASIS FOR EACH ITEM for (uint32_t i = 0; i < childCount; i++) { const CSSNodeRef child = CSSNodeListGet(node->children, i); @@ -1423,13 +1443,17 @@ static void layoutNodeImpl(const CSSNodeRef node, currentAbsoluteChild = child; child->nextChild = NULL; } else { - computeChildFlexBasis(node, - child, - availableInnerWidth, - widthMeasureMode, - availableInnerHeight, - heightMeasureMode, - direction); + if (child == singleFlexChild) { + child->layout.computedFlexBasis = 0; + } else { + computeChildFlexBasis(node, + child, + availableInnerWidth, + widthMeasureMode, + availableInnerHeight, + heightMeasureMode, + direction); + } } } @@ -2165,17 +2189,17 @@ static inline bool newMeasureSizeIsStricterAndStillValid(CSSMeasureMode sizeMode } bool CSSNodeCanUseCachedMeasurement(const CSSMeasureMode widthMode, - const float width, - const CSSMeasureMode heightMode, - const float height, - const CSSMeasureMode lastWidthMode, - const float lastWidth, - const CSSMeasureMode lastHeightMode, - const float lastHeight, - const float lastComputedWidth, - const float lastComputedHeight, - const float marginRow, - const float marginColumn) { + const float width, + const CSSMeasureMode heightMode, + const float height, + const CSSMeasureMode lastWidthMode, + const float lastWidth, + const CSSMeasureMode lastHeightMode, + const float lastHeight, + const float lastComputedWidth, + const float lastComputedHeight, + const float marginRow, + const float marginColumn) { if (lastComputedHeight < 0 || lastComputedWidth < 0) { return false; } @@ -2193,19 +2217,16 @@ bool CSSNodeCanUseCachedMeasurement(const CSSMeasureMode widthMode, newMeasureSizeIsStricterAndStillValid( widthMode, width - marginRow, lastWidthMode, lastWidth, lastComputedWidth); - const bool heightIsCompatible = hasSameHeightSpec || - newSizeIsExactAndMatchesOldMeasuredSize(heightMode, - height - marginColumn, - lastComputedHeight) || - oldSizeIsUnspecifiedAndStillFits(heightMode, + const bool heightIsCompatible = + hasSameHeightSpec || newSizeIsExactAndMatchesOldMeasuredSize(heightMode, height - marginColumn, - lastHeightMode, lastComputedHeight) || - newMeasureSizeIsStricterAndStillValid(heightMode, - height - marginColumn, - lastHeightMode, - lastHeight, - lastComputedHeight); + oldSizeIsUnspecifiedAndStillFits(heightMode, + height - marginColumn, + lastHeightMode, + lastComputedHeight) || + newMeasureSizeIsStricterAndStillValid( + heightMode, height - marginColumn, lastHeightMode, lastHeight, lastComputedHeight); return widthIsCompatible && heightIsCompatible; } diff --git a/csharp/tests/Facebook.CSSLayout/CSSLayoutFlexTest.cs b/csharp/tests/Facebook.CSSLayout/CSSLayoutFlexTest.cs index 90c99330..72042ac2 100644 --- a/csharp/tests/Facebook.CSSLayout/CSSLayoutFlexTest.cs +++ b/csharp/tests/Facebook.CSSLayout/CSSLayoutFlexTest.cs @@ -41,6 +41,12 @@
+ +
+
+
+
+
* */ @@ -399,5 +405,56 @@ namespace Facebook.CSSLayout Assert.AreEqual(20, root_child2.LayoutHeight); } + [Test] + public void Test_flex_grow_shrink_at_most() + { + CSSNode root = new CSSNode(); + root.StyleWidth = 100; + root.StyleHeight = 100; + + CSSNode root_child0 = new CSSNode(); + root.Insert(0, root_child0); + + CSSNode root_child0_child0 = new CSSNode(); + root_child0_child0.FlexGrow = 1; + root_child0_child0.FlexShrink = 1; + root_child0.Insert(0, root_child0_child0); + root.StyleDirection = CSSDirection.LeftToRight; + root.CalculateLayout(); + + Assert.AreEqual(0, root.LayoutX); + Assert.AreEqual(0, root.LayoutY); + Assert.AreEqual(100, root.LayoutWidth); + Assert.AreEqual(100, root.LayoutHeight); + + Assert.AreEqual(0, root_child0.LayoutX); + Assert.AreEqual(0, root_child0.LayoutY); + Assert.AreEqual(100, root_child0.LayoutWidth); + Assert.AreEqual(0, root_child0.LayoutHeight); + + Assert.AreEqual(0, root_child0_child0.LayoutX); + Assert.AreEqual(0, root_child0_child0.LayoutY); + Assert.AreEqual(100, root_child0_child0.LayoutWidth); + Assert.AreEqual(0, root_child0_child0.LayoutHeight); + + root.StyleDirection = CSSDirection.RightToLeft; + root.CalculateLayout(); + + Assert.AreEqual(0, root.LayoutX); + Assert.AreEqual(0, root.LayoutY); + Assert.AreEqual(100, root.LayoutWidth); + Assert.AreEqual(100, root.LayoutHeight); + + Assert.AreEqual(0, root_child0.LayoutX); + Assert.AreEqual(0, root_child0.LayoutY); + Assert.AreEqual(100, root_child0.LayoutWidth); + Assert.AreEqual(0, root_child0.LayoutHeight); + + Assert.AreEqual(0, root_child0_child0.LayoutX); + Assert.AreEqual(0, root_child0_child0.LayoutY); + Assert.AreEqual(100, root_child0_child0.LayoutWidth); + Assert.AreEqual(0, root_child0_child0.LayoutHeight); + } + } } diff --git a/gentest/fixtures/CSSLayoutFlexTest.html b/gentest/fixtures/CSSLayoutFlexTest.html index 5689a6e7..30da202e 100644 --- a/gentest/fixtures/CSSLayoutFlexTest.html +++ b/gentest/fixtures/CSSLayoutFlexTest.html @@ -29,3 +29,9 @@
+ +
+
+
+
+
diff --git a/java/jni/CSSJNI.cpp b/java/jni/CSSJNI.cpp index e1c86237..16869f1a 100644 --- a/java/jni/CSSJNI.cpp +++ b/java/jni/CSSJNI.cpp @@ -40,7 +40,8 @@ static void _jniTransferLayoutOutputsRecursive(CSSNodeRef root) { } static void _jniPrint(CSSNodeRef node) { - auto obj = adopt_local(Environment::current()->NewLocalRef(reinterpret_cast(CSSNodeGetContext(node)))); + auto obj = adopt_local( + Environment::current()->NewLocalRef(reinterpret_cast(CSSNodeGetContext(node)))); cout << obj->toString() << endl; } @@ -49,10 +50,11 @@ static CSSSize _jniMeasureFunc(CSSNodeRef node, CSSMeasureMode widthMode, float height, CSSMeasureMode heightMode) { - auto obj = adopt_local(Environment::current()->NewLocalRef(reinterpret_cast(CSSNodeGetContext(node)))); + auto obj = adopt_local( + Environment::current()->NewLocalRef(reinterpret_cast(CSSNodeGetContext(node)))); static auto measureFunc = findClassLocal("com/facebook/csslayout/CSSNode") - ->getMethod("measure"); + ->getMethod("measure"); _jniTransferLayoutDirection(node, obj); const auto measureResult = measureFunc(obj, width, widthMode, height, heightMode); diff --git a/java/tests/com/facebook/csslayout/CSSLayoutFlexTest.java b/java/tests/com/facebook/csslayout/CSSLayoutFlexTest.java index 5056e307..ccc641c6 100644 --- a/java/tests/com/facebook/csslayout/CSSLayoutFlexTest.java +++ b/java/tests/com/facebook/csslayout/CSSLayoutFlexTest.java @@ -41,6 +41,12 @@
+ +
+
+
+
+
* */ @@ -392,4 +398,54 @@ public class CSSLayoutFlexTest { assertEquals(20, root_child2.getLayoutHeight(), 0.0f); } + @Test + public void test_flex_grow_shrink_at_most() { + final CSSNode root = new CSSNode(); + root.setStyleWidth(100); + root.setStyleHeight(100); + + final CSSNode root_child0 = new CSSNode(); + root.addChildAt(root_child0, 0); + + final CSSNode root_child0_child0 = new CSSNode(); + root_child0_child0.setFlexGrow(1); + root_child0_child0.setFlexShrink(1); + root_child0.addChildAt(root_child0_child0, 0); + root.setDirection(CSSDirection.LTR); + root.calculateLayout(null); + + assertEquals(0, root.getLayoutX(), 0.0f); + assertEquals(0, root.getLayoutY(), 0.0f); + assertEquals(100, root.getLayoutWidth(), 0.0f); + assertEquals(100, root.getLayoutHeight(), 0.0f); + + assertEquals(0, root_child0.getLayoutX(), 0.0f); + assertEquals(0, root_child0.getLayoutY(), 0.0f); + assertEquals(100, root_child0.getLayoutWidth(), 0.0f); + assertEquals(0, root_child0.getLayoutHeight(), 0.0f); + + assertEquals(0, root_child0_child0.getLayoutX(), 0.0f); + assertEquals(0, root_child0_child0.getLayoutY(), 0.0f); + assertEquals(100, root_child0_child0.getLayoutWidth(), 0.0f); + assertEquals(0, root_child0_child0.getLayoutHeight(), 0.0f); + + root.setDirection(CSSDirection.RTL); + root.calculateLayout(null); + + assertEquals(0, root.getLayoutX(), 0.0f); + assertEquals(0, root.getLayoutY(), 0.0f); + assertEquals(100, root.getLayoutWidth(), 0.0f); + assertEquals(100, root.getLayoutHeight(), 0.0f); + + assertEquals(0, root_child0.getLayoutX(), 0.0f); + assertEquals(0, root_child0.getLayoutY(), 0.0f); + assertEquals(100, root_child0.getLayoutWidth(), 0.0f); + assertEquals(0, root_child0.getLayoutHeight(), 0.0f); + + assertEquals(0, root_child0_child0.getLayoutX(), 0.0f); + assertEquals(0, root_child0_child0.getLayoutY(), 0.0f); + assertEquals(100, root_child0_child0.getLayoutWidth(), 0.0f); + assertEquals(0, root_child0_child0.getLayoutHeight(), 0.0f); + } + } diff --git a/tests/CSSLayoutFlexTest.cpp b/tests/CSSLayoutFlexTest.cpp index 7a5c76e7..65991c06 100644 --- a/tests/CSSLayoutFlexTest.cpp +++ b/tests/CSSLayoutFlexTest.cpp @@ -41,6 +41,12 @@
+ +
+
+
+
+
* */ @@ -381,3 +387,52 @@ TEST(CSSLayoutTest, flex_basis_overrides_main_size) { CSSNodeFreeRecursive(root); } + +TEST(CSSLayoutTest, flex_grow_shrink_at_most) { + const CSSNodeRef root = CSSNodeNew(); + CSSNodeStyleSetWidth(root, 100); + CSSNodeStyleSetHeight(root, 100); + + const CSSNodeRef root_child0 = CSSNodeNew(); + CSSNodeInsertChild(root, root_child0, 0); + + const CSSNodeRef root_child0_child0 = CSSNodeNew(); + CSSNodeStyleSetFlexGrow(root_child0_child0, 1); + CSSNodeStyleSetFlexShrink(root_child0_child0, 1); + CSSNodeInsertChild(root_child0, root_child0_child0, 0); + CSSNodeCalculateLayout(root, CSSUndefined, CSSUndefined, CSSDirectionLTR); + + ASSERT_EQ(0, CSSNodeLayoutGetLeft(root)); + ASSERT_EQ(0, CSSNodeLayoutGetTop(root)); + ASSERT_EQ(100, CSSNodeLayoutGetWidth(root)); + ASSERT_EQ(100, CSSNodeLayoutGetHeight(root)); + + ASSERT_EQ(0, CSSNodeLayoutGetLeft(root_child0)); + ASSERT_EQ(0, CSSNodeLayoutGetTop(root_child0)); + ASSERT_EQ(100, CSSNodeLayoutGetWidth(root_child0)); + ASSERT_EQ(0, CSSNodeLayoutGetHeight(root_child0)); + + ASSERT_EQ(0, CSSNodeLayoutGetLeft(root_child0_child0)); + ASSERT_EQ(0, CSSNodeLayoutGetTop(root_child0_child0)); + ASSERT_EQ(100, CSSNodeLayoutGetWidth(root_child0_child0)); + ASSERT_EQ(0, CSSNodeLayoutGetHeight(root_child0_child0)); + + CSSNodeCalculateLayout(root, CSSUndefined, CSSUndefined, CSSDirectionRTL); + + ASSERT_EQ(0, CSSNodeLayoutGetLeft(root)); + ASSERT_EQ(0, CSSNodeLayoutGetTop(root)); + ASSERT_EQ(100, CSSNodeLayoutGetWidth(root)); + ASSERT_EQ(100, CSSNodeLayoutGetHeight(root)); + + ASSERT_EQ(0, CSSNodeLayoutGetLeft(root_child0)); + ASSERT_EQ(0, CSSNodeLayoutGetTop(root_child0)); + ASSERT_EQ(100, CSSNodeLayoutGetWidth(root_child0)); + ASSERT_EQ(0, CSSNodeLayoutGetHeight(root_child0)); + + ASSERT_EQ(0, CSSNodeLayoutGetLeft(root_child0_child0)); + ASSERT_EQ(0, CSSNodeLayoutGetTop(root_child0_child0)); + ASSERT_EQ(100, CSSNodeLayoutGetWidth(root_child0_child0)); + ASSERT_EQ(0, CSSNodeLayoutGetHeight(root_child0_child0)); + + CSSNodeFreeRecursive(root); +} diff --git a/tests/CSSLayoutMeasureCacheTest.cpp b/tests/CSSLayoutMeasureCacheTest.cpp index c0a8e6fa..9378b579 100644 --- a/tests/CSSLayoutMeasureCacheTest.cpp +++ b/tests/CSSLayoutMeasureCacheTest.cpp @@ -17,7 +17,8 @@ static CSSSize _measureMax(CSSNodeRef node, CSSMeasureMode heightMode) { int *measureCount = (int *)CSSNodeGetContext(node); - *measureCount = *measureCount + 1; + (*measureCount)++; + return CSSSize { .width = widthMode == CSSMeasureModeUndefined ? 10 : width, .height = heightMode == CSSMeasureModeUndefined ? 10 : height, diff --git a/tests/CSSLayoutMeasureTest.cpp b/tests/CSSLayoutMeasureTest.cpp index cfed9363..e31063c2 100644 --- a/tests/CSSLayoutMeasureTest.cpp +++ b/tests/CSSLayoutMeasureTest.cpp @@ -10,18 +10,42 @@ #include #include -#if GTEST_HAS_DEATH_TEST static CSSSize _measure(CSSNodeRef node, float width, CSSMeasureMode widthMode, float height, CSSMeasureMode heightMode) { + int *measureCount = (int*) CSSNodeGetContext(node); + if (measureCount) { + (*measureCount)++; + } + return CSSSize { - .width = 0, - .height = 0, + .width = 10, + .height = 10, }; } +TEST(CSSLayoutTest, dont_measure_single_grow_shrink_child) { + const CSSNodeRef root = CSSNodeNew(); + CSSNodeStyleSetWidth(root, 100); + CSSNodeStyleSetHeight(root, 100); + + int measureCount = 0; + + const CSSNodeRef root_child0 = CSSNodeNew(); + CSSNodeSetContext(root_child0, &measureCount); + CSSNodeSetMeasureFunc(root_child0, _measure); + CSSNodeStyleSetFlexGrow(root_child0, 1); + CSSNodeStyleSetFlexShrink(root_child0, 1); + CSSNodeInsertChild(root, root_child0, 0); + + CSSNodeCalculateLayout(root, CSSUndefined, CSSUndefined, CSSDirectionLTR); + + ASSERT_EQ(0, measureCount); +} + +#if GTEST_HAS_DEATH_TEST TEST(CSSLayoutTest, cannot_add_child_to_node_with_measure_func) { const CSSNodeRef root = CSSNodeNew(); CSSNodeSetMeasureFunc(root, _measure);