From de466db1a5b20095532622487363a0632648e335 Mon Sep 17 00:00:00 2001 From: Tyler Freeman Date: Thu, 16 Feb 2023 13:48:20 -0800 Subject: [PATCH] feat(non linear font scaling): interpolate between two tables if one cannot be found for a given scale MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a new scale is defined in the OEM’s Settings XML that doesn't match one of the hardcoded default tables, one will be generated by interpolating between two default tables. This way the experience is guaranteed for the user, even if the OEM forgets to define their tables to match their font scaling settings. Test: atest FrameworksCoreTests:android.content.res.FontScaleConverterFactoryTest Bug: b/247861716 Change-Id: If9c7f87f6dd4b99afa8579cfa0b52e313d761beb --- .../res/FontScaleConverterFactory.java | 70 +++++++++++++++++-- .../res/FontScaleConverterFactoryTest.kt | 54 ++++++++++++-- 2 files changed, 113 insertions(+), 11 deletions(-) diff --git a/core/java/android/content/res/FontScaleConverterFactory.java b/core/java/android/content/res/FontScaleConverterFactory.java index ffdc7b3807cff..6b09c303e3cd7 100644 --- a/core/java/android/content/res/FontScaleConverterFactory.java +++ b/core/java/android/content/res/FontScaleConverterFactory.java @@ -18,6 +18,7 @@ package android.content.res; import android.annotation.NonNull; import android.annotation.Nullable; +import android.util.MathUtils; import android.util.SparseArray; import com.android.internal.annotations.VisibleForTesting; @@ -98,22 +99,81 @@ public class FontScaleConverterFactory { 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 + // Also, fontScale==0 should not have a curve either. + // And ignore negative font scales; that's just silly. return null; } FontScaleConverter lookupTable = get(fontScale); - // TODO(b/247861716): interpolate between two tables when null + if (lookupTable != null) { + return lookupTable; + } - return lookupTable; + // Didn't find an exact match: interpolate between two existing tables + final int index = LOOKUP_TABLES.indexOfKey(getKey(fontScale)); + if (index >= 0) { + // This should never happen, should have been covered by get() above. + return LOOKUP_TABLES.valueAt(index); + } + // Didn't find an exact match: interpolate between two existing tables + final int lowerIndex = -(index + 1) - 1; + final int higherIndex = lowerIndex + 1; + if (lowerIndex < 0 || higherIndex >= LOOKUP_TABLES.size()) { + // We have gone beyond our bounds and have nothing to interpolate between. Just give + // them a straight linear table instead. + // This works because when FontScaleConverter encounters a size beyond its bounds, it + // calculates a linear fontScale factor using the ratio of the last element pair. + return new FontScaleConverter(new float[] {1f}, new float[] {fontScale}); + } else { + float startScale = getScaleFromKey(LOOKUP_TABLES.keyAt(lowerIndex)); + float endScale = getScaleFromKey(LOOKUP_TABLES.keyAt(higherIndex)); + float interpolationPoint = MathUtils.constrainedMap( + /* rangeMin= */ 0f, + /* rangeMax= */ 1f, + startScale, + endScale, + fontScale + ); + return createInterpolatedTableBetween( + LOOKUP_TABLES.valueAt(lowerIndex), + LOOKUP_TABLES.valueAt(higherIndex), + interpolationPoint); + } + } + + @NonNull + private static FontScaleConverter createInterpolatedTableBetween( + FontScaleConverter start, + FontScaleConverter end, + float interpolationPoint + ) { + float[] commonSpSizes = new float[] { 8f, 10f, 12f, 14f, 18f, 20f, 24f, 30f, 100f}; + float[] dpInterpolated = new float[commonSpSizes.length]; + + for (int i = 0; i < commonSpSizes.length; i++) { + float sp = commonSpSizes[i]; + float startDp = start.convertSpToDp(sp); + float endDp = end.convertSpToDp(sp); + dpInterpolated[i] = MathUtils.lerp(startDp, endDp, interpolationPoint); + } + + return new FontScaleConverter(commonSpSizes, dpInterpolated); + } + + private static int getKey(float fontScale) { + return (int) (fontScale * SCALE_KEY_MULTIPLIER); + } + + private static float getScaleFromKey(int key) { + return (float) key / SCALE_KEY_MULTIPLIER; } private static void put(float scaleKey, @NonNull FontScaleConverter fontScaleConverter) { - LOOKUP_TABLES.put((int) (scaleKey * SCALE_KEY_MULTIPLIER), fontScaleConverter); + LOOKUP_TABLES.put(getKey(scaleKey), fontScaleConverter); } @Nullable private static FontScaleConverter get(float scaleKey) { - return LOOKUP_TABLES.get((int) (scaleKey * SCALE_KEY_MULTIPLIER)); + return LOOKUP_TABLES.get(getKey(scaleKey)); } } diff --git a/core/tests/coretests/src/android/content/res/FontScaleConverterFactoryTest.kt b/core/tests/coretests/src/android/content/res/FontScaleConverterFactoryTest.kt index 249e2468d87e0..0c4e5e9ab892d 100644 --- a/core/tests/coretests/src/android/content/res/FontScaleConverterFactoryTest.kt +++ b/core/tests/coretests/src/android/content/res/FontScaleConverterFactoryTest.kt @@ -43,14 +43,56 @@ class FontScaleConverterFactoryTest { assertThat(table.convertSpToDp(0F)).isWithin(CONVERSION_TOLERANCE).of(0f) } - @SmallTest - fun missingLookupTableReturnsNull() { - assertThat(FontScaleConverterFactory.forScale(3F)).isNull() + @LargeTest + @Test + fun missingLookupTablePastEnd_returnsLinear() { + val table = FontScaleConverterFactory.forScale(3F)!! + generateSequenceOfFractions(-10000f..10000f, step = 0.01f) + .map { + assertThat(table.convertSpToDp(it)).isWithin(CONVERSION_TOLERANCE).of(it * 3f) + } + assertThat(table.convertSpToDp(1F)).isWithin(CONVERSION_TOLERANCE).of(3f) + assertThat(table.convertSpToDp(8F)).isWithin(CONVERSION_TOLERANCE).of(24f) + assertThat(table.convertSpToDp(10F)).isWithin(CONVERSION_TOLERANCE).of(30f) + assertThat(table.convertSpToDp(5F)).isWithin(CONVERSION_TOLERANCE).of(15f) + assertThat(table.convertSpToDp(0F)).isWithin(CONVERSION_TOLERANCE).of(0f) + assertThat(table.convertSpToDp(50F)).isWithin(CONVERSION_TOLERANCE).of(150f) + assertThat(table.convertSpToDp(100F)).isWithin(CONVERSION_TOLERANCE).of(300f) } @SmallTest - fun missingLookupTable105ReturnsNull() { - assertThat(FontScaleConverterFactory.forScale(1.05F)).isNull() + fun missingLookupTable110_returnsInterpolated() { + val table = FontScaleConverterFactory.forScale(1.1F)!! + + assertThat(table.convertSpToDp(1F)).isWithin(CONVERSION_TOLERANCE).of(1.1f) + assertThat(table.convertSpToDp(8F)).isWithin(CONVERSION_TOLERANCE).of(8f * 1.1f) + assertThat(table.convertSpToDp(10F)).isWithin(CONVERSION_TOLERANCE).of(11f) + assertThat(table.convertSpToDp(5F)).isWithin(CONVERSION_TOLERANCE).of(5f * 1.1f) + assertThat(table.convertSpToDp(0F)).isWithin(CONVERSION_TOLERANCE).of(0f) + assertThat(table.convertSpToDp(50F)).isLessThan(50f * 1.1f) + assertThat(table.convertSpToDp(100F)).isLessThan(100f * 1.1f) + } + + @Test + fun missingLookupTable199_returnsInterpolated() { + val table = FontScaleConverterFactory.forScale(1.9999F)!! + assertThat(table.convertSpToDp(1F)).isWithin(CONVERSION_TOLERANCE).of(2f) + assertThat(table.convertSpToDp(8F)).isWithin(CONVERSION_TOLERANCE).of(16f) + assertThat(table.convertSpToDp(10F)).isWithin(CONVERSION_TOLERANCE).of(20f) + assertThat(table.convertSpToDp(5F)).isWithin(CONVERSION_TOLERANCE).of(10f) + assertThat(table.convertSpToDp(0F)).isWithin(CONVERSION_TOLERANCE).of(0f) + } + + @Test + fun missingLookupTable160_returnsInterpolated() { + val table = FontScaleConverterFactory.forScale(1.6F)!! + assertThat(table.convertSpToDp(1F)).isWithin(CONVERSION_TOLERANCE).of(1f * 1.6F) + assertThat(table.convertSpToDp(8F)).isWithin(CONVERSION_TOLERANCE).of(8f * 1.6F) + assertThat(table.convertSpToDp(10F)).isWithin(CONVERSION_TOLERANCE).of(10f * 1.6F) + assertThat(table.convertSpToDp(20F)).isLessThan(20f * 1.6F) + assertThat(table.convertSpToDp(100F)).isLessThan(100f * 1.6F) + assertThat(table.convertSpToDp(5F)).isWithin(CONVERSION_TOLERANCE).of(5f * 1.6F) + assertThat(table.convertSpToDp(0F)).isWithin(CONVERSION_TOLERANCE).of(0f) } @SmallTest @@ -83,7 +125,7 @@ class FontScaleConverterFactoryTest { @Test fun allFeasibleScalesAndConversionsDoNotCrash() { generateSequenceOfFractions(-10f..10f, step = 0.01f) - .mapNotNull{ FontScaleConverterFactory.forScale(it) } + .mapNotNull{ FontScaleConverterFactory.forScale(it) }!! .flatMap{ table -> generateSequenceOfFractions(-2000f..2000f, step = 0.01f) .map{ Pair(table, it) }