From 26beed28c25c3bee080b8abe618c7c9c733a708b Mon Sep 17 00:00:00 2001 From: Geoffrey Soubrier Date: Wed, 26 Aug 2026 09:09:19 +0200 Subject: [PATCH] subtract rounded edges in pixel space when rounding layout results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: **problem:** `roundLayoutResultsToPixelGrid()` derives a node's width and height as the difference of two independently rounded absolute edges. `roundValueToPixelGrid()` divides the rounded value back by `pointScaleFactor` and narrows it to `float` before returning, so the subtraction operates on two values that each already carry a representation error, and that error leaks into the resulting dimension. It only surfaces when `pointScaleFactor` makes the division inexact — `n/2` is dyadic and always exactly representable, `n/3` almost never is — and when the absolute coordinates are large enough for one float ULP to be significant. Sweeping 1001 grid-aligned offsets around 3800pt with `pointScaleFactor` 3, a node measured at exactly 288.0 is committed 287.999755859375 at 192 of them, one ULP short at that magnitude. This defeats a guarantee the function makes deliberately, and which its own comment states: "If a node has a custom measure function we never want to round down its size as this could lead to unwanted text truncation." Each edge is rounded in the intended direction; the subtraction of the two narrowed operands is not. The consequence is visible in React Native. A multi-line `` far down a long list commits a height a fraction of a point below what it measured. iOS TextKit applies a strict "does this line fit in the remaining height" test, so that shortfall makes it drop the entire trailing line, rendering visually truncated text with blank space left in its place while `onLayout` and `onTextLayout` both report the complete measurement. It reproduces on 3x screens only, tracks scroll position rather than content, and survives remounting — all consistent with the arithmetic. Measured on a minimal repro (iPhone 17 simulator, 20 fresh launches per arm, same instrumentation, only this change differing): 19 truncated renders before, 0 after, with the committed height going from 287.999755859375 to exactly 288.000000000000. **fix:** round in pixel space, where a grid-aligned value is an exact integer, so the subtraction is exact and a single narrowing happens at the end. `roundValueToPixelGridScaled()` returns the rounded value still scaled by `pointScaleFactor`, and `pixelGridValueToPoints()` converts back once. The public `roundValueToPixelGrid()` keeps its exact signature and behaviour as a thin wrapper, so `YGRoundValueToPixelGrid()` and the `Cache.cpp` callers are unaffected. regression: `YGRoundingMeasureFuncTest.rounding_measured_size_is_never_rounded_down_at_large_offsets` sweeps the offsets described above and asserts the committed height is never below the measured one. It fails on main and passes with this change. Note the exact comparison: `ASSERT_FLOAT_EQ` tolerates 4 ULPs and would not catch a 1-ULP shortfall. The full suite passes (843/843). ## Changelog: [General] [Fixed] - Layout dimensions are no longer rounded below their measured size at large offsets on non-integer pixel scales --- tests/YGRoundingMeasureFuncTest.cpp | 68 +++++++++++++++++++++++++ yoga/algorithm/PixelGrid.cpp | 79 +++++++++++++++++++++-------- yoga/algorithm/PixelGrid.h | 11 ++++ 3 files changed, 138 insertions(+), 20 deletions(-) diff --git a/tests/YGRoundingMeasureFuncTest.cpp b/tests/YGRoundingMeasureFuncTest.cpp index c654a280ec..140a6a16b0 100644 --- a/tests/YGRoundingMeasureFuncTest.cpp +++ b/tests/YGRoundingMeasureFuncTest.cpp @@ -8,6 +8,9 @@ #include #include +#include +#include + static YGSize _measureFloor( YGNodeConstRef /*node*/, float width, @@ -137,3 +140,68 @@ TEST( YGConfigFree(config); } +static YGSize _measureExactMultipleOfLineHeight( + YGNodeConstRef /*node*/, + float /*width*/, + YGMeasureMode /*widthMode*/, + float /*height*/, + YGMeasureMode /*heightMode*/) { + // 12 lines of 24pt: a height that is exactly representable and exactly on the pixel grid at any + // scale factor, so any shortfall in the committed height comes from rounding, not from the input. + return YGSize{ + 300.0f, + 288.0f, + }; +} + +// A node with a measure function must never be committed a size smaller than it measured — the +// rounding code calls this out explicitly ("we never want to round down its size as this could lead +// to unwanted text truncation"), and forces ceil/floor on the node's edges to guarantee it. +// +// That guarantee used to be defeated by how the dimension was derived. Rounding each edge back to +// points narrows it to float, and the dimension is the difference of two such edges, so each +// operand's representation error leaked into the result. It only surfaced when pointScaleFactor made +// the conversion inexact (n/2 is dyadic and always exact, n/3 almost never is) and when the absolute +// coordinates were large enough for one float ULP to matter. A node measured at exactly 288.0 could +// then be committed 287.999755859375 — enough for a platform text engine applying a strict "does this +// line still fit" test to silently drop an entire trailing line. +// +// Note the exact comparison: ASSERT_FLOAT_EQ tolerates 4 ULPs and would not catch a 1-ULP shortfall. +TEST(YogaTest, rounding_measured_size_is_never_rounded_down_at_large_offsets) { + const float pointScaleFactor = 3.0f; + const float measuredHeight = 288.0f; + + // Sweep pixel-grid-aligned offsets through a range where one float ULP is significant. + for (int scaledOffset = 11000; scaledOffset <= 12000; scaledOffset++) { + const float offset = static_cast(scaledOffset) / pointScaleFactor; + + YGConfigRef config = YGConfigNew(); + YGConfigSetPointScaleFactor(config, pointScaleFactor); + + YGNodeRef root = YGNodeNewWithConfig(config); + YGNodeStyleSetWidth(root, 400.0f); + YGNodeStyleSetHeight(root, offset + 1000.0f); + + YGNodeRef root_child0 = YGNodeNewWithConfig(config); + YGNodeStyleSetWidth(root_child0, 400.0f); + YGNodeStyleSetHeight(root_child0, offset); + YGNodeInsertChild(root, root_child0, 0); + + YGNodeRef root_child1 = YGNodeNewWithConfig(config); + YGNodeSetMeasureFunc(root_child1, _measureExactMultipleOfLineHeight); + YGNodeInsertChild(root, root_child1, 1); + + YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); + + // Printed at full float precision: the shortfall is a single ULP, so the default formatting + // would render both values as "288" and hide the difference. + const float committedHeight = YGNodeLayoutGetHeight(root_child1); + ASSERT_EQ(measuredHeight, committedHeight) + << std::setprecision(std::numeric_limits::max_digits10) + << "measured height " << measuredHeight << " was committed as " << committedHeight + << " at offset " << offset; + + YGNodeFreeRecursive(root); + YGConfigFree(config); + } +} diff --git a/yoga/algorithm/PixelGrid.cpp b/yoga/algorithm/PixelGrid.cpp index 61de2be2e8..dac169af75 100644 --- a/yoga/algorithm/PixelGrid.cpp +++ b/yoga/algorithm/PixelGrid.cpp @@ -12,7 +12,18 @@ namespace facebook::yoga { -float roundValueToPixelGrid( +// Rounds `value` to the pixel grid and returns the result *in pixel space* (i.e. still multiplied by +// `pointScaleFactor`), where a grid-aligned value is always an exact integer. +// +// Callers that need a difference of two rounded values must subtract in this space rather than convert each +// operand back to points first: `scaledValue / pointScaleFactor` is generally not representable (for a 3x +// screen it almost never is), and narrowing each operand to `float` before subtracting leaks that +// representation error into the result. The error grows with the magnitude of the operands, so for a node far +// down a long scrolling list it becomes large enough to matter — a height of exactly 288.0 points can come +// back as 287.999755859375, which is enough for a text node to lose an entire trailing line when the platform +// text engine checks whether the last line still fits. Subtracting two exact integers first, and narrowing +// once at the end, keeps the returned dimension exactly grid-aligned. +double roundValueToPixelGridScaled( const double value, const double pointScaleFactor, const bool forceCeil, @@ -57,6 +68,25 @@ float roundValueToPixelGrid( ? 1.0 : 0.0); } + return scaledValue; +} + +float roundValueToPixelGrid( + const double value, + const double pointScaleFactor, + const bool forceCeil, + const bool forceFloor) { + const double scaledValue = + roundValueToPixelGridScaled(value, pointScaleFactor, forceCeil, forceFloor); + return (std::isnan(scaledValue) || std::isnan(pointScaleFactor)) + ? YGUndefined + : (float)(scaledValue / pointScaleFactor); +} + +// Converts a pixel-space value produced by `roundValueToPixelGridScaled()` back to points. +static float pixelGridValueToPoints( + const double scaledValue, + const double pointScaleFactor) { return (std::isnan(scaledValue) || std::isnan(pointScaleFactor)) ? YGUndefined : (float)(scaledValue / pointScaleFactor); @@ -86,13 +116,15 @@ void roundLayoutResultsToPixelGrid( // size as this could lead to unwanted text truncation. const bool textRounding = node->getNodeType() == NodeType::Text; - node->setLayoutPosition( - roundValueToPixelGrid(nodeLeft, pointScaleFactor, false, textRounding), - PhysicalEdge::Left); + const double scaledLeft = + roundValueToPixelGridScaled(nodeLeft, pointScaleFactor, false, textRounding); + const double scaledTop = + roundValueToPixelGridScaled(nodeTop, pointScaleFactor, false, textRounding); node->setLayoutPosition( - roundValueToPixelGrid(nodeTop, pointScaleFactor, false, textRounding), - PhysicalEdge::Top); + pixelGridValueToPoints(scaledLeft, pointScaleFactor), PhysicalEdge::Left); + node->setLayoutPosition( + pixelGridValueToPoints(scaledTop, pointScaleFactor), PhysicalEdge::Top); // We multiply dimension by scale factor and if the result is close to the // whole number, we don't have any fraction To verify if the result is close @@ -106,25 +138,32 @@ void roundLayoutResultsToPixelGrid( const bool hasFractionalHeight = !yoga::inexactEquals(round(scaledNodeHeight), scaledNodeHeight); + // The dimensions are derived as the difference of two rounded absolute edges. Both operands are exact + // integers in pixel space, so subtracting there and narrowing once yields an exactly grid-aligned + // dimension; converting each edge back to points first and subtracting in `float` would not. + const double scaledAbsoluteLeft = + roundValueToPixelGridScaled(absoluteNodeLeft, pointScaleFactor, false, textRounding); + const double scaledAbsoluteRight = roundValueToPixelGridScaled( + absoluteNodeRight, + pointScaleFactor, + (textRounding && hasFractionalWidth), + (textRounding && !hasFractionalWidth)); + + const double scaledAbsoluteTop = + roundValueToPixelGridScaled(absoluteNodeTop, pointScaleFactor, false, textRounding); + const double scaledAbsoluteBottom = roundValueToPixelGridScaled( + absoluteNodeBottom, + pointScaleFactor, + (textRounding && hasFractionalHeight), + (textRounding && !hasFractionalHeight)); + node->getLayout().setDimension( Dimension::Width, - roundValueToPixelGrid( - absoluteNodeRight, - pointScaleFactor, - (textRounding && hasFractionalWidth), - (textRounding && !hasFractionalWidth)) - - roundValueToPixelGrid( - absoluteNodeLeft, pointScaleFactor, false, textRounding)); + pixelGridValueToPoints(scaledAbsoluteRight - scaledAbsoluteLeft, pointScaleFactor)); node->getLayout().setDimension( Dimension::Height, - roundValueToPixelGrid( - absoluteNodeBottom, - pointScaleFactor, - (textRounding && hasFractionalHeight), - (textRounding && !hasFractionalHeight)) - - roundValueToPixelGrid( - absoluteNodeTop, pointScaleFactor, false, textRounding)); + pixelGridValueToPoints(scaledAbsoluteBottom - scaledAbsoluteTop, pointScaleFactor)); } for (yoga::Node* child : node->getChildren()) { diff --git a/yoga/algorithm/PixelGrid.h b/yoga/algorithm/PixelGrid.h index f3bf79f98a..ecc23480fb 100644 --- a/yoga/algorithm/PixelGrid.h +++ b/yoga/algorithm/PixelGrid.h @@ -12,6 +12,17 @@ namespace facebook::yoga { +// Round a point value to the nearest physical pixel based on DPI +// (pointScaleFactor), returning the result in pixel space (still scaled by +// pointScaleFactor), where a grid-aligned value is an exact integer. Use this, +// rather than subtracting two `roundValueToPixelGrid()` results, whenever a +// difference of two rounded values is needed. +double roundValueToPixelGridScaled( + double value, + double pointScaleFactor, + bool forceCeil, + bool forceFloor); + // Round a point value to the nearest physical pixel based on DPI // (pointScaleFactor) float roundValueToPixelGrid(