From c87de81d8381ed42f10f9d57df322e88550d457c Mon Sep 17 00:00:00 2001 From: Nicolo' Mazzucato Date: Fri, 17 Mar 2023 11:09:55 +0000 Subject: Make remote Unfold progress receiver resilient to jank If the process sending progresses has jank or long main thread pauses, the receiving process would show the same jank pattern. With this cl, on the receving and a filter is added so smooth those cases, and result in an always smooth animation despite sending process jank (for now, sending process is always sysui, and receiving process is always launcher) Bug: 274409068 Test: UnfoldRemoteFilterTest, RemoteUnfoldTransitionReceiverTest and manual Change-Id: I2cb2e32a8639d364495e7133191e0cd8b8c77649 --- .../progress/RemoteUnfoldTransitionReceiverTest.kt | 56 ++++++++++++-- .../unfold/progress/TestUnfoldProgressListener.kt | 2 +- .../unfold/progress/UnfoldRemoteFilterTest.kt | 71 ++++++++++++++++++ .../android/systemui/unfold/UnfoldRemoteModule.kt | 3 + .../systemui/unfold/dagger/UseReceivingFilter.kt | 20 +++++ .../progress/RemoteUnfoldTransitionReceiver.kt | 59 +++++++++++++-- .../systemui/unfold/progress/UnfoldRemoteFilter.kt | 85 ++++++++++++++++++++++ 7 files changed, 285 insertions(+), 11 deletions(-) create mode 100644 packages/SystemUI/tests/src/com/android/systemui/unfold/progress/UnfoldRemoteFilterTest.kt create mode 100644 packages/SystemUI/unfold/src/com/android/systemui/unfold/dagger/UseReceivingFilter.kt create mode 100644 packages/SystemUI/unfold/src/com/android/systemui/unfold/progress/UnfoldRemoteFilter.kt diff --git a/packages/SystemUI/tests/src/com/android/systemui/unfold/progress/RemoteUnfoldTransitionReceiverTest.kt b/packages/SystemUI/tests/src/com/android/systemui/unfold/progress/RemoteUnfoldTransitionReceiverTest.kt index 0e7e039e69e2..4989a21791fb 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/unfold/progress/RemoteUnfoldTransitionReceiverTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/unfold/progress/RemoteUnfoldTransitionReceiverTest.kt @@ -18,6 +18,7 @@ package com.android.systemui.unfold.progress import android.testing.AndroidTestingRunner import androidx.test.filters.SmallTest +import androidx.test.platform.app.InstrumentationRegistry import com.android.systemui.SysuiTestCase import org.junit.Before import org.junit.Test @@ -27,23 +28,27 @@ import org.junit.runner.RunWith @SmallTest class RemoteUnfoldTransitionReceiverTest : SysuiTestCase() { - private val progressProvider = RemoteUnfoldTransitionReceiver { it.run() } + private val progressProvider = + RemoteUnfoldTransitionReceiver(useReceivingFilter = true) { runOnMainSync(it) } + private val progressProviderWithoutFilter = + RemoteUnfoldTransitionReceiver(useReceivingFilter = false) { it.run() } private val listener = TestUnfoldProgressListener() @Before fun setUp() { progressProvider.addCallback(listener) + progressProviderWithoutFilter.addCallback(listener) } @Test - fun onTransitionStarted_propagated() { + fun onTransitionStarted_withFilter_propagated() { progressProvider.onTransitionStarted() listener.assertStarted() } @Test - fun onTransitionProgress_propagated() { + fun onTransitionProgress_withFilter_propagated() { progressProvider.onTransitionStarted() progressProvider.onTransitionProgress(0.5f) @@ -52,7 +57,7 @@ class RemoteUnfoldTransitionReceiverTest : SysuiTestCase() { } @Test - fun onTransitionEnded_propagated() { + fun onTransitionEnded_withFilter_propagated() { progressProvider.onTransitionStarted() progressProvider.onTransitionProgress(0.5f) @@ -62,11 +67,52 @@ class RemoteUnfoldTransitionReceiverTest : SysuiTestCase() { } @Test - fun onTransitionStarted_afterCallbackRemoved_notPropagated() { + fun onTransitionStarted_withFilter_afterCallbackRemoved_notPropagated() { progressProvider.removeCallback(listener) progressProvider.onTransitionStarted() listener.assertNotStarted() } + + @Test + fun onTransitionStarted_withoutFilter_propagated() { + progressProviderWithoutFilter.onTransitionStarted() + + listener.assertStarted() + } + + @Test + fun onTransitionProgress_withoutFilter_propagated() { + progressProviderWithoutFilter.onTransitionStarted() + + progressProviderWithoutFilter.onTransitionProgress(0.5f) + + listener.assertLastProgress(0.5f) + } + + @Test + fun onTransitionEnded_withoutFilter_propagated() { + progressProviderWithoutFilter.onTransitionStarted() + progressProviderWithoutFilter.onTransitionProgress(0.5f) + + progressProviderWithoutFilter.onTransitionFinished() + + listener.ensureTransitionFinished() + } + + @Test + fun onTransitionStarted_withoutFilter_afterCallbackRemoved_notPropagated() { + progressProviderWithoutFilter.removeCallback(listener) + + progressProviderWithoutFilter.onTransitionStarted() + + listener.assertNotStarted() + } + + private fun runOnMainSync(f: Runnable) { + InstrumentationRegistry.getInstrumentation().runOnMainSync { f.run() } + // Sleep as the animator used from the filter has a callback that happens at every frame. + Thread.sleep(60) + } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/unfold/progress/TestUnfoldProgressListener.kt b/packages/SystemUI/tests/src/com/android/systemui/unfold/progress/TestUnfoldProgressListener.kt index f6532070d720..1ce25725b298 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/unfold/progress/TestUnfoldProgressListener.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/unfold/progress/TestUnfoldProgressListener.kt @@ -126,7 +126,7 @@ class TestUnfoldProgressListener : UnfoldTransitionProgressProvider.TransitionPr } fun assertLastProgress(progress: Float) { - assertThat(progressHistory.last()).isEqualTo(progress) + assertThat(progressHistory.last()).isWithin(1.0E-4F).of(progress) } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/unfold/progress/UnfoldRemoteFilterTest.kt b/packages/SystemUI/tests/src/com/android/systemui/unfold/progress/UnfoldRemoteFilterTest.kt new file mode 100644 index 000000000000..f14009aad033 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/unfold/progress/UnfoldRemoteFilterTest.kt @@ -0,0 +1,71 @@ +/* + * 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.systemui.unfold.progress + +import android.testing.AndroidTestingRunner +import androidx.test.filters.SmallTest +import androidx.test.platform.app.InstrumentationRegistry +import com.android.systemui.SysuiTestCase +import kotlin.time.Duration +import kotlin.time.Duration.Companion.milliseconds +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidTestingRunner::class) +@SmallTest +class UnfoldRemoteFilterTest : SysuiTestCase() { + private val listener = TestUnfoldProgressListener() + + private val progressProvider = UnfoldRemoteFilter(listener) + + @Test + fun onTransitionStarted_propagated() { + runOnMainThreadWithInterval({ progressProvider.onTransitionStarted() }) + listener.assertStarted() + } + + @Test + fun onTransitionProgress_withInterval_propagated() { + runOnMainThreadWithInterval( + { progressProvider.onTransitionStarted() }, + { progressProvider.onTransitionProgress(0.5f) } + ) + + listener.assertLastProgress(0.5f) + } + + @Test + fun onTransitionEnded_propagated() { + runOnMainThreadWithInterval( + { progressProvider.onTransitionStarted() }, + { progressProvider.onTransitionProgress(0.5f) }, + { progressProvider.onTransitionFinished() }, + ) + + listener.ensureTransitionFinished() + } + + private fun runOnMainThreadWithInterval( + vararg blocks: () -> Unit, + interval: Duration = 60.milliseconds + ) { + blocks.forEach { + InstrumentationRegistry.getInstrumentation().runOnMainSync { it() } + Thread.sleep(interval.inWholeMilliseconds) + } + } +} diff --git a/packages/SystemUI/unfold/src/com/android/systemui/unfold/UnfoldRemoteModule.kt b/packages/SystemUI/unfold/src/com/android/systemui/unfold/UnfoldRemoteModule.kt index b395d9c07662..a639df539cb9 100644 --- a/packages/SystemUI/unfold/src/com/android/systemui/unfold/UnfoldRemoteModule.kt +++ b/packages/SystemUI/unfold/src/com/android/systemui/unfold/UnfoldRemoteModule.kt @@ -17,6 +17,7 @@ package com.android.systemui.unfold import com.android.systemui.unfold.config.UnfoldTransitionConfig +import com.android.systemui.unfold.dagger.UseReceivingFilter import com.android.systemui.unfold.progress.RemoteUnfoldTransitionReceiver import com.android.systemui.unfold.util.ATraceLoggerTransitionProgressListener import dagger.Module @@ -42,4 +43,6 @@ class UnfoldRemoteModule { remoteReceiver.addCallback(traceListener) return Optional.of(remoteReceiver) } + + @Provides @UseReceivingFilter fun useReceivingFilter(): Boolean = true } diff --git a/packages/SystemUI/unfold/src/com/android/systemui/unfold/dagger/UseReceivingFilter.kt b/packages/SystemUI/unfold/src/com/android/systemui/unfold/dagger/UseReceivingFilter.kt new file mode 100644 index 000000000000..60e9307e8457 --- /dev/null +++ b/packages/SystemUI/unfold/src/com/android/systemui/unfold/dagger/UseReceivingFilter.kt @@ -0,0 +1,20 @@ +/* + * Copyright (C) 2022 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.systemui.unfold.dagger + +import javax.inject.Qualifier + +/** Annotates whether to use a filter in [RemoteUnfoldTransitionReceiver]. */ +@Qualifier @Retention(AnnotationRetention.RUNTIME) annotation class UseReceivingFilter diff --git a/packages/SystemUI/unfold/src/com/android/systemui/unfold/progress/RemoteUnfoldTransitionReceiver.kt b/packages/SystemUI/unfold/src/com/android/systemui/unfold/progress/RemoteUnfoldTransitionReceiver.kt index 5e4bcc97520e..b2c26c455b76 100644 --- a/packages/SystemUI/unfold/src/com/android/systemui/unfold/progress/RemoteUnfoldTransitionReceiver.kt +++ b/packages/SystemUI/unfold/src/com/android/systemui/unfold/progress/RemoteUnfoldTransitionReceiver.kt @@ -16,9 +16,13 @@ package com.android.systemui.unfold.progress +import android.util.Log +import androidx.annotation.BinderThread +import androidx.annotation.FloatRange import com.android.systemui.unfold.UnfoldTransitionProgressProvider import com.android.systemui.unfold.UnfoldTransitionProgressProvider.TransitionProgressListener import com.android.systemui.unfold.dagger.UnfoldMain +import com.android.systemui.unfold.dagger.UseReceivingFilter import java.util.concurrent.Executor import javax.inject.Inject @@ -30,21 +34,40 @@ import javax.inject.Inject */ class RemoteUnfoldTransitionReceiver @Inject -constructor(@UnfoldMain private val executor: Executor) : - UnfoldTransitionProgressProvider, IUnfoldTransitionListener.Stub() { +constructor( + @UseReceivingFilter useReceivingFilter: Boolean, + @UnfoldMain private val executor: Executor +) : UnfoldTransitionProgressProvider, IUnfoldTransitionListener.Stub() { private val listeners: MutableSet = mutableSetOf() + private val outputProgressListener = ProcessedProgressListener() + private val filter: TransitionProgressListener? = + if (useReceivingFilter) { + UnfoldRemoteFilter(outputProgressListener) + } else { + null + } + @BinderThread override fun onTransitionStarted() { - executor.execute { listeners.forEach { it.onTransitionStarted() } } + executor.execute { + filter?.onTransitionStarted() ?: outputProgressListener.onTransitionStarted() + } } + @BinderThread override fun onTransitionProgress(progress: Float) { - executor.execute { listeners.forEach { it.onTransitionProgress(progress) } } + executor.execute { + filter?.onTransitionProgress(progress) + ?: outputProgressListener.onTransitionProgress(progress) + } } + @BinderThread override fun onTransitionFinished() { - executor.execute { listeners.forEach { it.onTransitionFinished() } } + executor.execute { + filter?.onTransitionFinished() ?: outputProgressListener.onTransitionFinished() + } } override fun addCallback(listener: TransitionProgressListener) { @@ -58,4 +81,30 @@ constructor(@UnfoldMain private val executor: Executor) : override fun destroy() { listeners.clear() } + + private inner class ProcessedProgressListener : TransitionProgressListener { + override fun onTransitionStarted() { + log { "onTransitionStarted" } + listeners.forEach { it.onTransitionStarted() } + } + + override fun onTransitionProgress(@FloatRange(from = 0.0, to = 1.0) progress: Float) { + log { "onTransitionProgress" } + listeners.forEach { it.onTransitionProgress(progress) } + } + + override fun onTransitionFinished() { + log { "onTransitionFinished" } + listeners.forEach { it.onTransitionFinished() } + } + } + + private fun log(s: () -> String) { + if (DEBUG) { + Log.d(TAG, s()) + } + } } + +private const val TAG = "RemoteUnfoldReceiver" +private val DEBUG = false diff --git a/packages/SystemUI/unfold/src/com/android/systemui/unfold/progress/UnfoldRemoteFilter.kt b/packages/SystemUI/unfold/src/com/android/systemui/unfold/progress/UnfoldRemoteFilter.kt new file mode 100644 index 000000000000..0b019d1285e3 --- /dev/null +++ b/packages/SystemUI/unfold/src/com/android/systemui/unfold/progress/UnfoldRemoteFilter.kt @@ -0,0 +1,85 @@ +package com.android.systemui.unfold.progress + +import android.os.Trace +import android.util.Log +import androidx.dynamicanimation.animation.FloatPropertyCompat +import androidx.dynamicanimation.animation.SpringAnimation +import androidx.dynamicanimation.animation.SpringForce +import com.android.systemui.unfold.UnfoldTransitionProgressProvider + +/** + * Makes progress received from other processes resilient to jank. + * + * Sender and receiver processes might have different frame-rates. If the sending process is + * dropping a frame due to jank (or generally because it's main thread is too busy), we don't want + * the receiving process to drop progress frames as well. For this reason, a spring animator pass + * (with very high stiffness) is applied to the incoming progress. This adds a small delay to the + * progress (~30ms), but guarantees an always smooth animation on the receiving end. + */ +class UnfoldRemoteFilter( + private val listener: UnfoldTransitionProgressProvider.TransitionProgressListener +) : UnfoldTransitionProgressProvider.TransitionProgressListener { + + private val springAnimation = + SpringAnimation(this, AnimationProgressProperty).apply { + spring = + SpringForce().apply { + dampingRatio = SpringForce.DAMPING_RATIO_NO_BOUNCY + stiffness = 100_000f + finalPosition = 1.0f + } + setMinValue(0f) + setMaxValue(1f) + minimumVisibleChange = 0.001f + } + + private var inProgress = false + + private var processedProgress: Float = 0.0f + set(newProgress) { + if (inProgress) { + logCounter({ "$TAG#filtered_progress" }, newProgress) + listener.onTransitionProgress(newProgress) + } else { + Log.e(TAG, "Filtered progress received received while animation not in progress.") + } + field = newProgress + } + + override fun onTransitionStarted() { + listener.onTransitionStarted() + inProgress = true + } + + override fun onTransitionProgress(progress: Float) { + logCounter({ "$TAG#plain_remote_progress" }, progress) + if (inProgress) { + springAnimation.animateToFinalPosition(progress) + } else { + Log.e(TAG, "Progress received while not in progress.") + } + } + + override fun onTransitionFinished() { + inProgress = false + listener.onTransitionFinished() + } + + private object AnimationProgressProperty : + FloatPropertyCompat("UnfoldRemoteFilter") { + + override fun setValue(provider: UnfoldRemoteFilter, value: Float) { + provider.processedProgress = value + } + + override fun getValue(provider: UnfoldRemoteFilter): Float = provider.processedProgress + } + private fun logCounter(name: () -> String, progress: Float) { + if (DEBUG) { + Trace.setCounter(name(), (progress * 100).toLong()) + } + } +} + +private val TAG = "UnfoldRemoteFilter" +private val DEBUG = false -- cgit v1.2.3-59-g8ed1b