[casha-cashu]

Summary of Changes

This pull request addresses UX and stability issues in lyrics display, playlist locating, and playback transition scheduling:

1. Lyrics Line Layout Stability & Reflow Jump Prevention (LyricsSheet.kt):

   • Issue: Switching between active and inactive lyric lines caused lines or words to abruptly reflow and jump to new lines mid-sentence because font weight switched between Normal and Bold, altering glyph metrics. An overlapping Box containing an invisible duplicate bold Text was previously used as a workaround.
   • Fix: Standardized FontWeight.Bold for synchronized lyrics lines (LyricLineRow and LyricWordSpan), eliminating font-metric differences between active and inactive states so text layouts remain completely stable without needing invisible duplicate measure layers. Plain (unsynchronized) lyrics retain the theme's regular weight (titleLarge) and 0.75f secondary scale since they do not toggle active states. Removed redundant overflow = TextOverflow.Clip and experimental wrapping parameters.
   • Steps to reproduce: Play a synchronized song with longer lyric lines on narrow viewports; notice words wrapping onto the next line the moment the line becomes active.

2. Initial Lyrics Autoscroll Dropped on Opening (LyricsSheet.kt):

   • Issue: When opening the lyrics sheet while a song is already playing, LaunchedEffect in SyncedLyricsList checked if (listState.layoutInfo.totalItemsCount == 0) return@LaunchedEffect. If the items had not yet been laid out during the initial composition pass, the effect returned and never re-triggered if currentLineIndex remained constant, leaving the view unscrolled.
   • Fix: Replaced the early return with snapshotFlow { listState.layoutInfo.totalItemsCount }.first { it > 0 }, suspending until the layout is ready and reliably performing the initial alignment.
   • Steps to reproduce: Start playback of any synchronized song and wait until several lines in; tap to open the lyrics sheet. Notice lyrics occasionally remaining positioned at the very top.

3. Locate Button Viewport Visibility Predicate (LibrarySongsTab.kt):

   • Issue: visibleItemsInfo in LazyColumn includes items extending into bottom content padding (floating mini-player + bottom bar + 30dp). The previous index check (currentSongListIndex in first until first + count) considered the currently playing song "visible" even when it was completely covered by the bottom bar and mini-player, preventing the locate floating action button from showing.
   • Fix: Extracted a pure predicate isItemFullyVisibleInViewport that verifies the item's offset and size against the unobstructed viewport (layoutInfo.viewportStartOffset + layoutInfo.beforeContentPadding through layoutInfo.viewportEndOffset - layoutInfo.afterContentPadding).
   • Regression tests: Added unit tests in LibrarySongsTabTest.kt verifying item visibility in the clear zone, under top/bottom content padding, outside viewport bounds, and invalid/inverted bounds.
   • Steps to reproduce: Scroll the songs list until the currently playing song is tucked beneath the bottom mini-player; the locate button now correctly appears and scrolls the item into clear view when tapped.

4. Bounded Duration Wait & Safe Rescheduling in TransitionController (TransitionController.kt):

   • Issue: In TransitionController, while waiting for track duration when player is paused or idle, a while ((duration == C.TIME_UNSET || duration <= 0) && isActive) loop repeatedly delayed by 500 ms indefinitely. Additionally, if the wait timed out, needsDurationReschedule could remain set after a later successful wait in the same job, leading to redundant cancellations and reschedules.
   • Fix: Bounded the wait to 10 attempts (~5 seconds). If timed out, engine.cancelNext() is called to release the prepared auxiliary player and needsDurationReschedule is set to retry when the player reports ready or updates duration. needsDurationReschedule is cleared at the top of collectLatest and immediately upon obtaining a valid duration.
   • Steps to reproduce: Pause playback at transition point or inspect background coroutines during idle playback states.

Philosophy & Dependencies

• Strictly FOSS-compliant.
• No AI or closed cloud services added.
• No schema or Room migrations.
• Zero new dependencies.
• Verified with ./gradlew testDebugUnitTest and ./gradlew lintDebug (all passing).

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues remain.

Summary

This PR makes lyrics scrolling wait for the list to appear, skips near-zero blur effects, and gives plain lyrics consistent text widths.

• The lyrics sheet waits for its list before scrolling to the current line.
• Lyrics clear inactive blur and give plain text the full row.
• The songs tab shows the locate action when the current row is covered.
• Crossfade scheduling retries when track duration becomes available.

_{Reviews (4) · Last reviewed commit: "Merge upstream/main into fix/lyrics-layo..." · Reviewed by Greptile}

[lostf1sh]

Ran :app:compileDebugKotlin :app:lintDebug :app:testDebugUnitTest on the PR head: it compiles, lint passes (the one warning is in a file this PR does not touch), and all 1250 unit tests pass.

Two things need changing before merge, the rest is cleanup:

• The LibrarySongsTab change is a no-op, and the locate button bug it describes is still there. Inline comment has the actual cause.
• needsDurationReschedule can stay set after a later successful wait, which causes a redundant cancel and reschedule. Inline comment.
• Every lyric line is now bold, which is a visible design change the description does not mention. Please call it out so it can be decided on purpose.

Smaller points:

• Item 4 of the description still says scheduling is aborted; the second commit reschedules instead.
• No regression tests were added. The visibility predicate is a pure function waiting to be extracted and tested.
• The repo uses plain imperative commit subjects (Fix ...) rather than fix(scope): prefixes, and prefers one fix per PR. Four unrelated fixes in one PR makes it harder to merge the ready parts (the autoscroll fix is one) on their own.

[lostf1sh]

This is the same range as before. visibleItemsInfo in a LazyColumn without sticky headers is contiguous, so first().index..last().index and first until first + count cover identical indices, and the locate button behaves exactly as it does on main.

The symptom in the description has a different cause: visibleItemsInfo still includes rows that sit under the bottom contentPadding (bottom bar + mini player + 30dp), so the current song counts as visible while the mini player covers it. To fix it, compare the item's offset + size against layoutInfo.viewportEndOffset - layoutInfo.afterContentPadding (and offset against viewportStartOffset + beforeContentPadding) instead of relying on the index range. That predicate fits in a small pure function with a unit test, which the contributing guidelines ask for on behaviour changes.

[lostf1sh]

The flag stays set when the wait later succeeds inside the same job. Sequence: the loop times out and sets needsDurationReschedule = true; the settings combine emits again, collectLatest re-runs this block, and this time player.duration is valid, so the transition is scheduled normally. Nothing clears the flag on that path (only scheduleTransitionFor and the listener hooks do). The next onTimelineChanged with a non-playlist reason, or a pause and resume, then sees the stale flag, cancels the already scheduled job and starts over with another 1.5 s delay and a fresh prepareNext. Clearing the flag as soon as the loop obtains a valid duration (or at the top of the collectLatest block) closes that gap.

Smaller point: engine.prepareNext(transitionTarget) has already run by the time we bail out here, so the auxiliary player stays prepared with nothing scheduled until the next scheduleTransitionFor. That matches the old behaviour, but an engine.cancelNext() before returning would release it.

[lostf1sh]

This makes every lyric line bold: non-current synced lines, non-highlighted words and plain lyrics, which on main render at the theme's titleLarge weight (regular). That is a visible design change for everyone who opens lyrics, not only a layout fix, and the description does not mention it. If the uniform weight is the intended look, say so in the description so it can be judged on that basis. The secondary text scale change from 0.75 to 0.72 (lines 1496 and 1716) is in the same category.

[lostf1sh]

Hyphens.None and TextOverflow.Clip are already the defaults (TextStyle.hyphens resolves to None, and Text defaults overflow to Clip), so those additions change nothing. LineBreak.Paragraph only switches the strategy from Simple to HighQuality; a word longer than the line still gets broken mid-word under either strategy, so it does not back the "prevent awkward mid-word breaks" claim. None of this is harmful, but I would drop the no-op properties and reword the description.

[lostf1sh]

BlurEffectCache.get already returns null for radiusPx <= 0f, and roundToInt() maps anything below 0.5 px to 0, so this guard only differs from the previous call at exactly 0.5 px. Fine to keep, but it is not a behaviour change.

[lostf1sh]

This one checks out. LaunchedEffect runs before the first measure pass, so totalItemsCount is 0 on open, and none of the keys re-trigger the effect once the list is laid out (metrics comes from BoxWithConstraints, which is already resolved). Suspending on the snapshot is the right fix.

[casha-cashu]

Thanks for the thorough review and clear pointers, @lostf1sh!

I have addressed all the feedback in commit bd64441:

1. LibrarySongsTab.kt: Extracted pure visibility predicate isItemFullyVisibleInViewport that tests item offsets against the unobstructed viewport (viewportStartOffset + beforeContentPadding .. viewportEndOffset - afterContentPadding), correctly detecting when an item sits behind the bottom bar and mini-player.
2. LibrarySongsTabTest.kt: Added unit tests covering middle visible items, exact viewport bounds, partial/full coverage under bottom content padding, top padding, items completely outside viewport, and invalid/inverted bounds.
3. TransitionController.kt:
   • needsDurationReschedule is now reset to false at the top of collectLatest and immediately after the duration loop succeeds.
   • Invoked engine.cancelNext() when the duration wait times out to cleanly release the auxiliary player.
4. LyricsSheet.kt:
   • Reverted withLyricsFontSize and PlainLyricsLine to regular weight (titleLarge) and secondary scale 0.75f.
   • Dropped no-op properties (Hyphens.None, TextOverflow.Clip, LineBreak.Paragraph).
   • Retained uniform FontWeight.Bold for synchronized lines (LyricLineRow and LyricWordSpan) because it guarantees invariant text metrics across active/inactive states and completely eliminates word wrapping jumps without invisible duplicate measure layers.
5. PR Description: Updated description to accurately describe the predicate calculation, duration wait retry behavior, bold synced styling rationale, and referenced the new regression tests.

Verified locally with ./gradlew testDebugUnitTest and ./gradlew lintDebug (both pass with 0 errors).