Avoid redundant font variation resets (#59531)

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)

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,
+    )
   }
 
   /**

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
     }
   }
 }

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

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<Paint>()
     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<TextPaint>()
     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<Paint>()
+    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<Paint>()
+    val typeface = mock<Typeface>()
+    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<Paint>()
+    val baseTypeface = mock<Typeface>()
+    whenever(paint.typeface).thenReturn(mock<Typeface>())
+    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<TextPaint>()
+    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<TextPaint>()
+    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<Paint>()
+      val typeface = mock<Typeface>()
+      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<Paint>()
+      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
   }

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(