Fix issue with alternating flex direction and percent postions (#1663)

Summary:
X-link: https://github.com/facebook/react-native/pull/44792

Pull Request resolved: https://github.com/facebook/yoga/pull/1663

Fixing https://github.com/facebook/yoga/issues/1658. We had a problem where if a child had a different flex direction than its parent, and it also set a position as a percent, it would look at the wrong axis to evaluate the percent. What was happening was we were passing in the container's mainAxis size and crossAxis size to use to evaluate the position size if it was a percent. However, we matched these sizes with the main/cross axis of the child - which is wrong if the flex direction is different.

I changed it so that the function just takes in ownerWidth and ownerHeight then calls isRow to determine which one to use for the main/cross axis position. This reduces the ambiguity quite a bit imo.

Changelog: [Internal]

Reviewed By: NickGerleman

Differential Revision: D58172416

fbshipit-source-id: eafd8069e03493fc56c41a76879d1ad9b7e9236d

gentest/fixtures/YGFlexDirectionTest.html

@@ -400,3 +400,8 @@
     <div style="width: 10px;"></div>
   </div>
 </div>
+
+<div id="flex_direction_alternating_with_percent" style="height: 300px; width: 200px; flex-direction: column;">
+  <div style="height: 50%; width: 50%; left: 10%; top: 10%; flex-direction: row;">
+  </div>
+</div>

java/tests/com/facebook/yoga/YGFlexDirectionTest.java

@@ -4,7 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @generated SignedSource<<4007f83eb3e84f3ee3fcf46d6d7be3bc>>
+ * @generated SignedSource<<5bb2b698f40c9c95737a77fff84f7700>>
  * generated by gentest/gentest-driver.ts from gentest/fixtures/YGFlexDirectionTest.html
  */
 
@@ -4242,6 +4242,49 @@ public class YGFlexDirectionTest {
     assertEquals(100f, root_child0_child2.getLayoutHeight(), 0.0f);
   }
 
+  @Test
+  public void test_flex_direction_alternating_with_percent() {
+    YogaConfig config = YogaConfigFactory.create();
+
+    final YogaNode root = createNode(config);
+    root.setPositionType(YogaPositionType.ABSOLUTE);
+    root.setWidth(200f);
+    root.setHeight(300f);
+
+    final YogaNode root_child0 = createNode(config);
+    root_child0.setFlexDirection(YogaFlexDirection.ROW);
+    root_child0.setPositionPercent(YogaEdge.LEFT, 10f);
+    root_child0.setPositionPercent(YogaEdge.TOP, 10f);
+    root_child0.setWidthPercent(50f);
+    root_child0.setHeightPercent(50f);
+    root.addChildAt(root_child0, 0);
+    root.setDirection(YogaDirection.LTR);
+    root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED);
+
+    assertEquals(0f, root.getLayoutX(), 0.0f);
+    assertEquals(0f, root.getLayoutY(), 0.0f);
+    assertEquals(200f, root.getLayoutWidth(), 0.0f);
+    assertEquals(300f, root.getLayoutHeight(), 0.0f);
+
+    assertEquals(20f, root_child0.getLayoutX(), 0.0f);
+    assertEquals(30f, root_child0.getLayoutY(), 0.0f);
+    assertEquals(100f, root_child0.getLayoutWidth(), 0.0f);
+    assertEquals(150f, root_child0.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(200f, root.getLayoutWidth(), 0.0f);
+    assertEquals(300f, root.getLayoutHeight(), 0.0f);
+
+    assertEquals(120f, root_child0.getLayoutX(), 0.0f);
+    assertEquals(30f, root_child0.getLayoutY(), 0.0f);
+    assertEquals(100f, root_child0.getLayoutWidth(), 0.0f);
+    assertEquals(150f, root_child0.getLayoutHeight(), 0.0f);
+  }
+
   private YogaNode createNode(YogaConfig config) {
     return mNodeFactory.create(config);
   }

javascript/tests/generated/YGFlexDirectionTest.test.ts

@@ -4,7 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @generated SignedSource<<61e2e5c148d45c0bbb6bc886991bf3b9>>
+ * @generated SignedSource<<5aa80de7d8424e35b5b1b205ff44f4cc>>
  * generated by gentest/gentest-driver.ts from gentest/fixtures/YGFlexDirectionTest.html
  */
 
@@ -4510,3 +4510,51 @@ test('flex_direction_row_reverse_inner_padding_end', () => {
     config.free();
   }
 });
+test('flex_direction_alternating_with_percent', () => {
+  const config = Yoga.Config.create();
+  let root;
+
+  try {
+    root = Yoga.Node.create(config);
+    root.setPositionType(PositionType.Absolute);
+    root.setWidth(200);
+    root.setHeight(300);
+
+    const root_child0 = Yoga.Node.create(config);
+    root_child0.setFlexDirection(FlexDirection.Row);
+    root_child0.setPosition(Edge.Left, "10%");
+    root_child0.setPosition(Edge.Top, "10%");
+    root_child0.setWidth("50%");
+    root_child0.setHeight("50%");
+    root.insertChild(root_child0, 0);
+    root.calculateLayout(undefined, undefined, Direction.LTR);
+
+    expect(root.getComputedLeft()).toBe(0);
+    expect(root.getComputedTop()).toBe(0);
+    expect(root.getComputedWidth()).toBe(200);
+    expect(root.getComputedHeight()).toBe(300);
+
+    expect(root_child0.getComputedLeft()).toBe(20);
+    expect(root_child0.getComputedTop()).toBe(30);
+    expect(root_child0.getComputedWidth()).toBe(100);
+    expect(root_child0.getComputedHeight()).toBe(150);
+
+    root.calculateLayout(undefined, undefined, Direction.RTL);
+
+    expect(root.getComputedLeft()).toBe(0);
+    expect(root.getComputedTop()).toBe(0);
+    expect(root.getComputedWidth()).toBe(200);
+    expect(root.getComputedHeight()).toBe(300);
+
+    expect(root_child0.getComputedLeft()).toBe(120);
+    expect(root_child0.getComputedTop()).toBe(30);
+    expect(root_child0.getComputedWidth()).toBe(100);
+    expect(root_child0.getComputedHeight()).toBe(150);
+  } finally {
+    if (typeof root !== 'undefined') {
+      root.freeRecursive();
+    }
+
+    config.free();
+  }
+});

tests/generated/YGFlexDirectionTest.cpp

@@ -5,7 +5,7 @@
  * LICENSE file in the root directory of this source tree.
  *
  * clang-format off
- * @generated SignedSource<<4a6a69c4e9bfda6dc73198791f553e13>>
+ * @generated SignedSource<<552f1533812daf0793244bbc8c465e17>>
  * generated by gentest/gentest-driver.ts from gentest/fixtures/YGFlexDirectionTest.html
  */
 
@@ -4283,3 +4283,47 @@ TEST(YogaTest, flex_direction_row_reverse_inner_padding_end) {
 
   YGConfigFree(config);
 }
+
+TEST(YogaTest, flex_direction_alternating_with_percent) {
+  const YGConfigRef config = YGConfigNew();
+
+  const YGNodeRef root = YGNodeNewWithConfig(config);
+  YGNodeStyleSetPositionType(root, YGPositionTypeAbsolute);
+  YGNodeStyleSetWidth(root, 200);
+  YGNodeStyleSetHeight(root, 300);
+
+  const YGNodeRef root_child0 = YGNodeNewWithConfig(config);
+  YGNodeStyleSetFlexDirection(root_child0, YGFlexDirectionRow);
+  YGNodeStyleSetPositionPercent(root_child0, YGEdgeLeft, 10);
+  YGNodeStyleSetPositionPercent(root_child0, YGEdgeTop, 10);
+  YGNodeStyleSetWidthPercent(root_child0, 50);
+  YGNodeStyleSetHeightPercent(root_child0, 50);
+  YGNodeInsertChild(root, root_child0, 0);
+  YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);
+
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
+  ASSERT_FLOAT_EQ(200, YGNodeLayoutGetWidth(root));
+  ASSERT_FLOAT_EQ(300, YGNodeLayoutGetHeight(root));
+
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetLeft(root_child0));
+  ASSERT_FLOAT_EQ(30, YGNodeLayoutGetTop(root_child0));
+  ASSERT_FLOAT_EQ(100, YGNodeLayoutGetWidth(root_child0));
+  ASSERT_FLOAT_EQ(150, YGNodeLayoutGetHeight(root_child0));
+
+  YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionRTL);
+
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
+  ASSERT_FLOAT_EQ(200, YGNodeLayoutGetWidth(root));
+  ASSERT_FLOAT_EQ(300, YGNodeLayoutGetHeight(root));
+
+  ASSERT_FLOAT_EQ(120, YGNodeLayoutGetLeft(root_child0));
+  ASSERT_FLOAT_EQ(30, YGNodeLayoutGetTop(root_child0));
+  ASSERT_FLOAT_EQ(100, YGNodeLayoutGetWidth(root_child0));
+  ASSERT_FLOAT_EQ(150, YGNodeLayoutGetHeight(root_child0));
+
+  YGNodeFreeRecursive(root);
+
+  YGConfigFree(config);
+}

yoga/algorithm/CalculateLayout.cpp

@@ -545,12 +545,8 @@ static float computeFlexBasisForChildren(
     if (performLayout) {
       // Set the initial position (relative to the owner).
       const Direction childDirection = child->resolveDirection(direction);
-      const float mainDim =
-          isRow(mainAxis) ? availableInnerWidth : availableInnerHeight;
-      const float crossDim =
-          isRow(mainAxis) ? availableInnerHeight : availableInnerWidth;
       child->setPosition(
-          childDirection, mainDim, crossDim, availableInnerWidth);
+          childDirection, availableInnerWidth, availableInnerHeight);
     }
 
     if (child->style().positionType() == PositionType::Absolute) {
@@ -2388,8 +2384,7 @@ void calculateLayout(
           markerData,
           0, // tree root
           gCurrentGenerationCount.load(std::memory_order_relaxed))) {
-    node->setPosition(
+    node->setPosition(node->getLayout().direction(), ownerWidth, ownerHeight);
-        node->getLayout().direction(), ownerWidth, ownerHeight, ownerWidth);
     roundLayoutResultsToPixelGrid(node, 0.0f, 0.0f);
   }
 

yoga/node/Node.cpp

@@ -230,9 +230,8 @@ float Node::relativePosition(
 
 void Node::setPosition(
     const Direction direction,
-    const float mainSize,
+    const float ownerWidth,
-    const float crossSize,
+    const float ownerHeight) {
-    const float ownerWidth) {
   /* Root nodes should be always layouted as LTR, so we don't return negative
    * values. */
   const Direction directionRespectingRoot =
@@ -244,10 +243,14 @@ void Node::setPosition(
 
   // In the case of position static these are just 0. See:
   // https://www.w3.org/TR/css-position-3/#valdef-position-static
-  const float relativePositionMain =
+  const float relativePositionMain = relativePosition(
-      relativePosition(mainAxis, directionRespectingRoot, mainSize);
+      mainAxis,
-  const float relativePositionCross =
+      directionRespectingRoot,
-      relativePosition(crossAxis, directionRespectingRoot, crossSize);
+      isRow(mainAxis) ? ownerWidth : ownerHeight);
+  const float relativePositionCross = relativePosition(
+      crossAxis,
+      directionRespectingRoot,
+      isRow(mainAxis) ? ownerHeight : ownerWidth);
 
   const auto mainAxisLeadingEdge = inlineStartEdge(mainAxis, direction);
   const auto mainAxisTrailingEdge = inlineEndEdge(mainAxis, direction);

yoga/node/Node.h

@@ -229,11 +229,7 @@ class YG_EXPORT Node : public ::YGNode {
   void setLayoutBorder(float border, PhysicalEdge edge);
   void setLayoutPadding(float padding, PhysicalEdge edge);
   void setLayoutPosition(float position, PhysicalEdge edge);
-  void setPosition(
+  void setPosition(Direction direction, float ownerWidth, float ownerHeight);
-      Direction direction,
-      float mainSize,
-      float crossSize,
-      float ownerWidth);
 
   // Other methods
   Style::Length resolveFlexBasisPtr() const;