From da94513967737d605b58bffbaccbe90b1f152e1c Mon Sep 17 00:00:00 2001 From: Tyler Freeman Date: Thu, 13 Apr 2023 23:57:08 +0000 Subject: [PATCH 1/3] feat(non linear font scaling): add FontScaleConverterFactory.isNonLinearFontScalingActive() This helps us determine if we can skip certain calculations as a performance optimization, for instance when calculating line height proportionally. Bug: 273326061 Test: atest frameworks/base/core/java/android/content/res/FontScaleConverterFactory.java Change-Id: I364a978b457c9b7a7073b34fa8cc1f3abaeabbac --- .../res/FontScaleConverterFactory.java | 26 ++++++++++++++++--- .../res/FontScaleConverterFactoryTest.kt | 16 ++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/core/java/android/content/res/FontScaleConverterFactory.java b/core/java/android/content/res/FontScaleConverterFactory.java index 6b09c303e3cd7..5eb65262918d6 100644 --- a/core/java/android/content/res/FontScaleConverterFactory.java +++ b/core/java/android/content/res/FontScaleConverterFactory.java @@ -34,6 +34,8 @@ public class FontScaleConverterFactory { @VisibleForTesting static final SparseArray LOOKUP_TABLES = new SparseArray<>(); + private static float sMinScaleBeforeCurvesApplied = 1.05f; + static { // These were generated by frameworks/base/tools/fonts/font-scaling-array-generator.js and // manually tweaked for optimum readability. @@ -82,10 +84,29 @@ public class FontScaleConverterFactory { new float[] { 16f, 20f, 24f, 26f, 30f, 34f, 36f, 38f, 100}) ); + sMinScaleBeforeCurvesApplied = getScaleFromKey(LOOKUP_TABLES.keyAt(0)) - 0.02f; + if (sMinScaleBeforeCurvesApplied <= 1.0f) { + throw new IllegalStateException( + "You should only apply non-linear scaling to font scales > 1" + ); + } } private FontScaleConverterFactory() {} + /** + * Returns true if non-linear font scaling curves would be in effect for the given scale, false + * if the scaling would follow a linear curve or for no scaling. + * + *

Example usage: + * isNonLinearFontScalingActive(getResources().getConfiguration().fontScale) + * + * @hide + */ + public static boolean isNonLinearFontScalingActive(float fontScale) { + return fontScale >= sMinScaleBeforeCurvesApplied; + } + /** * Finds a matching FontScaleConverter for the given fontScale factor. * @@ -97,10 +118,7 @@ public class FontScaleConverterFactory { */ @Nullable public static FontScaleConverter forScale(float fontScale) { - if (fontScale <= 1) { - // We don't need non-linear curves for shrinking text or for 100%. - // Also, fontScale==0 should not have a curve either. - // And ignore negative font scales; that's just silly. + if (!isNonLinearFontScalingActive(fontScale)) { return null; } diff --git a/core/tests/coretests/src/android/content/res/FontScaleConverterFactoryTest.kt b/core/tests/coretests/src/android/content/res/FontScaleConverterFactoryTest.kt index a0d8dcf830e87..ba6c8fab48d4d 100644 --- a/core/tests/coretests/src/android/content/res/FontScaleConverterFactoryTest.kt +++ b/core/tests/coretests/src/android/content/res/FontScaleConverterFactoryTest.kt @@ -122,6 +122,22 @@ class FontScaleConverterFactoryTest { } } + @SmallTest + fun testIsNonLinearFontScalingActive() { + assertThat(FontScaleConverterFactory.isNonLinearFontScalingActive(1f)).isFalse() + assertThat(FontScaleConverterFactory.isNonLinearFontScalingActive(0f)).isFalse() + assertThat(FontScaleConverterFactory.isNonLinearFontScalingActive(-1f)).isFalse() + assertThat(FontScaleConverterFactory.isNonLinearFontScalingActive(0.85f)).isFalse() + assertThat(FontScaleConverterFactory.isNonLinearFontScalingActive(1.02f)).isFalse() + assertThat(FontScaleConverterFactory.isNonLinearFontScalingActive(1.10f)).isFalse() + assertThat(FontScaleConverterFactory.isNonLinearFontScalingActive(1.15f)).isTrue() + assertThat(FontScaleConverterFactory.isNonLinearFontScalingActive(1.1499999f)) + .isTrue() + assertThat(FontScaleConverterFactory.isNonLinearFontScalingActive(1.5f)).isTrue() + assertThat(FontScaleConverterFactory.isNonLinearFontScalingActive(2f)).isTrue() + assertThat(FontScaleConverterFactory.isNonLinearFontScalingActive(3f)).isTrue() + } + @LargeTest @Test fun allFeasibleScalesAndConversionsDoNotCrash() { From a4d59758d4790e7f32d11699a0292199dee9c12b Mon Sep 17 00:00:00 2001 From: Tyler Freeman Date: Thu, 23 Mar 2023 23:31:11 +0000 Subject: [PATCH 2/3] fix(non linear font scaling): preserve proportionality of lineHeight when non-linear font scaling is in effect This ensures that the design intent is preserved when using large font scales and non-linear font scaling: the line height proportions should still match even when using SP units for both text size and line height. If the dev does not define both textSize and lineHeight in SP units, we don't do the corrective recalculation. Unfortunately, this could result in cramped-looking text due to the lineHeight not scaling with text size. There are linters to warn the developer of this though, so we hope they do the right thing. Bug: 273326061 Test: atest cts/tests/tests/widget/src/android/widget/cts/TextViewFontScalingTest.kt Change-Id: I17f9b7c8d3e0e63deed4037ebf0e99b690bc694a --- core/java/android/widget/TextView.java | 48 +++++++++++++++++++++++--- 1 file changed, 43 insertions(+), 5 deletions(-) diff --git a/core/java/android/widget/TextView.java b/core/java/android/widget/TextView.java index 67c9f8ca00489..65a937bdc4bc7 100644 --- a/core/java/android/widget/TextView.java +++ b/core/java/android/widget/TextView.java @@ -62,6 +62,7 @@ import android.content.pm.PackageManager; import android.content.res.ColorStateList; import android.content.res.CompatibilityInfo; import android.content.res.Configuration; +import android.content.res.FontScaleConverterFactory; import android.content.res.Resources; import android.content.res.TypedArray; import android.content.res.XmlResourceParser; @@ -1233,7 +1234,8 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener defStyleAttr, defStyleRes); int firstBaselineToTopHeight = -1; int lastBaselineToBottomHeight = -1; - int lineHeight = -1; + float lineHeight = -1f; + int lineHeightUnit = -1; readTextAppearance(context, a, attributes, true /* styleArray */); @@ -1583,7 +1585,13 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener break; case com.android.internal.R.styleable.TextView_lineHeight: - lineHeight = a.getDimensionPixelSize(attr, -1); + TypedValue peekValue = a.peekValue(attr); + if (peekValue != null && peekValue.type == TypedValue.TYPE_DIMENSION) { + lineHeightUnit = peekValue.getComplexUnit(); + lineHeight = TypedValue.complexToFloat(peekValue.data); + } else { + lineHeight = a.getDimensionPixelSize(attr, -1); + } break; } } @@ -1936,7 +1944,11 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener setLastBaselineToBottomHeight(lastBaselineToBottomHeight); } if (lineHeight >= 0) { - setLineHeight(lineHeight); + if (lineHeightUnit == -1) { + setLineHeightPx(lineHeight); + } else { + setLineHeight(lineHeightUnit, lineHeight); + } } } @@ -6236,8 +6248,34 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener @TypedValue.ComplexDimensionUnit int unit, @FloatRange(from = 0) float lineHeight ) { - setLineHeightPx( - TypedValue.applyDimension(unit, lineHeight, getDisplayMetricsOrSystem())); + var metrics = getDisplayMetricsOrSystem(); + // We can avoid the recalculation if we know non-linear font scaling isn't being used + // (an optimization for the majority case). + // We also don't try to do the recalculation unless both textSize and lineHeight are in SP. + if (!FontScaleConverterFactory.isNonLinearFontScalingActive( + getResources().getConfiguration().fontScale) + || unit != TypedValue.COMPLEX_UNIT_SP + || mTextSizeUnit != TypedValue.COMPLEX_UNIT_SP + ) { + setLineHeightPx(TypedValue.applyDimension(unit, lineHeight, metrics)); + return; + } + + // Recalculate a proportional line height when non-linear font scaling is in effect. + // Otherwise, a desired 2x line height at font scale 1.0 will not be 2x at font scale 2.0, + // due to non-linear font scaling compressing higher SP sizes. See b/273326061 for details. + // We know they are using SP units for both the text size and the line height + // at this point, so determine the ratio between them. This is the *intended* line spacing + // multiplier if font scale == 1.0. We can then determine what the pixel value for the line + // height would be if we preserved proportions. + var textSizePx = getTextSize(); + var textSizeSp = TypedValue.convertPixelsToDimension( + TypedValue.COMPLEX_UNIT_SP, + textSizePx, + metrics + ); + var ratio = lineHeight / textSizeSp; + setLineHeightPx(textSizePx * ratio); } /** From fa043bb5d9673bfa0f66fd9dfd680918b11e8456 Mon Sep 17 00:00:00 2001 From: Tyler Freeman Date: Thu, 13 Apr 2023 19:27:34 +0000 Subject: [PATCH 3/3] fix(non linear font scaling): recalculate proportional line height if they setTextSize() after setting line height This will only recalculate if both were set in SP, since that's the only time the proportion calculation happens anyway. Bug: 273326061 Test: atest cts/tests/tests/widget/src/android/widget/cts/TextViewFontScalingTest.kt Change-Id: I41ad12cdef6f1b93b60deca039c9ab72bb59e39f --- core/java/android/util/TypedValue.java | 20 ++++++++++++---- core/java/android/widget/TextView.java | 32 ++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/core/java/android/util/TypedValue.java b/core/java/android/util/TypedValue.java index b93e3386b7cca..330a9fce1b83b 100644 --- a/core/java/android/util/TypedValue.java +++ b/core/java/android/util/TypedValue.java @@ -385,10 +385,22 @@ public class TypedValue { * * @return The complex unit type. */ - public int getComplexUnit() - { - return COMPLEX_UNIT_MASK & (data>>TypedValue.COMPLEX_UNIT_SHIFT); - } + public int getComplexUnit() { + return getUnitFromComplexDimension(data); + } + + /** + * Return the complex unit type for the given complex dimension. For example, a dimen type + * with value 12sp will return {@link #COMPLEX_UNIT_SP}. Use with values created with {@link + * #createComplexDimension(int, int)} etc. + * + * @return The complex unit type. + * + * @hide + */ + public static int getUnitFromComplexDimension(int complexDimension) { + return COMPLEX_UNIT_MASK & (complexDimension >> TypedValue.COMPLEX_UNIT_SHIFT); + } /** * Converts an unpacked complex data value holding a dimension to its final floating point pixel diff --git a/core/java/android/widget/TextView.java b/core/java/android/widget/TextView.java index 65a937bdc4bc7..3fbb50581b25e 100644 --- a/core/java/android/widget/TextView.java +++ b/core/java/android/widget/TextView.java @@ -868,6 +868,14 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener @UnsupportedAppUsage private float mSpacingAdd = 0.0f; + /** + * Remembers what line height was set to originally, before we broke it down into raw pixels. + * + *

This is stored as a complex dimension with both value and unit packed into one field! + * {@see TypedValue} + */ + private int mLineHeightComplexDimen; + private int mBreakStrategy; private int mHyphenationFrequency; private int mJustificationMode; @@ -4641,6 +4649,7 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener if (size != mTextPaint.getTextSize()) { mTextPaint.setTextSize(size); + maybeRecalculateLineHeight(); if (shouldRequestLayout && mLayout != null) { // Do not auto-size right after setting the text size. mNeedsAutoSizeText = false; @@ -6226,6 +6235,9 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener if (lineHeight != fontHeight) { // Set lineSpacingExtra by the difference of lineSpacing with lineHeight setLineSpacing(lineHeight - fontHeight, 1f); + + mLineHeightComplexDimen = + TypedValue.createComplexDimension(lineHeight, TypedValue.COMPLEX_UNIT_PX); } } @@ -6258,6 +6270,9 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener || mTextSizeUnit != TypedValue.COMPLEX_UNIT_SP ) { setLineHeightPx(TypedValue.applyDimension(unit, lineHeight, metrics)); + + // Do this last so it overwrites what setLineHeightPx() sets it to. + mLineHeightComplexDimen = TypedValue.createComplexDimension(lineHeight, unit); return; } @@ -6276,6 +6291,23 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener ); var ratio = lineHeight / textSizeSp; setLineHeightPx(textSizePx * ratio); + + // Do this last so it overwrites what setLineHeightPx() sets it to. + mLineHeightComplexDimen = TypedValue.createComplexDimension(lineHeight, unit); + } + + private void maybeRecalculateLineHeight() { + if (mLineHeightComplexDimen == 0) { + return; + } + int unit = TypedValue.getUnitFromComplexDimension(mLineHeightComplexDimen); + if (unit != TypedValue.COMPLEX_UNIT_SP) { + // The lineHeight was never supplied in SP, so we didn't do any fancy recalculations + // in setLineHeight(). We don't need to recalculate. + return; + } + + setLineHeight(unit, TypedValue.complexToFloat(mLineHeightComplexDimen)); } /**