From 73add672e1444df6fdd940f6ddc233419eeb4cd7 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Sat, 3 Oct 2026 06:32:22 +0000 Subject: [PATCH] keep the previous artwork visible and stop the neighbour flashing mid-swipe Swipe-back now goes to the previous track (previousTrack) instead of restarting, so the previous peek stays visible past the restart threshold. The frozen strip only hands back once the live neighbour props match it, so a stale neighbour can no longer flash while the slide settles. --- .../subtracks/playback/PlaybackController.kt | 29 +++++++++++--- .../subtracks/ui/playback/NowPlayingScreen.kt | 40 +++++++++++++------ .../playback/PlaybackControllerTest.kt | 8 +++- .../ui/playback/NowPlayingTapsTest.kt | 28 ++++++++++++- 4 files changed, 83 insertions(+), 22 deletions(-) diff --git a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt index 6dca6ad2..47d39f98 100644 --- a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt +++ b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt @@ -25,8 +25,6 @@ import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import kotlin.random.Random -internal const val PLAYBACK_RESTART_THRESHOLD_MS = 3_000L - data class QueueItem( val id: String, val title: String, @@ -747,7 +745,7 @@ class PlaybackController( fun previous() { val player = player ?: return - if (player.currentPositionMs > PLAYBACK_RESTART_THRESHOLD_MS) { + if (player.currentPositionMs > RESTART_THRESHOLD_MS) { scope.launch { startLock.withLock { player.seekTo(0) } refresh() @@ -788,6 +786,26 @@ class PlaybackController( } } + fun previousTrack() { + val snapshot = snapshot ?: return + if (snapshot.size == 0L) return + val current = currentPosition() ?: return + if (offline) { + scope.launch { + downloads.awaitLoaded() + val target = + downloadedAtOrBefore(snapshot, current - 1) + ?: if (repeatMode == RepeatMode.All) downloadedAtOrBefore(snapshot, snapshot.size - 1) else null + target?.let { jumpTo(it) } + } + return + } + when { + current > 0 -> jumpTo(current - 1) + repeatMode == RepeatMode.All -> jumpTo(snapshot.size - 1) + } + } + fun addToQueue( sourceId: Long, kind: QueueKind, @@ -906,15 +924,13 @@ class PlaybackController( suspend fun previousItem(): QueueItem? { var current: QueueSnapshot? = null var position: Long? = null - var restarting = false startLock.withLock { current = snapshot position = currentPosition() - restarting = (player?.currentPositionMs ?: 0L) > PLAYBACK_RESTART_THRESHOLD_MS } val snapshot = current ?: return null val at = position ?: return null - if (at <= 0 || restarting) return null + if (at <= 0) return null return queueRepository.itemAt(snapshot, at - 1)?.toQueueItem() } @@ -1513,6 +1529,7 @@ class PlaybackController( const val BUFFERING_INDICATOR_DELAY_MS = 1_000L const val QUEUE_WINDOW_RADIUS = 25L const val WINDOW_SHIFT_DELAY_MS = 400L + const val RESTART_THRESHOLD_MS = 3_000L const val SEED_PIN_TRIES = 1_000_000 const val FILE_SCHEME = "file:" diff --git a/app/src/main/kotlin/com/subtracks/ui/playback/NowPlayingScreen.kt b/app/src/main/kotlin/com/subtracks/ui/playback/NowPlayingScreen.kt index da1ad155..4164aa39 100644 --- a/app/src/main/kotlin/com/subtracks/ui/playback/NowPlayingScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/playback/NowPlayingScreen.kt @@ -77,7 +77,6 @@ import com.subtracks.data.model.Song import com.subtracks.data.repo.DownloadRepository import com.subtracks.data.repo.LibraryRepository import com.subtracks.data.source.StarType -import com.subtracks.playback.PLAYBACK_RESTART_THRESHOLD_MS import com.subtracks.playback.PlaybackController import com.subtracks.playback.PlaybackState import com.subtracks.playback.QueueItem @@ -153,6 +152,21 @@ internal fun rotateStrip( listOf(null, base.getOrNull(0), base.getOrNull(1)) } +internal fun handBackReady( + frozen: List, + itemId: String?, + previousArt: NowPlayingArt?, + nextArt: NowPlayingArt?, +): Boolean { + val centre = frozen.getOrNull(1)?.id ?: return false + if (centre != itemId) return false + return when { + frozen.getOrNull(2) == null -> previousArt?.id == frozen.getOrNull(0)?.id + frozen.getOrNull(0) == null -> nextArt?.id == frozen.getOrNull(2)?.id + else -> true + } +} + @Composable fun NowPlayingRoute( onBack: () -> Unit, @@ -211,8 +225,7 @@ fun NowPlayingRoute( name = item.title, ) var adjacent by remember { mutableStateOf(AdjacentArt()) } - val previousReachable = positionMs <= PLAYBACK_RESTART_THRESHOLD_MS - LaunchedEffect(state.item?.id, state.layout, previousReachable) { + LaunchedEffect(state.item?.id, state.layout) { if (state.item == null) { adjacent = AdjacentArt() return@LaunchedEffect @@ -230,8 +243,9 @@ fun NowPlayingRoute( title = sourceTitle.value, coverArt = art, thumbnailRef = thumbnail, - previousArt = if (previousReachable) adjacent.previous else null, + previousArt = adjacent.previous, nextArt = adjacent.next, + onSkipPrevious = controller::previousTrack, artwork = rememberArtworkColors(thumbnail ?: art, fallbackName = state.item?.title), onBack = onBack, onQueue = onQueue, @@ -263,6 +277,7 @@ fun NowPlayingScreen( onPlayPause: () -> Unit, onNext: () -> Unit, onPrevious: () -> Unit, + onSkipPrevious: () -> Unit = onPrevious, onShuffle: () -> Unit = {}, onRepeat: () -> Unit = {}, onMore: () -> Unit = {}, @@ -317,18 +332,19 @@ fun NowPlayingScreen( fun commitSwipe(direction: Int) { frozenStrip = rotateStrip(strip, direction) swipeOffsetX += direction * coverWidthPx - if (direction > 0) onNext() else onPrevious() + if (direction > 0) onNext() else onSkipPrevious() animateSwipe(0f) } - // The rotated strip keeps the slide continuous while the controller catches up. Hand back once - // it reports the target track, or after a grace period when the action did not move there (a - // restart, an offline skip), so a frozen strip can never stay desynced from playback. - LaunchedEffect(state.item?.id, frozenStrip, dragging) { + // The rotated strip keeps the slide continuous while the controller catches up. It only hands + // back once the live props agree with the frozen slots, so a stale neighbour can never flash + // during the slide; the timeout covers an action that does not move (an offline skip). + LaunchedEffect(state.item?.id, frozenStrip, dragging, previousArt, nextArt) { val pending = frozenStrip ?: return@LaunchedEffect - if (pending.getOrNull(1)?.id == state.item?.id) { + if (dragging) return@LaunchedEffect + if (handBackReady(pending, state.item?.id, previousArt, nextArt)) { frozenStrip = null - } else if (!dragging) { + } else { delay(HAND_BACK_TIMEOUT_MS) if (frozenStrip === pending) frozenStrip = null } @@ -422,7 +438,7 @@ fun NowPlayingScreen( } SwipeAction.Previous -> { - onPrevious() + onSkipPrevious() animateSwipe(0f) } diff --git a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt index f60248ea..29400916 100644 --- a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt @@ -213,7 +213,7 @@ class PlaybackControllerTest { } @Test - fun previousItemIsNullOncePreviousWouldRestartTheTrack() { + fun previousTrackMovesBackEvenOncePreviousWouldRestart() { seedAlbum(3, sourceId = 1) controller.playAlbum(1, "al1", 1) await { @@ -222,8 +222,12 @@ class PlaybackControllerTest { } handle.positionMs = 3_100 + controller.previousTrack() - assertNull(runBlocking { controller.previousItem() }) + await { + controller.state.value.item + ?.id == "s1" + } } @Test diff --git a/app/src/test/kotlin/com/subtracks/ui/playback/NowPlayingTapsTest.kt b/app/src/test/kotlin/com/subtracks/ui/playback/NowPlayingTapsTest.kt index 2365ce99..26fd869b 100644 --- a/app/src/test/kotlin/com/subtracks/ui/playback/NowPlayingTapsTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/playback/NowPlayingTapsTest.kt @@ -56,6 +56,7 @@ class NowPlayingTapsTest { onArtistClick: (() -> Unit)? = null, onNext: () -> Unit = {}, onPrevious: () -> Unit = {}, + onSkipPrevious: () -> Unit = onPrevious, previousArt: NowPlayingArt? = null, nextArt: NowPlayingArt? = null, ) { @@ -72,6 +73,7 @@ class NowPlayingTapsTest { onPlayPause = {}, onNext = onNext, onPrevious = onPrevious, + onSkipPrevious = onSkipPrevious, onAlbumClick = onAlbumClick, onArtistClick = onArtistClick, onSeek = {}, @@ -163,12 +165,34 @@ class NowPlayingTapsTest { fun swipingRightOnTheCoverGoesToThePreviousTrack() { var next = false var previous = false - render(onNext = { next = true }, onPrevious = { previous = true }) + var skipPrevious = false + render( + onNext = { next = true }, + onPrevious = { previous = true }, + onSkipPrevious = { skipPrevious = true }, + ) composeRule.onNodeWithTag(NOW_PLAYING_COVER_TAG).performTouchInput { swipeRight() } - assertTrue("a right swipe should go to the previous track", previous) + assertTrue("a right swipe should skip to the previous track", skipPrevious) assertTrue("a right swipe should not skip to the next track", !next) + assertTrue("a right swipe should not use the restart-on-previous button", !previous) + } + + @Test + fun theStripHandsBackOnlyWhenTheLivePropsAgree() { + val before = adjacentArt("s0") + val current = adjacentArt("s1") + val next = adjacentArt("s2") + val after = adjacentArt("s3") + val frozenNext = listOf(current, next, null) + val frozenPrevious = listOf(null, before, current) + + assertTrue(!handBackReady(frozenNext, itemId = "s1", previousArt = before, nextArt = after)) + assertTrue(!handBackReady(frozenNext, itemId = "s2", previousArt = before, nextArt = after)) + assertTrue(handBackReady(frozenNext, itemId = "s2", previousArt = current, nextArt = after)) + assertTrue(handBackReady(frozenPrevious, itemId = "s0", previousArt = null, nextArt = current)) + assertTrue(!handBackReady(frozenPrevious, itemId = "s1", previousArt = null, nextArt = current)) } @Test