From 3e6aa70893c069aa44b6a267dc4f19ce29faa58f Mon Sep 17 00:00:00 2001 From: Josh Watzman Date: Thu, 3 Sep 2026 15:03:43 +0100 Subject: [PATCH] Don't break a flex line on floating-point rounding noise A content-sized wrapping row sizes itself to the max-content sum of its items, then recovers the space available to those items by subtracting its padding and border back off (calculateAvailableInnerDimension). In float32 that add-then-subtract round-trip can land a fraction of an ulp below the sum it came from. For an item with a measure function the measurement cache still returns the max-content measurement in that situation: oldSizeIsMaxContentAndStillFits accepts a cached width up to the inexactEquals epsilon (1e-4) wider than the space now available. calculateFlexLine then compared that basis against availableInnerMainDim with a bare `>`, so the basis the cache had just accepted as fitting read as overflow and the line broke. Because the container had already been sized for a single line, the result was a box one line tall with its items laid out on two, overflowing its own padding and border. In React Native this reproduced on both iOS and Android and was extremely sensitive to the measured text width, since only some values leave a residue inside the window. Make the overflow test tolerant of the same epsilon the cache uses, so the two agree. Items without a measure function are unaffected: they take their exact style width in both passes and never enter that cache path. The regression test sweeps fractional measured widths whose content sum sits just under 128 while the outer width sits just over it, so adding padding and border crosses a float32 binade and the subtraction cannot always recover the original value. Co-Authored-By: Claude Opus 5 (1M context) --- tests/YGFlexWrapRoundingTest.cpp | 86 ++++++++++++++++++++++++++++++++ yoga/algorithm/FlexLine.cpp | 21 ++++++-- 2 files changed, 104 insertions(+), 3 deletions(-) create mode 100644 tests/YGFlexWrapRoundingTest.cpp diff --git a/tests/YGFlexWrapRoundingTest.cpp b/tests/YGFlexWrapRoundingTest.cpp new file mode 100644 index 0000000000..4a306f8a63 --- /dev/null +++ b/tests/YGFlexWrapRoundingTest.cpp @@ -0,0 +1,86 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include +#include + +namespace { + +// Reports the fractional width supplied through the node's context, standing in +// for a text measurement whose result is not representable on the pixel grid. +YGSize measureFractionalWidth( + YGNodeConstRef node, + float /*width*/, + YGMeasureMode /*widthMode*/, + float /*height*/, + YGMeasureMode /*heightMode*/) { + const float* measuredWidth = + static_cast(YGNodeGetContext(const_cast(node))); + return YGSize{*measuredWidth, 20.0f}; +} + +} // namespace + +// A content-sized wrapping row sizes itself to the max-content sum of its +// items, then recovers the space available to those items by subtracting its +// padding and border back off. When a measure function contributes a fractional +// width, that float32 round-trip can land a fraction of an ulp below the sum +// it came from, while the measurement cache still returns the max-content +// measurement unchanged because it treats the difference as equal +// (oldSizeIsMaxContentAndStillFits). The line must not break in that case: the +// container has already been sized for a single line, so breaking leaves its +// items laid out on two lines inside a box only tall enough for one. +// +// The widths swept here put the row's content sum just under 128 and its outer +// width just over it, so adding padding and border crosses a float32 binade and +// the subtraction cannot always recover the original value. +TEST(YogaTest, content_sized_wrap_row_does_not_break_line_on_rounding) { + for (int step = 0; step < 100; step++) { + float measuredWidth = 84.0f + static_cast(step) / 100.0f; + SCOPED_TRACE( + "measured width " + std::to_string(measuredWidth) + " (step " + + std::to_string(step) + ")"); + + YGNodeRef root = YGNodeNew(); + YGNodeStyleSetWidth(root, 400.0f); + YGNodeStyleSetHeight(root, 400.0f); + + // Shrink-to-fit in the cross axis, so its width comes from its content. + YGNodeRef wrapper = YGNodeNew(); + YGNodeStyleSetAlignSelf(wrapper, YGAlignFlexStart); + YGNodeStyleSetBorder(wrapper, YGEdgeAll, 1.0f); + YGNodeInsertChild(root, wrapper, 0); + + YGNodeRef row = YGNodeNew(); + YGNodeStyleSetFlexDirection(row, YGFlexDirectionRow); + YGNodeStyleSetFlexWrap(row, YGWrapWrap); + YGNodeStyleSetPadding(row, YGEdgeAll, 16.0f); + YGNodeInsertChild(wrapper, row, 0); + + YGNodeRef measured = YGNodeNew(); + YGNodeSetContext(measured, &measuredWidth); + YGNodeSetMeasureFunc(measured, measureFractionalWidth); + YGNodeInsertChild(row, measured, 0); + + YGNodeRef rigid = YGNodeNew(); + YGNodeStyleSetWidth(rigid, 20.0f); + YGNodeStyleSetHeight(rigid, 20.0f); + YGNodeInsertChild(row, rigid, 1); + + YGNodeCalculateLayout(root, 400.0f, 400.0f, YGDirectionLTR); + + // Both items fit on one line by construction: the row is content-sized and + // the root is far wider than the content needs. + EXPECT_EQ(YGNodeLayoutGetTop(rigid), YGNodeLayoutGetTop(measured)); + + // A single line of 20pt content plus 16pt of padding on each side. If the + // line broke, the items occupy two lines while the box keeps this height. + EXPECT_EQ(52.0f, YGNodeLayoutGetHeight(row)); + + YGNodeFreeRecursive(root); + } +} diff --git a/yoga/algorithm/FlexLine.cpp b/yoga/algorithm/FlexLine.cpp index dc0a300add..ab92b8efc3 100644 --- a/yoga/algorithm/FlexLine.cpp +++ b/yoga/algorithm/FlexLine.cpp @@ -10,6 +10,7 @@ #include #include #include +#include namespace facebook::yoga { @@ -74,12 +75,26 @@ FlexLine calculateFlexLine( ownerWidth) .unwrap(); + const float requiredMainDim = sizeConsumedIncludingMinConstraint + + flexBasisWithMinAndMaxConstraints + childMarginMainAxis + + childLeadingGapMainAxis; + // If this is a multi-line flow and this item pushes us over the available // size, we've hit the end of the current line. Break out of the loop and // lay out the current line. - if (sizeConsumedIncludingMinConstraint + flexBasisWithMinAndMaxConstraints + - childMarginMainAxis + childLeadingGapMainAxis > - availableInnerMainDim && + // + // The overflow test is tolerant of the same epsilon the measurement cache + // uses when it decides a cached measurement "still fits" (see + // oldSizeIsMaxContentAndStillFits in Cache.cpp). A content-sized wrap + // container adds padding and border to the max-content sum to size itself + // and then subtracts them again to recover availableInnerMainDim, and in + // float32 that round-trip can land a few ulps low. The cache accepts a + // basis up to the epsilon wider than the space now available and hands + // back the max-content measurement unchanged; without the same tolerance + // here, that basis reads as overflow and the line breaks, leaving the + // container sized for one line with its items laid out on two. + if (requiredMainDim > availableInnerMainDim && + !yoga::inexactEquals(requiredMainDim, availableInnerMainDim) && isNodeFlexWrap && !itemsInFlow.empty()) { break; }