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
diff --git a/core/api/current.txt b/core/api/current.txt
index 4fd8cf0..7fffccd3 100644
--- a/core/api/current.txt
+++ b/core/api/current.txt
@@ -13584,8 +13584,8 @@
}
@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 {
diff --git a/core/java/android/content/res/FontScaleConverterFactory.java b/core/java/android/content/res/FontScaleConverterFactory.java
index 05a1dd4..5d31cc0 100644
--- a/core/java/android/content/res/FontScaleConverterFactory.java
+++ b/core/java/android/content/res/FontScaleConverterFactory.java
@@ -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,60 +38,76 @@
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(
- /* scaleKey= */ 1.15f,
- new FontScaleConverterImpl(
- /* fromSp= */
- new float[] { 8f, 10f, 12f, 14f, 18f, 20f, 24f, 30f, 100},
- /* toDp= */
- new float[] { 9.2f, 11.5f, 13.8f, 16.4f, 19.8f, 21.8f, 25.2f, 30f, 100})
- );
+ synchronized (LOOKUP_TABLES_WRITE_LOCK) {
+ putInto(
+ sLookupTables,
+ /* scaleKey= */ 1.15f,
+ new FontScaleConverterImpl(
+ /* fromSp= */
+ new float[] { 8f, 10f, 12f, 14f, 18f, 20f, 24f, 30f, 100},
+ /* toDp= */
+ new float[] { 9.2f, 11.5f, 13.8f, 16.4f, 19.8f, 21.8f, 25.2f, 30f, 100})
+ );
- put(
- /* scaleKey= */ 1.3f,
- new FontScaleConverterImpl(
- /* fromSp= */
- new float[] { 8f, 10f, 12f, 14f, 18f, 20f, 24f, 30f, 100},
- /* toDp= */
- new float[] {10.4f, 13f, 15.6f, 18.8f, 21.6f, 23.6f, 26.4f, 30f, 100})
- );
+ putInto(
+ sLookupTables,
+ /* scaleKey= */ 1.3f,
+ new FontScaleConverterImpl(
+ /* fromSp= */
+ new float[] { 8f, 10f, 12f, 14f, 18f, 20f, 24f, 30f, 100},
+ /* toDp= */
+ new float[] {10.4f, 13f, 15.6f, 18.8f, 21.6f, 23.6f, 26.4f, 30f, 100})
+ );
- put(
- /* scaleKey= */ 1.5f,
- new FontScaleConverterImpl(
- /* fromSp= */
- new float[] { 8f, 10f, 12f, 14f, 18f, 20f, 24f, 30f, 100},
- /* toDp= */
- new float[] { 12f, 15f, 18f, 22f, 24f, 26f, 28f, 30f, 100})
- );
+ putInto(
+ sLookupTables,
+ /* scaleKey= */ 1.5f,
+ new FontScaleConverterImpl(
+ /* fromSp= */
+ new float[] { 8f, 10f, 12f, 14f, 18f, 20f, 24f, 30f, 100},
+ /* toDp= */
+ new float[] { 12f, 15f, 18f, 22f, 24f, 26f, 28f, 30f, 100})
+ );
- put(
- /* scaleKey= */ 1.8f,
- new FontScaleConverterImpl(
- /* fromSp= */
- new float[] { 8f, 10f, 12f, 14f, 18f, 20f, 24f, 30f, 100},
- /* toDp= */
- new float[] {14.4f, 18f, 21.6f, 24.4f, 27.6f, 30.8f, 32.8f, 34.8f, 100})
- );
+ putInto(
+ sLookupTables,
+ /* scaleKey= */ 1.8f,
+ new FontScaleConverterImpl(
+ /* fromSp= */
+ new float[] { 8f, 10f, 12f, 14f, 18f, 20f, 24f, 30f, 100},
+ /* toDp= */
+ new float[] {14.4f, 18f, 21.6f, 24.4f, 27.6f, 30.8f, 32.8f, 34.8f, 100})
+ );
- put(
- /* scaleKey= */ 2f,
- new FontScaleConverterImpl(
- /* fromSp= */
- new float[] { 8f, 10f, 12f, 14f, 18f, 20f, 24f, 30f, 100},
- /* toDp= */
- new float[] { 16f, 20f, 24f, 26f, 30f, 34f, 36f, 38f, 100})
- );
+ putInto(
+ sLookupTables,
+ /* scaleKey= */ 2f,
+ new FontScaleConverterImpl(
+ /* fromSp= */
+ new float[] { 8f, 10f, 12f, 14f, 18f, 20f, 24f, 30f, 100},
+ /* 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 @@
* <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 @@
*/
@FlaggedApi(Flags.FLAG_FONT_SCALE_CONVERTER_PUBLIC)
@Nullable
+ @AnyThread
public static FontScaleConverter forScale(float fontScale) {
if (!isNonLinearFontScalingActive(fontScale)) {
return null;
@@ -132,15 +151,15 @@
}
// 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 @@
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 @@
fontScale
);
FontScaleConverter converter = createInterpolatedTableBetween(
- LOOKUP_TABLES.valueAt(lowerIndex),
- LOOKUP_TABLES.valueAt(higherIndex),
+ sLookupTables.valueAt(lowerIndex),
+ sLookupTables.valueAt(higherIndex),
interpolationPoint
);
@@ -209,11 +228,24 @@
}
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));
}
}
diff --git a/core/tests/coretests/src/android/content/res/FontScaleConverterFactoryTest.kt b/core/tests/coretests/src/android/content/res/FontScaleConverterFactoryTest.kt
index 6c73e92..8308e7c 100644
--- a/core/tests/coretests/src/android/content/res/FontScaleConverterFactoryTest.kt
+++ b/core/tests/coretests/src/android/content/res/FontScaleConverterFactoryTest.kt
@@ -99,10 +99,10 @@
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 @@
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 @@
@SmallTest
@Test
fun tablesMatchAndAreMonotonicallyIncreasing() {
- FontScaleConverterFactory.LOOKUP_TABLES.forEach { _, lookupTable ->
+ FontScaleConverterFactory.sLookupTables.forEach { _, lookupTable ->
if (lookupTable !is FontScaleConverterImpl) {
throw IllegalStateException("Didn't return a FontScaleConverterImpl")
}