From 78e21a0274c78ce4fead78542b0e89dcfcbf1b15 Mon Sep 17 00:00:00 2001 From: davotoula Date: Thu, 23 Apr 2026 17:02:12 +0200 Subject: [PATCH] =?UTF-8?q?Code=20review:=20Dead=20videoGroup=20parameter?= =?UTF-8?q?=20in=20inner=20RenderTopButtons=20overload.=20Confirmed=20via?= =?UTF-8?q?=20git=20show=20origin/main=20=E2=80=94=20the=20branch=20introd?= =?UTF-8?q?uced=20it,=20and=20it's=20only=20passed=20through=20but=20never?= =?UTF-8?q?=20read=20inside=20the=20body=20(the=20outer=20overload=20uses?= =?UTF-8?q?=20=20=20videoGroup=20separately=20for=20VideoQualityPopup).=20?= =?UTF-8?q?Removed=20from=20signature=20and=20both=20call=20sites=20(previ?= =?UTF-8?q?ew=20+=20outer=20wrapper).?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RenderTopButtons runs for every visible video tile, so recomputing the top-bar / overflow action lists on every recomposition added avoidable per-frame filter+map allocations. Memoize on the inputs that actually change availability. Also drop the outer Box around AnimatedOverflowMenuButton — the inner OverflowMenuButton already provides its own sized anchor Box, so the wrapper added no layout value. --- .../composable/controls/RenderTopButtons.kt | 42 +++++++++++-------- .../ui/screen/loggedIn/search/SearchScreen.kt | 1 + 2 files changed, 25 insertions(+), 18 deletions(-) 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 903040429..02816fa75 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 @@ -81,7 +81,6 @@ fun RenderTopButtonsPreview() { Box(Modifier.background(BitcoinOrange)) { RenderTopButtons( mediaData = MediaItemData("http://test.mp4"), - videoGroup = null, hasMultipleQualities = false, qualityButton = {}, controllerVisible = remember { mutableStateOf(true) }, @@ -141,7 +140,6 @@ fun RenderTopButtons( RenderTopButtons( mediaData = mediaData, - videoGroup = videoGroup, hasMultipleQualities = hasMultipleQualities, qualityButton = { VideoQualityButton( @@ -186,7 +184,6 @@ fun RenderTopButtons( @Composable fun RenderTopButtons( mediaData: MediaItemData, - videoGroup: Tracks.Group?, hasMultipleQualities: Boolean, qualityButton: @Composable () -> Unit, controllerVisible: MutableState, @@ -217,8 +214,19 @@ fun RenderTopButtons( VideoPlayerAction.PictureInPicture -> pipSupported } - val topBarActions = buttonItems.filter { it.location == VideoButtonLocation.TopBar && isAvailable(it.action) }.map { it.action } - val overflowActions = buttonItems.filter { it.location == VideoButtonLocation.OverflowMenu && isAvailable(it.action) }.map { it.action } + val canFullscreen = onZoomClick != null + val topBarActions = + remember(buttonItems, canFullscreen, hasMultipleQualities, isLive, pipSupported) { + buttonItems + .filter { it.location == VideoButtonLocation.TopBar && isAvailable(it.action) } + .map { it.action } + } + val overflowActions = + remember(buttonItems, canFullscreen, hasMultipleQualities, isLive, pipSupported) { + buttonItems + .filter { it.location == VideoButtonLocation.OverflowMenu && isAvailable(it.action) } + .map { it.action } + } Row(modifier) { topBarActions.forEach { action -> @@ -274,19 +282,17 @@ fun RenderTopButtons( } if (overflowActions.isNotEmpty()) { - Box { - AnimatedOverflowMenuButton( - controllerVisible = controllerVisible, - actions = overflowActions, - onFullscreenClick = onZoomClick, - onMuteClick = { onMuteClick(!startingMuteState) }, - startingMuteState = startingMuteState, - onQualityClick = onOverflowQualityClick, - onShareClick = { shareDialogVisible.value = true }, - onSaveClick = saveAction, - onPipClick = onPictureInPictureClick, - ) - } + AnimatedOverflowMenuButton( + controllerVisible = controllerVisible, + actions = overflowActions, + onFullscreenClick = onZoomClick, + onMuteClick = { onMuteClick(!startingMuteState) }, + startingMuteState = startingMuteState, + onQualityClick = onOverflowQualityClick, + onShareClick = { shareDialogVisible.value = true }, + onSaveClick = saveAction, + onPipClick = onPictureInPictureClick, + ) } if (shareDialogVisible.value) { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/search/SearchScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/search/SearchScreen.kt index 6d2a9cad7..51ec08f03 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/search/SearchScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/search/SearchScreen.kt @@ -216,6 +216,7 @@ private fun hasNonDefaultFilters( @OptIn(ExperimentalMaterial3Api::class) @Composable +@Suppress("AssignedValueIsNeverRead") private fun SearchFilterRow(searchBarViewModel: SearchBarViewModel) { val currentScope by searchBarViewModel.scope.collectAsStateWithLifecycle() val currentSource by searchBarViewModel.source.collectAsStateWithLifecycle()