From f409d7f5880f8d705625adc46241677339a2ccfe Mon Sep 17 00:00:00 2001 From: Selim Cinek Date: Fri, 26 Jun 2020 21:34:04 -0700 Subject: [PATCH] Ensured that all players have the same size Previously, Media players could have different sizes if for example their album art was gone. We now enforce that all of them have the same size, determined by the max of their wrap_content heights. Fixes: 159864229 Test: add a player without album art, e.g. a resumable player. Observe consistent measurements with others Change-Id: Iaacb2b78b7d92ed21e382d69b3bf599dbd7b1a6e --- .../systemui/media/MediaCarouselController.kt | 1 + .../com/android/systemui/media/MediaHost.kt | 3 +- .../systemui/media/MediaHostStatesManager.kt | 13 +++++++- .../systemui/media/MediaViewController.kt | 33 +++++++++++++++++-- 4 files changed, 44 insertions(+), 6 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt b/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt index cd169244b910c..1a730c39dcd3e 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt @@ -299,6 +299,7 @@ class MediaCarouselController @Inject constructor( if (numPages == 1) { pageIndicator.setLocation(0f) } + updatePageIndicatorAlpha() } /** diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaHost.kt b/packages/SystemUI/src/com/android/systemui/media/MediaHost.kt index 07a7e618b301a..570de635ea1d2 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaHost.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaHost.kt @@ -1,6 +1,5 @@ package com.android.systemui.media -import android.graphics.PointF import android.graphics.Rect import android.util.ArraySet import android.view.View @@ -101,7 +100,7 @@ class MediaHost @Inject constructor( } // This will trigger a state change that ensures that we now have a state available state.measurementInput = input - return mediaHostStatesManager.getPlayerDimensions(state) + return mediaHostStatesManager.updateCarouselDimensions(location, state) } } diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaHostStatesManager.kt b/packages/SystemUI/src/com/android/systemui/media/MediaHostStatesManager.kt index f90af2a01de0f..d3954b70ca710 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaHostStatesManager.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaHostStatesManager.kt @@ -31,6 +31,12 @@ class MediaHostStatesManager @Inject constructor() { private val callbacks: MutableSet = mutableSetOf() private val controllers: MutableSet = mutableSetOf() + /** + * The overall sizes of the carousel. This is needed to make sure all players in the carousel + * have equal size. + */ + val carouselSizes: MutableMap = mutableMapOf() + /** * A map with all media states of all locations. */ @@ -45,6 +51,7 @@ class MediaHostStatesManager @Inject constructor() { if (!hostState.equals(currentState)) { val newState = hostState.copy() mediaHostStates.put(location, newState) + updateCarouselDimensions(location, hostState) // First update all the controllers to ensure they get the chance to measure for (controller in controllers) { controller.stateCallback.onHostStateChanged(location, newState) @@ -61,7 +68,10 @@ class MediaHostStatesManager @Inject constructor() { * Get the dimensions of all players combined, which determines the overall height of the * media carousel and the media hosts. */ - fun getPlayerDimensions(hostState: MediaHostState): MeasurementOutput { + fun updateCarouselDimensions( + @MediaLocation location: Int, + hostState: MediaHostState + ): MeasurementOutput { val result = MeasurementOutput(0, 0) for (controller in controllers) { val measurement = controller.getMeasurementsForState(hostState) @@ -74,6 +84,7 @@ class MediaHostStatesManager @Inject constructor() { } } } + carouselSizes[location] = result return result } diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaViewController.kt b/packages/SystemUI/src/com/android/systemui/media/MediaViewController.kt index 033a42a03240a..83cfdd5f2699d 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaViewController.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaViewController.kt @@ -78,6 +78,16 @@ class MediaViewController @Inject constructor( */ private val tmpState = TransitionViewState() + /** + * A temporary state used to store intermediate measurements. + */ + private val tmpState2 = TransitionViewState() + + /** + * A temporary state used to store intermediate measurements. + */ + private val tmpState3 = TransitionViewState() + /** * A temporary cache key to be used to look up cache entries */ @@ -304,8 +314,8 @@ class MediaViewController @Inject constructor( // Obtain the view state that we'd want to be at the end // 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 - val endViewState = obtainViewState(endHostState) ?: return - + var endViewState = obtainViewState(endHostState) ?: return + endViewState = updateViewStateToCarouselSize(endViewState, endLocation, tmpState2)!! layoutController.setMeasureState(endViewState) // If the view isn't bound, we can drop the animation, otherwise we'll execute it @@ -315,7 +325,8 @@ class MediaViewController @Inject constructor( } val result: TransitionViewState - val startViewState = obtainViewState(startHostState) + var startViewState = obtainViewState(startHostState) + startViewState = updateViewStateToCarouselSize(startViewState, startLocation, tmpState3) if (!endHostState.visible) { // Let's handle the case where the end is gone first. In this case we take the @@ -350,6 +361,22 @@ class MediaViewController @Inject constructor( animationDelay) } + private fun updateViewStateToCarouselSize( + viewState: TransitionViewState?, + location: Int, + outState: TransitionViewState + ) : TransitionViewState? { + val result = viewState?.copy(outState) ?: return null + val overrideSize = mediaHostStatesManager.carouselSizes[location] + 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) + } + return result + } + /** * Retrieves the [TransitionViewState] and [MediaHostState] of a [@MediaLocation]. * In the event of [location] not being visible, [locationWhenHidden] will be used instead.