From 30bf453f7fe4bd356e973a8e4a6667d19664f7a4 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Wed, 17 Apr 2024 11:11:47 -0700 Subject: [PATCH] Fixup margin: auto and justification behavior for overflowed containers (#44069) Summary: X-link: https://github.com/facebook/yoga/pull/1646 Fixes https://github.com/facebook/yoga/issues/978 1. Don't allow auto margin spaces to become a negative length 2. Replicate fallback alignment behavior specified by box-alignment spec that we are using for align-content. Reviewed By: joevilches Differential Revision: D56091577 --- .../ReactCommon/yoga/yoga/algorithm/Align.h | 43 ++++++++++++++ .../yoga/yoga/algorithm/CalculateLayout.cpp | 56 +++++-------------- .../yoga/yoga/algorithm/FlexLine.cpp | 17 ++++-- .../yoga/yoga/algorithm/FlexLine.h | 3 + 4 files changed, 74 insertions(+), 45 deletions(-) diff --git a/packages/react-native/ReactCommon/yoga/yoga/algorithm/Align.h b/packages/react-native/ReactCommon/yoga/yoga/algorithm/Align.h index e9ab6df3b7c2a7..ac4dfcde91e0bd 100644 --- a/packages/react-native/ReactCommon/yoga/yoga/algorithm/Align.h +++ b/packages/react-native/ReactCommon/yoga/yoga/algorithm/Align.h @@ -26,4 +26,47 @@ inline Align resolveChildAlignment( return align; } +/** + * Fallback alignment to use on overflow + * https://www.w3.org/TR/css-align-3/#distribution-values + */ +constexpr Align fallbackAlignment(Align align) { + switch (align) { + // Fallback to flex-start + case Align::SpaceBetween: + case Align::Stretch: + return Align::FlexStart; + + // Fallback to safe center. TODO: This should be aligned to Start + // instead of FlexStart (for row-reverse containers) + case Align::SpaceAround: + case Align::SpaceEvenly: + return Align::FlexStart; + default: + return align; + } +} + +/** + * Fallback alignment to use on overflow + * https://www.w3.org/TR/css-align-3/#distribution-values + */ +constexpr Justify fallbackAlignment(Justify align) { + switch (align) { + // Fallback to flex-start + case Justify::SpaceBetween: + // TODO: Support `justify-content: stretch` + // case Justify::Stretch: + return Justify::FlexStart; + + // Fallback to safe center. TODO: This should be aligned to Start + // instead of FlexStart (for row-reverse containers) + case Justify::SpaceAround: + case Justify::SpaceEvenly: + return Justify::FlexStart; + default: + return align; + } +} + } // namespace facebook::yoga diff --git a/packages/react-native/ReactCommon/yoga/yoga/algorithm/CalculateLayout.cpp b/packages/react-native/ReactCommon/yoga/yoga/algorithm/CalculateLayout.cpp index 62d8a24b769671..0d34129ba01401 100644 --- a/packages/react-native/ReactCommon/yoga/yoga/algorithm/CalculateLayout.cpp +++ b/packages/react-native/ReactCommon/yoga/yoga/algorithm/CalculateLayout.cpp @@ -959,27 +959,16 @@ static void justifyMainAxis( } } - int numberOfAutoMarginsOnCurrentLine = 0; - for (size_t i = startOfLineIndex; i < flexLine.endOfLineIndex; i++) { - auto child = node->getChild(i); - if (child->style().positionType() != PositionType::Absolute) { - if (child->style().flexStartMarginIsAuto(mainAxis, direction)) { - numberOfAutoMarginsOnCurrentLine++; - } - if (child->style().flexEndMarginIsAuto(mainAxis, direction)) { - numberOfAutoMarginsOnCurrentLine++; - } - } - } - // In order to position the elements in the main axis, we have two controls. // The space between the beginning and the first element and the space between // each two elements. float leadingMainDim = 0; float betweenMainDim = gap; - const Justify justifyContent = node->style().justifyContent(); + const Justify justifyContent = flexLine.layout.remainingFreeSpace >= 0 + ? node->style().justifyContent() + : fallbackAlignment(node->style().justifyContent()); - if (numberOfAutoMarginsOnCurrentLine == 0) { + if (flexLine.numberOfAutoMargins == 0) { switch (justifyContent) { case Justify::Center: leadingMainDim = flexLine.layout.remainingFreeSpace / 2; @@ -989,8 +978,7 @@ static void justifyMainAxis( break; case Justify::SpaceBetween: if (flexLine.itemsInFlow.size() > 1) { - betweenMainDim += - yoga::maxOrDefined(flexLine.layout.remainingFreeSpace, 0.0f) / + betweenMainDim += flexLine.layout.remainingFreeSpace / static_cast(flexLine.itemsInFlow.size() - 1); } break; @@ -1043,9 +1031,10 @@ static void justifyMainAxis( // We need to do that only for relative elements. Absolute elements do not // take part in that phase. if (childStyle.positionType() != PositionType::Absolute) { - if (child->style().flexStartMarginIsAuto(mainAxis, direction)) { + if (child->style().flexStartMarginIsAuto(mainAxis, direction) && + flexLine.layout.remainingFreeSpace > 0.0f) { flexLine.layout.mainDim += flexLine.layout.remainingFreeSpace / - static_cast(numberOfAutoMarginsOnCurrentLine); + static_cast(flexLine.numberOfAutoMargins); } if (performLayout) { @@ -1059,9 +1048,10 @@ static void justifyMainAxis( flexLine.layout.mainDim += betweenMainDim; } - if (child->style().flexEndMarginIsAuto(mainAxis, direction)) { + if (child->style().flexEndMarginIsAuto(mainAxis, direction) && + flexLine.layout.remainingFreeSpace > 0.0f) { flexLine.layout.mainDim += flexLine.layout.remainingFreeSpace / - static_cast(numberOfAutoMarginsOnCurrentLine); + static_cast(flexLine.numberOfAutoMargins); } bool canSkipFlex = !performLayout && sizingModeCrossDim == SizingMode::StretchFit; @@ -1749,27 +1739,11 @@ static void calculateLayoutImpl( const float remainingAlignContentDim = innerCrossDim - totalLineCrossDim; - // Apply fallback alignments on overflow - // https://www.w3.org/TR/css-align-3/#distribution-values - const auto appliedAlignContent = - remainingAlignContentDim >= 0 ? node->style().alignContent() : [&]() { - switch (node->style().alignContent()) { - // Fallback to flex-start - case Align::SpaceBetween: - case Align::Stretch: - return Align::FlexStart; - - // Fallback to safe center. TODO: This should be aligned to Start - // instead of FlexStart (for row-reverse containers) - case Align::SpaceAround: - case Align::SpaceEvenly: - return Align::FlexStart; - default: - return node->style().alignContent(); - } - }(); + const auto alignContent = remainingAlignContentDim >= 0 + ? node->style().alignContent() + : fallbackAlignment(node->style().alignContent()); - switch (appliedAlignContent) { + switch (alignContent) { case Align::FlexEnd: currentLead += remainingAlignContentDim; break; diff --git a/packages/react-native/ReactCommon/yoga/yoga/algorithm/FlexLine.cpp b/packages/react-native/ReactCommon/yoga/yoga/algorithm/FlexLine.cpp index 50cd6f0abe1bb5..d3a954caafc7f0 100644 --- a/packages/react-native/ReactCommon/yoga/yoga/algorithm/FlexLine.cpp +++ b/packages/react-native/ReactCommon/yoga/yoga/algorithm/FlexLine.cpp @@ -27,6 +27,7 @@ FlexLine calculateFlexLine( float sizeConsumed = 0.0f; float totalFlexGrowFactors = 0.0f; float totalFlexShrinkScaledFactors = 0.0f; + size_t numberOfAutoMargins = 0; size_t endOfLineIndex = startOfLineIndex; size_t firstElementInLineIndex = startOfLineIndex; @@ -49,6 +50,13 @@ FlexLine calculateFlexLine( continue; } + if (child->style().flexStartMarginIsAuto(mainAxis, ownerDirection)) { + numberOfAutoMargins++; + } + if (child->style().flexEndMarginIsAuto(mainAxis, ownerDirection)) { + numberOfAutoMargins++; + } + const bool isFirstElementInLine = (endOfLineIndex - firstElementInLineIndex) == 0; @@ -102,10 +110,11 @@ FlexLine calculateFlexLine( } return FlexLine{ - std::move(itemsInFlow), - sizeConsumed, - endOfLineIndex, - FlexLineRunningLayout{ + .itemsInFlow = std::move(itemsInFlow), + .sizeConsumed = sizeConsumed, + .endOfLineIndex = endOfLineIndex, + .numberOfAutoMargins = numberOfAutoMargins, + .layout = FlexLineRunningLayout{ totalFlexGrowFactors, totalFlexShrinkScaledFactors, }}; diff --git a/packages/react-native/ReactCommon/yoga/yoga/algorithm/FlexLine.h b/packages/react-native/ReactCommon/yoga/yoga/algorithm/FlexLine.h index 0f888be50738ec..3287cd6cf4dcd8 100644 --- a/packages/react-native/ReactCommon/yoga/yoga/algorithm/FlexLine.h +++ b/packages/react-native/ReactCommon/yoga/yoga/algorithm/FlexLine.h @@ -52,6 +52,9 @@ struct FlexLine { // The index of the first item beyond the current line. const size_t endOfLineIndex{0}; + // Number of edges along the line flow with an auto margin. + const size_t numberOfAutoMargins{0}; + // Layout information about the line computed in steps after line-breaking FlexLineRunningLayout layout{}; };