diff --git a/crates/qbz-qt/build.rs b/crates/qbz-qt/build.rs index 07befd393..3ed180917 100644 --- a/crates/qbz-qt/build.rs +++ b/crates/qbz-qt/build.rs @@ -144,6 +144,7 @@ fn main() { "qml/theme/QbzTheme.qml", "qml/theme/RoundedImage.qml", "qml/views/AlbumView.qml", + "qml/views/ArtistReleasesView.qml", "qml/views/ArtistView.qml", "qml/views/BlacklistManagerView.qml", "qml/views/HomeView.qml", diff --git a/crates/qbz-qt/qml/controls/QbzSectionHeader.qml b/crates/qbz-qt/qml/controls/QbzSectionHeader.qml index f3a09f52d..f580a18a7 100644 --- a/crates/qbz-qt/qml/controls/QbzSectionHeader.qml +++ b/crates/qbz-qt/qml/controls/QbzSectionHeader.qml @@ -11,9 +11,11 @@ // pageLeft()/pageRight() (the host owns the page-step math); and, since the // Playlist Manager landed, `collapsible` + `collapsed` + toggled() and // `titleSecondary`. -// Deliberately NOT absorbed: ArtistView.ReleaseSection's header (adds a -// sort dropdown + "See discography" — distinct arms, still stubs) and the -// uppercase 10-11px eyebrow mini-headers (different type scale). +// Deliberately NOT absorbed: ArtistView.ReleaseSection's header (adds a sort +// dropdown and a "See discography" link — two arms this header does not have, +// and both are now LIVE controls rather than the stubs this note used to +// describe) and the uppercase 10-11px eyebrow mini-headers (different type +// scale). // // ── THE `id: root` REFACTOR CAME FIRST, AND IT WAS NOT COSMETIC ─────────── // This file used to have no root id: its Text read `text: parent.title` and diff --git a/crates/qbz-qt/qml/shell/AppShell.qml b/crates/qbz-qt/qml/shell/AppShell.qml index 97e2ea65b..ab0677310 100644 --- a/crates/qbz-qt/qml/shell/AppShell.qml +++ b/crates/qbz-qt/qml/shell/AppShell.qml @@ -180,6 +180,14 @@ Rectangle { : QbzShell.currentView === "mostplayedalbums" ? "../views/PlayHistoryView.qml" : QbzShell.currentView === "label" ? "../views/LabelView.qml" : QbzShell.currentView === "labelreleases" ? "../views/LabelReleasesView.qml" + // Artist page > a release section > "See discography", and the + // album page's "From the same artist" View all. Same two-file + // contract as the rest of this chain (nav_qt.rs:9-16): + // artist_releases_qt::open records the id, this arm mounts it. + // Forgetting the arm is NOT a crash — the ternary falls through + // to "" and the pane goes blank with nothing logged, i.e. the + // dead "See discography" in a new costume. + : QbzShell.currentView === "artistreleases" ? "../views/ArtistReleasesView.qml" // For You > Qobuz Mixes. The whole chain existed — // HomeView's tile -> QbzHome.openMix -> foryou_qt -> nav_qt // records the "mix" view — and MixView.qml is written and diff --git a/crates/qbz-qt/qml/views/AlbumView.qml b/crates/qbz-qt/qml/views/AlbumView.qml index 81fa38691..cce11dfc9 100644 --- a/crates/qbz-qt/qml/views/AlbumView.qml +++ b/crates/qbz-qt/qml/views/AlbumView.qml @@ -1059,30 +1059,57 @@ Rectangle { // (album_qt.rs spawn_deferred_rows). While a fetch is out the // rail's placeholder holds its band; when it resolves to nothing // both the placeholder and the rail are gone — no empty frame. + // + // WIDTH — every rail below spells `parent.width - 64` on purpose. + // `page` is a Column with leftPadding/rightPadding 32, and a QML + // Column's padding does NOT narrow `parent.width` for its + // children: it only shifts them. A child that says + // `width: parent.width` therefore comes out 64px too wide and + // hangs 32px off the right edge — which put SectionRail's + // right-aligned chevron Row entirely outside the viewport (owner + // report: the bottom rails had no right chevron at all, and the + // left one sat jammed against the window edge). The inset is + // applied at the MOUNT SITES, not inside SectionRail / + // RailSkeleton, exactly as the page's other direct children + // already do it — see :442, :480, :690 and :695. Item { visible: moreRail.visible || moreRailSk.visible; width: 1; height: 40 } RailSkeleton { id: moreRailSk + width: parent.width - 64 visible: root.moreLoading phase: skeletonPhase.on cardCount: root.railSkeletonCount } SectionRail { id: moreRail + width: parent.width - 64 visible: (album.moreFromArtist || []).length > 0 title: QbzSession.tr("From the same artist", QbzSession.trRev) items: album.moreFromArtist || [] coverMap: root.coverMap + // album/AlbumPageView.slint:1124-1132 — this rail is the ONE + // carousel on the album page with `show-view-all: true`, and + // its link is the SECOND door to the artist discography page + // (main.rs:11839-11865 handles it with release_type "album" and + // the name taken from AlbumState.artist, which is why + // openReleases takes the name as a parameter). The other door + // is "See discography" on the artist page itself. + showViewAll: true + onViewAllClicked: QbzArtist.openReleases(root.header.artistId || "", + root.header.artist || "", "album") } Item { visible: sugRail.visible || sugRailSk.visible; width: 1; height: 40 } RailSkeleton { id: sugRailSk + width: parent.width - 64 visible: root.suggestionsLoading phase: skeletonPhase.on cardCount: root.railSkeletonCount } SectionRail { id: sugRail + width: parent.width - 64 visible: (album.suggestions || []).length > 0 title: QbzSession.tr("Listening suggestions", QbzSession.trRev) items: album.suggestions || [] @@ -1096,12 +1123,14 @@ Rectangle { Item { visible: simRail.visible || simRailSk.visible; width: 1; height: 40 } RailSkeleton { id: simRailSk + width: parent.width - 64 visible: root.similarLoading phase: skeletonPhase.on cardCount: root.railSkeletonCount } SectionRail { id: simRail + width: parent.width - 64 visible: (album.similarAlbums || []).length > 0 title: QbzSession.tr("Similar albums", QbzSession.trRev) items: album.similarAlbums || [] diff --git a/crates/qbz-qt/qml/views/ArtistReleasesView.qml b/crates/qbz-qt/qml/views/ArtistReleasesView.qml new file mode 100644 index 000000000..b873d4078 --- /dev/null +++ b/crates/qbz-qt/qml/views/ArtistReleasesView.qml @@ -0,0 +1,340 @@ +// Artist discography page — QML port of artist/ArtistReleasesView.slint (257 +// lines, read in full for this port): the full, paginated grid for ONE release +// bucket of one artist, reached from the artist page's "See discography" link +// and from the album page's "From the same artist" View all. +// +// (Before this file existed, "See discography" was a fully lit, pointer-cursor +// Text with an EMPTY MouseArea under a POC-NOTE saying the page was out of +// scope. That note and that emptiness are both gone.) +// +// Layout, read off the .slint: +// :36-103 fixed 56px header — artist kicker + bucket title LEFT, the sort +// select RIGHT, both vertically centred on y=25 +// :131-138 body Column padding 32 / 32, top 8, bottom 100, spacing 0 +// :121-129 infinite scroll: within 600px of the bottom, has-more, and +// neither a first page nor a tail page already in flight +// :140-203 THREE mutually exclusive body states — loading, error+Retry, +// empty — then the grid, then the tail spinner +// :205-231 200x266 cards, gap 24, view-mode hard-wired "grid" +// :242-256 scrollbar below the header +// +// WHAT THIS PAGE DELIBERATELY DOES NOT HAVE (checked against the .slint, not +// against its cousins): no search field, no group-by, no Hi-Res filter, no +// grid/list toggle, and no "Load more" button. LabelReleasesView has all five; +// this is a sibling of DiscoverBrowseView, not of that page. One sort select is +// the entire toolbar. +// +// NAV BUTTONS: the .slint draws a NavButtons pair at x=32 and starts its text +// stack 16px after it. In this port back/forward live in the shell header +// (HeaderBar.qml:251-266), so the stack starts at x=48 — the 32 + 0 + 16 that +// DiscoverBrowseView.qml:14-18 already establishes as the convention. The back +// affordance is present, it is just drawn once for the whole app. +// +// OFFLINE: shell/AppShell.slint:503-511 mounts the whole view under +// `!OfflineState.offline`, so it simply does not exist offline. This port has +// no per-route unmount, so the page honours the same gate with the seam it does +// have — QbzOfflinePlaceholder over a hidden body — and `artist_releases_qt:: +// open` refuses to navigate here offline in the first place. +// +// POC-NOTEs (pre-existing and port-wide, NOT deferrals introduced here): +// - no scroll restore. The .slint restores NavState.scroll-restore under the +// "artist-releases" scope (:110-119); this port has no counterpart to +// NavState.report-scroll / restore-scope on ANY page (DiscoverBrowseView.qml +// :20-21 records the same gap). Half a mechanism would be worse than none. +// - back/forward re-mounts this view against whatever document is still on +// QbzArtist.artistReleasesJson rather than re-fetching, because the port's +// nav stack stores no payload (nav_qt.rs:9-23). That is exactly how +// `labelreleases` already behaves; the Rust generation guard is what keeps a +// late page from painting into the wrong bucket. + +import QtQuick +import QtQuick.Window +import com.blitzfc.qbz +import "../controls" +import "../theme" + +Rectangle { + id: root + + color: ambientOn ? "transparent" : theme.surfaceMain + readonly property bool ambientOn: QbzShell.ambientMode > 0 && QbzPlayer.npHasTrack + radius: 12 + + QbzTheme { id: theme } + + // ONE document, re-parsed on every publish — a fresh string is a fresh + // object, so the Repeaters below always see a NEW reference ("Rebind + // requires a NEW object reference", cards/PlaylistCollage.qml). Nothing in + // this file ever mutates `doc` in place. + readonly property var doc: { + try { + return JSON.parse(QbzArtist.artistReleasesJson) + } catch (e) { + return {} + } + } + readonly property var albums: doc.albums || [] + + // .slint:85-99 — the five options in the reference's order, and the index + // <-> wire-key round trip. "A–Z"/"Z–A" carry a U+2013 EN DASH: the msgid in + // the catalogue is the en-dash form, and a hyphen would go untranslated in + // all seven non-English locales. (LibraryToolbar's "Title A-Z" is a + // DIFFERENT, hyphenated msgid — do not copy it here.) + readonly property var sortLabels: [ + QbzSession.tr("Default", QbzSession.trRev), + QbzSession.tr("Newest", QbzSession.trRev), + QbzSession.tr("Oldest", QbzSession.trRev), + QbzSession.tr("A–Z", QbzSession.trRev), + QbzSession.tr("Z–A", QbzSession.trRev), + ] + // These five strings are BOTH what gets persisted (artist_prefs.rs, the + // store the artist page's per-section picker shares) and what the sort + // functions switch on, on either side of the bridge. + readonly property var sortKeys: ["default", "newest", "oldest", "title-asc", "title-desc"] + // .slint:24-28. A BINDING, never an assignment: QbzSelect does not + // self-assign currentIndex (QbzSelect.qml:299-300), and this is what seats + // the picker on the sort the page opened with — which is the user's + // persisted choice for this bucket, not "Default". + readonly property int sortIndex: Math.max(0, root.sortKeys.indexOf(root.doc.sortBy || "default")) + + // ============================ offline gate ============================ + QbzOfflinePlaceholder { + visible: QbzSession.offline + anchors.centerIn: parent + showSettingsAction: true + onSettingsClicked: QbzShell.navigateTo("settings") + } + + Column { + anchors.fill: parent + spacing: 0 + visible: !QbzSession.offline + + // --- Fixed 56px header ------------------------------------------- + Item { + id: header + width: parent.width + height: 56 + + Rectangle { + anchors.fill: parent + color: root.ambientOn ? theme.surfaceMainA30 : theme.surfaceMain + } + + // .slint:47-70 — a COLUMN, spacing 2. (A Row would top-align its + // children; this stack is vertical precisely so the kicker sits on + // the title.) `y: 25 - height/2` is the header's centring + // convention — 25, not 28, and it is the .slint's own value at :43. + Column { + x: 48 + y: 25 - height / 2 + spacing: 2 + + // The artist name. SERVER TEXT — never translated (.slint:54 + // says so in as many words). + Text { + text: root.doc.name || "" + color: theme.textMuted + font.pixelSize: 11 + font.weight: theme.weightSemibold + font.letterSpacing: 0.6 + elide: Text.ElideRight + width: Math.max(0, sortSel.x - 48 - 16) + } + // The bucket title, translated in RUST + // (artist_qt::release_type_title) because it travels inside the + // JSON document — hence artist_releases_qt::republish on a + // language change. + // + // The width cap is not decoration: "Compilations" in German + // against a narrow window would otherwise run under the sort + // select, which draws after it and therefore on top of it. + Text { + text: root.doc.title || "" + color: theme.textPrimary + font.pixelSize: theme.fontSection + font.weight: theme.weightBold + elide: Text.ElideRight + width: Math.max(0, sortSel.x - 48 - 16) + } + } + + // .slint:74-102. `sm: true` + `menuWidth: 150` are the reference's + // own values verbatim: QbzSelect.qml:34 is `width = menuWidth - 40` + // under `sm`, so 150 lands at the 110 x 30 the reference computes + // (QbzSelect.slint:88). Passing 190 "to get 150" is the exact + // mistake that file's :42-50 note documents. + QbzSelect { + id: sortSel + x: parent.width - width - 32 + y: 25 - height / 2 + sm: true + menuWidth: 150 + options: root.sortLabels + currentIndex: root.sortIndex + onSelected: function (i) { + QbzArtist.releasesSetSort(root.sortKeys[i] || "default") + } + } + } + + // --- Scrolling grid ------------------------------------------------ + Item { + width: parent.width + height: parent.height - 56 + + Flickable { + id: flick + anchors.fill: parent + // A rectangular scissor — it ignores `radius`. The root's own + // radius-12 fill is what rounds this pane to the AppShell bezel. + clip: true + contentWidth: width + contentHeight: page.implicitHeight + boundsBehavior: Flickable.StopAtBounds + + // Infinite scroll, guarded exactly as the .slint is (:123-126). + // There is NO "Load more" button on this page. + onContentYChanged: { + if (contentY + height >= contentHeight - 600 + && root.doc.hasMore === true + && root.doc.loadMoreLoading !== true + && root.doc.loading !== true) { + // Snapshot the ids already on screen so ONLY the page + // that lands fades in. BEFORE the bridge call, which + // may republish synchronously + // (LabelReleasesView.qml:327's rule). + collection.armTailFade() + QbzArtist.releasesLoadMore() + } + } + + Column { + id: page + width: parent.width + leftPadding: 32 + rightPadding: 32 + topPadding: 8 + bottomPadding: 100 + spacing: 0 + + // --- State 1: first page in flight (.slint:140-156) ---- + Column { + visible: root.doc.loading === true + anchors.horizontalCenter: parent.horizontalCenter + spacing: 12 + + QbzSpinner { + size: 36 + anchors.horizontalCenter: parent.horizontalCenter + } + Text { + anchors.horizontalCenter: parent.horizontalCenter + text: QbzSession.tr("Loading…", QbzSession.trRev) + color: theme.textMuted + font.pixelSize: 14 + horizontalAlignment: Text.AlignHCenter + } + } + + // --- State 2: the first page failed (.slint:158-193) --- + // Only the FIRST page raises this; a failed tail page keeps + // what is on screen and just stops the tail spinner + // (main.rs:15129 vs :3599). + Column { + visible: root.doc.loading !== true && root.doc.loadError === true + anchors.horizontalCenter: parent.horizontalCenter + spacing: 12 + + Text { + anchors.horizontalCenter: parent.horizontalCenter + text: QbzSession.tr("Failed to load Library", QbzSession.trRev) + color: theme.textSecondary + font.pixelSize: 14 + horizontalAlignment: Text.AlignHCenter + } + // .slint:169-191 — 34 tall, radius 6, 16px horizontal + // padding, elevated -> hover fill. + Rectangle { + anchors.horizontalCenter: parent.horizontalCenter + width: retryLabel.implicitWidth + 32 + height: 34 + radius: 6 + color: retryArea.containsMouse ? theme.surfaceHover : theme.surfaceElevated + + Text { + id: retryLabel + anchors.centerIn: parent + text: QbzSession.tr("Retry", QbzSession.trRev) + color: theme.textPrimary + font.pixelSize: 13 + font.weight: theme.weightMedium + } + MouseArea { + id: retryArea + anchors.fill: parent + hoverEnabled: true + cursorShape: Qt.PointingHandCursor + onClicked: QbzArtist.releasesRetry() + } + } + } + + // --- State 3: loaded, and the bucket is empty ---------- + // (.slint:195-203. A sibling of the grid, not an arm of it.) + Text { + visible: root.doc.loading !== true + && root.doc.loadError !== true + && root.albums.length === 0 + anchors.horizontalCenter: parent.horizontalCenter + text: QbzSession.tr("No releases found.", QbzSession.trRev) + color: theme.textMuted + font.pixelSize: 14 + } + + // --- The grid (.slint:205-231) ------------------------- + // `viewMode: "grid"` is HARD-WIRED, as in the reference + // (:207): this page has no toggle. + AlbumCollection { + id: collection + visible: root.doc.loading !== true && root.doc.loadError !== true + width: parent.width - 64 + // Identity of the catalog on screen. Opening ANOTHER + // bucket (or another artist) disarms a tail fade that + // was armed by a scroll whose page never landed, so the + // new grid's first paint is never coloured by it. + collectionKey: (root.doc.id || "") + ":" + (root.doc.releaseType || "") + albums: root.albums + viewMode: "grid" + cardWidth: 200 + cardHeight: 266 + cardGap: 24 + flick: flick + // The page Column's padding-top. The loading / error + // branches are mutually exclusive with the grid, so + // nothing else sits above it (.slint:213-222). + contentOffset: 8 + } + + // --- Tail spinner (.slint:233-237) --------------------- + Item { visible: root.doc.loadMoreLoading === true; width: 1; height: 24 } + QbzSpinner { + visible: root.doc.loadMoreLoading === true + size: 28 + anchors.horizontalCenter: parent.horizontalCenter + } + } + } + + // Tracks only the scrolling grid — the header is a sibling above, + // which is the .slint's `y: 56px` / `height: parent.height - 56px`. + QbzScrollBar { + anchors.right: parent.right + anchors.rightMargin: 4 + anchors.top: parent.top + anchors.bottom: parent.bottom + target: flick + } + } + } +} diff --git a/crates/qbz-qt/qml/views/ArtistView.qml b/crates/qbz-qt/qml/views/ArtistView.qml index 1ecd89f0d..e21a98479 100644 --- a/crates/qbz-qt/qml/views/ArtistView.qml +++ b/crates/qbz-qt/qml/views/ArtistView.qml @@ -16,10 +16,15 @@ // MusicBrainz is off in Settings or the artist has no confident MB match, is // simply ABSENT — never an error frame, and nothing is requested. // -// POC-NOTEs: blacklist banner, artist Scene, Share, Create Collection, radio +// POC-NOTEs: blacklist banner, artist Scene, Create Collection, radio // engines (dropdown inert), multi-select, the sticky behavior of the JUMP TO // bar (it scrolls with the page). // +// "Share" left this list when the seam landed: the ⋯ entry now calls +// QbzArtist.share (artist_bridge.rs -> src/share_qt.rs), which copies +// play.qobuz.com/artist/{id} and raises the same "Link copied" toast as +// ArtistPageView.slint:530-538 / main.rs:12749. +// // Header atmosphere (ArtistPageView.slint:120-147, 211-247): wired through // the shared controls/HeaderGradient.qml — the SAME component AlbumView // mounts, because the .slint paints both headers from identical blocks. It @@ -82,22 +87,145 @@ Rectangle { // A section whose albums all filter out DISAPPEARS (artist.rs:1033), and // Load more is suppressed while a filter is active (:1042 — appending // would bring back unfiltered items). + // + // ---- THE PAGED TAIL IS MERGED HERE, NOT PUSHED INTO `artist` ---------- + // Owner, 2026-08-02: "el error esta en que nunca cargan mas albumes." + // The old handler parsed the page and pushed it into + // `artist.releaseSections[i].cards` IN PLACE, then fired artistChanged(). + // Both Repeaters (`model: releaseSections` at :1997/:2114 and + // `model: section.cards` inside ReleaseSection) were then handed the very + // SAME array object they already held, and the unfiltered arm below used + // to `return all` — the identical array again. Nothing was dirty, so + // nothing rebuilt: the page was fetched, parsed and appended to a data + // structure no view ever re-read. This tree already writes the rule down, + // in cards/PlaylistCollage.qml: "Rebind requires a NEW object reference." + // (PRE-EXISTING, not a regression from today's smooth-append work — that + // pass only added the `releaseMoreLanded` call to the handler. What it + // changed is that the button now shows an in-flight placeholder, which is + // what made a silent no-op visible.) + // + // So the appended pages live in `root.releaseOverlay` (see its header) + // and are merged HERE, which builds a NEW section object with a NEW cards + // array whenever the overlay has anything for that bucket — the one thing + // that actually invalidates both Repeaters. `artist` is never mutated, so + // a republish of the document (the progressive Magazine / MusicBrainz + // passes) cannot lose the tail either. + // + // De-dup by album id is MANDATORY on this path and not merely defensive: + // artist_qt.rs `merge_release_page` (:686) folds every landed page into + // the STASHED document too, so the moment the next enrichment pass + // republishes, the document already carries the cards the overlay holds. + // Without the id filter every appended album would appear twice from that + // frame on. (Qobuz can also repeat a row across pages by itself.) readonly property var releaseSections: { var all = artist.releaseSections || [] - if (root.needle === "") return all var out = [] for (var i = 0; i < all.length; i++) { + var src = all[i] + var extra = root.releaseOverlay[src.releaseType] + var cards = src.cards || [] + // The bucket's persisted sort, stamped on the row by + // artist_qt.rs `map_artist` / `resort_section`. It has to be COPIED + // into every rebuilt literal below: an object literal carries only + // the keys it names, so a `sortBy` left out here makes the header's + // QbzSelect snap back to "Default" the moment this arm is taken. + var sortBy = src.sortBy || "default" + // `hasMore` is only overridden once a page has actually LANDED + // (`extra.hasMore` stays undefined otherwise): the document's copy + // is the truth until then, and blanking it would hide the button — + // and with it the in-flight placeholder QbzLoadMore draws under it. + var hasMore = src.hasMore === true + var k + if (extra !== undefined) { + var seen = {} + var merged = [] + for (k = 0; k < cards.length; k++) { + merged.push(cards[k]) + seen[cards[k].id] = true + } + var tail = extra.cards || [] + for (k = 0; k < tail.length; k++) { + if (seen[tail[k].id]) continue + seen[tail[k].id] = true + merged.push(tail[k]) + } + // THE TAIL IS APPENDED, SO IT MUST BE RE-SORTED HERE. + // artist.rs:1245-1258 `append_release_page` re-sorts after + // folding a page in, "before computing the artwork jobs at + // their post-sort positions" — under "A–Z" a page-2 album + // belongs wherever its title puts it, not at the bottom. + // artist_qt.rs `merge_release_page` does that to the STASHED + // document; this does it to the copy actually on screen, which + // is a DIFFERENT array (the overlay never reaches the stash). + // Fixing only one of the two leaves the grid correct until the + // next enrichment republish, or wrong until it — see the + // overlay's own header for why the two copies exist. + cards = root.sortReleaseCards(merged, sortBy) + if (extra.hasMore !== undefined) hasMore = extra.hasMore === true + } + if (root.needle === "") { + // No overlay for this bucket -> hand back the document's own + // object: nothing was appended, so there is nothing to rebind + // and a fresh wrapper would only churn delegates. (The document + // arrives already sorted — artist_qt.rs sorts at page build and + // on every `setSectionSort` — so there is nothing to do to it.) + out.push(extra === undefined ? src + : { "releaseType": src.releaseType, "title": src.title, + "cards": cards, "hasMore": hasMore, "sortBy": sortBy }) + continue + } + // Filtered arm: the merge happened FIRST, so an appended album + // that matches the needle is searchable like any other — but + // `hasMore` is still forced false (artist.rs:1042) and only the + // KEPT cards travel, so the overlay can never leak a filtered-out + // card into this arm. var kept = [] - var cards = all[i].cards || [] for (var j = 0; j < cards.length; j++) if ((cards[j].title || "").toLowerCase().indexOf(root.needle) >= 0) kept.push(cards[j]) if (kept.length === 0) continue - out.push({ "releaseType": all[i].releaseType, "title": all[i].title, - "cards": kept, "hasMore": false }) + out.push({ "releaseType": src.releaseType, "title": src.title, + "cards": kept, "hasMore": false, "sortBy": sortBy }) } return out } + /// JS twin of `artist_qt::sort_release_cards`, itself the port of + /// `album_map::sort_album_items` (crates/qbz/src/album_map.rs:244-264). + /// + /// Both exist because the bucket exists TWICE: Rust owns the stashed + /// document (page 1 plus every page `merge_release_page` folded in) and + /// sorts it there, while `releaseOverlay` holds the pages Load more brought + /// in on THIS side and is concatenated onto the document's array by the + /// projection above — an array Rust never sees. The two must agree, so the + /// arms are kept identical. + /// + /// Returns a NEW array (`slice()` before `sort()`): the projection binds to + /// the result, and sorting the caller's array in place would hand the + /// Repeaters the same reference back — "Rebind requires a NEW object + /// reference" (cards/PlaylistCollage.qml). + /// + /// `year` on these cards is the PLAIN 4-digit year (artist_qt.rs + /// `map_release` slices `dates.original[..4]`), so the string compare is + /// chronological. "default" and every unknown key fall through UNSORTED — + /// `album_map.rs:262` is a bare `_ => {}`, because "Default" means "the + /// order Qobuz sent". + function sortReleaseCards(cards, sort) { + function byTitle(a, b) { + var x = (a.title || "").toLowerCase(), y = (b.title || "").toLowerCase() + return x < y ? -1 : (x > y ? 1 : 0) + } + function byYear(a, b) { + var x = a.year || "", y = b.year || "" + return x < y ? -1 : (x > y ? 1 : 0) + } + switch (sort) { + case "oldest": return cards.slice().sort(byYear) + case "newest": return cards.slice().sort(function (a, b) { return byYear(b, a) }) + case "title-asc": return cards.slice().sort(byTitle) + case "title-desc": return cards.slice().sort(function (a, b) { return byTitle(b, a) }) + default: return cards + } + } readonly property var labels: artist.labels || [] readonly property var similarArtists: artist.similarArtists || [] readonly property var playlists: artist.playlists || [] @@ -150,6 +278,253 @@ Rectangle { property bool appearsOnExpanded: false property bool otherExpanded: false + // ---- Release-section "Load more": in-flight + append-fade state ------- + // controls/QbzLoadMore.qml owns the button and the placeholder row under + // it; the APPEARANCE of what lands is the HOST's job (that file's "WHAT + // THIS CONTROL DOES *NOT* DO" note) because only this view knows the index + // the appended page starts at. + // + // Both maps live on `root`, keyed by releaseType, and NOT on the + // ReleaseSection instances: the Repeater at :1787 re-creates every + // delegate whenever `artist` changes (a `var` property notifies on every + // write, and the document is republished once per enrichment pass — + // stories, then each MusicBrainz section), and one of those republishes is + // the very frame the new page lands in. A flag parked on the section would + // be destroyed exactly when it was needed. + // + // `releasePending` is COPY-ON-WRITE because the ReleaseSection BINDS + // `busy` to it, so it has to notify. `releaseFade` is mutated IN PLACE on + // purpose: it is read once, imperatively, by each card's + // Component.onCompleted, and a notify there would re-run bindings for + // nothing. + property var releasePending: ({}) + /// releaseType -> { from: , + /// at: }. + property var releaseFade: ({}) + + /// ---- The appended pages themselves ----------------------------------- + /// releaseType -> { cards: [], + /// hasMore: , + /// pages: }. + /// + /// The document owns page 1 and stays the source of truth for it; this + /// overlay owns everything Load more brought in, and `releaseSections` + /// (:115) merges the two into a NEW array so the Repeaters rebuild. It is + /// COPY-ON-WRITE for exactly that reason — the projection binds to it, so + /// a write has to notify. + /// + /// Why an overlay and not a push into `artist`: `artist` is + /// `JSON.parse(QbzArtist.artistJson)` (:52), a BINDING. Every progressive + /// republish (Magazine stories, then each MusicBrainz section — see + /// artist_qt.rs) re-runs that parse and yields a brand-new object, so + /// anything written onto the parsed copy is discarded on the next pass. + /// This is the same reasoning `localToggles` (:437) already carries for + /// the heart/pin state. + property var releaseOverlay: ({}) + /// releaseType -> { sent: , + /// next: }. + /// + /// The paging cursor is the count of rows RECEIVED from the server, which + /// is NOT the count on screen: both sides drop duplicate ids (here and in + /// artist_qt.rs `merge_release_page`), so after a page that repeated three + /// rows the on-screen count trails the server cursor by three. Handing the + /// on-screen count back as the offset would re-request rows already held, + /// and the dedup would then swallow them — a Load more that visibly does + /// nothing, which is the very bug this block exists to kill. + /// + /// Mutated IN PLACE on purpose, same as `releaseFade`: it is written from + /// the landing handler and read imperatively from the button's onClicked, + /// and no binding reads it. A notifying write here would tear down and + /// rebuild every AlbumCard in every release grid on each click, for + /// nothing. + property var releaseCursor: ({}) + /// How long after a landing the threshold stays live. The republish that + /// carries the page re-creates the delegates in the same tick, so this is + /// generous; what it buys is that a LATER, unrelated republish (a + /// MusicBrainz pass) re-creates that tail WITHOUT re-fading it. + readonly property int releaseFadeWindowMs: 1500 + + /// Called from the section's Load more BEFORE the bridge call, so the + /// threshold is the count that was on screen at click time. RETURNS the + /// offset the caller must pass to the bridge (see `releaseCursor`). + /// + /// `sorted` = this bucket carries a non-default sort. The append fade is + /// then SUPPRESSED, deliberately: `releaseFadeFrom` hands back an INDEX and + /// the card delegate fades everything at or past it, which only means "the + /// page that just landed" while new rows sit at the TAIL. Under "A–Z" the + /// projection interleaves them (see `sortReleaseCards`), so that band would + /// dissolve an arbitrary run of cards the user has been looking at all + /// along. The reference has no fade at all here, so no fade is strictly + /// closer to it than the wrong one — and tracking the new ids through a + /// re-sort is not a thing the .slint does either. + function releaseMoreRequested(releaseType, loadedCount, sorted) { + if (sorted === true) delete releaseFade[releaseType] + else releaseFade[releaseType] = { "from": loadedCount, "at": 0 } + var c = releaseCursor[releaseType] + // First page of this bucket: nothing has been fetched through the + // paging endpoint yet, so the document's own row count IS the server + // cursor. Afterwards the cursor is authoritative. + var off = (c !== undefined && c.next !== undefined) ? c.next : loadedCount + releaseCursor[releaseType] = { "sent": off, + "next": (c !== undefined ? c.next : undefined) } + var m = Object.assign({}, root.releasePending) + m[releaseType] = true + root.releasePending = m + // artist_bridge.rs emits releaseSectionReady only on SUCCESS + // (main.rs:872 just logs the error and returns), so a failed page would + // otherwise leave the skeleton up forever. Bound it with the same 8s + // QbzLoadMore's own skeleton settles at, after which the button comes + // back armed. + releaseSettle.restart() + return off + } + /// The page landed (or came back empty): fold it into the overlay, advance + /// the cursor, arm the fade window and disarm the placeholder. + /// + /// `cards` is the RAW page as the bridge sent it; `hasMore` is the + /// server's own flag for the bucket. + /// + /// ORDER IS LOAD-BEARING. The overlay write is COPY-ON-WRITE, so it + /// notifies, so `releaseSections` re-evaluates and the Repeater at :1997 + /// destroys and rebuilds every section delegate — QbzLoadMore and every + /// AlbumCard included — SYNCHRONOUSLY, inside that assignment. Everything + /// those rebuilt delegates read has to be true BEFORE it happens: + /// - `releaseFade[rt].at`, or each appended card's Component.onCompleted + /// asks `releaseFadeFrom()` while the landing still reads "in flight" + /// (at === 0 -> MAX_VALUE) and the page snaps in with no fade at all; + /// - `releasePending`, or the button is re-created with `busy` still + /// true and the skeleton flashes back for a frame UNDER the page that + /// just landed (the note the old handler carried, kept verbatim in + /// spirit). + /// So: cursor + fade + placeholder first, overlay LAST. + function releaseMoreLanded(releaseType, cards, hasMore) { + // ---- 1. the cursor (in place — nothing binds to it) --------------- + // Advance by the RAW page length, not by what survived the dedup: the + // server counted every row it sent. + var c = releaseCursor[releaseType] + var base = (c !== undefined && c.sent !== undefined) ? c.sent : 0 + releaseCursor[releaseType] = { "sent": base, "next": base + cards.length } + + // ---- 2. fade window + placeholder --------------------------------- + if (releaseFade[releaseType] !== undefined) + releaseFade[releaseType].at = Date.now() + if (root.releasePending[releaseType] !== undefined) { + var m = Object.assign({}, root.releasePending) + delete m[releaseType] + root.releasePending = m + } + + // ---- 3. the overlay (COPY-ON-WRITE — the projection binds to it) --- + var prev = root.releaseOverlay[releaseType] + var tail = (prev !== undefined && prev.cards !== undefined) + ? prev.cards.slice() : [] + var seen = {} + var i + // Page 1 lives in the document, and once artist_qt.rs + // `merge_release_page` has folded an earlier page into the STASHED + // document a republish puts that page there too — so both are checked + // before anything is kept, and the overlay never grows a row the + // document already carries. + var doc = root.artist.releaseSections || [] + for (i = 0; i < doc.length; i++) { + if (doc[i].releaseType !== releaseType) continue + var dc = doc[i].cards || [] + for (var j = 0; j < dc.length; j++) seen[dc[j].id] = true + break + } + for (i = 0; i < tail.length; i++) seen[tail[i].id] = true + var before = tail.length + for (i = 0; i < cards.length; i++) { + if (seen[cards[i].id]) continue + seen[cards[i].id] = true + tail.push(cards[i]) + } + // The reference CAPS the index at 4 pages per bucket and hands off to + // the discography page beyond that: artist.rs:718-720 ("Page 1 is the + // embedded bucket; 3 more loads reach the cap" — MAX_INDEX_PAGES = 4), + // enforced at append_release_page:1268. This port has that page now + // (QbzArtist.openReleases, the section title's own click), so the cap + // carries over 1:1 — an uncapped index button would page a 300-album + // Singles bucket into this grid forever, which is exactly what the + // dedicated page exists to absorb. + var pages = ((prev !== undefined && prev.pages !== undefined) ? prev.pages : 1) + 1 + // …and `!appended_ids.is_empty()` is the same line's other clause: a + // page that contributed NOTHING after the dedup (all rows already on + // screen, or empty outright) kills the button no matter what the flag + // says — leaving it up would offer a page that can never change the + // grid, and every further click would burn a request and land here + // again. + var more = (hasMore === true) && (tail.length > before) && (pages < 4) + var o = Object.assign({}, root.releaseOverlay) + o[releaseType] = { "cards": tail, "hasMore": more, "pages": pages } + root.releaseOverlay = o + } + /// Index the appended page of `releaseType` starts at — +inf when this + /// section never paged, when its page has not landed yet, or when the + /// landing is already stale. Fading unconditionally would re-dissolve the + /// whole grid on every enrichment pass, which reads as a flicker, not as + /// polish. + function releaseFadeFrom(releaseType) { + var e = releaseFade[releaseType] + if (e === undefined || e.at === 0) + return Number.MAX_VALUE + if (Date.now() - e.at > root.releaseFadeWindowMs) + return Number.MAX_VALUE + return e.from + } + /// The user picked a sort for one bucket (ReleaseGrid.slint:81-89). Hand it + /// to the bridge — `QbzArtist.setSectionSort` persists it by release_type + /// and re-sorts the STASHED document, which comes back as a republished + /// `artistJson` — and reconcile the local paging state first. + /// + /// The OVERLAY's CARDS for this bucket are dropped. Nothing is lost by + /// that: an overlay entry is only ever written when a page LANDS, and by + /// then artist_qt.rs `load_release_page` has already folded that page into + /// the stashed document (`merge_release_page`, called before the signal is + /// emitted) — so every card the overlay holds is in the document the + /// republish is about to carry, now in the new order. Keeping the cards + /// would leave a second, stale copy of those rows whose only remaining job + /// is to be deduped away. + /// + /// `hasMore` and `pages` are KEPT (the entry survives as a card-less + /// stub). They are paging state, not row data, and neither is in the + /// document: merge_release_page writes the SERVER's has_more flag into the + /// stash, not the capped one, so dropping the whole entry after the cap + /// closed a bucket would let the document's `hasMore: true` resurrect the + /// button — and the reference's LOADED_PAGES counter survives a sort + /// change too (nothing in artist.rs `resort_section` touches it). + /// + /// The CURSOR is deliberately kept. It counts rows the SERVER has sent, and + /// the server's own order (`release_date`, hardcoded on purpose — the five + /// picker keys are applied locally and never reach the API) has not changed + /// just because we re-ordered them here. Resetting it would make the next + /// Load more re-request pages we already hold, which the dedup would then + /// swallow — the "button does nothing" bug the cursor exists to prevent. + /// + /// The FADE threshold is dropped for the same reason `releaseMoreRequested` + /// suppresses it under a custom sort: it is an index into an order that no + /// longer exists, and the republish re-creates every delegate. + function releaseSortChanged(releaseType, sortKey) { + delete releaseFade[releaseType] + if (root.releaseOverlay[releaseType] !== undefined) { + var e = Object.assign({}, root.releaseOverlay[releaseType]) + delete e.cards + var o = Object.assign({}, root.releaseOverlay) + o[releaseType] = e + root.releaseOverlay = o + } + QbzArtist.setSectionSort(releaseType, sortKey) + } + Timer { + id: releaseSettle + interval: 8000 + repeat: false + onTriggered: root.releasePending = ({}) + } + // ---- Network sidebar open/closed ------------------------------------- // ShellState.content-constrained (state.slint:4114): window under the NPB // breakpoint AND a right panel (Queue / Lyrics) open. NOT a raw @@ -357,23 +732,29 @@ Rectangle { Connections { target: QbzArtist function onReleaseSectionReady(releaseType, cardsJson, hasMore) { + // Hand the page to the OVERLAY (root.releaseOverlay) and let the + // `releaseSections` projection merge it. It used to be pushed + // straight into `root.artist.releaseSections[i].cards` followed by + // a manual `artistChanged()`, and that is why "nunca cargan mas + // albumes": the push mutated the array IN PLACE, so both Repeaters + // were re-handed the identical array reference and QML had nothing + // to invalidate — the fake `artistChanged()` re-ran every OTHER + // binding on the page and left the two that mattered untouched, + // because `releaseSections`' unfiltered arm returned `all`, the + // document's own array, unchanged. + // + // The signal is emitted only on SUCCESS (main.rs:872 logs errors + // and returns), and an empty page comes through here too — that is + // what clears the skeleton when a bucket runs out. var cards = JSON.parse(cardsJson) - // The document, not root.releaseSections: that one is a FILTERED - // projection and may hand back fresh objects, so a push into it - // would be dropped on the next re-evaluation. - var sections = root.artist.releaseSections || [] - for (var i = 0; i < sections.length; i++) { - if (sections[i].releaseType === releaseType) { - var seen = {} - for (var j = 0; j < sections[i].cards.length; j++) seen[sections[i].cards[j].id] = true - for (j = 0; j < cards.length; j++) { - if (!seen[cards[j].id]) sections[i].cards.push(cards[j]) - } - sections[i].hasMore = hasMore - break - } - } - root.artistChanged() + root.releaseMoreLanded(releaseType, cards, hasMore) + // The appended covers are not in the document (artist_qt.rs folds + // them into the STASHED copy, which only reaches us on the next + // enrichment republish), so the document-driven dispatch would + // leave the new row as blank tiles until an unrelated pass + // happened to land. dispatchCovers() reads the overlay too, and + // `dispatchedCovers` makes the re-scan free. + root.dispatchCovers() } } Component.onCompleted: { @@ -407,6 +788,18 @@ Rectangle { networkOpen = !contentConstrained dismissedDiscovery = ({}) localToggles = ({}) + // A fresh artist must never inherit the previous one's paging state: + // a stale threshold would fade a band of the new artist's first page + // for no reason, a stale pending flag would show a skeleton under a + // button nobody pressed, a stale OVERLAY would graft the previous + // artist's albums onto this one's grids (release types are shared + // keys — every artist has an "album" bucket), and a stale CURSOR would + // page the new artist from the old one's offset. + releasePending = ({}) + releaseFade = ({}) + releaseOverlay = ({}) + releaseCursor = ({}) + releaseSettle.stop() membersExpanded = false groupsExpanded = false collabsExpanded = false @@ -473,6 +866,17 @@ Rectangle { for (i = 0; i < rawSections.length; i++) for (j = 0; j < (rawSections[i].cards || []).length; j++) if (rawSections[i].cards[j].artUrl) urls.push(rawSections[i].cards[j].artUrl) + // …and the pages "Load more" appended, which are NOT in the document: + // they live in root.releaseOverlay until artist_qt.rs's stashed copy + // reaches us through the next enrichment republish. Off the overlay + // rather than off `releaseSections` for the same reason the lists + // above are RAW — the projection is filtered, and a covers dispatch + // must not depend on the search box. + for (var rt in root.releaseOverlay) { + var tail = root.releaseOverlay[rt].cards || [] + for (i = 0; i < tail.length; i++) + if (tail[i].artUrl) urls.push(tail[i].artUrl) + } for (i = 0; i < rawPlaylists.length; i++) if (rawPlaylists[i].artUrl) urls.push(rawPlaylists[i].artUrl) // Magazine story thumbnails ride the same pipeline (arc-cdn URLs). @@ -557,11 +961,21 @@ Rectangle { elide: Text.ElideRight } // Cover with hover play overlay. + // + // RADIUS 4, not `theme.radiusSm` (8): rows/TrackRow.qml:553 — the + // SHARED row this component is a fork of, and the one PlaylistView + // / the album detail / search all draw — rounds its artwork cell + // at 4. The fork had drifted to 8 and the owner saw it ("los album + // arts de Popular tracks esta mas redondo que otros, por ejemplo, + // los de playlistview"). All THREE stacked elements carry the same + // number — the cell, the image and the hover scrim — or the scrim's + // corners stick out past the art. The ROW's own `radius: 8` above + // is correct and matches TrackRow.qml:288; only the cover moved. Rectangle { width: showAlbum ? cols.colArt : cols.colNumber height: showAlbum ? cols.colArt : 28 anchors.verticalCenter: parent.verticalCenter - radius: theme.radiusSm + radius: 4 color: theme.surfaceElevated clip: true QbzIcon { @@ -574,11 +988,11 @@ Rectangle { RoundedImage { anchors.fill: parent source: root.coverMap[row.artUrl] || "" - radius: theme.radiusSm + radius: 4 } Rectangle { anchors.fill: parent - radius: theme.radiusSm + radius: 4 color: "#000000" opacity: trArea.containsMouse || isActive ? 0.6 : 0.0 Behavior on opacity { NumberAnimation { duration: 150 } } @@ -1018,6 +1432,10 @@ Rectangle { // Release section (ReleaseGrid). component ReleaseSection: Column { + // Named because the card delegate below reaches for `section` from a + // nested scope, and an inline component's own root id is the one + // handle that is unambiguous there. + id: relSection property var section: ({}) property string anchorId: "" width: parent ? parent.width : 0 @@ -1048,35 +1466,95 @@ Rectangle { anchors.fill: parent hoverEnabled: true cursorShape: Qt.PointingHandCursor - // POC-NOTE: dedicated discography page out of scope. + // ReleaseGrid.slint:63-68 -> ArtistPageView.slint:928-929 + // -> ArtistActions.open-releases -> main.rs:15055-15087, + // which records the nav entry and mounts + // ArtistReleasesView on this bucket. + // + // The artist NAME travels with the call: the second door to + // the same page (AlbumView's "From the same artist" View + // all) has no artist document open and supplies its own, + // exactly as main.rs:11844 does. + // + // `relSection.section`, not a bare `section` — the inline + // component's own root id is the one unambiguous handle + // from a nested scope, which is why `relSection` is named + // at all (:1409-1411). `root.` is the same pattern the card + // delegate below already uses for `coverMap` and + // `releaseFadeFrom`. + // + // (What stood here was an empty MouseArea under a POC-NOTE + // reading "dedicated discography page out of scope" — a + // fully lit, pointer-cursor control whose entire effect was + // nothing. The page now exists; the note is gone with it.) + onClicked: QbzArtist.openReleases(root.artist.id || "", + root.artist.name || "", + relSection.section.releaseType || "") } } - // Per-section sort (ReleaseGrid.slint:70 QbzSelect). No seam: - // `set-section-sort` persists per release_type and re-sorts - // server-side, and neither exists on this bridge — so the control - // is DIMMED and carries no MouseArea at all instead of offering a - // pointer cursor over a menu that never opens. - Rectangle { + // Per-section sort — ReleaseGrid.slint:70-90, the SAME control the + // reference mounts (primitives/QbzSelect.slint), here through the + // port's existing replica controls/QbzSelect.qml. + // + // `sm: true` + `menuWidth: 150` are the .slint's own values, + // verbatim: QbzSelect.qml:34 is `width = menuWidth - 40` under + // `sm`, so 150 lands at the 110 x 30 the reference computes at + // QbzSelect.slint:88. Passing 190 "to get 150" is the exact mistake + // that file's :42-50 note documents. + // + // `id: sortBtn` is load-bearing beyond this block — the section + // title above measures `parent.width - seeAll.width - sortBtn.width + // - 24`. The control grows 28 -> 30 tall with the swap; that IS the + // reference (QbzSelect.slint `sm` is 30px). Row children are + // top-aligned, hence the explicit verticalCenter. + // + // (What stood here was a dimmed Rectangle with no MouseArea, whose + // note claimed `set-section-sort` "re-sorts server-side" and that + // no seam existed. Both halves were wrong: main.rs:14997-15007 sorts + // the ALREADY-LOADED cards in process and never touches the API — + // see artist_qt::resort_section — and the label lied twice on top of + // that, reading "Newest" over data in server order.) + QbzSelect { id: sortBtn - width: sortRow.width - height: 28 - radius: 5 - opacity: 0.4 anchors.verticalCenter: parent.verticalCenter - color: theme.surfaceElevated - Row { - id: sortRow - height: parent.height - leftPadding: 10 - rightPadding: 10 - spacing: 6 - Text { - anchors.verticalCenter: parent.verticalCenter - text: QbzSession.tr("Newest", QbzSession.trRev) - color: theme.textSecondary - font.pixelSize: 12 - } - QbzIcon { name: "chevron-down"; width: 12; height: 12; anchors.verticalCenter: parent.verticalCenter; tintName: "secondary" } + sm: true + menuWidth: 150 + // The .slint's five options in its order (:74-79). "A–Z"/"Z–A" + // carry a U+2013 EN DASH — the msgid in the catalogue is the + // en-dash form, and a hyphen would silently go untranslated in + // all seven non-English locales. (LibraryToolbar's "Title A-Z" + // is a DIFFERENT, hyphenated msgid — do not copy it here.) + options: [QbzSession.tr("Default", QbzSession.trRev), + QbzSession.tr("Newest", QbzSession.trRev), + QbzSession.tr("Oldest", QbzSession.trRev), + QbzSession.tr("A–Z", QbzSession.trRev), + QbzSession.tr("Z–A", QbzSession.trRev)] + // ReleaseGrid.slint:34-39 — the wire key back to an index. This + // is what makes the picker come back showing the sort the user + // actually chose: `sortBy` is stamped on the section by + // artist_qt.rs at page build from the persisted pref, so a + // revisited artist seats the control without a click. + // + // A BINDING, never an assignment: QbzSelect does not + // self-assign `currentIndex` (it closes the popup and emits, + // QbzSelect.qml:299-300), and every enrichment republish + // re-creates this delegate — an assignment would be lost. + currentIndex: { + var s = relSection.section.sortBy || "default" + return s === "newest" ? 1 : s === "oldest" ? 2 + : s === "title-asc" ? 3 : s === "title-desc" ? 4 : 0 + } + // ReleaseGrid.slint:81-89 — index to wire key. These five + // strings are BOTH what gets persisted and what the sort + // functions switch on, on either side of the bridge. + // + // `relSection.section`, not a bare `section`: this header lives + // inside an inline `component`, which does not see the enclosing + // document's ids — `relSection` exists for exactly this. + onSelected: function (i) { + root.releaseSortChanged(relSection.section.releaseType, + i === 1 ? "newest" : i === 2 ? "oldest" + : i === 3 ? "title-asc" : i === 4 ? "title-desc" : "default") } } } @@ -1089,6 +1567,7 @@ Rectangle { Repeater { model: section.cards delegate: AlbumCard { + id: relCard albumId: modelData.id title: modelData.title // The subtitle slot carries the YEAR on the artist page, @@ -1115,35 +1594,84 @@ Rectangle { // artist_qt::map_release stamps this the same way it // stamps the pin; false inverted the first click. isFavorite: modelData.isFavorite === true + + // ---- smooth append (owner, 2026-08-02) --------------- + // "que la aparicion de lo que se cargue, sea smooth" — + // fade in ONLY the page Load more just brought in. The + // Repeater re-creates EVERY delegate on any republish (see + // `releasePending` at the top of this file), so an + // unconditional fade would re-dissolve the whole grid on + // an unrelated pass and read as a flicker. + // + // `opacity` carries NO binding on purpose: the assignment + // below would destroy one, and a Behavior would then fire + // for every card (Behaviors are inert only DURING + // creation, and Component.onCompleted runs after it). A + // card below the threshold is never touched at all, so it + // is created — and stays — fully opaque. + NumberAnimation { + id: appendFade + target: relCard + property: "opacity" + from: 0.0 + to: 1.0 + // 220ms / OutCubic is the content duration; the + // skeleton above it fades at 180ms. + duration: 220 + easing.type: Easing.OutCubic + } + Component.onCompleted: { + if (index >= root.releaseFadeFrom(relSection.section.releaseType)) { + relCard.opacity = 0.0 + appendFade.start() + } + } } } } - Row { + // The shared affordance (controls/QbzLoadMore.qml) — this site is the + // `a` arm its header cites: plain centred text, 28 tall, and the same + // `section.hasMore` gate and bridge call as before. What is new is the + // placeholder row it draws underneath while the page is in flight. + QbzLoadMore { visible: section.hasMore width: parent.width - Item { width: (parent.width - loadMoreBtn.width) / 2; height: 1 } - Rectangle { - id: loadMoreBtn - width: loadMoreText.implicitWidth + 24 - height: 28 - color: "transparent" - Text { - id: loadMoreText - anchors.centerIn: parent - text: QbzSession.tr("Load more", QbzSession.trRev) - color: loadMoreArea.containsMouse ? theme.textPrimary : theme.textSecondary - font.pixelSize: 13 - } - MouseArea { - id: loadMoreArea - anchors.fill: parent - hoverEnabled: true - cursorShape: Qt.PointingHandCursor - onClicked: QbzArtist.loadReleaseSection(artist.id, section.releaseType, section.cards.length) - } + // The grid above has a 224 x 270 PITCH: cards/AlbumCard.qml:186-189 + // is 200 x 246 and the Grid adds 24px of column/row spacing — the + // same pitch SearchView's block was built around, so the + // placeholder lands exactly where the first appended row will. + skeleton: "cards" + cellW: 224 + cellH: 270 + // There is NO per-section loading flag to bind to: the bridge + // publishes one artist document plus the `releaseSectionReady` + // signal (artist_bridge.rs:60-66) and nothing in between. So the + // in-flight state is driven locally, off root's map, cleared by + // that signal and capped by root's 8s settle timer (the error path + // emits nothing at all — main.rs:872). + busy: root.releasePending[section.releaseType] === true + onClicked: { + // BEFORE the call: the threshold is the count on screen now + // (`section.cards` is the MERGED array — document page 1 plus + // every page the overlay holds — so the fade threshold lines + // up with the tail this request will add). + // + // The OFFSET is not that count: releaseMoreRequested returns + // the server cursor (see `releaseCursor`), which is the number + // of rows the server has actually sent. They differ as soon as + // a page repeats a row, and passing the on-screen count then + // re-requests rows we already have — the dedup swallows them + // and the button appears to do nothing, forever. + // + // The third argument suppresses the append fade when this + // bucket carries a non-default sort: the page will be + // INTERLEAVED, not appended, so an index threshold would fade + // the wrong cards (see releaseMoreRequested). + var off = root.releaseMoreRequested(section.releaseType, section.cards.length, + (section.sortBy || "default") !== "default") + QbzArtist.loadReleaseSection(artist.id, section.releaseType, off) } - Item { width: (parent.width - loadMoreBtn.width) / 2; height: 1 } } } @@ -1587,32 +2115,47 @@ Rectangle { Repeater { model: topTracks.length delegate: PopularTrackRow { - visible: root.topTracksExpanded || index < root.preview - height: visible ? 50 : 0 + // Smooth reveal (owner, 2026-08-02). This expander is + // CLIENT-SIDE: the model is a COUNT, so every row is + // already instantiated and "Load more" only flipped + // `visible` — twenty rows arriving in one frame, which + // is the "carga de golpe" the owner reported. + // + // OPACITY ONLY, deliberately: the row is 50 tall with + // a 36px cover and text anchored to its verticalCenter + // and it does NOT clip, so animating `height` would + // paint those children outside the row box and over + // its neighbours for the whole 220ms. Height still + // snaps (the space opens at once); the content + // dissolves into it. Same 220ms / OutCubic the + // appended release cards use. + // + // The gate is this explicit `revealed`, not the item's + // own `visible`: Item.visible is EFFECTIVE visibility, + // so binding opacity to it would re-fade every row + // whenever the whole catalog Column is shown again + // (tab switch, artistLoading clearing). + readonly property bool revealed: root.topTracksExpanded || index < root.preview + visible: revealed + height: revealed ? 50 : 0 + opacity: revealed ? 1.0 : 0.0 + Behavior on opacity { NumberAnimation { duration: 220; easing.type: Easing.OutCubic } } row: topTracks[index] rowIndex: index showAlbum: true } } Item { visible: topTracks.length > root.preview; width: 1; height: 4 } - Rectangle { + // The shared affordance — the `b` arm of controls/QbzLoadMore + // .qml's header: same plain 28-tall shape, same label pair, + // same client-side toggle. No `busy` and no skeleton: there is + // no fetch, the reveal is instant (the rows above own the + // smoothing). + QbzLoadMore { visible: topTracks.length > root.preview width: parent.width - height: 28 - color: "transparent" - Text { - anchors.centerIn: parent - text: root.topTracksExpanded ? QbzSession.tr("View less", QbzSession.trRev) : QbzSession.tr("Load more", QbzSession.trRev) - color: loadMoreTopArea.containsMouse ? theme.textPrimary : theme.textSecondary - font.pixelSize: 13 - MouseArea { - id: loadMoreTopArea - anchors.fill: parent - hoverEnabled: true - cursorShape: Qt.PointingHandCursor - onClicked: root.topTracksExpanded = !root.topTracksExpanded - } - } + label: root.topTracksExpanded ? QbzSession.tr("View less", QbzSession.trRev) : QbzSession.tr("Load more", QbzSession.trRev) + onClicked: root.topTracksExpanded = !root.topTracksExpanded } // --- Latest release -------------------------------------- @@ -1676,32 +2219,26 @@ Rectangle { Repeater { model: appearsOn.length delegate: PopularTrackRow { - visible: root.appearsOnExpanded || index < root.preview - height: visible ? 50 : 0 + // Same client-side reveal as Popular Tracks above + // — same component, same 50px pitch, same reason + // for fading opacity and letting height snap. + readonly property bool revealed: root.appearsOnExpanded || index < root.preview + visible: revealed + height: revealed ? 50 : 0 + opacity: revealed ? 1.0 : 0.0 + Behavior on opacity { NumberAnimation { duration: 220; easing.type: Easing.OutCubic } } row: appearsOn[index] rowIndex: index showAlbum: false } } Item { visible: appearsOn.length > root.preview; width: 1; height: 4 } - Rectangle { + // Shared affordance, `b` arm again (client-side toggle). + QbzLoadMore { visible: appearsOn.length > root.preview width: parent.width - height: 28 - color: "transparent" - Text { - anchors.centerIn: parent - text: root.appearsOnExpanded ? QbzSession.tr("View less", QbzSession.trRev) : QbzSession.tr("Load more", QbzSession.trRev) - color: loadMoreAppArea.containsMouse ? theme.textPrimary : theme.textSecondary - font.pixelSize: 13 - MouseArea { - id: loadMoreAppArea - anchors.fill: parent - hoverEnabled: true - cursorShape: Qt.PointingHandCursor - onClicked: root.appearsOnExpanded = !root.appearsOnExpanded - } - } + label: root.appearsOnExpanded ? QbzSession.tr("View less", QbzSession.trRev) : QbzSession.tr("Load more", QbzSession.trRev) + onClicked: root.appearsOnExpanded = !root.appearsOnExpanded } } @@ -2390,7 +2927,7 @@ Rectangle { model: [ { "label": QbzSession.tr("Create Artist Collection", QbzSession.trRev), "icon": "library-big", "action": "disco", "live": true }, { "label": QbzSession.tr("Artist Scene", QbzSession.trRev), "icon": "map-pin", "action": "stub", "live": false }, - { "label": QbzSession.tr("Share", QbzSession.trRev), "icon": "link", "action": "stub", "live": false }, + { "label": QbzSession.tr("Share", QbzSession.trRev), "icon": "link", "action": "share", "live": true }, { "label": root.toggleState("artistPin", artist.isPinned) ? QbzSession.tr("Unpin", QbzSession.trRev) : QbzSession.tr("Pin", QbzSession.trRev), "icon": root.toggleState("artistPin", artist.isPinned) ? "pin-filled" : "pin", "action": "pin", "live": true }, ] delegate: Rectangle { @@ -2432,6 +2969,23 @@ Rectangle { // "build-collection")`. This is the ONLY route // to it; the nav flyout has no builder entry. QbzDisco.open(artist.id) + } else if (modelData.action === "share") { + // ArtistPageView.slint:530-538 -> main.rs + // :12749 `media-action("artist", id, + // "share")`. The bridge builds the + // play.qobuz.com/artist/{id} URL, copies it + // and raises the "Link copied" toast, exactly + // as the .slint arm does. Nothing is copied + // QML-side here (unlike TrackRow.qml's + // Loader/TextEdit idiom): cxx-qt-lib exposes + // no QClipboard, and only Rust can publish + // QbzShell.toastJson, so the copy and its + // confirmation stay on one side. + // + // Link-only — artists have no Song.link path + // (share.rs's Song.link resolvers serve the + // track and album menus). + QbzArtist.share(artist.id) } } } diff --git a/crates/qbz-qt/qml/views/SectionRail.qml b/crates/qbz-qt/qml/views/SectionRail.qml index 3e26cd2e9..1ad8df18f 100644 --- a/crates/qbz-qt/qml/views/SectionRail.qml +++ b/crates/qbz-qt/qml/views/SectionRail.qml @@ -14,6 +14,13 @@ Column { property var items: [] // The host's url-keyed cover map ({url: file://path}). property var coverMap: ({}) + /// Carousel.slint's `show-view-all` (default false, as there). The ONE + /// mount that turns it on today is AlbumView's "From the same artist" rail + /// — album/AlbumPageView.slint:1124-1132 — whose link opens the artist's + /// discography. Every other SectionRail in the tree is untouched: the + /// property defaults off and QbzSectionHeader draws nothing for it. + property bool showViewAll: false + signal viewAllClicked() QbzTheme { id: theme } width: parent ? parent.width : 0 @@ -27,6 +34,10 @@ Column { QbzSectionHeader { title: root.title + // The shared header already implements both halves + // (QbzSectionHeader.qml:42-54, :107-131) — this only forwards them. + showViewAll: root.showViewAll + onViewAllClicked: root.viewAllClicked() leftEnabled: rail.contentX > 1 rightEnabled: rail.contentX < root.maxScroll - 1 onPageLeft: rail.contentX = Math.max(0, rail.contentX - root.step) diff --git a/crates/qbz-qt/src/album_qt.rs b/crates/qbz-qt/src/album_qt.rs index ac49f74c4..0d68b05f8 100644 --- a/crates/qbz-qt/src/album_qt.rs +++ b/crates/qbz-qt/src/album_qt.rs @@ -36,7 +36,7 @@ use cxx_qt_lib::QString; use qbz_app::shell::AppRuntime; use qbz_core::LoggingAdapter; use qbz_models::{Album, Track}; -use serde::Serialize; +use serde::{Deserialize, Serialize}; use serde_json::json; use crate::home_qt; @@ -98,7 +98,15 @@ pub struct TrackRow { pub is_favorite: bool, } -#[derive(Clone, Serialize)] +/// `Deserialize` rides along with `Serialize` so a card can be read BACK out of +/// the stashed artist document: `artist_qt::sort_release_values` re-sorts a +/// release bucket that already lives on disk-shaped JSON (the stash the +/// progressive enrichment passes republish), and doing that through the typed +/// struct keeps ONE copy of the sort arms — `artist_qt::sort_release_cards` — +/// instead of a second comparator written against raw `serde_json::Value`. +/// The `#[serde(rename)]`s below are bidirectional, so the camelCase the QML +/// document carries round-trips unchanged. +#[derive(Clone, Serialize, Deserialize)] pub struct AlbumCardData { pub id: String, pub title: String, diff --git a/crates/qbz-qt/src/artist_bridge.rs b/crates/qbz-qt/src/artist_bridge.rs index b4175b98a..c049f86c1 100644 --- a/crates/qbz-qt/src/artist_bridge.rs +++ b/crates/qbz-qt/src/artist_bridge.rs @@ -6,8 +6,13 @@ //! documented precedent). The QML type name comes from the QObject //! (`QbzArtist`), not from the module, so QML is unaffected. //! -//! Props: the one artist-view JSON document + its loading flag. -//! Invokables: openArtist, loadReleaseSection (per-bucket "Load more"). +//! Props: the one artist-view JSON document + its loading flag, and the one +//! discography-page document (artistReleasesJson — the sub-view reached from +//! "See discography", state and all). +//! Invokables: openArtist, resolveMusician, share, loadReleaseSection +//! (per-bucket "Load more"), setSectionSort (per-bucket release sort, persisted +//! by release_type), and the discography page's four — +//! openReleases / releasesLoadMore / releasesSetSort / releasesRetry. //! Signal: releaseSectionReady (the next page of one releases bucket). //! //! `view_param_id` is NOT here and NOT in album_bridge.rs: it was a @@ -41,6 +46,12 @@ pub mod qbz_artist_bridge { // ONE JSON document (artist_qt.rs ArtistViewData: header + top // tracks + releases buckets + labels/similar/playlists). #[qproperty(QString, artist_json)] + // --- Discography sub-view ------------------------------------------ + // ONE JSON document too (artist_releases_qt.rs ArtistReleasesDoc: + // header + the paged grid + loading / error / has-more / sort). Its + // `loading` rides INSIDE the document rather than as a second + // property — the reason is written down on that struct. + #[qproperty(QString, artist_releases_json)] type QbzArtist = super::QbzArtistRust; @@ -57,10 +68,79 @@ pub mod qbz_artist_bridge { #[qinvokable] fn resolve_musician(self: Pin<&mut QbzArtist>, name: QString, role: QString); + /// Header ⋯ → Share. `ArtistPageView.slint:530-538` fires + /// `media-action("artist", ArtistState.id, "share")`; the arm at + /// `main.rs:12749` copies `share::qobuz_artist_url(&id)` and raises + /// the "Link copied" success toast. Link-only — artists have no + /// Song.link/Album.link path (see share_qt.rs). + /// + /// `#[auto_cxx_name]` on this block makes the QML name + /// `QbzArtist.share(...)`; renaming the Rust fn renames the QML call, + /// which QML only discovers when the menu opens. + #[qinvokable] + fn share(self: Pin<&mut QbzArtist>, artist_id: QString); + /// ArtistView per-section "Load more" — the next releases page. #[qinvokable] fn load_release_section(self: Pin<&mut QbzArtist>, artist_id: QString, release_type: QString, offset: i32); + /// ArtistView per-section sort — `ReleaseGrid.slint:26` + /// `set-section-sort(release-type, sort)`, fired by that view's + /// QbzSelect. `sort` is one of `default` | `newest` | `oldest` | + /// `title-asc` | `title-desc` (the .slint's index mapping at :81-89), + /// and those five strings are BOTH the persisted values and the + /// sort-function keys. + /// + /// Persists by release_type and re-sorts the LOADED bucket in place — + /// the reference never refetches (main.rs:14997-15007 is the whole + /// handler; see the wire note on `artist_qt::resort_section`). + /// + /// `#[auto_cxx_name]` makes the QML name `QbzArtist.setSectionSort`. + #[qinvokable] + fn set_section_sort(self: Pin<&mut QbzArtist>, release_type: QString, sort: QString); + + // --- Discography page ---------------------------------------------- + // `#[auto_cxx_name]` makes the QML names QbzArtist.openReleases / + // .releasesLoadMore / .releasesSetSort / .releasesRetry. QML resolves + // singleton members LAZILY: a call with no matching invokable here + // compiles clean and throws the first time a user clicks, so these four + // and the QML that calls them are one edit. + + /// "See discography" (artist/ReleaseGrid.slint:63-68 -> + /// ArtistPageView.slint:928-929 -> main.rs:15055-15087) and the album + /// page's "From the same artist" View all + /// (album/AlbumPageView.slint:1131 -> main.rs:11839-11865, which pins + /// `release_type` to "album"). + /// + /// The NAME travels with the call rather than being read out of the + /// artist document, because the second door has no artist document + /// open — it passes `AlbumState.artist`, exactly as main.rs:11844 does. + #[qinvokable] + fn open_releases( + self: Pin<&mut QbzArtist>, + artist_id: QString, + artist_name: QString, + release_type: QString, + ); + + /// Infinite-scroll tail (ArtistReleasesView.slint:121-129). The page + /// has no "Load more" button — the view fires this when the flick gets + /// within 600px of the bottom, and every guard lives in Rust as well so + /// a burst of scroll frames cannot queue two pages. + #[qinvokable] + fn releases_load_more(self: Pin<&mut QbzArtist>); + + /// The header's sort select. Client-side only, persisted per + /// release_type in the SAME store as the artist page's per-section + /// picker (artist_prefs.rs) — see artist_releases_qt::set_sort. + #[qinvokable] + fn releases_set_sort(self: Pin<&mut QbzArtist>, sort: QString); + + /// The error state's Retry button (main.rs:15158-15176) — a full reset + /// + refetch that records NO nav entry. + #[qinvokable] + fn releases_retry(self: Pin<&mut QbzArtist>); + /// Emitted with the next page of a releases bucket. #[qsignal] fn release_section_ready(self: Pin<&mut QbzArtist>, release_type: QString, cards_json: QString, has_more: bool); @@ -75,6 +155,7 @@ use qbz_artist_bridge::QbzArtist; pub struct QbzArtistRust { artist_loading: bool, artist_json: QString, + artist_releases_json: QString, } impl Default for QbzArtistRust { @@ -82,6 +163,9 @@ impl Default for QbzArtistRust { Self { artist_loading: false, artist_json: QString::from("{}"), + // "{}" and not "": the views JSON.parse this straight, and an empty + // string throws into the catch arm on the very first frame. + artist_releases_json: QString::from("{}"), } } } @@ -114,8 +198,55 @@ impl qbz_artist_bridge::QbzArtist { pub fn load_release_section(self: Pin<&mut Self>, artist_id: QString, release_type: QString, offset: i32) { crate::load_release_section(artist_id.to_string(), release_type.to_string(), offset); } + /// Straight through to `artist_qt`, with no `crate::` forwarder in main.rs: + /// unlike `load_release_section` (main.rs:856) this path is SYNCHRONOUS and + /// touches no `AppRuntime` — it persists a small json and patches the + /// already-stashed document. Wrapping it in an async hop would only add a + /// frame of latency between the click and the re-ordered grid. + pub fn set_section_sort(self: Pin<&mut Self>, release_type: QString, sort: QString) { + crate::artist_qt::resort_section(&release_type.to_string(), &sort.to_string()); + } + pub fn resolve_musician(self: Pin<&mut Self>, name: QString, role: QString) { crate::resolve_musician(name.to_string(), role.to_string()); } + /// Straight through to `share_qt` — no nav and no bridge state is touched, + /// so there is no `crate::` forwarder in main.rs for this one (the + /// `crate::open_artist` style exists because that one mutates both). + pub fn share(self: Pin<&mut Self>, artist_id: QString) { + crate::share_qt::share_artist(artist_id.to_string()); + } + + // --- Discography page --------------------------------------------------- + // One-line delegates into `artist_releases_qt`, with no `crate::` forwarder + // in main.rs: that indirection exists for the paths that mutate main.rs's + // own state (LAST_DETAIL, the nav latch). This page owns its state in its + // own module and records its own nav entry, so there is nothing for main.rs + // to coordinate. + + pub fn open_releases( + self: Pin<&mut Self>, + artist_id: QString, + artist_name: QString, + release_type: QString, + ) { + crate::artist_releases_qt::open( + artist_id.to_string(), + artist_name.to_string(), + release_type.to_string(), + ); + } + + pub fn releases_load_more(self: Pin<&mut Self>) { + crate::artist_releases_qt::load_more(); + } + + pub fn releases_set_sort(self: Pin<&mut Self>, sort: QString) { + crate::artist_releases_qt::set_sort(sort.to_string()); + } + + pub fn releases_retry(self: Pin<&mut Self>) { + crate::artist_releases_qt::retry(); + } } diff --git a/crates/qbz-qt/src/artist_prefs.rs b/crates/qbz-qt/src/artist_prefs.rs new file mode 100644 index 000000000..e21f5275c --- /dev/null +++ b/crates/qbz-qt/src/artist_prefs.rs @@ -0,0 +1,156 @@ +//! Artist page per-section release sort — the Qt half of +//! `crates/qbz/src/artist_prefs.rs` (78 lines, read in full for this port). +//! +//! SAME document, SAME disk key: `/qbz/artist_ui.json`, an object +//! whose `section_sort` member maps `release_type` -> sort key +//! (`newest` | `oldest` | `title-asc` | `title-desc`). That is deliberate and +//! is the whole point of the reference's design note — the choice is keyed by +//! release_type ONLY, never by artist, "so a chosen sort sticks across artists +//! and restarts". A user who alternates between the shipping Slint build and +//! this one must find the same sort, so the snake_case key and the five exact +//! wire strings are not ours to rename. +//! +//! Like `library_prefs.rs` (favorites_ui.json) and `local_state.rs` +//! (locallibrary_ui.json), this file follows the PORT's writer, not the +//! Slint's: +//! +//! - the Slint `set_sort()` serializes its whole `Prefs` struct through +//! `fs::write` (O_TRUNC — a reader that lands mid-write sees an empty file +//! and answers `Prefs::default()`), and in doing so drops any key the file +//! carries that its struct does not know about; +//! - here a write is a read-modify-write of the parsed OBJECT, published +//! through `settings_qt::write_json_object_atomic` (temp file + `rename(2)`), +//! so a concurrent reader sees the whole old document or the whole new one, +//! and any key `artist_ui.json` gains later survives our writes untouched. +//! +//! There is deliberately NO process-local cache here. The Slint keeps a +//! `thread_local!` one (`artist_prefs.rs:19-22`); this file is co-owned with a +//! second process, and it is read at most once per artist page load (one +//! `read_all()` for every bucket), so a stale cache is the worse trade. + +use std::collections::HashMap; +use std::path::PathBuf; + +use serde_json::{Map, Value}; + +/// The on-disk member name. SNAKE_CASE because the Slint's +/// `struct Prefs { section_sort: HashMap }` (artist_prefs.rs:16) +/// reads this exact name — renaming it silently splits the two frontends' +/// stored sorts instead of erroring. +const SECTION_SORT_KEY: &str = "section_sort"; + +/// The value stored for "leave the order the server sent". Never written to +/// disk: see `apply_sort`. +pub(crate) const DEFAULT_SORT: &str = "default"; + +fn store_path() -> Option { + Some(dirs::data_dir()?.join("qbz").join("artist_ui.json")) +} + +/// Every persisted bucket sort, `release_type -> sort`. Read ONCE per artist +/// page build (`artist_qt::map_artist`) rather than once per bucket. +/// +/// Any failure — no data dir, unreadable file, a `section_sort` that is not an +/// object — yields an empty map, i.e. every bucket falls back to "default". +/// That matches the Slint's `unwrap_or_default()` on a bad parse. +pub(crate) fn read_all() -> HashMap { + let Some(path) = store_path() else { + return HashMap::new(); + }; + let Some(doc) = crate::settings_qt::read_json_object(&path) else { + return HashMap::new(); + }; + let Some(map) = doc.get(SECTION_SORT_KEY).and_then(|v| v.as_object()) else { + return HashMap::new(); + }; + map.iter() + .filter_map(|(rt, v)| v.as_str().map(|s| (rt.clone(), s.to_string()))) + .collect() +} + +/// ONE bucket's persisted sort — the port of `qbz/src/artist_prefs.rs:45-52` +/// `get_sort`, which `artist_releases::reset` (`artist_releases.rs:25`) calls to +/// seat the discography page's picker on the choice the user already made on +/// the artist page. The two surfaces deliberately SHARE the store: the pref is +/// keyed by release_type only, so opening "See discography" on a bucket sorted +/// A–Z lands on an A–Z page. +/// +/// The artist page itself does NOT go through here — `map_artist` reads the +/// whole map once per page build (`read_all`) rather than once per bucket. This +/// accessor exists for the single-bucket callers. +pub(crate) fn get_sort(release_type: &str) -> String { + read_all() + .get(release_type) + .cloned() + .unwrap_or_else(|| DEFAULT_SORT.to_string()) +} + +/// The `section_sort` edit, factored out of `set_sort` so it can be tested +/// without a data dir. +/// +/// "default" REMOVES the entry instead of storing the string (the Slint's +/// `artist_prefs.rs:61-67`, "'default' removes the entry to keep the file +/// small"). Storing it would round-trip correctly today but would diverge from +/// the file the Slint writes, which is the one thing this module exists to +/// avoid. +fn apply_sort(map: &mut Map, release_type: &str, sort: &str) { + if sort == DEFAULT_SORT { + map.remove(release_type); + } else { + map.insert(release_type.to_string(), Value::String(sort.to_string())); + } +} + +/// Persist one bucket's sort. Read-modify-write of the WHOLE document so the +/// `section_sort` sub-object is merged, never rebuilt, and no sibling key is +/// lost (see the header). +pub(crate) fn set_sort(release_type: &str, sort: &str) { + let Some(path) = store_path() else { + return; + }; + // `None` means the file exists but did not parse: the caller must not + // write, or it would replace a document it never managed to read + // (settings_qt::read_json_object). + let Some(mut doc) = crate::settings_qt::read_json_object(&path) else { + return; + }; + let mut sorts = match doc.get(SECTION_SORT_KEY) { + Some(Value::Object(m)) => m.clone(), + _ => Map::new(), + }; + apply_sort(&mut sorts, release_type, sort); + doc.insert(SECTION_SORT_KEY.to_string(), Value::Object(sorts)); + crate::settings_qt::write_json_object_atomic(&path, &doc); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn the_disk_key_stays_snake_case_for_the_slint() { + // qbz/src/artist_prefs.rs:16 reads this exact member name. + assert_eq!(SECTION_SORT_KEY, "section_sort"); + } + + #[test] + fn default_removes_the_entry_rather_than_storing_it() { + let mut map = Map::new(); + apply_sort(&mut map, "album", "title-asc"); + assert_eq!(map.get("album").and_then(|v| v.as_str()), Some("title-asc")); + apply_sort(&mut map, "album", "default"); + assert!(map.get("album").is_none(), "'default' must delete the key"); + } + + #[test] + fn buckets_are_independent() { + // The pref is keyed by release_type ONLY — one artist's choice for + // "album" must not disturb "live" (and applies to every artist). + let mut map = Map::new(); + apply_sort(&mut map, "album", "newest"); + apply_sort(&mut map, "live", "oldest"); + apply_sort(&mut map, "album", "default"); + assert!(map.get("album").is_none()); + assert_eq!(map.get("live").and_then(|v| v.as_str()), Some("oldest")); + } +} diff --git a/crates/qbz-qt/src/artist_qt.rs b/crates/qbz-qt/src/artist_qt.rs index 558ac802c..6c9338152 100644 --- a/crates/qbz-qt/src/artist_qt.rs +++ b/crates/qbz-qt/src/artist_qt.rs @@ -67,6 +67,33 @@ const RELEASE_SECTION_ORDER: &[(&str, &str)] = &[ ("next", "Upcoming"), ]; +/// Title-case a raw release_type key — the fallback for a bucket the table +/// above does not know (`qbz/src/artist.rs:117-123` `title_case`). +fn title_case(key: &str) -> String { + let mut chars = key.chars(); + match chars.next() { + Some(first) => first.to_uppercase().collect::() + chars.as_str(), + None => String::new(), + } +} + +/// Display title for one release_type — the discography page's header +/// (`qbz/src/artist.rs:107-114` `release_type_title`). +/// +/// Same table, same fallback, as the per-section headers `map_artist` builds +/// below: the page the user lands on after "See discography" must be titled +/// exactly like the section they clicked from. +/// +/// Translated HERE rather than in QML because it travels inside a JSON +/// document — which is also why `artist_releases_qt::republish` exists. +pub(crate) fn release_type_title(release_type: &str) -> String { + RELEASE_SECTION_ORDER + .iter() + .find(|(rt, _)| *rt == release_type) + .map(|(_, title)| qbz_i18n::t(title)) + .unwrap_or_else(|| title_case(release_type)) +} + #[derive(Clone, Serialize)] pub struct ArtistReleaseSection { #[serde(rename = "releaseType")] @@ -75,6 +102,20 @@ pub struct ArtistReleaseSection { #[serde(rename = "hasMore")] pub has_more: bool, pub cards: Vec, + /// The bucket's persisted sort (`default` | `newest` | `oldest` | + /// `title-asc` | `title-desc`) — the Slint's `ArtistReleaseSection.sort-by`. + /// + /// It travels WITH the data because that is what makes the picker come + /// back showing the user's choice: `ReleaseGrid.slint:34-39` maps this + /// string back to a `current-index` for its QbzSelect, and the QML port + /// does the same (`ArtistView.qml`, the ReleaseSection header). The value + /// is stamped here at page build from `artist_prefs::read_all()`, so the + /// FIRST paint is already in the right order — the cards below are sorted + /// before this struct exists (`artist.rs:786-812`: "Apply the persisted + /// per-bucket sort up front so the first paint already honors the user's + /// choice"). + #[serde(rename = "sortBy")] + pub sort_by: String, } #[derive(Clone, Serialize)] @@ -293,6 +334,77 @@ pub(crate) fn map_release(release: &PageArtistRelease) -> AlbumCardData { } } +/// `album_map::sort_album_items` (crates/qbz/src/album_map.rs:244-264) for the +/// artist page's release buckets — the SAME function `label_qt::sort_cards` +/// already ports for `HomeCard`, here for `AlbumCardData`. +/// +/// `year` on these cards is the PLAIN 4-digit year (`map_release` slices +/// `dates.original[..4]`), so the lexicographic compare IS a chronological one. +/// `title`/`artist` are case-insensitive, per the reference's doc comment. +/// +/// "default" — and any key this function does not know — falls through +/// UNSORTED (`album_map.rs:262` is a bare `_ => {}`). That is not an oversight: +/// "Default" in the picker means "the order Qobuz sent", which is exactly what +/// leaving the vector alone produces, and it is why `artist_prefs::set_sort` +/// DELETES the entry for it instead of storing a no-op. +/// +/// `artist-asc` / `artist-desc` are unreachable from the artist page's picker +/// (its five options stop at title-desc) but are carried anyway so this stays +/// 1:1 with `sort_album_items`. +/// +/// The discography page has its OWN copy of these arms +/// (`artist_releases_qt::sort_cards`) rather than reusing this one: its rows +/// are `home_qt::HomeCard` (they must carry `artPath` for `AlbumCollection`), +/// not `AlbumCardData`, so the two differ in their item type and nothing else. +/// `label_qt::sort_cards` is the third copy, for the same reason. +pub(crate) fn sort_release_cards(items: &mut [AlbumCardData], sort: &str) { + match sort { + "oldest" | "year-asc" => items.sort_by(|a, b| a.year.cmp(&b.year)), + "newest" | "year-desc" => items.sort_by(|a, b| b.year.cmp(&a.year)), + "title-asc" => items.sort_by(|a, b| a.title.to_lowercase().cmp(&b.title.to_lowercase())), + "title-desc" => items.sort_by(|a, b| b.title.to_lowercase().cmp(&a.title.to_lowercase())), + "artist-asc" => items.sort_by(|a, b| a.artist.to_lowercase().cmp(&b.artist.to_lowercase())), + "artist-desc" => { + items.sort_by(|a, b| b.artist.to_lowercase().cmp(&a.artist.to_lowercase())) + } + _ => {} + } +} + +/// `sort_release_cards` over the STASHED document's `cards` array, which is raw +/// JSON rather than typed rows (the stash is a `serde_json::Value` — see +/// `ARTIST_DOC`). +/// +/// The round-trip through `AlbumCardData` is deliberate: it keeps ONE copy of +/// the sort arms above instead of a second comparator written against +/// `Value`, and these arrays are at most a few dozen rows. A row that fails to +/// deserialize leaves the whole bucket untouched rather than half-sorted — the +/// document is what the view paints, and a partial re-order would be worse +/// than none. +fn sort_release_values(items: &mut [serde_json::Value], sort: &str) { + if sort == crate::artist_prefs::DEFAULT_SORT { + return; + } + let parsed: Result, _> = items + .iter() + .map(|v| serde_json::from_value::(v.clone())) + .collect(); + let mut cards = match parsed { + Ok(cards) => cards, + Err(e) => { + log::warn!("[qbz-qt] release bucket not sortable ({e}) — leaving its order alone"); + return; + } + }; + sort_release_cards(&mut cards, sort); + for (slot, card) in items.iter_mut().zip(cards.iter()) { + match serde_json::to_value(card) { + Ok(value) => *slot = value, + Err(e) => log::warn!("[qbz-qt] release card re-serialize failed: {e}"), + } + } +} + /// artist.rs `map_track` (Popular Tracks / Appears On rows — album title + /// cover ride the nested album object). fn map_track(index: usize, track: PageArtistTrack) -> TrackRow { @@ -488,6 +600,17 @@ fn map_artist(page: PageArtistResponse) -> ArtistViewData { .map(|(index, track)| map_track(index, track)) .collect(); + // The persisted per-bucket sorts, read ONCE for the whole page (the Slint + // calls `artist_prefs::get_sort` per bucket off a thread_local cache; + // this port has no cache by design — see artist_prefs.rs's header). + let saved_sorts = crate::artist_prefs::read_all(); + let sort_for = |rt: &str| { + saved_sorts + .get(rt) + .cloned() + .unwrap_or_else(|| crate::artist_prefs::DEFAULT_SORT.to_string()) + }; + // One section per non-empty bucket in the official order; "download" // drains but never renders (Purchase Only is hidden upstream too). let mut release_sections: Vec = Vec::new(); @@ -496,35 +619,44 @@ fn map_artist(page: PageArtistResponse) -> ArtistViewData { bucket_cards.remove(rt); continue; } - if let Some(cards) = bucket_cards.remove(rt) { + if let Some(mut cards) = bucket_cards.remove(rt) { if cards.is_empty() { continue; } + // artist.rs:786-812 — the sort is applied at page BUILD, not on + // first click, "so the first paint already honors the user's + // choice", and `sort_by` is stamped so the picker seats itself on + // that choice instead of snapping back to Default on a revisit. + let sort = sort_for(rt); + sort_release_cards(&mut cards, &sort); release_sections.push(ArtistReleaseSection { release_type: rt.to_string(), title: qbz_i18n::t(title), has_more: bucket_has_more.get(rt).copied().unwrap_or(false), cards, + sort_by: sort, }); } } // Leftovers: server buckets unknown to the table, appended (title-cased). - for (rt, cards) in bucket_cards { + // These render a full ReleaseSection header, sort picker included, so they + // must round-trip the pref exactly like the known buckets. + for (rt, mut cards) in bucket_cards { if cards.is_empty() { continue; } - let title = { - let mut chars = rt.chars(); - match chars.next() { - Some(first) => first.to_uppercase().collect::() + chars.as_str(), - None => rt.clone(), - } - }; + // Factored into `title_case` (above) when the discography page needed + // the same fallback — one copy, so a leftover bucket's section header + // and its discography page header cannot drift apart. + let title = title_case(&rt); + let sort = sort_for(&rt); + sort_release_cards(&mut cards, &sort); release_sections.push(ArtistReleaseSection { has_more: false, release_type: rt, title, cards, + sort_by: sort, }); } @@ -710,6 +842,15 @@ fn merge_release_page( return; }; section["hasMore"] = json!(has_more); + // The bucket's live sort, read off the row itself — artist.rs:1245-1258 + // `append_release_page` does exactly this (`let sort = row.sort_by...`) + // before re-sorting, because under a custom sort a page appended at the + // TAIL is simply in the wrong place. + let sort = section + .get("sortBy") + .and_then(|v| v.as_str()) + .unwrap_or(crate::artist_prefs::DEFAULT_SORT) + .to_string(); let Some(existing) = section.get_mut("cards").and_then(|v| v.as_array_mut()) else { return; }; @@ -725,6 +866,96 @@ fn merge_release_page( existing.push(value); } } + // No-op under "default" (the whole point of that key), so the common path + // pays nothing. + sort_release_values(existing, &sort); +} + +/// Whether the stashed artist document still belongs to `artist_id` — the +/// SAME test `merge_release_page` opens with, exported so main.rs's +/// `load_release_section` can apply it to the OTHER half of a landed page. +/// +/// The page travels on two legs: `merge_release_page` folds it into the stash +/// (guarded — a stash for a different artist drops it), and the +/// `releaseSectionReady` signal hands it to the view. The signal carries NO +/// artist id, and ArtistView.qml folds whatever arrives into +/// `root.releaseOverlay` keyed by release_type alone — keys every artist +/// shares ("album" exists on all of them). So a page requested on artist A +/// that lands after artist B's document has been published would graft A's +/// albums onto B's album grid, permanently (the overlay is only reset when the +/// artist ID changes again). Emitting only while the stash still names the +/// requesting artist closes that leak at its source; the view cannot, because +/// the signal does not tell it whose page this is. +pub(crate) fn stash_is_for(artist_id: &str) -> bool { + match ARTIST_DOC.lock() { + Ok(guard) => guard + .as_ref() + .map(|(_, doc)| doc.get("id").and_then(|v| v.as_str()) == Some(artist_id)) + .unwrap_or(false), + Err(_) => false, + } +} + +/// Artist page per-section sort — the port of `crates/qbz/src/artist.rs:1178-1210` +/// `resort_section` ("Re-sort one release bucket in place […] and persist the +/// choice"), reached from `QbzArtist.setSectionSort` (artist_bridge.rs). +/// +/// THE SORT NEVER LEAVES THIS PROCESS. `main.rs:14997-15007` — the whole Slint +/// handler — calls `artist::resort_section` and nothing else: no refetch, no +/// query param. Every artist-page `get_releases_grid` call in BOTH trees passes +/// the constant `Some("release_date")` (here: `load_release_page`), and the five +/// picker keys have no server equivalent; `album_map::sort_album_items` applies +/// them locally over the cards already loaded. So `load_release_page` is +/// deliberately left untouched by this feature. +/// +/// Persist FIRST (so the choice survives even if the page is being torn down), +/// then patch the stashed document and republish it through the existing +/// generation-guarded `publish_patch` — the port's ONE transport for the artist +/// view. Re-serializing the whole document is also what makes QML notice: a JS +/// array handed back by the same reference re-triggers nothing +/// (cards/PlaylistCollage.qml's rule), whereas a fresh `artistJson` string +/// re-runs the parse. +/// +/// Reading the generation off the stash rather than off `ARTIST_GEN` is what +/// keeps a click aimed at the page ON SCREEN: if the user has already navigated +/// away, the stash belongs to another artist and `publish_patch` drops the +/// edit. +/// +/// 1:1 note — picking "Default" after "A–Z" does NOT restore the server order +/// until the page is reloaded, because "default" is a no-op sort over the +/// vector as it stands. The reference behaves identically (`sort_album_items` +/// falls through on that key); restoring it would need an untouched copy of the +/// server order that neither tree keeps. +pub(crate) fn resort_section(release_type: &str, sort: &str) { + crate::artist_prefs::set_sort(release_type, sort); + // The guard is dropped at the end of this statement — `publish_patch` + // takes the same non-reentrant lock. + let generation = ARTIST_DOC + .lock() + .ok() + .and_then(|guard| guard.as_ref().map(|(gen, _)| *gen)); + let Some(generation) = generation else { + return; + }; + let release_type = release_type.to_string(); + let sort = sort.to_string(); + publish_patch(generation, move |doc| { + let Some(sections) = doc.get_mut("releaseSections").and_then(|v| v.as_array_mut()) else { + return; + }; + let Some(section) = sections.iter_mut().find(|s| { + s.get("releaseType").and_then(|v| v.as_str()) == Some(release_type.as_str()) + }) else { + return; + }; + // Stamp the key BEFORE the cards: `sortBy` is what seats the picker + // (and what `merge_release_page` reads for the next page), so it has to + // be written even when the sort itself is a no-op. + section["sortBy"] = json!(sort); + if let Some(cards) = section.get_mut("cards").and_then(|v| v.as_array_mut()) { + sort_release_values(cards, &sort); + } + }); } /// Fetch + publish (perf-marked like phase 5), then kick the progressive diff --git a/crates/qbz-qt/src/artist_releases_qt.rs b/crates/qbz-qt/src/artist_releases_qt.rs new file mode 100644 index 000000000..e94b1514a --- /dev/null +++ b/crates/qbz-qt/src/artist_releases_qt.rs @@ -0,0 +1,495 @@ +//! Dedicated artist discography page — the Qt half of +//! `crates/qbz/src/artist_releases.rs` (99 lines, the whole file) plus the four +//! `main.rs` handlers that drive it: +//! +//! - `main.rs:15055-15087` `.on_open_releases` -> `open` +//! - `main.rs:3570-3607` `navigate_artist_releases` (the fetch half) +//! - `main.rs:15098-15135` `.on_load_more` -> `load_more` +//! - `main.rs:15143-15148` `.on_set_sort` -> `set_sort` +//! - `main.rs:15158-15176` `.on_retry` -> `retry` +//! +//! The page is the full, paginated grid for ONE release bucket of ONE artist, +//! reached from the artist page's "See discography" link +//! (`artist/ReleaseGrid.slint:54-69`) and from the album page's "From the same +//! artist" View-all (`album/AlbumPageView.slint:1123-1136`, which pins the +//! bucket to `"album"`). Two doors, one page — hence `open` takes the artist +//! NAME as a parameter instead of reading it off the artist document: the album +//! page supplies its own (`main.rs:11844-11848`). +//! +//! # Why this file does NOT call `artist_qt::load_release_page` +//! +//! That function exists for the artist page's per-section "Load more" and it +//! ends in `merge_release_page`, which folds every fetched page into the +//! STASHED ARTIST DOCUMENT (`artist_qt::ARTIST_DOC`). Paging this page ten deep +//! would silently grow the artist page's own "Albums" grid to 200 cards behind +//! the user's back. The reference keeps `ArtistState.release-sections` and +//! `ArtistReleasesState.albums` strictly disjoint (two states, one endpoint), +//! so this file issues the identical core call itself: +//! +//! ```text +//! get_releases_grid(id, release_type, RELEASE_PAGE_SIZE, offset, Some("release_date")) +//! ``` +//! +//! Same 20-row page size, same server sort — there is no separate discography +//! endpoint, and `Some("release_date")` is what makes the picker's "Default" +//! option mean something (see `sort_cards`). +//! +//! # Why the rows are `home_qt::HomeCard`, not `album_qt::AlbumCardData` +//! +//! The view mounts the shared `views/AlbumCollection.qml`, whose delegates read +//! `modelData.artPath` (`AlbumCollection.qml:391`) — a Rust-resolved cache path. +//! `AlbumCardData` carries `artUrl` and NO `artPath`, which is why the artist +//! page resolves its covers in QML through `QbzShell.sidebarArtworkWindow` -> +//! `QbzLibrary.libraryArtworkReady` -> its own `coverMap`. Publishing +//! `AlbumCardData` into `AlbumCollection` compiles, mounts, and draws every +//! cover BLANK. So the fetch runs the `label_qt::fetch_releases` two-pass +//! (`label_qt.rs:800-847`): `home_qt::attach_card_art` for what is already on +//! disk, publish, `artwork_qt::download_missing`, re-attach, publish again. +//! +//! # The subtitle slot carries the YEAR +//! +//! `qbz/src/artist.rs:670-698` `card_to_item`: "the card subtitle slot should +//! show the release year — the artist is redundant since we're already on their +//! page". It routes `year` through the card's `artist` field and blanks +//! `artist_id` so the line is inert, while `plain_year` keeps the real value +//! for the sort. `HomeCard` has no `plain_year`, so here `artist` AND `year` +//! both carry the year and `sort_cards` reads `year` — never `artist`, which +//! only happens to work while every release has one. + +use std::collections::HashSet; +use std::sync::Mutex; + +use cxx_qt_lib::QString; +use qbz_models::PageArtistRelease; +use serde::Serialize; + +use crate::home_qt::HomeCard; + +/// Page state. ONE bucket of ONE artist at a time — navigating to another +/// bucket resets it wholesale, exactly as `artist_releases::reset` does. +#[derive(Default)] +struct State { + /// Bumped by every `open` / `retry`. A page that lands for a previous + /// bucket (or a previous artist) is DROPPED rather than painted — the + /// `label_qt.rs:697` guard. It matters more here than there: the nav stack + /// stores no payload (`nav_qt.rs:9-23`), so back/forward re-mounts this + /// view against whatever document is still on the bridge, and a late page + /// painting into the wrong bucket would be indistinguishable from data. + generation: u64, + artist_id: String, + name: String, + release_type: String, + /// The translated bucket title (`artist_qt::release_type_title`). Built in + /// RUST, so it cannot re-translate through `trRev` — see `republish`. + title: String, + cards: Vec, + /// Rows the SERVER has handed over, which is the offset of the next page. + /// + /// Deliberately NOT `cards.len()` (the reference's `loaded_count`, + /// `artist_releases.rs:94-99`): blacklist filtering and id-dedup both make + /// the rendered count smaller than the fetched one, and feeding the smaller + /// number back as an offset re-requests rows already seen and skips the + /// tail. Invisible with an empty blacklist, which is why the reference gets + /// away with it. + offset: u32, + has_more: bool, + loading: bool, + load_more_loading: bool, + load_error: bool, + /// `default` | `newest` | `oldest` | `title-asc` | `title-desc` — the same + /// five wire strings the artist page's per-section picker persists, and the + /// same store (`artist_prefs`). Seeded on open from the persisted value + /// (`artist_releases.rs:25`), so a bucket the user sorted A–Z opens A–Z. + sort_by: String, +} + +static RELEASES: Mutex> = Mutex::new(None); + +fn with_state(f: impl FnOnce(&mut State) -> R) -> R { + let mut guard = RELEASES.lock().unwrap(); + f(guard.get_or_insert_with(State::default)) +} + +/// The ONE document this page paints from (`ui/state.slint:3996-4019` +/// `ArtistReleasesState`, camelCased for QML). +/// +/// `loading` lives INSIDE the document rather than on a second bridge property. +/// `label_qt` publishes `labelReleasesLoading` separately only because it +/// predates the single-document convention; this page has no cross-view reader, +/// so one property is one fewer bridge member and one fewer thing to keep in +/// sync. QML re-parses on every change, so a fresh string is a fresh object — +/// the "Rebind requires a NEW object reference" rule +/// (`cards/PlaylistCollage.qml`) is satisfied by construction. +#[derive(Serialize)] +struct ArtistReleasesDoc<'a> { + id: &'a str, + name: &'a str, + #[serde(rename = "releaseType")] + release_type: &'a str, + title: &'a str, + albums: &'a [HomeCard], + #[serde(rename = "hasMore")] + has_more: bool, + loading: bool, + #[serde(rename = "loadMoreLoading")] + load_more_loading: bool, + #[serde(rename = "loadError")] + load_error: bool, + #[serde(rename = "sortBy")] + sort_by: &'a str, +} + +fn publish() { + let json = with_state(|s| { + let doc = ArtistReleasesDoc { + id: &s.artist_id, + name: &s.name, + release_type: &s.release_type, + title: &s.title, + albums: &s.cards, + has_more: s.has_more, + loading: s.loading, + load_more_loading: s.load_more_loading, + load_error: s.load_error, + sort_by: &s.sort_by, + }; + serde_json::to_string(&doc).unwrap_or_else(|_| "{}".to_string()) + }); + crate::artist_bridge::ui(move |mut b| { + b.as_mut() + .set_artist_releases_json(QString::from(json.as_str())); + }); +} + +// --------------------------------------------------------------------------- +// Mapping +// --------------------------------------------------------------------------- + +/// One server release -> one `AlbumCollection` row, or `None` when the album is +/// blacklisted. +/// +/// The blacklist test runs on the release's REAL artist id — i.e. BEFORE the +/// subtitle transposition blanks it (`qbz/src/artist.rs:167` and +/// `artist_releases.rs:50-52` both filter; `artist_qt::load_release_page` does +/// NOT, which is a separate gap on the artist page's own Load-more path). +/// Getting the order wrong here would leave the album axis working and silently +/// disable the artist axis. +fn map_card(release: &PageArtistRelease) -> Option { + // Reuse the artist page's mapper for every field value (pin badge, heart, + // quality tier + detail, genre, the 4-digit year, the best cover url). + let card = crate::artist_qt::map_release(release); + if crate::artist_blacklist::card_blacklisted(&card.id, &card.artist_id) { + return None; + } + Some(HomeCard { + id: card.id, + title: card.title, + // The YEAR, not the artist — see the file header. + artist: card.year.clone(), + artist_id: String::new(), + genre: card.genre, + year: card.year, + quality_tier: card.quality_tier, + quality_detail: card.quality_detail, + art_url: card.art_url, + is_pinned: card.is_pinned, + is_favorite: card.is_favorite, + // ribbon / ribbon_kind / rank / plays / category / playlist_* stay + // empty: this grid is albums only and mounts no ribbon. + ..Default::default() + }) +} + +/// `album_map::sort_album_items` (`qbz/src/album_map.rs:246-264`) — the twin of +/// `label_qt::sort_cards`, minus the artist arms (this page's picker has five +/// options and stops at title-desc, and `artist` holds the year here anyway). +/// +/// `year` is the PLAIN 4-digit year, so the lexicographic compare IS a +/// chronological one. +/// +/// `"default"` falls through UNSORTED, and that is the whole point of the key: +/// the fetch asks the server for `release_date` order, so leaving the vector +/// alone IS "the order Qobuz sent". Applying any "sensible default" here is a +/// visible parity break on the very first frame. +fn sort_cards(items: &mut [HomeCard], sort: &str) { + match sort { + "oldest" | "year-asc" => items.sort_by(|a, b| a.year.cmp(&b.year)), + "newest" | "year-desc" => items.sort_by(|a, b| b.year.cmp(&a.year)), + "title-asc" => items.sort_by(|a, b| a.title.to_lowercase().cmp(&b.title.to_lowercase())), + "title-desc" => items.sort_by(|a, b| b.title.to_lowercase().cmp(&a.title.to_lowercase())), + _ => {} + } +} + +// --------------------------------------------------------------------------- +// Entry points (QbzArtist invokables) +// --------------------------------------------------------------------------- + +/// `artist_releases::reset` (`artist_releases.rs:14-26`) — the header is seeded +/// and the grid enters its loading state BEFORE anything is fetched, which is +/// also the order `navigate_artist_releases` uses (`main.rs:3583-3591`: the view +/// flips first, so the page mounts already spinning). +fn reset(artist_id: &str, name: &str, release_type: &str) -> u64 { + with_state(|s| { + s.generation = s.generation.wrapping_add(1); + s.artist_id = artist_id.to_string(); + s.name = name.to_string(); + s.release_type = release_type.to_string(); + s.title = crate::artist_qt::release_type_title(release_type); + s.cards.clear(); + s.offset = 0; + s.has_more = false; + s.loading = true; + s.load_more_loading = false; + s.load_error = false; + s.sort_by = crate::artist_prefs::get_sort(release_type); + s.generation + }) +} + +/// "See discography" / the album page's "From the same artist" View all. +/// +/// The two guards are the reference's own: the whole view is mounted under +/// `!OfflineState.offline` (`shell/AppShell.slint:503-511`), and +/// `.on_open_releases` returns silently on an empty artist id +/// (`main.rs:15065`). Neither logs — an empty id means the caller has no artist +/// on screen, not that something failed. +pub fn open(artist_id: String, artist_name: String, release_type: String) { + if crate::offline_fwd::engine().status().is_offline() { + return; + } + if artist_id.is_empty() { + return; + } + let generation = reset(&artist_id, &artist_name, &release_type); + // The route is a TWO-FILE contract (nav_qt.rs:9-16): this records the id, + // AppShell.qml's Loader ternary mounts it. `QbzShell.navigateTo` is NOT + // usable — it carries no payload and clears LAST_DETAIL (main.rs:1607). + crate::nav_qt::record("artistreleases"); + publish(); + fetch(generation, artist_id, release_type, 0, true); +} + +/// Infinite-scroll tail (`main.rs:15098-15135`). Bails when a page is already +/// in flight or the server said there is nothing left; the first page's +/// `loading` counts as in-flight too (the .slint's own guard, +/// `ArtistReleasesView.slint:123-128`, tests all three). +pub fn load_more() { + let Some((generation, artist_id, release_type, offset)) = with_state(|s| { + if s.loading || s.load_more_loading || !s.has_more { + return None; + } + s.load_more_loading = true; + Some(( + s.generation, + s.artist_id.clone(), + s.release_type.clone(), + s.offset, + )) + }) else { + return; + }; + publish(); + fetch(generation, artist_id, release_type, offset, false); +} + +/// The sort picker (`main.rs:15143-15148`) — persist, re-order the LOADED set +/// in place, republish. NO fetch: none of the five keys has a server +/// equivalent, every `get_releases_grid` call in both trees passes the constant +/// `Some("release_date")`, and `sort_album_items` applies the picker locally. +/// +/// The store is `artist_prefs`, SHARED with the artist page's per-section +/// picker: sorting here also re-seats that one on the next visit, and vice +/// versa. That is the reference's design, not a coincidence — the pref is keyed +/// by release_type alone. +/// +/// 1:1 note carried over from `artist_qt::resort_section`: picking "Default" +/// after "A–Z" does NOT restore the server order until the page is reloaded, +/// because "default" is a no-op over the vector as it stands. The reference +/// behaves identically. +pub fn set_sort(sort: String) { + // Persist FIRST and OUTSIDE the lock — it is a read-modify-write of a file + // shared with the Slint build, and there is no reason to hold the page + // state across it. + let release_type = with_state(|s| s.release_type.clone()); + crate::artist_prefs::set_sort(&release_type, &sort); + with_state(|s| { + s.sort_by = sort; + let key = s.sort_by.clone(); + sort_cards(&mut s.cards, &key); + }); + publish(); +} + +/// The error state's Retry button (`main.rs:15158-15176`) — a full re-run of +/// the open body from the state's OWN id/name/release_type, and deliberately +/// WITHOUT `nav_qt::record`: the user is already standing on this entry, and +/// pushing a second one would make Back a no-op. +pub fn retry() { + let Some((artist_id, name, release_type)) = with_state(|s| { + if s.artist_id.is_empty() { + return None; + } + Some(( + s.artist_id.clone(), + s.name.clone(), + s.release_type.clone(), + )) + }) else { + return; + }; + if crate::offline_fwd::engine().status().is_offline() { + return; + } + let generation = reset(&artist_id, &name, &release_type); + publish(); + fetch(generation, artist_id, release_type, 0, true); +} + +/// Live language switch. The bucket title is a Rust-translated string sitting +/// INSIDE a JSON document, so `QbzSession.trRev` cannot re-translate it — the +/// same reason `myqbz_qt::republish_all` is called unconditionally from +/// `apply_language`. A no-op on an empty document (nothing has been opened). +/// +/// This is wired (main.rs `apply_language`), unlike +/// `label_qt::republish_for_language`, which is `#[allow(dead_code)]` and whose +/// own doc comment says "GLUE NEEDED: nothing calls this yet". +pub fn republish() { + let has_page = with_state(|s| { + if s.artist_id.is_empty() { + return false; + } + s.title = crate::artist_qt::release_type_title(&s.release_type); + true + }); + if has_page { + publish(); + } +} + +// --------------------------------------------------------------------------- +// Fetch +// --------------------------------------------------------------------------- + +/// One page. `first` distinguishes the two error arms the reference keeps +/// apart: the FIRST page failing raises the retryable error state +/// (`main.rs:3599-3605`), a later page failing only clears +/// `load_more_loading` and leaves what is already on screen alone +/// (`main.rs:15129`). +fn fetch(generation: u64, artist_id: String, release_type: String, offset: u32, first: bool) { + let runtime = crate::app(); + crate::spawn(async move { + let Ok(id) = artist_id.parse::() else { + log::warn!("[qbz-qt] artist releases: invalid artist id {artist_id}"); + fail(generation, first); + return; + }; + let resp = match runtime + .core() + .get_releases_grid( + id, + &release_type, + crate::artist_qt::RELEASE_PAGE_SIZE, + offset, + Some("release_date"), + ) + .await + { + Ok(resp) => resp, + Err(e) => { + log::warn!("[qbz-qt] artist releases page failed ({artist_id}/{release_type}): {e}"); + fail(generation, first); + return; + } + }; + + let returned = resp.items.len() as u32; + let server_has_more = resp.has_more; + let mut cards: Vec = resp.items.iter().filter_map(map_card).collect(); + let missing = crate::home_qt::attach_card_art(&mut cards); + + { + let mut guard = RELEASES.lock().unwrap(); + let Some(s) = guard.as_mut() else { + return; + }; + if s.generation != generation { + return; + } + // Dedupe by id against what is already loaded — Qobuz can repeat a + // row across pages, and the reference dedupes on the same axis + // (`artist_releases.rs:44-57`, its `seen` set). + let seen: HashSet = s.cards.iter().map(|c| c.id.clone()).collect(); + s.cards + .extend(cards.into_iter().filter(|c| !seen.contains(&c.id))); + s.offset = offset.saturating_add(returned); + // `&& returned > 0` on top of the server flag: with the raw flag a + // bucket that answers "has_more" with an empty page pins + // `load_more` in a loop against an offset that never advances + // (label_qt.rs:820 guards the same way). + s.has_more = server_has_more && returned > 0; + s.loading = false; + s.load_more_loading = false; + s.load_error = false; + // Re-sort the FULL set, not just the arrival: under a custom sort a + // page appended at the tail is simply in the wrong place + // (`artist_releases.rs:62` sorts everything after the append). + let sort = s.sort_by.clone(); + sort_cards(&mut s.cards, &sort); + } + publish(); + + if missing.is_empty() { + return; + } + // Second pass — covers that were not on disk yet. Same shape as + // `label_qt::fetch_releases`: download, re-attach over the WHOLE set + // (the page just landed sorted into the middle of it), republish. + crate::artwork_qt::download_missing(missing).await; + let mut all = { + let mut guard = RELEASES.lock().unwrap(); + let Some(s) = guard.as_mut() else { + return; + }; + if s.generation != generation { + return; + } + std::mem::take(&mut s.cards) + }; + let _ = crate::home_qt::attach_card_art(&mut all); + { + let mut guard = RELEASES.lock().unwrap(); + let Some(s) = guard.as_mut() else { + return; + }; + if s.generation != generation { + // The take above already emptied the state for a page the user + // has navigated away from; `reset` has since refilled it, so + // dropping `all` on the floor is correct. + return; + } + s.cards = all; + } + publish(); + }); +} + +/// The two error arms (see `fetch`). Generation-guarded like every other write. +fn fail(generation: u64, first: bool) { + let changed = with_state(|s| { + if s.generation != generation { + return false; + } + if first { + s.loading = false; + s.load_error = true; + s.has_more = false; + } else { + s.load_more_loading = false; + } + true + }); + if changed { + publish(); + } +} diff --git a/crates/qbz-qt/src/main.rs b/crates/qbz-qt/src/main.rs index 023b8813e..7a8651529 100644 --- a/crates/qbz-qt/src/main.rs +++ b/crates/qbz-qt/src/main.rs @@ -113,10 +113,24 @@ mod myqbz_builder_fetch_qt; mod artist_blacklist; mod reco_dismiss_qt; mod blacklist_qt; +// Artist page per-section release sort, persisted by release_type. Co-owns +// `/qbz/artist_ui.json` with the Slint's crates/qbz/src/artist_prefs.rs +// exactly the way library_prefs co-owns favorites_ui.json. +mod artist_prefs; +// The dedicated discography page — one release bucket of one artist, paged. +// Reached from the artist page's "See discography" and from the album page's +// "From the same artist" View all. A plain module, NOT a bridge: it publishes +// onto QbzArtist.artistReleasesJson (artist_bridge.rs) and records its own nav +// entry, the label_qt shape. +mod artist_releases_qt; // Shared in-app toast publisher (the port of qbz-slint-common's toast.rs). // A plain module, NOT a bridge — it publishes onto QbzShell.toastJson, and // `controls/QbzToast.qml` owns the auto-hide timer. mod toast_qt; +// Share links + the system clipboard (the port of crates/qbz/src/share.rs). +// A plain module, NOT a bridge — the artist header's ⋯ → Share reaches it +// through QbzArtist.share (artist_bridge.rs). +mod share_qt; mod nav_qt; mod browse_qt; mod cast_qt; @@ -860,6 +874,20 @@ pub(crate) fn load_release_section(artist_id: String, release_type: String, offs .await { Ok((cards, has_more)) => { + // The user may have opened ANOTHER artist while this page was + // in flight. `merge_release_page` already dropped the stash + // merge in that case (its id guard), but the signal carries no + // artist id, and ArtistView.qml keys its append overlay by + // release_type alone — emitting here would graft this artist's + // page onto the new artist's same-named bucket. Same test the + // merge used (artist_qt::stash_is_for), applied to the second + // leg of the same delivery. + if !artist_qt::stash_is_for(&artist_id) { + log::info!( + "[qbz-qt] dropping stale release page ({release_type}): artist changed" + ); + return; + } let json = serde_json::to_string(&cards).unwrap_or_else(|_| "[]".into()); artist_bridge::ui(move |mut b| { b.as_mut().release_section_ready( @@ -1668,6 +1696,11 @@ pub(crate) fn apply_language(code: String) { myqbz_qt::republish_all(); myqbz_detail_qt::republish(); myqbz_builder_qt::republish(); + // Same reason, same unconditional call: the discography page's header + // title is `artist_qt::release_type_title(...)`, a Rust-translated string + // living inside a JSON document, and LAST_DETAIL only ever latches "album" + // or "artist" — it cannot reach this page. No-op when nothing is open. + artist_releases_qt::republish(); let (view, id) = LAST_DETAIL.lock().unwrap().clone(); if !id.is_empty() { match view.as_str() { diff --git a/crates/qbz-qt/src/nav_qt.rs b/crates/qbz-qt/src/nav_qt.rs index fcbf6eb27..b714566ca 100644 --- a/crates/qbz-qt/src/nav_qt.rs +++ b/crates/qbz-qt/src/nav_qt.rs @@ -16,10 +16,10 @@ //! which is exactly why it is easy to miss. //! //! Ids in use: `home`, `library`, `local`, `localalbum`, `album`, `artist`, -//! `settings`, `search`, `playlist`, `discoverbrowse`, `playlistbrowse`, -//! `recentalbums`, `mostplayedalbums`, `label`, `labelreleases`, `mix`, -//! `mixtapes`, `collections`, `mixtapedetail`, `discobuilder`, `blacklist`, -//! `playlistmanager`. +//! `artistreleases`, `settings`, `search`, `playlist`, `discoverbrowse`, +//! `playlistbrowse`, `recentalbums`, `mostplayedalbums`, `label`, +//! `labelreleases`, `mix`, `mixtapes`, `collections`, `mixtapedetail`, +//! `discobuilder`, `blacklist`, `playlistmanager`. use std::sync::Mutex; diff --git a/scripts/qt-diag.sh b/scripts/qt-diag.sh new file mode 100755 index 000000000..06f4a4dee --- /dev/null +++ b/scripts/qt-diag.sh @@ -0,0 +1,132 @@ +#!/usr/bin/env bash +# QBZ Qt — RENDERING DIAGNOSTIC. Launches the app with Qt's scene-graph and RHI +# logging on, samples the process's CPU while you drive it, and leaves ONE log +# file you can copy off the machine. +# +# Why this exists: the whole "this port runs on the software renderer" doctrine +# — repeated across ~8 QML file headers and used to justify pre-baked icon +# tints, Canvas-based rounded covers and a blanket no-effects rule — came from +# an observation made OFFSCREEN, and QT_QPA_PLATFORM=offscreen forces software +# by definition. On Linux, QSG_INFO in a real window says otherwise: OpenGL RHI, +# threaded render loop, Intel Arc. macOS is still unmeasured. So: measure, in a +# REAL graphical session, never over ssh and never offscreen. +# +# WHAT TO DO WHILE IT RUNS (the log is only as good as the session): +# 1. Let it sit idle on Home for ~15s. +# 2. Play a track. Let it settle ~15s. +# 3. Switch the now-playing bar through its modes (New / Classic / Small / +# Large), pausing ~15s on each — Large is the one that mounts the FFT +# spectrum, which is the suspect. +# 4. Hover a few album cards. +# 5. Close the app normally. The summary is written on exit. +# +# Usage: ./scripts/qt-diag.sh # release binary, must already be built +# DEBUG=1 ./scripts/qt-diag.sh # debug binary instead +# INTERVAL=2 ./scripts/qt-diag.sh # CPU sample period in seconds (default 2) +set -uo pipefail +cd "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/.." + +UNAME_S="$(uname -s)" +PROFILE=$([[ "${DEBUG:-0}" == 1 ]] && echo debug || echo release) +BIN="crates/target/${PROFILE}/qbz-qt" +INTERVAL="${INTERVAL:-2}" + +[[ -x "${BIN}" ]] || { echo "[qt-diag] no binary at ${BIN} — build first (./scripts/qt-run.sh NORUN=1)" >&2; exit 1; } + +# Refuse to produce a log that answers the wrong question. Offscreen forces +# software, and a headless ssh session has no compositor to talk to — either +# would make the backend line meaningless, which is the one line we are here for. +if [[ "${QT_QPA_PLATFORM:-}" == offscreen ]]; then + echo "[qt-diag] QT_QPA_PLATFORM=offscreen forces the software backend — that is the mistake this script exists to correct. Unset it." >&2 + exit 1 +fi +if [[ "${UNAME_S}" != "Darwin" && -z "${DISPLAY:-}${WAYLAND_DISPLAY:-}" ]]; then + echo "[qt-diag] no DISPLAY / WAYLAND_DISPLAY — run this from the machine's own graphical session, not over ssh." >&2 + exit 1 +fi + +stamp="$(date +%Y%m%d-%H%M%S)" +host="$(hostname -s 2>/dev/null || hostname)" +LOG="${PWD}/qt-diag-${host}-${stamp}.log" + +{ + echo "==============================================================" + echo " QBZ Qt rendering diagnostic" + echo " host : ${host}" + echo " when : $(date '+%Y-%m-%d %H:%M:%S %Z')" + echo " os : $(uname -srm)" + [[ "${UNAME_S}" == "Darwin" ]] && echo " macos : $(sw_vers -productVersion)" + echo " binary : ${BIN} ($(date -r "${BIN}" '+%Y-%m-%d %H:%M' 2>/dev/null || stat -c %y "${BIN}" 2>/dev/null))" + echo " profile : ${PROFILE}" + echo "==============================================================" + echo + echo "---- display ----" + if [[ "${UNAME_S}" == "Darwin" ]]; then + system_profiler SPDisplaysDataType 2>/dev/null | grep -iE 'resolution|ui looks like|chipset|vram|metal' || true + else + xrandr 2>/dev/null | grep -E ' connected|\*' || echo "(xrandr unavailable — wayland?)" + echo "session type: ${XDG_SESSION_TYPE:-unknown}" + fi + echo + echo "---- qt ----" + (qmake6 -query QT_VERSION 2>/dev/null || /opt/homebrew/opt/qt/bin/qmake -query QT_VERSION 2>/dev/null || echo unknown) | sed 's/^/qt version: /' + echo "effects modules present:" + for d in /usr/lib64/qt6/qml /usr/lib/qt6/qml /opt/homebrew/share/qt/qml; do + [[ -d "$d/QtQuick/Effects" ]] && echo " QtQuick.Effects -> $d" + [[ -d "$d/Qt5Compat/GraphicalEffects" ]] && echo " Qt5Compat.GraphicalEffects -> $d" + done + echo "backend forced by env: QT_QUICK_BACKEND='${QT_QUICK_BACKEND:-}' QSG_RHI_BACKEND='${QSG_RHI_BACKEND:-}' QSG_RENDER_LOOP='${QSG_RENDER_LOOP:-}'" + echo + echo "---- app log (QSG_INFO + scenegraph/rhi categories) ----" +} > "${LOG}" + +echo "[qt-diag] log -> ${LOG}" +echo "[qt-diag] drive the app as described in this script's header, then close it normally." + +# The two lines that answer the question are 'Creating QRhi with backend X' and +# 'Prefer software device: N'. The rhi/scenegraph categories carry them. +QSG_INFO=1 \ +QT_LOGGING_RULES="qt.scenegraph.general=true;qt.rhi.general=true" \ +"./${BIN}" >> "${LOG}" 2>&1 & +app_pid=$! + +# CPU sampler. ps reports a per-process percentage on both platforms; on macOS +# it is a share of ONE core (so >100 is possible), same as Linux. +samples="$(mktemp)" +( + while kill -0 "${app_pid}" 2>/dev/null; do + cpu=$(ps -o %cpu= -p "${app_pid}" 2>/dev/null | tr -d ' ') + rss=$(ps -o rss= -p "${app_pid}" 2>/dev/null | tr -d ' ') + [[ -n "${cpu}" ]] && printf '%s %s %s\n' "$(date '+%H:%M:%S')" "${cpu}" "${rss:-0}" >> "${samples}" + sleep "${INTERVAL}" + done +) & +sampler=$! + +trap 'kill "${app_pid}" 2>/dev/null || true' INT TERM +wait "${app_pid}" 2>/dev/null +kill "${sampler}" 2>/dev/null || true +wait "${sampler}" 2>/dev/null || true + +{ + echo + echo "---- cpu samples (time, %cpu of one core, rss KB) ----" + cat "${samples}" 2>/dev/null + echo + echo "---- cpu summary ----" + awk '{n++; s+=$2; if($2>mx)mx=$2; if(n==1||$2/dev/null + echo + echo "---- what to look for ----" + echo " * 'Creating QRhi with backend ' -> OpenGL/Metal/Vulkan = GPU; 'software' = not." + echo " * 'Prefer software device: 1' -> something asked for software explicitly." + echo " * 'Animation Driver: using vsync' -> ms per frame; 5.56 = 180Hz, 10.0 = 100Hz." + echo " * a max% that tracks the NPB Large window is the FFT spectrum band." +} >> "${LOG}" +rm -f "${samples}" + +echo +echo "[qt-diag] done. Backend line:" +grep -aE 'Creating QRhi|Prefer software|using vsync' "${LOG}" | sed 's/^/ /' || echo " (none found — did the window open?)" +echo "[qt-diag] full log: ${LOG}"