diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/VideoView.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/VideoView.kt index 0160a12468..d7263930d6 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/VideoView.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/VideoView.kt @@ -115,13 +115,16 @@ fun VideoView( // Resolve the aspect ratio once per composition. Prime the URL-keyed cache from the imeta // dim tag so the next time this video appears (PiP, dialog, list re-enter) the cache hits - // without waiting for ExoPlayer's onVideoSizeChanged. + // without waiting for ExoPlayer's onVideoSizeChanged. Keys are primitive width/height so + // a freshly parsed DimensionTag instance for the same event doesn't re-run this lambda — + // DimensionTag uses reference equality, not structural. + val dimW = dimensions?.width + val dimH = dimensions?.height val ratio = - remember(videoUri, dimensions) { - val fromDim = dimensions?.takeIf { it.hasSize() } - if (fromDim != null) { - MediaAspectRatioCache.add(videoUri, fromDim.width, fromDim.height) - fromDim.aspectRatio() + remember(videoUri, dimW, dimH) { + if (dimW != null && dimH != null && dimW > 0 && dimH > 0) { + MediaAspectRatioCache.add(videoUri, dimW, dimH) + dimW.toFloat() / dimH.toFloat() } else { MediaAspectRatioCache.get(videoUri) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/GradientOverlay.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/GradientOverlay.kt index 938aa8ef9b..be5d946f8e 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/GradientOverlay.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/GradientOverlay.kt @@ -38,24 +38,33 @@ import androidx.compose.ui.unit.dp private val FadeIn = fadeIn() private val FadeOut = fadeOut() -private val TopGradientColors = - listOf( - Color.Black.copy(alpha = 0.6f), - Color.Black.copy(alpha = 0.3f), - Color.Transparent, +// Both gradient brushes are static; pre-build them once at class init so we don't allocate a +// new Brush on every recomposition while the controllers are visible (which is most of the +// time during playback / interaction). +private val TopGradientBrush = + Brush.verticalGradient( + colors = + listOf( + Color.Black.copy(alpha = 0.6f), + Color.Black.copy(alpha = 0.3f), + Color.Transparent, + ), ) -private val BottomGradientColors = - listOf( - Color.Transparent, - Color.Black.copy(alpha = 0.4f), - Color.Black.copy(alpha = 0.7f), +private val BottomGradientBrush = + Brush.verticalGradient( + colors = + listOf( + Color.Transparent, + Color.Black.copy(alpha = 0.4f), + Color.Black.copy(alpha = 0.7f), + ), ) @Composable private fun GradientOverlay( controllerVisible: State, - colors: List, + brush: Brush, height: Dp, modifier: Modifier = Modifier, ) { @@ -70,7 +79,7 @@ private fun GradientOverlay( Modifier .fillMaxWidth() .height(height) - .background(brush = Brush.verticalGradient(colors = colors)), + .background(brush = brush), ) } } @@ -80,11 +89,11 @@ fun TopGradientOverlay( controllerVisible: State, modifier: Modifier = Modifier, height: Dp = 80.dp, -) = GradientOverlay(controllerVisible, TopGradientColors, height, modifier) +) = GradientOverlay(controllerVisible, TopGradientBrush, height, modifier) @Composable fun BottomGradientOverlay( controllerVisible: State, modifier: Modifier = Modifier, height: Dp = 120.dp, -) = GradientOverlay(controllerVisible, BottomGradientColors, height, modifier) +) = GradientOverlay(controllerVisible, BottomGradientBrush, height, modifier) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/MuteButton.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/MuteButton.kt index 5c44ba3c89..787c7e30a3 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/MuteButton.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/MuteButton.kt @@ -47,9 +47,7 @@ import com.vitorpamplona.amethyst.ui.theme.Size30Modifier import com.vitorpamplona.amethyst.ui.theme.Size50Modifier import com.vitorpamplona.amethyst.ui.theme.ThemeComparisonColumn import com.vitorpamplona.amethyst.ui.theme.VolumeBottomIconSize -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.delay -import kotlinx.coroutines.launch @Preview @Composable @@ -79,11 +77,12 @@ fun MuteButton( ) } + // LaunchedEffect already runs on Main, and delay() suspends without holding a thread, so + // the previous launch(Dispatchers.IO) was just unnecessary dispatcher hopping for a state + // mutation that's also fine on Main. LaunchedEffect(key1 = controllerVisible) { - launch(Dispatchers.IO) { - delay(2000) - holdOn.value = false - } + delay(2000) + holdOn.value = false } val mutedInstance = remember(startingMuteState) { mutableStateOf(startingMuteState) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/OverflowMenu.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/OverflowMenu.kt index b8a115a5c0..1bf719a92e 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/OverflowMenu.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/OverflowMenu.kt @@ -50,6 +50,8 @@ import com.vitorpamplona.amethyst.ui.theme.Size20Modifier import com.vitorpamplona.amethyst.ui.theme.Size50Modifier import com.vitorpamplona.amethyst.ui.theme.ThemeComparisonColumn import com.vitorpamplona.amethyst.ui.theme.VolumeBottomIconSize +import kotlinx.collections.immutable.ImmutableList +import kotlinx.collections.immutable.persistentListOf private val FadeIn = fadeIn() private val FadeOut = fadeOut() @@ -60,7 +62,7 @@ fun OverflowMenuButtonPreview() { ThemeComparisonColumn { Box(Modifier.background(BitcoinOrange)) { OverflowMenuButton( - actions = listOf(VideoPlayerAction.Share, VideoPlayerAction.Download, VideoPlayerAction.PictureInPicture), + actions = persistentListOf(VideoPlayerAction.Share, VideoPlayerAction.Download, VideoPlayerAction.PictureInPicture), startingMuteState = false, onFullscreenClick = {}, onMuteClick = {}, @@ -76,7 +78,7 @@ fun OverflowMenuButtonPreview() { @Composable fun AnimatedOverflowMenuButton( controllerVisible: State, - actions: List, + actions: ImmutableList, startingMuteState: Boolean, onFullscreenClick: (() -> Unit)?, onMuteClick: () -> Unit, @@ -107,7 +109,7 @@ fun AnimatedOverflowMenuButton( @Composable fun OverflowMenuButton( - actions: List, + actions: ImmutableList, startingMuteState: Boolean, onFullscreenClick: (() -> Unit)?, onMuteClick: () -> Unit, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/RenderTopButtons.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/RenderTopButtons.kt index d7c1645f0d..5d182bd842 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/RenderTopButtons.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/RenderTopButtons.kt @@ -70,6 +70,7 @@ import com.vitorpamplona.amethyst.ui.theme.PinBottomIconSize import com.vitorpamplona.amethyst.ui.theme.Size20Modifier import com.vitorpamplona.amethyst.ui.theme.Size50Modifier import com.vitorpamplona.amethyst.ui.theme.ThemeComparisonColumn +import kotlinx.collections.immutable.toImmutableList @Preview @Composable @@ -194,7 +195,10 @@ fun RenderTopButtons( modifier: Modifier, accountViewModel: AccountViewModel, ) { - val buttonItems by accountViewModel.videoPlayerButtonItemsFlow().collectAsStateWithLifecycle() + // Hold the StateFlow itself across recompositions so collectAsStateWithLifecycle isn't + // keyed on the result of a property getter call that happens every recompose. + val buttonItemsFlow = remember(accountViewModel) { accountViewModel.videoPlayerButtonItemsFlow() } + val buttonItems by buttonItemsFlow.collectAsStateWithLifecycle() val shareDialogVisible = remember { mutableStateOf(false) } val saveAction = rememberSaveMediaAction { context -> @@ -212,17 +216,22 @@ fun RenderTopButtons( } val canFullscreen = onZoomClick != null + // ImmutableList so Compose can treat the action lists as stable parameters when they're + // passed through to AnimatedOverflowMenuButton — a plain List is unstable and forces the + // overflow tree to recompose whenever any unrelated parent state ticks. val topBarActions = remember(buttonItems, canFullscreen, hasMultipleQualities, isLive, pipSupported) { buttonItems .filter { it.location == VideoButtonLocation.TopBar && isAvailable(it.action) } .map { it.action } + .toImmutableList() } val overflowActions = remember(buttonItems, canFullscreen, hasMultipleQualities, isLive, pipSupported) { buttonItems .filter { it.location == VideoButtonLocation.OverflowMenu && isAvailable(it.action) } .map { it.action } + .toImmutableList() } Row(modifier) { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/GifVideoView.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/GifVideoView.kt index 1b33389f0e..e9bb15e63a 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/GifVideoView.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/GifVideoView.kt @@ -35,6 +35,7 @@ import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.SideEffect import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue +import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip @@ -68,7 +69,18 @@ fun GifVideoView( accountViewModel: AccountViewModel, thumbhash: String? = null, ) { - val ratio = dimensions?.aspectRatio() ?: MediaAspectRatioCache.get(videoUri) + // Keys are primitive width/height so a freshly parsed DimensionTag instance for the same + // event doesn't re-run this lambda — DimensionTag uses reference equality, not structural. + val dimW = dimensions?.width + val dimH = dimensions?.height + val ratio = + remember(videoUri, dimW, dimH) { + if (dimW != null && dimH != null && dimH > 0) { + dimW.toFloat() / dimH.toFloat() + } else { + MediaAspectRatioCache.get(videoUri) + } + } val autoPlay = accountViewModel.settings.autoPlayVideos() val borderModifier = if (roundedCorner) MaterialTheme.colorScheme.imageModifier else Modifier val context = LocalContext.current