Skip to content

Commit

Permalink
Fix issue where borderStart and borderEnd were swapped with row reverse
Browse files Browse the repository at this point in the history
Differential Revision: https://www.internalfb.com/diff/D50348085?entry_point=27

fbshipit-source-id: c6f9bf543224f5d2912f3491f7c6917ebc2b3fd9
  • Loading branch information
Joe Vilches authored and facebook-github-bot committed Oct 17, 2023
1 parent c4958d2 commit 1f4ff62
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ inline float paddingAndBorderForAxis(
const yoga::Node* const node,
const FlexDirection axis,
const float widthSize) {
return node->getFlexStartPaddingAndBorder(axis, widthSize) +
node->getFlexEndPaddingAndBorder(axis, 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);
}

inline FloatOptional boundAxisWithinMinAndMax(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,8 +346,8 @@ static void layoutAbsoluteChild(
if (child->isFlexStartPositionDefined(FlexDirection::Row) &&
child->isFlexEndPositionDefined(FlexDirection::Row)) {
childWidth = node->getLayout().measuredDimension(Dimension::Width) -
(node->getFlexStartBorder(FlexDirection::Row) +
node->getFlexEndBorder(FlexDirection::Row)) -
(node->getInlineStartBorder(FlexDirection::Row, direction) +
node->getInlineEndBorder(FlexDirection::Row, direction)) -
(child->getFlexStartPosition(FlexDirection::Row, width) +
child->getFlexEndPosition(FlexDirection::Row, width));
childWidth =
Expand All @@ -366,8 +366,8 @@ static void layoutAbsoluteChild(
if (child->isFlexStartPositionDefined(FlexDirection::Column) &&
child->isFlexEndPositionDefined(FlexDirection::Column)) {
childHeight = node->getLayout().measuredDimension(Dimension::Height) -
(node->getFlexStartBorder(FlexDirection::Column) +
node->getFlexEndBorder(FlexDirection::Column)) -
(node->getInlineStartBorder(FlexDirection::Column, direction) +
node->getInlineEndBorder(FlexDirection::Column, direction)) -
(child->getFlexStartPosition(FlexDirection::Column, height) +
child->getFlexEndPosition(FlexDirection::Column, height));
childHeight =
Expand Down Expand Up @@ -451,7 +451,7 @@ static void layoutAbsoluteChild(
child->setLayoutPosition(
node->getLayout().measuredDimension(dimension(mainAxis)) -
child->getLayout().measuredDimension(dimension(mainAxis)) -
node->getFlexEndBorder(mainAxis) -
node->getInlineEndBorder(mainAxis, direction) -
child->getInlineEndMargin(
mainAxis, direction, isMainAxisRow ? width : height) -
child->getFlexEndPosition(mainAxis, isMainAxisRow ? width : height),
Expand Down Expand Up @@ -479,7 +479,7 @@ static void layoutAbsoluteChild(
child->getFlexStartPosition(
mainAxis,
node->getLayout().measuredDimension(dimension(mainAxis))) +
node->getFlexStartBorder(mainAxis) +
node->getInlineStartBorder(mainAxis, direction) +
child->getInlineStartMargin(
mainAxis,
direction,
Expand All @@ -492,7 +492,7 @@ static void layoutAbsoluteChild(
child->setLayoutPosition(
node->getLayout().measuredDimension(dimension(crossAxis)) -
child->getLayout().measuredDimension(dimension(crossAxis)) -
node->getFlexEndBorder(crossAxis) -
node->getInlineEndBorder(crossAxis, direction) -
child->getInlineEndMargin(
crossAxis, direction, isMainAxisRow ? height : width) -
child->getFlexEndPosition(
Expand Down Expand Up @@ -523,7 +523,7 @@ static void layoutAbsoluteChild(
child->getFlexStartPosition(
crossAxis,
node->getLayout().measuredDimension(dimension(crossAxis))) +
node->getFlexStartBorder(crossAxis) +
node->getInlineStartBorder(crossAxis, direction) +
child->getInlineStartMargin(
crossAxis,
direction,
Expand Down Expand Up @@ -1199,10 +1199,16 @@ static void justifyMainAxis(
const float availableInnerWidth,
const bool performLayout) {
const auto& style = node->getStyle();

const float leadingPaddingAndBorderMain =
node->getFlexStartPaddingAndBorder(mainAxis, ownerWidth);
node->hasErrata(Errata::StartingEndingEdgeFromFlexDirection)
? node->getInlineStartPaddingAndBorder(mainAxis, direction, ownerWidth)
: node->getFlexStartPaddingAndBorder(mainAxis, direction, ownerWidth);
const float trailingPaddingAndBorderMain =
node->getFlexEndPaddingAndBorder(mainAxis, ownerWidth);
node->hasErrata(Errata::StartingEndingEdgeFromFlexDirection)
? node->getInlineEndPaddingAndBorder(mainAxis, direction, ownerWidth)
: node->getFlexEndPaddingAndBorder(mainAxis, direction, 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
Expand Down Expand Up @@ -1306,7 +1312,7 @@ static void justifyMainAxis(
// margin/border).
child->setLayoutPosition(
child->getFlexStartPosition(mainAxis, availableInnerMainDim) +
node->getFlexStartBorder(mainAxis) +
node->getInlineStartBorder(mainAxis, direction) +
child->getInlineStartMargin(
mainAxis, direction, availableInnerWidth),
flexStartEdge(mainAxis));
Expand Down Expand Up @@ -1380,7 +1386,8 @@ static void justifyMainAxis(
} else if (performLayout) {
child->setLayoutPosition(
childLayout.position[flexStartEdge(mainAxis)] +
node->getFlexStartBorder(mainAxis) + leadingMainDim,
node->getInlineStartBorder(mainAxis, direction) +
leadingMainDim,
flexStartEdge(mainAxis));
}
}
Expand Down Expand Up @@ -1518,12 +1525,14 @@ static void calculateLayoutImpl(
const float marginAxisRow = marginRowLeading + marginRowTrailing;
const float marginAxisColumn = marginColumnLeading + marginColumnTrailing;

node->setLayoutBorder(node->getFlexStartBorder(flexRowDirection), startEdge);
node->setLayoutBorder(node->getFlexEndBorder(flexRowDirection), endEdge);
node->setLayoutBorder(
node->getFlexStartBorder(flexColumnDirection), YGEdgeTop);
node->getInlineStartBorder(flexRowDirection, direction), startEdge);
node->setLayoutBorder(
node->getInlineEndBorder(flexRowDirection, direction), endEdge);
node->setLayoutBorder(
node->getInlineStartBorder(flexColumnDirection, direction), YGEdgeTop);
node->setLayoutBorder(
node->getFlexEndBorder(flexColumnDirection), YGEdgeBottom);
node->getInlineEndBorder(flexColumnDirection, direction), YGEdgeBottom);

node->setLayoutPadding(
node->getFlexStartPadding(flexRowDirection, ownerWidth), startEdge);
Expand Down Expand Up @@ -1594,9 +1603,9 @@ static void calculateLayoutImpl(
const float paddingAndBorderAxisMain =
paddingAndBorderForAxis(node, mainAxis, ownerWidth);
const float leadingPaddingAndBorderCross =
node->getFlexStartPaddingAndBorder(crossAxis, ownerWidth);
node->getInlineStartPaddingAndBorder(crossAxis, direction, ownerWidth);
const float trailingPaddingAndBorderCross =
node->getFlexEndPaddingAndBorder(crossAxis, ownerWidth);
node->getInlineEndPaddingAndBorder(crossAxis, direction, ownerWidth);
const float paddingAndBorderAxisCross =
leadingPaddingAndBorderCross + trailingPaddingAndBorderCross;

Expand Down Expand Up @@ -1855,7 +1864,7 @@ static void calculateLayoutImpl(
if (isChildLeadingPosDefined) {
child->setLayoutPosition(
child->getFlexStartPosition(crossAxis, availableInnerCrossDim) +
node->getFlexStartBorder(crossAxis) +
node->getInlineStartBorder(crossAxis, direction) +
child->getInlineStartMargin(
crossAxis, direction, availableInnerWidth),
flexStartEdge(crossAxis));
Expand All @@ -1866,7 +1875,7 @@ static void calculateLayoutImpl(
yoga::isUndefined(
child->getLayout().position[flexStartEdge(crossAxis)])) {
child->setLayoutPosition(
node->getFlexStartBorder(crossAxis) +
node->getInlineStartBorder(crossAxis, direction) +
child->getInlineStartMargin(
crossAxis, direction, availableInnerWidth),
flexStartEdge(crossAxis));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,32 @@ inline YGEdge inlineEndEdge(
return YGEdgeBottom;
}

/**
* The physical edges that YGEdgeStart and YGEdgeEnd correspond to (e.g.
* left/right) are soley dependent on the direction. However, there are cases
* where we want the flex start/end edge (i.e. which edge is the start/end
* for laying out flex items), which can be distinct from the corresponding
* inline edge. In these cases we need to know which "relative edge"
* (YGEdgeStart/YGEdgeEnd) corresponds to the said flex start/end edge as these
* relative edges can be used instead of physical ones when defining certain
* attributes like border or padding.
*/
inline YGEdge flexStartRelativeEdge(
FlexDirection flexDirection,
Direction direction) {
const YGEdge leadLayoutEdge = inlineStartEdge(flexDirection, direction);
const YGEdge leadFlexItemEdge = flexStartEdge(flexDirection);
return leadLayoutEdge == leadFlexItemEdge ? YGEdgeStart : YGEdgeEnd;
}

inline YGEdge flexEndRelativeEdge(
FlexDirection flexDirection,
Direction direction) {
const YGEdge trailLayoutEdge = inlineEndEdge(flexDirection, direction);
const YGEdge trailFlexItemEdge = flexEndEdge(flexDirection);
return trailLayoutEdge == trailFlexItemEdge ? YGEdgeEnd : YGEdgeStart;
}

inline Dimension dimension(const FlexDirection flexDirection) {
switch (flexDirection) {
case FlexDirection::Column:
Expand Down
64 changes: 54 additions & 10 deletions packages/react-native/ReactCommon/yoga/yoga/node/Node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -157,18 +157,41 @@ float Node::getInlineEndMargin(
return resolveValue(trailingMargin, widthSize).unwrapOrDefault(0.0f);
}

float Node::getFlexStartBorder(FlexDirection axis) const {
float Node::getInlineStartBorder(FlexDirection axis, Direction direction)
const {
const YGEdge startEdge = getInlineStartEdgeUsingErrata(axis, direction);
YGValue leadingBorder = isRow(axis)
? computeEdgeValueForRow(style_.border(), YGEdgeStart, startEdge)
: computeEdgeValueForColumn(style_.border(), startEdge);

return maxOrDefined(leadingBorder.value, 0.0f);
}

float Node::getFlexStartBorder(FlexDirection axis, Direction direction) const {
const YGEdge leadRelativeFlexItemEdge =
flexStartRelativeEdge(axis, direction);
YGValue leadingBorder = isRow(axis)
? computeEdgeValueForRow(
style_.border(), YGEdgeStart, flexStartEdge(axis))
style_.border(), leadRelativeFlexItemEdge, flexStartEdge(axis))
: computeEdgeValueForColumn(style_.border(), flexStartEdge(axis));

return maxOrDefined(leadingBorder.value, 0.0f);
}

float Node::getFlexEndBorder(FlexDirection axis) const {
float Node::getInlineEndBorder(FlexDirection axis, Direction direction) const {
const YGEdge endEdge = getInlineEndEdgeUsingErrata(axis, direction);
YGValue trailingBorder = isRow(axis)
? computeEdgeValueForRow(style_.border(), YGEdgeEnd, endEdge)
: computeEdgeValueForColumn(style_.border(), endEdge);

return maxOrDefined(trailingBorder.value, 0.0f);
}

float Node::getFlexEndBorder(FlexDirection axis, Direction direction) const {
const YGEdge trailRelativeFlexItemEdge = flexEndRelativeEdge(axis, direction);
YGValue trailingBorder = isRow(axis)
? computeEdgeValueForRow(style_.border(), YGEdgeEnd, flexEndEdge(axis))
? computeEdgeValueForRow(
style_.border(), trailRelativeFlexItemEdge, flexEndEdge(axis))
: computeEdgeValueForColumn(style_.border(), flexEndEdge(axis));

return maxOrDefined(trailingBorder.value, 0.0f);
Expand All @@ -191,14 +214,35 @@ float Node::getFlexEndPadding(FlexDirection axis, float widthSize) const {
return maxOrDefined(resolveValue(trailingPadding, widthSize).unwrap(), 0.0f);
}

float Node::getFlexStartPaddingAndBorder(FlexDirection axis, float widthSize)
const {
return getFlexStartPadding(axis, widthSize) + getFlexStartBorder(axis);
float Node::getInlineStartPaddingAndBorder(
FlexDirection axis,
Direction direction,
float widthSize) const {
return getFlexStartPadding(axis, widthSize) +
getInlineStartBorder(axis, direction);
}

float Node::getFlexEndPaddingAndBorder(FlexDirection axis, float widthSize)
const {
return getFlexEndPadding(axis, widthSize) + getFlexEndBorder(axis);
float Node::getFlexStartPaddingAndBorder(
FlexDirection axis,
Direction direction,
float widthSize) const {
return getFlexStartPadding(axis, widthSize) +
getFlexStartBorder(axis, direction);
}

float Node::getInlineEndPaddingAndBorder(
FlexDirection axis,
Direction direction,
float widthSize) const {
return getFlexEndPadding(axis, widthSize) +
getInlineEndBorder(axis, direction);
}

float Node::getFlexEndPaddingAndBorder(
FlexDirection axis,
Direction direction,
float widthSize) const {
return getFlexEndPadding(axis, widthSize) + getFlexEndBorder(axis, direction);
}

float Node::getMarginForAxis(FlexDirection axis, float widthSize) const {
Expand Down
28 changes: 24 additions & 4 deletions packages/react-native/ReactCommon/yoga/yoga/node/Node.h
Original file line number Diff line number Diff line change
Expand Up @@ -208,12 +208,32 @@ class YG_EXPORT Node : public ::YGNode {
FlexDirection axis,
Direction direction,
float widthSize) const;
float getFlexStartBorder(FlexDirection flexDirection) const;
float getFlexEndBorder(FlexDirection flexDirection) const;
float getFlexStartBorder(FlexDirection flexDirection, Direction direction)
const;
float getInlineStartBorder(FlexDirection flexDirection, Direction direction)
const;
float getFlexEndBorder(FlexDirection flexDirection, Direction direction)
const;
float getInlineEndBorder(FlexDirection flexDirection, Direction direction)
const;
float getFlexStartPadding(FlexDirection axis, float widthSize) const;
float getFlexEndPadding(FlexDirection axis, float widthSize) const;
float getFlexStartPaddingAndBorder(FlexDirection axis, float widthSize) const;
float getFlexEndPaddingAndBorder(FlexDirection axis, float widthSize) const;
float getFlexStartPaddingAndBorder(
FlexDirection axis,
Direction direction,
float widthSize) const;
float getInlineStartPaddingAndBorder(
FlexDirection axis,
Direction direction,
float widthSize) const;
float getFlexEndPaddingAndBorder(
FlexDirection axis,
Direction direction,
float widthSize) const;
float getInlineEndPaddingAndBorder(
FlexDirection axis,
Direction direction,
float widthSize) const;
float getMarginForAxis(FlexDirection axis, float widthSize) const;
float getGapForAxis(FlexDirection axis) const;
// Setters
Expand Down

0 comments on commit 1f4ff62

Please sign in to comment.