Reimplement Android lineHeight positioning/determination (#47271)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/47271

Let's keep the recent goals of centering (instead of arbitrary prioritizing ascent), ala spec, and make some changes to allow overlapping interior line-boxes, and make the implementation a lot simpler, instead of the cruft it has been accumulating.

The new simple versions is implemented as the only `CustomLineHeightSpan`. This replaces the code used when `enableAndroidLineHeightCentering` is enabled (which is now the default).

Legacy path is renamed to `LegacyLineHeightSpan`, slated to be deleted if rollout goes well.

We cannot yet cause text to overflow the bounds of the underlying TextView until potentially large later work related to ReactTextView reimplementation.

There's a somewhat arbitrary choice here, when rounding, to whether we ceil ascent vs descent when pixels don't evenly split. This does result in a visual difference, and for sake of avoiding breakage of screenshots, I left the same choice as before.

Changelog:
[Android][Fixed] - Reimplement Android lineHeight positioning/determination

Reviewed By: javache

Differential Revision: D64716557

fbshipit-source-id: 5a947377df7cfee9dff4484c840939f527caf94b

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsDefaults.kt

@@ -4,7 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @generated SignedSource<<57ac7a4a01e8639c22244a237a6dba1d>>
+ * @generated SignedSource<<09dd6fb517b7b965c5cc554a075e5590>>
  */
 
 /**
@@ -35,7 +35,7 @@ public open class ReactNativeFeatureFlagsDefaults : ReactNativeFeatureFlagsProvi
 
   override fun enableAlignItemsBaselineOnFabricIOS(): Boolean = true
 
-  override fun enableAndroidLineHeightCentering(): Boolean = false
+  override fun enableAndroidLineHeightCentering(): Boolean = true
 
   override fun enableBridgelessArchitecture(): Boolean = false
 

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java

@@ -21,6 +21,7 @@ import com.facebook.infer.annotation.Assertions;
 import com.facebook.react.bridge.ReadableArray;
 import com.facebook.react.bridge.ReadableMap;
 import com.facebook.react.common.ReactConstants;
+import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags;
 import com.facebook.react.uimanager.IllegalViewOperationException;
 import com.facebook.react.uimanager.LayoutShadowNode;
 import com.facebook.react.uimanager.NativeViewHierarchyOptimizer;
@@ -34,6 +35,7 @@ import com.facebook.react.views.text.internal.ReactTextInlineImageShadowNode;
 import com.facebook.react.views.text.internal.span.CustomLetterSpacingSpan;
 import com.facebook.react.views.text.internal.span.CustomLineHeightSpan;
 import com.facebook.react.views.text.internal.span.CustomStyleSpan;
+import com.facebook.react.views.text.internal.span.LegacyLineHeightSpan;
 import com.facebook.react.views.text.internal.span.ReactAbsoluteSizeSpan;
 import com.facebook.react.views.text.internal.span.ReactBackgroundColorSpan;
 import com.facebook.react.views.text.internal.span.ReactClickableSpan;
@@ -229,7 +231,11 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode {
       if (!Float.isNaN(effectiveLineHeight)
           && (parentTextAttributes == null
               || parentTextAttributes.getEffectiveLineHeight() != effectiveLineHeight)) {
-        ops.add(new SetSpanOperation(start, end, new CustomLineHeightSpan(effectiveLineHeight)));
+        if (ReactNativeFeatureFlags.enableAndroidLineHeightCentering()) {
+          ops.add(new SetSpanOperation(start, end, new CustomLineHeightSpan(effectiveLineHeight)));
+        } else {
+          ops.add(new SetSpanOperation(start, end, new LegacyLineHeightSpan(effectiveLineHeight)));
+        }
       }
       ops.add(new SetSpanOperation(start, end, new ReactTagSpan(textShadowNode.getReactTag())));
     }

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.java

@@ -32,12 +32,14 @@ import com.facebook.react.bridge.WritableArray;
 import com.facebook.react.common.ReactConstants;
 import com.facebook.react.common.build.ReactBuildConfig;
 import com.facebook.react.common.mapbuffer.MapBuffer;
+import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags;
 import com.facebook.react.uimanager.PixelUtil;
 import com.facebook.react.uimanager.ReactAccessibilityDelegate.AccessibilityRole;
 import com.facebook.react.uimanager.ReactAccessibilityDelegate.Role;
 import com.facebook.react.views.text.internal.span.CustomLetterSpacingSpan;
 import com.facebook.react.views.text.internal.span.CustomLineHeightSpan;
 import com.facebook.react.views.text.internal.span.CustomStyleSpan;
+import com.facebook.react.views.text.internal.span.LegacyLineHeightSpan;
 import com.facebook.react.views.text.internal.span.ReactAbsoluteSizeSpan;
 import com.facebook.react.views.text.internal.span.ReactBackgroundColorSpan;
 import com.facebook.react.views.text.internal.span.ReactClickableSpan;
@@ -317,9 +319,15 @@ public class TextLayoutManager {
                       textAttributes.mTextShadowColor)));
         }
         if (!Float.isNaN(textAttributes.getEffectiveLineHeight())) {
-          ops.add(
-              new SetSpanOperation(
-                  start, end, new CustomLineHeightSpan(textAttributes.getEffectiveLineHeight())));
+          if (ReactNativeFeatureFlags.enableAndroidLineHeightCentering()) {
+            ops.add(
+                new SetSpanOperation(
+                    start, end, new CustomLineHeightSpan(textAttributes.getEffectiveLineHeight())));
+          } else {
+            ops.add(
+                new SetSpanOperation(
+                    start, end, new LegacyLineHeightSpan(textAttributes.getEffectiveLineHeight())));
+          }
         }
 
         ops.add(new SetSpanOperation(start, end, new ReactTagSpan(reactTag)));

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/internal/span/CustomLineHeightSpan.kt

@@ -9,105 +9,48 @@ package com.facebook.react.views.text.internal.span
 
 import android.graphics.Paint.FontMetricsInt
 import android.text.style.LineHeightSpan
-import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags
 import kotlin.math.ceil
 import kotlin.math.floor
-import kotlin.math.min
 
 /**
- * We use a custom [LineHeightSpan], because `lineSpacingExtra` is broken. Details here:
- * https://github.com/facebook/react-native/issues/7546
+ * Implements a [LineHeightSpan] which follows web-like behavior for line height, unlike
+ * LineHeightSpan.Standard which only effects space between the baselines of adjacent line boxes
+ * (does not impact space before the first line or after the last).
  */
 public class CustomLineHeightSpan(height: Float) : LineHeightSpan, ReactSpan {
   public val lineHeight: Int = ceil(height.toDouble()).toInt()
 
-  private fun chooseCenteredHeight(fm: FontMetricsInt) {
-    if (fm.descent > lineHeight) {
-      // Show as much descent as possible
-      fm.descent = lineHeight
-      fm.bottom = 0
-      fm.top = 0
-      fm.ascent = 0
-    } else if (-fm.ascent + fm.descent > lineHeight) {
-      // Calculate the amount we are over, and split the adjustment between descent and ascent
-      val difference = -(lineHeight + fm.ascent - fm.descent) / 2
-      val remainder = difference % 2
-      fm.ascent = fm.ascent + difference
-      fm.descent = fm.descent - difference - remainder
-      fm.top = fm.ascent
-      fm.bottom = fm.descent
-    } else if (-fm.top + fm.bottom > lineHeight) {
-      // Calculate the amount we are over, and split the adjustment between top and bottom
-
-      val excess = ((-fm.top + fm.bottom) - lineHeight) / 2
-
-      fm.top += excess
-      fm.bottom -= excess
-    } else {
-      // Show proportionally additional ascent / top & descent / bottom
-      val additional = lineHeight - (-fm.top + fm.bottom)
-
-      // Round up for the negative values and down for the positive values  (arbitrary choice)
-      // So that bottom - top equals additional even if it's an odd number.
-      val top = (fm.top - ceil(additional / 2.0f)).toInt()
-      val bottom = (fm.bottom + floor(additional / 2.0f)).toInt()
-
-      fm.top = top
-      fm.ascent = top
-      fm.descent = bottom
-      fm.bottom = bottom
-    }
-  }
-
-  private fun chooseOriginalHeight(fm: FontMetricsInt) {
-    // This is more complicated that I wanted it to be. You can find a good explanation of what the
-    // FontMetrics mean here: http://stackoverflow.com/questions/27631736.
-    // The general solution is that if there's not enough height to show the full line height, we
-    // will prioritize in this order: descent, ascent, bottom, top
-
-    if (fm.descent > lineHeight) {
-      // Show as much descent as possible
-      fm.descent = min(lineHeight.toDouble(), fm.descent.toDouble()).toInt()
-      fm.bottom = fm.descent
-      fm.ascent = 0
-      fm.top = fm.ascent
-    } else if (-fm.ascent + fm.descent > lineHeight) {
-      // Show all descent, and as much ascent as possible
-      fm.bottom = fm.descent
-      fm.ascent = -lineHeight + fm.descent
-      fm.top = fm.ascent
-    } else if (-fm.ascent + fm.bottom > lineHeight) {
-      // Show all ascent, descent, as much bottom as possible
-      fm.top = fm.ascent
-      fm.bottom = fm.ascent + lineHeight
-    } else if (-fm.top + fm.bottom > lineHeight) {
-      // Show all ascent, descent, bottom, as much top as possible
-      fm.top = fm.bottom - lineHeight
-    } else {
-      // Show proportionally additional ascent / top & descent / bottom
-      val additional = lineHeight - (-fm.top + fm.bottom)
-
-      // Round up for the negative values and down for the positive values  (arbitrary choice)
-      // So that bottom - top equals additional even if it's an odd number.
-      val top = (fm.top - ceil(additional / 2.0f)).toInt()
-      val bottom = (fm.bottom + floor(additional / 2.0f)).toInt()
-
-      fm.top = top
-      fm.ascent = top
-      fm.descent = bottom
-      fm.bottom = bottom
-    }
-  }
-
   public override fun chooseHeight(
-      text: CharSequence?,
+      text: CharSequence,
       start: Int,
       end: Int,
       spanstartv: Int,
       v: Int,
       fm: FontMetricsInt,
   ) {
-    if (ReactNativeFeatureFlags.enableAndroidLineHeightCentering()) chooseCenteredHeight(fm)
-    else chooseOriginalHeight(fm)
+    // https://www.w3.org/TR/css-inline-3/#inline-height
+    // When its computed line-height is not normal, its layout bounds are derived solely from
+    // metrics of its first available font (ignoring glyphs from other fonts), and leading is used
+    // to adjust the effective A and D to add up to the used line-height. Calculate the leading L as
+    // L = line-height - (A + D). Half the leading (its half-leading) is added above A of the first
+    // available font, and the other half below D of the first available font, giving an effective
+    // ascent above the baseline of A′ = A + L/2, and an effective descent of D′ = D + L/2. However,
+    // if line-fit-edge is not leading and this is not the root inline box, if the half-leading is
+    // positive, treat it as zero. The layout bounds exactly encloses this effective A′ and D′.
+
+    val leading = lineHeight - ((-fm.ascent) + fm.descent)
+    fm.ascent -= ceil(leading / 2.0f).toInt()
+    fm.descent += floor(leading / 2.0f).toInt()
+
+    // The top of the first line, and the bottom of the last line, may influence bounds of the
+    // paragraph, so we match them to the text ascent/descent. It is otherwise desirable to allow
+    // line boxes to overlap (to allow too large glyphs to be drawn outside them), so we do not
+    // adjust the top/bottom of interior line-boxes.
+    if (start == 0) {
+      fm.top = fm.ascent
+    }
+    if (end == text.length) {
+      fm.bottom = fm.descent
+    }
   }
 }

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/internal/span/LegacyLineHeightSpan.kt

@@ -0,0 +1,69 @@
+/*
+ * 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.
+ */
+
+package com.facebook.react.views.text.internal.span
+
+import android.graphics.Paint.FontMetricsInt
+import android.text.style.LineHeightSpan
+import kotlin.math.ceil
+import kotlin.math.floor
+import kotlin.math.min
+
+/**
+ * We use a custom [LineHeightSpan], because `lineSpacingExtra` is broken. Details here:
+ * https://github.com/facebook/react-native/issues/7546
+ */
+public class LegacyLineHeightSpan(height: Float) : LineHeightSpan, ReactSpan {
+  public val lineHeight: Int = ceil(height.toDouble()).toInt()
+
+  public override fun chooseHeight(
+      text: CharSequence?,
+      start: Int,
+      end: Int,
+      spanstartv: Int,
+      v: Int,
+      fm: FontMetricsInt,
+  ) {
+    // This is more complicated that I wanted it to be. You can find a good explanation of what the
+    // FontMetrics mean here: http://stackoverflow.com/questions/27631736.
+    // The general solution is that if there's not enough height to show the full line height, we
+    // will prioritize in this order: descent, ascent, bottom, top
+
+    if (fm.descent > lineHeight) {
+      // Show as much descent as possible
+      fm.descent = min(lineHeight.toDouble(), fm.descent.toDouble()).toInt()
+      fm.bottom = fm.descent
+      fm.ascent = 0
+      fm.top = fm.ascent
+    } else if (-fm.ascent + fm.descent > lineHeight) {
+      // Show all descent, and as much ascent as possible
+      fm.bottom = fm.descent
+      fm.ascent = -lineHeight + fm.descent
+      fm.top = fm.ascent
+    } else if (-fm.ascent + fm.bottom > lineHeight) {
+      // Show all ascent, descent, as much bottom as possible
+      fm.top = fm.ascent
+      fm.bottom = fm.ascent + lineHeight
+    } else if (-fm.top + fm.bottom > lineHeight) {
+      // Show all ascent, descent, bottom, as much top as possible
+      fm.top = fm.bottom - lineHeight
+    } else {
+      // Show proportionally additional ascent / top & descent / bottom
+      val additional = lineHeight - (-fm.top + fm.bottom)
+
+      // Round up for the negative values and down for the positive values  (arbitrary choice)
+      // So that bottom - top equals additional even if it's an odd number.
+      val top = (fm.top - ceil(additional / 2.0f)).toInt()
+      val bottom = (fm.bottom + floor(additional / 2.0f)).toInt()
+
+      fm.top = top
+      fm.ascent = top
+      fm.descent = bottom
+      fm.bottom = bottom
+    }
+  }
+}

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java

@@ -50,6 +50,7 @@ import com.facebook.react.bridge.ReactContext;
 import com.facebook.react.bridge.ReactSoftExceptionLogger;
 import com.facebook.react.common.ReactConstants;
 import com.facebook.react.common.build.ReactBuildConfig;
+import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags;
 import com.facebook.react.uimanager.BackgroundStyleApplicator;
 import com.facebook.react.uimanager.LengthPercentage;
 import com.facebook.react.uimanager.LengthPercentageType;
@@ -71,6 +72,7 @@ import com.facebook.react.views.text.TextLayoutManager;
 import com.facebook.react.views.text.internal.span.CustomLetterSpacingSpan;
 import com.facebook.react.views.text.internal.span.CustomLineHeightSpan;
 import com.facebook.react.views.text.internal.span.CustomStyleSpan;
+import com.facebook.react.views.text.internal.span.LegacyLineHeightSpan;
 import com.facebook.react.views.text.internal.span.ReactAbsoluteSizeSpan;
 import com.facebook.react.views.text.internal.span.ReactBackgroundColorSpan;
 import com.facebook.react.views.text.internal.span.ReactForegroundColorSpan;
@@ -866,7 +868,13 @@ public class ReactEditText extends AppCompatEditText {
 
     float lineHeight = mTextAttributes.getEffectiveLineHeight();
     if (!Float.isNaN(lineHeight)) {
-      workingText.setSpan(new CustomLineHeightSpan(lineHeight), 0, workingText.length(), spanFlags);
+      if (ReactNativeFeatureFlags.enableAndroidLineHeightCentering()) {
+        workingText.setSpan(
+            new CustomLineHeightSpan(lineHeight), 0, workingText.length(), spanFlags);
+      } else {
+        workingText.setSpan(
+            new LegacyLineHeightSpan(lineHeight), 0, workingText.length(), spanFlags);
+      }
     }
   }
 

packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/CustomLineHeightSpanTest.kt

@@ -1,127 +0,0 @@
-/*
- * 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.
- */
-
-package com.facebook.react.views.text
-
-import android.graphics.Paint
-import com.facebook.react.internal.featureflags.ReactNativeFeatureFlagsForTests
-import com.facebook.react.views.text.internal.span.CustomLineHeightSpan
-import org.assertj.core.api.Assertions.assertThat
-import org.junit.Before
-import org.junit.Test
-import org.junit.runner.RunWith
-import org.robolectric.RobolectricTestRunner
-
-@RunWith(RobolectricTestRunner::class)
-class CustomLineHeightSpanTest {
-
-  @Before
-  fun setUp() {
-    ReactNativeFeatureFlagsForTests.setUp()
-  }
-
-  @Test
-  fun evenLineHeightShouldIncreaseAllMetricsProportionally() {
-    val customLineHeightSpan = CustomLineHeightSpan(22f)
-    val fm =
-        Paint.FontMetricsInt().apply {
-          top = -10
-          ascent = -5
-          descent = 5
-          bottom = 10
-        }
-    customLineHeightSpan.chooseHeight("Hi", 0, 2, 0, 0, fm)
-    assertThat(fm.top).isEqualTo(-11)
-    assertThat(fm.ascent).isEqualTo(-11)
-    assertThat(fm.descent).isEqualTo(11)
-    assertThat(fm.bottom).isEqualTo(11)
-    assertThat(fm.bottom - fm.top).isEqualTo(22)
-  }
-
-  @Test
-  fun oddLineHeightShouldAlsoWork() {
-    val customLineHeightSpan = CustomLineHeightSpan(23f)
-    val fm =
-        Paint.FontMetricsInt().apply {
-          top = -10
-          ascent = -5
-          descent = 5
-          bottom = 10
-        }
-    customLineHeightSpan.chooseHeight("Hi", 0, 2, 0, 0, fm)
-    assertThat(fm.descent - fm.ascent).isEqualTo(23)
-    assertThat(fm.bottom - fm.top).isEqualTo(23)
-  }
-
-  @Test
-  fun shouldReduceTopFirst() {
-    val customLineHeightSpan = CustomLineHeightSpan(19f)
-    val fm =
-        Paint.FontMetricsInt().apply {
-          top = -10
-          ascent = -5
-          descent = 5
-          bottom = 10
-        }
-    customLineHeightSpan.chooseHeight("Hi", 0, 2, 0, 0, fm)
-    assertThat(fm.top).isEqualTo(-9)
-    assertThat(fm.ascent).isEqualTo(-5)
-    assertThat(fm.descent).isEqualTo(5)
-    assertThat(fm.bottom).isEqualTo(10)
-  }
-
-  @Test
-  fun shouldReduceBottomSecond() {
-    val customLineHeightSpan = CustomLineHeightSpan(14f)
-    val fm =
-        Paint.FontMetricsInt().apply {
-          top = -10
-          ascent = -5
-          descent = 5
-          bottom = 10
-        }
-    customLineHeightSpan.chooseHeight("Hi", 0, 2, 0, 0, fm)
-    assertThat(fm.top).isEqualTo(-5)
-    assertThat(fm.ascent).isEqualTo(-5)
-    assertThat(fm.descent).isEqualTo(5)
-    assertThat(fm.bottom).isEqualTo(9)
-  }
-
-  @Test
-  fun shouldReduceAscentThird() {
-    val customLineHeightSpan = CustomLineHeightSpan(9f)
-    val fm =
-        Paint.FontMetricsInt().apply {
-          top = -10
-          ascent = -5
-          descent = 5
-          bottom = 10
-        }
-    customLineHeightSpan.chooseHeight("Hi", 0, 2, 0, 0, fm)
-    assertThat(fm.top).isEqualTo(-4)
-    assertThat(fm.ascent).isEqualTo(-4)
-    assertThat(fm.descent).isEqualTo(5)
-    assertThat(fm.bottom).isEqualTo(5)
-  }
-
-  @Test
-  fun shouldReduceDescentLast() {
-    val customLineHeightSpan = CustomLineHeightSpan(4f)
-    val fm =
-        Paint.FontMetricsInt().apply {
-          top = -10
-          ascent = -5
-          descent = 5
-          bottom = 10
-        }
-    customLineHeightSpan.chooseHeight("Hi", 0, 2, 0, 0, fm)
-    assertThat(fm.top).isEqualTo(0)
-    assertThat(fm.ascent).isEqualTo(0)
-    assertThat(fm.descent).isEqualTo(4)
-    assertThat(fm.bottom).isEqualTo(4)
-  }
-}

packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsDefaults.h

@@ -4,7 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @generated SignedSource<<a560db0675e155df4901f0e7f28b6a09>>
+ * @generated SignedSource<<2964ff45378547dd40a9b0df440f8869>>
  */
 
 /**
@@ -52,7 +52,7 @@ class ReactNativeFeatureFlagsDefaults : public ReactNativeFeatureFlagsProvider {
   }
 
   bool enableAndroidLineHeightCentering() override {
-    return false;
+    return true;
   }
 
   bool enableBridgelessArchitecture() override {

packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js

@@ -109,12 +109,11 @@ const definitions: FeatureFlagDefinitions = {
       },
     },
     enableAndroidLineHeightCentering: {
-      defaultValue: false,
+      defaultValue: true,
       metadata: {
-        dateAdded: '2024-09-11',
         description:
           'When enabled, custom line height calculation will be centered from top to bottom.',
-        purpose: 'experimentation',
+        purpose: 'release',
       },
     },
     enableBridgelessArchitecture: {

packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js

@@ -4,7 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @generated SignedSource<<4aeb1a4c162c49501c765f4c5892b09e>>
+ * @generated SignedSource<<27ae96c2bc3459bd89e52063a8ed9490>>
  * @flow strict
  */
 
@@ -223,7 +223,7 @@ export const enableAlignItemsBaselineOnFabricIOS: Getter<boolean> = createNative
 /**
  * When enabled, custom line height calculation will be centered from top to bottom.
  */
-export const enableAndroidLineHeightCentering: Getter<boolean> = createNativeFlagGetter('enableAndroidLineHeightCentering', false);
+export const enableAndroidLineHeightCentering: Getter<boolean> = createNativeFlagGetter('enableAndroidLineHeightCentering', true);
 /**
  * Feature flag to enable the new bridgeless architecture. Note: Enabling this will force enable the following flags: `useTurboModules` & `enableFabricRenderer.
  */