Skip to content

perf: critical Compose recomposition hotspot fixes #2049

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
theovilardo merged 1 commit into from
May 19, 2026
Merged

perf: critical Compose recomposition hotspot fixes #2049

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 89 additions & 62 deletions app/src/main/java/com/theveloper/pixelplay/presentation/components/CastBottomSheet.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
)
)
}
}
}
Expand Down
7 changes: 5 additions & 2 deletions app/src/main/java/com/theveloper/pixelplay/presentation/components/DailyMixSection.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
126 changes: 58 additions & 68 deletions app/src/main/java/com/theveloper/pixelplay/presentation/components/LyricsSheet.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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<T>, 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
Expand All @@ -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()

Expand Down Expand Up @@ -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,
Expand Down
Loading