From 13da6bdb93a4611f09e1b8ef2b5bc45dc88c6754 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Mon, 25 Sep 2023 20:24:13 -0700 Subject: [PATCH] Remove usage of Gutters arrays and YGGutter as index (#1406) Summary: X-link: https://github.com/facebook/react-native/pull/39597 Pull Request resolved: 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. Differential Revision: D49530923 fbshipit-source-id: 651b2937ef1eefc2885dc517a80d819af885f97b --- yoga/Yoga.cpp | 4 ++-- yoga/algorithm/CalculateLayout.cpp | 9 ++++---- yoga/algorithm/FlexLine.cpp | 2 +- yoga/debug/NodeToString.cpp | 16 ++++--------- yoga/node/Node.cpp | 37 +++++------------------------- yoga/node/Node.h | 10 +------- yoga/style/Style.h | 24 +++++++++++++++---- 7 files changed, 39 insertions(+), 63 deletions(-) diff --git a/yoga/Yoga.cpp b/yoga/Yoga.cpp index a0119acc8b..865a984e58 100644 --- a/yoga/Yoga.cpp +++ b/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/yoga/algorithm/CalculateLayout.cpp b/yoga/algorithm/CalculateLayout.cpp index 384de71358..21a6729f36 100644 --- a/yoga/algorithm/CalculateLayout.cpp +++ b/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/yoga/algorithm/FlexLine.cpp b/yoga/algorithm/FlexLine.cpp index aab71aa20d..a9c044eef3 100644 --- a/yoga/algorithm/FlexLine.cpp +++ b/yoga/algorithm/FlexLine.cpp @@ -33,7 +33,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/yoga/debug/NodeToString.cpp b/yoga/debug/NodeToString.cpp index 9eb090116e..429c2305d3 100644 --- a/yoga/debug/NodeToString.cpp +++ b/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/yoga/node/Node.cpp b/yoga/node/Node.cpp index 782c68385e..8600a8ba61 100644 --- a/yoga/node/Node.cpp +++ b/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/yoga/node/Node.h b/yoga/node/Node.h index 09c05ebedf..a98128220e 100644 --- a/yoga/node/Node.h +++ b/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/yoga/style/Style.h b/yoga/style/Style.h index 781832e3ac..3cc1508ad3 100644 --- a/yoga/style/Style.h +++ b/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_) &&