From d702ba7fee5894a86cdfde689b85492bf6194d5c Mon Sep 17 00:00:00 2001 From: Hawkwood Glazier Date: Sat, 29 Apr 2023 16:39:26 +0000 Subject: [PATCH] Do not use Paint.setFontVariationSettings as basis for variant Typefaces Post cache eviction, it was possible to generate the wrong typeface for an animation if the typeface was edited but not the font variation settings as Paint.setFontVariationSettings would not. If the typeface must be changed, reinitialization of the cache is also required. Bug: 279877970 Test: Manually checked variant typeface generation Change-Id: I5f76c0aa3d8aca7c1096c34c09b5861b980fe0ff --- .../systemui/animation/TextAnimator.kt | 39 ++++++++++++++----- .../systemui/animation/TextInterpolator.kt | 14 +++---- .../systemui/animation/TextAnimatorTest.kt | 4 -- .../animation/TextInterpolatorTest.kt | 4 +- 4 files changed, 37 insertions(+), 24 deletions(-) diff --git a/packages/SystemUI/animation/src/com/android/systemui/animation/TextAnimator.kt b/packages/SystemUI/animation/src/com/android/systemui/animation/TextAnimator.kt index 9346a2f7ebb7a..00108940e6ec6 100644 --- a/packages/SystemUI/animation/src/com/android/systemui/animation/TextAnimator.kt +++ b/packages/SystemUI/animation/src/com/android/systemui/animation/TextAnimator.kt @@ -23,8 +23,8 @@ import android.animation.ValueAnimator import android.graphics.Canvas import android.graphics.Typeface import android.graphics.fonts.Font +import android.graphics.fonts.FontVariationAxis import android.text.Layout -import android.text.TextPaint import android.util.LruCache private const val DEFAULT_ANIMATION_DURATION: Long = 300 @@ -33,18 +33,39 @@ private const val TYPEFACE_CACHE_MAX_ENTRIES = 5 typealias GlyphCallback = (TextAnimator.PositionedGlyph, Float) -> Unit interface TypefaceVariantCache { - fun getTypefaceForVariant(fvar: String, targetPaint: TextPaint): Typeface? + fun getTypefaceForVariant(fvar: String?): Typeface? + + companion object { + fun createVariantTypeface(baseTypeface: Typeface, fVar: String?): Typeface { + if (fVar.isNullOrEmpty()) { + return baseTypeface + } + + val axes = FontVariationAxis.fromFontVariationSettings(fVar).toMutableList() + axes.removeIf { !baseTypeface.isSupportedAxes(it.getOpenTypeTagValue()) } + if (axes.isEmpty()) { + return baseTypeface + } + return Typeface.createFromTypefaceWithVariation(baseTypeface, axes) + } + } } -class TypefaceVariantCacheImpl() : TypefaceVariantCache { +class TypefaceVariantCacheImpl( + var baseTypeface: Typeface, +) : TypefaceVariantCache { private val cache = LruCache(TYPEFACE_CACHE_MAX_ENTRIES) - override fun getTypefaceForVariant(fvar: String, targetPaint: TextPaint): Typeface? { + override fun getTypefaceForVariant(fvar: String?): Typeface? { + if (fvar == null) { + return baseTypeface + } cache.get(fvar)?.let { return it } - targetPaint.fontVariationSettings = fvar - return targetPaint.typeface?.also { cache.put(fvar, it) } + return TypefaceVariantCache + .createVariantTypeface(baseTypeface, fvar) + .also { cache.put(fvar, it) } } } @@ -78,7 +99,7 @@ class TextAnimator( layout: Layout, private val invalidateCallback: () -> Unit, ) { - var typefaceCache: TypefaceVariantCache = TypefaceVariantCacheImpl() + var typefaceCache: TypefaceVariantCache = TypefaceVariantCacheImpl(layout.paint.typeface) get() = field set(value) { field = value @@ -244,8 +265,7 @@ class TextAnimator( } if (!fvar.isNullOrBlank()) { - textInterpolator.targetPaint.typeface = - typefaceCache.getTypefaceForVariant(fvar, textInterpolator.targetPaint) + textInterpolator.targetPaint.typeface = typefaceCache.getTypefaceForVariant(fvar) } if (color != null) { @@ -339,4 +359,3 @@ class TextAnimator( ) } } - diff --git a/packages/SystemUI/animation/src/com/android/systemui/animation/TextInterpolator.kt b/packages/SystemUI/animation/src/com/android/systemui/animation/TextInterpolator.kt index 79189bc3e5c3c..8ed8d8fb61fd0 100644 --- a/packages/SystemUI/animation/src/com/android/systemui/animation/TextInterpolator.kt +++ b/packages/SystemUI/animation/src/com/android/systemui/animation/TextInterpolator.kt @@ -38,6 +38,8 @@ class TextInterpolator( * Once you modified the style parameters, you have to call reshapeText to recalculate base text * layout. * + * Do not bypass the cache and update the typeface or font variation directly. + * * @return a paint object */ val basePaint = TextPaint(layout.paint) @@ -48,6 +50,8 @@ class TextInterpolator( * Once you modified the style parameters, you have to call reshapeText to recalculate target * text layout. * + * Do not bypass the cache and update the typeface or font variation directly. + * * @return a paint object */ val targetPaint = TextPaint(layout.paint) @@ -217,14 +221,8 @@ class TextInterpolator( run.fontRuns.forEach { fontRun -> fontRun.baseFont = fontInterpolator.lerp(fontRun.baseFont, fontRun.targetFont, progress) - val fvar = run { - val tmpFontVariationsArray = mutableListOf() - fontRun.baseFont.axes.forEach { - tmpFontVariationsArray.add(FontVariationAxis(it.tag, it.styleValue)) - } - FontVariationAxis.toFontVariationSettings(tmpFontVariationsArray) - } - basePaint.typeface = typefaceCache.getTypefaceForVariant(fvar, basePaint) + val fvar = FontVariationAxis.toFontVariationSettings(fontRun.baseFont.axes) + basePaint.typeface = typefaceCache.getTypefaceForVariant(fvar) } } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/animation/TextAnimatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/animation/TextAnimatorTest.kt index d7aa6e063d1ec..14ad3acf7fb09 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/animation/TextAnimatorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/animation/TextAnimatorTest.kt @@ -39,10 +39,6 @@ import org.mockito.Mockito.verify import kotlin.math.ceil -private val PAINT = TextPaint().apply { - textSize = 32f -} - @RunWith(AndroidTestingRunner::class) @SmallTest class TextAnimatorTest : SysuiTestCase() { diff --git a/packages/SystemUI/tests/src/com/android/systemui/animation/TextInterpolatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/animation/TextInterpolatorTest.kt index 063757acc1a16..f6fcd16cfd00a 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/animation/TextInterpolatorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/animation/TextInterpolatorTest.kt @@ -49,7 +49,7 @@ private val VF_FONT = Font.Builder(File("/system/fonts/Roboto-Regular.ttf")).bui private fun Font.toTypeface() = Typeface.CustomFallbackBuilder(FontFamily.Builder(this).build()).build() -private val PAINT = TextPaint().apply { +internal val PAINT = TextPaint().apply { typeface = Font.Builder(VF_FONT).setFontVariationSettings("'wght' 400").build().toTypeface() textSize = 32f } @@ -79,7 +79,7 @@ class TextInterpolatorTest : SysuiTestCase() { @Before fun setup() { - typefaceCache = TypefaceVariantCacheImpl() + typefaceCache = TypefaceVariantCacheImpl(PAINT.typeface) } @Test