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
This commit is contained in:
Selim Cinek
2022-12-12 21:49:02 +01:00
parent 1f9eb48888
commit fab7478218
4 changed files with 97 additions and 15 deletions

View File

@@ -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
}
/**

View File

@@ -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<Int, WidgetState> = 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

View File

@@ -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,

View File

@@ -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