From 701290d638478a176858724222f5d01d0c133c19 Mon Sep 17 00:00:00 2001 From: Fabrizio Cucci Date: Mon, 27 Jul 2026 03:43:14 -0700 Subject: [PATCH] fix(ios): prevent pixel-grid rounding from clipping text Summary: Re-lands the iOS text measurement rounding fix from PR #54260 / D85438723, which was reverted by D86791267. This version adds unit test coverage and moves the rounding into a helper that can be tested. On iOS, text measurement is rounded up to the device pixel grid. Converting the measured size from double to float and then rounding it to the pixel grid in Yoga can lose precision for a value that sits on a pixel boundary. That value can round down instead of up and clip the final line (height) or the trailing glyph (width). Fixes #53450. The fix adds a small epsilon before ceil on both width and height. This matches the approach the legacy architecture has used since D7074168 (a534672e). The rounding now lives in a helper, internal_roundTextMeasurementToPixelGrid, so it can be covered by unit tests. The helper keeps the internal_ prefix so it stays out of the public C++ API snapshot. On the earlier revert: D85438723 was backed out after wildeMarketplaceScreenshot-e2e.js failed. Running that test on an OnDemand shows what actually happens. It passes on trunk. With this change the ad renders one physical pixel taller (height 44px to 45px, width unchanged). The top 44 rows are byte-for-byte identical to the baseline and the extra row is blank, so the content renders correctly and the failure is only the pixel-exact size check rejecting a 1px-different image. This is expected for the epsilon and is resolved by re-capturing the baseline, which is what D7074168 did in its own commit. The test is also independently flaky (test infra has flagged it, and it renders autoplaying video and carousels), which is a separate reason its earlier auto-rebaseline looked wrong. Rendering verification (wildeMarketplaceScreenshot, config browse_dynamic_image_layer_frame_overlay_high_res_sr_video), captured on an OnDemand. This is the element-scoped capture the test compares (the mp_feed_ad_item ad cell), not a full screen. The two are identical except for one blank pixel row at the bottom. | Before (trunk, 274x44) | After (this change, 274x45) | | --- | --- | | {F1993025868} | {F1993025869} | Changelog: [iOS] [Fixed] - Prevent the final line or trailing glyph of text from being clipped due to pixel-grid rounding precision loss (#53450) Reviewed By: cipolleschi Differential Revision: D113691958 --- .../TextMeasurementRounding.h | 43 +++++++++++++++++ .../textlayoutmanager/RCTTextLayoutManager.mm | 8 +++- .../tests/TextLayoutManagerTest.cpp | 47 +++++++++++++++++++ .../js/examples/Text/TextSharedExamples.js | 29 ++++++++++++ 4 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 packages/react-native/ReactCommon/react/renderer/textlayoutmanager/TextMeasurementRounding.h diff --git a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/TextMeasurementRounding.h b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/TextMeasurementRounding.h new file mode 100644 index 000000000000..fcf158fdf16e --- /dev/null +++ b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/TextMeasurementRounding.h @@ -0,0 +1,43 @@ +/* + * 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. + */ + +#pragma once + +#include + +#include + +namespace facebook::react { + +/* + * Rounds a raw text measurement up to the device pixel grid. + * + * A small epsilon is added to each dimension before ceil, so a value that sits + * effectively on a physical-pixel boundary gains one extra physical pixel + * instead of being rounded down. This works around precision loss when a + * measurement is converted from double to float and then rounded to the pixel + * grid in Yoga: without it, such a value can round down and clip the final line + * (height) or trailing glyph (width) of text. Dimensions that already have + * sub-pixel headroom are unaffected. See facebook/react-native issue #53450; + * this mirrors the legacy architecture fix in D7074168, which applied the same + * epsilon to both dimensions. + * + * This helper lives in its own header only so the unit test in + * `textlayoutmanager:tests` can exercise it directly. In production it is used + * only by RCTTextLayoutManager.mm, and the `internal_` prefix keeps it out of + * the public C++ API snapshot. + */ +inline Size internal_roundTextMeasurementToPixelGrid(Size size, Float pointScaleFactor) +{ + constexpr auto kEpsilon = static_cast(0.001); + return Size{ + .width = std::ceil((size.width + kEpsilon) * pointScaleFactor) / pointScaleFactor, + .height = std::ceil((size.height + kEpsilon) * pointScaleFactor) / pointScaleFactor, + }; +} + +} // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTTextLayoutManager.mm b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTTextLayoutManager.mm index 1dfb60edb9b5..ac9b63e1e5e7 100644 --- a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTTextLayoutManager.mm +++ b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTTextLayoutManager.mm @@ -14,6 +14,7 @@ #import #import #import +#import #import #import @@ -581,8 +582,11 @@ - (TextMeasurement)_measureTextStorage:(NSTextStorage *)textStorage size.height = enumeratedLinesHeight; } - size = (CGSize){ceil(size.width * layoutContext.pointScaleFactor) / layoutContext.pointScaleFactor, - ceil(size.height * layoutContext.pointScaleFactor) / layoutContext.pointScaleFactor}; + facebook::react::Size roundedSize = facebook::react::internal_roundTextMeasurementToPixelGrid( + {.width = static_cast(size.width), + .height = static_cast(size.height)}, + layoutContext.pointScaleFactor); + size = CGSize{roundedSize.width, roundedSize.height}; NSRange visibleGlyphRange = [layoutManager glyphRangeForTextContainer:textContainer]; diff --git a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/tests/TextLayoutManagerTest.cpp b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/tests/TextLayoutManagerTest.cpp index bd0a053f64bc..d19545a83f67 100644 --- a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/tests/TextLayoutManagerTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/tests/TextLayoutManagerTest.cpp @@ -7,7 +7,10 @@ #include +#include + #include +#include using namespace facebook::react; @@ -97,3 +100,47 @@ TEST(TextLayoutManagerTest, pointScaleFactorAffectsPreparedTextCacheHash) { std::hash{}(lhs), std::hash{}(rhs)); } + +// Tests for internal_roundTextMeasurementToPixelGrid (the pixel-grid rounding +// used by text measurement). A small epsilon is added before ceil so a +// dimension that lands exactly on a pixel boundary gains one physical pixel of +// slack (avoiding double->float->Yoga precision from clipping the last line or +// trailing glyph), while dimensions with sub-pixel headroom are left untouched. +// pointScaleFactor 3 (an @3x screen) is used so one physical pixel is 1/3 pt. +// These lock in the behavior of D7074168 / PR #54260 in the new architecture, +// with the unit coverage that change lacked. +namespace { +constexpr Float kScale = 3.0; +long heightInPixels(Size raw) { + return std::lround( + internal_roundTextMeasurementToPixelGrid(raw, kScale).height * kScale); +} +long widthInPixels(Size raw) { + return std::lround( + internal_roundTextMeasurementToPixelGrid(raw, kScale).width * kScale); +} +} // namespace + +// Height exactly on a pixel boundary gains one extra physical pixel (60 -> 61) +// so the final line is not clipped after layout rounding. +TEST(TextMeasurementRoundingTest, addsSlackWhenHeightOnPixelBoundary) { + EXPECT_EQ(heightInPixels(Size{100.0, 20.0}), 61); +} + +// Width exactly on a pixel boundary likewise gains one extra physical pixel so +// the trailing glyph is not clipped (matches the legacy both-dimensions fix). +TEST(TextMeasurementRoundingTest, addsSlackWhenWidthOnPixelBoundary) { + EXPECT_EQ(widthInPixels(Size{20.0, 100.0}), 61); +} + +// Height with sub-pixel headroom must NOT be inflated: 20.1pt -> 60.3px -> +// 61px, never 62. The epsilon only nudges boundary values, so text is not made +// taller than needed. +TEST(TextMeasurementRoundingTest, doesNotInflateHeightWithSubpixelHeadroom) { + EXPECT_EQ(heightInPixels(Size{100.0, 20.1}), 61); +} + +// Width with sub-pixel headroom must NOT be inflated either: 20.1pt -> 61px. +TEST(TextMeasurementRoundingTest, doesNotInflateWidthWithSubpixelHeadroom) { + EXPECT_EQ(widthInPixels(Size{20.1, 100.0}), 61); +} diff --git a/packages/rn-tester/js/examples/Text/TextSharedExamples.js b/packages/rn-tester/js/examples/Text/TextSharedExamples.js index 3be718720a46..9690eff87b5d 100644 --- a/packages/rn-tester/js/examples/Text/TextSharedExamples.js +++ b/packages/rn-tester/js/examples/Text/TextSharedExamples.js @@ -242,6 +242,27 @@ const styles = StyleSheet.create({ }, }); +function LastLineClippingExample(): React.Node { + // Wrapped paragraphs at assorted font sizes are prone to the final line being + // clipped by one physical pixel after layout rounding (issue #53450). The + // border makes any clipping of the last line visible. + return ( + + {[11, 12, 13, 14, 15, 16, 17].map(fontSize => ( + + + {`(${fontSize}px) This sentence wraps across multiple lines so the ` + + 'final line sits near a pixel boundary and must render fully, ' + + 'without being cut off.'} + + + ))} + + ); +} + export default [ { title: 'Empty Text', @@ -277,4 +298,12 @@ export default [ description: 'Shows the a11y behavior of Text with role="link"', render: TextWithLinkRoleExample, }, + { + title: 'Wrapped text last-line clipping', + name: 'wrappedLastLineClipping', + description: + 'The final line of wrapped text must render fully, not clipped by one ' + + 'physical pixel after layout rounding (issue #53450).', + render: LastLineClippingExample, + }, ] as ReadonlyArray;