Skip to content

Commit

Permalink
Remove usage of Gutters arrays and YGGutter as index (#1406)
Browse files Browse the repository at this point in the history
Summary:
X-link: facebook/react-native#39597

Pull Request resolved: #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
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Sep 26, 2023
1 parent 007154b commit 13da6bd
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 63 deletions.
4 changes: 2 additions & 2 deletions yoga/Yoga.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
9 changes: 4 additions & 5 deletions yoga/algorithm/CalculateLayout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down Expand Up @@ -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 =
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion yoga/algorithm/FlexLine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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++) {
Expand Down
16 changes: 5 additions & 11 deletions yoga/debug/NodeToString.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
37 changes: 6 additions & 31 deletions yoga/node/Node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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(
Expand Down
10 changes: 1 addition & 9 deletions yoga/node/Node.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down
24 changes: 20 additions & 4 deletions yoga/style/Style.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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_) &&
Expand Down

0 comments on commit 13da6bd

Please sign in to comment.