Skip to content

Commit

Permalink
Fix bug where absolute nodes were not insetted correctly in certain c…
Browse files Browse the repository at this point in the history 
…ases (#43417)

Summary:
X-link: facebook/yoga#1593

Pull Request resolved: #43417

There was a bug where we did not position absolute nodes correctly if the static node had a different main/cross axis from the containing node. This fixes that. The change is somewhat complicated unfortunately but I tried to add sufficient comments to explain what is happening

Reviewed By: NickGerleman

Differential Revision: D54703955

fbshipit-source-id: 096c643f61d4f9bb3ee6278d675ebd69b57350d7
  • Loading branch information
@joevilches @huntie
joevilches authored and huntie committed Mar 18, 2024
1 parent bc745cb commit 8830ed6
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 60 deletions.
104 changes: 64 additions & 40 deletions packages/react-native/ReactCommon/yoga/yoga/algorithm/AbsoluteLayout.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -480,14 +480,10 @@ void layoutAbsoluteDescendants(
LayoutData& layoutMarkerData,
uint32_t currentDepth,
uint32_t generationCount,
float currentNodeMainOffsetFromContainingBlock,
float currentNodeCrossOffsetFromContainingBlock,
float currentNodeLeftOffsetFromContainingBlock,
float currentNodeTopOffsetFromContainingBlock,
float containingNodeAvailableInnerWidth,
float containingNodeAvailableInnerHeight) {
const FlexDirection mainAxis = resolveDirection(
currentNode->style().flexDirection(), currentNodeDirection);
const FlexDirection crossAxis =
resolveCrossDirection(mainAxis, currentNodeDirection);
for (auto child : currentNode->getChildren()) {
if (child->style().display() == Display::None) {
continue;
Expand Down Expand Up @@ -516,45 +512,73 @@ void layoutAbsoluteDescendants(
currentDepth,
generationCount);

const bool isMainAxisRow = isRow(mainAxis);
const bool mainInsetsDefined = isMainAxisRow
? child->style().horizontalInsetsDefined()
: child->style().verticalInsetsDefined();
const bool crossInsetsDefined = isMainAxisRow
? child->style().verticalInsetsDefined()
: child->style().horizontalInsetsDefined();

const float childMainOffsetFromParent = mainInsetsDefined
? (child->getLayout().position(flexStartEdge(mainAxis)) -
currentNodeMainOffsetFromContainingBlock)
: child->getLayout().position(flexStartEdge(mainAxis));
const float childCrossOffsetFromParent = crossInsetsDefined
? (child->getLayout().position(flexStartEdge(crossAxis)) -
currentNodeCrossOffsetFromContainingBlock)
: child->getLayout().position(flexStartEdge(crossAxis));

child->setLayoutPosition(
childMainOffsetFromParent, flexStartEdge(mainAxis));
child->setLayoutPosition(
childCrossOffsetFromParent, flexStartEdge(crossAxis));

if (needsTrailingPosition(mainAxis)) {
setChildTrailingPosition(currentNode, child, mainAxis);
/*
* At this point the child has its position set but only on its the
* parent's flexStart edge. Additionally, this position should be
* interpreted relative to the containing block of the child if it had
* insets defined. So we need to adjust the position by subtracting the
* the parents offset from the containing block. However, getting that
* offset is complicated since the two nodes can have different main/cross
* axes.
*/
const FlexDirection parentMainAxis = resolveDirection(
currentNode->style().flexDirection(), currentNodeDirection);
const FlexDirection parentCrossAxis =
resolveCrossDirection(parentMainAxis, currentNodeDirection);

if (needsTrailingPosition(parentMainAxis)) {
const bool mainInsetsDefined = isRow(parentMainAxis)
? child->style().horizontalInsetsDefined()
: child->style().verticalInsetsDefined();
setChildTrailingPosition(
mainInsetsDefined ? containingNode : currentNode,
child,
parentMainAxis);
}
if (needsTrailingPosition(crossAxis)) {
setChildTrailingPosition(currentNode, child, crossAxis);
if (needsTrailingPosition(parentCrossAxis)) {
const bool crossInsetsDefined = isRow(parentCrossAxis)
? child->style().horizontalInsetsDefined()
: child->style().verticalInsetsDefined();
setChildTrailingPosition(
crossInsetsDefined ? containingNode : currentNode,
child,
parentCrossAxis);
}

/*
* At this point we know the left and top physical edges of the child are
* set with positions that are relative to the containing block if insets
* are defined
*/
const float childLeftPosition =
child->getLayout().position(PhysicalEdge::Left);
const float childTopPosition =
child->getLayout().position(PhysicalEdge::Top);

const float childLeftOffsetFromParent =
child->style().horizontalInsetsDefined()
? (childLeftPosition - currentNodeLeftOffsetFromContainingBlock)
: childLeftPosition;
const float childTopOffsetFromParent =
child->style().verticalInsetsDefined()
? (childTopPosition - currentNodeTopOffsetFromContainingBlock)
: childTopPosition;

child->setLayoutPosition(childLeftOffsetFromParent, PhysicalEdge::Left);
child->setLayoutPosition(childTopOffsetFromParent, PhysicalEdge::Top);
} else if (
child->style().positionType() == PositionType::Static &&
!child->alwaysFormsContainingBlock()) {
const Direction childDirection =
child->resolveDirection(currentNodeDirection);
const float childMainOffsetFromContainingBlock =
currentNodeMainOffsetFromContainingBlock +
child->getLayout().position(flexStartEdge(mainAxis));
const float childCrossOffsetFromContainingBlock =
currentNodeCrossOffsetFromContainingBlock +
child->getLayout().position(flexStartEdge(crossAxis));
// By now all descendants of the containing block that are not absolute
// will have their positions set for left and top.
const float childLeftOffsetFromContainingBlock =
currentNodeLeftOffsetFromContainingBlock +
child->getLayout().position(PhysicalEdge::Left);
const float childTopOffsetFromContainingBlock =
currentNodeTopOffsetFromContainingBlock +
child->getLayout().position(PhysicalEdge::Top);

layoutAbsoluteDescendants(
containingNode,
Expand All @@ -564,8 +588,8 @@ void layoutAbsoluteDescendants(
layoutMarkerData,
currentDepth + 1,
generationCount,
childMainOffsetFromContainingBlock,
childCrossOffsetFromContainingBlock,
childLeftOffsetFromContainingBlock,
childTopOffsetFromContainingBlock,
containingNodeAvailableInnerWidth,
containingNodeAvailableInnerHeight);
}
Expand Down
39 changes: 19 additions & 20 deletions packages/react-native/ReactCommon/yoga/yoga/algorithm/CalculateLayout.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2045,26 +2045,7 @@ static void calculateLayoutImpl(
}

if (performLayout) {
// STEP 10: SIZING AND POSITIONING ABSOLUTE CHILDREN
// Let the containing block layout its absolute descendants. By definition
// the containing block will not be static unless we are at the root.
if (node->style().positionType() != PositionType::Static ||
node->alwaysFormsContainingBlock() || depth == 1) {
layoutAbsoluteDescendants(
node,
node,
isMainAxisRow ? sizingModeMainDim : sizingModeCrossDim,
direction,
layoutMarkerData,
depth,
generationCount,
0.0f,
0.0f,
availableInnerWidth,
availableInnerHeight);
}

// STEP 11: SETTING TRAILING POSITIONS FOR CHILDREN
// STEP 10: SETTING TRAILING POSITIONS FOR CHILDREN
const bool needsMainTrailingPos = needsTrailingPosition(mainAxis);
const bool needsCrossTrailingPos = needsTrailingPosition(crossAxis);

Expand All @@ -2087,6 +2068,24 @@ static void calculateLayoutImpl(
}
}
}

// STEP 11: SIZING AND POSITIONING ABSOLUTE CHILDREN
// Let the containing block layout its absolute descendants.
if (node->style().positionType() != PositionType::Static ||
node->alwaysFormsContainingBlock() || depth == 1) {
layoutAbsoluteDescendants(
node,
node,
isMainAxisRow ? sizingModeMainDim : sizingModeCrossDim,
direction,
layoutMarkerData,
depth,
generationCount,
0.0f,
0.0f,
availableInnerWidth,
availableInnerHeight);
}
}
}

Expand Down

0 comments on commit 8830ed6

Please sign in to comment.