From 3d0f6d2a5ce6a0e7ae48f638a78703313536068b Mon Sep 17 00:00:00 2001 From: Robert Horvath Date: Tue, 12 Apr 2022 18:53:24 +0200 Subject: [PATCH] Improve handling of TV PiPs bigger than movement bounds For very tall/wide expanded PiPs, the positioning algorithm considered the anchor position as invalid, as it crossed the movement bounds. It assumed that if no free valid position is found the PiP must be stashed and there exist keep clear areas overlapping the unstash position that caused that stashing. But in the case of these big PiPs there were no overlapping keep clear areas, resulting in a NPE. This change improves handling of oversize PiPs, allowing the anchor position to cross movement bounds without stashing, and improves null safety when searching for the stash position. Bug: 228948019 Test: atest TvPipKeepClearAlgorithmTest Change-Id: I6a7c6c316d03ca8902b9cfb1e0c4abcb873eff18 --- .../shell/pip/tv/TvPipKeepClearAlgorithm.kt | 124 ++++++++++-------- .../pip/tv/TvPipKeepClearAlgorithmTest.kt | 23 ++++ 2 files changed, 94 insertions(+), 53 deletions(-) diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipKeepClearAlgorithm.kt b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipKeepClearAlgorithm.kt index 09d202abfbdea..07dccd58abfdd 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipKeepClearAlgorithm.kt +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipKeepClearAlgorithm.kt @@ -131,10 +131,10 @@ class TvPipKeepClearAlgorithm(private val clock: () -> Long) { val pipSizeWithAllDecors = addDecors(pipSize) val pipAnchorBoundsWithAllDecors = - getNormalPipAnchorBounds(pipSizeWithAllDecors, transformedMovementBounds) + getNormalPipAnchorBounds(pipSizeWithAllDecors, transformedMovementBounds) val pipAnchorBoundsWithPermanentDecors = - removeTemporaryDecorsTransformed(pipAnchorBoundsWithAllDecors) + removeTemporaryDecorsTransformed(pipAnchorBoundsWithAllDecors) val result = calculatePipPositionTransformed( pipAnchorBoundsWithPermanentDecors, transformedRestrictedAreas, @@ -150,7 +150,7 @@ class TvPipKeepClearAlgorithm(private val clock: () -> Long) { return Placement( pipBounds, anchorBounds, - getStashType(pipBounds, movementBounds), + getStashType(pipBounds, unstashedDestBounds), unstashedDestBounds, result.unstashTime ) @@ -185,7 +185,10 @@ class TvPipKeepClearAlgorithm(private val clock: () -> Long) { restrictedAreas: Set, unrestrictedAreas: Set ): Placement { - if (restrictedAreas.isEmpty() && unrestrictedAreas.isEmpty()) { + // If PiP is not covered by any keep clear areas, we can leave it at the anchor bounds + val keepClearAreas = restrictedAreas + unrestrictedAreas + if (keepClearAreas.none { it.intersects(pipAnchorBounds) }) { + lastAreasOverlappingUnstashPosition = emptySet() return Placement(pipAnchorBounds, pipAnchorBounds) } @@ -204,9 +207,8 @@ class TvPipKeepClearAlgorithm(private val clock: () -> Long) { ?: findFreeMovePosition(pipAnchorBounds, emptySet(), unrestrictedAreas) ?: pipAnchorBounds - val keepClearAreas = restrictedAreas + unrestrictedAreas val areasOverlappingUnstashPosition = - keepClearAreas.filter { Rect.intersects(it, unstashBounds) }.toSet() + keepClearAreas.filterTo(mutableSetOf()) { it.intersects(unstashBounds) } val areasOverlappingUnstashPositionChanged = !lastAreasOverlappingUnstashPosition.containsAll(areasOverlappingUnstashPosition) lastAreasOverlappingUnstashPosition = areasOverlappingUnstashPosition @@ -228,19 +230,22 @@ class TvPipKeepClearAlgorithm(private val clock: () -> Long) { return Placement( stashedBounds, pipAnchorBounds, - getStashType(stashedBounds, transformedMovementBounds), + getStashType(stashedBounds, unstashBounds), unstashBounds, unstashTime ) } @PipBoundsState.StashType - private fun getStashType(stashedBounds: Rect, movementBounds: Rect): Int { + private fun getStashType(stashedBounds: Rect, unstashedDestBounds: Rect?): Int { + if (unstashedDestBounds == null) { + return STASH_TYPE_NONE + } return when { - stashedBounds.left < movementBounds.left -> STASH_TYPE_LEFT - stashedBounds.right > movementBounds.right -> STASH_TYPE_RIGHT - stashedBounds.top < movementBounds.top -> STASH_TYPE_TOP - stashedBounds.bottom > movementBounds.bottom -> STASH_TYPE_BOTTOM + stashedBounds.left < unstashedDestBounds.left -> STASH_TYPE_LEFT + stashedBounds.right > unstashedDestBounds.right -> STASH_TYPE_RIGHT + stashedBounds.top < unstashedDestBounds.top -> STASH_TYPE_TOP + stashedBounds.bottom > unstashedDestBounds.bottom -> STASH_TYPE_BOTTOM else -> STASH_TYPE_NONE } } @@ -368,57 +373,69 @@ class TvPipKeepClearAlgorithm(private val clock: () -> Long) { val areasOverlappingPipX = keepClearAreas.filter { it.intersectsX(bounds) } val areasOverlappingPipY = keepClearAreas.filter { it.intersectsY(bounds) } - if (screenBounds.bottom - bounds.bottom <= bounds.top - screenBounds.top) { - val fullStashTop = screenBounds.bottom - stashOffset + if (areasOverlappingPipX.isNotEmpty()) { + if (screenBounds.bottom - bounds.bottom <= bounds.top - screenBounds.top) { + val fullStashTop = screenBounds.bottom - stashOffset - val maxBottom = areasOverlappingPipX.maxByOrNull { it.bottom }!!.bottom - val partialStashTop = maxBottom + pipAreaPadding + val maxBottom = areasOverlappingPipX.maxByOrNull { it.bottom }!!.bottom + val partialStashTop = maxBottom + pipAreaPadding - val downPosition = Rect(bounds) - downPosition.offsetTo(bounds.left, min(fullStashTop, partialStashTop)) - stashCandidates += downPosition - } - if (screenBounds.bottom - bounds.bottom >= bounds.top - screenBounds.top) { - val fullStashBottom = screenBounds.top - bounds.height() + stashOffset + val newTop = min(fullStashTop, partialStashTop) + if (newTop > bounds.top) { + val downPosition = Rect(bounds) + downPosition.offsetTo(bounds.left, newTop) + stashCandidates += downPosition + } + } + if (screenBounds.bottom - bounds.bottom >= bounds.top - screenBounds.top) { + val fullStashBottom = screenBounds.top - bounds.height() + stashOffset - val minTop = areasOverlappingPipX.minByOrNull { it.top }!!.top - val partialStashBottom = minTop - bounds.height() - pipAreaPadding + val minTop = areasOverlappingPipX.minByOrNull { it.top }!!.top + val partialStashBottom = minTop - bounds.height() - pipAreaPadding - val upPosition = Rect(bounds) - upPosition.offsetTo(bounds.left, max(fullStashBottom, partialStashBottom)) - stashCandidates += upPosition + val newTop = max(fullStashBottom, partialStashBottom) + if (newTop < bounds.top) { + val upPosition = Rect(bounds) + upPosition.offsetTo(bounds.left, newTop) + stashCandidates += upPosition + } + } } - if (screenBounds.right - bounds.right <= bounds.left - screenBounds.left) { - val fullStashRight = screenBounds.right - stashOffset + if (areasOverlappingPipY.isNotEmpty()) { + if (screenBounds.right - bounds.right <= bounds.left - screenBounds.left) { + val fullStashRight = screenBounds.right - stashOffset - val maxRight = areasOverlappingPipY.maxByOrNull { it.right }!!.right - val partialStashRight = maxRight + pipAreaPadding + val maxRight = areasOverlappingPipY.maxByOrNull { it.right }!!.right + val partialStashRight = maxRight + pipAreaPadding - val rightPosition = Rect(bounds) - rightPosition.offsetTo(min(fullStashRight, partialStashRight), bounds.top) - stashCandidates += rightPosition - } - if (screenBounds.right - bounds.right >= bounds.left - screenBounds.left) { - val fullStashLeft = screenBounds.left - bounds.width() + stashOffset + val newLeft = min(fullStashRight, partialStashRight) + if (newLeft > bounds.left) { + val rightPosition = Rect(bounds) + rightPosition.offsetTo(newLeft, bounds.top) + stashCandidates += rightPosition + } + } + if (screenBounds.right - bounds.right >= bounds.left - screenBounds.left) { + val fullStashLeft = screenBounds.left - bounds.width() + stashOffset - val minLeft = areasOverlappingPipY.minByOrNull { it.left }!!.left - val partialStashLeft = minLeft - bounds.width() - pipAreaPadding + val minLeft = areasOverlappingPipY.minByOrNull { it.left }!!.left + val partialStashLeft = minLeft - bounds.width() - pipAreaPadding - val leftPosition = Rect(bounds) - leftPosition.offsetTo(max(fullStashLeft, partialStashLeft), bounds.top) - stashCandidates += leftPosition - } - - if (stashCandidates.isEmpty()) { - return bounds + val newLeft = max(fullStashLeft, partialStashLeft) + if (newLeft < bounds.left) { + val leftPosition = Rect(bounds) + leftPosition.offsetTo(newLeft, bounds.top) + stashCandidates += leftPosition + } + } } return stashCandidates.minByOrNull { val dx = abs(it.left - bounds.left) val dy = abs(it.top - bounds.top) return@minByOrNull dx + dy - }!! + } ?: bounds } /** @@ -768,7 +785,7 @@ class TvPipKeepClearAlgorithm(private val clock: () -> Long) { } /** - * Adds space around [size] to leave space for decorations that will be drawn around the pip + * Adds space around [size] to leave space for decorations that will be drawn around the PiP */ private fun addDecors(size: Size): Size { val bounds = Rect(0, 0, size.width, size.height) @@ -779,7 +796,7 @@ class TvPipKeepClearAlgorithm(private val clock: () -> Long) { } /** - * Removes the space that was reserved for permanent decorations around the pip + * Removes the space that was reserved for permanent decorations around the PiP * @param bounds the bounds (in screen space) to remove the insets from */ private fun removePermanentDecors(bounds: Rect): Rect { @@ -789,19 +806,20 @@ class TvPipKeepClearAlgorithm(private val clock: () -> Long) { } /** - * Removes the space that was reserved for temporary decorations around the pip + * Removes the space that was reserved for temporary decorations around the PiP * @param bounds the bounds (in base case) to remove the insets from */ private fun removeTemporaryDecorsTransformed(bounds: Rect): Rect { if (pipTemporaryDecorInsets == Insets.NONE) return bounds - var reverseInsets = Insets.subtract(Insets.NONE, pipTemporaryDecorInsets) - var boundsInScreenSpace = fromTransformedSpace(bounds) + val reverseInsets = Insets.subtract(Insets.NONE, pipTemporaryDecorInsets) + val boundsInScreenSpace = fromTransformedSpace(bounds) boundsInScreenSpace.inset(reverseInsets) return toTransformedSpace(boundsInScreenSpace) } private fun Rect.offsetCopy(dx: Int, dy: Int) = Rect(this).apply { offset(dx, dy) } - private fun Rect.intersectsY(other: Rect) = bottom >= other.top && top <= other.bottom private fun Rect.intersectsX(other: Rect) = right >= other.left && left <= other.right + private fun Rect.intersectsY(other: Rect) = bottom >= other.top && top <= other.bottom + private fun Rect.intersects(other: Rect) = intersectsX(other) && intersectsY(other) } diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip/tv/TvPipKeepClearAlgorithmTest.kt b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip/tv/TvPipKeepClearAlgorithmTest.kt index 9919214642fca..46f388d0ce0ec 100644 --- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip/tv/TvPipKeepClearAlgorithmTest.kt +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip/tv/TvPipKeepClearAlgorithmTest.kt @@ -25,6 +25,7 @@ import org.junit.runner.RunWith import com.android.wm.shell.pip.PipBoundsState.STASH_TYPE_NONE import com.android.wm.shell.pip.PipBoundsState.STASH_TYPE_BOTTOM import com.android.wm.shell.pip.PipBoundsState.STASH_TYPE_RIGHT +import com.android.wm.shell.pip.PipBoundsState.STASH_TYPE_TOP import com.android.wm.shell.pip.tv.TvPipKeepClearAlgorithm.Placement import org.junit.Before import org.junit.Test @@ -433,6 +434,28 @@ class TvPipKeepClearAlgorithmTest { assertEquals(currentTime + algorithm.stashDuration, placement.unstashTime) } + @Test + fun test_ExpandedPiPHeightExceedsMovementBounds_AtAnchor() { + gravity = Gravity.RIGHT or Gravity.CENTER_VERTICAL + pipSize = Size(DEFAULT_PIP_SIZE.width, SCREEN_SIZE.height) + testAnchorPosition() + } + + @Test + fun test_ExpandedPiPHeightExceedsMovementBounds_BottomBar_StashedUp() { + gravity = Gravity.RIGHT or Gravity.CENTER_VERTICAL + pipSize = Size(DEFAULT_PIP_SIZE.width, SCREEN_SIZE.height) + val bottomBar = makeBottomBar(96) + unrestrictedAreas.add(bottomBar) + + val expectedBounds = getExpectedAnchorBounds() + expectedBounds.offset(0, -bottomBar.height() - PADDING) + val placement = getActualPlacement() + assertEquals(expectedBounds, placement.bounds) + assertEquals(STASH_TYPE_TOP, placement.stashType) + assertEquals(getExpectedAnchorBounds(), placement.unstashDestinationBounds) + } + @Test fun test_PipInsets() { val permInsets = Insets.of(-1, -2, -3, -4)