mirror of
https://github.com/JustinZeus/Wholphin.git
synced 2026-07-08 15:50:16 +02:00
Optimize recompositions on focus change (#1138)
<!-- By submitting this pull request, you acknowledge that you have read the [contributing guide](https://github.com/damontecres/Wholphin/blob/main/CONTRIBUTING.md, including the AI/LLM policy, and [developer's guide](https://github.com/damontecres/Wholphin/blob/main/DEVELOPMENT.md) --> ## Description <!-- Describe the changes in detail --> 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 <!-- If this is a new feature or a change, there must be a discussion in an issue first, reference the issue here --> <!-- If fixing a bug, reference the bug issue here, or describe the bug, including steps to reproduce --> 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 <!-- Describe how this change was tested and on what device(s) --> 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 <!-- Please include screenshots if the PR alters any UI elements --> main: <img width="402" height="508" alt="image" src="https://github.com/user-attachments/assets/6aaac6af-50d6-4f21-b465-e78bccf7e985" /> my branch: <img width="401" height="563" alt="image" src="https://github.com/user-attachments/assets/1034c0e2-9a40-4a05-8a07-f30b2e10301d" /> same navigation, on the same content. (down, right x4 ## AI or LLM usage <!-- If you used any AI or LLM assistance, please list where in the code and how you tested it --> 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
This commit is contained in:
Leon Omelan
GitHub
parent
4ea21be430
commit
762a5e6dcc
5 changed files with 185 additions and 113 deletions
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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 <T> 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 <T> ItemRow(
|
|||
) {
|
||||
itemsIndexed(items) { index, item ->
|
||||
val cardModifier =
|
||||
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,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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() }),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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,10 +157,13 @@ fun SeriesOverviewContent(
|
|||
TabRow(
|
||||
selectedTabIndex = selectedTabIndex,
|
||||
tabs = tabs,
|
||||
onClick = {
|
||||
onClick =
|
||||
remember {
|
||||
{
|
||||
selectedTabIndex = it
|
||||
onChangeSeason.invoke(it)
|
||||
currentOnChangeSeason(it)
|
||||
requestFocusAfterSeason = true
|
||||
}
|
||||
},
|
||||
focusRequesters = focusRequesters,
|
||||
modifier =
|
||||
|
|
|
|||
|
|
@ -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,15 +124,18 @@ fun HomePage(
|
|||
var showDeleteDialog by remember { mutableStateOf<RowColumnItem?>(null) }
|
||||
val playlistState by playlistViewModel.playlistState.observeAsState(PlaylistLoadingState.Pending)
|
||||
var position by rememberPosition()
|
||||
HomePageContent(
|
||||
homeRows = homeRows,
|
||||
position = position,
|
||||
onFocusPosition = { position = it },
|
||||
onClickItem = { clickedPosition, item ->
|
||||
|
||||
val onFocusPosition = remember { { it: RowColumn -> position = it } }
|
||||
val onClickItem =
|
||||
remember {
|
||||
{ clickedPosition: RowColumn, item: BaseItem ->
|
||||
position = clickedPosition
|
||||
viewModel.navigationManager.navigateTo(item.destination())
|
||||
},
|
||||
onLongClickItem = { clickedPosition, item ->
|
||||
}
|
||||
}
|
||||
val onLongClickItem =
|
||||
remember {
|
||||
{ clickedPosition: RowColumn, item: BaseItem ->
|
||||
position = clickedPosition
|
||||
val dialogItems =
|
||||
buildMoreDialogItemsForHome(
|
||||
|
|
@ -166,10 +171,22 @@ fun HomePage(
|
|||
fromLongClick = true,
|
||||
items = dialogItems,
|
||||
)
|
||||
},
|
||||
onClickPlay = { _, item ->
|
||||
}
|
||||
}
|
||||
val onClickPlay =
|
||||
remember {
|
||||
{ _: RowColumn, item: BaseItem ->
|
||||
viewModel.navigationManager.navigateTo(Destination.Playback(item))
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
HomePageContent(
|
||||
homeRows = homeRows,
|
||||
position = position,
|
||||
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,10 +299,12 @@ fun HomePageContent(
|
|||
|
||||
val density = LocalDensity.current
|
||||
val spaceAbovePx =
|
||||
remember(density) {
|
||||
with(density) {
|
||||
// The size of the row titles & spacing
|
||||
50.dp.toPx()
|
||||
}
|
||||
}
|
||||
val defaultBringIntoViewSpec = LocalBringIntoViewSpec.current
|
||||
CompositionLocalProvider(
|
||||
LocalBringIntoViewSpec provides ScrollToTopBringIntoViewSpec(spaceAbovePx),
|
||||
|
|
@ -329,14 +350,20 @@ fun HomePageContent(
|
|||
ItemRow(
|
||||
title = row.title,
|
||||
items = row.items,
|
||||
onClickItem = { index, item ->
|
||||
onClickItem =
|
||||
remember(rowIndex, onClickItem) {
|
||||
{ index, item ->
|
||||
onClickItem.invoke(RowColumn(rowIndex, index), item)
|
||||
}
|
||||
},
|
||||
onLongClickItem = { index, item ->
|
||||
onLongClickItem =
|
||||
remember(rowIndex, onLongClickItem) {
|
||||
{ index, item ->
|
||||
onLongClickItem.invoke(
|
||||
RowColumn(rowIndex, index),
|
||||
item,
|
||||
)
|
||||
}
|
||||
},
|
||||
modifier =
|
||||
Modifier
|
||||
|
|
@ -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) },
|
||||
)
|
||||
},
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue