From 762a5e6dcc3bdef329feff97e020fc2ba8156dc5 Mon Sep 17 00:00:00 2001 From: Leon Omelan Date: Wed, 1 Apr 2026 00:39:02 +0200 Subject: [PATCH] Optimize recompositions on focus change (#1138) ## Description I've look at the app using Layout inspector to see if there are any recompositions that shouldn't be happening while navigating normally in the app. I've noticed two issues 1. Every BannerCard was being recomposed on every cursor move, which added a lot of overhead to home screen navigation 2. Tab row in the Series View was being recomposed while navigating across episodes in given season. ### Related issues Performance on the home screen is lacking, especially on low end devices, while this isn't a full fix, it will improve the navigation, as the app will be making significantly less work. Probably will fix https://github.com/damontecres/Wholphin/issues/1043 ### Testing Using Layout Inspector built-in recomposition counter and highlights it's easy to see components which recompose while they shouldn't. I've tested it on a 4K TV emulator on my M1 Macbook Pro ## Screenshots main: image my branch: image same navigation, on the same content. (down, right x4 ## AI or LLM usage Gemini built in to Android Studio helped me identify non-stable parts of the code. It also helped me understand and implement fixes in the HomePage code. I've took the time to check manually that it didn't alter any behavior, and verified the fixes using Layout Inspector and recomposition counter --- .../wholphin/ui/cards/BannerCard.kt | 12 +- .../damontecres/wholphin/ui/cards/ItemRow.kt | 42 ++-- .../wholphin/ui/components/TabRow.kt | 30 ++- .../ui/detail/series/SeriesOverviewContent.kt | 18 +- .../damontecres/wholphin/ui/main/HomePage.kt | 196 ++++++++++-------- 5 files changed, 185 insertions(+), 113 deletions(-) diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/cards/BannerCard.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/cards/BannerCard.kt index a05af761..8d00edfd 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/cards/BannerCard.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/cards/BannerCard.kt @@ -19,6 +19,7 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberUpdatedState import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -98,10 +99,15 @@ fun BannerCard( } } var imageError by remember(imageUrl) { mutableStateOf(false) } + + // Stabilize callbacks to prevent AsyncImage from recomposing + val currentOnClick by rememberUpdatedState(onClick) + val currentOnLongClick by rememberUpdatedState(onLongClick) + Card( modifier = modifier.size(cardHeight * aspectRatio, cardHeight), - onClick = onClick, - onLongClick = onLongClick, + onClick = { currentOnClick() }, + onLongClick = { currentOnLongClick() }, interactionSource = interactionSource, colors = CardDefaults.colors( @@ -119,7 +125,7 @@ fun BannerCard( model = imageUrl, contentDescription = null, contentScale = imageContentScale, - onError = { imageError = true }, + onError = remember { { imageError = true } }, modifier = Modifier.fillMaxSize(), ) } else { diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/cards/ItemRow.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/cards/ItemRow.kt index 314a7e1a..5b929a9b 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/cards/ItemRow.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/cards/ItemRow.kt @@ -12,6 +12,7 @@ import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberUpdatedState import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import androidx.compose.ui.focus.FocusRequester @@ -45,6 +46,10 @@ fun ItemRow( val firstFocus = remember { FocusRequester() } val focusRequester = remember { FocusRequester() } var position by rememberInt() + + val currentOnClickItem by rememberUpdatedState(onClickItem) + val currentOnLongClickItem by rememberUpdatedState(onLongClickItem) + Column( verticalArrangement = Arrangement.spacedBy(8.dp), modifier = @@ -73,23 +78,36 @@ fun ItemRow( ) { itemsIndexed(items) { index, item -> val cardModifier = - if (index == position) { - Modifier.focusRequester(firstFocus) - } else { - Modifier + remember(index, position) { + if (index == position) { + Modifier.focusRequester(firstFocus) + } else { + Modifier + } } + + val onClick = + remember(index, item) { + { + position = index + if (item != null) currentOnClickItem(index, item) + } + } + + val onLongClick = + remember(index, item) { + { + position = index + if (item != null) currentOnLongClickItem(index, item) + } + } + cardContent.invoke( index, item, cardModifier, - { - position = index - if (item != null) onClickItem.invoke(index, item) - }, - { - position = index - if (item != null) onLongClickItem.invoke(index, item) - }, + onClick, + onLongClick, ) } } diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/components/TabRow.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/components/TabRow.kt index 25b51698..f54fda5b 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/components/TabRow.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/components/TabRow.kt @@ -19,9 +19,9 @@ import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberUpdatedState import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -40,7 +40,6 @@ import androidx.compose.ui.unit.sp import androidx.tv.material3.MaterialTheme import androidx.tv.material3.Text import com.github.damontecres.wholphin.ui.PreviewTvSpec -import com.github.damontecres.wholphin.ui.ifElse import com.github.damontecres.wholphin.ui.theme.WholphinTheme import com.github.damontecres.wholphin.ui.tryRequestFocus import timber.log.Timber @@ -60,6 +59,11 @@ fun TabRow( } } var rowHasFocus by remember { mutableStateOf(false) } + + val currentSelectedTabIndex by rememberUpdatedState(selectedTabIndex) + val currentFocusRequesters by rememberUpdatedState(focusRequesters) + val currentOnClick by rememberUpdatedState(onClick) + LazyRow( state = state, modifier = @@ -71,14 +75,16 @@ fun TabRow( onEnter = { // If entering from left or right, use last or first tab // Otherwise use the selected tab - Timber.v("onEnter requestedFocusDirection=$requestedFocusDirection, selectedTabIndex=$selectedTabIndex") + val index = currentSelectedTabIndex + val requesters = currentFocusRequesters + Timber.v("onEnter requestedFocusDirection=$requestedFocusDirection, selectedTabIndex=$index") val focusRequester = if (requestedFocusDirection == FocusDirection.Left) { - focusRequesters.lastOrNull() + requesters.lastOrNull() } else if (requestedFocusDirection == FocusDirection.Right) { - focusRequesters.firstOrNull() + requesters.firstOrNull() } else { - focusRequesters.getOrNull(selectedTabIndex) + requesters.getOrNull(index) } (focusRequester ?: FocusRequester.Default).tryRequestFocus() } @@ -86,15 +92,19 @@ fun TabRow( ) { itemsIndexed(tabs) { index, tabTitle -> val interactionSource = remember { MutableInteractionSource() } + val onTabClick = + remember(index) { + { + currentOnClick(index) + } + } Tab( title = tabTitle, selected = index == selectedTabIndex, rowActive = rowHasFocus, interactionSource = interactionSource, - onClick = { - onClick.invoke(index) - }, - modifier = Modifier.focusRequester(focusRequesters[index]), + onClick = onTabClick, + modifier = Modifier.focusRequester(focusRequesters.getOrElse(index) { FocusRequester() }), ) } } diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesOverviewContent.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesOverviewContent.kt index b32bbd30..eeb1607c 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesOverviewContent.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesOverviewContent.kt @@ -30,6 +30,7 @@ import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.rememberUpdatedState import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier @@ -95,7 +96,7 @@ fun SeriesOverviewContent( ) { val scope = rememberCoroutineScope() val bringIntoViewRequester = remember { BringIntoViewRequester() } - var selectedTabIndex by rememberSaveable(position) { mutableIntStateOf(position.seasonTabIndex) } + var selectedTabIndex by rememberSaveable(position.seasonTabIndex) { mutableIntStateOf(position.seasonTabIndex) } LaunchedEffect(selectedTabIndex) { logTab("series_overview", selectedTabIndex) } @@ -120,6 +121,8 @@ fun SeriesOverviewContent( } val focusRequesters = remember(seasons) { List(seasons.size) { FocusRequester() } } + val currentOnChangeSeason by rememberUpdatedState(onChangeSeason) + Box( modifier = modifier @@ -154,11 +157,14 @@ fun SeriesOverviewContent( TabRow( selectedTabIndex = selectedTabIndex, tabs = tabs, - onClick = { - selectedTabIndex = it - onChangeSeason.invoke(it) - requestFocusAfterSeason = true - }, + onClick = + remember { + { + selectedTabIndex = it + currentOnChangeSeason(it) + requestFocusAfterSeason = true + } + }, focusRequesters = focusRequesters, modifier = Modifier diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/main/HomePage.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/main/HomePage.kt index 94281b78..9addbb0e 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/main/HomePage.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/main/HomePage.kt @@ -22,10 +22,12 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.collectAsState +import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.livedata.observeAsState import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberUpdatedState import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -122,54 +124,69 @@ fun HomePage( var showDeleteDialog by remember { mutableStateOf(null) } val playlistState by playlistViewModel.playlistState.observeAsState(PlaylistLoadingState.Pending) var position by rememberPosition() + + val onFocusPosition = remember { { it: RowColumn -> position = it } } + val onClickItem = + remember { + { clickedPosition: RowColumn, item: BaseItem -> + position = clickedPosition + viewModel.navigationManager.navigateTo(item.destination()) + } + } + val onLongClickItem = + remember { + { clickedPosition: RowColumn, item: BaseItem -> + position = clickedPosition + val dialogItems = + buildMoreDialogItemsForHome( + context = context, + item = item, + seriesId = item.data.seriesId, + playbackPosition = item.playbackPosition, + watched = item.played, + favorite = item.favorite, + canDelete = viewModel.canDelete(item, preferences.appPreferences), + actions = + MoreDialogActions( + navigateTo = viewModel.navigationManager::navigateTo, + onClickWatch = { itemId, played -> + viewModel.setWatched(itemId, played) + }, + onClickFavorite = { itemId, favorite -> + viewModel.setFavorite(itemId, favorite) + }, + onClickAddPlaylist = { itemId -> + playlistViewModel.loadPlaylists(MediaType.VIDEO) + showPlaylistDialog = itemId + }, + onSendMediaInfo = viewModel.mediaReportService::sendReportFor, + onClickDelete = { + showDeleteDialog = RowColumnItem(position, item) + }, + ), + ) + dialog = + DialogParams( + title = item.title ?: "", + fromLongClick = true, + items = dialogItems, + ) + } + } + val onClickPlay = + remember { + { _: RowColumn, item: BaseItem -> + viewModel.navigationManager.navigateTo(Destination.Playback(item)) + } + } + HomePageContent( homeRows = homeRows, position = position, - onFocusPosition = { position = it }, - onClickItem = { clickedPosition, item -> - position = clickedPosition - viewModel.navigationManager.navigateTo(item.destination()) - }, - onLongClickItem = { clickedPosition, item -> - position = clickedPosition - val dialogItems = - buildMoreDialogItemsForHome( - context = context, - item = item, - seriesId = item.data.seriesId, - playbackPosition = item.playbackPosition, - watched = item.played, - favorite = item.favorite, - canDelete = viewModel.canDelete(item, preferences.appPreferences), - actions = - MoreDialogActions( - navigateTo = viewModel.navigationManager::navigateTo, - onClickWatch = { itemId, played -> - viewModel.setWatched(itemId, played) - }, - onClickFavorite = { itemId, favorite -> - viewModel.setFavorite(itemId, favorite) - }, - onClickAddPlaylist = { itemId -> - playlistViewModel.loadPlaylists(MediaType.VIDEO) - showPlaylistDialog = itemId - }, - onSendMediaInfo = viewModel.mediaReportService::sendReportFor, - onClickDelete = { - showDeleteDialog = RowColumnItem(position, item) - }, - ), - ) - dialog = - DialogParams( - title = item.title ?: "", - fromLongClick = true, - items = dialogItems, - ) - }, - onClickPlay = { _, item -> - viewModel.navigationManager.navigateTo(Destination.Playback(item)) - }, + onFocusPosition = onFocusPosition, + onClickItem = onClickItem, + onLongClickItem = onLongClickItem, + onClickPlay = onClickPlay, loadingState = refreshing, showClock = preferences.appPreferences.interfacePreferences.showClock, onUpdateBackdrop = viewModel::updateBackdrop, @@ -210,6 +227,8 @@ fun HomePage( ) } } + + else -> {} } } @@ -239,12 +258,17 @@ fun HomePageContent( }, ) { val focusedItem = - position.let { - (homeRows.getOrNull(it.row) as? HomeRowLoadingState.Success)?.items?.getOrNull(it.column) + remember(homeRows, position) { + (homeRows.getOrNull(position.row) as? HomeRowLoadingState.Success)?.items?.getOrNull(position.column) } - val rowFocusRequesters = remember(homeRows) { List(homeRows.size) { FocusRequester() } } + val rowFocusRequesters = remember(homeRows.size) { List(homeRows.size) { FocusRequester() } } var firstFocused by remember { mutableStateOf(false) } + + val currentPosition by rememberUpdatedState(position) + val currentOnFocusPosition by rememberUpdatedState(onFocusPosition) + val currentOnClickPlay by rememberUpdatedState(onClickPlay) + if (takeFocus) { LaunchedEffect(homeRows) { if (!firstFocused && homeRows.isNotEmpty()) { @@ -266,11 +290,6 @@ fun HomePageContent( } } } - LaunchedEffect(position) { - if (position.row >= 0) { - listState.animateScrollToItem(position.row) - } - } LaunchedEffect(onUpdateBackdrop, focusedItem) { focusedItem?.let { onUpdateBackdrop.invoke(it) } } @@ -280,9 +299,11 @@ fun HomePageContent( val density = LocalDensity.current val spaceAbovePx = - with(density) { - // The size of the row titles & spacing - 50.dp.toPx() + remember(density) { + with(density) { + // The size of the row titles & spacing + 50.dp.toPx() + } } val defaultBringIntoViewSpec = LocalBringIntoViewSpec.current CompositionLocalProvider( @@ -329,15 +350,21 @@ fun HomePageContent( ItemRow( title = row.title, items = row.items, - onClickItem = { index, item -> - onClickItem.invoke(RowColumn(rowIndex, index), item) - }, - onLongClickItem = { index, item -> - onLongClickItem.invoke( - RowColumn(rowIndex, index), - item, - ) - }, + onClickItem = + remember(rowIndex, onClickItem) { + { index, item -> + onClickItem.invoke(RowColumn(rowIndex, index), item) + } + }, + onLongClickItem = + remember(rowIndex, onLongClickItem) { + { index, item -> + onLongClickItem.invoke( + RowColumn(rowIndex, index), + item, + ) + } + }, modifier = Modifier .fillMaxWidth() @@ -346,6 +373,26 @@ fun HomePageContent( .animateItem(), horizontalPadding = viewOptions.spacing.dp, cardContent = { index, item, cardModifier, onClick, onLongClick -> + val onFocus = + remember(rowIndex, index) { + { isFocused: Boolean -> + if (isFocused) { + currentOnFocusPosition(RowColumn(rowIndex, index)) + } + } + } + val onKey = + remember(item) { + { event: androidx.compose.ui.input.key.KeyEvent -> + if (isPlayKeyUp(event) && item?.type?.playable == true) { + Timber.v("Clicked play on ${item.id}") + currentOnClickPlay(currentPosition, item) + true + } else { + false + } + } + } HomePageCardContent( index = index, item = item, @@ -354,23 +401,8 @@ fun HomePageContent( viewOptions = viewOptions, modifier = cardModifier - .onFocusChanged { - if (it.isFocused) { - onFocusPosition?.invoke( - RowColumn(rowIndex, index), - ) - } - }.onKeyEvent { - if (isPlayKeyUp(it) && item?.type?.playable == true) { - Timber.v("Clicked play on ${item.id}") - onClickPlay.invoke( - position, - item, - ) - return@onKeyEvent true - } - return@onKeyEvent false - }, + .onFocusChanged { onFocus(it.isFocused) } + .onKeyEvent { onKey(it) }, ) }, )