From 65d8f66b50471d2fb4ddd5e63e17fcc808623110 Mon Sep 17 00:00:00 2001 From: Melissa Liu Date: Wed, 11 Sep 2024 14:53:09 -0700 Subject: [PATCH] improve text line height calculation (#46362) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/46362 Changelog: [Android] [Fixed] - improve text line height calculation ## Context There is a recurring issue in React Native where text containing accented characters, non-latin scripts, and special glyphs experiences truncation when line height is set too small, even when the line height equals or exceeds the font size. This problem has a significant impact, particularly when rendering complex text in multiple languages or special character sets. See: https://docs.google.com/document/d/1W-A80gKAyhVbz_WKktSwwJP5qm6h6ZBjFNcsVbknXhI/edit?usp=sharing for more context ## Investigation Previously, when font metrics (ascent, descent, top, bottom) exceeded the line height, the logic arbitrarily prioritized descent over ascent and bottom top. This led to vertical misalignment and text clipping at the top of the text. ## Proposed Implementation: 1. Descent Exceeds Line Height: Descent is capped to fit within the line height, setting ascent and top to 0, similar to the current behavior. 2. Shrink ascent and descent equally: When the combined ascent and descent exceed the line height, the vertical deficit is split proportionally between them, ensuring even distribution of the space. 3. Proportionally shrink top and bottom: If the top and bottom together exceed the line height, reductions are now applied proportionally based on the delta between top and ascent and bottom and descent. Reviewed By: NickGerleman Differential Revision: D62295350 fbshipit-source-id: 81381e8ea18ba9059488468295778c21781e4e7a --- .../internal/span/CustomLineHeightSpan.kt | 75 +++++++++++++++++-- .../views/text/CustomLineHeightSpanTest.kt | 7 ++ 2 files changed, 74 insertions(+), 8 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/internal/span/CustomLineHeightSpan.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/internal/span/CustomLineHeightSpan.kt index 830d987439cbb9..3eeb3abc3e7993 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/internal/span/CustomLineHeightSpan.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/internal/span/CustomLineHeightSpan.kt @@ -9,6 +9,8 @@ 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.abs import kotlin.math.ceil import kotlin.math.floor import kotlin.math.min @@ -20,14 +22,59 @@ import kotlin.math.min public class CustomLineHeightSpan(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 - ) { + 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) { + val excess = (-fm.top + fm.bottom) - lineHeight + + // Calculate the differences from top to ascent and bottom to descent + val topToAscent = abs(fm.top - fm.ascent) + val bottomToDescent = abs(fm.bottom - fm.descent) + + // Calculate the total delta + val totalDelta = topToAscent + bottomToDescent + + // Calculate proportional reductions + val topReduction = (excess * topToAscent / totalDelta).toInt() + val bottomReduction = (excess * bottomToDescent / totalDelta).toInt() + + // Adjust fm.top and fm.bottom + fm.top += topReduction + fm.bottom += bottomReduction + + // If there's a remainder, put it on the top + val remainder = excess - (topReduction + bottomReduction) + fm.top += remainder + } 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 @@ -66,4 +113,16 @@ public class CustomLineHeightSpan(height: Float) : LineHeightSpan, ReactSpan { fm.bottom = bottom } } + + public override fun chooseHeight( + text: CharSequence?, + start: Int, + end: Int, + spanstartv: Int, + v: Int, + fm: FontMetricsInt, + ) { + if (ReactNativeFeatureFlags.enableAndroidLineHeightCentering()) chooseCenteredHeight(fm) + else chooseOriginalHeight(fm) + } } diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/CustomLineHeightSpanTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/CustomLineHeightSpanTest.kt index f55c90af78de44..bed18787a9e6ae 100644 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/CustomLineHeightSpanTest.kt +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/CustomLineHeightSpanTest.kt @@ -8,8 +8,10 @@ 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 @@ -17,6 +19,11 @@ import org.robolectric.RobolectricTestRunner @RunWith(RobolectricTestRunner::class) class CustomLineHeightSpanTest { + @Before + fun setUp() { + ReactNativeFeatureFlagsForTests.setUp() + } + @Test fun evenLineHeightShouldIncreaseAllMetricsProportionally() { val customLineHeightSpan = CustomLineHeightSpan(22f)