mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-12 09:13:23 +00:00
perf(video): tighten remember keys and stabilize controller-overlay tree
Round-4 audit cleanups. Each item is small but each runs on the hot path
that recomposes during every active video, so they add up while scrolling.
P1 — DimensionTag identity invalidating remember:
- DimensionTag (in quartz) is a regular class with no equals override, so
reference equality means a freshly parsed tag for the same event is !=
to the previous one. The remember(videoUri, dimensions) blocks added in
the earlier perf commits were re-running their lambda on every recompose.
Switch to primitive (width, height) keys in VideoView and GifVideoView so
the cache lookups + MediaAspectRatioCache writes only fire when the
dimensions actually change.
P1 — Static gradient brushes:
- TopGradientOverlay / BottomGradientOverlay were calling
Brush.verticalGradient(colors = colors) inside the modifier chain, which
allocated a fresh Brush on every recomposition while the controllers
were visible (i.e. on every active video most of the time). Pre-build
both brushes as file-level vals so they're allocated exactly once per
process.
P2 — ImmutableList for action collections:
- RenderTopButtons / AnimatedOverflowMenuButton / OverflowMenuButton were
passing List<VideoPlayerAction> across composable boundaries. Plain List
is unstable in Compose, forcing the overflow tree to recompose any time
an unrelated parent state (volume, tracks, controllerVisible) ticked.
Use ImmutableList end-to-end via toImmutableList() at the producer side.
P2 — videoPlayerButtonItemsFlow remember:
- accountViewModel.videoPlayerButtonItemsFlow() was being called fresh
every recomposition, with the result handed straight to
collectAsStateWithLifecycle. Hoist the call into remember(accountViewModel)
so the flow reference is stable.
P2 — MuteButton dispatcher cleanup:
- The 2-second hold timer was using LaunchedEffect { launch(Dispatchers.IO)
{ delay(2000); holdOn.value = false } }. The wrapped launch was just
redundant dispatcher hopping — delay() doesn't hold a thread and the
Compose write is fine on Main. Inline it.
This commit is contained in:
+9
-6
@@ -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)
|
||||
}
|
||||
|
||||
+23
-14
@@ -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<Boolean>,
|
||||
colors: List<Color>,
|
||||
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<Boolean>,
|
||||
modifier: Modifier = Modifier,
|
||||
height: Dp = 80.dp,
|
||||
) = GradientOverlay(controllerVisible, TopGradientColors, height, modifier)
|
||||
) = GradientOverlay(controllerVisible, TopGradientBrush, height, modifier)
|
||||
|
||||
@Composable
|
||||
fun BottomGradientOverlay(
|
||||
controllerVisible: State<Boolean>,
|
||||
modifier: Modifier = Modifier,
|
||||
height: Dp = 120.dp,
|
||||
) = GradientOverlay(controllerVisible, BottomGradientColors, height, modifier)
|
||||
) = GradientOverlay(controllerVisible, BottomGradientBrush, height, modifier)
|
||||
|
||||
+5
-6
@@ -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) }
|
||||
|
||||
+5
-3
@@ -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<Boolean>,
|
||||
actions: List<VideoPlayerAction>,
|
||||
actions: ImmutableList<VideoPlayerAction>,
|
||||
startingMuteState: Boolean,
|
||||
onFullscreenClick: (() -> Unit)?,
|
||||
onMuteClick: () -> Unit,
|
||||
@@ -107,7 +109,7 @@ fun AnimatedOverflowMenuButton(
|
||||
|
||||
@Composable
|
||||
fun OverflowMenuButton(
|
||||
actions: List<VideoPlayerAction>,
|
||||
actions: ImmutableList<VideoPlayerAction>,
|
||||
startingMuteState: Boolean,
|
||||
onFullscreenClick: (() -> Unit)?,
|
||||
onMuteClick: () -> Unit,
|
||||
|
||||
+10
-1
@@ -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) {
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user