Skip to content

Commit

Permalink
Simplify Edge Resolution (#42254)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #42254

X-link: facebook/yoga#1550

This change aims to simplify how we resolve edges. This operation happens many, many times, and has gotten complex and slow when paired with StyleValuePool.

This starts reshaping so that `yoga::Style` can resolve a style prop for a given edge. This is closer to the ideal computed style API to avoid recalcing this so many times, but doesn't address that.

This relies on removing the errata related to row-reverse, and cleans up the removal started in the last change.

This has no measurable perf effect under CompactValue, but has a >10% uplift in perf when using StyleValueHandle, where we can trivially check if a handle points to a defined value without resolving it, but only within `yoga::Style` since we don't expose the handle outside of it.

More quantifiably, we go from 2.35 million StyleValuePool reads to 993k. The rest are checks on the handle.

Reviewed By: joevilches

Differential Revision: D52605596

fbshipit-source-id: 0b366963a899e376f99ce3d75cd5f14a25d60cec
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Jan 19, 2024
1 parent 0e4a3da commit 40c4552
Show file tree
Hide file tree
Showing 14 changed files with 653 additions and 715 deletions.
18 changes: 9 additions & 9 deletions packages/react-native/ReactCommon/yoga/yoga/YGNodeLayout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,39 +25,39 @@ float getResolvedLayoutProperty(const YGNodeConstRef nodeRef, const Edge edge) {

if (edge == Edge::Start) {
if (node->getLayout().direction() == Direction::RTL) {
return (node->getLayout().*LayoutMember)(Edge::Right);
return (node->getLayout().*LayoutMember)(PhysicalEdge::Right);
} else {
return (node->getLayout().*LayoutMember)(Edge::Left);
return (node->getLayout().*LayoutMember)(PhysicalEdge::Left);
}
}

if (edge == Edge::End) {
if (node->getLayout().direction() == Direction::RTL) {
return (node->getLayout().*LayoutMember)(Edge::Left);
return (node->getLayout().*LayoutMember)(PhysicalEdge::Left);
} else {
return (node->getLayout().*LayoutMember)(Edge::Right);
return (node->getLayout().*LayoutMember)(PhysicalEdge::Right);
}
}

return (node->getLayout().*LayoutMember)(edge);
return (node->getLayout().*LayoutMember)(static_cast<PhysicalEdge>(edge));
}

} // namespace

float YGNodeLayoutGetLeft(const YGNodeConstRef node) {
return resolveRef(node)->getLayout().position(Edge::Left);
return resolveRef(node)->getLayout().position(PhysicalEdge::Left);
}

float YGNodeLayoutGetTop(const YGNodeConstRef node) {
return resolveRef(node)->getLayout().position(Edge::Top);
return resolveRef(node)->getLayout().position(PhysicalEdge::Top);
}

float YGNodeLayoutGetRight(const YGNodeConstRef node) {
return resolveRef(node)->getLayout().position(Edge::Right);
return resolveRef(node)->getLayout().position(PhysicalEdge::Right);
}

float YGNodeLayoutGetBottom(const YGNodeConstRef node) {
return resolveRef(node)->getLayout().position(Edge::Bottom);
return resolveRef(node)->getLayout().position(PhysicalEdge::Bottom);
}

float YGNodeLayoutGetWidth(const YGNodeConstRef node) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ static inline void setFlexStartLayoutPosition(
const FlexDirection axis,
const float containingBlockWidth) {
child->setLayoutPosition(
child->getFlexStartMargin(axis, direction, containingBlockWidth) +
child->style().computeFlexStartMargin(
axis, direction, containingBlockWidth) +
parent->getLayout().border(flexStartEdge(axis)) +
parent->getLayout().padding(flexStartEdge(axis)),
flexStartEdge(axis));
Expand All @@ -36,7 +37,8 @@ static inline void setFlexEndLayoutPosition(
getPositionOfOppositeEdge(
parent->getLayout().border(flexEndEdge(axis)) +
parent->getLayout().padding(flexEndEdge(axis)) +
child->getFlexEndMargin(axis, direction, containingBlockWidth),
child->style().computeFlexEndMargin(
axis, direction, containingBlockWidth),
axis,
parent,
child),
Expand All @@ -57,12 +59,13 @@ static inline void setCenterLayoutPosition(
parent->getLayout().padding(flexEndEdge(axis));
const float childOuterSize =
child->getLayout().measuredDimension(dimension(axis)) +
child->getMarginForAxis(axis, containingBlockWidth);
child->style().computeMarginForAxis(axis, containingBlockWidth);
child->setLayoutPosition(
(parentContentBoxSize - childOuterSize) / 2.0f +
parent->getLayout().border(flexStartEdge(axis)) +
parent->getLayout().padding(flexStartEdge(axis)) +
child->getFlexStartMargin(axis, direction, containingBlockWidth),
child->style().computeFlexStartMargin(
axis, direction, containingBlockWidth),
flexStartEdge(axis));
}

Expand Down Expand Up @@ -150,30 +153,32 @@ static void positionAbsoluteChildLegacy(
: ((resolveChildAlignment(parent, child) == Align::FlexEnd) ^
(parent->style().flexWrap() == Wrap::WrapReverse));

if (child->isFlexEndPositionDefined(axis, direction) &&
!child->isFlexStartPositionDefined(axis, direction)) {
if (child->style().isFlexEndPositionDefined(axis, direction) &&
!child->style().isFlexStartPositionDefined(axis, direction)) {
child->setLayoutPosition(
containingNode->getLayout().measuredDimension(dimension(axis)) -
child->getLayout().measuredDimension(dimension(axis)) -
containingNode->getFlexEndBorder(axis, direction) -
child->getFlexEndMargin(
containingNode->style().computeFlexEndBorder(axis, direction) -
child->style().computeFlexEndMargin(
axis,
direction,
isAxisRow ? containingBlockWidth : containingBlockHeight) -
child->getFlexEndPosition(
child->style().computeFlexEndPosition(
axis,
direction,
isAxisRow ? containingBlockWidth : containingBlockHeight),
flexStartEdge(axis));
} else if (
!child->isFlexStartPositionDefined(axis, direction) && shouldCenter) {
!child->style().isFlexStartPositionDefined(axis, direction) &&
shouldCenter) {
child->setLayoutPosition(
(parent->getLayout().measuredDimension(dimension(axis)) -
child->getLayout().measuredDimension(dimension(axis))) /
2.0f,
flexStartEdge(axis));
} else if (
!child->isFlexStartPositionDefined(axis, direction) && shouldFlexEnd) {
!child->style().isFlexStartPositionDefined(axis, direction) &&
shouldFlexEnd) {
child->setLayoutPosition(
(parent->getLayout().measuredDimension(dimension(axis)) -
child->getLayout().measuredDimension(dimension(axis))),
Expand Down Expand Up @@ -218,25 +223,29 @@ static void positionAbsoluteChildImpl(
// to the flex-start edge because this algorithm works by positioning on the
// flex-start edge and then filling in the flex-end direction at the end if
// necessary.
if (child->isInlineStartPositionDefined(axis, direction)) {
if (child->style().isInlineStartPositionDefined(axis, direction)) {
const float positionRelativeToInlineStart =
child->getInlineStartPosition(axis, direction, containingBlockSize) +
containingNode->getInlineStartBorder(axis, direction) +
child->getInlineStartMargin(axis, direction, containingBlockSize);
child->style().computeInlineStartPosition(
axis, direction, containingBlockSize) +
containingNode->style().computeInlineStartBorder(axis, direction) +
child->style().computeInlineStartMargin(
axis, direction, containingBlockSize);
const float positionRelativeToFlexStart =
inlineStartEdge(axis, direction) != flexStartEdge(axis)
? getPositionOfOppositeEdge(
positionRelativeToInlineStart, axis, containingNode, child)
: positionRelativeToInlineStart;

child->setLayoutPosition(positionRelativeToFlexStart, flexStartEdge(axis));
} else if (child->isInlineEndPositionDefined(axis, direction)) {
} else if (child->style().isInlineEndPositionDefined(axis, direction)) {
const float positionRelativeToInlineStart =
containingNode->getLayout().measuredDimension(dimension(axis)) -
child->getLayout().measuredDimension(dimension(axis)) -
containingNode->getInlineEndBorder(axis, direction) -
child->getInlineEndMargin(axis, direction, containingBlockSize) -
child->getInlineEndPosition(axis, direction, containingBlockSize);
containingNode->style().computeInlineEndBorder(axis, direction) -
child->style().computeInlineEndMargin(
axis, direction, containingBlockSize) -
child->style().computeInlineEndPosition(
axis, direction, containingBlockSize);
const float positionRelativeToFlexStart =
inlineStartEdge(axis, direction) != flexStartEdge(axis)
? getPositionOfOppositeEdge(
Expand Down Expand Up @@ -303,10 +312,10 @@ void layoutAbsoluteChild(
SizingMode childWidthSizingMode = SizingMode::MaxContent;
SizingMode childHeightSizingMode = SizingMode::MaxContent;

auto marginRow =
child->getMarginForAxis(FlexDirection::Row, containingBlockWidth);
auto marginColumn =
child->getMarginForAxis(FlexDirection::Column, containingBlockWidth);
auto marginRow = child->style().computeMarginForAxis(
FlexDirection::Row, containingBlockWidth);
auto marginColumn = child->style().computeMarginForAxis(
FlexDirection::Column, containingBlockWidth);

if (child->hasDefiniteLength(Dimension::Width, containingBlockWidth)) {
childWidth = child->getResolvedDimension(Dimension::Width)
Expand All @@ -316,15 +325,19 @@ void layoutAbsoluteChild(
} else {
// If the child doesn't have a specified width, compute the width based on
// the left/right offsets if they're defined.
if (child->isFlexStartPositionDefined(FlexDirection::Row, direction) &&
child->isFlexEndPositionDefined(FlexDirection::Row, direction)) {
if (child->style().isFlexStartPositionDefined(
FlexDirection::Row, direction) &&
child->style().isFlexEndPositionDefined(
FlexDirection::Row, direction)) {
childWidth =
containingNode->getLayout().measuredDimension(Dimension::Width) -
(containingNode->getFlexStartBorder(FlexDirection::Row, direction) +
containingNode->getFlexEndBorder(FlexDirection::Row, direction)) -
(child->getFlexStartPosition(
(containingNode->style().computeFlexStartBorder(
FlexDirection::Row, direction) +
containingNode->style().computeFlexEndBorder(
FlexDirection::Row, direction)) -
(child->style().computeFlexStartPosition(
FlexDirection::Row, direction, containingBlockWidth) +
child->getFlexEndPosition(
child->style().computeFlexEndPosition(
FlexDirection::Row, direction, containingBlockWidth));
childWidth = boundAxis(
child,
Expand All @@ -343,16 +356,19 @@ void layoutAbsoluteChild(
} else {
// If the child doesn't have a specified height, compute the height based
// on the top/bottom offsets if they're defined.
if (child->isFlexStartPositionDefined(FlexDirection::Column, direction) &&
child->isFlexEndPositionDefined(FlexDirection::Column, direction)) {
if (child->style().isFlexStartPositionDefined(
FlexDirection::Column, direction) &&
child->style().isFlexEndPositionDefined(
FlexDirection::Column, direction)) {
childHeight =
containingNode->getLayout().measuredDimension(Dimension::Height) -
(containingNode->getFlexStartBorder(
(containingNode->style().computeFlexStartBorder(
FlexDirection::Column, direction) +
containingNode->getFlexEndBorder(FlexDirection::Column, direction)) -
(child->getFlexStartPosition(
containingNode->style().computeFlexEndBorder(
FlexDirection::Column, direction)) -
(child->style().computeFlexStartPosition(
FlexDirection::Column, direction, containingBlockHeight) +
child->getFlexEndPosition(
child->style().computeFlexEndPosition(
FlexDirection::Column, direction, containingBlockHeight));
childHeight = boundAxis(
child,
Expand Down Expand Up @@ -414,9 +430,11 @@ void layoutAbsoluteChild(
depth,
generationCount);
childWidth = child->getLayout().measuredDimension(Dimension::Width) +
child->getMarginForAxis(FlexDirection::Row, containingBlockWidth);
child->style().computeMarginForAxis(
FlexDirection::Row, containingBlockWidth);
childHeight = child->getLayout().measuredDimension(Dimension::Height) +
child->getMarginForAxis(FlexDirection::Column, containingBlockWidth);
child->style().computeMarginForAxis(
FlexDirection::Column, containingBlockWidth);
}

calculateLayoutInternal(
Expand Down Expand Up @@ -479,11 +497,12 @@ void layoutAbsoluteDescendants(
const float containingBlockWidth = absoluteErrata
? containingNodeAvailableInnerWidth
: containingNode->getLayout().measuredDimension(Dimension::Width) -
containingNode->getBorderForAxis(FlexDirection::Row);
containingNode->style().computeBorderForAxis(FlexDirection::Row);
const float containingBlockHeight = absoluteErrata
? containingNodeAvailableInnerHeight
: containingNode->getLayout().measuredDimension(Dimension::Height) -
containingNode->getBorderForAxis(FlexDirection::Column);
containingNode->style().computeBorderForAxis(
FlexDirection::Column);

layoutAbsoluteChild(
containingNode,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ float calculateBaseline(const yoga::Node* node) {
}

const float baseline = calculateBaseline(baselineChild);
return baseline + baselineChild->getLayout().position(Edge::Top);
return baseline + baselineChild->getLayout().position(PhysicalEdge::Top);
}

bool isBaselineLayout(const yoga::Node* node) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ inline float paddingAndBorderForAxis(
const float widthSize) {
// The total padding/border for a given axis does not depend on the direction
// so hardcoding LTR here to avoid piping direction to this function
return node->getInlineStartPaddingAndBorder(axis, Direction::LTR, widthSize) +
node->getInlineEndPaddingAndBorder(axis, Direction::LTR, widthSize);
return node->style().computeInlineStartPaddingAndBorder(
axis, Direction::LTR, widthSize) +
node->style().computeInlineEndPaddingAndBorder(
axis, Direction::LTR, widthSize);
}

inline FloatOptional boundAxisWithinMinAndMax(
Expand Down
Loading

0 comments on commit 40c4552

Please sign in to comment.