From 684fffd7f8a6eb4294272373cd71ab72a6050369 Mon Sep 17 00:00:00 2001 From: vicrodh <103399283+vicrodh@users.noreply.github.com> Date: Tue, 4 Aug 2026 09:41:37 -0600 Subject: [PATCH] feat(qt): the mini queue and lyrics surfaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Block B4, the last code block of the miniplayer/tray contract. MiniQueueSurface — the current track first, then the FULL upcoming list: uncapped, unpaginated, a position number where the main list has artwork, and hover beating active on the row background. A recycling ListView, since a 2,000-track queue in a 380 px card is exactly the case the track's perf rule forbids materialising. Its document is applied IMPERATIVELY, and that is the perf rule, not style. A bound model hands the view a new array on every publish that differs, which is a full model reset: a user scrolled to row 120 gets yanked to the top every time the track advances. The upstream guard only suppresses IDENTICAL republishes — a real change is precisely what it lets through. The cure is the port's own, from Cortinilla: latch contentY, apply, restore under Qt.callLater so a burst costs one restore. MiniLyricsSurface — the empty test is on the line COUNT, not on status; there is no loading, error or offline state in the mini. It mounts its own LyricsSyncEngine, which is what makes the reference's ensure_lyrics_engine hack unnecessary: the karaoke advances with the sidebar panel closed. Both surfaces are gated on QbzMini.open, not on the surface alone. The mini window is created once and never destroyed, so a surface-only gate would leave them mounted and re-parsing a whole JSON document on every queue or lyrics edit for the rest of the session with the mini nowhere on screen. The lyrics pump additionally reads the MINI WINDOW's visibility. Section 4.5 spells the gate as open + surface + synced, but QbzMini.open is a logical flag that stays true when the window is minimised, and the general rule says that gate must read the mini's own window. The window term is added, not substituted. queue_play routes remote-first and RETURNS on Err — an error must never fall through to local playback — and its row-to-index mapping calls the tested helper rather than restating it. Row 0 resumes only when a current track exists; with none it plays upcoming[0], which is the reference's off-by-one this port declines to inherit. mini_qt::clamp_to_implemented is DELETED. It was the temporary guard that kept an unimplemented surface from stranding the user on a blank card, and its doc said the last block to land would remove it. Driven and seen: ten rows with their numbers, titles, artists, durations, the E badges on the two explicit tracks, and no artwork. What is NOT seen, because it needs sound: a row click actually playing, the off-by-one on live behaviour, and the karaoke advancing. cc --- crates/qbz-qt/build.rs | 2 + .../qml/miniplayer/MiniLyricsSurface.qml | 168 ++++++++++ .../qml/miniplayer/MiniQueueSurface.qml | 288 ++++++++++++++++++ crates/qbz-qt/qml/miniplayer/MiniShell.qml | 32 +- crates/qbz-qt/src/mini_bridge.rs | 115 ++++++- crates/qbz-qt/src/mini_qt.rs | 67 ++-- crates/qbz-qt/src/now_playing.rs | 15 + 7 files changed, 625 insertions(+), 62 deletions(-) create mode 100644 crates/qbz-qt/qml/miniplayer/MiniLyricsSurface.qml create mode 100644 crates/qbz-qt/qml/miniplayer/MiniQueueSurface.qml diff --git a/crates/qbz-qt/build.rs b/crates/qbz-qt/build.rs index 387d1b58d..be41e18d7 100644 --- a/crates/qbz-qt/build.rs +++ b/crates/qbz-qt/build.rs @@ -324,6 +324,8 @@ fn main() { "qml/miniplayer/MiniCoverArt.qml", "qml/miniplayer/MiniCompactSurface.qml", "qml/miniplayer/MiniArtworkSurface.qml", + "qml/miniplayer/MiniQueueSurface.qml", + "qml/miniplayer/MiniLyricsSurface.qml", "qml/miniplayer/MiniFooter.qml", "qml/miniplayer/MiniWindowControls.qml", "qml/miniplayer/MiniSeek.qml", diff --git a/crates/qbz-qt/qml/miniplayer/MiniLyricsSurface.qml b/crates/qbz-qt/qml/miniplayer/MiniLyricsSurface.qml new file mode 100644 index 000000000..7ba6b948c --- /dev/null +++ b/crates/qbz-qt/qml/miniplayer/MiniLyricsSurface.qml @@ -0,0 +1,168 @@ +// MiniLyricsSurface — surface 4 (2026-08-03 miniplayer/tray contract §4.5), +// port of `crates/qbz-ui/ui/miniplayer/MiniLyricsSurface.slint` (46 L, read +// whole). +// +// THE EMPTY TEST IS ON THE LINE COUNT, NOT ON `status` +// (`MiniLyricsSurface.slint:11`). The mini has no loading state, no error +// state and no offline-miss state — only "lines" and "no lines" (§15 trap 13). +// The sidebar panel's `status === 2 && lines.length > 0` is deliberately NOT +// reproduced here. +// +// THE SLINT VIEW TAKES ITS DATA FROM A GLOBAL; THE QT COMPONENT TAKES IT AS +// PROPS. `LyricsLinesView.qml` declares `lines: []`, `synced: false`, +// `showTranslation: false` and `sync: null` as ordinary properties with inert +// defaults (`:35-39`), so "pass only the sizing overrides" would mount a +// permanently empty, permanently monolingual view. The mount below is §4.5's +// complete nine-binding list and nothing else — the four sizing numbers are +// the only DIVERGENCE from the component's defaults; the five data bindings +// are the mount. +// +// Everything §4.5 leaves out is left out on purpose: `dimmingMode` (2), +// `autoFollow` (true), `uppercase` (false), `fontIndex` (0) and `activeColor` +// (theme.accent) stay at the component's defaults. The SIDEBAR passes those +// five from the user's prefs (`LyricsPanel.qml:255-263`); the Slint mini +// renders with the fixed values baked into its own view instance +// (`miniplayer.rs:519-534`), so leaving the Qt defaults in place is what +// reproduces it. +// +// `seekRequested` stays UNCONNECTED. The signal exists and the sidebar honours +// it, but the Slint mini has no click-to-seek and its own view header records +// the parity reason. An inert handler that does nothing would be a deferral +// artifact, so there is none. +// +// `show-attribution: false` (`LyricsLinesView.slint:298`) is DEAD in the +// reference — declared and read nowhere, the attribution having moved to a +// fixed footer in the sidebar. The Qt component has no such property and MUST +// NOT grow one (§12-P9). +// +// NO COLOUR LITERALS: every colour on this surface is a theme token, so the +// #RRGGBBAA -> #AARRGGBB conversion the port owes Slint literals has nothing +// to convert here. + +import QtQuick +import QtQuick.Window +import com.blitzfc.qbz +import "../shell" +import "../theme" + +Item { + id: root + + QbzTheme { id: theme } + + // {status, synced, provider, error, translationAvailable, lines[]}. The + // property's Rust default is a full-shape document, which is why the + // sidebar parses it unguarded (`LyricsPanel.qml:36`). + readonly property var doc: JSON.parse(QbzLyrics.docJson) + readonly property var lines: root.doc.lines !== undefined && root.doc.lines !== null + ? root.doc.lines : [] + readonly property bool synced: root.doc.synced === true + // THE gate — count only (see the header). + readonly property bool hasLines: root.lines.length > 0 + + // --- The mini's OWN sync engine (§4.5, §3.2) --------------------------- + // A SECOND instance, not a share of the sidebar's. It is stateless per + // instance (`_index`/`_progress`/`_animMs` are file-local) and calls only + // read-only invokables (`QbzLyrics.activeIndexAt`, `fillFractionAt`), so + // two instances cannot fight. + // + // The gate is the MINI's own open+surface — never the sidebar's + // `QbzShell.lyricsOpen`, which nothing here writes — and it carries the + // panel's `synced` term (`LyricsPanel.qml:55`) so the 33/250 ms pump never + // ticks on an unsynced document. Gating on the mini's own state is exactly + // what makes the reference's `ensure_lyrics_engine` hack unnecessary: the + // karaoke advances with the sidebar panel CLOSED. + // §4.5 spells this gate as open + surface + synced. The window term is + // ADDED, not substituted, because §7-M8 is a general rule the mini cannot + // opt out of: "the windowShowing gate must read THE MINI'S OWN WINDOW", + // called there the single most likely copy-paste bug in this port. + // `QbzMini.open` is a LOGICAL flag — it stays true when the compositor or + // the taskbar minimises the mini, and the 33 ms pump would then pull three + // invokables thirty times a second, animating a window nobody can see. + // The blessed shape is `AmbientField.qml:42`; reading it off the MINI is + // the whole point, so `root.Window.window` here is the mini, never the + // shell. + readonly property bool windowShowing: root.Window.window !== null + && root.Window.window.visibility !== Window.Hidden + && root.Window.window.visibility !== Window.Minimized + + LyricsSyncEngine { + id: syncEngine + gateOpen: QbzMini.open && QbzMini.surface === 4 + && root.windowShowing && root.hasLines && root.synced + } + + // Every doc commit resets the ladder and re-lands on the right line — the + // Slint `kick()` on commit, and the same two calls the sidebar makes + // (`LyricsPanel.qml:58-66`). + Connections { + target: QbzLyrics + function onDocJsonChanged() { + syncEngine.reset() + syncEngine.kick() + } + } + + // --- Empty state (MiniLyricsSurface.slint:13-35) ----------------------- + // VerticalLayout { alignment: center; spacing: 10; padding: 24 }: a + // centred 30x30 mic-vocal glyph in text-muted, then the 13 px label. The + // 24 px padding is uniform on all four sides, so the column's width is the + // surface minus 48. + Column { + anchors.centerIn: parent + width: Math.max(0, root.width - 48) + visible: !root.hasLines + spacing: 10 + + // The icon's own centring wrapper (`:20-22` is a + // HorizontalLayout { alignment: center }). + Item { + width: parent.width + height: 30 + QbzIcon { + anchors.horizontalCenter: parent.horizontalCenter + name: "mic-vocal" + tintName: "muted" + width: 30 + height: 30 + } + } + + Text { + width: parent.width + text: QbzSession.tr("No lyrics", QbzSession.trRev) + color: theme.textMuted + font.pixelSize: 13 + horizontalAlignment: Text.AlignHCenter + wrapMode: Text.WordWrap + } + } + + // --- Populated (:37-45) ------------------------------------------------ + // The nine bindings of §4.5, in its order: the five that are the mount, + // then the four sizing overrides. The 280 px scale the sidebar computes is + // the SIDEBAR's; this card is 340-380 px wide and owns its own numbers, + // which is what `sidePadding` and `lineGap` exist for. + LyricsLinesView { + anchors.fill: parent + visible: root.hasLines + + lines: root.lines + synced: root.synced + // No toggle of its own: the mini FOLLOWS the sidebar's bilingual state + // through the shared model, exactly as `mirror_lyrics_gated` copies it + // (`crates/qbz/src/miniplayer.rs:610-612`). + showTranslation: QbzLyrics.showTranslation + sync: syncEngine + // §13-D5b, a divergence in the port's favour: the reference's mini + // cannot honour lite-fill (its window's LyricsState instance is never + // mirrored, so it always pays the animated karaoke sweep). Here the + // singleton is shared. Do not "fix" it back. + liteFill: QbzLyrics.liteFill + + sidePadding: 18 // component default 6 + lineGap: 10 // component default 12 + sizeInactive: 13 // == the component default, passed because the + sizeActive: 15 // reference states both explicitly + } +} diff --git a/crates/qbz-qt/qml/miniplayer/MiniQueueSurface.qml b/crates/qbz-qt/qml/miniplayer/MiniQueueSurface.qml new file mode 100644 index 000000000..bf9f4ce06 --- /dev/null +++ b/crates/qbz-qt/qml/miniplayer/MiniQueueSurface.qml @@ -0,0 +1,288 @@ +// MiniQueueSurface — surface 3 (2026-08-03 miniplayer/tray contract §4.4), +// port of `crates/qbz-ui/ui/miniplayer/MiniQueueSurface.slint` (139 L, read +// whole). +// +// The current track plus the FULL upcoming list — not capped at 20, not +// paginated, not de-duplicated. Rows mirror the main sidebar's UP NEXT rows +// and stop there: no hover actions, no heart, no context menu, no +// drag-reorder, no remove, no stop-after marker, no section headers. The +// header of the reference states the rule this surface exists under — "the +// mini must not have MORE than the main" (:6). +// +// TWO THINGS THAT LOOK LIKE OMISSIONS AND ARE NOT: +// +// - NO ARTWORK. The reference builds these rows with an EMPTY prior-image map +// (`crates/qbz/src/queue.rs:1032-1033`) and renders a POSITION NUMBER in +// its place (§15 trap 11). Do not "improve" it. +// - HOVER BEATS ACTIVE on the row background. The reference's ternary tests +// hover FIRST (`:45-47`), so hovering the playing row paints surfaceHover, +// not the accent tint (§15 trap 15). +// +// THE DATA is `QbzMini.miniQueueJson`, built by `mini_qt::mini_queue_doc` and +// published from the ONE queue funnel (`queue_qt::publish` -> its own +// identical-document guard, §7-M9), never from a fetch of this surface's own +// (§7-M1). Neither existing document serves: `queue_json` is PAGED at 40 and +// `coverflow_json` carries no duration and no explicit flag (§4.4.1). +// `currentId` — not a per-row flag — is the highlight key (`:42`). +// +// DIVERGENCE §13-D7: a recycling ListView with `reuseItems: true`, where the +// reference is a Flickable + VerticalLayout that materialises every row. An +// unbounded queue in a 380 px window is exactly the case TRACK-RULES §6 +// forbids (§7-M5). The structural consequence is deliberate: the delegate +// reads `index`, never a captured loop variable, and it carries no popup of +// its own — with reuseItems every recycled delegate would get its own +// instance (`PmListRow.qml:14`, `PmFolderMenu.qml:3`). +// +// NO COLOUR LITERALS LIVE HERE. §4.11's only entry for this file is +// `accent.with-alpha(0.10)`, which is a token expression rather than a hex +// string, so the #RRGGBBAA -> #AARRGGBB conversion the port owes every Slint +// literal has nothing to convert on this surface. + +import QtQuick +import com.blitzfc.qbz +import "../theme" + +Item { + id: root + + QbzTheme { id: theme } + + // Parsed ONCE per publish, not per row (§7). The bridge's default is the + // full-shape `{"currentId":"","rows":[]}` (mini_qt.rs's MINI_QUEUE_EMPTY), + // so `.rows` exists in the pre-publish frame too; the try/catch is the + // same belt MiniShell wears over its own queue read. + // APPLIED IMPERATIVELY, not bound — and that is §7-M9, not style. + // + // A bound `model:` hands the ListView a NEW JS array on every publish whose + // content differs, which is a full model reset: every delegate destroyed + // and `contentY` clamped to 0. The upstream identical-document guard + // (`queue_qt.rs`'s LAST_MINI_QUEUE_JSON) only suppresses IDENTICAL + // republishes; a real change is exactly what it lets through. So a user + // scrolled to row 120 of a long queue is yanked back to the top every time + // the track advances — once per track, forever. The reference does not + // have this problem because it keeps the offset on the Flickable and only + // rebuilds the `for` inside it (MiniQueueSurface.slint:30-36). + // + // The cure is the port's own, and its comment describes this exact failure: + // Cortinilla.qml:34-70 (latch contentY, apply, restore under Qt.callLater + // so a burst of republishes costs ONE restore). + property var doc: ({ "currentId": "", "rows": [] }) + property var rows: [] + property string currentId: "" + + function parseDoc() { + try { + return JSON.parse(QbzMini.miniQueueJson) + } catch (e) { + return ({ "currentId": "", "rows": [] }) + } + } + + property real _restoreY: 0 + function applyDoc() { + root._restoreY = list.contentY + root.doc = root.parseDoc() + root.rows = root.doc.rows || [] + root.currentId = root.doc.currentId !== undefined ? root.doc.currentId : "" + Qt.callLater(root._restoreScroll) + } + function _restoreScroll() { + var maxY = Math.max(0, list.contentHeight - list.height) + list.contentY = Math.max(0, Math.min(root._restoreY, maxY)) + } + + Component.onCompleted: root.applyDoc() + Connections { + target: QbzMini + function onMiniQueueJsonChanged() { root.applyDoc() } + } + + // --- Empty state (MiniQueueSurface.slint:15-23) ------------------------- + // A single Text at 100% x 100%, centred on BOTH axes inside itself — not a + // centred label inside a wrapper. + Text { + anchors.fill: parent + visible: root.rows.length === 0 + text: QbzSession.tr("Queue is empty", QbzSession.trRev) + color: theme.textMuted + font.pixelSize: 13 + horizontalAlignment: Text.AlignHCenter + verticalAlignment: Text.AlignVCenter + } + + // --- The list (:26-40, :129-137) --------------------------------------- + // padding-top 6 / padding-bottom 8 (asymmetric, and that IS the + // reference's) and NO spacing — the rows are flush, so the pitch equals + // the 44 px row height. Side padding lives INSIDE the row, because a + // vertical ListView writes its delegates' `x` and a delegate-side `x` + // would be overwritten (`AddToMixtapeModal.qml:352-361`). + ListView { + id: list + anchors.fill: parent + visible: root.rows.length > 0 + topMargin: 6 + bottomMargin: 8 + spacing: 0 + cacheBuffer: 200 + reuseItems: true + clip: true + boundsBehavior: Flickable.StopAtBounds + model: root.rows + + delegate: Rectangle { + id: row + + required property var modelData + required property int index + + width: list.width + height: 44 + radius: 4 + antialiasing: true + + // `item.id != "" && item.id == current-track-id` (:42) — BOTH + // halves, so an empty id is never active. + readonly property bool active: row.modelData.id !== "" + && row.modelData.id === root.currentId + + // :45-47 — hover is tested FIRST. See the header. + color: rowArea.containsMouse + ? theme.surfaceHover + : (row.active + ? Qt.rgba(theme.accent.r, theme.accent.g, theme.accent.b, 0.10) + : "transparent") + + // Declared FIRST so every visual below paints on top of it, which + // is the order the reference declares its TouchArea in and for the + // same stated reason (:49-56). It is the row's ONLY hit area. + MouseArea { + id: rowArea + anchors.fill: parent + hoverEnabled: true + cursorShape: Qt.PointingHandCursor + // `index`, never a captured variable — the delegate is + // RECYCLED (§13-D7). The bridge owns the rest: remote-first, + // the row-0 test against `currentId`, and the flat upcoming + // index (§4.4.3). + onClicked: QbzMini.queuePlay(row.index) + } + + // The active bar (:58-65): 3 x 36 at x = 0, i.e. UNDER the 12 px + // left padding the number column starts at. + Rectangle { + visible: row.active + x: 0 + y: 4 + width: 3 + height: row.height - 8 + radius: 3 + color: theme.accent + } + + // Number column (:77-86): a FIXED 22 px, centred both ways, the + // 1-based DISPLAY position over the whole rendered list — which is + // the unfiltered list, because this surface has no filter (§8.1). + Text { + id: num + x: 12 + width: 22 + anchors.verticalCenter: parent.verticalCenter + text: String(row.index + 1) + color: row.active ? theme.accent : theme.textMuted + font.pixelSize: 12 + horizontalAlignment: Text.AlignHCenter + } + + // Duration (:116-123): NO fixed width — it sizes to its content, + // so the middle column's width moves with the string. + Text { + id: dur + anchors.right: parent.right + anchors.rightMargin: 12 + anchors.verticalCenter: parent.verticalCenter + text: row.modelData.duration !== undefined ? row.modelData.duration : "" + color: theme.textMuted + font.pixelSize: 11 + } + + // The middle column (:88-113). The reference's + // `VerticalLayout { horizontal-stretch: 1; alignment: center }`: + // the stretch is the anchor pair below, and `alignment: center` — + // which makes the two texts a vertically-centred block of their + // natural heights rather than a stretch across the 36 px content + // box — is anchors.verticalCenter on a Column whose height is its + // implicit one. It is also what keeps a row with no artist + // optically centred instead of top-aligned. + // + // The 9 px gaps on both sides are the content layout's `spacing`. + Column { + id: mid + anchors.left: num.right + anchors.leftMargin: 9 + anchors.right: dur.left + anchors.rightMargin: 9 + anchors.verticalCenter: parent.verticalCenter + spacing: 2 + + // Title + badge row (:92-106), gap 5. Same shape as the two + // display surfaces: the title's box shrinks by the badge, and + // the badge is CENTRED on the title line (the reference wraps + // it in a `VerticalLayout { alignment: center }`), not + // baseline-aligned. + Item { + width: mid.width + height: title.implicitHeight + + Text { + id: title + anchors.left: parent.left + // badge.width is the constant 13, so this is not a loop. + width: parent.width - (badge.visible ? badge.width + 5 : 0) + text: row.modelData.title !== undefined ? row.modelData.title : "" + color: row.active ? theme.textPrimary : theme.textSecondary + font.pixelSize: 12 + font.weight: Font.DemiBold + elide: Text.ElideRight + } + + MiniExplicitBadge { + id: badge + visible: row.modelData.explicit === true + anchors.left: title.right + anchors.leftMargin: 5 + anchors.verticalCenter: title.verticalCenter + } + } + + // CONDITIONAL (:107-112) — an empty artist makes this a + // ONE-line row, and in a Column an invisible child takes its + // spacing with it, which is exactly Slint's `if artist != ""`. + Text { + width: mid.width + visible: row.modelData.artist !== undefined + && row.modelData.artist !== "" + text: row.modelData.artist !== undefined ? row.modelData.artist : "" + color: theme.textMuted + font.pixelSize: 10 + elide: Text.ElideRight + } + } + } + } + + // The scrollbar (:129-137): a SIBLING of the list overlaid on the rows' + // right edge, not a gutter carved out of them. The Slint header claims its + // bar "sits in its own gutter" while this surface overlays it on + // full-width rows; the overlay is reproduced 1:1 (§12-P3) — it auto-hides, + // and reserving 14 px would move every row's duration column. + // + // `target` is REQUIRED (§15 trap 36) and `visible` is NOT assigned here: + // the component owns that binding, and overriding it gives a short list a + // full-height thumb on gutter hover (`PlaylistManagerView.qml:221-224`). + QbzScrollBar { + target: list + anchors.right: parent.right + anchors.top: parent.top + anchors.bottom: parent.bottom + } +} diff --git a/crates/qbz-qt/qml/miniplayer/MiniShell.qml b/crates/qbz-qt/qml/miniplayer/MiniShell.qml index 5fa253b9f..9f8b23783 100644 --- a/crates/qbz-qt/qml/miniplayer/MiniShell.qml +++ b/crates/qbz-qt/qml/miniplayer/MiniShell.qml @@ -213,8 +213,12 @@ Rectangle { // The router: one gated Loader per surface, each with its own INLINE // component (§8 rule 4 — not one Loader with a `source` string, whose - // item is an untyped QObject). MiniQueueSurface (3) and - // MiniLyricsSurface (4) join it in B4. + // item is an untyped QObject). + // + // Only the two DISPLAY surfaces take npExplicit. The queue rows carry + // their own `explicit` flag per row (mini_qt.rs's MiniQueueRow) and the + // lyrics surface renders no badge at all, so handing either of them the + // now-playing flag would be a property nothing reads. Loader { anchors.fill: parent active: root.surfaceId === 1 @@ -229,6 +233,30 @@ Rectangle { MiniArtworkSurface { npExplicit: root.npExplicit } } } + // Both of these carry `QbzMini.open` in their gate, and that term is + // not decoration. The mini window is created ONCE and never destroyed + // (Main.qml only hides it), so a Loader gated on `surfaceId` ALONE + // keeps its surface mounted after the user leaves the mini — and both + // of these surfaces re-parse a whole JSON document on every publish. + // A user who opened the mini once on the queue surface would then pay + // a full parse of a 2,000-track document on the GUI thread on every + // enqueue and every skip, for the rest of the session, with the mini + // nowhere on screen. §7-M1's hazard is stated in exactly those words: + // work that continues "including while the mini is closed". + Loader { + anchors.fill: parent + active: QbzMini.open && root.surfaceId === 3 + sourceComponent: Component { + MiniQueueSurface { } + } + } + Loader { + anchors.fill: parent + active: QbzMini.open && root.surfaceId === 4 + sourceComponent: Component { + MiniLyricsSurface { } + } + } } // --- The footer (MiniShell.slint:59-63) -------------------------------- diff --git a/crates/qbz-qt/src/mini_bridge.rs b/crates/qbz-qt/src/mini_bridge.rs index a8dc00417..78608aecc 100644 --- a/crates/qbz-qt/src/mini_bridge.rs +++ b/crates/qbz-qt/src/mini_bridge.rs @@ -124,6 +124,17 @@ pub mod qbz_mini { #[qinvokable] fn save_geometry(self: Pin<&mut QbzMini>, width: f32, height: f32); + /// Activate a queue-surface row (contract A-13, §4.4.3). `row` indexes + /// `mini_queue_json.rows` — the DISPLAY list, whose first entry is the + /// current track only when there IS one. + /// + /// Remote-first, and `Err` RETURNS (§15 trap 9); row 0 resumes ONLY + /// when the document carries a current track (the off-by-one the + /// reference has and this port fixes); everything else is an index + /// into the UNFILTERED upcoming list (§15 trap 10). + #[qinvokable] + fn queue_play(self: Pin<&mut QbzMini>, row: i32); + /// Close the whole APP from the mini (contract A-12, §4.6). Exits the /// mini first, then routes to the ONE close choreography by emitting /// `close_app_requested` below. @@ -438,6 +449,94 @@ impl qbz_mini::QbzMini { crate::mini_qt::save_geometry(w, h); } + /// Contract A-13 — a queue-surface row was clicked. + /// + /// Both numbers this needs come out of the document THIS object already + /// holds: no second `get_queue_state_full()`, no second poll (§7-M1). The + /// resolution is deliberately synchronous and on the Qt thread — it is a + /// JSON read of a string the bridge owns, and the reference resolves its + /// own row on the UI thread for the same reason (`miniplayer.rs:345-363`). + /// + /// THE ORDER, which is the whole of §4.4.3 and §8.1's third row: + /// + /// 1. `currentId != ""` decides whether row 0 IS the current track. The + /// reference assumes it always is (`miniplayer.rs:379-388`) while its + /// own model pushes that row only `if let Some(t)` — so with no current + /// track, clicking row 0 merely toggles play there. `queue_row_target` + /// is the fix and UT-2 is its lock. + /// 2. The row's id is resolved and parsed BEFORE anything is dispatched, + /// because the remote ladder needs the track id and a bad row must cost + /// nothing. A non-numeric id returns silently, 1:1 with `:361-363`. + /// 3. The subtraction (`row - 1`) happens AFTER the current-track test, + /// never before. + /// + /// Then, on the runtime: the remote-first ladder, in its three-arm form. + /// `Ok(true)` the peer took it; `Ok(false)` fall through to local; **`Err` + /// logs and RETURNS** — an error means the peer's state is unknown and + /// starting local audio would double-play (§15 trap 9). The idiom is + /// `queue_qt::reorder_upcoming`'s (`src/queue_qt.rs:679-687`). + pub fn queue_play(self: Pin<&mut Self>, row: i32) { + if row < 0 { + return; + } + let doc: serde_json::Value = match serde_json::from_str(&self.mini_queue_json.to_string()) { + Ok(doc) => doc, + Err(e) => { + log::warn!("[qbz-qt] mini: queue document unreadable: {e}"); + return; + } + }; + let has_current = doc + .get("currentId") + .and_then(|v| v.as_str()) + .is_some_and(|id| !id.is_empty()); + // Bounds check on the DISPLAY list, exactly where the reference makes + // it (`miniplayer.rs:352-358` checks against `row_count()`). + let Some(id) = doc + .get("rows") + .and_then(|rows| rows.get(row as usize)) + .and_then(|r| r.get("id")) + .and_then(|v| v.as_str()) + else { + log::warn!("[qbz-qt] mini: queue row {row} out of range"); + return; + }; + let Ok(track_id) = id.parse::() else { + // Silent, 1:1 with the reference: a non-numeric id has no local + // fallback to take (`miniplayer.rs:361-363`). + return; + }; + let target = crate::mini_qt::queue_row_target(has_current, row); + let runtime = crate::app(); + crate::spawn(async move { + if let Some(svc) = crate::qconnect_qt::service() { + match svc.play_remote_renderer_track_if_active(track_id).await { + Ok(true) => return, // handled by the peer + Ok(false) => {} // not applicable -> local path + Err(e) => { + log::warn!("[qbz-qt] mini: queue play remote handoff: {e}"); + return; // connected but errored -> do NOT play locally + } + } + } + match target { + // "resume if paused (no restart)" — never a seek, never a + // re-open of the same stream. Clicking the row that is already + // playing is a no-op. + crate::mini_qt::MiniRowTarget::Resume => { + if !crate::now_playing::playing() { + crate::transport_toggle_play(); + } + } + // The UNFILTERED upcoming domain — `play_upcoming_flat`, never + // the page-local `play_upcoming` (§15 trap 10 / trap 23). + crate::mini_qt::MiniRowTarget::Upcoming(index) => { + crate::queue_qt::play_upcoming_flat(&runtime, index.max(0) as usize).await; + } + } + }); + } + /// Contract A-12 — close the APP from the mini's red X. /// /// The order is the reference's (`crates/qbz/src/miniplayer.rs:235-251`): @@ -474,20 +573,16 @@ impl qbz_mini::QbzMini { // `QbzMini.surface = n` uses, so the keyboard path cannot skip the // persist or the surface-3 queue kick. // - // `clamp_to_implemented` is the TEMPORARY guard that keeps a key from - // landing the user on a blank card while its surface does not exist - // yet. **B3 has taken its own entry out**: the micro card IS the footer - // this block landed, so `mini.micro` writes 0 unclamped. Only `4` and - // `5` are still clamped, and B4 deletes the function with them — see - // the function's own doc. The keys stay BOUND rather than being removed - // from the table, because the table is the user's rebindable one and - // the cheatsheet already lists all five. + // All five arms are UNCLAMPED. The `clamp_to_implemented` guard that + // used to wrap `3` and `4` existed only while their surfaces were + // unwritten; B4 landed both and deleted it, so a key now lands exactly + // where the user's binding says. match action.id { "mini.micro" => apply_surface(self, 0), "mini.compact" => apply_surface(self, 1), "mini.artwork" => apply_surface(self, 2), - "mini.queue" => apply_surface(self, crate::mini_qt::clamp_to_implemented(3)), - "mini.lyrics" => apply_surface(self, crate::mini_qt::clamp_to_implemented(4)), + "mini.queue" => apply_surface(self, 3), + "mini.lyrics" => apply_surface(self, 4), // Both leave the mini — 1:1 with `keybindings.rs:641`, where the // two ids share one arm. "ui.miniPlayer" | "ui.escape" => exit_now(self), diff --git a/crates/qbz-qt/src/mini_qt.rs b/crates/qbz-qt/src/mini_qt.rs index 3927cd4ba..be7688951 100644 --- a/crates/qbz-qt/src/mini_qt.rs +++ b/crates/qbz-qt/src/mini_qt.rs @@ -138,9 +138,9 @@ pub(crate) fn mini_queue_doc(state: &QueueState) -> String { /// What a mini queue row activation resolves to, once the remote-first ladder /// has declined it. Kept separate from the invokable so the mapping is /// testable without a runtime (UT-2 is the lock on the §4.4.3 fix). -// Consumed by `QbzMini.queue_play` (contract row A-13), which lands with the -// queue surface rather than with this projection. -#[allow(dead_code)] +/// +/// Consumed by `QbzMini.queue_play` (contract row A-13), which landed with the +/// queue surface in block B4. #[derive(Debug, PartialEq, Eq)] pub(crate) enum MiniRowTarget { /// Row 0 WITH a current track: resume if paused, never restart @@ -160,8 +160,8 @@ pub(crate) enum MiniRowTarget { /// unconditionally — so with no current track, clicking row 0 (which IS /// `upcoming[0]`) merely toggles play. Here the shift is driven by whether the /// document actually carries a current row. -// Same consumer as `MiniRowTarget` (contract row A-13). -#[allow(dead_code)] +/// +/// Same consumer as `MiniRowTarget` (contract row A-13). pub(crate) fn queue_row_target(has_current: bool, row: i32) -> MiniRowTarget { if has_current { if row == 0 { @@ -219,39 +219,21 @@ fn surface_for_default_view(default_view: &str, last_surface: i32) -> i32 { } } +/// The opening surface, UNCLAMPED — every one of the five renders. +/// +/// This used to run through a `clamp_to_implemented` guard, because +/// `mini_surface` and `mini_default_view` live in the ui_prefs.json co-owned +/// with the shipping Slint app: a user whose Slint miniplayer was last on +/// Queue would have opened the Qt mini onto a blank card. B3 landed the micro +/// footer and B4 the queue and lyrics surfaces, so the clamp had no entries +/// left and went with them — as its own doc required of the last block to +/// remove one. **Do not reintroduce it**: a fallback here would now silently +/// override the user's persisted surface. pub(crate) fn initial_surface() -> i32 { - clamp_to_implemented(surface_for_default_view( + surface_for_default_view( &crate::settings_qt::pref_str(KEY_DEFAULT_VIEW, "remember"), crate::settings_qt::pref_i32(KEY_SURFACE, DEFAULT_SURFACE), - )) -} - -/// TEMPORARY, and it deletes itself: surfaces 0 (micro), 3 (queue) and 4 -/// (lyrics) have no content yet — the footer that IS the micro card is block -/// B3's, and the queue and lyrics surfaces are B4's. -/// -/// This matters because `mini_surface` and `mini_default_view` live in the -/// ui_prefs.json **co-owned with the shipping Slint app**. A user whose Slint -/// miniplayer was last on Queue, or whose default view is "micro", would open -/// the Qt mini onto a blank rounded box with no footer and no capsule — i.e. -/// no visible way out but Escape. Falling back to the artwork surface (the -/// reference's own default, `ui_prefs.rs`'s `mini_surface: 2`) is the honest -/// behaviour until the surfaces exist. -/// -/// **Removal is part of B3 and B4**, in the same commit that lands each -/// surface: B3 drops 0 from the list, B4 drops 3 and 4, and the last one to go -/// deletes this function. The keyboard arms in `mini_bridge::key_pressed` are -/// clamped by the same rule and lose it the same way. -/// -/// **B3 has dropped 0**: the micro card IS `MiniFooter.qml` in mode 2, so -/// surface 0 now renders and its keyboard arm (`mini.micro`) is unclamped. -/// Only 3 (queue) and 4 (lyrics) remain, and B4 deletes this function with -/// them. -pub(crate) fn clamp_to_implemented(surface: i32) -> i32 { - match surface { - 0 | 1 | 2 => surface, - _ => DEFAULT_SURFACE, - } + ) } pub(crate) fn initial_background_blur() -> bool { @@ -423,21 +405,6 @@ mod tests { assert_eq!(geometry_to_persist(4, 500.0, 700.0), Some((500.0, 700.0))); } - /// The temporary clamp's own boundary, asserted so the block that removes - /// an entry cannot forget the arm it owns. B3 landed the micro footer, so - /// 0 passes; 3 and 4 still fall back to the artwork surface until B4. - #[test] - fn clamp_admits_every_implemented_surface() { - assert_eq!(clamp_to_implemented(0), 0); - assert_eq!(clamp_to_implemented(1), 1); - assert_eq!(clamp_to_implemented(2), 2); - assert_eq!(clamp_to_implemented(3), DEFAULT_SURFACE); - assert_eq!(clamp_to_implemented(4), DEFAULT_SURFACE); - // Out-of-range values from a co-owned prefs document, too. - assert_eq!(clamp_to_implemented(-1), DEFAULT_SURFACE); - assert_eq!(clamp_to_implemented(9), DEFAULT_SURFACE); - } - #[test] fn default_view_pref_maps_to_the_slint_surface_table() { // miniplayer.rs:110-119 — "remember"/unknown fall back to the last diff --git a/crates/qbz-qt/src/now_playing.rs b/crates/qbz-qt/src/now_playing.rs index 468f9ada6..f6e4fefa9 100644 --- a/crates/qbz-qt/src/now_playing.rs +++ b/crates/qbz-qt/src/now_playing.rs @@ -251,6 +251,21 @@ pub(crate) fn seed_meta() -> (String, String) { with_model(|m| (m.artist_id.clone(), m.title.clone())).0 } +/// The transport flag as PUBLISHED — `QbzPlayer.npPlaying`'s own source, and +/// therefore the exact Qt twin of the Slint `NowPlayingState.playing` the +/// miniplayer's row-0 arm reads (`crates/qbz/src/miniplayer.rs:381-385`: +/// *"Index 0 = the current track -> resume if paused (no restart)"*). +/// +/// Read here rather than off `player().state.is_playing()` on purpose: this +/// model is fed by the cast publisher (`cast_qt.rs:1173`) and the QConnect +/// mirror as well as by the local poll (`set_position` above), so it stays +/// true while the deck is a cast device and the local engine is idle. The +/// engine-level read would report "paused" there and turn a row-0 click into +/// a PAUSE of the cast session. +pub(crate) fn playing() -> bool { + with_model(|m| m.playing).0 +} + pub fn set_playing(playing: bool) { mutate(|m| m.playing = playing); }