Skip to content

Commit

Permalink
Introduce isDefined() and remove cases of !isUndefined() (facebook#41209
Browse files Browse the repository at this point in the history 
)

Summary:
Pull Request resolved: facebook#41209

X-link: facebook/yoga#1439

There are so many instances in this code base where we use the double negative of `!yoga::isUndefined(<something>)`. This is not as easy to read since because of this double negative imo. Additionally, sometimes we have really long chains like `!longVariableName.longFunctionName(longArgumentName).isUndefined()` and it is hard to see that this undefined is inverted.

This just replaces all instances of inverted `isUndefined()` with `isDefined()` so its easier to read.

Reviewed By: NickGerleman

Differential Revision: D50705523

fbshipit-source-id: b0ee34edabafc5cfd32f71e7667da2957a06e3fb
  • Loading branch information
@facebook-github-bot
Joe Vilches authored and facebook-github-bot committed Nov 3, 2023
1 parent 917956e commit f2822fa
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 70 deletions.
92 changes: 45 additions & 47 deletions packages/react-native/ReactCommon/yoga/yoga/algorithm/CalculateLayout.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,24 +61,24 @@ static inline bool styleDefinesDimension(
const yoga::Node* const node,
const FlexDirection axis,
const float ownerSize) {
bool isUndefined =
yoga::isUndefined(node->getResolvedDimension(dimension(axis)).value);
bool isDefined =
yoga::isDefined(node->getResolvedDimension(dimension(axis)).value);

auto resolvedDimension = node->getResolvedDimension(dimension(axis));
return !(
resolvedDimension.unit == YGUnitAuto ||
resolvedDimension.unit == YGUnitUndefined ||
(resolvedDimension.unit == YGUnitPoint && !isUndefined &&
(resolvedDimension.unit == YGUnitPoint && isDefined &&
resolvedDimension.value < 0.0f) ||
(resolvedDimension.unit == YGUnitPercent && !isUndefined &&
(resolvedDimension.unit == YGUnitPercent && isDefined &&
(resolvedDimension.value < 0.0f || yoga::isUndefined(ownerSize))));
}

static inline bool isLayoutDimensionDefined(
const yoga::Node* const node,
const FlexDirection axis) {
const float value = node->getLayout().measuredDimension(dimension(axis));
return !yoga::isUndefined(value) && value >= 0.0f;
return yoga::isDefined(value) && value >= 0.0f;
}

static void setChildTrailingPosition(
Expand Down Expand Up @@ -111,7 +111,7 @@ static void constrainMaxSizeForMode(
: maxSize.unwrap();
break;
case MeasureMode::Undefined:
if (!maxSize.isUndefined()) {
if (maxSize.isDefined()) {
*mode = MeasureMode::AtMost;
*size = maxSize.unwrap();
}
Expand Down Expand Up @@ -150,7 +150,7 @@ static void computeFlexBasisForChild(
const bool isColumnStyleDimDefined =
styleDefinesDimension(child, FlexDirection::Column, ownerHeight);

if (!resolvedFlexBasis.isUndefined() && !yoga::isUndefined(mainAxisSize)) {
if (resolvedFlexBasis.isDefined() && yoga::isDefined(mainAxisSize)) {
if (child->getLayout().computedFlexBasis.isUndefined() ||
(child->getConfig()->isExperimentalFeatureEnabled(
ExperimentalFeature::WebFlexBasis) &&
Expand Down Expand Up @@ -210,22 +210,22 @@ static void computeFlexBasisForChild(
// major browsers appear to implement the following logic.
if ((!isMainAxisRow && node->getStyle().overflow() == Overflow::Scroll) ||
node->getStyle().overflow() != Overflow::Scroll) {
if (yoga::isUndefined(childWidth) && !yoga::isUndefined(width)) {
if (yoga::isUndefined(childWidth) && yoga::isDefined(width)) {
childWidth = width;
childWidthMeasureMode = MeasureMode::AtMost;
}
}

if ((isMainAxisRow && node->getStyle().overflow() == Overflow::Scroll) ||
node->getStyle().overflow() != Overflow::Scroll) {
if (yoga::isUndefined(childHeight) && !yoga::isUndefined(height)) {
if (yoga::isUndefined(childHeight) && yoga::isDefined(height)) {
childHeight = height;
childHeightMeasureMode = MeasureMode::AtMost;
}
}

const auto& childStyle = child->getStyle();
if (!childStyle.aspectRatio().isUndefined()) {
if (childStyle.aspectRatio().isDefined()) {
if (!isMainAxisRow && childWidthMeasureMode == MeasureMode::Exactly) {
childHeight = marginColumn +
(childWidth - marginRow) / childStyle.aspectRatio().unwrap();
Expand All @@ -242,23 +242,23 @@ static void computeFlexBasisForChild(
// the cross axis to be measured exactly with the available inner width

const bool hasExactWidth =
!yoga::isUndefined(width) && widthMode == MeasureMode::Exactly;
yoga::isDefined(width) && widthMode == MeasureMode::Exactly;
const bool childWidthStretch =
resolveChildAlignment(node, child) == Align::Stretch &&
childWidthMeasureMode != MeasureMode::Exactly;
if (!isMainAxisRow && !isRowStyleDimDefined && hasExactWidth &&
childWidthStretch) {
childWidth = width;
childWidthMeasureMode = MeasureMode::Exactly;
if (!childStyle.aspectRatio().isUndefined()) {
if (childStyle.aspectRatio().isDefined()) {
childHeight =
(childWidth - marginRow) / childStyle.aspectRatio().unwrap();
childHeightMeasureMode = MeasureMode::Exactly;
}
}

const bool hasExactHeight =
!yoga::isUndefined(height) && heightMode == MeasureMode::Exactly;
yoga::isDefined(height) && heightMode == MeasureMode::Exactly;
const bool childHeightStretch =
resolveChildAlignment(node, child) == Align::Stretch &&
childHeightMeasureMode != MeasureMode::Exactly;
Expand All @@ -267,7 +267,7 @@ static void computeFlexBasisForChild(
childHeight = height;
childHeightMeasureMode = MeasureMode::Exactly;

if (!childStyle.aspectRatio().isUndefined()) {
if (childStyle.aspectRatio().isDefined()) {
childWidth =
(childHeight - marginColumn) * childStyle.aspectRatio().unwrap();
childWidthMeasureMode = MeasureMode::Exactly;
Expand Down Expand Up @@ -382,7 +382,7 @@ static void layoutAbsoluteChild(
// flexible.
const auto& childStyle = child->getStyle();
if (yoga::isUndefined(childWidth) ^ yoga::isUndefined(childHeight)) {
if (!childStyle.aspectRatio().isUndefined()) {
if (childStyle.aspectRatio().isDefined()) {
if (yoga::isUndefined(childWidth)) {
childWidth = marginRow +
(childHeight - marginColumn) * childStyle.aspectRatio().unwrap();
Expand All @@ -407,7 +407,7 @@ static void layoutAbsoluteChild(
// wrap to the size of its owner. This is the same behavior as many browsers
// implement.
if (!isMainAxisRow && yoga::isUndefined(childWidth) &&
widthMode != MeasureMode::Undefined && !yoga::isUndefined(width) &&
widthMode != MeasureMode::Undefined && yoga::isDefined(width) &&
width > 0) {
childWidth = width;
childWidthMeasureMode = MeasureMode::AtMost;
Expand Down Expand Up @@ -678,9 +678,9 @@ static bool measureNodeWithFixedSize(
const MeasureMode heightMeasureMode,
const float ownerWidth,
const float ownerHeight) {
if ((!yoga::isUndefined(availableWidth) &&
if ((yoga::isDefined(availableWidth) &&
widthMeasureMode == MeasureMode::AtMost && availableWidth <= 0.0f) ||
(!yoga::isUndefined(availableHeight) &&
(yoga::isDefined(availableHeight) &&
heightMeasureMode == MeasureMode::AtMost && availableHeight <= 0.0f) ||
(widthMeasureMode == MeasureMode::Exactly &&
heightMeasureMode == MeasureMode::Exactly)) {
Expand Down Expand Up @@ -736,7 +736,7 @@ static float calculateAvailableInnerDimension(
float availableInnerDim = availableDim - paddingAndBorder;
// Max dimension overrides predefined dimension value; Min dimension in turn
// overrides both of the above
if (!yoga::isUndefined(availableInnerDim)) {
if (yoga::isDefined(availableInnerDim)) {
// We want to make sure our available height does not violate min and max
// constraints
const FloatOptional minDimensionOptional =
Expand Down Expand Up @@ -880,15 +880,15 @@ static float distributeFreeSpaceSecondPass(
.unwrap();
float updatedMainSize = childFlexBasis;

if (!yoga::isUndefined(flexLine.layout.remainingFreeSpace) &&
if (yoga::isDefined(flexLine.layout.remainingFreeSpace) &&
flexLine.layout.remainingFreeSpace < 0) {
flexShrinkScaledFactor =
-currentLineChild->resolveFlexShrink() * childFlexBasis;
// Is this child able to shrink?
if (flexShrinkScaledFactor != 0) {
float childSize;

if (!yoga::isUndefined(flexLine.layout.totalFlexShrinkScaledFactors) &&
if (yoga::isDefined(flexLine.layout.totalFlexShrinkScaledFactors) &&
flexLine.layout.totalFlexShrinkScaledFactors == 0) {
childSize = childFlexBasis + flexShrinkScaledFactor;
} else {
Expand All @@ -906,7 +906,7 @@ static float distributeFreeSpaceSecondPass(
availableInnerWidth);
}
} else if (
!yoga::isUndefined(flexLine.layout.remainingFreeSpace) &&
yoga::isDefined(flexLine.layout.remainingFreeSpace) &&
flexLine.layout.remainingFreeSpace > 0) {
flexGrowFactor = currentLineChild->resolveFlexGrow();

Expand Down Expand Up @@ -936,7 +936,7 @@ static float distributeFreeSpaceSecondPass(
MeasureMode childMainMeasureMode = MeasureMode::Exactly;

const auto& childStyle = currentLineChild->getStyle();
if (!childStyle.aspectRatio().isUndefined()) {
if (childStyle.aspectRatio().isDefined()) {
childCrossSize = isMainAxisRow
? (childMainSize - marginMain) / childStyle.aspectRatio().unwrap()
: (childMainSize - marginMain) * childStyle.aspectRatio().unwrap();
Expand Down Expand Up @@ -1062,7 +1062,7 @@ static void distributeFreeSpaceFirstPass(
-currentLineChild->resolveFlexShrink() * childFlexBasis;

// Is this child able to shrink?
if (!yoga::isUndefined(flexShrinkScaledFactor) &&
if (yoga::isDefined(flexShrinkScaledFactor) &&
flexShrinkScaledFactor != 0) {
baseMainSize = childFlexBasis +
flexLine.layout.remainingFreeSpace /
Expand All @@ -1074,8 +1074,7 @@ static void distributeFreeSpaceFirstPass(
baseMainSize,
availableInnerMainDim,
availableInnerWidth);
if (!yoga::isUndefined(baseMainSize) &&
!yoga::isUndefined(boundMainSize) &&
if (yoga::isDefined(baseMainSize) && yoga::isDefined(boundMainSize) &&
baseMainSize != boundMainSize) {
// By excluding this item's size and flex factor from remaining, this
// item's min/max constraints should also trigger in the second pass
Expand All @@ -1088,12 +1087,12 @@ static void distributeFreeSpaceFirstPass(
}
}
} else if (
!yoga::isUndefined(flexLine.layout.remainingFreeSpace) &&
yoga::isDefined(flexLine.layout.remainingFreeSpace) &&
flexLine.layout.remainingFreeSpace > 0) {
flexGrowFactor = currentLineChild->resolveFlexGrow();

// Is this child able to grow?
if (!yoga::isUndefined(flexGrowFactor) && flexGrowFactor != 0) {
if (yoga::isDefined(flexGrowFactor) && flexGrowFactor != 0) {
baseMainSize = childFlexBasis +
flexLine.layout.remainingFreeSpace /
flexLine.layout.totalFlexGrowFactors * flexGrowFactor;
Expand All @@ -1104,8 +1103,7 @@ static void distributeFreeSpaceFirstPass(
availableInnerMainDim,
availableInnerWidth);

if (!yoga::isUndefined(baseMainSize) &&
!yoga::isUndefined(boundMainSize) &&
if (yoga::isDefined(baseMainSize) && yoga::isDefined(boundMainSize) &&
baseMainSize != boundMainSize) {
// By excluding this item's size and flex factor from remaining, this
// item's min/max constraints should also trigger in the second pass
Expand Down Expand Up @@ -1219,10 +1217,10 @@ static void justifyMainAxis(
// remainingFreeSpace is 0 when min main dimension is not given
if (measureModeMainDim == MeasureMode::AtMost &&
flexLine.layout.remainingFreeSpace > 0) {
if (!style.minDimension(dimension(mainAxis)).isUndefined() &&
!yoga::resolveValue(
style.minDimension(dimension(mainAxis)), mainAxisownerSize)
.isUndefined()) {
if (style.minDimension(dimension(mainAxis)).isDefined() &&
yoga::resolveValue(
style.minDimension(dimension(mainAxis)), mainAxisownerSize)
.isDefined()) {
// This condition makes sure that if the size of main dimension(after
// considering child nodes main dim, leading and trailing padding etc)
// falls below min dimension, then the remainingFreeSpace is reassigned
Expand Down Expand Up @@ -1743,21 +1741,21 @@ static void calculateLayoutImpl(
const float maxInnerMainDim =
isMainAxisRow ? maxInnerWidth : maxInnerHeight;

if (!yoga::isUndefined(minInnerMainDim) &&
if (yoga::isDefined(minInnerMainDim) &&
flexLine.sizeConsumed < minInnerMainDim) {
availableInnerMainDim = minInnerMainDim;
} else if (
!yoga::isUndefined(maxInnerMainDim) &&
yoga::isDefined(maxInnerMainDim) &&
flexLine.sizeConsumed > maxInnerMainDim) {
availableInnerMainDim = maxInnerMainDim;
} else {
bool useLegacyStretchBehaviour =
node->hasErrata(Errata::StretchFlexBasis);

if (!useLegacyStretchBehaviour &&
((!yoga::isUndefined(flexLine.layout.totalFlexGrowFactors) &&
((yoga::isDefined(flexLine.layout.totalFlexGrowFactors) &&
flexLine.layout.totalFlexGrowFactors == 0) ||
(!yoga::isUndefined(node->resolveFlexGrow()) &&
(yoga::isDefined(node->resolveFlexGrow()) &&
node->resolveFlexGrow() == 0))) {
// If we don't have any children to flex or we can't flex the node
// itself, space we've used is all space we need. Root node also
Expand All @@ -1769,7 +1767,7 @@ static void calculateLayoutImpl(
}
}

if (!sizeBasedOnContent && !yoga::isUndefined(availableInnerMainDim)) {
if (!sizeBasedOnContent && yoga::isDefined(availableInnerMainDim)) {
flexLine.layout.remainingFreeSpace =
availableInnerMainDim - flexLine.sizeConsumed;
} else if (flexLine.sizeConsumed < 0) {
Expand Down Expand Up @@ -1910,7 +1908,7 @@ static void calculateLayoutImpl(
float childMainSize =
child->getLayout().measuredDimension(dimension(mainAxis));
const auto& childStyle = child->getStyle();
float childCrossSize = !childStyle.aspectRatio().isUndefined()
float childCrossSize = childStyle.aspectRatio().isDefined()
? child->getMarginForAxis(crossAxis, availableInnerWidth) +
(isMainAxisRow
? childMainSize / childStyle.aspectRatio().unwrap()
Expand Down Expand Up @@ -2013,7 +2011,7 @@ static void calculateLayoutImpl(
if (performLayout && (isNodeFlexWrap || isBaselineLayout(node))) {
float crossDimLead = 0;
float currentLead = leadingPaddingAndBorderCross;
if (!yoga::isUndefined(availableInnerCrossDim)) {
if (yoga::isDefined(availableInnerCrossDim)) {
const float remainingAlignContentDim =
availableInnerCrossDim - totalLineCrossDim;
switch (node->getStyle().alignContent()) {
Expand Down Expand Up @@ -2691,9 +2689,9 @@ void calculateLayout(
.unwrap() +
node->getMarginForAxis(FlexDirection::Row, ownerWidth));
widthMeasureMode = MeasureMode::Exactly;
} else if (!yoga::resolveValue(
style.maxDimension(Dimension::Width), ownerWidth)
.isUndefined()) {
} else if (yoga::resolveValue(
style.maxDimension(Dimension::Width), ownerWidth)
.isDefined()) {
width = yoga::resolveValue(style.maxDimension(Dimension::Width), ownerWidth)
.unwrap();
widthMeasureMode = MeasureMode::AtMost;
Expand All @@ -2713,9 +2711,9 @@ void calculateLayout(
.unwrap() +
node->getMarginForAxis(FlexDirection::Column, ownerWidth));
heightMeasureMode = MeasureMode::Exactly;
} else if (!yoga::resolveValue(
style.maxDimension(Dimension::Height), ownerHeight)
.isUndefined()) {
} else if (yoga::resolveValue(
style.maxDimension(Dimension::Height), ownerHeight)
.isDefined()) {
height =
yoga::resolveValue(style.maxDimension(Dimension::Height), ownerHeight)
.unwrap();
Expand Down
4 changes: 2 additions & 2 deletions packages/react-native/ReactCommon/yoga/yoga/debug/NodeToString.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ static void appendFloatOptionalIfDefined(
std::string& base,
const std::string key,
const FloatOptional num) {
if (!num.isUndefined()) {
if (num.isDefined()) {
appendFormattedString(base, "%s: %g; ", key.c_str(), num.unwrap());
}
}
Expand Down Expand Up @@ -177,7 +177,7 @@ void nodeToString(
appendEdges(str, "padding", style.padding());
appendEdges(str, "border", style.border());

if (!style.gap(Gutter::All).isUndefined()) {
if (style.gap(Gutter::All).isDefined()) {
appendNumberIfNotUndefined(str, "gap", style.gap(Gutter::All));
} else {
appendNumberIfNotUndefined(str, "column-gap", style.gap(Gutter::Column));
Expand Down
Loading

0 comments on commit f2822fa

Please sign in to comment.