From e2ccb35be6c9d1f39e3c1e4b93df8d57ccdcaa07 Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Fri, 30 Oct 2026 03:10:34 -0700 Subject: [PATCH] Avoid redundant font variation resets (#59531) Summary: Only clear matching `fontVariationSettings` before reapplying them when the effective typeface has changed. Changed settings already bypass the platform equality shortcut, and an unchanged typeface does not need to be replaced on `Paint`. Preserve `TextView` typeface bookkeeping and compare its effective typeface after the setter. If the platform rejects unsupported axes, explicitly clear previous settings rather than retaining stale axes. Changelog: [Android][Fixed] - Avoid unnecessary font variation resets while preserving correct text measurement. Differential Revision: D127793810 --- .../react/views/text/ReactTypefaceUtils.kt | 36 ++++- .../react/views/text/TextLayoutManager.kt | 13 +- .../text/internal/span/CustomStyleSpan.kt | 11 +- .../react/views/textinput/ReactEditText.kt | 21 ++- ...xtLayoutManagerFontWeightAdjustmentTest.kt | 123 ++++++++++++++++++ .../textinput/ReactTextInputPropertyTest.kt | 70 ++++++++++ 6 files changed, 254 insertions(+), 20 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTypefaceUtils.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTypefaceUtils.kt index 04e3f06dd521..46f0d778d05a 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTypefaceUtils.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTypefaceUtils.kt @@ -173,17 +173,45 @@ public object ReactTypefaceUtils { } } - internal fun applyFontVariationSettings(paint: Paint, fontVariationSettings: String?) { + internal fun applyTypeface( + paint: Paint, + typeface: Typeface?, + fontVariationSettings: String?, + ) { + val typefaceChanged = paint.typeface !== typeface + if (typefaceChanged) { + paint.typeface = typeface + } + applyFontVariationSettings(paint, fontVariationSettings, typefaceChanged) + } + + internal fun applyFontVariationSettings( + paint: Paint, + fontVariationSettings: String?, + typefaceChanged: Boolean = true, + ) { if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) { return } try { - // Paint skips unchanged settings even after setTypeface, so clear them first. - if (paint.fontVariationSettings != null && fontVariationSettings != null) { + val previousSettings = paint.fontVariationSettings + // Paint retains the settings string when its typeface changes. + val needsReset = + typefaceChanged && + fontVariationSettings != null && + previousSettings == fontVariationSettings + if (needsReset) { + paint.setFontVariationSettings(null) + } + if ( + !paint.setFontVariationSettings(fontVariationSettings) && + previousSettings != null && + !needsReset + ) { + // Unsupported axes leave the previous settings applied. paint.setFontVariationSettings(null) } - paint.setFontVariationSettings(fontVariationSettings) } catch (exception: IllegalArgumentException) { // Paint instances are reused, so explicitly clear axes from a previous layout. paint.setFontVariationSettings(null) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.kt index d906e9d78e6b..9254910f6cd8 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.kt @@ -1091,6 +1091,7 @@ internal object TextLayoutManager { paint.letterSpacing = baseTextAttributes.letterSpacing } + var typefaceToApply = paint.typeface if ( baseTextAttributes.fontStyle != ReactConstants.UNSET || baseTextAttributes.fontWeight != ReactConstants.UNSET || @@ -1106,9 +1107,7 @@ internal object TextLayoutManager { baseTextAttributes.fontFamily, assets, ) - paint.setTypeface( - ReactTypefaceUtils.applyFontWeightAdjustment(typeface, fontWeightAdjustment), - ) + typefaceToApply = ReactTypefaceUtils.applyFontWeightAdjustment(typeface, fontWeightAdjustment) if ( baseTextAttributes.fontStyle != ReactConstants.UNSET && @@ -1125,12 +1124,16 @@ internal object TextLayoutManager { } else { val typeface = ReactTypefaceUtils.applyFontWeightAdjustment(null, fontWeightAdjustment) if (typeface != null) { - paint.setTypeface(typeface) + typefaceToApply = typeface } } paint.fontFeatureSettings = baseTextAttributes.fontFeatureSettings - ReactTypefaceUtils.applyFontVariationSettings(paint, baseTextAttributes.fontVariationSettings) + ReactTypefaceUtils.applyTypeface( + paint, + typefaceToApply, + baseTextAttributes.fontVariationSettings, + ) } /** diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/internal/span/CustomStyleSpan.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/internal/span/CustomStyleSpan.kt index a0607b012be6..425bf37c0e08 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/internal/span/CustomStyleSpan.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/internal/span/CustomStyleSpan.kt @@ -93,13 +93,10 @@ internal class CustomStyleSpan( ReactTypefaceUtils.applyStyles(paint.typeface, style, weight, family, assetManager) val adjustedTypeface = ReactTypefaceUtils.applyFontWeightAdjustment(typeface, fontWeightAdjustment) - paint.apply { - fontFeatureSettings = fontFeatureSettingsParam - setTypeface(adjustedTypeface) - ReactTypefaceUtils.applyFontVariationSettings(this, fontVariationSettingsParam) - isSubpixelText = true - isLinearText = true - } + paint.fontFeatureSettings = fontFeatureSettingsParam + ReactTypefaceUtils.applyTypeface(paint, adjustedTypeface, fontVariationSettingsParam) + paint.isSubpixelText = true + paint.isLinearText = true } } } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.kt index 807740e806db..677bd9f4eceb 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.kt @@ -726,14 +726,27 @@ public open class ReactEditText public constructor(context: Context) : AppCompat typefaceDirty = false - val newTypeface = applyStyles(typeface, fontStyle, fontWeight, fontFamily, context.assets) + val oldTypeface = typeface + val newTypeface = applyStyles(oldTypeface, fontStyle, fontWeight, fontFamily, context.assets) typeface = newTypeface + val typefaceChanged = oldTypeface !== typeface if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - // TextView skips unchanged settings even after a typeface change, so clear them first. - if (getFontVariationSettings() != null && parsedFontVariationSettings != null) { + val previousSettings = getFontVariationSettings() + // TextView retains the settings string when its typeface changes. + val needsReset = + typefaceChanged && + parsedFontVariationSettings != null && + previousSettings == parsedFontVariationSettings + if (needsReset) { + super.setFontVariationSettings(null) + } + if ( + !super.setFontVariationSettings(parsedFontVariationSettings) && + previousSettings != null && + !needsReset + ) { super.setFontVariationSettings(null) } - super.setFontVariationSettings(parsedFontVariationSettings) } // Match behavior of CustomStyleSpan and enable SUBPIXEL_TEXT_FLAG when setting anything diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/TextLayoutManagerFontWeightAdjustmentTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/TextLayoutManagerFontWeightAdjustmentTest.kt index 992b1e9b1cb5..66aef2db3d41 100644 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/TextLayoutManagerFontWeightAdjustmentTest.kt +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/TextLayoutManagerFontWeightAdjustmentTest.kt @@ -195,6 +195,7 @@ class TextLayoutManagerFontWeightAdjustmentTest { fun `reused paint re-applies unchanged font variation settings to a new typeface`() { val paint = mock() whenever(paint.fontVariationSettings).thenReturn("'wght' 600") + whenever(paint.setFontVariationSettings("'wght' 600")).thenReturn(true) ReactTypefaceUtils.applyFontVariationSettings(paint, "'wght' 600") @@ -219,6 +220,7 @@ class TextLayoutManagerFontWeightAdjustmentTest { fun `scratch text paint re-applies font variation settings on repeated measurement`() { val paint = mock() whenever(paint.fontVariationSettings).thenReturn("'wght' 600") + whenever(paint.setFontVariationSettings("'wght' 600")).thenReturn(true) val textAttributes = TextAttributeProps.fromReadableMap( ReactStylesDiffMap( @@ -240,6 +242,127 @@ class TextLayoutManagerFontWeightAdjustmentTest { } } + @Test + fun `changed variation settings do not require a preliminary reset`() { + val paint = mock() + whenever(paint.fontVariationSettings).thenReturn("'wght' 400") + whenever(paint.setFontVariationSettings("'wght' 600")).thenReturn(true) + + ReactTypefaceUtils.applyFontVariationSettings(paint, "'wght' 600") + + verify(paint).setFontVariationSettings("'wght' 600") + verify(paint, never()).setFontVariationSettings(null) + } + + @Test + fun `unchanged typeface and variations do not require a reset`() { + val paint = mock() + val typeface = mock() + whenever(paint.typeface).thenReturn(typeface) + whenever(paint.fontVariationSettings).thenReturn("'wght' 600") + whenever(paint.setFontVariationSettings("'wght' 600")).thenReturn(true) + + ReactTypefaceUtils.applyTypeface(paint, typeface, "'wght' 600") + + verify(paint, never()).setTypeface(any()) + verify(paint, never()).setFontVariationSettings(null) + verify(paint).setFontVariationSettings("'wght' 600") + } + + @Test + fun `replacing a typeface still resets identical variations before applying`() { + val paint = mock() + val baseTypeface = mock() + whenever(paint.typeface).thenReturn(mock()) + whenever(paint.fontVariationSettings).thenReturn("'wght' 600") + whenever(paint.setFontVariationSettings("'wght' 600")).thenReturn(true) + + ReactTypefaceUtils.applyTypeface(paint, baseTypeface, "'wght' 600") + + inOrder(paint) { + verify(paint).setTypeface(baseTypeface) + verify(paint).setFontVariationSettings(null) + verify(paint).setFontVariationSettings("'wght' 600") + } + } + + @Test + fun `span skips resetting an unchanged typeface and variations`() { + val assets = RuntimeEnvironment.getApplication().assets + val typeface = ReactTypefaceUtils.applyStyles(null, Typeface.NORMAL, 600, "sans-serif", assets) + val paint = mock() + whenever(paint.typeface).thenReturn(typeface) + whenever(paint.fontVariationSettings).thenReturn("'wght' 600") + whenever(paint.setFontVariationSettings("'wght' 600")).thenReturn(true) + val span = CustomStyleSpan(Typeface.NORMAL, 600, null, "'wght' 600", "sans-serif", assets) + + span.updateMeasureState(paint) + + verify(paint, never()).setTypeface(any()) + verify(paint, never()).setFontVariationSettings(null) + verify(paint).setFontVariationSettings("'wght' 600") + } + + @Test + fun `scratch text paint skips resetting an unchanged typeface and variations`() { + val assets = RuntimeEnvironment.getApplication().assets + val typeface = ReactTypefaceUtils.applyStyles(null, Typeface.NORMAL, 600, "sans-serif", assets) + val paint = mock() + whenever(paint.typeface).thenReturn(typeface) + whenever(paint.fontVariationSettings).thenReturn("'wght' 600") + whenever(paint.setFontVariationSettings("'wght' 600")).thenReturn(true) + val attributes = + TextAttributeProps.fromReadableMap( + ReactStylesDiffMap( + JavaOnlyMap.of( + "fontFamily", + "sans-serif", + "fontWeight", + "600", + "fontVariationSettings", + "'wght' 600", + ), + ), + ) + + TextLayoutManager.updateTextPaint(paint, attributes, assets, 0) + + verify(paint, never()).setTypeface(any()) + verify(paint, never()).setFontVariationSettings(null) + verify(paint).setFontVariationSettings("'wght' 600") + } + + @Test + fun `unsupported variations clear previous settings with unchanged or replaced typeface`() { + for (typefaceChanged in listOf(false, true)) { + val paint = mock() + val typeface = mock() + whenever(paint.typeface).thenReturn(if (typefaceChanged) mock() else typeface) + whenever(paint.fontVariationSettings).thenReturn("'wght' 600") + whenever(paint.setFontVariationSettings("'XXXX' 1")).thenReturn(false) + + ReactTypefaceUtils.applyTypeface(paint, typeface, "'XXXX' 1") + + inOrder(paint) { + verify(paint).setFontVariationSettings("'XXXX' 1") + verify(paint).setFontVariationSettings(null) + } + } + } + + @Test + fun `null and empty variations still clear previous settings`() { + for (settings in listOf(null, "")) { + val paint = mock() + whenever(paint.fontVariationSettings).thenReturn("'wght' 600") + whenever(paint.setFontVariationSettings(settings)).thenReturn(true) + + ReactTypefaceUtils.applyFontVariationSettings(paint, settings, false) + + verify(paint).setFontVariationSettings(settings) + } + } + private companion object { const val FONT_WEIGHT_ADJUSTMENT_BOLD_TEXT = 300 } diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/textinput/ReactTextInputPropertyTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/textinput/ReactTextInputPropertyTest.kt index dac9993d5ecc..ebbc9519494d 100644 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/textinput/ReactTextInputPropertyTest.kt +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/textinput/ReactTextInputPropertyTest.kt @@ -36,12 +36,18 @@ import com.facebook.react.uimanager.ThemedReactContext import com.facebook.react.uimanager.style.BorderRadiusProp import com.facebook.react.views.text.DefaultStyleValuesUtil.getDefaultTextColorHint import com.facebook.react.views.text.ReactTextUpdate +import com.facebook.react.views.text.ReactTypefaceUtils import com.facebook.react.views.text.internal.span.CustomStyleSpan import com.facebook.testutils.shadows.ShadowSoLoader import org.assertj.core.api.Assertions.assertThat import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import org.mockito.kotlin.doReturn +import org.mockito.kotlin.never +import org.mockito.kotlin.spy +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever import org.robolectric.RobolectricTestRunner import org.robolectric.RuntimeEnvironment import org.robolectric.annotation.Config @@ -532,6 +538,70 @@ class ReactTextInputPropertyTest { assertThat(view.parsedFontVariationSettings).isNull() } + @RequiresApi(Build.VERSION_CODES.O) + @Test + fun testUnchangedTypefaceDoesNotResetFontVariations() { + manager.updateProperties( + view, + buildStyles("fontFamily", "sans-serif", "fontVariationSettings", "'wght' 600"), + ) + val spyView = spy(view) + val baseTypeface = + ReactTypefaceUtils.applyStyles(view.typeface, 0, 400, "sans-serif", themedContext.assets) + doReturn(baseTypeface).whenever(spyView).typeface + doReturn("'wght' 600").whenever(spyView).fontVariationSettings + doReturn(true).whenever(spyView).setFontVariationSettings("'wght' 600") + + spyView.setFontFamily("sans-serif") + spyView.maybeUpdateTypeface() + + verify(spyView, never()).setFontVariationSettings(null) + } + + @RequiresApi(Build.VERSION_CODES.O) + @Test + fun testChangedFontVariationsDoNotRequirePreliminaryReset() { + manager.updateProperties(view, buildStyles("fontFamily", "sans-serif")) + val spyView = spy(view) + doReturn("'wght' 400").whenever(spyView).fontVariationSettings + doReturn(true).whenever(spyView).setFontVariationSettings("'wght' 600") + + spyView.setFontWeight("700") + spyView.setReactFontVariationSettings("'wght' 600") + spyView.maybeUpdateTypeface() + + verify(spyView, never()).setFontVariationSettings(null) + } + + @RequiresApi(Build.VERSION_CODES.O) + @Test + fun testTypefaceChangeStillResetsMatchingFontVariations() { + manager.updateProperties(view, buildStyles("fontFamily", "sans-serif")) + val spyView = spy(view) + doReturn("'wght' 600").whenever(spyView).fontVariationSettings + + spyView.setFontWeight("700") + spyView.setReactFontVariationSettings("'wght' 600") + spyView.maybeUpdateTypeface() + + verify(spyView).setFontVariationSettings(null) + } + + @RequiresApi(Build.VERSION_CODES.O) + @Test + fun testUnsupportedFontVariationsClearPreviousAxes() { + manager.updateProperties(view, buildStyles("fontFamily", "sans-serif")) + val spyView = spy(view) + val settings = "'XXXX' 1" + doReturn("'wght' 600").whenever(spyView).fontVariationSettings + doReturn(false).whenever(spyView).setFontVariationSettings(settings) + + spyView.setReactFontVariationSettings(settings) + spyView.maybeUpdateTypeface() + + verify(spyView).setFontVariationSettings(null) + } + @Test fun testFontVariationSettingsOverrideFontWeightRegardlessOfPropOrder() { manager.updateProperties(