diff options
author | Jernej Virag <jernej@google.com> | 2023-04-19 14:19:54 +0200 |
---|---|---|
committer | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2023-04-22 01:33:42 +0000 |
commit | b5d889f5fd0278eaf8f41949e68e50be5af0929f (patch) | |
tree | 050ebf1bbf7ac0f8f042fc3ad95b848ff69b63e8 | |
parent | 6005e82bc73f6369fbd3cadb778296345dd1e77f (diff) | |
download | base-b5d889f5fd0278eaf8f41949e68e50be5af0929f.tar.gz |
Restrict maximum size of FontInterpolator font caches
This fixes a memory leak where Fonts were leaked within detailed lock
screen animations. We're occasionally animating fonts via several axes
which causes a low occurence of cache hits. This resulted in hundreds of
Font instances being kept in the cache.
Bug: 275486055
Test: Verified behaviour on cheetah
Expanded and ran unit tests for affected classes
Benchmarked rendering time with reduced cache - increase of
duration of draw() command for 0.3ms with 10 item cache.
50 item cache increased draw() command duration for 0.15ms on
average which is not worth the tradeoff in memory.
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:b7bc5640491d0a4cc0fd6c02f73c82912b1e3996)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:39dbe90004924912f49e141b2051bc27752f7c1a)
Merged-In: Id6e2b127f44f5e553ff7ac4ba640fd4f6f4698b0
Change-Id: Id6e2b127f44f5e553ff7ac4ba640fd4f6f4698b0
3 files changed, 43 insertions, 10 deletions
diff --git a/packages/SystemUI/animation/src/com/android/systemui/animation/FontInterpolator.kt b/packages/SystemUI/animation/src/com/android/systemui/animation/FontInterpolator.kt index f0a82113c3a3..83e44b69812b 100644 --- a/packages/SystemUI/animation/src/com/android/systemui/animation/FontInterpolator.kt +++ b/packages/SystemUI/animation/src/com/android/systemui/animation/FontInterpolator.kt @@ -18,8 +18,10 @@ package com.android.systemui.animation import android.graphics.fonts.Font import android.graphics.fonts.FontVariationAxis +import android.util.LruCache import android.util.MathUtils import android.util.MathUtils.abs +import androidx.annotation.VisibleForTesting import java.lang.Float.max import java.lang.Float.min @@ -34,6 +36,10 @@ private const val FONT_ITALIC_MIN = 0f private const val FONT_ITALIC_ANIMATION_STEP = 0.1f private const val FONT_ITALIC_DEFAULT_VALUE = 0f +// Benchmarked via Perfetto, difference between 10 and 50 entries is about 0.3ms in +// frame draw time on a Pixel 6. +@VisibleForTesting const val FONT_CACHE_MAX_ENTRIES = 10 + /** Provide interpolation of two fonts by adjusting font variation settings. */ class FontInterpolator { @@ -81,8 +87,8 @@ class FontInterpolator { // Font interpolator has two level caches: one for input and one for font with different // variation settings. No synchronization is needed since FontInterpolator is not designed to be // thread-safe and can be used only on UI thread. - private val interpCache = hashMapOf<InterpKey, Font>() - private val verFontCache = hashMapOf<VarFontKey, Font>() + private val interpCache = LruCache<InterpKey, Font>(FONT_CACHE_MAX_ENTRIES) + private val verFontCache = LruCache<VarFontKey, Font>(FONT_CACHE_MAX_ENTRIES) // Mutable keys for recycling. private val tmpInterpKey = InterpKey(null, null, 0f) @@ -152,7 +158,7 @@ class FontInterpolator { tmpVarFontKey.set(start, newAxes) val axesCachedFont = verFontCache[tmpVarFontKey] if (axesCachedFont != null) { - interpCache[InterpKey(start, end, progress)] = axesCachedFont + interpCache.put(InterpKey(start, end, progress), axesCachedFont) return axesCachedFont } @@ -160,8 +166,8 @@ class FontInterpolator { // Font.Builder#build won't throw IOException since creating fonts from existing fonts will // not do any IO work. val newFont = Font.Builder(start).setFontVariationSettings(newAxes.toTypedArray()).build() - interpCache[InterpKey(start, end, progress)] = newFont - verFontCache[VarFontKey(start, newAxes)] = newFont + interpCache.put(InterpKey(start, end, progress), newFont) + verFontCache.put(VarFontKey(start, newAxes), newFont) return newFont } 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 9e9929e79d47..3ee97be360f0 100644 --- a/packages/SystemUI/animation/src/com/android/systemui/animation/TextAnimator.kt +++ b/packages/SystemUI/animation/src/com/android/systemui/animation/TextAnimator.kt @@ -24,8 +24,10 @@ import android.graphics.Canvas import android.graphics.Typeface import android.graphics.fonts.Font import android.text.Layout +import android.util.LruCache private const val DEFAULT_ANIMATION_DURATION: Long = 300 +private const val TYPEFACE_CACHE_MAX_ENTRIES = 5 typealias GlyphCallback = (TextAnimator.PositionedGlyph, Float) -> Unit /** @@ -114,7 +116,7 @@ class TextAnimator(layout: Layout, private val invalidateCallback: () -> Unit) { private val fontVariationUtils = FontVariationUtils() - private val typefaceCache = HashMap<String, Typeface?>() + private val typefaceCache = LruCache<String, Typeface>(TYPEFACE_CACHE_MAX_ENTRIES) fun updateLayout(layout: Layout) { textInterpolator.layout = layout @@ -218,12 +220,12 @@ class TextAnimator(layout: Layout, private val invalidateCallback: () -> Unit) { } if (!fvar.isNullOrBlank()) { - textInterpolator.targetPaint.typeface = - typefaceCache.getOrElse(fvar) { - textInterpolator.targetPaint.fontVariationSettings = fvar + textInterpolator.targetPaint.typeface = typefaceCache.get(fvar) ?: run { + textInterpolator.targetPaint.fontVariationSettings = fvar + textInterpolator.targetPaint.typeface?.also { typefaceCache.put(fvar, textInterpolator.targetPaint.typeface) - textInterpolator.targetPaint.typeface } + } } if (color != null) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/animation/FontInterpolatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/animation/FontInterpolatorTest.kt index 8a5c5b58d058..57a355f4e127 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/animation/FontInterpolatorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/animation/FontInterpolatorTest.kt @@ -106,4 +106,29 @@ class FontInterpolatorTest : SysuiTestCase() { val reversedFont = interp.lerp(endFont, startFont, 0.5f) assertThat(resultFont).isSameInstanceAs(reversedFont) } + + @Test + fun testCacheMaxSize() { + val interp = FontInterpolator() + + val startFont = Font.Builder(sFont) + .setFontVariationSettings("'wght' 100") + .build() + val endFont = Font.Builder(sFont) + .setFontVariationSettings("'wght' 1") + .build() + val resultFont = interp.lerp(startFont, endFont, 0.5f) + for (i in 0..FONT_CACHE_MAX_ENTRIES + 1) { + val f1 = Font.Builder(sFont) + .setFontVariationSettings("'wght' ${i * 100}") + .build() + val f2 = Font.Builder(sFont) + .setFontVariationSettings("'wght' $i") + .build() + interp.lerp(f1, f2, 0.5f) + } + + val cachedFont = interp.lerp(startFont, endFont, 0.5f) + assertThat(resultFont).isNotSameInstanceAs(cachedFont) + } } |