Repository navigation
Conversation
…the view resizes ReactTextView fits the font size by shrinking the font size spans of its Spannable in place. When the view's size changes, onSizeChanged schedules another fit, but that fit starts from the largest span it finds, which is the already-shrunk size, so the text can shrink further but never grow back. It only grew back when a state update happened to deliver a new Spannable with the original sizes. Keep the size spans the text had before the first fit, and restore them (and the paint's base text size) before fitting again. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Re-land of react#59063 (reverted in ad2f451). A ParagraphShadowNode cloned without new props or children (e.g. by Yoga, because a sibling changed) keeps its attributed string and measured layouts instead of rebuilding them, so unchanged siblings no longer get a state update and a re-layout. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- measureContent looked up a carried layout before getContent checked that the carried content was built for the current font size multiplier and layout direction. Check (and drop) it first. - Don't carry prepared layouts of text with adjustsFontSizeToFit: its layout depends on the constraints, not only on the measured size that findUsableLayout matches on. - Prefer layouts measured for the current revision over carried ones of the same size. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@javache has imported this pull request. If you are a Meta employee, you can view this in D126622505. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Re-land of #59063, which was reverted in ad2f451 because it broke
textAdjustsDynamicLayouton Android. This PR has three commits: the Android fix for that regression, #59063 unchanged, and fixes for the two prepared-layout cache concerns raised in the revert.#59063 (unchanged). A
ParagraphShadowNodecloned without new props or children (e.g. by Yoga, because a sibling changed) keeps its attributed string and measured layouts. Unchanged siblings then get no state update and no re-layout. On a Galaxy A16, changing one of 1000 labels takes 66% less time to pixels than on currentmain(benchmarks at the end).1. Why
textAdjustsDynamicLayoutbroke, and the fix (ReactTextView).ReactTextViewfitsadjustsFontSizeToFittext by shrinking the font size spans of its Spannable in place. When the view grows,onSizeChangedschedules another fit, but that fit starts from the largest span it finds, which is the already-shrunk size. The text can shrink further but never grow back. It only grew back when a state update happened to deliver a new Spannable with the original sizes. #59063 removed the spurious state update the example relied on, so the text stayed small after the container grew.This is also a bug on
maintoday, just one that depends on order: in RNTester, Set Height to 60 → 20 → 40 right after opening the example leaves the text small at 40, because no state update lands on that last resize.With this change, the view keeps the font size spans the text had before the first fit and restores them, along with the paint's base text size, before each new fit.
2. Carried content is validated before its layouts are used.
measureContentlooked up a carried layout beforegetContentchecked that the carried content was built for the current font size multiplier and layout direction. WithenablePreparedTextLayout, a font scale change while the app is running reused layouts measured at the old scale, so text was drawn at the new size into boxes measured at the old one and was cut off. The check now runs first.3. Equal measured sizes don't imply equal layouts.
findUsableLayoutmatches a layout by its measured size. Text withadjustsFontSizeToFitcan measure the same under different constraints with a different font size, so its prepared layouts are no longer carried across revisions. When several layouts of the same size exist, the one measured for the current revision is preferred over carried ones.Changelog:
[GENERAL] [FIXED] - Avoid state updates and re-layout of unchanged
Textsiblings when a sibling changes[ANDROID] [FIXED] -
adjustsFontSizeToFittext grows back when its container growsTest Plan:
./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest --tests "com.facebook.react.views.text.*": 94 tests pass. The newadjustsFontSizeToFitGrowsBackWhenContainerHeightGrowsfails without theReactTextViewchange (font size stays at 4 instead of 40).yarn format-check-java,format-check-kotlinandformat-check-cpppass on the changed files.textAdjustsDynamicLayout, release build on an Android emulator:mainadjustsFontSizeToFittext, mixed LTR/RTL texts, nested spans and a counter. Release builds; Android on a Galaxy A16, iOS on an iPhone Air simulator (iOS 26.5). ✓ = identical to the remount.enablePreparedTextLayoutadjustsFontSizeToFit)directionLTR → RTL → LTR on the parentfixTextLayoutOnPixelDensityChangeonnlohmann/json.hppdoesn't build with this machine's Apple clang, so I'm relying on CI), and a physical iPhone.Benchmarks. Samsung Galaxy A16, RNTester release builds of current
main(c7429b0) and this PR, with a text benchmark screen that is not part of this PR. The two builds alternated round by round; medians of 12 runs (4 rounds × 3 runs after a warm-up run), battery 27.6–27.9 °C.mainenablePreparedTextLayout: time to pixelsenablePreparedTextLayout: JS-thread CPUMounts: time to pixels is unchanged (−2% to +4%, within run-to-run noise). With
enablePreparedTextLayout, mounting 1000 labels uses about 10 ms more JS-thread CPU and about 40 ms more total CPU (+9%) in every round, probably from copying the prepared layouts into clones, without a change in time to pixels.Two earlier rounds were left out: in them both builds produced many extra frames per update, probably because Android Studio's screen mirroring was attached to the phone at the time. This PR was still faster in those rounds (1000 labels: 164 ms vs 2406 ms).
🤖 Generated with Claude Code