From 4da0d44e55485bf622aae85a6b7a92e342f3eaf2 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Tue, 3 Oct 2023 10:08:10 -0700 Subject: [PATCH] Remove usage of Gutters arrays and YGGutter as index (#39597) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/39597 X-link: https://github.com/facebook/yoga/pull/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 fbshipit-source-id: 47b06a7301fb283acc493dba159f496159d59580 --- .../components/view/YogaStylableProps.cpp | 26 ++++++------ .../components/view/propsConversions.h | 42 +++++++++++-------- .../ReactCommon/yoga/yoga/Yoga.cpp | 4 +- .../yoga/yoga/algorithm/CalculateLayout.cpp | 9 ++-- .../yoga/yoga/algorithm/FlexLine.cpp | 2 +- .../yoga/yoga/debug/NodeToString.cpp | 16 +++---- .../ReactCommon/yoga/yoga/node/Node.cpp | 37 +++------------- .../ReactCommon/yoga/yoga/node/Node.h | 10 +---- .../ReactCommon/yoga/yoga/style/Style.h | 24 +++++++++-- 9 files changed, 77 insertions(+), 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 index 8e3d0ed53e2ac5..fb7f99584c12d6 100644 --- 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 index 8850a8e1aa865d..218d1ce3ca8a3d 100644 --- 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 index a0119acc8bcd88..865a984e5801b7 100644 --- 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(gapLength); - updateIndexedStyleProp(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 index 384de7135885d1..21a6729f368ea5 100644 --- 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(childCount - 1); + totalMainDim += + node->getGapForAxis(mainAxis) * static_cast(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 index d2034bf2f92c5d..d1070d858b1c6e 100644 --- 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 index 9eb090116e4675..429c2305d301c1 100644 --- 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 index 782c68385e0cd1..8600a8ba61f684 100644 --- 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 index 09c05ebedfa3e5..a98128220e5a4a 100644 --- 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 index 781832e3ac8fb8..3cc1508ad366a2 100644 --- 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 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_) &&