From 5531f6274bae4f039a6ce41c8a54a785e9eea6bf Mon Sep 17 00:00:00 2001 From: William Xiao Date: Wed, 5 Jul 2023 18:03:19 -0700 Subject: [PATCH] Remove unnecessary animation cancellations when exiting low light This CL removes the unnecessary cancellation in the transition coordinator and also scales the dream overlay exit animation from lasting >1s so that a self-cancellation isn't needed after the view is no longer visible. Also greatly simplifies the method we use to truncate the long animations. Test: atest LowLightDreamManagerTest LowLightTransitionCoordinatorTest TruncatedInterpolatorTest Test: manually verified that enabling low light doesn't crash if tapping during the animation Bug: 285666217 Fixed: 285666217 Change-Id: Ib0dfe4fe4d030888607e1f03d57f1a084a5fba10 --- .../dream/lowlight/LowLightDreamManager.kt | 6 +++ .../lowlight/LowLightTransitionCoordinator.kt | 10 ---- .../lowlight/util/TruncatedInterpolator.kt | 54 +++++++++++++++++++ libs/dream/lowlight/tests/Android.bp | 1 + .../LowLightDreamManagerTest.kt | 15 ++++++ .../LowLightTransitionCoordinatorTest.kt | 20 ------- .../util/TruncatedInterpolatorTest.kt | 53 ++++++++++++++++++ .../DreamOverlayAnimationsController.kt | 42 +++++++-------- 8 files changed, 149 insertions(+), 52 deletions(-) create mode 100644 libs/dream/lowlight/src/com/android/dream/lowlight/util/TruncatedInterpolator.kt create mode 100644 libs/dream/lowlight/tests/src/com/android/dream/lowlight/util/TruncatedInterpolatorTest.kt diff --git a/libs/dream/lowlight/src/com/android/dream/lowlight/LowLightDreamManager.kt b/libs/dream/lowlight/src/com/android/dream/lowlight/LowLightDreamManager.kt index 96bfb78eff0d3..d1dd8df81a3e5 100644 --- a/libs/dream/lowlight/src/com/android/dream/lowlight/LowLightDreamManager.kt +++ b/libs/dream/lowlight/src/com/android/dream/lowlight/LowLightDreamManager.kt @@ -23,6 +23,7 @@ import android.content.ComponentName import android.util.Log import com.android.dream.lowlight.dagger.LowLightDreamModule import com.android.dream.lowlight.dagger.qualifiers.Application +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job import kotlinx.coroutines.TimeoutCancellationException @@ -103,6 +104,11 @@ class LowLightDreamManager @Inject constructor( ) } catch (ex: TimeoutCancellationException) { Log.e(TAG, "timed out while waiting for low light animation", ex) + } catch (ex: CancellationException) { + Log.w(TAG, "low light transition animation cancelled") + // Catch the cancellation so that we still set the system dream component if the + // animation is cancelled, such as by a user tapping to wake as the transition to + // low light happens. } dreamManager.setSystemDreamComponent( if (shouldEnterLowLight) lowLightDreamComponent else null diff --git a/libs/dream/lowlight/src/com/android/dream/lowlight/LowLightTransitionCoordinator.kt b/libs/dream/lowlight/src/com/android/dream/lowlight/LowLightTransitionCoordinator.kt index 473603002b212..de1aee5986678 100644 --- a/libs/dream/lowlight/src/com/android/dream/lowlight/LowLightTransitionCoordinator.kt +++ b/libs/dream/lowlight/src/com/android/dream/lowlight/LowLightTransitionCoordinator.kt @@ -110,15 +110,5 @@ class LowLightTransitionCoordinator @Inject constructor() { } } animator.addListener(listener) - continuation.invokeOnCancellation { - try { - animator.removeListener(listener) - animator.cancel() - } catch (exception: IndexOutOfBoundsException) { - // TODO(b/285666217): remove this try/catch once a proper fix is implemented. - // Cancelling the animator can cause an exception since we may be removing a - // listener during the cancellation. See b/285666217 for more details. - } - } } } diff --git a/libs/dream/lowlight/src/com/android/dream/lowlight/util/TruncatedInterpolator.kt b/libs/dream/lowlight/src/com/android/dream/lowlight/util/TruncatedInterpolator.kt new file mode 100644 index 0000000000000..f69c84dafbb28 --- /dev/null +++ b/libs/dream/lowlight/src/com/android/dream/lowlight/util/TruncatedInterpolator.kt @@ -0,0 +1,54 @@ +/* + * Copyright (C) 2023 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 com.android.dream.lowlight.util + +import android.view.animation.Interpolator + +/** + * Interpolator wrapper that shortens another interpolator from its original duration to a portion + * of that duration. + * + * For example, an `originalDuration` of 1000 and a `newDuration` of 200 results in an animation + * that when played for 200ms is the exact same as the first 200ms of a 1000ms animation if using + * the original interpolator. + * + * This is useful for the transition between the user dream and the low light clock as some + * animations are defined in the spec to be longer than the total duration of the animation. For + * example, the low light clock exit translation animation is defined to last >1s while the actual + * fade out of the low light clock is only 250ms, meaning the clock isn't visible anymore after + * 250ms. + * + * Since the dream framework currently only allows one dream to be visible and running, we use this + * interpolator to play just the first 250ms of the translation animation. Simply reducing the + * duration of the animation would result in the text exiting much faster than intended, so a custom + * interpolator is needed. + */ +class TruncatedInterpolator( + private val baseInterpolator: Interpolator, + originalDuration: Float, + newDuration: Float +) : Interpolator { + private val scaleFactor: Float + + init { + scaleFactor = newDuration / originalDuration + } + + override fun getInterpolation(input: Float): Float { + return baseInterpolator.getInterpolation(input * scaleFactor) + } +} diff --git a/libs/dream/lowlight/tests/Android.bp b/libs/dream/lowlight/tests/Android.bp index 2d79090cd7d4c..64b53cbb5c5a7 100644 --- a/libs/dream/lowlight/tests/Android.bp +++ b/libs/dream/lowlight/tests/Android.bp @@ -27,6 +27,7 @@ android_test { "androidx.test.runner", "androidx.test.rules", "androidx.test.ext.junit", + "animationlib", "frameworks-base-testutils", "junit", "kotlinx_coroutines_test", diff --git a/libs/dream/lowlight/tests/src/com.android.dream.lowlight/LowLightDreamManagerTest.kt b/libs/dream/lowlight/tests/src/com.android.dream.lowlight/LowLightDreamManagerTest.kt index 2a886bc317886..de84adb2e5c2d 100644 --- a/libs/dream/lowlight/tests/src/com.android.dream.lowlight/LowLightDreamManagerTest.kt +++ b/libs/dream/lowlight/tests/src/com.android.dream.lowlight/LowLightDreamManagerTest.kt @@ -152,6 +152,21 @@ class LowLightDreamManagerTest { verify(mDreamManager).setSystemDreamComponent(DREAM_COMPONENT) } + @Test + fun setAmbientLightMode_animationCancelled_SetsSystemDream() = testScope.runTest { + mLowLightDreamManager.setAmbientLightMode(LowLightDreamManager.AMBIENT_LIGHT_MODE_LOW_LIGHT) + runCurrent() + cancelEnterAnimations() + runCurrent() + // Animation never finishes, but we should still set the system dream + verify(mDreamManager).setSystemDreamComponent(DREAM_COMPONENT) + } + + private fun cancelEnterAnimations() { + val listener = withArgCaptor { verify(mEnterAnimator).addListener(capture()) } + listener.onAnimationCancel(mEnterAnimator) + } + private fun completeEnterAnimations() { val listener = withArgCaptor { verify(mEnterAnimator).addListener(capture()) } listener.onAnimationEnd(mEnterAnimator) diff --git a/libs/dream/lowlight/tests/src/com.android.dream.lowlight/LowLightTransitionCoordinatorTest.kt b/libs/dream/lowlight/tests/src/com.android.dream.lowlight/LowLightTransitionCoordinatorTest.kt index 4c526a6ac69d0..9ae304f9763a7 100644 --- a/libs/dream/lowlight/tests/src/com.android.dream.lowlight/LowLightTransitionCoordinatorTest.kt +++ b/libs/dream/lowlight/tests/src/com.android.dream.lowlight/LowLightTransitionCoordinatorTest.kt @@ -158,26 +158,6 @@ class LowLightTransitionCoordinatorTest { assertThat(job.isCancelled).isTrue() } - @Test - fun shouldCancelAnimatorWhenJobCancelled() = testScope.runTest { - whenever(mEnterListener.onBeforeEnterLowLight()).thenReturn(mAnimator) - val coordinator = LowLightTransitionCoordinator() - coordinator.setLowLightEnterListener(mEnterListener) - val job = launch { - coordinator.waitForLowLightTransitionAnimation(timeout = TIMEOUT, entering = true) - } - runCurrent() - // Animator listener is added and the runnable is not run yet. - verify(mAnimator).addListener(mAnimatorListenerCaptor.capture()) - verify(mAnimator, never()).cancel() - assertThat(job.isCompleted).isFalse() - - job.cancel() - // We should have removed the listener and cancelled the animator - verify(mAnimator).removeListener(mAnimatorListenerCaptor.value) - verify(mAnimator).cancel() - } - companion object { private val TIMEOUT = 1.toDuration(DurationUnit.SECONDS) } diff --git a/libs/dream/lowlight/tests/src/com/android/dream/lowlight/util/TruncatedInterpolatorTest.kt b/libs/dream/lowlight/tests/src/com/android/dream/lowlight/util/TruncatedInterpolatorTest.kt new file mode 100644 index 0000000000000..190f02e97136f --- /dev/null +++ b/libs/dream/lowlight/tests/src/com/android/dream/lowlight/util/TruncatedInterpolatorTest.kt @@ -0,0 +1,53 @@ +/* + * Copyright (C) 2023 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 com.android.dream.lowlight.util + +import android.testing.AndroidTestingRunner +import androidx.test.filters.SmallTest +import com.android.app.animation.Interpolators +import com.google.common.truth.Truth +import org.junit.Test +import org.junit.runner.RunWith + +@SmallTest +@RunWith(AndroidTestingRunner::class) +class TruncatedInterpolatorTest { + @Test + fun truncatedInterpolator_matchesRegularInterpolator() { + val originalInterpolator = Interpolators.EMPHASIZED + val truncatedInterpolator = + TruncatedInterpolator(originalInterpolator, ORIGINAL_DURATION_MS, NEW_DURATION_MS) + + // Both interpolators should start at the same value. + var animationPercent = 0f + Truth.assertThat(truncatedInterpolator.getInterpolation(animationPercent)) + .isEqualTo(originalInterpolator.getInterpolation(animationPercent)) + + animationPercent = 1f + Truth.assertThat(truncatedInterpolator.getInterpolation(animationPercent)) + .isEqualTo(originalInterpolator.getInterpolation(animationPercent * DURATION_RATIO)) + + animationPercent = 0.25f + Truth.assertThat(truncatedInterpolator.getInterpolation(animationPercent)) + .isEqualTo(originalInterpolator.getInterpolation(animationPercent * DURATION_RATIO)) + } + + companion object { + private const val ORIGINAL_DURATION_MS: Float = 1000f + private const val NEW_DURATION_MS: Float = 200f + private const val DURATION_RATIO: Float = NEW_DURATION_MS / ORIGINAL_DURATION_MS + } +} diff --git a/packages/SystemUI/src/com/android/systemui/dreams/DreamOverlayAnimationsController.kt b/packages/SystemUI/src/com/android/systemui/dreams/DreamOverlayAnimationsController.kt index ee046c27d72e2..484bf3d51f365 100644 --- a/packages/SystemUI/src/com/android/systemui/dreams/DreamOverlayAnimationsController.kt +++ b/packages/SystemUI/src/com/android/systemui/dreams/DreamOverlayAnimationsController.kt @@ -26,6 +26,7 @@ import androidx.core.animation.doOnEnd import androidx.lifecycle.Lifecycle import androidx.lifecycle.repeatOnLifecycle import com.android.app.animation.Interpolators +import com.android.dream.lowlight.util.TruncatedInterpolator import com.android.systemui.R import com.android.systemui.complication.ComplicationHostViewController import com.android.systemui.complication.ComplicationLayoutParams @@ -204,31 +205,28 @@ constructor( translationYAnimator( from = 0f, to = -mDreamInTranslationYDistance.toFloat(), - durationMs = mDreamInTranslationYDurationMs, + durationMs = mDreamInComplicationsAnimDurationMs, delayMs = 0, - interpolator = Interpolators.EMPHASIZED + // Truncate the animation from the full duration to match the alpha + // animation so that the whole animation ends at the same time. + interpolator = + TruncatedInterpolator( + Interpolators.EMPHASIZED, + /*originalDuration=*/ mDreamInTranslationYDurationMs.toFloat(), + /*newDuration=*/ mDreamInComplicationsAnimDurationMs.toFloat() + ) ), alphaAnimator( - from = - mCurrentAlphaAtPosition.getOrDefault( - key = POSITION_BOTTOM, - defaultValue = 1f - ), - to = 0f, - durationMs = mDreamInComplicationsAnimDurationMs, - delayMs = 0, - positions = POSITION_BOTTOM - ) - .apply { - doOnEnd { - // The logical end of the animation is once the alpha and blur - // animations finish, end the animation so that any listeners are - // notified. The Y translation animation is much longer than all of - // the other animations due to how the spec is defined, but is not - // expected to run to completion. - mAnimator?.end() - } - }, + from = + mCurrentAlphaAtPosition.getOrDefault( + key = POSITION_BOTTOM, + defaultValue = 1f + ), + to = 0f, + durationMs = mDreamInComplicationsAnimDurationMs, + delayMs = 0, + positions = POSITION_BOTTOM + ), alphaAnimator( from = mCurrentAlphaAtPosition.getOrDefault(