Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit bae1102a authored by Tyler Freeman's avatar Tyler Freeman
Browse files

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
parent da9efd89

core/java/android/content/res/FontScaleConverter.java

+4 −8
Original line number Diff line number Diff line
@@ -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 {

core/tests/coretests/src/android/content/res/FontScaleConverterFactoryTest.kt

+67 −0
Original line number Diff line number Diff line
@@ -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 }

}