Fixed issue with first line element gap handling. (facebook#1408)

Summary: If the first element of a line is not contributing (e.g. position absolute), an additional gap will be added to the line, because the first gap element of the line is never identified (wrong start index). Fix: raise the index of the first line element until we find an element that is contributing to the line. Pull Request resolved: facebook#1408 Reviewed By: yungsters Differential Revision: D49722065 Pulled By: NickGerleman fbshipit-source-id: 1068cb0b11ae4b04ec8d063e70540cce06181d5a
NickGerleman · Oct 3, 2023 · 07cabca · 07cabca
1 parent b03a821
commit 07cabca
Show file tree

Hide file tree

Showing 5 changed files with 288 additions and 1 deletion.
diff --git a/gentest/fixtures/YGGapTest.html b/gentest/fixtures/YGGapTest.html
@@ -34,6 +34,13 @@
   <div style="width: 20px; height: 20px"></div>
 </div>
 
+<div id="column_gap_start_index" style="flex-direction: row; flex-wrap: wrap; width: 80px; column-gap: 10px; row-gap: 20px;">
+  <div style="width: 20px; height: 20px; position: absolute"></div>
+  <div style="width: 20px; height: 20px"></div>
+  <div style="width: 20px; height: 20px"></div>
+  <div style="width: 20px; height: 20px"></div>
+</div>
+
 <div id="column_gap_justify_flex_start" style="flex-direction: row; justify-content: flex-start; width: 100px; height: 100px; column-gap: 10px;">
   <div style="width: 20px;"></div>
   <div style="width: 20px;"></div>

diff --git a/java/tests/com/facebook/yoga/YGGapTest.java b/java/tests/com/facebook/yoga/YGGapTest.java
@@ -485,6 +485,95 @@ public void test_column_row_gap_wrapping() {
     assertEquals(20f, root_child8.getLayoutHeight(), 0.0f);
   }
 
+  @Test
+  public void test_column_gap_start_index() {
+    YogaConfig config = YogaConfigFactory.create();
+    config.setExperimentalFeatureEnabled(YogaExperimentalFeature.ABSOLUTE_PERCENTAGE_AGAINST_PADDING_EDGE, true);
+
+    final YogaNode root = createNode(config);
+    root.setFlexDirection(YogaFlexDirection.ROW);
+    root.setWrap(YogaWrap.WRAP);
+    root.setWidth(80f);
+    root.setGap(YogaGutter.COLUMN, 10f);
+    root.setGap(YogaGutter.ROW, 20f);
+
+    final YogaNode root_child0 = createNode(config);
+    root_child0.setPositionType(YogaPositionType.ABSOLUTE);
+    root_child0.setWidth(20f);
+    root_child0.setHeight(20f);
+    root.addChildAt(root_child0, 0);
+
+    final YogaNode root_child1 = createNode(config);
+    root_child1.setWidth(20f);
+    root_child1.setHeight(20f);
+    root.addChildAt(root_child1, 1);
+
+    final YogaNode root_child2 = createNode(config);
+    root_child2.setWidth(20f);
+    root_child2.setHeight(20f);
+    root.addChildAt(root_child2, 2);
+
+    final YogaNode root_child3 = createNode(config);
+    root_child3.setWidth(20f);
+    root_child3.setHeight(20f);
+    root.addChildAt(root_child3, 3);
+    root.setDirection(YogaDirection.LTR);
+    root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED);
+
+    assertEquals(0f, root.getLayoutX(), 0.0f);
+    assertEquals(0f, root.getLayoutY(), 0.0f);
+    assertEquals(80f, root.getLayoutWidth(), 0.0f);
+    assertEquals(20f, root.getLayoutHeight(), 0.0f);
+
+    assertEquals(0f, root_child0.getLayoutX(), 0.0f);
+    assertEquals(0f, root_child0.getLayoutY(), 0.0f);
+    assertEquals(20f, root_child0.getLayoutWidth(), 0.0f);
+    assertEquals(20f, root_child0.getLayoutHeight(), 0.0f);
+
+    assertEquals(0f, root_child1.getLayoutX(), 0.0f);
+    assertEquals(0f, root_child1.getLayoutY(), 0.0f);
+    assertEquals(20f, root_child1.getLayoutWidth(), 0.0f);
+    assertEquals(20f, root_child1.getLayoutHeight(), 0.0f);
+
+    assertEquals(30f, root_child2.getLayoutX(), 0.0f);
+    assertEquals(0f, root_child2.getLayoutY(), 0.0f);
+    assertEquals(20f, root_child2.getLayoutWidth(), 0.0f);
+    assertEquals(20f, root_child2.getLayoutHeight(), 0.0f);
+
+    assertEquals(60f, root_child3.getLayoutX(), 0.0f);
+    assertEquals(0f, root_child3.getLayoutY(), 0.0f);
+    assertEquals(20f, root_child3.getLayoutWidth(), 0.0f);
+    assertEquals(20f, root_child3.getLayoutHeight(), 0.0f);
+
+    root.setDirection(YogaDirection.RTL);
+    root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED);
+
+    assertEquals(0f, root.getLayoutX(), 0.0f);
+    assertEquals(0f, root.getLayoutY(), 0.0f);
+    assertEquals(80f, root.getLayoutWidth(), 0.0f);
+    assertEquals(20f, root.getLayoutHeight(), 0.0f);
+
+    assertEquals(60f, root_child0.getLayoutX(), 0.0f);
+    assertEquals(0f, root_child0.getLayoutY(), 0.0f);
+    assertEquals(20f, root_child0.getLayoutWidth(), 0.0f);
+    assertEquals(20f, root_child0.getLayoutHeight(), 0.0f);
+
+    assertEquals(60f, root_child1.getLayoutX(), 0.0f);
+    assertEquals(0f, root_child1.getLayoutY(), 0.0f);
+    assertEquals(20f, root_child1.getLayoutWidth(), 0.0f);
+    assertEquals(20f, root_child1.getLayoutHeight(), 0.0f);
+
+    assertEquals(30f, root_child2.getLayoutX(), 0.0f);
+    assertEquals(0f, root_child2.getLayoutY(), 0.0f);
+    assertEquals(20f, root_child2.getLayoutWidth(), 0.0f);
+    assertEquals(20f, root_child2.getLayoutHeight(), 0.0f);
+
+    assertEquals(0f, root_child3.getLayoutX(), 0.0f);
+    assertEquals(0f, root_child3.getLayoutY(), 0.0f);
+    assertEquals(20f, root_child3.getLayoutWidth(), 0.0f);
+    assertEquals(20f, root_child3.getLayoutHeight(), 0.0f);
+  }
+
   @Test
   public void test_column_gap_justify_flex_start() {
     YogaConfig config = YogaConfigFactory.create();

diff --git a/javascript/tests/generated/YGGapTest.test.ts b/javascript/tests/generated/YGGapTest.test.ts
@@ -515,6 +515,101 @@ test('column_row_gap_wrapping', () => {
     config.free();
   }
 });
+test('column_gap_start_index', () => {
+  const config = Yoga.Config.create();
+  let root;
+
+  config.setExperimentalFeatureEnabled(ExperimentalFeature.AbsolutePercentageAgainstPaddingEdge, true);
+
+  try {
+    root = Yoga.Node.create(config);
+    root.setFlexDirection(FlexDirection.Row);
+    root.setFlexWrap(Wrap.Wrap);
+    root.setWidth(80);
+    root.setGap(Gutter.Column, 10);
+    root.setGap(Gutter.Row, 20);
+
+    const root_child0 = Yoga.Node.create(config);
+    root_child0.setPositionType(PositionType.Absolute);
+    root_child0.setWidth(20);
+    root_child0.setHeight(20);
+    root.insertChild(root_child0, 0);
+
+    const root_child1 = Yoga.Node.create(config);
+    root_child1.setWidth(20);
+    root_child1.setHeight(20);
+    root.insertChild(root_child1, 1);
+
+    const root_child2 = Yoga.Node.create(config);
+    root_child2.setWidth(20);
+    root_child2.setHeight(20);
+    root.insertChild(root_child2, 2);
+
+    const root_child3 = Yoga.Node.create(config);
+    root_child3.setWidth(20);
+    root_child3.setHeight(20);
+    root.insertChild(root_child3, 3);
+    root.calculateLayout(undefined, undefined, Direction.LTR);
+
+    expect(root.getComputedLeft()).toBe(0);
+    expect(root.getComputedTop()).toBe(0);
+    expect(root.getComputedWidth()).toBe(80);
+    expect(root.getComputedHeight()).toBe(20);
+
+    expect(root_child0.getComputedLeft()).toBe(0);
+    expect(root_child0.getComputedTop()).toBe(0);
+    expect(root_child0.getComputedWidth()).toBe(20);
+    expect(root_child0.getComputedHeight()).toBe(20);
+
+    expect(root_child1.getComputedLeft()).toBe(0);
+    expect(root_child1.getComputedTop()).toBe(0);
+    expect(root_child1.getComputedWidth()).toBe(20);
+    expect(root_child1.getComputedHeight()).toBe(20);
+
+    expect(root_child2.getComputedLeft()).toBe(30);
+    expect(root_child2.getComputedTop()).toBe(0);
+    expect(root_child2.getComputedWidth()).toBe(20);
+    expect(root_child2.getComputedHeight()).toBe(20);
+
+    expect(root_child3.getComputedLeft()).toBe(60);
+    expect(root_child3.getComputedTop()).toBe(0);
+    expect(root_child3.getComputedWidth()).toBe(20);
+    expect(root_child3.getComputedHeight()).toBe(20);
+
+    root.calculateLayout(undefined, undefined, Direction.RTL);
+
+    expect(root.getComputedLeft()).toBe(0);
+    expect(root.getComputedTop()).toBe(0);
+    expect(root.getComputedWidth()).toBe(80);
+    expect(root.getComputedHeight()).toBe(20);
+
+    expect(root_child0.getComputedLeft()).toBe(60);
+    expect(root_child0.getComputedTop()).toBe(0);
+    expect(root_child0.getComputedWidth()).toBe(20);
+    expect(root_child0.getComputedHeight()).toBe(20);
+
+    expect(root_child1.getComputedLeft()).toBe(60);
+    expect(root_child1.getComputedTop()).toBe(0);
+    expect(root_child1.getComputedWidth()).toBe(20);
+    expect(root_child1.getComputedHeight()).toBe(20);
+
+    expect(root_child2.getComputedLeft()).toBe(30);
+    expect(root_child2.getComputedTop()).toBe(0);
+    expect(root_child2.getComputedWidth()).toBe(20);
+    expect(root_child2.getComputedHeight()).toBe(20);
+
+    expect(root_child3.getComputedLeft()).toBe(0);
+    expect(root_child3.getComputedTop()).toBe(0);
+    expect(root_child3.getComputedWidth()).toBe(20);
+    expect(root_child3.getComputedHeight()).toBe(20);
+  } finally {
+    if (typeof root !== 'undefined') {
+      root.freeRecursive();
+    }
+
+    config.free();
+  }
+});
 test('column_gap_justify_flex_start', () => {
   const config = Yoga.Config.create();
   let root;

diff --git a/tests/generated/YGGapTest.cpp b/tests/generated/YGGapTest.cpp
@@ -476,6 +476,96 @@ TEST(YogaTest, column_row_gap_wrapping) {
   YGConfigFree(config);
 }
 
+TEST(YogaTest, column_gap_start_index) {
+  const YGConfigRef config = YGConfigNew();
+  YGConfigSetExperimentalFeatureEnabled(config, YGExperimentalFeatureAbsolutePercentageAgainstPaddingEdge, true);
+
+  const YGNodeRef root = YGNodeNewWithConfig(config);
+  YGNodeStyleSetFlexDirection(root, YGFlexDirectionRow);
+  YGNodeStyleSetFlexWrap(root, YGWrapWrap);
+  YGNodeStyleSetWidth(root, 80);
+  YGNodeStyleSetGap(root, YGGutterColumn, 10);
+  YGNodeStyleSetGap(root, YGGutterRow, 20);
+
+  const YGNodeRef root_child0 = YGNodeNewWithConfig(config);
+  YGNodeStyleSetPositionType(root_child0, YGPositionTypeAbsolute);
+  YGNodeStyleSetWidth(root_child0, 20);
+  YGNodeStyleSetHeight(root_child0, 20);
+  YGNodeInsertChild(root, root_child0, 0);
+
+  const YGNodeRef root_child1 = YGNodeNewWithConfig(config);
+  YGNodeStyleSetWidth(root_child1, 20);
+  YGNodeStyleSetHeight(root_child1, 20);
+  YGNodeInsertChild(root, root_child1, 1);
+
+  const YGNodeRef root_child2 = YGNodeNewWithConfig(config);
+  YGNodeStyleSetWidth(root_child2, 20);
+  YGNodeStyleSetHeight(root_child2, 20);
+  YGNodeInsertChild(root, root_child2, 2);
+
+  const YGNodeRef root_child3 = YGNodeNewWithConfig(config);
+  YGNodeStyleSetWidth(root_child3, 20);
+  YGNodeStyleSetHeight(root_child3, 20);
+  YGNodeInsertChild(root, root_child3, 3);
+  YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);
+
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
+  ASSERT_FLOAT_EQ(80, YGNodeLayoutGetWidth(root));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetHeight(root));
+
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetWidth(root_child0));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetHeight(root_child0));
+
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child1));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child1));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetWidth(root_child1));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetHeight(root_child1));
+
+  ASSERT_FLOAT_EQ(30, YGNodeLayoutGetLeft(root_child2));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child2));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetWidth(root_child2));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetHeight(root_child2));
+
+  ASSERT_FLOAT_EQ(60, YGNodeLayoutGetLeft(root_child3));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child3));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetWidth(root_child3));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetHeight(root_child3));
+
+  YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionRTL);
+
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
+  ASSERT_FLOAT_EQ(80, YGNodeLayoutGetWidth(root));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetHeight(root));
+
+  ASSERT_FLOAT_EQ(60, YGNodeLayoutGetLeft(root_child0));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetWidth(root_child0));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetHeight(root_child0));
+
+  ASSERT_FLOAT_EQ(60, YGNodeLayoutGetLeft(root_child1));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child1));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetWidth(root_child1));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetHeight(root_child1));
+
+  ASSERT_FLOAT_EQ(30, YGNodeLayoutGetLeft(root_child2));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child2));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetWidth(root_child2));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetHeight(root_child2));
+
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child3));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child3));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetWidth(root_child3));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetHeight(root_child3));
+
+  YGNodeFreeRecursive(root);
+
+  YGConfigFree(config);
+}
+
 TEST(YogaTest, column_gap_justify_flex_start) {
   const YGConfigRef config = YGConfigNew();
   YGConfigSetExperimentalFeatureEnabled(config, YGExperimentalFeatureAbsolutePercentageAgainstPaddingEdge, true);

diff --git a/yoga/algorithm/FlexLine.cpp b/yoga/algorithm/FlexLine.cpp
@@ -28,6 +28,7 @@ FlexLine calculateFlexLine(
   float totalFlexGrowFactors = 0.0f;
   float totalFlexShrinkScaledFactors = 0.0f;
   size_t endOfLineIndex = startOfLineIndex;
+  size_t firstElementInLineIndex = startOfLineIndex;
 
   float sizeConsumedIncludingMinConstraint = 0;
   const FlexDirection mainAxis = resolveDirection(
@@ -40,10 +41,15 @@ FlexLine calculateFlexLine(
     auto child = node->getChild(endOfLineIndex);
     if (child->getStyle().display() == Display::None ||
         child->getStyle().positionType() == PositionType::Absolute) {
+      if (firstElementInLineIndex == endOfLineIndex) {
+        // We haven't found the first contributing element in the line yet.
+        firstElementInLineIndex++;
+      }
       continue;
     }
 
-    const bool isFirstElementInLine = (endOfLineIndex - startOfLineIndex) == 0;
+    const bool isFirstElementInLine =
+        (endOfLineIndex - firstElementInLineIndex) == 0;
 
     child->setLineIndex(lineCount);
     const float childMarginMainAxis =