[ui-compose] Rename showSurface to coverSurface in PresentationState

This renaming helps prevent the consumer from including `PlayerSurface` in a Compose tree based on `state.showSurface`. This field intuitively felt like the default would be not to show the surface (correct), but had an unintended consequence of not initialising `AndroidExternal(Embedded)Surface` (since the PlayerSurface Composable would be omitted).

To avoid this confusion, the `PresentationState` should communicate to the consumer when the Surface is not ready and hence should be covered with some overlay (shutter) to prevent poor UX from observing Surface resizing/freezing/flickering.

PiperOrigin-RevId: 714951607
This commit is contained in:
jbibik 2025-01-13 06:56:01 -08:00 committed by Copybara-Service
parent d18ad57e30
commit 52387bb975
2 changed files with 17 additions and 11 deletions

View File

@ -114,17 +114,21 @@ private fun MediaPlayerScreen(player: Player, modifier: Modifier = Modifier) {
val presentationState = rememberPresentationState(player) val presentationState = rememberPresentationState(player)
val scaledModifier = Modifier.resizeWithContentScale(contentScale, presentationState.videoSizeDp) val scaledModifier = Modifier.resizeWithContentScale(contentScale, presentationState.videoSizeDp)
// Only use MediaPlayerScreen's modifier once for the top level Composable
Box(modifier) { Box(modifier) {
// Always leave PlayerSurface to be part of the Compose tree because it will be initialised in
// the process. If this composable is guarded by some condition, it might never become visible
// because the Player will not emit the relevant event, e.g. the first frame being ready.
PlayerSurface( PlayerSurface(
player = player, player = player,
surfaceType = SURFACE_TYPE_SURFACE_VIEW, surfaceType = SURFACE_TYPE_SURFACE_VIEW,
modifier = scaledModifier.noRippleClickable { showControls = !showControls }, modifier = scaledModifier.noRippleClickable { showControls = !showControls },
) )
if (!presentationState.showSurface) { if (presentationState.coverSurface) {
// hide the surface that is being prepared behind a shutter // Cover the surface that is being prepared with a shutter
// TODO: picking scaledModifier here makes the shutter invisible // Do not use scaledModifier here, makes the Box be measured at 0x0
Box(modifier.background(Color.Black)) Box(Modifier.matchParentSize().background(Color.Black))
} }
if (showControls) { if (showControls) {

View File

@ -57,8 +57,9 @@ fun rememberPresentationState(player: Player): PresentationState {
* [Density.toPx]. Note that for cases where `pixelWidthHeightRatio` is not equal to 1, the * [Density.toPx]. Note that for cases where `pixelWidthHeightRatio` is not equal to 1, the
* rescaling will be down, i.e. reducing the width or the height to achieve the same aspect ratio * rescaling will be down, i.e. reducing the width or the height to achieve the same aspect ratio
* in square pixels. * in square pixels.
* @property[showSurface] set to true when the Player emits [Player.EVENT_RENDERED_FIRST_FRAME] and * @property[coverSurface] set to false when the Player emits [Player.EVENT_RENDERED_FIRST_FRAME]
* reset to false on [Player.EVENT_TRACKS_CHANGED] depending on the number and type of tracks. * and reset back to true on [Player.EVENT_TRACKS_CHANGED] depending on the number and type of
* tracks.
* @property[keepContentOnReset] whether the currently displayed video frame or media artwork is * @property[keepContentOnReset] whether the currently displayed video frame or media artwork is
* kept visible when tracks change. Defaults to false. * kept visible when tracks change. Defaults to false.
*/ */
@ -67,7 +68,7 @@ class PresentationState(private val player: Player) {
var videoSizeDp: Size? by mutableStateOf(getVideoSizeDp(player)) var videoSizeDp: Size? by mutableStateOf(getVideoSizeDp(player))
private set private set
var showSurface by mutableStateOf(false) var coverSurface by mutableStateOf(true)
private set private set
var keepContentOnReset: Boolean = false var keepContentOnReset: Boolean = false
@ -86,7 +87,8 @@ class PresentationState(private val player: Player) {
} }
} }
if (events.contains(Player.EVENT_RENDERED_FIRST_FRAME)) { if (events.contains(Player.EVENT_RENDERED_FIRST_FRAME)) {
showSurface = true // open shutter, video available
coverSurface = false
} }
if (events.contains(Player.EVENT_TRACKS_CHANGED)) { if (events.contains(Player.EVENT_TRACKS_CHANGED)) {
maybeHideSurface(player) maybeHideSurface(player)
@ -112,17 +114,17 @@ class PresentationState(private val player: Player) {
player.isCommandAvailable(Player.COMMAND_GET_TRACKS) && !player.currentTracks.isEmpty player.isCommandAvailable(Player.COMMAND_GET_TRACKS) && !player.currentTracks.isEmpty
if (!shouldKeepSurfaceVisible(player)) { if (!shouldKeepSurfaceVisible(player)) {
if (!keepContentOnReset && !hasTracks) { if (!keepContentOnReset && !hasTracks) {
showSurface = false coverSurface = true
} }
if (hasTracks && !hasSelectedVideoTrack()) { if (hasTracks && !hasSelectedVideoTrack()) {
showSurface = false coverSurface = true
} }
} }
} }
private fun shouldKeepSurfaceVisible(player: Player): Boolean { private fun shouldKeepSurfaceVisible(player: Player): Boolean {
// Suppress the shutter if transitioning to an unprepared period within the same window. This // Suppress the shutter if transitioning to an unprepared period within the same window. This
// is necessary to avoid closing the shutter (i.e hiding the surface) when such a transition // is necessary to avoid closing the shutter (i.e covering the surface) when such a transition
// occurs. See: https://github.com/google/ExoPlayer/issues/5507. // occurs. See: https://github.com/google/ExoPlayer/issues/5507.
val timeline = val timeline =
if (player.isCommandAvailable(Player.COMMAND_GET_TIMELINE)) player.currentTimeline if (player.isCommandAvailable(Player.COMMAND_GET_TIMELINE)) player.currentTimeline