Revert "Cache the variation instance of Typeface"
Revert submission 28490963-font_var_typeface_cache
Reason for revert: DroidMonitor. Potential culprit for b/358347869 - verifying through ABTD before revert submission. This is part of the standard investigation process, and does not mean your CL will be reverted.
Reverted changes: /q/submissionid:28490963-font_var_typeface_cache
Change-Id: Ia3e291421dc8fe424245a3ed64c8b88bc1f8dc84
diff --git a/apct-tests/perftests/core/src/android/text/VariableFontPerfTest.java b/apct-tests/perftests/core/src/android/text/VariableFontPerfTest.java
index c34936f..fbe67a4 100644
--- a/apct-tests/perftests/core/src/android/text/VariableFontPerfTest.java
+++ b/apct-tests/perftests/core/src/android/text/VariableFontPerfTest.java
@@ -19,7 +19,6 @@
import android.graphics.Paint;
import android.graphics.RecordingCanvas;
import android.graphics.RenderNode;
-import android.graphics.Typeface;
import android.perftests.utils.BenchmarkState;
import android.perftests.utils.PerfStatusReporter;
@@ -121,34 +120,13 @@
public void testSetFontVariationSettings() {
final BenchmarkState state = mPerfStatusReporter.getBenchmarkState();
final Paint paint = new Paint(PAINT);
+ final Random random = new Random(0);
while (state.keepRunning()) {
state.pauseTiming();
- paint.setTypeface(null);
- paint.setFontVariationSettings(null);
- Typeface.clearTypefaceCachesForTestingPurpose();
+ int weight = random.nextInt(1000);
state.resumeTiming();
- paint.setFontVariationSettings("'wght' 450");
+ paint.setFontVariationSettings("'wght' " + weight);
}
- Typeface.clearTypefaceCachesForTestingPurpose();
}
-
- @Test
- public void testSetFontVariationSettings_Cached() {
- final BenchmarkState state = mPerfStatusReporter.getBenchmarkState();
- final Paint paint = new Paint(PAINT);
- Typeface.clearTypefaceCachesForTestingPurpose();
-
- while (state.keepRunning()) {
- state.pauseTiming();
- paint.setTypeface(null);
- paint.setFontVariationSettings(null);
- state.resumeTiming();
-
- paint.setFontVariationSettings("'wght' 450");
- }
-
- Typeface.clearTypefaceCachesForTestingPurpose();
- }
-
}
diff --git a/core/java/android/text/flags/flags.aconfig b/core/java/android/text/flags/flags.aconfig
index 4d176f2..bb3f6c9 100644
--- a/core/java/android/text/flags/flags.aconfig
+++ b/core/java/android/text/flags/flags.aconfig
@@ -277,13 +277,3 @@
purpose: PURPOSE_BUGFIX
}
}
-
-flag {
- name: "typeface_cache_for_var_settings"
- namespace: "text"
- description: "Cache Typeface instance for font variation settings."
- bug: "355462362"
- metadata {
- purpose: PURPOSE_BUGFIX
- }
-}
\ No newline at end of file
diff --git a/core/tests/coretests/src/android/graphics/PaintFontVariationTest.java b/core/tests/coretests/src/android/graphics/PaintFontVariationTest.java
deleted file mode 100644
index 8a54e5b..0000000
--- a/core/tests/coretests/src/android/graphics/PaintFontVariationTest.java
+++ /dev/null
@@ -1,74 +0,0 @@
-/*
- * Copyright (C) 2015 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package android.graphics;
-
-import static com.google.common.truth.Truth.assertThat;
-
-import android.platform.test.annotations.RequiresFlagsEnabled;
-import android.platform.test.flag.junit.CheckFlagsRule;
-import android.platform.test.flag.junit.DeviceFlagsValueProvider;
-import android.test.InstrumentationTestCase;
-
-import androidx.test.filters.SmallTest;
-import androidx.test.runner.AndroidJUnit4;
-
-import com.android.text.flags.Flags;
-
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-
-/**
- * PaintTest tests {@link Paint}.
- */
-@SmallTest
-@RunWith(AndroidJUnit4.class)
-public class PaintFontVariationTest extends InstrumentationTestCase {
- @Rule
- public final CheckFlagsRule mCheckFlagsRule = DeviceFlagsValueProvider.createCheckFlagsRule();
-
- @RequiresFlagsEnabled(Flags.FLAG_TYPEFACE_CACHE_FOR_VAR_SETTINGS)
- @Test
- public void testDerivedFromSameTypeface() {
- final Paint p = new Paint();
-
- p.setTypeface(Typeface.SANS_SERIF);
- assertThat(p.setFontVariationSettings("'wght' 450")).isTrue();
- Typeface first = p.getTypeface();
-
- p.setTypeface(Typeface.SANS_SERIF);
- assertThat(p.setFontVariationSettings("'wght' 480")).isTrue();
- Typeface second = p.getTypeface();
-
- assertThat(first.getDerivedFrom()).isSameInstanceAs(second.getDerivedFrom());
- }
-
- @RequiresFlagsEnabled(Flags.FLAG_TYPEFACE_CACHE_FOR_VAR_SETTINGS)
- @Test
- public void testDerivedFromChained() {
- final Paint p = new Paint();
-
- p.setTypeface(Typeface.SANS_SERIF);
- assertThat(p.setFontVariationSettings("'wght' 450")).isTrue();
- Typeface first = p.getTypeface();
-
- assertThat(p.setFontVariationSettings("'wght' 480")).isTrue();
- Typeface second = p.getTypeface();
-
- assertThat(first.getDerivedFrom()).isSameInstanceAs(second.getDerivedFrom());
- }
-}
diff --git a/core/tests/coretests/src/android/graphics/PaintTest.java b/core/tests/coretests/src/android/graphics/PaintTest.java
index 878ba70..0dec756 100644
--- a/core/tests/coretests/src/android/graphics/PaintTest.java
+++ b/core/tests/coretests/src/android/graphics/PaintTest.java
@@ -16,22 +16,13 @@
package android.graphics;
-import static com.google.common.truth.Truth.assertThat;
-
import static org.junit.Assert.assertNotEquals;
-import android.platform.test.annotations.RequiresFlagsEnabled;
-import android.platform.test.flag.junit.CheckFlagsRule;
-import android.platform.test.flag.junit.DeviceFlagsValueProvider;
import android.test.InstrumentationTestCase;
import android.text.TextUtils;
import androidx.test.filters.SmallTest;
-import com.android.text.flags.Flags;
-
-import org.junit.Rule;
-
import java.util.Arrays;
import java.util.HashSet;
@@ -39,9 +30,6 @@
* PaintTest tests {@link Paint}.
*/
public class PaintTest extends InstrumentationTestCase {
- @Rule
- public final CheckFlagsRule mCheckFlagsRule = DeviceFlagsValueProvider.createCheckFlagsRule();
-
private static final String FONT_PATH = "fonts/HintedAdvanceWidthTest-Regular.ttf";
static void assertEquals(String message, float[] expected, float[] actual) {
@@ -415,33 +403,4 @@
assertEquals(6, getClusterCount(p, rtlStr + ltrStr));
assertEquals(9, getClusterCount(p, ltrStr + rtlStr + ltrStr));
}
-
- @RequiresFlagsEnabled(Flags.FLAG_TYPEFACE_CACHE_FOR_VAR_SETTINGS)
- public void testDerivedFromSameTypeface() {
- final Paint p = new Paint();
-
- p.setTypeface(Typeface.SANS_SERIF);
- assertThat(p.setFontVariationSettings("'wght' 450")).isTrue();
- Typeface first = p.getTypeface();
-
- p.setTypeface(Typeface.SANS_SERIF);
- assertThat(p.setFontVariationSettings("'wght' 480")).isTrue();
- Typeface second = p.getTypeface();
-
- assertThat(first.getDerivedFrom()).isSameInstanceAs(second.getDerivedFrom());
- }
-
- @RequiresFlagsEnabled(Flags.FLAG_TYPEFACE_CACHE_FOR_VAR_SETTINGS)
- public void testDerivedFromChained() {
- final Paint p = new Paint();
-
- p.setTypeface(Typeface.SANS_SERIF);
- assertThat(p.setFontVariationSettings("'wght' 450")).isTrue();
- Typeface first = p.getTypeface();
-
- assertThat(p.setFontVariationSettings("'wght' 480")).isTrue();
- Typeface second = p.getTypeface();
-
- assertThat(first.getDerivedFrom()).isSameInstanceAs(second.getDerivedFrom());
- }
}
diff --git a/graphics/java/android/graphics/Typeface.java b/graphics/java/android/graphics/Typeface.java
index 889a7785..fd78816 100644
--- a/graphics/java/android/graphics/Typeface.java
+++ b/graphics/java/android/graphics/Typeface.java
@@ -56,7 +56,6 @@
import com.android.internal.annotations.GuardedBy;
import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.util.Preconditions;
-import com.android.text.flags.Flags;
import dalvik.annotation.optimization.CriticalNative;
import dalvik.annotation.optimization.FastNative;
@@ -75,7 +74,6 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
-import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@@ -145,23 +143,6 @@
private static final LruCache<String, Typeface> sDynamicTypefaceCache = new LruCache<>(16);
private static final Object sDynamicCacheLock = new Object();
- private static final LruCache<Long, LruCache<String, Typeface>> sVariableCache =
- new LruCache<>(16);
- private static final Object sVariableCacheLock = new Object();
-
- /** @hide */
- @VisibleForTesting
- public static void clearTypefaceCachesForTestingPurpose() {
- synchronized (sWeightCacheLock) {
- sWeightTypefaceCache.clear();
- }
- synchronized (sDynamicCacheLock) {
- sDynamicTypefaceCache.evictAll();
- }
- synchronized (sVariableCacheLock) {
- sVariableCache.evictAll();
- }
- }
@GuardedBy("SYSTEM_FONT_MAP_LOCK")
static Typeface sDefaultTypeface;
@@ -214,8 +195,6 @@
@UnsupportedAppUsage
public final long native_instance;
- private final Typeface mDerivedFrom;
-
private final String mSystemFontFamilyName;
private final Runnable mCleaner;
@@ -295,18 +274,6 @@
}
/**
- * Returns the Typeface used for creating this Typeface.
- *
- * Maybe null if this is not derived from other Typeface.
- * TODO(b/357707916): Make this public API.
- * @hide
- */
- @VisibleForTesting
- public final @Nullable Typeface getDerivedFrom() {
- return mDerivedFrom;
- }
-
- /**
* Returns the system font family name if the typeface was created from a system font family,
* otherwise returns null.
*/
@@ -1054,51 +1021,9 @@
return typeface;
}
- private static String axesToVarKey(@NonNull List<FontVariationAxis> axes) {
- // The given list can be mutated because it is allocated in Paint#setFontVariationSettings.
- // Currently, Paint#setFontVariationSettings is the only code path reaches this method.
- axes.sort(Comparator.comparingInt(FontVariationAxis::getOpenTypeTagValue));
- StringBuilder sb = new StringBuilder();
- for (int i = 0; i < axes.size(); ++i) {
- final FontVariationAxis fva = axes.get(i);
- sb.append(fva.getTag());
- sb.append(fva.getStyleValue());
- }
- return sb.toString();
- }
-
- /**
- * TODO(b/357707916): Make this public API.
- * @hide
- */
+ /** @hide */
public static Typeface createFromTypefaceWithVariation(@Nullable Typeface family,
@NonNull List<FontVariationAxis> axes) {
- if (Flags.typefaceCacheForVarSettings()) {
- final Typeface target = (family == null) ? Typeface.DEFAULT : family;
- final Typeface base = (target.mDerivedFrom == null) ? target : target.mDerivedFrom;
-
- final String key = axesToVarKey(axes);
-
- synchronized (sVariableCacheLock) {
- LruCache<String, Typeface> innerCache = sVariableCache.get(base.native_instance);
- if (innerCache == null) {
- // Cache up to 16 var instance per root Typeface
- innerCache = new LruCache<>(16);
- sVariableCache.put(base.native_instance, innerCache);
- } else {
- Typeface cached = innerCache.get(key);
- if (cached != null) {
- return cached;
- }
- }
- Typeface typeface = new Typeface(
- nativeCreateFromTypefaceWithVariation(base.native_instance, axes),
- base.getSystemFontFamilyName(), base);
- innerCache.put(key, typeface);
- return typeface;
- }
- }
-
final Typeface base = family == null ? Typeface.DEFAULT : family;
Typeface typeface = new Typeface(
nativeCreateFromTypefaceWithVariation(base.native_instance, axes),
@@ -1259,19 +1184,11 @@
// don't allow clients to call this directly
@UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023)
private Typeface(long ni) {
- this(ni, null, null);
+ this(ni, null);
}
-
// don't allow clients to call this directly
- // This is kept for robolectric.
private Typeface(long ni, @Nullable String systemFontFamilyName) {
- this(ni, systemFontFamilyName, null);
- }
-
- // don't allow clients to call this directly
- private Typeface(long ni, @Nullable String systemFontFamilyName,
- @Nullable Typeface derivedFrom) {
if (ni == 0) {
throw new RuntimeException("native typeface cannot be made");
}
@@ -1281,7 +1198,6 @@
mStyle = nativeGetStyle(ni);
mWeight = nativeGetWeight(ni);
mSystemFontFamilyName = systemFontFamilyName;
- mDerivedFrom = derivedFrom;
}
/**