Fix Android text mis-measurement when using fontVariationSettings - #58320
Open
jkadamczyk wants to merge 1 commit into
Open
Fix Android text mis-measurement when using fontVariationSettings#58320jkadamczyk wants to merge 1 commit into
jkadamczyk wants to merge 1 commit into
Conversation
…plied to a reused Paint Summary: `fontVariationSettings` (react#57804, react#57805) is applied with `Paint.setFontVariationSettings` after `Paint.setTypeface`. Android's `Paint.setFontVariationSettings` returns early when the requested settings string equals the value the paint already holds, and `Paint.setTypeface` does not clear that stored string. On the legacy variation path (every shipped Android release), the variation instance lives on the typeface, so once `setTypeface` replaces the typeface the axes are gone, but the paint still reports the old settings and the second `setFontVariationSettings` call is a no-op. React Native hits this sequence on every measurement after the first. `TextLayoutManager.scratchPaintWithAttributes` reuses a thread-local `TextPaint` and resets it with `setTypeface(null)`, which leaves the stale settings string in place. Any subsequent `<Text>` with the same `fontVariationSettings` string is then measured with the un-varied typeface, while the `TextView` draws it through its own paint where the axes apply. The same happens inside `StaticLayout`, whose pooled `MeasuredParagraph`/`TextLine` paints copy the settings string via `Paint.set`. `ReactEditText` has the equivalent problem through `TextView.setFontVariationSettings`, which also skips unchanged values. With a variable font whose default instance is lighter than the requested `wght`, Yoga receives regular-width metrics while semibold glyphs are drawn, so `numberOfLines={1}` text is ellipsized ("Get Start...") and wrapped text breaks in different places than it renders. This change clears the paint's settings before re-applying them in `ReactTypefaceUtils.applyFontVariationSettings` and in `ReactEditText.maybeUpdateTypeface`, so the axes are always rebuilt on the current typeface. Paints without previous settings are unaffected. Changelog: [ANDROID] [FIXED] - Fix text measured with the wrong width when `fontVariationSettings` are re-applied to a reused `Paint` Test Plan: - `./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest --tests "*TextLayoutManagerFontWeightAdjustmentTest*" --tests "*ReactTextInputPropertyTest*"`: 8/8 and 27/27 pass. The three new tests cover the reused-paint reset, the fresh-paint path (no spurious clear), and the measurement scratch paint through `TextLayoutManager.updateTextPaint`. Without the production change, `reused paint re-applies unchanged font variation settings to a new typeface` and `scratch text paint re-applies font variation settings on repeated measurement` fail. - `yarn lint-kotlin-check` (`./gradlew ktfmtCheck`): passes. - Manual, Android 17 emulator (API 37), variable Inter font registered via `ReactFontManager.addCustomFont` with entries for weights 100–900, `<Text numberOfLines={1} style={{ fontFamily: "InterVariable", fontWeight: "600", fontVariationSettings: "'wght' 600" }}>`: - Before: the second and later semibold texts are measured as regular and ellipsized; button label rendered as "Get Start...". - After: labels render in full and the measured bounds match the drawn text for `'wght' 600`, `650` and `700`, including repeated identical styles and `TextInput`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Hi @jkadamczyk! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
|
@javache has imported this pull request. If you are a Meta employee, you can view this in D118791592. |
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
1 participant
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:
fontVariationSettings(#57804, #57805) is applied withPaint.setFontVariationSettingsafterPaint.setTypeface. Android'sPaint.setFontVariationSettingsreturns early when the requested settings string equals the value the paint already holds, andPaint.setTypefacedoes not clear that stored string. On the legacy variation path, the variation instance lives on the typeface, so oncesetTypefacereplaces the typeface the axes are gone, but the paint still reports the old settings and the secondsetFontVariationSettingscall is a no-op.With a variable font whose default instance is lighter than the requested
wght, Yoga receives regular-width metrics while semibold glyphs are drawn, sonumberOfLines={1}text is ellipsized ("Get Start...") and wrapped text breaks in different places than it renders.This change clears the paint's settings before re-applying them in
ReactTypefaceUtils.applyFontVariationSettingsand inReactEditText.maybeUpdateTypeface, so the axes are always rebuilt on the current typeface. Paints without previous settings are unaffected.Changelog:
[ANDROID] [FIXED] - Fix text measured with the wrong width when
fontVariationSettingsare re-applied to a reusedPaintTest Plan:
Ran test suite, validated the fix in my patched react-native instance that backports variable fonts support.