From bcd39307af99f7e8b4a16f29cbff339a8700c4eb Mon Sep 17 00:00:00 2001 From: Jernej Virag Date: Wed, 24 May 2023 14:41:06 +0200 Subject: [PATCH] Animate clock fonts with fewer discrete steps Currently we're churning a big amount of memory allocating many variations of font glyphs as we're transitioning between clock states. That causes a significant amount of allocations, jank possibilities and memory bloat. This reduces the amount of taken font variations to, at most, 30 which significantly increases the amount of font cache hits in FontInterpolator and reduces the amount of memory we allocate for glyph paths. Visually, the difference isn't noticable. Bug: 275486055 Test: tested visually on foldable and Cheetah on udc-dev builds Change-Id: I99dd4108eef34156c4f8f79bbb042703325ed82d --- .../systemui/animation/FontInterpolator.kt | 12 +++ .../systemui/animation/TextAnimator.kt | 23 ++++-- .../shared/clocks/AnimatableClockView.kt | 4 +- .../systemui/animation/TextAnimatorTest.kt | 79 ++++++++----------- 4 files changed, 67 insertions(+), 51 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 83e44b69812b2..7e1bfb921ca9a 100644 --- a/packages/SystemUI/animation/src/com/android/systemui/animation/FontInterpolator.kt +++ b/packages/SystemUI/animation/src/com/android/systemui/animation/FontInterpolator.kt @@ -18,6 +18,7 @@ package com.android.systemui.animation import android.graphics.fonts.Font import android.graphics.fonts.FontVariationAxis +import android.util.Log import android.util.LruCache import android.util.MathUtils import android.util.MathUtils.abs @@ -114,6 +115,9 @@ class FontInterpolator { tmpInterpKey.set(start, end, progress) val cachedFont = interpCache[tmpInterpKey] if (cachedFont != null) { + if (DEBUG) { + Log.d(LOG_TAG, "[$progress] Interp. cache hit for $tmpInterpKey") + } return cachedFont } @@ -159,6 +163,9 @@ class FontInterpolator { val axesCachedFont = verFontCache[tmpVarFontKey] if (axesCachedFont != null) { interpCache.put(InterpKey(start, end, progress), axesCachedFont) + if (DEBUG) { + Log.d(LOG_TAG, "[$progress] Axis cache hit for $tmpVarFontKey") + } return axesCachedFont } @@ -168,6 +175,9 @@ class FontInterpolator { val newFont = Font.Builder(start).setFontVariationSettings(newAxes.toTypedArray()).build() interpCache.put(InterpKey(start, end, progress), newFont) verFontCache.put(VarFontKey(start, newAxes), newFont) + if (DEBUG) { + Log.d(LOG_TAG, "[$progress] Cache MISS for $tmpInterpKey / $tmpVarFontKey") + } return newFont } @@ -233,6 +243,8 @@ class FontInterpolator { (v.coerceIn(min, max) / step).toInt() * step companion object { + private const val LOG_TAG = "FontInterpolator" + private val DEBUG = Log.isLoggable(LOG_TAG, Log.DEBUG) private val EMPTY_AXES = arrayOf() // Returns true if given two font instance can be interpolated. 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 00108940e6ec6..16ddf0c36d9d1 100644 --- a/packages/SystemUI/animation/src/com/android/systemui/animation/TextAnimator.kt +++ b/packages/SystemUI/animation/src/com/android/systemui/animation/TextAnimator.kt @@ -26,6 +26,7 @@ import android.graphics.fonts.Font import android.graphics.fonts.FontVariationAxis import android.text.Layout import android.util.LruCache +import kotlin.math.roundToInt private const val DEFAULT_ANIMATION_DURATION: Long = 300 private const val TYPEFACE_CACHE_MAX_ENTRIES = 5 @@ -63,9 +64,9 @@ class TypefaceVariantCacheImpl( return it } - return TypefaceVariantCache - .createVariantTypeface(baseTypeface, fvar) - .also { cache.put(fvar, it) } + return TypefaceVariantCache.createVariantTypeface(baseTypeface, fvar).also { + cache.put(fvar, it) + } } } @@ -74,7 +75,6 @@ class TypefaceVariantCacheImpl( * * Currently this class can provide text style animation for text weight and text size. For example * the simple view that draws text with animating text size is like as follows: - * *
 
  * ```
  *     class SimpleTextAnimation : View {
@@ -97,6 +97,7 @@ class TypefaceVariantCacheImpl(
  */
 class TextAnimator(
     layout: Layout,
+    numberOfAnimationSteps: Int? = null, // Only do this number of discrete animation steps.
     private val invalidateCallback: () -> Unit,
 ) {
     var typefaceCache: TypefaceVariantCache = TypefaceVariantCacheImpl(layout.paint.typeface)
@@ -112,7 +113,8 @@ class TextAnimator(
         ValueAnimator.ofFloat(1f).apply {
             duration = DEFAULT_ANIMATION_DURATION
             addUpdateListener {
-                textInterpolator.progress = it.animatedValue as Float
+                textInterpolator.progress =
+                    calculateProgress(it.animatedValue as Float, numberOfAnimationSteps)
                 invalidateCallback()
             }
             addListener(
@@ -123,6 +125,17 @@ class TextAnimator(
             )
         }
 
+    private fun calculateProgress(animProgress: Float, numberOfAnimationSteps: Int?): Float {
+        if (numberOfAnimationSteps != null) {
+            // This clamps the progress to the nearest value of "numberOfAnimationSteps"
+            // discrete values between 0 and 1f.
+            return (animProgress * numberOfAnimationSteps).roundToInt() /
+                numberOfAnimationSteps.toFloat()
+        }
+
+        return animProgress
+    }
+
     sealed class PositionedGlyph {
 
         /** Mutable X coordinate of the glyph position relative from drawing offset. */
diff --git a/packages/SystemUI/customization/src/com/android/systemui/shared/clocks/AnimatableClockView.kt b/packages/SystemUI/customization/src/com/android/systemui/shared/clocks/AnimatableClockView.kt
index 465b73e6de19d..648ef03895cd3 100644
--- a/packages/SystemUI/customization/src/com/android/systemui/shared/clocks/AnimatableClockView.kt
+++ b/packages/SystemUI/customization/src/com/android/systemui/shared/clocks/AnimatableClockView.kt
@@ -74,7 +74,8 @@ class AnimatableClockView @JvmOverloads constructor(
     private var onTextAnimatorInitialized: Runnable? = null
 
     @VisibleForTesting var textAnimatorFactory: (Layout, () -> Unit) -> TextAnimator =
-        { layout, invalidateCb -> TextAnimator(layout, invalidateCb) }
+        { layout, invalidateCb ->
+            TextAnimator(layout, NUM_CLOCK_FONT_ANIMATION_STEPS, invalidateCb) }
     @VisibleForTesting var isAnimationEnabled: Boolean = true
     @VisibleForTesting var timeOverrideInMillis: Long? = null
 
@@ -567,6 +568,7 @@ class AnimatableClockView @JvmOverloads constructor(
         private const val CHARGE_ANIM_DURATION_PHASE_0: Long = 500
         private const val CHARGE_ANIM_DURATION_PHASE_1: Long = 1000
         private const val COLOR_ANIM_DURATION: Long = 400
+        private const val NUM_CLOCK_FONT_ANIMATION_STEPS = 30
 
         // Constants for the animation
         private val MOVE_INTERPOLATOR = Interpolators.EMPHASIZED
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 14ad3acf7fb09..263d3750c657f 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/animation/TextAnimatorTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/animation/TextAnimatorTest.kt
@@ -26,18 +26,17 @@ import android.text.TextPaint
 import androidx.test.filters.SmallTest
 import com.android.systemui.SysuiTestCase
 import com.google.common.truth.Truth.assertThat
+import kotlin.math.ceil
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.ArgumentCaptor
-import org.mockito.Mockito.`when`
 import org.mockito.Mockito.eq
 import org.mockito.Mockito.inOrder
 import org.mockito.Mockito.mock
 import org.mockito.Mockito.never
 import org.mockito.Mockito.times
 import org.mockito.Mockito.verify
-
-import kotlin.math.ceil
+import org.mockito.Mockito.`when`
 
 @RunWith(AndroidTestingRunner::class)
 @SmallTest
@@ -56,15 +55,13 @@ class TextAnimatorTest : SysuiTestCase() {
         val paint = mock(TextPaint::class.java)
         `when`(textInterpolator.targetPaint).thenReturn(paint)
 
-        val textAnimator = TextAnimator(layout, {}).apply {
-            this.textInterpolator = textInterpolator
-            this.animator = valueAnimator
-        }
+        val textAnimator =
+            TextAnimator(layout, null, {}).apply {
+                this.textInterpolator = textInterpolator
+                this.animator = valueAnimator
+            }
 
-        textAnimator.setTextStyle(
-                weight = 400,
-                animate = true
-        )
+        textAnimator.setTextStyle(weight = 400, animate = true)
 
         // If animation is requested, the base state should be rebased and the target state should
         // be updated.
@@ -88,15 +85,13 @@ class TextAnimatorTest : SysuiTestCase() {
         val paint = mock(TextPaint::class.java)
         `when`(textInterpolator.targetPaint).thenReturn(paint)
 
-        val textAnimator = TextAnimator(layout, {}).apply {
-            this.textInterpolator = textInterpolator
-            this.animator = valueAnimator
-        }
+        val textAnimator =
+            TextAnimator(layout, null, {}).apply {
+                this.textInterpolator = textInterpolator
+                this.animator = valueAnimator
+            }
 
-        textAnimator.setTextStyle(
-                weight = 400,
-                animate = false
-        )
+        textAnimator.setTextStyle(weight = 400, animate = false)
 
         // If animation is not requested, the progress should be 1 which is end of animation and the
         // base state is rebased to target state by calling rebase.
@@ -118,15 +113,16 @@ class TextAnimatorTest : SysuiTestCase() {
         `when`(textInterpolator.targetPaint).thenReturn(paint)
         val animationEndCallback = mock(Runnable::class.java)
 
-        val textAnimator = TextAnimator(layout, {}).apply {
-            this.textInterpolator = textInterpolator
-            this.animator = valueAnimator
-        }
+        val textAnimator =
+            TextAnimator(layout, null, {}).apply {
+                this.textInterpolator = textInterpolator
+                this.animator = valueAnimator
+            }
 
         textAnimator.setTextStyle(
-                weight = 400,
-                animate = true,
-                onAnimationEnd = animationEndCallback
+            weight = 400,
+            animate = true,
+            onAnimationEnd = animationEndCallback
         )
 
         // Verify animationEnd callback has been added.
@@ -144,34 +140,27 @@ class TextAnimatorTest : SysuiTestCase() {
         val layout = makeLayout("Hello, World", PAINT)
         val valueAnimator = mock(ValueAnimator::class.java)
         val textInterpolator = mock(TextInterpolator::class.java)
-        val paint = TextPaint().apply {
-            typeface = Typeface.createFromFile("/system/fonts/Roboto-Regular.ttf")
-        }
+        val paint =
+            TextPaint().apply {
+                typeface = Typeface.createFromFile("/system/fonts/Roboto-Regular.ttf")
+            }
         `when`(textInterpolator.targetPaint).thenReturn(paint)
 
-        val textAnimator = TextAnimator(layout, {}).apply {
-            this.textInterpolator = textInterpolator
-            this.animator = valueAnimator
-        }
+        val textAnimator =
+            TextAnimator(layout, null, {}).apply {
+                this.textInterpolator = textInterpolator
+                this.animator = valueAnimator
+            }
 
-        textAnimator.setTextStyle(
-                weight = 400,
-                animate = true
-        )
+        textAnimator.setTextStyle(weight = 400, animate = true)
 
         val prevTypeface = paint.typeface
 
-        textAnimator.setTextStyle(
-                weight = 700,
-                animate = true
-        )
+        textAnimator.setTextStyle(weight = 700, animate = true)
 
         assertThat(paint.typeface).isNotSameInstanceAs(prevTypeface)
 
-        textAnimator.setTextStyle(
-                weight = 400,
-                animate = true
-        )
+        textAnimator.setTextStyle(weight = 400, animate = true)
 
         assertThat(paint.typeface).isSameInstanceAs(prevTypeface)
     }