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

Commit 6dbd9626 authored by Tyler Freeman's avatar Tyler Freeman
Browse files

fix(non linear font scaling): make FontScaleConverterFactory thread-safe

We use the same technique as CopyOnWriteArrayList to protect the writes
only. Reads can be a bit out of date since they are only used for
caching.

Bug: b/239736383
Flag: android.content.res.font_scale_converter_public
Test: atest FrameworksCoreTests:android.content.res.FontScaleConverterFactoryTest
Change-Id: I2cfefcf7a5c6f56294e87d4c35afc8499c2e7c02
parent a407ee86
Loading
Loading
Loading
Loading
+2 −2
Original line number Diff line number Diff line
@@ -13584,8 +13584,8 @@ package android.content.res {
  }
  @FlaggedApi("android.content.res.font_scale_converter_public") public class FontScaleConverterFactory {
    method @FlaggedApi("android.content.res.font_scale_converter_public") @Nullable public static android.content.res.FontScaleConverter forScale(float);
    method @FlaggedApi("android.content.res.font_scale_converter_public") public static boolean isNonLinearFontScalingActive(float);
    method @FlaggedApi("android.content.res.font_scale_converter_public") @AnyThread @Nullable public static android.content.res.FontScaleConverter forScale(float);
    method @FlaggedApi("android.content.res.font_scale_converter_public") @AnyThread public static boolean isNonLinearFontScalingActive(float);
  }
  public class ObbInfo implements android.os.Parcelable {
+88 −56
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@

package android.content.res;

import android.annotation.AnyThread;
import android.annotation.FlaggedApi;
import android.annotation.NonNull;
import android.annotation.Nullable;
@@ -37,15 +38,26 @@ public class FontScaleConverterFactory {
    private static final float SCALE_KEY_MULTIPLIER = 100f;

    /** @hide */
    // GuardedBy("LOOKUP_TABLES_WRITE_LOCK") but only for writes!
    @VisibleForTesting
    public static final SparseArray<FontScaleConverter> LOOKUP_TABLES = new SparseArray<>();
    @NonNull
    public static volatile SparseArray<FontScaleConverter> sLookupTables = new SparseArray<>();

    /**
     * This is a write lock only! We don't care about synchronization on reads; they can be a bit
     * out of date. But all writes have to be atomic, so we use this similar to a
     * CopyOnWriteArrayList.
     */
    private static final Object LOOKUP_TABLES_WRITE_LOCK = new Object();

    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.
        put(
        synchronized (LOOKUP_TABLES_WRITE_LOCK) {
            putInto(
                    sLookupTables,
                    /* scaleKey= */ 1.15f,
                    new FontScaleConverterImpl(
                            /* fromSp= */
@@ -54,7 +66,8 @@ public class FontScaleConverterFactory {
                            new float[] { 9.2f, 11.5f, 13.8f, 16.4f, 19.8f, 21.8f, 25.2f,   30f,  100})
            );

        put(
            putInto(
                    sLookupTables,
                    /* scaleKey= */ 1.3f,
                    new FontScaleConverterImpl(
                            /* fromSp= */
@@ -63,7 +76,8 @@ public class FontScaleConverterFactory {
                            new float[] {10.4f,   13f, 15.6f, 18.8f, 21.6f, 23.6f, 26.4f,   30f,  100})
            );

        put(
            putInto(
                    sLookupTables,
                    /* scaleKey= */ 1.5f,
                    new FontScaleConverterImpl(
                            /* fromSp= */
@@ -72,7 +86,8 @@ public class FontScaleConverterFactory {
                            new float[] {  12f,   15f,   18f,   22f,   24f,   26f,   28f,   30f,  100})
            );

        put(
            putInto(
                    sLookupTables,
                    /* scaleKey= */ 1.8f,
                    new FontScaleConverterImpl(
                            /* fromSp= */
@@ -81,7 +96,8 @@ public class FontScaleConverterFactory {
                            new float[] {14.4f,   18f, 21.6f, 24.4f, 27.6f, 30.8f, 32.8f, 34.8f,  100})
            );

        put(
            putInto(
                    sLookupTables,
                    /* scaleKey= */ 2f,
                    new FontScaleConverterImpl(
                            /* fromSp= */
@@ -89,8 +105,9 @@ public class FontScaleConverterFactory {
                            /* toDp=   */
                            new float[] {  16f,   20f,   24f,   26f,   30f,   34f,   36f,   38f,  100})
            );
        }

        sMinScaleBeforeCurvesApplied = getScaleFromKey(LOOKUP_TABLES.keyAt(0)) - 0.02f;
        sMinScaleBeforeCurvesApplied = getScaleFromKey(sLookupTables.keyAt(0)) - 0.02f;
        if (sMinScaleBeforeCurvesApplied <= 1.0f) {
            throw new IllegalStateException(
                    "You should only apply non-linear scaling to font scales > 1"
@@ -108,6 +125,7 @@ public class FontScaleConverterFactory {
     * <code>isNonLinearFontScalingActive(getResources().getConfiguration().fontScale)</code>
     */
    @FlaggedApi(Flags.FLAG_FONT_SCALE_CONVERTER_PUBLIC)
    @AnyThread
    public static boolean isNonLinearFontScalingActive(float fontScale) {
        return fontScale >= sMinScaleBeforeCurvesApplied;
    }
@@ -121,6 +139,7 @@ public class FontScaleConverterFactory {
     */
    @FlaggedApi(Flags.FLAG_FONT_SCALE_CONVERTER_PUBLIC)
    @Nullable
    @AnyThread
    public static FontScaleConverter forScale(float fontScale) {
        if (!isNonLinearFontScalingActive(fontScale)) {
            return null;
@@ -132,15 +151,15 @@ public class FontScaleConverterFactory {
        }

        // Didn't find an exact match: interpolate between two existing tables
        final int index = LOOKUP_TABLES.indexOfKey(getKey(fontScale));
        final int index = sLookupTables.indexOfKey(getKey(fontScale));
        if (index >= 0) {
            // This should never happen, should have been covered by get() above.
            return LOOKUP_TABLES.valueAt(index);
            return sLookupTables.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()) {
        if (lowerIndex < 0 || higherIndex >= sLookupTables.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
@@ -157,8 +176,8 @@ public class FontScaleConverterFactory {

            return converter;
        } else {
            float startScale = getScaleFromKey(LOOKUP_TABLES.keyAt(lowerIndex));
            float endScale = getScaleFromKey(LOOKUP_TABLES.keyAt(higherIndex));
            float startScale = getScaleFromKey(sLookupTables.keyAt(lowerIndex));
            float endScale = getScaleFromKey(sLookupTables.keyAt(higherIndex));
            float interpolationPoint = MathUtils.constrainedMap(
                    /* rangeMin= */ 0f,
                    /* rangeMax= */ 1f,
@@ -167,8 +186,8 @@ public class FontScaleConverterFactory {
                    fontScale
            );
            FontScaleConverter converter = createInterpolatedTableBetween(
                    LOOKUP_TABLES.valueAt(lowerIndex),
                    LOOKUP_TABLES.valueAt(higherIndex),
                    sLookupTables.valueAt(lowerIndex),
                    sLookupTables.valueAt(higherIndex),
                    interpolationPoint
            );

@@ -209,11 +228,24 @@ public class FontScaleConverterFactory {
    }

    private static void put(float scaleKey, @NonNull FontScaleConverter fontScaleConverter) {
        LOOKUP_TABLES.put(getKey(scaleKey), fontScaleConverter);
        // Dollar-store CopyOnWriteSparseArray, since this is the only write op we need.
        synchronized (LOOKUP_TABLES_WRITE_LOCK) {
            var newTable = sLookupTables.clone();
            putInto(newTable, scaleKey, fontScaleConverter);
            sLookupTables = newTable;
        }
    }

    private static void putInto(
            SparseArray<FontScaleConverter> table,
            float scaleKey,
            @NonNull FontScaleConverter fontScaleConverter
    ) {
        table.put(getKey(scaleKey), fontScaleConverter);
    }

    @Nullable
    private static FontScaleConverter get(float scaleKey) {
        return LOOKUP_TABLES.get(getKey(scaleKey));
        return sLookupTables.get(getKey(scaleKey));
    }
}
+7 −7
Original line number Diff line number Diff line
@@ -99,10 +99,10 @@ class FontScaleConverterFactoryTest {
    fun missingLookupTable_cachesInterpolated() {
        val table = FontScaleConverterFactory.forScale(1.6F)!!

        assertThat(FontScaleConverterFactory.LOOKUP_TABLES.contains((1.6F * 100).toInt())).isTrue()
        assertThat(FontScaleConverterFactory.sLookupTables.contains((1.6F * 100).toInt())).isTrue()
        // Double check known existing values
        assertThat(FontScaleConverterFactory.LOOKUP_TABLES.contains((1.5F * 100).toInt())).isTrue()
        assertThat(FontScaleConverterFactory.LOOKUP_TABLES.contains((1.7F * 100).toInt())).isFalse()
        assertThat(FontScaleConverterFactory.sLookupTables.contains((1.5F * 100).toInt())).isTrue()
        assertThat(FontScaleConverterFactory.sLookupTables.contains((1.7F * 100).toInt())).isFalse()
    }

    @Test
@@ -110,10 +110,10 @@ class FontScaleConverterFactoryTest {
    fun missingLookupTablePastEnd_cachesLinear() {
        val table = FontScaleConverterFactory.forScale(3F)!!

        assertThat(FontScaleConverterFactory.LOOKUP_TABLES.contains((3F * 100).toInt())).isTrue()
        assertThat(FontScaleConverterFactory.sLookupTables.contains((3F * 100).toInt())).isTrue()
        // Double check known existing values
        assertThat(FontScaleConverterFactory.LOOKUP_TABLES.contains((1.5F * 100).toInt())).isTrue()
        assertThat(FontScaleConverterFactory.LOOKUP_TABLES.contains((1.7F * 100).toInt())).isFalse()
        assertThat(FontScaleConverterFactory.sLookupTables.contains((1.5F * 100).toInt())).isTrue()
        assertThat(FontScaleConverterFactory.sLookupTables.contains((1.7F * 100).toInt())).isFalse()
    }

    @SmallTest
@@ -134,7 +134,7 @@ class FontScaleConverterFactoryTest {
    @SmallTest
    @Test
    fun tablesMatchAndAreMonotonicallyIncreasing() {
        FontScaleConverterFactory.LOOKUP_TABLES.forEach { _, lookupTable ->
        FontScaleConverterFactory.sLookupTables.forEach { _, lookupTable ->
            if (lookupTable !is FontScaleConverterImpl) {
                throw IllegalStateException("Didn't return a FontScaleConverterImpl")
            }