Remove usage of Gutters arrays and YGGutter as index (#39597)

Summary: X-link: facebook/yoga#1406 Similar in vain to D49362819, we want to stop exposing pre-resolved CompactValue, and allow enum class usage without becoming annoying. This also simplifies gap resolution a bit. I moved this to Style, to make it clear we aren't relying on any node state. I plan to do some similar cleanup for other resolution later. Reviewed By: rshest Differential Revision: D49530923
facebook · Oct 3, 2023 · 6b44f59 · 6b44f59
1 parent f174f02
commit 6b44f59
Show file tree

Hide file tree

Showing 9 changed files with 77 additions and 93 deletions.
diff --git a/packages/react-native/ReactCommon/react/renderer/components/view/YogaStylableProps.cpp b/packages/react-native/ReactCommon/react/renderer/components/view/YogaStylableProps.cpp
@@ -195,10 +195,14 @@ static inline T const getFieldValue(
   REBUILD_YG_FIELD_SWITCH_CASE_INDEXED_SETTER(                         \
       field, setter, yoga::Dimension::Height, heightStr);
 
-#define REBUILD_FIELD_YG_GUTTER(field, rowGapStr, columnGapStr, gapStr)      \
-  REBUILD_YG_FIELD_SWITCH_CASE_INDEXED(field, YGGutterRow, rowGapStr);       \
-  REBUILD_YG_FIELD_SWITCH_CASE_INDEXED(field, YGGutterColumn, columnGapStr); \
-  REBUILD_YG_FIELD_SWITCH_CASE_INDEXED(field, YGGutterAll, gapStr);
+#define REBUILD_FIELD_YG_GUTTER(                    \
+    field, setter, rowGapStr, columnGapStr, gapStr) \
+  REBUILD_YG_FIELD_SWITCH_CASE_INDEXED_SETTER(      \
+      field, setter, YGGutterRow, rowGapStr);       \
+  REBUILD_YG_FIELD_SWITCH_CASE_INDEXED_SETTER(      \
+      field, setter, YGGutterColumn, columnGapStr); \
+  REBUILD_YG_FIELD_SWITCH_CASE_INDEXED_SETTER(      \
+      field, setter, YGGutterAll, gapStr);
 
 #define REBUILD_FIELD_YG_EDGES(field, prefix, suffix)                          \
   REBUILD_YG_FIELD_SWITCH_CASE_INDEXED(                                        \
@@ -250,7 +254,7 @@ void YogaStylableProps::setProp(
     REBUILD_FIELD_SWITCH_CASE_YSP(flexShrink);
     REBUILD_FIELD_SWITCH_CASE_YSP(flexBasis);
     REBUILD_FIELD_SWITCH_CASE2(positionType, "position");
-    REBUILD_FIELD_YG_GUTTER(gap, "rowGap", "columnGap", "gap");
+    REBUILD_FIELD_YG_GUTTER(gap, setGap, "rowGap", "columnGap", "gap");
     REBUILD_FIELD_SWITCH_CASE_YSP(aspectRatio);
     REBUILD_FIELD_YG_DIMENSION(dimension, setDimension, "width", "height");
     REBUILD_FIELD_YG_DIMENSION(
@@ -325,16 +329,14 @@ SharedDebugStringConvertibleList YogaStylableProps::getDebugProps() const {
           "flexGrow", yogaStyle.flexGrow(), defaultYogaStyle.flexGrow()),
       debugStringConvertibleItem(
           "rowGap",
-          yogaStyle.gap()[YGGutterRow],
-          defaultYogaStyle.gap()[YGGutterRow]),
+          yogaStyle.gap(YGGutterRow),
+          defaultYogaStyle.gap(YGGutterRow)),
       debugStringConvertibleItem(
           "columnGap",
-          yogaStyle.gap()[YGGutterColumn],
-          defaultYogaStyle.gap()[YGGutterColumn]),
+          yogaStyle.gap(YGGutterColumn),
+          defaultYogaStyle.gap(YGGutterColumn)),
       debugStringConvertibleItem(
-          "gap",
-          yogaStyle.gap()[YGGutterAll],
-          defaultYogaStyle.gap()[YGGutterAll]),
+          "gap", yogaStyle.gap(YGGutterAll), defaultYogaStyle.gap(YGGutterAll)),
       debugStringConvertibleItem(
           "flexShrink", yogaStyle.flexShrink(), defaultYogaStyle.flexShrink()),
       debugStringConvertibleItem(

diff --git a/packages/react-native/ReactCommon/react/renderer/components/view/propsConversions.h b/packages/react-native/ReactCommon/react/renderer/components/view/propsConversions.h
@@ -244,26 +244,32 @@ static inline yoga::Style convertRawProp(
       sourceValue.padding(),
       yogaStyle.padding());
 
-  yogaStyle.gap()[YGGutterRow] = convertRawProp(
-      context,
-      rawProps,
-      "rowGap",
-      sourceValue.gap()[YGGutterRow],
-      yogaStyle.gap()[YGGutterRow]);
+  yogaStyle.setGap(
+      YGGutterRow,
+      convertRawProp(
+          context,
+          rawProps,
+          "rowGap",
+          sourceValue.gap(YGGutterRow),
+          yogaStyle.gap(YGGutterRow)));
 
-  yogaStyle.gap()[YGGutterColumn] = convertRawProp(
-      context,
-      rawProps,
-      "columnGap",
-      sourceValue.gap()[YGGutterColumn],
-      yogaStyle.gap()[YGGutterColumn]);
+  yogaStyle.setGap(
+      YGGutterColumn,
+      convertRawProp(
+          context,
+          rawProps,
+          "columnGap",
+          sourceValue.gap(YGGutterColumn),
+          yogaStyle.gap(YGGutterColumn)));
 
-  yogaStyle.gap()[YGGutterAll] = convertRawProp(
-      context,
-      rawProps,
-      "gap",
-      sourceValue.gap()[YGGutterAll],
-      yogaStyle.gap()[YGGutterAll]);
+  yogaStyle.setGap(
+      YGGutterAll,
+      convertRawProp(
+          context,
+          rawProps,
+          "gap",
+          sourceValue.gap(YGGutterAll),
+          yogaStyle.gap(YGGutterAll)));
 
   yogaStyle.border() = convertRawProp(
       context,

diff --git a/packages/react-native/ReactCommon/yoga/yoga/Yoga.cpp b/packages/react-native/ReactCommon/yoga/yoga/Yoga.cpp
@@ -635,11 +635,11 @@ void YGNodeStyleSetGap(
     const YGGutter gutter,
     const float gapLength) {
   auto length = CompactValue::ofMaybe<YGUnitPoint>(gapLength);
-  updateIndexedStyleProp<MSVC_HINT(gap)>(node, &Style::gap, gutter, length);
+  updateIndexedStyleProp<&Style::gap, &Style::setGap>(node, gutter, length);
 }
 
 float YGNodeStyleGetGap(const YGNodeConstRef node, const YGGutter gutter) {
-  auto gapLength = resolveRef(node)->getStyle().gap()[gutter];
+  auto gapLength = resolveRef(node)->getStyle().gap(gutter);
   if (gapLength.isUndefined() || gapLength.isAuto()) {
     return YGUndefined;
   }

diff --git a/packages/react-native/ReactCommon/yoga/yoga/algorithm/CalculateLayout.cpp b/packages/react-native/ReactCommon/yoga/yoga/algorithm/CalculateLayout.cpp
@@ -1215,7 +1215,7 @@ static void justifyMainAxis(
       node->getLeadingPaddingAndBorder(mainAxis, ownerWidth).unwrap();
   const float trailingPaddingAndBorderMain =
       node->getTrailingPaddingAndBorder(mainAxis, ownerWidth).unwrap();
-  const float gap = node->getGapForAxis(mainAxis, 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
   if (measureModeMainDim == MeasureMode::AtMost &&
@@ -1666,8 +1666,8 @@ static void calculateLayoutImpl(
       generationCount);
 
   if (childCount > 1) {
-    totalMainDim += node->getGapForAxis(mainAxis, availableInnerCrossDim) *
-        static_cast<float>(childCount - 1);
+    totalMainDim +=
+        node->getGapForAxis(mainAxis) * static_cast<float>(childCount - 1);
   }
 
   const bool mainAxisOverflows =
@@ -1690,8 +1690,7 @@ static void calculateLayoutImpl(
   // Accumulated cross dimensions of all lines so far.
   float totalLineCrossDim = 0;
 
-  const float crossAxisGap =
-      node->getGapForAxis(crossAxis, availableInnerCrossDim);
+  const float crossAxisGap = node->getGapForAxis(crossAxis);
 
   // Max main dimension of all the lines.
   float maxLineMainDim = 0;

diff --git a/packages/react-native/ReactCommon/yoga/yoga/algorithm/FlexLine.cpp b/packages/react-native/ReactCommon/yoga/yoga/algorithm/FlexLine.cpp
@@ -34,7 +34,7 @@ FlexLine calculateFlexLine(
   const FlexDirection mainAxis = resolveDirection(
       node->getStyle().flexDirection(), node->resolveDirection(ownerDirection));
   const bool isNodeFlexWrap = node->getStyle().flexWrap() != Wrap::NoWrap;
-  const float gap = node->getGapForAxis(mainAxis, availableInnerWidth);
+  const float gap = node->getGapForAxis(mainAxis);
 
   // Add items to the current line until it's full or we run out of items.
   for (; endOfLineIndex < node->getChildren().size(); endOfLineIndex++) {

diff --git a/packages/react-native/ReactCommon/yoga/yoga/debug/NodeToString.cpp b/packages/react-native/ReactCommon/yoga/yoga/debug/NodeToString.cpp
@@ -180,17 +180,11 @@ void nodeToString(
     appendEdges(str, "padding", style.padding());
     appendEdges(str, "border", style.border());
 
-    if (yoga::Node::computeColumnGap(
-            style.gap(), CompactValue::ofUndefined()) !=
-        yoga::Node::computeColumnGap(
-            yoga::Node{}.getStyle().gap(), CompactValue::ofUndefined())) {
-      appendNumberIfNotUndefined(
-          str, "column-gap", style.gap()[YGGutterColumn]);
-    }
-    if (yoga::Node::computeRowGap(style.gap(), CompactValue::ofUndefined()) !=
-        yoga::Node::computeRowGap(
-            yoga::Node{}.getStyle().gap(), CompactValue::ofUndefined())) {
-      appendNumberIfNotUndefined(str, "row-gap", style.gap()[YGGutterRow]);
+    if (!style.gap(YGGutterAll).isUndefined()) {
+      appendNumberIfNotUndefined(str, "gap", style.gap(YGGutterAll));
+    } else {
+      appendNumberIfNotUndefined(str, "column-gap", style.gap(YGGutterColumn));
+      appendNumberIfNotUndefined(str, "row-gap", style.gap(YGGutterRow));
     }
 
     appendNumberIfNotAuto(str, "width", style.dimension(Dimension::Width));

diff --git a/packages/react-native/ReactCommon/yoga/yoga/node/Node.cpp b/packages/react-native/ReactCommon/yoga/yoga/node/Node.cpp
@@ -89,30 +89,6 @@ CompactValue Node::computeEdgeValueForColumn(
   }
 }
 
-CompactValue Node::computeRowGap(
-    const Style::Gutters& gutters,
-    CompactValue defaultValue) {
-  if (!gutters[YGGutterRow].isUndefined()) {
-    return gutters[YGGutterRow];
-  } else if (!gutters[YGGutterAll].isUndefined()) {
-    return gutters[YGGutterAll];
-  } else {
-    return defaultValue;
-  }
-}
-
-CompactValue Node::computeColumnGap(
-    const Style::Gutters& gutters,
-    CompactValue defaultValue) {
-  if (!gutters[YGGutterColumn].isUndefined()) {
-    return gutters[YGGutterColumn];
-  } else if (!gutters[YGGutterAll].isUndefined()) {
-    return gutters[YGGutterAll];
-  } else {
-    return defaultValue;
-  }
-}
-
 FloatOptional Node::getLeadingPosition(
     const FlexDirection axis,
     const float axisSize) const {
@@ -202,13 +178,12 @@ FloatOptional Node::getMarginForAxis(
   return getLeadingMargin(axis, widthSize) + getTrailingMargin(axis, widthSize);
 }
 
-float Node::getGapForAxis(const FlexDirection axis, const float widthSize)
-    const {
-  auto gap = isRow(axis)
-      ? computeColumnGap(style_.gap(), CompactValue::ofUndefined())
-      : computeRowGap(style_.gap(), CompactValue::ofUndefined());
-  auto resolvedGap = yoga::resolveValue(gap, widthSize);
-  return maxOrDefined(resolvedGap.unwrap(), 0);
+float Node::getGapForAxis(const FlexDirection axis) const {
+  auto gap = isRow(axis) ? style_.resolveColumnGap() : style_.resolveRowGap();
+  // TODO: Validate percentage gap, and expose ability to set percentage to
+  // public API
+  auto resolvedGap = yoga::resolveValue(gap, 0.0f /*ownerSize*/);
+  return maxOrDefined(resolvedGap.unwrap(), 0.0f);
 }
 
 YGSize Node::measure(

diff --git a/packages/react-native/ReactCommon/yoga/yoga/node/Node.h b/packages/react-native/ReactCommon/yoga/yoga/node/Node.h
@@ -191,14 +191,6 @@ class YG_EXPORT Node : public ::YGNode {
       YGEdge edge,
       CompactValue defaultValue);
 
-  static CompactValue computeRowGap(
-      const Style::Gutters& gutters,
-      CompactValue defaultValue);
-
-  static CompactValue computeColumnGap(
-      const Style::Gutters& gutters,
-      CompactValue defaultValue);
-
   // Methods related to positions, margin, padding and border
   FloatOptional getLeadingPosition(
       const FlexDirection axis,
@@ -231,7 +223,7 @@ class YG_EXPORT Node : public ::YGNode {
   FloatOptional getMarginForAxis(
       const FlexDirection axis,
       const float widthSize) const;
-  float getGapForAxis(const FlexDirection axis, const float widthSize) const;
+  float getGapForAxis(const FlexDirection axis) const;
   // Setters
 
   void setContext(void* context) {

diff --git a/packages/react-native/ReactCommon/yoga/yoga/style/Style.h b/packages/react-native/ReactCommon/yoga/yoga/style/Style.h
@@ -274,11 +274,11 @@ class YG_EXPORT Style {
     return {*this};
   }
 
-  const Gutters& gap() const {
-    return gap_;
+  CompactValue gap(YGGutter gutter) const {
+    return gap_[gutter];
   }
-  IdxRef<YGGutter, &Style::gap_> gap() {
-    return {*this};
+  void setGap(YGGutter gutter, CompactValue value) {
+    gap_[gutter] = value;
   }
 
   CompactValue dimension(Dimension axis) const {
@@ -310,6 +310,22 @@ class YG_EXPORT Style {
     return {*this};
   }
 
+  CompactValue resolveColumnGap() const {
+    if (!gap_[YGGutterColumn].isUndefined()) {
+      return gap_[YGGutterColumn];
+    } else {
+      return gap_[YGGutterAll];
+    }
+  }
+
+  CompactValue resolveRowGap() const {
+    if (!gap_[YGGutterRow].isUndefined()) {
+      return gap_[YGGutterRow];
+    } else {
+      return gap_[YGGutterAll];
+    }
+  }
+
   bool operator==(const Style& other) const {
     return flags == other.flags && inexactEquals(flex_, other.flex_) &&
         inexactEquals(flexGrow_, other.flexGrow_) &&