From fab7478218872668c07c2514fd0cfe34a8ab6643 Mon Sep 17 00:00:00 2001 From: Selim Cinek Date: Mon, 12 Dec 2022 21:49:02 +0100 Subject: [PATCH] Fix clipping during media squishing and general behavior We were previously remeasuring in every frame when squashing media, because the measuredState was changed, leading to a requestLayout. We're now separating the height from the measureHeight to avoid this. Additionally do we adjust the height of the background children now properly, which were previously just based on the measured height but never updated. This fixed a couple of issues where the view could be stuck in an odd height because of the squishing. Test: Test media squishing in split shade, portait, landscape, observe normal squishing Test: atest SystemUITests Bug: 261534024 Change-Id: I357f261e3cbbb80ddd383caca74bbc3081465d7b --- .../media/controls/ui/MediaViewController.kt | 65 +++++++++++++++---- .../util/animation/TransitionLayout.kt | 28 +++++++- .../animation/TransitionLayoutController.kt | 9 +++ .../controls/ui/MediaViewControllerTest.kt | 10 ++- 4 files changed, 97 insertions(+), 15 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaViewController.kt b/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaViewController.kt index 4feb9844cb8b8..322421318cb80 100644 --- a/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaViewController.kt +++ b/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaViewController.kt @@ -82,6 +82,13 @@ constructor( R.id.header_artist, R.id.actionPlayPause, ) + + val backgroundIds = + setOf( + R.id.album_art, + R.id.turbulence_noise_view, + R.id.touch_ripple_view, + ) } /** A listener when the current dimensions of the player change */ @@ -295,7 +302,8 @@ constructor( squishFraction: Float ): TransitionViewState { val squishedViewState = viewState.copy() - squishedViewState.height = (squishedViewState.height * squishFraction).toInt() + val squishedHeight = (squishedViewState.measureHeight * squishFraction).toInt() + squishedViewState.height = squishedHeight controlIds.forEach { id -> squishedViewState.widgetStates.get(id)?.let { state -> state.alpha = calculateAlpha(squishFraction, CONTROLS_DELAY, DURATION) @@ -308,6 +316,14 @@ constructor( } } + // We are not overriding the squishedViewStates height but only the children to avoid + // them remeasuring the whole view. Instead it just remains as the original size + backgroundIds.forEach { id -> + squishedViewState.widgetStates.get(id)?.let { state -> + state.height = squishedHeight + } + } + RecommendationViewHolder.mediaContainersIds.forEach { id -> squishedViewState.widgetStates.get(id)?.let { state -> state.alpha = calculateAlpha(squishFraction, MEDIACONTAINERS_DELAY, DURATION) @@ -421,10 +437,9 @@ constructor( fun getMeasurementsForState(hostState: MediaHostState): MeasurementOutput? = traceSection("MediaViewController#getMeasurementsForState") { // measurements should never factor in the squish fraction - val viewState = - obtainViewState(hostState.copy().also { it.squishFraction = 1.0f }) ?: return null - measurement.measuredWidth = viewState.width - measurement.measuredHeight = viewState.height + val viewState = obtainViewState(hostState) ?: return null + measurement.measuredWidth = viewState.measureWidth + measurement.measuredHeight = viewState.measureHeight return measurement } @@ -453,7 +468,7 @@ constructor( // The view might not be bound yet or has never been measured and in that case will be // reset once the state is fully available var endViewState = obtainViewState(endHostState) ?: return - endViewState = updateViewStateToCarouselSize(endViewState, endLocation, tmpState2)!! + endViewState = updateViewStateSize(endViewState, endLocation, tmpState2)!! layoutController.setMeasureState(endViewState) // If the view isn't bound, we can drop the animation, otherwise we'll execute it @@ -464,7 +479,7 @@ constructor( val result: TransitionViewState var startViewState = obtainViewState(startHostState) - startViewState = updateViewStateToCarouselSize(startViewState, startLocation, tmpState3) + startViewState = updateViewStateSize(startViewState, startLocation, tmpState3) if (!endHostState.visible) { // Let's handle the case where the end is gone first. In this case we take the @@ -517,18 +532,38 @@ constructor( ) } - private fun updateViewStateToCarouselSize( + private fun updateViewStateSize( viewState: TransitionViewState?, location: Int, outState: TransitionViewState ): TransitionViewState? { - val result = viewState?.copy(outState) ?: return null + var result = viewState?.copy(outState) ?: return null + val state = mediaHostStatesManager.mediaHostStates[location] val overrideSize = mediaHostStatesManager.carouselSizes[location] + var overridden = false overrideSize?.let { // To be safe we're using a maximum here. The override size should always be set // properly though. - result.height = Math.max(it.measuredHeight, result.height) - result.width = Math.max(it.measuredWidth, result.width) + if (result.measureHeight != it.measuredHeight + || result.measureWidth != it.measuredWidth) { + result.measureHeight = Math.max(it.measuredHeight, result.measureHeight) + result.measureWidth = Math.max(it.measuredWidth, result.measureWidth) + // The measureHeight and the shown height should both be set to the overridden height + result.height = result.measureHeight + result.width = result.measureWidth + // Make sure all background views are also resized such that their size is correct + backgroundIds.forEach { id -> + result.widgetStates.get(id)?.let { state -> + state.height = result.height + state.width = result.width + } + } + overridden = true + } + } + if (overridden && state != null && state.squishFraction <= 1f) { + // Let's squish the media player if our size was overridden + result = squishViewState(result, state.squishFraction) } logger.logMediaSize("update to carousel", result.width, result.height) return result @@ -562,7 +597,13 @@ constructor( */ private fun obtainViewStateForLocation(@MediaLocation location: Int): TransitionViewState? { val mediaHostState = mediaHostStatesManager.mediaHostStates[location] ?: return null - return obtainViewState(mediaHostState) + val viewState = obtainViewState(mediaHostState) + if (viewState != null) { + // update the size of the viewstate for the location with the override + updateViewStateSize(viewState, location, tmpState) + return tmpState + } + return viewState } /** diff --git a/packages/SystemUI/src/com/android/systemui/util/animation/TransitionLayout.kt b/packages/SystemUI/src/com/android/systemui/util/animation/TransitionLayout.kt index 46611e03e588f..08ee0af17fb0b 100644 --- a/packages/SystemUI/src/com/android/systemui/util/animation/TransitionLayout.kt +++ b/packages/SystemUI/src/com/android/systemui/util/animation/TransitionLayout.kt @@ -59,8 +59,8 @@ class TransitionLayout @JvmOverloads constructor( */ var measureState: TransitionViewState = TransitionViewState() set(value) { - val newWidth = value.width - val newHeight = value.height + val newWidth = value.measureWidth + val newHeight = value.measureHeight if (newWidth != desiredMeasureWidth || newHeight != desiredMeasureHeight) { desiredMeasureWidth = newWidth desiredMeasureHeight = newHeight @@ -318,8 +318,28 @@ class TransitionLayout @JvmOverloads constructor( class TransitionViewState { var widgetStates: MutableMap = mutableMapOf() + + /** + * The visible width of this ViewState. This may differ from the measuredWidth when e.g. + * squishing the view + */ var width: Int = 0 + + /** + * The visible height of this ViewState. This may differ from the measuredHeight when e.g. + * squishing the view + */ var height: Int = 0 + + /** + * The height that determines the measured dimensions of the view + */ + var measureHeight: Int = 0 + + /** + * The width that determines the measured dimensions of the view + */ + var measureWidth: Int = 0 var alpha: Float = 1.0f val translation = PointF() val contentTranslation = PointF() @@ -328,6 +348,8 @@ class TransitionViewState { val copy = reusedState ?: TransitionViewState() copy.width = width copy.height = height + copy.measureHeight = measureHeight + copy.measureWidth = measureWidth copy.alpha = alpha copy.translation.set(translation.x, translation.y) copy.contentTranslation.set(contentTranslation.x, contentTranslation.y) @@ -348,6 +370,8 @@ class TransitionViewState { } width = transitionLayout.measuredWidth height = transitionLayout.measuredHeight + measureWidth = width + measureHeight = height translation.set(0.0f, 0.0f) contentTranslation.set(0.0f, 0.0f) alpha = 1.0f diff --git a/packages/SystemUI/src/com/android/systemui/util/animation/TransitionLayoutController.kt b/packages/SystemUI/src/com/android/systemui/util/animation/TransitionLayoutController.kt index 0c9b48ecd8444..5d80292931071 100644 --- a/packages/SystemUI/src/com/android/systemui/util/animation/TransitionLayoutController.kt +++ b/packages/SystemUI/src/com/android/systemui/util/animation/TransitionLayoutController.kt @@ -234,6 +234,15 @@ open class TransitionLayoutController { progress).toInt() height = MathUtils.lerp(startState.height.toFloat(), endState.height.toFloat(), progress).toInt() + // If we're at the start, let's measure with the starting dimensions, otherwise always + // with the end state + if (progress == 0.0f) { + measureWidth = startState.measureWidth + measureHeight = startState.measureHeight + } else { + measureWidth = endState.measureWidth + measureHeight = endState.measureHeight + } translation.x = MathUtils.lerp(startState.translation.x, endState.translation.x, progress) translation.y = MathUtils.lerp(startState.translation.y, endState.translation.y, diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaViewControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaViewControllerTest.kt index 6b7615557d836..35b0eb6784416 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaViewControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaViewControllerTest.kt @@ -58,6 +58,7 @@ class MediaViewControllerTest : SysuiTestCase() { @Mock private lateinit var mockCopiedState: TransitionViewState @Mock private lateinit var detailWidgetState: WidgetState @Mock private lateinit var controlWidgetState: WidgetState + @Mock private lateinit var bgWidgetState: WidgetState @Mock private lateinit var mediaTitleWidgetState: WidgetState @Mock private lateinit var mediaContainerWidgetState: WidgetState @@ -75,7 +76,10 @@ class MediaViewControllerTest : SysuiTestCase() { @Test fun testObtainViewState_applySquishFraction_toPlayerTransitionViewState_height() { mediaViewController.attach(player, MediaViewController.TYPE.PLAYER) - player.measureState = TransitionViewState().apply { this.height = 100 } + player.measureState = TransitionViewState().apply { + this.height = 100 + this.measureHeight = 100 + } mediaHostStateHolder.expansion = 1f val widthMeasureSpec = View.MeasureSpec.makeMeasureSpec(100, View.MeasureSpec.EXACTLY) val heightMeasureSpec = View.MeasureSpec.makeMeasureSpec(100, View.MeasureSpec.EXACTLY) @@ -85,10 +89,12 @@ class MediaViewControllerTest : SysuiTestCase() { // Test no squish mediaHostStateHolder.squishFraction = 1f assertTrue(mediaViewController.obtainViewState(mediaHostStateHolder)!!.height == 100) + assertTrue(mediaViewController.obtainViewState(mediaHostStateHolder)!!.measureHeight == 100) // Test half squish mediaHostStateHolder.squishFraction = 0.5f assertTrue(mediaViewController.obtainViewState(mediaHostStateHolder)!!.height == 50) + assertTrue(mediaViewController.obtainViewState(mediaHostStateHolder)!!.measureHeight == 100) } @Test @@ -104,10 +110,12 @@ class MediaViewControllerTest : SysuiTestCase() { // Test no squish mediaHostStateHolder.squishFraction = 1f assertTrue(mediaViewController.obtainViewState(mediaHostStateHolder)!!.height == 100) + assertTrue(mediaViewController.obtainViewState(mediaHostStateHolder)!!.measureHeight == 100) // Test half squish mediaHostStateHolder.squishFraction = 0.5f assertTrue(mediaViewController.obtainViewState(mediaHostStateHolder)!!.height == 50) + assertTrue(mediaViewController.obtainViewState(mediaHostStateHolder)!!.measureHeight == 100) } @Test