Fix more text rounding bugs (#51303)

Summary:
Pull Request resolved: #51303

We do a lot of incorrect `cei()` in text measurement, compensated later by other incorrect bits.

There are a couple of interesting bits here:

A "desired width" is how large a hypothetical text layout would like to be. It is a floating point value, and to avoid truncation, a container must be larger than the desired width. So ceiling this is correct.

We are also passed available width, which may be an exact specification, not at a subpixel boundary. Ceiling this is totally incorrect, since Yoga will disregard our ceiled version, and we just created a layout, larger than our actual measure will be.

We must instead floor `availableWidth`, and we create our layout based off of that, to not give an extra physical pixel, that may not be given to us, if later layout rounding doesn't go our way. We then ceil `desiredWidth` earlier.

Finally, when we have an exact measuremode, we create the layout so that it uses guaranteedWidth`, but act as if it takes the full available space, per-contract, which may be a subpixel size larger. This means we cannot create layouts which cause truncation.

I used this change as an opportunity to clean up `createLayout()`, since we are gating anyways, to remove the duplicated paths, and to avoid the unnecessary `TextDirectionHeuristic` work for BoringLayout case, and since `StaticLayoutBuilder` already defaults to the heuristic we are manually setting.

Changelog:
[Android][Fixed] - Fix more text rounding bugs

Reviewed By: joevilches

Differential Revision: D74685353

fbshipit-source-id: 3700df657958c6efb46bfbbe674051d16a2b7c26

Author: NickGerleman

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlags.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<<255deaad99bac2448e0a11bad50a770b>>
+ * @generated SignedSource<<e9a109fd77667dd0cb945ef6ef9737f2>>
  */
 
 /**
@@ -42,6 +42,12 @@ public object ReactNativeFeatureFlags {
   @JvmStatic
   public fun animatedShouldSignalBatch(): Boolean = accessor.animatedShouldSignalBatch()
 
+  /**
+   * Do not incorrectly ceil the available width of an Android text layout
+   */
+  @JvmStatic
+  public fun avoidCeilingAvailableAndroidTextWidth(): Boolean = accessor.avoidCeilingAvailableAndroidTextWidth()
+
   /**
    * Use a C++ implementation of Native Animated instead of the platform implementation.
    */

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxAccessor.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<<de51ff5f161363975ef67549e717f010>>
+ * @generated SignedSource<<1a5cd689229d4e0c31070123045e84da>>
  */
 
 /**
@@ -22,6 +22,7 @@ package com.facebook.react.internal.featureflags
 internal class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccessor {
   private var commonTestFlagCache: Boolean? = null
   private var animatedShouldSignalBatchCache: Boolean? = null
+  private var avoidCeilingAvailableAndroidTextWidthCache: Boolean? = null
   private var cxxNativeAnimatedEnabledCache: Boolean? = null
   private var disableMainQueueSyncDispatchIOSCache: Boolean? = null
   private var disableMountItemReorderingAndroidCache: Boolean? = null
@@ -86,6 +87,15 @@ internal class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAcces
     return cached
   }
 
+  override fun avoidCeilingAvailableAndroidTextWidth(): Boolean {
+    var cached = avoidCeilingAvailableAndroidTextWidthCache
+    if (cached == null) {
+      cached = ReactNativeFeatureFlagsCxxInterop.avoidCeilingAvailableAndroidTextWidth()
+      avoidCeilingAvailableAndroidTextWidthCache = cached
+    }
+    return cached
+  }
+
   override fun cxxNativeAnimatedEnabled(): Boolean {
     var cached = cxxNativeAnimatedEnabledCache
     if (cached == null) {

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxInterop.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<<7a0e83bca17b862a0454985d75131959>>
+ * @generated SignedSource<<c39d0c1834797b3309b4fc5ce814280c>>
  */
 
 /**
@@ -32,6 +32,8 @@ public object ReactNativeFeatureFlagsCxxInterop {
 
   @DoNotStrip @JvmStatic public external fun animatedShouldSignalBatch(): Boolean
 
+  @DoNotStrip @JvmStatic public external fun avoidCeilingAvailableAndroidTextWidth(): Boolean
+
   @DoNotStrip @JvmStatic public external fun cxxNativeAnimatedEnabled(): Boolean
 
   @DoNotStrip @JvmStatic public external fun disableMainQueueSyncDispatchIOS(): Boolean

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<<918f59b35fc1af953f615a6e7eace76e>>
+ * @generated SignedSource<<7f357475254104729cc7910c14e1c1fb>>
  */
 
 /**
@@ -27,6 +27,8 @@ public open class ReactNativeFeatureFlagsDefaults : ReactNativeFeatureFlagsProvi
 
   override fun animatedShouldSignalBatch(): Boolean = false
 
+  override fun avoidCeilingAvailableAndroidTextWidth(): Boolean = true
+
   override fun cxxNativeAnimatedEnabled(): Boolean = false
 
   override fun disableMainQueueSyncDispatchIOS(): Boolean = false

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsLocalAccessor.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<<1b685268279bbfd65820e45f5028d4dd>>
+ * @generated SignedSource<<14cd1a58bd153dedda045a72c1494caa>>
  */
 
 /**
@@ -26,6 +26,7 @@ internal class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcc
 
   private var commonTestFlagCache: Boolean? = null
   private var animatedShouldSignalBatchCache: Boolean? = null
+  private var avoidCeilingAvailableAndroidTextWidthCache: Boolean? = null
   private var cxxNativeAnimatedEnabledCache: Boolean? = null
   private var disableMainQueueSyncDispatchIOSCache: Boolean? = null
   private var disableMountItemReorderingAndroidCache: Boolean? = null
@@ -92,6 +93,16 @@ internal class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcc
     return cached
   }
 
+  override fun avoidCeilingAvailableAndroidTextWidth(): Boolean {
+    var cached = avoidCeilingAvailableAndroidTextWidthCache
+    if (cached == null) {
+      cached = currentProvider.avoidCeilingAvailableAndroidTextWidth()
+      accessedFeatureFlags.add("avoidCeilingAvailableAndroidTextWidth")
+      avoidCeilingAvailableAndroidTextWidthCache = cached
+    }
+    return cached
+  }
+
   override fun cxxNativeAnimatedEnabled(): Boolean {
     var cached = cxxNativeAnimatedEnabledCache
     if (cached == null) {

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsProvider.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<<e9d98230a39a243ea748105a77ebd1e0>>
+ * @generated SignedSource<<593b1d64dc31038140032a6b0a439700>>
  */
 
 /**
@@ -27,6 +27,8 @@ public interface ReactNativeFeatureFlagsProvider {
 
   @DoNotStrip public fun animatedShouldSignalBatch(): Boolean
 
+  @DoNotStrip public fun avoidCeilingAvailableAndroidTextWidth(): Boolean
+
   @DoNotStrip public fun cxxNativeAnimatedEnabled(): Boolean
 
   @DoNotStrip public fun disableMainQueueSyncDispatchIOS(): Boolean

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

@@ -381,6 +381,108 @@ private static Layout createLayout(
       @Nullable TextUtils.TruncateAt ellipsizeMode,
       int maxNumberOfLines,
       TextPaint paint) {
+    if (ReactNativeFeatureFlags.avoidCeilingAvailableAndroidTextWidth()) {
+      return createLayoutWithCorrectRounding(
+          text,
+          boring,
+          width,
+          widthYogaMeasureMode,
+          includeFontPadding,
+          textBreakStrategy,
+          hyphenationFrequency,
+          alignment,
+          justificationMode,
+          ellipsizeMode,
+          maxNumberOfLines,
+          paint);
+    } else {
+      return createLayoutWithBuggedRounding(
+          text,
+          boring,
+          width,
+          widthYogaMeasureMode,
+          includeFontPadding,
+          textBreakStrategy,
+          hyphenationFrequency,
+          alignment,
+          justificationMode,
+          ellipsizeMode,
+          maxNumberOfLines,
+          paint);
+    }
+  }
+
+  private static Layout createLayoutWithCorrectRounding(
+      Spannable text,
+      BoringLayout.Metrics boring,
+      float width,
+      YogaMeasureMode widthYogaMeasureMode,
+      boolean includeFontPadding,
+      int textBreakStrategy,
+      int hyphenationFrequency,
+      Layout.Alignment alignment,
+      int justificationMode,
+      @Nullable TextUtils.TruncateAt ellipsizeMode,
+      int maxNumberOfLines,
+      TextPaint paint) {
+    // If our text is boring, and fully fits in the available space, we can represent the text
+    // layout as a BoringLayout
+    if (boring != null
+        && (widthYogaMeasureMode == YogaMeasureMode.UNDEFINED
+            || boring.width <= Math.floor(width))) {
+      int layoutWidth =
+          widthYogaMeasureMode == YogaMeasureMode.EXACTLY ? (int) Math.floor(width) : boring.width;
+      return BoringLayout.make(
+          text, paint, layoutWidth, alignment, 1.f, 0.f, boring, includeFontPadding);
+    }
+
+    int desiredWidth = (int) Math.ceil(Layout.getDesiredWidth(text, paint));
+
+    int layoutWidth =
+        widthYogaMeasureMode == YogaMeasureMode.EXACTLY
+            ? (int) Math.floor(width)
+            : widthYogaMeasureMode == YogaMeasureMode.UNDEFINED
+                ? desiredWidth
+                : Math.min(desiredWidth, (int) Math.floor(width));
+
+    StaticLayout.Builder builder =
+        StaticLayout.Builder.obtain(text, 0, text.length(), paint, layoutWidth)
+            .setAlignment(alignment)
+            .setLineSpacing(0.f, 1.f)
+            .setIncludePad(includeFontPadding)
+            .setBreakStrategy(textBreakStrategy)
+            .setHyphenationFrequency(hyphenationFrequency);
+
+    if (ReactNativeFeatureFlags.incorporateMaxLinesDuringAndroidLayout()) {
+      if (maxNumberOfLines != ReactConstants.UNSET && maxNumberOfLines != 0) {
+        builder.setEllipsize(ellipsizeMode).setMaxLines(maxNumberOfLines);
+      }
+    }
+
+    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
+      builder.setJustificationMode(justificationMode);
+    }
+
+    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
+      builder.setUseLineSpacingFromFallbacks(true);
+    }
+
+    return builder.build();
+  }
+
+  private static Layout createLayoutWithBuggedRounding(
+      Spannable text,
+      BoringLayout.Metrics boring,
+      float width,
+      YogaMeasureMode widthYogaMeasureMode,
+      boolean includeFontPadding,
+      int textBreakStrategy,
+      int hyphenationFrequency,
+      Layout.Alignment alignment,
+      int justificationMode,
+      @Nullable TextUtils.TruncateAt ellipsizeMode,
+      int maxNumberOfLines,
+      TextPaint paint) {
     Layout layout;
 
     int spanLength = text.length();
@@ -788,6 +890,12 @@ private static float calculateWidth(
       YogaMeasureMode widthYogaMeasureMode,
       int calculatedLineCount) {
     if (ReactNativeFeatureFlags.useAndroidTextLayoutWidthDirectly()) {
+      // Our layout must be created at a physical pixel boundary, so may be sized smaller by a
+      // subpixel compared to the assigned layout width.
+      if (widthYogaMeasureMode == YogaMeasureMode.EXACTLY) {
+        return width;
+      }
+
       return layout.getWidth();
     }
 
@@ -829,7 +937,7 @@ private static float calculateWidth(
     // where the container is measured smaller than text. Math.ceil prevents it
     // See T136756103 for investigation
     if (android.os.Build.VERSION.SDK_INT > Build.VERSION_CODES.Q) {
-      calculatedWidth = (float) Math.ceil(calculatedWidth);
+      calculatedWidth = Math.min((float) Math.ceil(calculatedWidth), width);
     }
     return calculatedWidth;
   }

packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.cpp

@@ -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<<7199b7c0c5d3b389e504f615a813b6c5>>
+ * @generated SignedSource<<d74dc8182a14fddbd7118149298515bd>>
  */
 
 /**
@@ -51,6 +51,12 @@ class ReactNativeFeatureFlagsJavaProvider
     return method(javaProvider_);
   }
 
+  bool avoidCeilingAvailableAndroidTextWidth() override {
+    static const auto method =
+        getReactNativeFeatureFlagsProviderJavaClass()->getMethod<jboolean()>("avoidCeilingAvailableAndroidTextWidth");
+    return method(javaProvider_);
+  }
+
   bool cxxNativeAnimatedEnabled() override {
     static const auto method =
         getReactNativeFeatureFlagsProviderJavaClass()->getMethod<jboolean()>("cxxNativeAnimatedEnabled");
@@ -335,6 +341,11 @@ bool JReactNativeFeatureFlagsCxxInterop::animatedShouldSignalBatch(
   return ReactNativeFeatureFlags::animatedShouldSignalBatch();
 }
 
+bool JReactNativeFeatureFlagsCxxInterop::avoidCeilingAvailableAndroidTextWidth(
+    facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop> /*unused*/) {
+  return ReactNativeFeatureFlags::avoidCeilingAvailableAndroidTextWidth();
+}
+
 bool JReactNativeFeatureFlagsCxxInterop::cxxNativeAnimatedEnabled(
     facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop> /*unused*/) {
   return ReactNativeFeatureFlags::cxxNativeAnimatedEnabled();
@@ -597,6 +608,9 @@ void JReactNativeFeatureFlagsCxxInterop::registerNatives() {
       makeNativeMethod(
         "animatedShouldSignalBatch",
         JReactNativeFeatureFlagsCxxInterop::animatedShouldSignalBatch),
+      makeNativeMethod(
+        "avoidCeilingAvailableAndroidTextWidth",
+        JReactNativeFeatureFlagsCxxInterop::avoidCeilingAvailableAndroidTextWidth),
       makeNativeMethod(
         "cxxNativeAnimatedEnabled",
         JReactNativeFeatureFlagsCxxInterop::cxxNativeAnimatedEnabled),

packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.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<<3d383541d829ce105ac5e97a5d35ded1>>
+ * @generated SignedSource<<a5ab0022f6a01bd6e929d36d9d87db10>>
  */
 
 /**
@@ -36,6 +36,9 @@ class JReactNativeFeatureFlagsCxxInterop
   static bool animatedShouldSignalBatch(
     facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);
 
+  static bool avoidCeilingAvailableAndroidTextWidth(
+    facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);
+
   static bool cxxNativeAnimatedEnabled(
     facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);
 

packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.cpp

@@ -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<<33afce4bb21ad12cb920eee24ba921b2>>
+ * @generated SignedSource<<88fdbea2f97f628187164a47a9737da0>>
  */
 
 /**
@@ -34,6 +34,10 @@ bool ReactNativeFeatureFlags::animatedShouldSignalBatch() {
   return getAccessor().animatedShouldSignalBatch();
 }
 
+bool ReactNativeFeatureFlags::avoidCeilingAvailableAndroidTextWidth() {
+  return getAccessor().avoidCeilingAvailableAndroidTextWidth();
+}
+
 bool ReactNativeFeatureFlags::cxxNativeAnimatedEnabled() {
   return getAccessor().cxxNativeAnimatedEnabled();
 }