From 6c3028eeb2c98cfc15d612c7dc43ac5da24f35ba Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Sun, 18 Oct 2026 02:35:55 +0000 Subject: [PATCH] library: keep the artist screen fast and avoid placeholder labels Rewrite the artist album lookup as indexed UNION branches instead of ORs that scanned the song table, and let artistName take the first indexed row. Keep the type label and album count blank until the album flow has emitted, so the page no longer flashes "Artist" and "0 albums" before showing the real values. --- .../com/subtracks/data/db/LibraryDao.kt | 53 ++++++++++++++----- .../ui/library/ArtistDetailScreen.kt | 21 +++++--- .../ui/library/ArtistDetailViewModel.kt | 6 ++- .../data/db/QueryTimingMeasurementTest.kt | 2 + .../ui/library/ArtistDetailScreenTest.kt | 17 +++++- 5 files changed, 78 insertions(+), 21 deletions(-) diff --git a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt index 72a14d4d..d0bd7543 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt @@ -945,9 +945,9 @@ interface LibraryDao { @Query( "SELECT COALESCE(" + "(SELECT name FROM song_artists WHERE sourceId = :sourceId AND artistId = :artistId " + - "AND name != '' ORDER BY songId, position LIMIT 1), " + + "AND name != '' LIMIT 1), " + "(SELECT artist FROM songs WHERE sourceId = :sourceId AND artistId = :artistId " + - "AND artist IS NOT NULL AND artist != '' ORDER BY id LIMIT 1))", + "AND artist IS NOT NULL AND artist != '' LIMIT 1))", ) fun artistName( sourceId: Long, @@ -973,9 +973,22 @@ interface LibraryDao { ): Flow> @Query( - "SELECT * FROM albums WHERE sourceId = :sourceId AND ($ALBUM_MATCHES_ARTIST " + - "OR id IN (SELECT albumId FROM songs WHERE sourceId = :sourceId AND $SONG_MATCHES_ARTIST)) " + - "ORDER BY year DESC, name COLLATE NOCASE, id", + "SELECT * FROM (" + + "SELECT albums.* FROM albums WHERE albums.sourceId = :sourceId AND albums.artistId = :artistId " + + "UNION " + + "SELECT albums.* FROM album_artists " + + "JOIN albums ON albums.sourceId = album_artists.sourceId AND albums.id = album_artists.albumId " + + "WHERE album_artists.sourceId = :sourceId AND album_artists.artistId = :artistId " + + "UNION " + + "SELECT albums.* FROM song_artists " + + "JOIN songs ON songs.sourceId = song_artists.sourceId AND songs.id = song_artists.songId " + + "JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " + + "WHERE song_artists.sourceId = :sourceId AND song_artists.artistId = :artistId " + + "UNION " + + "SELECT albums.* FROM songs " + + "JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " + + "WHERE songs.sourceId = :sourceId AND songs.artistId = :artistId" + + ") ORDER BY year DESC, name COLLATE NOCASE, id", ) fun albumsForArtist( sourceId: Long, @@ -1018,15 +1031,31 @@ interface LibraryDao { ): PagingSource @Query( - "SELECT * FROM albums WHERE sourceId = :sourceId AND (" + - "($ALBUM_MATCHES_ARTIST AND EXISTS (SELECT 1 FROM songs " + + "SELECT * FROM (" + + "SELECT albums.* FROM albums " + + "WHERE albums.sourceId = :sourceId AND albums.artistId = :artistId " + + "AND EXISTS (SELECT 1 FROM songs " + "JOIN song_downloads sd ON sd.sourceId = songs.sourceId AND sd.songId = songs.id " + - "WHERE songs.sourceId = albums.sourceId AND songs.albumId = albums.id AND sd.status = 'Completed')) " + - "OR EXISTS (SELECT 1 FROM songs " + + "WHERE songs.sourceId = albums.sourceId AND songs.albumId = albums.id AND sd.status = 'Completed') " + + "UNION " + + "SELECT albums.* FROM album_artists " + + "JOIN albums ON albums.sourceId = album_artists.sourceId AND albums.id = album_artists.albumId " + + "WHERE album_artists.sourceId = :sourceId AND album_artists.artistId = :artistId " + + "AND EXISTS (SELECT 1 FROM songs " + "JOIN song_downloads sd ON sd.sourceId = songs.sourceId AND sd.songId = songs.id " + - "WHERE songs.sourceId = albums.sourceId AND songs.albumId = albums.id " + - "AND sd.status = 'Completed' AND $SONG_MATCHES_ARTIST)) " + - "ORDER BY year DESC, name COLLATE NOCASE, id", + "WHERE songs.sourceId = albums.sourceId AND songs.albumId = albums.id AND sd.status = 'Completed') " + + "UNION " + + "SELECT albums.* FROM song_artists " + + "JOIN songs ON songs.sourceId = song_artists.sourceId AND songs.id = song_artists.songId " + + "JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " + + "JOIN song_downloads sd ON sd.sourceId = songs.sourceId AND sd.songId = songs.id AND sd.status = 'Completed' " + + "WHERE song_artists.sourceId = :sourceId AND song_artists.artistId = :artistId " + + "UNION " + + "SELECT albums.* FROM songs " + + "JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " + + "JOIN song_downloads sd ON sd.sourceId = songs.sourceId AND sd.songId = songs.id AND sd.status = 'Completed' " + + "WHERE songs.sourceId = :sourceId AND songs.artistId = :artistId" + + ") ORDER BY year DESC, name COLLATE NOCASE, id", ) fun albumsForArtistDownloaded( sourceId: Long, diff --git a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt index c8d643ea..133ea20d 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt @@ -159,7 +159,7 @@ fun ArtistDetailRoute( ) { val artist by viewModel.artist.collectAsStateWithLifecycle(initialValue = null) val artistName by viewModel.artistName.collectAsStateWithLifecycle(initialValue = null) - val albums by viewModel.albums.collectAsStateWithLifecycle(initialValue = emptyList()) + val albums by viewModel.albums.collectAsStateWithLifecycle(initialValue = null) val art by viewModel.art.collectAsStateWithLifecycle() val artThumbnail by viewModel.artThumbnail.collectAsStateWithLifecycle() val downloadStatus by viewModel.downloadStatus.collectAsStateWithLifecycle() @@ -192,8 +192,9 @@ fun ArtistDetailRoute( ArtistDetailScreen( artist = artist, artistName = artistName.orEmpty(), - isAlbumArtist = isAlbumArtist(albums, artistId), - albums = albums, + isAlbumArtist = albums?.let { isAlbumArtist(it, artistId) }, + albums = albums.orEmpty(), + albumsLoaded = albums != null, art = art, artThumbnail = artThumbnail, artwork = rememberArtworkColors(shortcutArt ?: artThumbnail ?: art, THEME_TRANSITION_MS, fallbackName = artistName), @@ -224,8 +225,9 @@ fun ArtistDetailScreen( artist: Artist?, modifier: Modifier = Modifier, artistName: String = "", - isAlbumArtist: Boolean = false, + isAlbumArtist: Boolean? = null, albums: List, + albumsLoaded: Boolean = true, art: CoverArtRef?, artThumbnail: CoverArtRef? = null, artwork: ArtworkColors? = null, @@ -252,7 +254,7 @@ fun ArtistDetailScreen( val barHeight = statusBarTop + TopAppBarDefaults.TopAppBarExpandedHeight val albumCount = albums.size.toLong() val displayName = artistName.ifBlank { artist?.name.orEmpty() } - val artistType = stringResource(if (isAlbumArtist) R.string.artist_type_album_artist else R.string.artist_type_artist) + val artistType = stringResource(if (isAlbumArtist == true) R.string.artist_type_album_artist else R.string.artist_type_artist) val imageTitleTop = ART_HEIGHT - TITLE_INSET - lineHeight val fadeStartPx = with(density) { (imageTitleTop - barHeight - FADE_LEAD).toPx() } val fadeEndPx = with(density) { (imageTitleTop + lineHeight / 2 - barHeight).toPx() } @@ -373,7 +375,7 @@ fun ArtistDetailScreen( modifier = Modifier.fillMaxWidth().padding(start = 16.dp, end = 8.dp, top = TITLE_INSET), ) { Text( - text = artistType, + text = if (albumsLoaded) artistType else "", style = MaterialTheme.typography.labelLarge, color = MaterialTheme.colorScheme.onSurfaceVariant, ) @@ -384,7 +386,12 @@ fun ArtistDetailScreen( verticalAlignment = Alignment.CenterVertically, ) { Text( - text = pluralStringResource(R.plurals.resources_album_count, albumCount.toInt(), albumCount), + text = + if (albumsLoaded) { + pluralStringResource(R.plurals.resources_album_count, albumCount.toInt(), albumCount) + } else { + "" + }, style = MaterialTheme.typography.bodyLarge, color = MaterialTheme.colorScheme.onSurfaceVariant, modifier = Modifier.weight(1f), diff --git a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailViewModel.kt b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailViewModel.kt index 604c1e84..9d280906 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailViewModel.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailViewModel.kt @@ -23,6 +23,8 @@ import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flatMapLatest +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch @@ -48,9 +50,11 @@ class ArtistDetailViewModel( val offline: StateFlow = sourceRepository.offline - val albums: Flow> = + val albums: Flow?> = combine(sourceId, sourceRepository.offline) { id, offline -> id to offline } .flatMapLatest { (id, offline) -> libraryRepository.artistAlbums(id, artistId, offline) } + .map, List?> { it } + .onStart { emit(null) } val downloadStatus: StateFlow = sourceId diff --git a/app/src/test/kotlin/com/subtracks/data/db/QueryTimingMeasurementTest.kt b/app/src/test/kotlin/com/subtracks/data/db/QueryTimingMeasurementTest.kt index 25614c7e..60419a78 100644 --- a/app/src/test/kotlin/com/subtracks/data/db/QueryTimingMeasurementTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/db/QueryTimingMeasurementTest.kt @@ -91,6 +91,8 @@ class QueryTimingMeasurementTest { timed("albums first page (recent)") { dao.albumsByRecent(1, 0, "").firstPage() } timed("artists first page (name)") { dao.artistsByName(1, 0, "").firstPage() } timed("artists first page (frequent)") { dao.artistsByFrequent(1, 0, "").firstPage() } + timed("artist albums") { dao.albumsForArtist(1, "ar0").first() } + timed("artist name") { dao.artistName(1, "ar0").first() } timed("playlists first page (name)") { dao.playlistsByName(1, "").firstPage() } timed("albums deep page") { dao.albumsByName(1, 0, "").deepPage(1000) } timed("albums search FTS") { dao.albumsByName(1, 0, "Album 1").firstPage() } diff --git a/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt b/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt index bd9ee49a..745d2f8c 100644 --- a/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt @@ -24,7 +24,8 @@ class ArtistDetailScreenTest { private fun render( artist: Artist?, artistName: String, - isAlbumArtist: Boolean, + isAlbumArtist: Boolean? = null, + albumsLoaded: Boolean = true, ) { composeRule.setContent { SubtracksTheme { @@ -33,6 +34,7 @@ class ArtistDetailScreenTest { artistName = artistName, isAlbumArtist = isAlbumArtist, albums = emptyList(), + albumsLoaded = albumsLoaded, art = null, coverArt = { _, _ -> null }, onBack = {}, @@ -62,4 +64,17 @@ class ArtistDetailScreenTest { composeRule.onNodeWithText("Album artist").assertExists() composeRule.onNodeWithContentDescription("More options").assertExists() } + + @Test + fun theTypeAndCountStayBlankUntilTheAlbumsLoad() { + render( + artist = Artist(sourceId = 1, id = "ar-1", name = "Radiohead", albumCount = 1, starred = null), + artistName = "Radiohead", + albumsLoaded = false, + ) + + composeRule.onNodeWithText("Artist").assertDoesNotExist() + composeRule.onNodeWithText("Album artist").assertDoesNotExist() + composeRule.onNodeWithText("0 albums").assertDoesNotExist() + } }