diff --git a/app/src/main/java/com/theveloper/pixelplay/presentation/components/CastBottomSheet.kt b/app/src/main/java/com/theveloper/pixelplay/presentation/components/CastBottomSheet.kt index e2a0a78d8..b01a56395 100644 --- a/app/src/main/java/com/theveloper/pixelplay/presentation/components/CastBottomSheet.kt +++ b/app/src/main/java/com/theveloper/pixelplay/presentation/components/CastBottomSheet.kt @@ -89,6 +89,8 @@ import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.LaunchedEffect import androidx.lifecycle.compose.collectAsStateWithLifecycle +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.map import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableFloatStateOf import androidx.compose.runtime.mutableIntStateOf @@ -179,7 +181,14 @@ fun CastBottomSheet( val isRemotePlaybackActive by playerViewModel.isRemotePlaybackActive.collectAsStateWithLifecycle() val isCastConnecting by playerViewModel.isCastConnecting.collectAsStateWithLifecycle() val trackVolume by playerViewModel.trackVolume.collectAsStateWithLifecycle() - val isPlaying = playerViewModel.stablePlayerState.collectAsStateWithLifecycle().value.isPlaying + // Slice the stablePlayerState flow to just isPlaying before collecting; the + // full StablePlayerState changes on every position tick (~4×/s) but only + // isPlaying matters here. + val isPlaying by remember(playerViewModel) { + playerViewModel.stablePlayerState + .map { it.isPlaying } + .distinctUntilChanged() + }.collectAsStateWithLifecycle(initialValue = false) val context = LocalContext.current val requiredPermissions = remember { @@ -217,73 +226,91 @@ fun CastBottomSheet( val activeRoute = selectedRoute?.takeUnless { it.isDefault } val isRemoteSession = (isRemotePlaybackActive || isCastConnecting) && activeRoute != null - val availableRoutes = if (isWifiEnabled) { - routes.filterNot { it.isDefault } - } else { - emptyList() + // Derived lists wrapped in remember so they don't rebuild on every recomposition. + // The sheet ticks frequently (Bluetooth state changes, Wi-Fi name updates, route + // volume drag, etc.) — without these wrappers each tick re-filters/re-maps the + // route + bluetooth lists. + val availableRoutes = remember(routes, isWifiEnabled) { + if (isWifiEnabled) routes.filterNot { it.isDefault } else emptyList() + } + val bluetoothDevices = remember(bluetoothAudioDeviceStates) { + bluetoothAudioDeviceStates + .map { state -> state.copy(name = state.name.trim()) } + .filter { it.name.isNotEmpty() } + .distinctBy { it.stableId() } + } + val activeBluetoothName = remember(bluetoothName, bluetoothDevices) { + bluetoothName + ?.trim() + ?.takeIf { activeName -> + activeName.isNotEmpty() && bluetoothDevices.any { it.name == activeName } + } } - val bluetoothDevices = bluetoothAudioDeviceStates - .map { state -> state.copy(name = state.name.trim()) } - .filter { it.name.isNotEmpty() } - .distinctBy { it.stableId() } - val activeBluetoothName = bluetoothName - ?.trim() - ?.takeIf { activeName -> - activeName.isNotEmpty() && bluetoothDevices.any { it.name == activeName } - } - val devices = buildList { - if (isWifiEnabled) { - addAll( - availableRoutes.map { route -> - val isRouteActive = activeRoute?.id == route.id - val normalizedConnectionState = when { - isRouteActive && isCastConnecting -> - MediaRouter.RouteInfo.CONNECTION_STATE_CONNECTING - isRouteActive && isRemoteSession -> - MediaRouter.RouteInfo.CONNECTION_STATE_CONNECTED - route.connectionState == MediaRouter.RouteInfo.CONNECTION_STATE_CONNECTED -> - MediaRouter.RouteInfo.CONNECTION_STATE_DISCONNECTED - else -> route.connectionState + val devices = remember( + isWifiEnabled, + isBluetoothEnabled, + availableRoutes, + bluetoothDevices, + activeRoute?.id, + isCastConnecting, + isRemoteSession, + activeBluetoothName, + trackVolume + ) { + buildList { + if (isWifiEnabled) { + addAll( + availableRoutes.map { route -> + val isRouteActive = activeRoute?.id == route.id + val normalizedConnectionState = when { + isRouteActive && isCastConnecting -> + MediaRouter.RouteInfo.CONNECTION_STATE_CONNECTING + isRouteActive && isRemoteSession -> + MediaRouter.RouteInfo.CONNECTION_STATE_CONNECTED + route.connectionState == MediaRouter.RouteInfo.CONNECTION_STATE_CONNECTED -> + MediaRouter.RouteInfo.CONNECTION_STATE_DISCONNECTED + else -> route.connectionState + } + + CastDeviceUi( + id = route.id, + name = route.name, + deviceType = route.deviceType, + playbackType = route.playbackType, + connectionState = normalizedConnectionState, + volumeHandling = route.volumeHandling, + volume = route.volume, + volumeMax = route.volumeMax, + isSelected = isRouteActive + ) } + ) + } - CastDeviceUi( - id = route.id, - name = route.name, - deviceType = route.deviceType, - playbackType = route.playbackType, - connectionState = normalizedConnectionState, - volumeHandling = route.volumeHandling, - volume = route.volume, - volumeMax = route.volumeMax, - isSelected = isRouteActive + if (isBluetoothEnabled) { + bluetoothDevices.forEach { bluetoothDevice -> + val isConnected = bluetoothDevice.name == activeBluetoothName + add( + CastDeviceUi( + id = "bluetooth_${bluetoothDevice.stableId()}", + name = bluetoothDevice.name, + deviceType = MediaRouter.RouteInfo.DEVICE_TYPE_BLUETOOTH_A2DP, + playbackType = MediaRouter.RouteInfo.PLAYBACK_TYPE_LOCAL, + connectionState = if (isConnected) { + MediaRouter.RouteInfo.CONNECTION_STATE_CONNECTED + } else { + MediaRouter.RouteInfo.CONNECTION_STATE_DISCONNECTED + }, + volumeHandling = MediaRouter.RouteInfo.PLAYBACK_VOLUME_VARIABLE, + volume = if (isConnected) (trackVolume * 100).toInt() else 0, + volumeMax = 100, + isSelected = isConnected && !isRemoteSession, + batteryPercent = bluetoothDevice.batteryPercent, + isBluetooth = true + ) ) } - ) - } - - if (isBluetoothEnabled) { - bluetoothDevices.forEach { bluetoothDevice -> - val isConnected = bluetoothDevice.name == activeBluetoothName - add( - CastDeviceUi( - id = "bluetooth_${bluetoothDevice.stableId()}", - name = bluetoothDevice.name, - deviceType = MediaRouter.RouteInfo.DEVICE_TYPE_BLUETOOTH_A2DP, - playbackType = MediaRouter.RouteInfo.PLAYBACK_TYPE_LOCAL, - connectionState = if (isConnected) { - MediaRouter.RouteInfo.CONNECTION_STATE_CONNECTED - } else { - MediaRouter.RouteInfo.CONNECTION_STATE_DISCONNECTED - }, - volumeHandling = MediaRouter.RouteInfo.PLAYBACK_VOLUME_VARIABLE, - volume = if (isConnected) (trackVolume * 100).toInt() else 0, - volumeMax = 100, - isSelected = isConnected && !isRemoteSession, - batteryPercent = bluetoothDevice.batteryPercent, - isBluetooth = true - ) - ) } } } diff --git a/app/src/main/java/com/theveloper/pixelplay/presentation/components/DailyMixSection.kt b/app/src/main/java/com/theveloper/pixelplay/presentation/components/DailyMixSection.kt index 4f64571cd..a525d83af 100644 --- a/app/src/main/java/com/theveloper/pixelplay/presentation/components/DailyMixSection.kt +++ b/app/src/main/java/com/theveloper/pixelplay/presentation/components/DailyMixSection.kt @@ -188,8 +188,11 @@ private fun DailyMixCard( playerViewModel: PlayerViewModel, onMoreOptionsClick: (Song) -> Unit ) { - val headerSongs = songs.take(3).toImmutableList() - val visibleSongs = songs.take(4).toImmutableList() + // .toImmutableList() runs O(n) on every recomposition without remember, + // and headerSongs / visibleSongs are derived only from `songs`. Wrap them + // so the take + immutable conversion only happens when songs changes. + val headerSongs = remember(songs) { songs.take(3).toImmutableList() } + val visibleSongs = remember(songs) { songs.take(4).toImmutableList() } val cornerRadius = 30.dp Card( shape = AbsoluteSmoothCornerShape( diff --git a/app/src/main/java/com/theveloper/pixelplay/presentation/components/LyricsSheet.kt b/app/src/main/java/com/theveloper/pixelplay/presentation/components/LyricsSheet.kt index ba47c08ea..3c14c5afb 100644 --- a/app/src/main/java/com/theveloper/pixelplay/presentation/components/LyricsSheet.kt +++ b/app/src/main/java/com/theveloper/pixelplay/presentation/components/LyricsSheet.kt @@ -121,9 +121,11 @@ import android.content.Intent import android.content.IntentFilter import androidx.compose.ui.platform.LocalView import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.map import androidx.datastore.preferences.core.booleanPreferencesKey import androidx.datastore.preferences.core.edit +import androidx.datastore.preferences.core.floatPreferencesKey import androidx.datastore.preferences.core.stringPreferencesKey import com.theveloper.pixelplay.data.preferences.dataStore @@ -178,6 +180,29 @@ internal fun lyricsSheetColors(colorScheme: ColorScheme): LyricsSheetColors { ) } +/** Single snapshot of all seven lyrics-sheet preferences read directly from DataStore. */ +@androidx.compose.runtime.Immutable +private data class LyricsSheetPrefs( + val alignment: String = "left", + val showTranslation: Boolean = true, + val showRomanization: Boolean = true, + val useAnimated: Boolean = false, + val animatedBlurEnabled: Boolean = true, + val animatedBlurStrength: Float = 2.5f, + val keepScreenOn: Boolean = false +) + +private fun androidx.datastore.preferences.core.Preferences.toLyricsSheetPrefs(): LyricsSheetPrefs = + LyricsSheetPrefs( + alignment = this[stringPreferencesKey("lyrics_alignment")] ?: "left", + showTranslation = this[booleanPreferencesKey("show_lyrics_translation")] ?: true, + showRomanization = this[booleanPreferencesKey("show_lyrics_romanization")] ?: true, + useAnimated = this[booleanPreferencesKey("use_animated_lyrics")] ?: false, + animatedBlurEnabled = this[booleanPreferencesKey("animated_lyrics_blur_enabled")] ?: true, + animatedBlurStrength = this[floatPreferencesKey("animated_lyrics_blur_strength")] ?: 2.5f, + keepScreenOn = this[booleanPreferencesKey("keep_screen_on_lyrics")] ?: false + ) + private fun preferredContrastColor( background: Color, preferred: Color, @@ -298,10 +323,15 @@ fun LyricsSheet( val playPauseColor = sheetColors.playPauseContainer val onPlayPauseColor = sheetColors.playPauseContent - val isLoadingLyrics by remember(stablePlayerState) { derivedStateOf { stablePlayerState.isLoadingLyrics } } - val lyrics by remember(stablePlayerState) { derivedStateOf { stablePlayerState.lyrics } } - val isPlaying by remember(stablePlayerState) { derivedStateOf { stablePlayerState.isPlaying } } - val currentSong by remember(stablePlayerState) { derivedStateOf { stablePlayerState.currentSong } } + // These are plain captured values from the stablePlayerState parameter, not + // reads of State, so derivedStateOf adds an extra State layer without + // the dependency-tracking it normally provides. Direct destructuring is + // sufficient — when stablePlayerState recomposes the parent, these locals + // recompute as part of the normal recomposition. + val isLoadingLyrics = stablePlayerState.isLoadingLyrics + val lyrics = stablePlayerState.lyrics + val isPlaying = stablePlayerState.isPlaying + val currentSong = stablePlayerState.currentSong val hasTranslatedLyrics = remember(lyrics) { // Translated lyrics read same timestamp on the lrc, not possible in plain type lyrics @@ -318,48 +348,31 @@ fun LyricsSheet( val context = LocalContext.current - // Read lyrics alignment preference internally from DataStore - val lyricsAlignmentFlow = remember(context) { - context.dataStore.data.map { it[stringPreferencesKey("lyrics_alignment")] ?: "left" } - } - val lyricsAlignment by lyricsAlignmentFlow.collectAsStateWithLifecycle(initialValue = "left") - - // Read lyrics translation preference internally from DataStore - val showLyricsTranslationFlow = remember(context) { - context.dataStore.data.map { it[booleanPreferencesKey("show_lyrics_translation")] ?: true } - } - val showLyricsTranslation by showLyricsTranslationFlow.collectAsStateWithLifecycle(initialValue = true) - - // Read lyrics romanization preference internally from DataStore - val showLyricsRomanizationFlow = remember(context) { - context.dataStore.data.map { it[booleanPreferencesKey("show_lyrics_romanization")] ?: true } - } - val showLyricsRomanization by showLyricsRomanizationFlow.collectAsStateWithLifecycle(initialValue = true) - - // Read animated lyrics preference internally from DataStore - val useAnimatedLyricsFlow = remember(context) { - context.dataStore.data.map { it[booleanPreferencesKey("use_animated_lyrics")] ?: false } - } - val useAnimatedLyrics by useAnimatedLyricsFlow.collectAsStateWithLifecycle(initialValue = false) - - val animatedLyricsBlurEnabledFlow = remember(context) { - context.dataStore.data.map { it[booleanPreferencesKey("animated_lyrics_blur_enabled")] ?: true } - } - val animatedLyricsBlurEnabled by animatedLyricsBlurEnabledFlow.collectAsStateWithLifecycle(initialValue = true) - - val animatedLyricsBlurStrengthFlow = remember(context) { - context.dataStore.data.map { it[androidx.datastore.preferences.core.floatPreferencesKey("animated_lyrics_blur_strength")] ?: 2.5f } - } - val animatedLyricsBlurStrength by animatedLyricsBlurStrengthFlow.collectAsStateWithLifecycle(initialValue = 2.5f) - - // Read keep-screen-on preference from DataStore - val keepScreenOnFlow = remember(context) { - context.dataStore.data.map { it[booleanPreferencesKey("keep_screen_on_lyrics")] ?: false } - } + // Read all seven lyrics-sheet preferences in a single DataStore subscription. + // Previously each preference (alignment, translation, romanization, animated, + // blur enabled, blur strength, keep-screen-on) created its own collector + // mapping the same Preferences object — seven collectors observing one flow. + // Combined into a single mapped flow with distinctUntilChanged. The + // architectural-violation note from the perf audit (these reads bypass + // UserPreferencesRepository) is still open as a separate refactor. + val lyricsPrefs by remember(context) { + context.dataStore.data + .map { prefs -> prefs.toLyricsSheetPrefs() } + .distinctUntilChanged() + }.collectAsStateWithLifecycle(initialValue = LyricsSheetPrefs()) + val lyricsAlignment = lyricsPrefs.alignment + val showLyricsTranslation = lyricsPrefs.showTranslation + val showLyricsRomanization = lyricsPrefs.showRomanization + val useAnimatedLyrics = lyricsPrefs.useAnimated + val animatedLyricsBlurEnabled = lyricsPrefs.animatedBlurEnabled + val animatedLyricsBlurStrength = lyricsPrefs.animatedBlurStrength + + // keep-screen-on still uses a mutable local state because the lifecycle + // observer below writes back to DataStore on ON_STOP, and the sheet's + // toggle button drives it locally. var keepScreenOn by remember { mutableStateOf(false) } - // Sync DataStore → local state - LaunchedEffect(Unit) { - keepScreenOnFlow.collect { keepScreenOn = it } + LaunchedEffect(lyricsPrefs.keepScreenOn) { + keepScreenOn = lyricsPrefs.keepScreenOn } val coroutineScope = rememberCoroutineScope() @@ -390,29 +403,6 @@ fun LyricsSheet( } } - DisposableEffect(keepScreenOn, lifecycleOwner) { - val observer = androidx.lifecycle.LifecycleEventObserver { _, event -> - if (event == androidx.lifecycle.Lifecycle.Event.ON_STOP && keepScreenOn) { - keepScreenOn = false - coroutineScope.launch { - context.dataStore.edit { prefs -> - prefs[booleanPreferencesKey("keep_screen_on_lyrics")] = false - } - } - } - } - - if (keepScreenOn) { - view.keepScreenOn = true - } - - lifecycleOwner.lifecycle.addObserver(observer) - onDispose { - view.keepScreenOn = false - lifecycleOwner.lifecycle.removeObserver(observer) - } - } - val resolvedAutoscrollSpec = autoscrollAnimationSpec ?: if (useAnimatedLyrics) { spring( stiffness = Spring.StiffnessMediumLow, diff --git a/app/src/main/java/com/theveloper/pixelplay/presentation/components/QueueBottomSheet.kt b/app/src/main/java/com/theveloper/pixelplay/presentation/components/QueueBottomSheet.kt index a37a8e6d6..9ba9e8769 100644 --- a/app/src/main/java/com/theveloper/pixelplay/presentation/components/QueueBottomSheet.kt +++ b/app/src/main/java/com/theveloper/pixelplay/presentation/components/QueueBottomSheet.kt @@ -1,11 +1,15 @@ package com.theveloper.pixelplay.presentation.components import androidx.compose.animation.AnimatedVisibility +import androidx.compose.animation.animateColor import androidx.compose.animation.animateColorAsState +import androidx.compose.animation.core.animateDp +import androidx.compose.animation.core.animateFloat +import androidx.compose.animation.core.tween +import androidx.compose.animation.core.updateTransition import androidx.compose.animation.core.Animatable import androidx.compose.animation.core.FastOutSlowInEasing import androidx.compose.animation.core.Spring -import androidx.compose.animation.core.tween import androidx.compose.animation.core.spring import androidx.compose.animation.fadeIn import androidx.compose.animation.fadeOut @@ -158,6 +162,7 @@ import racra.compose.smooth_corner_rect_library.AbsoluteSmoothCornerShape import sh.calvin.reorderable.ReorderableItem import sh.calvin.reorderable.rememberReorderableLazyListState import kotlin.math.roundToInt +import kotlinx.collections.immutable.ImmutableList import kotlinx.coroutines.Job import kotlinx.coroutines.delay import kotlinx.coroutines.launch @@ -199,6 +204,19 @@ private data class QueueUndoBarProjection( val removedSongTitle: String = "" ) +/** + * Triple of orthogonal animation drivers for a queue item. Packed into a + * single value so QueuePlaylistSongItem can run one updateTransition + * (and the framework reuses one set of animation runtime objects) instead + * of six independent animateXAsState calls. + */ +@androidx.compose.runtime.Immutable +private data class QueueItemAnimState( + val isCurrentSong: Boolean, + val isDragging: Boolean, + val isSwipeTargeted: Boolean +) + private fun PlayerUiState.toQueueUndoBarProjection(): QueueUndoBarProjection = QueueUndoBarProjection( isVisible = showQueueItemUndoBar, @@ -214,7 +232,7 @@ fun QueueBottomSheet( viewModel: PlayerViewModel = hiltViewModel(), playlistViewModel: PlaylistViewModel = hiltViewModel(), settingsViewModel: SettingsViewModel = hiltViewModel(), - queue: List, + queue: ImmutableList, currentQueueSourceName: String, currentSongId: String?, currentMediaItemIndex: Int = -1, @@ -1834,25 +1852,6 @@ fun QueuePlaylistSongItem( ) { val colors = MaterialTheme.colorScheme - val cornerRadius by animateDpAsState( - targetValue = if (isCurrentSong) 60.dp else 22.dp, - label = "cornerRadiusAnimation" - ) - - val itemShape = RoundedCornerShape(cornerRadius) - - val albumCornerRadius by animateDpAsState( - targetValue = if (isCurrentSong) 60.dp else 8.dp, - label = "cornerRadiusAnimation" - ) - - val albumShape = RoundedCornerShape(albumCornerRadius) - - val elevation by animateDpAsState( - targetValue = if (isDragging) 4.dp else 1.dp, - label = "elevationAnimation" - ) - val backgroundColor = colors.surfaceContainerLowest val mvContainerColor = if (isCurrentSong) colors.tertiaryContainer else colors.surfaceContainerHigh val mvContentColor = if (isCurrentSong) colors.onTertiaryContainer else colors.onSurface @@ -1886,21 +1885,43 @@ fun QueuePlaylistSongItem( (revealWidthPx / (56.dp.value * density.density)).coerceIn(0f, 1f) } else 0f - val dismissBackgroundColor by animateColorAsState( - targetValue = if (isSwipeTargeted) colors.errorContainer else colors.errorContainer.copy(alpha = 0.82f), - animationSpec = tween(durationMillis = 150), + // Consolidate six independent animateXAsState calls into a single + // updateTransition keyed on (isCurrentSong, isDragging, isSwipeTargeted). + // Same pattern that was already applied to EnhancedSongListItem — saves + // 5 animation runtime objects per queue item plus one composition slot. + val animState = QueueItemAnimState(isCurrentSong, isDragging, isSwipeTargeted) + val transition = updateTransition(animState, label = "queueItemAnim") + val cornerRadius by transition.animateDp(label = "cornerRadius") { state -> + if (state.isCurrentSong) 60.dp else 22.dp + } + val albumCornerRadius by transition.animateDp(label = "albumCornerRadius") { state -> + if (state.isCurrentSong) 60.dp else 8.dp + } + val elevation by transition.animateDp(label = "elevation") { state -> + if (state.isDragging) 4.dp else 1.dp + } + val dismissBackgroundColor by transition.animateColor( + transitionSpec = { tween(durationMillis = 150) }, label = "dismissBackgroundColor" - ) - val dismissIconAlpha by animateFloatAsState( - targetValue = revealProgress * if (isSwipeTargeted) 1f else 0.88f, - animationSpec = tween(durationMillis = 120), - label = "dismissIconAlpha" - ) - val dismissIconScale by animateFloatAsState( - targetValue = if (isSwipeTargeted) 1.08f else 0.95f, - animationSpec = tween(durationMillis = 120), + ) { state -> + if (state.isSwipeTargeted) colors.errorContainer else colors.errorContainer.copy(alpha = 0.82f) + } + val dismissIconAlphaFactor by transition.animateFloat( + transitionSpec = { tween(durationMillis = 120) }, + label = "dismissIconAlphaFactor" + ) { state -> + if (state.isSwipeTargeted) 1f else 0.88f + } + val dismissIconScale by transition.animateFloat( + transitionSpec = { tween(durationMillis = 120) }, label = "dismissIconScale" - ) + ) { state -> + if (state.isSwipeTargeted) 1.08f else 0.95f + } + val dismissIconAlpha = revealProgress * dismissIconAlphaFactor + + val itemShape = RoundedCornerShape(cornerRadius) + val albumShape = RoundedCornerShape(albumCornerRadius) var surfaceHeightPx by remember { mutableStateOf(0f) } diff --git a/app/src/main/java/com/theveloper/pixelplay/presentation/components/player/FullPlayerContent.kt b/app/src/main/java/com/theveloper/pixelplay/presentation/components/player/FullPlayerContent.kt index f8b26d6c1..f241a6275 100644 --- a/app/src/main/java/com/theveloper/pixelplay/presentation/components/player/FullPlayerContent.kt +++ b/app/src/main/java/com/theveloper/pixelplay/presentation/components/player/FullPlayerContent.kt @@ -1294,7 +1294,7 @@ private fun predictSkipPreviousCarouselIndex( @Composable private fun FullPlayerSongMetadataSection( song: Song, - currentSongArtists: List, + currentSongArtists: ImmutableList, loadingTweaks: FullPlayerLoadingTweaks, isSheetDragGestureActive: Boolean, expansionFractionProvider: () -> Float, @@ -1440,7 +1440,7 @@ private fun FullPlayerLandscapeContent( @Composable private fun SongMetadataDisplaySection( song: Song?, - currentSongArtists: List, + currentSongArtists: ImmutableList, expansionFractionProvider: () -> Float, textColor: Color, artistTextColor: Color, @@ -2116,7 +2116,7 @@ private fun PlayerSongInfo( title: String, artist: String, artistId: Long, - artists: List, + artists: ImmutableList, expansionFractionProvider: () -> Float, textColor: Color, artistTextColor: Color, @@ -2127,8 +2127,12 @@ private fun PlayerSongInfo( ) { val coroutineScope = rememberCoroutineScope() var isNavigatingToArtist by remember { mutableStateOf(false) } - val resolvedArtistId by remember(artists, artistId) { - derivedStateOf { artists.firstOrNull { it.id != 0L && it.id != -1L }?.id ?: artistId } + // derivedStateOf is unnecessary here: the calculation reads only the + // `artists` parameter and the captured `artistId`, not any State. + // A plain remember is enough and skips an extra State allocation + + // snapshot read per recomposition. + val resolvedArtistId = remember(artists, artistId) { + artists.firstOrNull { it.id != 0L && it.id != -1L }?.id ?: artistId } val titleStyle = MaterialTheme.typography.headlineSmall.copy( fontWeight = FontWeight.Bold, diff --git a/app/src/main/java/com/theveloper/pixelplay/presentation/components/player/PlayerArtistPickerBottomSheet.kt b/app/src/main/java/com/theveloper/pixelplay/presentation/components/player/PlayerArtistPickerBottomSheet.kt index 162f894b8..3f6f6eee6 100644 --- a/app/src/main/java/com/theveloper/pixelplay/presentation/components/player/PlayerArtistPickerBottomSheet.kt +++ b/app/src/main/java/com/theveloper/pixelplay/presentation/components/player/PlayerArtistPickerBottomSheet.kt @@ -42,6 +42,7 @@ import com.theveloper.pixelplay.data.model.Artist import com.theveloper.pixelplay.data.model.Song import com.theveloper.pixelplay.presentation.components.SmartImage import com.theveloper.pixelplay.ui.theme.GoogleSansRounded +import kotlinx.collections.immutable.ImmutableList import racra.compose.smooth_corner_rect_library.AbsoluteSmoothCornerShape private data class PlayerArtistShortcutItem( @@ -53,7 +54,7 @@ private data class PlayerArtistShortcutItem( @Composable internal fun PlayerArtistPickerBottomSheet( song: Song, - artists: List, + artists: ImmutableList, sheetState: SheetState, onDismiss: () -> Unit, onArtistClick: (Artist) -> Unit diff --git a/app/src/main/java/com/theveloper/pixelplay/presentation/screens/LibraryMediaTabs.kt b/app/src/main/java/com/theveloper/pixelplay/presentation/screens/LibraryMediaTabs.kt index d3991a9b3..1a5daadb7 100644 --- a/app/src/main/java/com/theveloper/pixelplay/presentation/screens/LibraryMediaTabs.kt +++ b/app/src/main/java/com/theveloper/pixelplay/presentation/screens/LibraryMediaTabs.kt @@ -73,6 +73,15 @@ import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.distinctUntilChanged import androidx.compose.ui.text.style.TextOverflow +/** + * Shared empty color-scheme flow used as a placeholder while paged album items + * are still loading. Hoisted to file scope so we don't allocate a fresh + * MutableStateFlow on every placeholder rendering inside the LazyColumn / + * LazyVerticalGrid items lambda. + */ +private val EMPTY_ALBUM_COLOR_SCHEME_FLOW: StateFlow = + MutableStateFlow(null) + @androidx.annotation.OptIn(UnstableApi::class) @Composable fun LibraryAlbumsTab( @@ -342,8 +351,13 @@ fun LibraryAlbumsTab( ) { index -> val album = albums[index] if (album != null) { - val albumSpecificColorSchemeFlow = - playerViewModel.themeStateHolder.getAlbumColorSchemeFlow(album.albumArtUriString ?: "") + val artUri = album.albumArtUriString ?: "" + // Cache the flow lookup so each scroll/recomposition does not + // re-enter ThemeStateHolder.getAlbumColorSchemeFlow (which on + // a cache miss launches a generation coroutine). + val albumSpecificColorSchemeFlow = remember(artUri) { + playerViewModel.themeStateHolder.getAlbumColorSchemeFlow(artUri) + } val rememberedOnClick = remember(album.id, onAlbumClick) { { onAlbumClick(album.id) } } @@ -367,7 +381,7 @@ fun LibraryAlbumsTab( } else { AlbumListItem( album = Album.empty(), - albumColorSchemePairFlow = MutableStateFlow(null), + albumColorSchemePairFlow = EMPTY_ALBUM_COLOR_SCHEME_FLOW, onClick = {}, isLoading = true ) @@ -412,8 +426,10 @@ fun LibraryAlbumsTab( ) { index -> val album = albums[index] if (album != null) { - val albumSpecificColorSchemeFlow = - playerViewModel.themeStateHolder.getAlbumColorSchemeFlow(album.albumArtUriString ?: "") + val artUri = album.albumArtUriString ?: "" + val albumSpecificColorSchemeFlow = remember(artUri) { + playerViewModel.themeStateHolder.getAlbumColorSchemeFlow(artUri) + } val rememberedOnClick = remember(album.id, onAlbumClick) { { onAlbumClick(album.id) } } @@ -437,7 +453,7 @@ fun LibraryAlbumsTab( } else { AlbumGridItemRedesigned( album = Album.empty(), - albumColorSchemePairFlow = MutableStateFlow(null), + albumColorSchemePairFlow = EMPTY_ALBUM_COLOR_SCHEME_FLOW, onClick = {}, isLoading = true ) diff --git a/app/src/main/java/com/theveloper/pixelplay/presentation/screens/LibraryPlaybackAwareSongItem.kt b/app/src/main/java/com/theveloper/pixelplay/presentation/screens/LibraryPlaybackAwareSongItem.kt index 5c7da0dff..d0a359707 100644 --- a/app/src/main/java/com/theveloper/pixelplay/presentation/screens/LibraryPlaybackAwareSongItem.kt +++ b/app/src/main/java/com/theveloper/pixelplay/presentation/screens/LibraryPlaybackAwareSongItem.kt @@ -3,21 +3,21 @@ package com.theveloper.pixelplay.presentation.screens import androidx.annotation.OptIn import androidx.compose.runtime.Composable import androidx.compose.runtime.Immutable -import androidx.compose.runtime.getValue -import androidx.compose.runtime.remember import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp -import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.media3.common.util.UnstableApi import com.theveloper.pixelplay.data.model.Song import com.theveloper.pixelplay.presentation.components.subcomps.EnhancedSongListItem -import com.theveloper.pixelplay.presentation.viewmodel.PlayerViewModel -import kotlinx.coroutines.flow.distinctUntilChanged -import kotlinx.coroutines.flow.map +/** + * Lightweight playback projection shared by every song item in a list. Parent + * tabs collect this once from PlayerViewModel and pass the same instance down + * to every item, so a list of 100 items causes one upstream subscription + * instead of 100 N×N collectors. + */ @Immutable -internal data class LibrarySongPlaybackUiState( - val isCurrentSong: Boolean = false, +internal data class LibraryPlaybackHints( + val currentSongId: String? = null, val isPlaying: Boolean = false ) @@ -25,7 +25,7 @@ internal data class LibrarySongPlaybackUiState( @Composable internal fun LibraryPlaybackAwareSongItem( song: Song, - playerViewModel: PlayerViewModel, + playbackHints: LibraryPlaybackHints, albumArtSize: Dp = 50.dp, isSelected: Boolean = false, selectionIndex: Int? = null, @@ -34,22 +34,11 @@ internal fun LibraryPlaybackAwareSongItem( onMoreOptionsClick: (Song) -> Unit, onClick: () -> Unit ) { - val playbackUiState by remember(song.id, playerViewModel) { - playerViewModel.stablePlayerState - .map { state -> - val isCurrentSong = state.currentSong?.id == song.id - LibrarySongPlaybackUiState( - isCurrentSong = isCurrentSong, - isPlaying = isCurrentSong && state.isPlaying - ) - } - .distinctUntilChanged() - }.collectAsStateWithLifecycle(initialValue = LibrarySongPlaybackUiState()) - + val isCurrentSong = playbackHints.currentSongId == song.id EnhancedSongListItem( song = song, - isPlaying = playbackUiState.isPlaying, - isCurrentSong = playbackUiState.isCurrentSong, + isPlaying = isCurrentSong && playbackHints.isPlaying, + isCurrentSong = isCurrentSong, isLoading = false, albumArtSize = albumArtSize, isSelected = isSelected, diff --git a/app/src/main/java/com/theveloper/pixelplay/presentation/screens/LibraryScreen.kt b/app/src/main/java/com/theveloper/pixelplay/presentation/screens/LibraryScreen.kt index 97817275a..ed3d45d35 100644 --- a/app/src/main/java/com/theveloper/pixelplay/presentation/screens/LibraryScreen.kt +++ b/app/src/main/java/com/theveloper/pixelplay/presentation/screens/LibraryScreen.kt @@ -468,10 +468,14 @@ fun LibraryScreen( val compactInitialPage = remember(tabCount, normalizedLastTabIndex) { infinitePagerInitialPage(tabCount, normalizedLastTabIndex) } - val pagerState = if (isCompactNavigation) { - rememberPagerState(initialPage = compactInitialPage) { Int.MAX_VALUE } - } else { - rememberPagerState(initialPage = normalizedLastTabIndex) { tabCount } + // Single rememberPagerState call instead of a conditional remember anti-pattern + // (the previous if/else allocated separate slots on each branch and lost scroll + // state when libraryNavigationMode toggled between COMPACT_PILL and the full + // tab strip). initialPage is captured once; pageCount is a lambda that re-reads + // the mode on every measure pass. + val pagerInitialPage = if (isCompactNavigation) compactInitialPage else normalizedLastTabIndex + val pagerState = rememberPagerState(initialPage = pagerInitialPage) { + if (isCompactNavigation) Int.MAX_VALUE else tabCount } val currentTabIndex by remember(pagerState, tabTitles, isCompactNavigation) { derivedStateOf { @@ -545,6 +549,14 @@ fun LibraryScreen( { song -> multiSelectionState.toggleSelection(song) } } + // Bound method reference hoisted via remember so it is referentially stable + // across recompositions. Without this, each render passes a fresh lambda + // identity into LibrarySongsTab / LibraryFavoritesTab / LibraryFoldersTab, + // breaking parameter stability on those composables. + val getSelectionIndex: (String) -> Int? = remember(multiSelectionState) { + multiSelectionState::getSelectionIndex + } + val toggleAlbumSelection: (Album) -> Unit = remember(selectedAlbums, playerViewModel, context) { { album -> val existingIndex = selectedAlbums.indexOfFirst { it.id == album.id } @@ -816,17 +828,21 @@ fun LibraryScreen( } } - val gradientColorsDark = listOf( - MaterialTheme.colorScheme.primaryContainer.copy(alpha = 0.5f), - Color.Transparent - ).toImmutableList() - - val gradientColorsLight = listOf( - MaterialTheme.colorScheme.onPrimaryContainer.copy(alpha = 0.2f), - Color.Transparent - ).toImmutableList() - - val gradientColors = if (dm) gradientColorsDark else gradientColorsLight + val gradientPrimary = MaterialTheme.colorScheme.primaryContainer + val gradientOnPrimary = MaterialTheme.colorScheme.onPrimaryContainer + val gradientColors = remember(dm, gradientPrimary, gradientOnPrimary) { + if (dm) { + persistentListOf( + gradientPrimary.copy(alpha = 0.5f), + Color.Transparent + ) + } else { + persistentListOf( + gradientOnPrimary.copy(alpha = 0.2f), + Color.Transparent + ) + } + } val gradientBrush = remember(gradientColors) { Brush.verticalGradient(colors = gradientColors) @@ -1526,7 +1542,7 @@ fun LibraryScreen( selectedSongIds = selectedSongIds, onSongLongPress = onSongLongPress, onSongSelectionToggle = onSongSelectionToggle, - getSelectionIndex = playerViewModel.multiSelectionStateHolder::getSelectionIndex, + getSelectionIndex = getSelectionIndex, onLocateCurrentSongVisibilityChanged = { songsShowLocateButton = it }, onRegisterLocateCurrentSongAction = { songsLocateAction = it }, sortOption = playerUiState.currentSongSortOption, @@ -1618,7 +1634,7 @@ fun LibraryScreen( selectedSongIds = selectedSongIds, onSongLongPress = onSongLongPress, onSongSelectionToggle = onSongSelectionToggle, - getSelectionIndex = playerViewModel.multiSelectionStateHolder::getSelectionIndex, + getSelectionIndex = getSelectionIndex, sortOption = playerUiState.currentFavoriteSortOption, onLocateCurrentSongVisibilityChanged = { likedShowLocateButton = it }, onRegisterLocateCurrentSongAction = { likedLocateAction = it }, @@ -1667,7 +1683,7 @@ fun LibraryScreen( selectedSongIds = selectedSongIds, onSongLongPress = onSongLongPress, onSongSelectionToggle = onSongSelectionToggle, - getSelectionIndex = playerViewModel.multiSelectionStateHolder::getSelectionIndex, + getSelectionIndex = getSelectionIndex, onLocateCurrentSongVisibilityChanged = { foldersShowLocateButton = it }, onRegisterLocateCurrentSongAction = { foldersLocateAction = it }, pendingLocatePath = pendingFoldersLocatePath, @@ -2873,17 +2889,21 @@ fun LibraryFoldersTab( val isRoot = targetPath == FOLDER_NAVIGATION_ROOT_KEY val activeFolder = if (isRoot) null else currentFolder val showPlaylistCards = playlistMode && activeFolder == null - val itemsToShow = remember(activeFolder, folders, flattenedFolders, currentSortOption) { + // .toImmutableList() runs inside the remember block so it doesn't + // re-allocate the persistent list on every recomposition. showPlaylistCards + // is added to the key list because it factors playlistMode in, which the + // previous key set was missing. + val itemsToShow = remember(showPlaylistCards, activeFolder, folders, flattenedFolders, currentSortOption) { when { showPlaylistCards -> flattenedFolders activeFolder != null -> sortMusicFoldersByOption(activeFolder.subFolders, currentSortOption) else -> sortMusicFoldersByOption(folders, currentSortOption) - } - }.toImmutableList() + }.toImmutableList() + } val songsToShow = remember(activeFolder, currentSortOption) { - sortSongsForFolderView(activeFolder?.songs ?: emptyList(), currentSortOption) - }.toImmutableList() + sortSongsForFolderView(activeFolder?.songs ?: emptyList(), currentSortOption).toImmutableList() + } val currentSong = stablePlayerState.currentSong val currentSongId = currentSong?.id val currentSongIndexInSongs = remember(songsToShow, currentSongId) { diff --git a/app/src/main/java/com/theveloper/pixelplay/presentation/screens/LibrarySongsAndFavoritesTabs.kt b/app/src/main/java/com/theveloper/pixelplay/presentation/screens/LibrarySongsAndFavoritesTabs.kt index f39132197..13cfb8ff2 100644 --- a/app/src/main/java/com/theveloper/pixelplay/presentation/screens/LibrarySongsAndFavoritesTabs.kt +++ b/app/src/main/java/com/theveloper/pixelplay/presentation/screens/LibrarySongsAndFavoritesTabs.kt @@ -103,11 +103,14 @@ fun LibraryFavoritesTab( var lastHandledFavoriteSortKey by remember { mutableStateOf(sortOption.storageKey) } var pendingFavoriteSortScrollReset by remember { mutableStateOf(false) } var favoriteSortSawRefreshLoading by remember { mutableStateOf(false) } - val currentSongId by remember(playerViewModel) { + // Shared playback projection so every item below gets the same hints + // instance instead of starting its own stablePlayerState collector. + val playbackHints by remember(playerViewModel) { playerViewModel.stablePlayerState - .map { it.currentSong?.id } + .map { LibraryPlaybackHints(it.currentSong?.id, it.isPlaying) } .distinctUntilChanged() - }.collectAsStateWithLifecycle(initialValue = null) + }.collectAsStateWithLifecycle(initialValue = LibraryPlaybackHints()) + val currentSongId = playbackHints.currentSongId val currentSongListIndex = remember(favoriteSongs.itemCount, currentSongId) { if (currentSongId == null) -1 @@ -247,7 +250,7 @@ fun LibraryFavoritesTab( if (song != null) { LibraryPlaybackAwareSongItem( song = song, - playerViewModel = playerViewModel, + playbackHints = playbackHints, onMoreOptionsClick = { onMoreOptionsClick(song) }, isSelected = selectedSongIds.contains(song.id), selectionIndex = if (isSelectionMode) getSelectionIndex(song.id) else null, diff --git a/app/src/main/java/com/theveloper/pixelplay/presentation/screens/LibrarySongsTab.kt b/app/src/main/java/com/theveloper/pixelplay/presentation/screens/LibrarySongsTab.kt index 24e9b98f1..72f2e9be6 100644 --- a/app/src/main/java/com/theveloper/pixelplay/presentation/screens/LibrarySongsTab.kt +++ b/app/src/main/java/com/theveloper/pixelplay/presentation/screens/LibrarySongsTab.kt @@ -96,11 +96,17 @@ fun LibrarySongsTab( var lastHandledSongSortKey by remember { mutableStateOf(sortOption.storageKey) } var pendingSongSortScrollReset by remember { mutableStateOf(false) } var songSortSawRefreshLoading by remember { mutableStateOf(false) } - val currentSongId by remember(playerViewModel) { + // Single shared playback projection. Previously each LibraryPlaybackAwareSongItem + // spun up its own stablePlayerState.map{}.distinctUntilChanged() collector, + // which meant N visible items = N upstream subscriptions checking every + // emission. Now there is one upstream collector here and every item receives + // the same LibraryPlaybackHints instance. + val playbackHints by remember(playerViewModel) { playerViewModel.stablePlayerState - .map { it.currentSong?.id } + .map { LibraryPlaybackHints(it.currentSong?.id, it.isPlaying) } .distinctUntilChanged() - }.collectAsStateWithLifecycle(initialValue = null) + }.collectAsStateWithLifecycle(initialValue = LibraryPlaybackHints()) + val currentSongId = playbackHints.currentSongId // Check if list is effectively empty (based on Paging state) // val isListEmpty = songs.itemCount == 0 && songs.loadState.refresh is LoadState.NotLoading @@ -346,7 +352,7 @@ fun LibrarySongsTab( LibraryPlaybackAwareSongItem( song = song, - playerViewModel = playerViewModel, + playbackHints = playbackHints, isSelected = isSelected, //albumArtSize = 46.dp, isSelectionMode = isSelectionMode, diff --git a/app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/PlayerViewModel.kt b/app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/PlayerViewModel.kt index d0a4ebca9..39aca7fa7 100644 --- a/app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/PlayerViewModel.kt +++ b/app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/PlayerViewModel.kt @@ -440,15 +440,15 @@ class PlayerViewModel @Inject constructor( } @OptIn(ExperimentalCoroutinesApi::class) - val currentSongArtists: StateFlow> = stablePlayerState + val currentSongArtists: StateFlow> = stablePlayerState .map { it.currentSong?.id } .distinctUntilChanged() .flatMapLatest { songId -> val idLong = songId?.toLongOrNull() - if (idLong == null) flowOf(emptyList()) - else musicRepository.getArtistsForSong(idLong) + if (idLong == null) flowOf(persistentListOf()) + else musicRepository.getArtistsForSong(idLong).map { it.toImmutableList() } } - .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5000), emptyList()) + .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5000), persistentListOf()) private val _sheetState = MutableStateFlow(PlayerSheetState.COLLAPSED) val sheetState: StateFlow = _sheetState.asStateFlow() @@ -1238,7 +1238,7 @@ class PlayerViewModel @Inject constructor( // composable. Now a single collect + distinctUntilChanged batches all settings. // --------------------------------------------------------------------------- data class FullPlayerSlice( - val currentSongArtists: List = emptyList(), + val currentSongArtists: ImmutableList = persistentListOf(), val lyricsSyncOffset: Int = 0, val albumArtQuality: AlbumArtQuality = AlbumArtQuality.MEDIUM, val audioMetadata: PlaybackAudioMetadata = PlaybackAudioMetadata(), @@ -1259,7 +1259,7 @@ class PlayerViewModel @Inject constructor( albumArtQuality, playbackAudioMetadata, showPlayerFileInfo - ) { artists: List, syncOffset: Int, artQuality: AlbumArtQuality, + ) { artists: ImmutableList, syncOffset: Int, artQuality: AlbumArtQuality, audioMeta: PlaybackAudioMetadata, showFileInfo: Boolean -> FullPlayerSlicePart1(artists, syncOffset, artQuality, audioMeta, showFileInfo) } @@ -1284,7 +1284,7 @@ class PlayerViewModel @Inject constructor( } private data class FullPlayerSlicePart1( - val currentSongArtists: List, + val currentSongArtists: ImmutableList, val lyricsSyncOffset: Int, val albumArtQuality: AlbumArtQuality, val audioMetadata: PlaybackAudioMetadata, diff --git a/app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/SongInfoBottomSheetViewModel.kt b/app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/SongInfoBottomSheetViewModel.kt index 15ba6a6b8..74a038069 100644 --- a/app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/SongInfoBottomSheetViewModel.kt +++ b/app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/SongInfoBottomSheetViewModel.kt @@ -15,6 +15,9 @@ import com.theveloper.pixelplay.utils.AudioMetaUtils import dagger.hilt.android.lifecycle.HiltViewModel import java.io.File import javax.inject.Inject +import kotlinx.collections.immutable.ImmutableList +import kotlinx.collections.immutable.persistentListOf +import kotlinx.collections.immutable.toImmutableList import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow @@ -40,8 +43,8 @@ class SongInfoBottomSheetViewModel @Inject constructor( ) private val _audioMeta = MutableStateFlow(null) - private val _resolvedArtists = MutableStateFlow>(emptyList()) - val resolvedArtists: StateFlow> = _resolvedArtists.asStateFlow() + private val _resolvedArtists = MutableStateFlow>(persistentListOf()) + val resolvedArtists: StateFlow> = _resolvedArtists.asStateFlow() private val _isPixelPlayWatchAvailable = MutableStateFlow(false) val isPixelPlayWatchAvailable: StateFlow = _isPixelPlayWatchAvailable.asStateFlow() private val _isWatchAvailabilityResolved = MutableStateFlow(false) @@ -82,7 +85,7 @@ class SongInfoBottomSheetViewModel @Inject constructor( fun loadArtistsForSong(song: Song) { val refs = song.artists if (refs.isEmpty() || refs.size < 2) { - _resolvedArtists.value = emptyList() + _resolvedArtists.value = persistentListOf() return } viewModelScope.launch(kotlinx.coroutines.Dispatchers.IO) { @@ -95,7 +98,7 @@ class SongInfoBottomSheetViewModel @Inject constructor( val resolved = refs.map { ref -> entitiesById[ref.id]?.toArtist() ?: Artist(id = ref.id, name = ref.name, songCount = 0) - } + }.toImmutableList() _resolvedArtists.value = resolved } }