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

Commit 367529e5 authored by Liana Kazanova (xWF)'s avatar Liana Kazanova (xWF) Committed by Android (Google) Code Review
Browse files

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
parent 4cefdc6d

apct-tests/perftests/core/src/android/text/VariableFontPerfTest.java

+3 −25
Original line number Diff line number Diff line
@@ -19,7 +19,6 @@ package android.text; 
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 class VariableFontPerfTest { 
    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();

            state.resumeTiming();



            paint.setFontVariationSettings("'wght' 450");

        }

        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);

            int weight = random.nextInt(1000);

            state.resumeTiming();



            paint.setFontVariationSettings("'wght' 450");

            paint.setFontVariationSettings("'wght' " + weight);

        }



        Typeface.clearTypefaceCachesForTestingPurpose();

    }



}

core/java/android/text/flags/flags.aconfig

+0 −10
Original line number Diff line number Diff line
@@ -277,13 +277,3 @@ flag { 
    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

core/tests/coretests/src/android/graphics/PaintFontVariationTest.java

deleted100644 → 0
+0 −74
Original line number Diff line number Diff line 
/*

 * 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());

    }

}

core/tests/coretests/src/android/graphics/PaintTest.java

+0 −41
Original line number Diff line number Diff line
@@ -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 @@ import java.util.HashSet; 
 * 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 @@ public class PaintTest extends InstrumentationTestCase { 
        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());

    }

}

graphics/java/android/graphics/Typeface.java

+2 −86
Original line number Diff line number Diff line
@@ -56,7 +56,6 @@ import android.util.SparseArray; 
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.nio.ByteOrder; 
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 @@ public class Typeface { 
    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 @@ public class Typeface { 
    @UnsupportedAppUsage

    public final long native_instance;



    private final Typeface mDerivedFrom;



    private final String mSystemFontFamilyName;



    private final Runnable mCleaner;
@@ -294,18 +273,6 @@ public class Typeface { 
        return (mStyle & ITALIC) != 0;

    }



    /**

     * 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 @@ public class Typeface { 
        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 @@ public class Typeface { 
    // 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 @@ public class Typeface { 
        mStyle = nativeGetStyle(ni);

        mWeight = nativeGetWeight(ni);

        mSystemFontFamilyName = systemFontFamilyName;

        mDerivedFrom = derivedFrom;

    }



    /**