From f5948812bb12aa2f6bae31b441f9238af44b3cac Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Fri, 25 Sep 2026 00:35:34 +0000 Subject: [PATCH] address review round 3: keep the search bar across recomposition, fix tab landing, page the queried songs queue --- .../com/subtracks/data/db/LibraryDao.kt | 109 ++++++++++++- .../subtracks/data/prefs/UserPreferences.kt | 13 +- .../subtracks/data/repo/QueueRepository.kt | 149 +++++++++++++++--- .../com/subtracks/ui/library/LibraryScreen.kt | 4 +- 4 files changed, 245 insertions(+), 30 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 71bf01bb..cb6adfa7 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt @@ -35,6 +35,13 @@ internal const val SONGS_IDS_SELECT = "AND (:starredFilter = 0 OR (:starredFilter = 1 AND songs.starred IS NOT NULL) OR " + "(:starredFilter = 2 AND songs.starred IS NULL)) " +internal const val SONGS_LIST_SELECT = + "SELECT songs.*, albums.coverArt AS coverArt FROM songs " + + "LEFT JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " + + "WHERE songs.sourceId = :sourceId " + + "AND (:starredFilter = 0 OR (:starredFilter = 1 AND songs.starred IS NOT NULL) OR " + + "(:starredFilter = 2 AND songs.starred IS NULL)) " + @Dao @DaoReturnTypeConverters(PagingSourceDaoReturnTypeConverter::class) interface LibraryDao { @@ -357,7 +364,7 @@ interface LibraryDao { "AND (:search = '' OR instr(lower(songs.title), lower(:search)) > 0 " + "OR instr(lower(songs.artist), lower(:search)) > 0 OR instr(lower(songs.album), lower(:search)) > 0) " + "ORDER BY albums.albumArtist COLLATE NOCASE, songs.album COLLATE NOCASE, songs.disc, songs.track, " + - "songs.title COLLATE NOCASE, songs.album COLLATE NOCASE, songs.disc, songs.track, songs.id", + "songs.title COLLATE NOCASE, songs.id", ) fun songs( sourceId: Long, @@ -622,4 +629,104 @@ interface LibraryDao { sourceId: Long, starredFilter: Int, ): List + + @Query( + "${SONGS_LIST_SELECT}ORDER BY albums.albumArtist COLLATE NOCASE, songs.album COLLATE NOCASE, songs.disc, songs.track, songs.title COLLATE NOCASE, songs.id LIMIT :limit OFFSET :offset", + ) + suspend fun songsPageByAlbum( + sourceId: Long, + starredFilter: Int, + limit: Int, + offset: Long, + ): List + + @Query( + "${SONGS_LIST_SELECT}ORDER BY albums.albumArtist COLLATE NOCASE DESC, songs.album COLLATE NOCASE DESC, songs.disc DESC, songs.track DESC, songs.title COLLATE NOCASE DESC, songs.id DESC LIMIT :limit OFFSET :offset", + ) + suspend fun songsPageByAlbumReversed( + sourceId: Long, + starredFilter: Int, + limit: Int, + offset: Long, + ): List + + @Query( + "${SONGS_LIST_SELECT}ORDER BY songs.title COLLATE NOCASE, songs.album COLLATE NOCASE, songs.disc, songs.track, songs.id LIMIT :limit OFFSET :offset", + ) + suspend fun songsPageByTitle( + sourceId: Long, + starredFilter: Int, + limit: Int, + offset: Long, + ): List + + @Query( + "${SONGS_LIST_SELECT}ORDER BY songs.title COLLATE NOCASE DESC, songs.album COLLATE NOCASE, songs.disc, songs.track, songs.id LIMIT :limit OFFSET :offset", + ) + suspend fun songsPageByTitleReversed( + sourceId: Long, + starredFilter: Int, + limit: Int, + offset: Long, + ): List + + @Query( + "${SONGS_LIST_SELECT}ORDER BY songs.artist COLLATE NOCASE, songs.album COLLATE NOCASE, songs.disc, songs.track, songs.id LIMIT :limit OFFSET :offset", + ) + suspend fun songsPageByArtist( + sourceId: Long, + starredFilter: Int, + limit: Int, + offset: Long, + ): List + + @Query( + "${SONGS_LIST_SELECT}ORDER BY songs.artist COLLATE NOCASE DESC, songs.album COLLATE NOCASE, songs.disc, songs.track, songs.id LIMIT :limit OFFSET :offset", + ) + suspend fun songsPageByArtistReversed( + sourceId: Long, + starredFilter: Int, + limit: Int, + offset: Long, + ): List + + @Query( + "${SONGS_LIST_SELECT}ORDER BY songs.starred IS NULL, songs.starred DESC, songs.album COLLATE NOCASE, songs.disc, songs.track, songs.id LIMIT :limit OFFSET :offset", + ) + suspend fun songsPageByStarred( + sourceId: Long, + starredFilter: Int, + limit: Int, + offset: Long, + ): List + + @Query( + "${SONGS_LIST_SELECT}ORDER BY songs.starred IS NULL, songs.starred ASC, songs.album COLLATE NOCASE, songs.disc, songs.track, songs.id LIMIT :limit OFFSET :offset", + ) + suspend fun songsPageByStarredReversed( + sourceId: Long, + starredFilter: Int, + limit: Int, + offset: Long, + ): List + + @Query( + "${SONGS_LIST_SELECT}ORDER BY songs.created DESC, songs.album COLLATE NOCASE, songs.disc, songs.track, songs.id LIMIT :limit OFFSET :offset", + ) + suspend fun songsPageByAdded( + sourceId: Long, + starredFilter: Int, + limit: Int, + offset: Long, + ): List + + @Query( + "${SONGS_LIST_SELECT}ORDER BY songs.created ASC, songs.album COLLATE NOCASE, songs.disc, songs.track, songs.id LIMIT :limit OFFSET :offset", + ) + suspend fun songsPageByAddedReversed( + sourceId: Long, + starredFilter: Int, + limit: Int, + offset: Long, + ): List } diff --git a/app/src/main/kotlin/com/subtracks/data/prefs/UserPreferences.kt b/app/src/main/kotlin/com/subtracks/data/prefs/UserPreferences.kt index cad8c80b..758d3a16 100644 --- a/app/src/main/kotlin/com/subtracks/data/prefs/UserPreferences.kt +++ b/app/src/main/kotlin/com/subtracks/data/prefs/UserPreferences.kt @@ -42,7 +42,17 @@ private val Context.preferences: DataStore by preferencesDataStore( class UserPreferences( private val store: DataStore, ) { - fun listQuery(tab: LibraryListTab): Flow = store.data.map { prefs -> decode(prefs[listQueryKey(tab)], tab.defaultSort) } + fun listQuery(tab: LibraryListTab): Flow = + store.data.map { prefs -> decode(prefs[listQueryKey(tab)] ?: legacyQuery(prefs, tab), tab.defaultSort) } + + private fun legacyQuery( + prefs: Preferences, + tab: LibraryListTab, + ): String? { + if (tab != LibraryListTab.Albums) return null + val sort = prefs[ALBUM_SORT]?.let { if (it == "RecentlyAdded") "Added" else it } ?: return null + return "$sort|0|0" + } suspend fun setListQuery( tab: LibraryListTab, @@ -83,6 +93,7 @@ class UserPreferences( } private companion object { + val ALBUM_SORT = stringPreferencesKey("album_sort") val MAX_BITRATE = intPreferencesKey("max_bitrate") val STREAM_FORMAT = stringPreferencesKey("stream_format") } diff --git a/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt index 0c5d9c2b..bc911841 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt @@ -4,6 +4,7 @@ import androidx.room3.deferredTransaction import androidx.room3.immediateTransaction import androidx.room3.useReaderConnection import androidx.room3.useWriterConnection +import com.subtracks.data.db.LibraryDao import com.subtracks.data.db.SubtracksDatabase import com.subtracks.data.model.PlaybackCursor import com.subtracks.data.model.QueueEntry @@ -173,6 +174,56 @@ class QueueRepository( } } + private suspend fun LibraryDao.songsPage( + sourceId: Long, + query: QueueSongQuery, + limit: Int, + offset: Long, + ): List { + val starred = query.starred.ordinal + return when (query.sort) { + SongSort.Album -> { + if (query.descending) { + songsPageByAlbumReversed(sourceId, starred, limit, offset) + } else { + songsPageByAlbum(sourceId, starred, limit, offset) + } + } + + SongSort.Title -> { + if (query.descending) { + songsPageByTitleReversed(sourceId, starred, limit, offset) + } else { + songsPageByTitle(sourceId, starred, limit, offset) + } + } + + SongSort.Artist -> { + if (query.descending) { + songsPageByArtistReversed(sourceId, starred, limit, offset) + } else { + songsPageByArtist(sourceId, starred, limit, offset) + } + } + + SongSort.Starred -> { + if (query.descending) { + songsPageByStarredReversed(sourceId, starred, limit, offset) + } else { + songsPageByStarred(sourceId, starred, limit, offset) + } + } + + SongSort.Added -> { + if (query.descending) { + songsPageByAddedReversed(sourceId, starred, limit, offset) + } else { + songsPageByAdded(sourceId, starred, limit, offset) + } + } + } + } + suspend fun replace( entries: List, shuffleOrder: LongArray? = null, @@ -247,23 +298,29 @@ class QueueRepository( snapshot: QueueSnapshot, position: Long, ): SongListItem? { - val flat = snapshot.flatPosition(position) ?: return null - val id = flatIds(snapshot).getOrNull(flat.toInt()) ?: return null - val sourceId = - snapshot.entries - .firstOrNull() - ?.entry - ?.sourceId ?: return null - return dao.songsByIds(sourceId, listOf(id)).firstOrNull() + if (snapshot.shuffled) { + val sourceId = + snapshot.entries + .firstOrNull() + ?.entry + ?.sourceId ?: return null + val flat = snapshot.flatPosition(position) ?: return null + val id = flatIds(snapshot).getOrNull(flat.toInt()) ?: return null + return dao.songsByIds(sourceId, listOf(id)).firstOrNull() + } + val (entry, offset) = snapshot.locate(position) ?: return null + return rows(entry, entry.offset + offset, 1).firstOrNull() } suspend fun flatIndexOf( snapshot: QueueSnapshot, songId: String, ): Long? { - if (!snapshot.shuffled) return null - val index = flatIds(snapshot).indexOf(songId) - return if (index >= 0) index.toLong() else null + val order = snapshot.shuffleOrder ?: return null + val flat = flatIds(snapshot).indexOf(songId) + if (flat < 0) return null + val position = order.indexOf(flat.toLong()) + return if (position >= 0) position.toLong() else null } suspend fun range( @@ -271,22 +328,37 @@ class QueueRepository( first: Long, last: Long, ): List { - if (snapshot.size == 0L) return emptyList() - val ids = flatIds(snapshot) - val requested = mutableListOf>() - for (position in first..last) { - val flat = snapshot.flatPosition(position) ?: continue - val id = ids.getOrNull(flat.toInt()) ?: continue - requested += position to id + if (snapshot.shuffled) { + val sourceId = + snapshot.entries + .firstOrNull() + ?.entry + ?.sourceId ?: return emptyList() + val ids = flatIds(snapshot) + val requested = mutableListOf>() + for (position in first..last) { + val flat = snapshot.flatPosition(position) ?: continue + val id = ids.getOrNull(flat.toInt()) ?: continue + requested += position to id + } + if (requested.isEmpty()) return emptyList() + val byId = dao.songsByIds(sourceId, requested.map { it.second }.distinct()).associateBy { it.song.id } + return requested.mapNotNull { (position, id) -> byId[id]?.let { QueueWindowItem(position, it) } } } - if (requested.isEmpty()) return emptyList() - val sourceId = - snapshot.entries - .firstOrNull() - ?.entry - ?.sourceId ?: return emptyList() - val byId = dao.songsByIds(sourceId, requested.map { it.second }.distinct()).associateBy { it.song.id } - return requested.mapNotNull { (position, id) -> byId[id]?.let { QueueWindowItem(position, it) } } + val items = mutableListOf() + var start = 0L + for (resolved in snapshot.entries) { + val end = start + resolved.length - 1 + val entryStart = start + start += resolved.length + if (resolved.length == 0L || end < first || entryStart > last) continue + val from = maxOf(first, entryStart) + val to = minOf(last, end) + val offset = resolved.entry.offset + (from - entryStart) + rows(resolved.entry, offset, (to - from + 1).toInt()) + .forEachIndexed { index, item -> items += QueueWindowItem(from + index, item) } + } + return items } suspend fun window( @@ -386,6 +458,31 @@ class QueueRepository( return ids } + private suspend fun rows( + entry: QueueEntry, + offset: Long, + limit: Int, + ): List = + when (entry.kind) { + QueueKind.Playlist -> { + dao.playlistSongs(entry.sourceId, entry.refId, offset, limit) + } + + QueueKind.Album -> { + dao.albumSongs(entry.sourceId, entry.refId, offset, limit) + } + + QueueKind.Songs -> { + songsQuery(entry.refId)?.let { query -> + db.libraryDao().songsPage(entry.sourceId, query, limit, offset) + } ?: dao.songs(entry.sourceId, offset, limit) + } + + QueueKind.Song -> { + dao.song(entry.sourceId, entry.refId, offset, limit) + } + } + private suspend fun QueueEntry.resolvedLength(): Long { val total = when (kind) { diff --git a/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt b/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt index 365d1bb9..4ec9b715 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt @@ -269,7 +269,7 @@ fun LibraryScreen( val searchFocus = remember { FocusRequester() } LaunchedEffect(pagerState) { - snapshotFlow { pagerState.currentPage }.collect { page -> + snapshotFlow { pagerState.settledPage }.collect { page -> onTabSelected(LibraryTab.entries[page]) } } @@ -278,7 +278,7 @@ fun LibraryScreen( if (pagerState.currentPage != selectedTab.ordinal) { pagerState.animateScrollToPage(selectedTab.ordinal) } - searchActive = false + searchActive = search.isNotEmpty() } val density = LocalDensity.current