From bae1102a51a5c4a88bb3edac7c540736b8c7522b Mon Sep 17 00:00:00 2001
From: Tyler Freeman <fuego@google.com>
Date: Thu, 1 Dec 2022 13:29:05 -0800
Subject: [PATCH] fix(non linear font scaling): fix crash with certain negative
 SP values

The crash was due to some old logic that was looking for near matches.
Now we only return exact matches, and let the interpolation deal with
near matches.

Add test to make sure it doesn't crash at any value passed in.

Test: atest FrameworksCoreTests:android.content.res.FontScaleConverterTest

Bug: b/260984829

Change-Id: Ic90d1c5b48862e4f78b90be5926c9ce6d468acf9
---
 .../content/res/FontScaleConverter.java       | 12 ++--
 .../res/FontScaleConverterFactoryTest.kt      | 67 +++++++++++++++++++
 2 files changed, 71 insertions(+), 8 deletions(-)

diff --git a/core/java/android/content/res/FontScaleConverter.java b/core/java/android/content/res/FontScaleConverter.java
index c7fdb16682e3d..457225d26610c 100644
--- a/core/java/android/content/res/FontScaleConverter.java
+++ b/core/java/android/content/res/FontScaleConverter.java
@@ -36,11 +36,6 @@ import java.util.Arrays;
  * @hide
  */
 public class FontScaleConverter {
-    /**
-     * How close the given SP should be to a canonical SP in the array before they are considered
-     * the same for lookup purposes.
-     */
-    private static final float THRESHOLD_FOR_MATCHING_SP = 0.02f;
 
     @VisibleForTesting
     final float[] mFromSpValues;
@@ -78,10 +73,11 @@ public class FontScaleConverter {
     public float convertSpToDp(float sp) {
         final float spPositive = Math.abs(sp);
         // TODO(b/247861374): find a match at a higher index?
-        final int spRounded = Math.round(spPositive);
         final float sign = Math.signum(sp);
-        final int index = Arrays.binarySearch(mFromSpValues, spRounded);
-        if (index >= 0 && Math.abs(spRounded - spPositive) < THRESHOLD_FOR_MATCHING_SP) {
+        // We search for exact matches only, even if it's just a little off. The interpolation will
+        // handle any non-exact matches.
+        final int index = Arrays.binarySearch(mFromSpValues, spPositive);
+        if (index >= 0) {
             // exact match, return the matching dp
             return sign * mToDpValues[index];
         } else {
diff --git a/core/tests/coretests/src/android/content/res/FontScaleConverterFactoryTest.kt b/core/tests/coretests/src/android/content/res/FontScaleConverterFactoryTest.kt
index cfca0375bb962..625c318d9efd7 100644
--- a/core/tests/coretests/src/android/content/res/FontScaleConverterFactoryTest.kt
+++ b/core/tests/coretests/src/android/content/res/FontScaleConverterFactoryTest.kt
@@ -18,8 +18,12 @@ package android.content.res
 
 import androidx.core.util.forEach
 import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.test.filters.LargeTest
 import androidx.test.filters.SmallTest
 import com.google.common.truth.Truth.assertThat
+import com.google.common.truth.Truth.assertWithMessage
+import kotlin.math.ceil
+import kotlin.math.floor
 import org.junit.Test
 import org.junit.runner.RunWith
 
@@ -72,7 +76,70 @@ class FontScaleConverterFactoryTest {
         }
     }
 
+    @LargeTest
+    @Test
+    fun allFeasibleScalesAndConversionsDoNotCrash() {
+        generateSequenceOfFractions(-10000f..10000f, step = 0.01f)
+            .mapNotNull{ FontScaleConverterFactory.forScale(it) }
+            .flatMap{ table ->
+                generateSequenceOfFractions(-10000f..10000f, step = 0.01f)
+                    .map{ Pair(table, it) }
+            }
+            .forEach { (table, sp) ->
+                try {
+                    assertWithMessage(
+                        "convertSpToDp(%s) on table: %s",
+                        sp.toString(),
+                        table.toString()
+                    )
+                        .that(table.convertSpToDp(sp))
+                        .isFinite()
+                } catch (e: Exception) {
+                    throw AssertionError("Exception during convertSpToDp($sp) on table: $table", e)
+                }
+            }
+    }
+
+    @Test
+    fun testGenerateSequenceOfFractions() {
+        val fractions = generateSequenceOfFractions(-1000f..1000f, step = 0.1f)
+            .toList()
+        fractions.forEach {
+            assertThat(it).isAtLeast(-1000f)
+            assertThat(it).isAtMost(1000f)
+        }
+
+        assertThat(fractions).isInStrictOrder()
+        assertThat(fractions).hasSize(1000 * 2 * 10 + 1) // Don't forget the 0 in the middle!
+
+        assertThat(fractions).contains(100f)
+        assertThat(fractions).contains(500.1f)
+        assertThat(fractions).contains(500.2f)
+        assertThat(fractions).contains(0.2f)
+        assertThat(fractions).contains(0f)
+        assertThat(fractions).contains(-10f)
+        assertThat(fractions).contains(-10f)
+        assertThat(fractions).contains(-10.3f)
+
+        assertThat(fractions).doesNotContain(-10.31f)
+        assertThat(fractions).doesNotContain(0.35f)
+        assertThat(fractions).doesNotContain(0.31f)
+        assertThat(fractions).doesNotContain(-.35f)
+    }
+
     companion object {
         private const val CONVERSION_TOLERANCE = 0.05f
     }
 }
+
+fun generateSequenceOfFractions(
+    range: ClosedFloatingPointRange<Float>,
+    step: Float
+): Sequence<Float> {
+    val multiplier = 1f / step
+    val start = floor(range.start * multiplier).toInt()
+    val endInclusive = ceil(range.endInclusive * multiplier).toInt()
+    return generateSequence(start) { it + 1 }
+        .takeWhile { it <= endInclusive }
+        .map{ it.toFloat() / multiplier }
+}