Fix issue where borderStart and borderEnd were swapped with row rever…

…se (#41022)

Summary:
Pull Request resolved: facebook/react-native#41022

X-link: facebook/yoga#1425

Just like D50140503 where marginStart and marginEnd were not working with row reverse, borderStart and borderEnd are not working either with row reverse either. The solution is similar - we were checking the flex item layout starting/ending edges and not the general layout starting/ending edges. This change makes it so that we look at the proper edge according to what direction is set.

One caveat is that in the case of border (and also padding) there is a callsite that actually wants to get the flex item layout's leading/trailing border and not the one dictated by direction. So, I made a new function to accommodate this and just swapped that callsite out.

Reviewed By: NickGerleman

Differential Revision: D50348085

fbshipit-source-id: eca2702c1753dbebb503034e2f0732684ad6c56e

packages/react-native/ReactCommon/yoga/yoga/algorithm/BoundAxis.h

@@ -21,8 +21,10 @@ inline float paddingAndBorderForAxis(
     const yoga::Node* const node,
     const FlexDirection axis,
     const float widthSize) {
-  return node->getFlexStartPaddingAndBorder(axis, widthSize) +
-      node->getFlexEndPaddingAndBorder(axis, widthSize);
+  // The total padding/border for a given axis does not depend on the direction
+  // so hardcoding LTR here to avoid piping direction to this function
+  return node->getInlineStartPaddingAndBorder(axis, Direction::LTR, widthSize) +
+      node->getInlineEndPaddingAndBorder(axis, Direction::LTR, widthSize);
 }
 
 inline FloatOptional boundAxisWithinMinAndMax(

packages/react-native/ReactCommon/yoga/yoga/algorithm/CalculateLayout.cpp

@@ -346,8 +346,8 @@ static void layoutAbsoluteChild(
     if (child->isFlexStartPositionDefined(FlexDirection::Row) &&
         child->isFlexEndPositionDefined(FlexDirection::Row)) {
       childWidth = node->getLayout().measuredDimension(Dimension::Width) -
-          (node->getFlexStartBorder(FlexDirection::Row) +
-           node->getFlexEndBorder(FlexDirection::Row)) -
+          (node->getInlineStartBorder(FlexDirection::Row, direction) +
+           node->getInlineEndBorder(FlexDirection::Row, direction)) -
           (child->getFlexStartPosition(FlexDirection::Row, width) +
            child->getFlexEndPosition(FlexDirection::Row, width));
       childWidth =
@@ -366,8 +366,8 @@ static void layoutAbsoluteChild(
     if (child->isFlexStartPositionDefined(FlexDirection::Column) &&
         child->isFlexEndPositionDefined(FlexDirection::Column)) {
       childHeight = node->getLayout().measuredDimension(Dimension::Height) -
-          (node->getFlexStartBorder(FlexDirection::Column) +
-           node->getFlexEndBorder(FlexDirection::Column)) -
+          (node->getInlineStartBorder(FlexDirection::Column, direction) +
+           node->getInlineEndBorder(FlexDirection::Column, direction)) -
           (child->getFlexStartPosition(FlexDirection::Column, height) +
            child->getFlexEndPosition(FlexDirection::Column, height));
       childHeight =
@@ -451,7 +451,7 @@ static void layoutAbsoluteChild(
     child->setLayoutPosition(
         node->getLayout().measuredDimension(dimension(mainAxis)) -
             child->getLayout().measuredDimension(dimension(mainAxis)) -
-            node->getFlexEndBorder(mainAxis) -
+            node->getInlineEndBorder(mainAxis, direction) -
             child->getInlineEndMargin(
                 mainAxis, direction, isMainAxisRow ? width : height) -
             child->getFlexEndPosition(mainAxis, isMainAxisRow ? width : height),
@@ -479,7 +479,7 @@ static void layoutAbsoluteChild(
         child->getFlexStartPosition(
             mainAxis,
             node->getLayout().measuredDimension(dimension(mainAxis))) +
-            node->getFlexStartBorder(mainAxis) +
+            node->getInlineStartBorder(mainAxis, direction) +
             child->getInlineStartMargin(
                 mainAxis,
                 direction,
@@ -492,7 +492,7 @@ static void layoutAbsoluteChild(
     child->setLayoutPosition(
         node->getLayout().measuredDimension(dimension(crossAxis)) -
             child->getLayout().measuredDimension(dimension(crossAxis)) -
-            node->getFlexEndBorder(crossAxis) -
+            node->getInlineEndBorder(crossAxis, direction) -
             child->getInlineEndMargin(
                 crossAxis, direction, isMainAxisRow ? height : width) -
             child->getFlexEndPosition(
@@ -523,7 +523,7 @@ static void layoutAbsoluteChild(
         child->getFlexStartPosition(
             crossAxis,
             node->getLayout().measuredDimension(dimension(crossAxis))) +
-            node->getFlexStartBorder(crossAxis) +
+            node->getInlineStartBorder(crossAxis, direction) +
             child->getInlineStartMargin(
                 crossAxis,
                 direction,
@@ -1199,10 +1199,16 @@ static void justifyMainAxis(
     const float availableInnerWidth,
     const bool performLayout) {
   const auto& style = node->getStyle();
+
   const float leadingPaddingAndBorderMain =
-      node->getFlexStartPaddingAndBorder(mainAxis, ownerWidth);
+      node->hasErrata(Errata::StartingEndingEdgeFromFlexDirection)
+      ? node->getInlineStartPaddingAndBorder(mainAxis, direction, ownerWidth)
+      : node->getFlexStartPaddingAndBorder(mainAxis, direction, ownerWidth);
   const float trailingPaddingAndBorderMain =
-      node->getFlexEndPaddingAndBorder(mainAxis, ownerWidth);
+      node->hasErrata(Errata::StartingEndingEdgeFromFlexDirection)
+      ? node->getInlineEndPaddingAndBorder(mainAxis, direction, ownerWidth)
+      : node->getFlexEndPaddingAndBorder(mainAxis, direction, ownerWidth);
+
   const float gap = node->getGapForAxis(mainAxis);
   // If we are using "at most" rules in the main axis, make sure that
   // remainingFreeSpace is 0 when min main dimension is not given
@@ -1306,7 +1312,7 @@ static void justifyMainAxis(
         // margin/border).
         child->setLayoutPosition(
             child->getFlexStartPosition(mainAxis, availableInnerMainDim) +
-                node->getFlexStartBorder(mainAxis) +
+                node->getInlineStartBorder(mainAxis, direction) +
                 child->getInlineStartMargin(
                     mainAxis, direction, availableInnerWidth),
             flexStartEdge(mainAxis));
@@ -1380,7 +1386,8 @@ static void justifyMainAxis(
       } else if (performLayout) {
         child->setLayoutPosition(
             childLayout.position[flexStartEdge(mainAxis)] +
-                node->getFlexStartBorder(mainAxis) + leadingMainDim,
+                node->getInlineStartBorder(mainAxis, direction) +
+                leadingMainDim,
             flexStartEdge(mainAxis));
       }
     }
@@ -1518,12 +1525,14 @@ static void calculateLayoutImpl(
   const float marginAxisRow = marginRowLeading + marginRowTrailing;
   const float marginAxisColumn = marginColumnLeading + marginColumnTrailing;
 
-  node->setLayoutBorder(node->getFlexStartBorder(flexRowDirection), startEdge);
-  node->setLayoutBorder(node->getFlexEndBorder(flexRowDirection), endEdge);
   node->setLayoutBorder(
-      node->getFlexStartBorder(flexColumnDirection), YGEdgeTop);
+      node->getInlineStartBorder(flexRowDirection, direction), startEdge);
+  node->setLayoutBorder(
+      node->getInlineEndBorder(flexRowDirection, direction), endEdge);
+  node->setLayoutBorder(
+      node->getInlineStartBorder(flexColumnDirection, direction), YGEdgeTop);
   node->setLayoutBorder(
-      node->getFlexEndBorder(flexColumnDirection), YGEdgeBottom);
+      node->getInlineEndBorder(flexColumnDirection, direction), YGEdgeBottom);
 
   node->setLayoutPadding(
       node->getFlexStartPadding(flexRowDirection, ownerWidth), startEdge);
@@ -1594,9 +1603,9 @@ static void calculateLayoutImpl(
   const float paddingAndBorderAxisMain =
       paddingAndBorderForAxis(node, mainAxis, ownerWidth);
   const float leadingPaddingAndBorderCross =
-      node->getFlexStartPaddingAndBorder(crossAxis, ownerWidth);
+      node->getInlineStartPaddingAndBorder(crossAxis, direction, ownerWidth);
   const float trailingPaddingAndBorderCross =
-      node->getFlexEndPaddingAndBorder(crossAxis, ownerWidth);
+      node->getInlineEndPaddingAndBorder(crossAxis, direction, ownerWidth);
   const float paddingAndBorderAxisCross =
       leadingPaddingAndBorderCross + trailingPaddingAndBorderCross;
 
@@ -1855,7 +1864,7 @@ static void calculateLayoutImpl(
           if (isChildLeadingPosDefined) {
             child->setLayoutPosition(
                 child->getFlexStartPosition(crossAxis, availableInnerCrossDim) +
-                    node->getFlexStartBorder(crossAxis) +
+                    node->getInlineStartBorder(crossAxis, direction) +
                     child->getInlineStartMargin(
                         crossAxis, direction, availableInnerWidth),
                 flexStartEdge(crossAxis));
@@ -1866,7 +1875,7 @@ static void calculateLayoutImpl(
               yoga::isUndefined(
                   child->getLayout().position[flexStartEdge(crossAxis)])) {
             child->setLayoutPosition(
-                node->getFlexStartBorder(crossAxis) +
+                node->getInlineStartBorder(crossAxis, direction) +
                     child->getInlineStartMargin(
                         crossAxis, direction, availableInnerWidth),
                 flexStartEdge(crossAxis));

packages/react-native/ReactCommon/yoga/yoga/algorithm/FlexDirection.h

@@ -97,6 +97,32 @@ inline YGEdge inlineEndEdge(
   return YGEdgeBottom;
 }
 
+/**
+ * The physical edges that YGEdgeStart and YGEdgeEnd correspond to (e.g.
+ * left/right) are soley dependent on the direction. However, there are cases
+ * where we want the flex start/end edge (i.e. which edge is the start/end
+ * for laying out flex items), which can be distinct from the corresponding
+ * inline edge. In these cases we need to know which "relative edge"
+ * (YGEdgeStart/YGEdgeEnd) corresponds to the said flex start/end edge as these
+ * relative edges can be used instead of physical ones when defining certain
+ * attributes like border or padding.
+ */
+inline YGEdge flexStartRelativeEdge(
+    FlexDirection flexDirection,
+    Direction direction) {
+  const YGEdge leadLayoutEdge = inlineStartEdge(flexDirection, direction);
+  const YGEdge leadFlexItemEdge = flexStartEdge(flexDirection);
+  return leadLayoutEdge == leadFlexItemEdge ? YGEdgeStart : YGEdgeEnd;
+}
+
+inline YGEdge flexEndRelativeEdge(
+    FlexDirection flexDirection,
+    Direction direction) {
+  const YGEdge trailLayoutEdge = inlineEndEdge(flexDirection, direction);
+  const YGEdge trailFlexItemEdge = flexEndEdge(flexDirection);
+  return trailLayoutEdge == trailFlexItemEdge ? YGEdgeEnd : YGEdgeStart;
+}
+
 inline Dimension dimension(const FlexDirection flexDirection) {
   switch (flexDirection) {
     case FlexDirection::Column:

packages/react-native/ReactCommon/yoga/yoga/node/Node.cpp

@@ -157,18 +157,41 @@ float Node::getInlineEndMargin(
   return resolveValue(trailingMargin, widthSize).unwrapOrDefault(0.0f);
 }
 
-float Node::getFlexStartBorder(FlexDirection axis) const {
+float Node::getInlineStartBorder(FlexDirection axis, Direction direction)
+    const {
+  const YGEdge startEdge = getInlineStartEdgeUsingErrata(axis, direction);
+  YGValue leadingBorder = isRow(axis)
+      ? computeEdgeValueForRow(style_.border(), YGEdgeStart, startEdge)
+      : computeEdgeValueForColumn(style_.border(), startEdge);
+
+  return maxOrDefined(leadingBorder.value, 0.0f);
+}
+
+float Node::getFlexStartBorder(FlexDirection axis, Direction direction) const {
+  const YGEdge leadRelativeFlexItemEdge =
+      flexStartRelativeEdge(axis, direction);
   YGValue leadingBorder = isRow(axis)
       ? computeEdgeValueForRow(
-            style_.border(), YGEdgeStart, flexStartEdge(axis))
+            style_.border(), leadRelativeFlexItemEdge, flexStartEdge(axis))
       : computeEdgeValueForColumn(style_.border(), flexStartEdge(axis));
 
   return maxOrDefined(leadingBorder.value, 0.0f);
 }
 
-float Node::getFlexEndBorder(FlexDirection axis) const {
+float Node::getInlineEndBorder(FlexDirection axis, Direction direction) const {
+  const YGEdge endEdge = getInlineEndEdgeUsingErrata(axis, direction);
+  YGValue trailingBorder = isRow(axis)
+      ? computeEdgeValueForRow(style_.border(), YGEdgeEnd, endEdge)
+      : computeEdgeValueForColumn(style_.border(), endEdge);
+
+  return maxOrDefined(trailingBorder.value, 0.0f);
+}
+
+float Node::getFlexEndBorder(FlexDirection axis, Direction direction) const {
+  const YGEdge trailRelativeFlexItemEdge = flexEndRelativeEdge(axis, direction);
   YGValue trailingBorder = isRow(axis)
-      ? computeEdgeValueForRow(style_.border(), YGEdgeEnd, flexEndEdge(axis))
+      ? computeEdgeValueForRow(
+            style_.border(), trailRelativeFlexItemEdge, flexEndEdge(axis))
       : computeEdgeValueForColumn(style_.border(), flexEndEdge(axis));
 
   return maxOrDefined(trailingBorder.value, 0.0f);
@@ -191,14 +214,35 @@ float Node::getFlexEndPadding(FlexDirection axis, float widthSize) const {
   return maxOrDefined(resolveValue(trailingPadding, widthSize).unwrap(), 0.0f);
 }
 
-float Node::getFlexStartPaddingAndBorder(FlexDirection axis, float widthSize)
-    const {
-  return getFlexStartPadding(axis, widthSize) + getFlexStartBorder(axis);
+float Node::getInlineStartPaddingAndBorder(
+    FlexDirection axis,
+    Direction direction,
+    float widthSize) const {
+  return getFlexStartPadding(axis, widthSize) +
+      getInlineStartBorder(axis, direction);
 }
 
-float Node::getFlexEndPaddingAndBorder(FlexDirection axis, float widthSize)
-    const {
-  return getFlexEndPadding(axis, widthSize) + getFlexEndBorder(axis);
+float Node::getFlexStartPaddingAndBorder(
+    FlexDirection axis,
+    Direction direction,
+    float widthSize) const {
+  return getFlexStartPadding(axis, widthSize) +
+      getFlexStartBorder(axis, direction);
+}
+
+float Node::getInlineEndPaddingAndBorder(
+    FlexDirection axis,
+    Direction direction,
+    float widthSize) const {
+  return getFlexEndPadding(axis, widthSize) +
+      getInlineEndBorder(axis, direction);
+}
+
+float Node::getFlexEndPaddingAndBorder(
+    FlexDirection axis,
+    Direction direction,
+    float widthSize) const {
+  return getFlexEndPadding(axis, widthSize) + getFlexEndBorder(axis, direction);
 }
 
 float Node::getMarginForAxis(FlexDirection axis, float widthSize) const {

packages/react-native/ReactCommon/yoga/yoga/node/Node.h

@@ -208,12 +208,32 @@ class YG_EXPORT Node : public ::YGNode {
       FlexDirection axis,
       Direction direction,
       float widthSize) const;
-  float getFlexStartBorder(FlexDirection flexDirection) const;
-  float getFlexEndBorder(FlexDirection flexDirection) const;
+  float getFlexStartBorder(FlexDirection flexDirection, Direction direction)
+      const;
+  float getInlineStartBorder(FlexDirection flexDirection, Direction direction)
+      const;
+  float getFlexEndBorder(FlexDirection flexDirection, Direction direction)
+      const;
+  float getInlineEndBorder(FlexDirection flexDirection, Direction direction)
+      const;
   float getFlexStartPadding(FlexDirection axis, float widthSize) const;
   float getFlexEndPadding(FlexDirection axis, float widthSize) const;
-  float getFlexStartPaddingAndBorder(FlexDirection axis, float widthSize) const;
-  float getFlexEndPaddingAndBorder(FlexDirection axis, float widthSize) const;
+  float getFlexStartPaddingAndBorder(
+      FlexDirection axis,
+      Direction direction,
+      float widthSize) const;
+  float getInlineStartPaddingAndBorder(
+      FlexDirection axis,
+      Direction direction,
+      float widthSize) const;
+  float getFlexEndPaddingAndBorder(
+      FlexDirection axis,
+      Direction direction,
+      float widthSize) const;
+  float getInlineEndPaddingAndBorder(
+      FlexDirection axis,
+      Direction direction,
+      float widthSize) const;
   float getMarginForAxis(FlexDirection axis, float widthSize) const;
   float getGapForAxis(FlexDirection axis) const;
   // Setters