perf(qt): stop republishing the Recommendations document for artwork
Covers that resolved after the document was published forced a second
`publish_snapshot`, and a republish hands QML a brand-new array:
`QQuickItemView::setModel()` then resets each rail's scroll offset and tears
down its QQmlDelegateModel. So every landing batch rebuilt all nine rails'
delegates, which is what a screen capture of a cold Recommendations load
shows — the rows visibly reassembling while covers trickle in, filling out of
order both between rows and within a row.
The fix is the pattern this port already applies twice: covers travel as a
url-keyed PATCH and the document is left alone. `foryou_qt::emit_foryou_art`
is the precedent for the For You rails, and `myqbz_bridge` cites it verbatim
for MyQBZ; the reco rails were the remaining holdout.
The download path is deliberately untouched. This changes only how a resolved
path is ANNOUNCED, never what is fetched or cached, so every other consumer of
the shared image cache — the now-playing bar, the queue, the `recent` rails —
sees exactly what it saw before.
Two traps this had to close, both already paid for once on the For You side:
* `sectionArtOf()` falls back to `item.artPath`. `recoArt` only ever holds
Recommendations urls, so for a Home / Editor's Picks / For You card the
lookup misses and the binding returns what it always returned. No surface
that reads a baked `artPath` can lose its cover to this — by construction,
not by care. The pinned rail, fed by its own store, is untouched.
* `anyItemArtPending` now consults the map. A patched cover never gets its
`artPath` back through a republish, so the bare "artUrl set and artPath
empty" predicate would be true forever and pin the 900ms skeleton pulse
Timer on.
`refreshRecoArt()` on mount is the third: HomeView is destroyed on every
navigation while this document is published once per session, so a fresh mount
must be re-handed what already resolved or the covers come back blank.
Weekly's two rails still republish their own narrower property; their blast
radius is two rails, not nine, and widening this change was not worth it.
Verified by owner smoke: the three checks (nothing else lost its covers, the
rails stop reassembling, covers survive navigating away and back) all pass.
cc
This commit is contained in:
3 files changed
+158
-11
No files matched your search
@@ -200,7 +200,49 @@ Rectangle {
|
||||
// never downloads, never republishes a document, and emits nothing at all
|
||||
// when nothing is resolved yet — so the first, cold mount is a no-op and the
|
||||
// in-flight download's own emit still fills the map.
|
||||
Component.onCompleted: QbzHome.refreshForyouArt()
|
||||
Component.onCompleted: { QbzHome.refreshForyouArt(); QbzHome.refreshRecoArt() }
|
||||
// Recommendations' rails used to get their covers by REPUBLISHING
|
||||
// `recoSectionsJson` once per landing batch, which hands `model:` a new JS
|
||||
// array — `QQuickItemView::setModel()` then resets the rail's scroll and
|
||||
// tears down its QQmlDelegateModel, so every batch rebuilt all nine rails'
|
||||
// delegates (the same defect the For You rails were moved off, :161). Rust
|
||||
// now emits a url-keyed patch instead and leaves the document alone.
|
||||
property var recoArt: ({})
|
||||
Connections {
|
||||
target: QbzHome
|
||||
function onRecoArtReady(patchJson) {
|
||||
var patch
|
||||
try {
|
||||
patch = JSON.parse(patchJson)
|
||||
} catch (e) {
|
||||
return
|
||||
}
|
||||
var m = root.recoArt
|
||||
var changed = false
|
||||
for (var url in patch) {
|
||||
if (m[url] !== patch[url]) {
|
||||
m[url] = patch[url]
|
||||
changed = true
|
||||
}
|
||||
}
|
||||
if (changed)
|
||||
root.recoArt = Object.assign({}, m) // rebind needs a NEW ref
|
||||
}
|
||||
}
|
||||
|
||||
/// Cover for a card that came out of a SECTIONS document: the patch map
|
||||
/// first, then whatever the document itself carried.
|
||||
///
|
||||
/// The fallback is what makes this safe to put on rails that render the
|
||||
/// other tabs too. `recoArt` only ever holds Recommendations urls, so for a
|
||||
/// Home / Editor's Picks / For You card the lookup misses and the result is
|
||||
/// `item.artPath` — byte for byte what the binding read before. No consumer
|
||||
/// of a baked `artPath` can lose its cover to this change.
|
||||
function sectionArtOf(item) {
|
||||
return (item && item.artUrl && root.recoArt[item.artUrl])
|
||||
|| (item ? (item.artPath || "") : "")
|
||||
}
|
||||
|
||||
/// The patch map first, then whatever the document happened to carry (a
|
||||
/// full Home reload publishes the paths it already has).
|
||||
function forYouArtOf(item) {
|
||||
@@ -295,9 +337,15 @@ Rectangle {
|
||||
}
|
||||
return false
|
||||
}
|
||||
// Consults the patch map, NOT a bare `artPath === ""`. A Recommendations
|
||||
// cover that arrives as a patch never gets its `artPath` back through a
|
||||
// republish any more, so the bare predicate would stay true forever and
|
||||
// pin the 900ms skeleton pulse Timer on — the exact trap the For You
|
||||
// rails hit (see anyForYouArtPending). Unchanged for every other tab,
|
||||
// where `sectionArtOf` just returns `artPath`.
|
||||
function anyItemArtPending(items) {
|
||||
for (var i = 0; i < items.length; i++) {
|
||||
if ((items[i].artUrl || "") !== "" && (items[i].artPath || "") === "")
|
||||
if ((items[i].artUrl || "") !== "" && root.sectionArtOf(items[i]) === "")
|
||||
return true
|
||||
}
|
||||
return false
|
||||
@@ -425,7 +473,7 @@ Rectangle {
|
||||
qualityTier: modelData.qualityTier
|
||||
ribbon: modelData.ribbon
|
||||
ribbonKind: modelData.ribbonKind
|
||||
artSource: modelData.artPath
|
||||
artSource: root.sectionArtOf(modelData)
|
||||
isPinned: modelData.isPinned
|
||||
// Snapshot url the pin payload persists (artPath is
|
||||
// a local cache path — see AlbumCard.artworkUrl).
|
||||
@@ -455,7 +503,7 @@ Rectangle {
|
||||
width: 200
|
||||
height: 200
|
||||
pending: (modelData.artUrl || "") !== ""
|
||||
coverSource: modelData.artPath || ""
|
||||
coverSource: root.sectionArtOf(modelData)
|
||||
phase: root.skelPhase
|
||||
cellIndex: index
|
||||
// A cover whose download fails republishes the
|
||||
@@ -529,7 +577,7 @@ Rectangle {
|
||||
clip: true
|
||||
RoundedImage {
|
||||
anchors.fill: parent
|
||||
source: card.artPath || ""
|
||||
source: root.sectionArtOf(card)
|
||||
radius: 4
|
||||
}
|
||||
}
|
||||
@@ -1184,7 +1232,7 @@ Rectangle {
|
||||
model: plRail.sectionData.items
|
||||
delegate: PlaylistCard {
|
||||
item: modelData
|
||||
artSource: modelData.artPath || ""
|
||||
artSource: root.sectionArtOf(modelData)
|
||||
// The card's own default is `false`; the
|
||||
// pin state travels on the row (home_qt
|
||||
// `map_playlist`), so it has to be handed
|
||||
@@ -1236,7 +1284,7 @@ Rectangle {
|
||||
model: arRail.sectionData.items
|
||||
delegate: ArtistCard {
|
||||
item: modelData
|
||||
artSource: modelData.artPath || ""
|
||||
artSource: root.sectionArtOf(modelData)
|
||||
// Same hand-over as the playlist rail —
|
||||
// home_qt `map_fav_artist` publishes the
|
||||
// pin state and the card defaults to false.
|
||||
|
||||
@@ -245,6 +245,14 @@ pub mod qbz_home {
|
||||
#[qinvokable]
|
||||
fn refresh_foryou_art(self: Pin<&mut QbzHome>);
|
||||
|
||||
/// Twin of `refresh_foryou_art` for the Recommendations rails. HomeView
|
||||
/// is destroyed on every navigation and this document is published once
|
||||
/// per session, so a fresh mount must be re-handed the covers that
|
||||
/// arrived as a patch — otherwise they come back blank and the pending
|
||||
/// probe never goes false, pinning the skeleton pulse Timer on.
|
||||
#[qinvokable]
|
||||
fn refresh_reco_art(self: Pin<&mut QbzHome>);
|
||||
|
||||
// --- Label releases sub-view --------------------------------------
|
||||
#[qinvokable]
|
||||
fn label_releases_load_more(self: Pin<&mut QbzHome>);
|
||||
@@ -304,6 +312,14 @@ pub mod qbz_home {
|
||||
/// `foryouArtReady`, i.e. the handler is `onForyouArtReady(patchJson)`.
|
||||
#[qsignal]
|
||||
fn foryou_art_ready(self: Pin<&mut QbzHome>, patch_json: QString);
|
||||
|
||||
/// Same contract as `foryouArtReady`, for the Recommendations tab's
|
||||
/// rails: covers resolved AFTER the document was published travel as a
|
||||
/// url-keyed patch instead of forcing a republish, which used to tear
|
||||
/// down every rail's delegate model on each landing batch. QML spelling
|
||||
/// is `recoArtReady`, so the handler is `onRecoArtReady(patchJson)`.
|
||||
#[qsignal]
|
||||
fn reco_art_ready(self: Pin<&mut QbzHome>, patch_json: QString);
|
||||
}
|
||||
|
||||
impl cxx_qt::Threading for QbzHome {}
|
||||
@@ -616,6 +632,18 @@ impl qbz_home::QbzHome {
|
||||
.foryou_art_ready(QString::from(patch.as_str()));
|
||||
}
|
||||
|
||||
/// Emitted SYNCHRONOUSLY for the same reason as `refresh_foryou_art`: this
|
||||
/// runs on the Qt thread from HomeView's `Component.onCompleted`, and the
|
||||
/// `ui()` hop no-ops until `boot()` has registered the thread.
|
||||
pub fn refresh_reco_art(mut self: Pin<&mut Self>) {
|
||||
let patch = crate::recommendations_qt::resolved_art_patch();
|
||||
if patch.is_empty() {
|
||||
return;
|
||||
}
|
||||
self.as_mut()
|
||||
.reco_art_ready(QString::from(patch.as_str()));
|
||||
}
|
||||
|
||||
// --- Label releases ----------------------------------------------------
|
||||
|
||||
pub fn label_releases_load_more(self: Pin<&mut Self>) {
|
||||
|
||||
@@ -48,7 +48,7 @@
|
||||
//! from artists you know" row exactly as the reference does with its larger
|
||||
//! played-or-favorited set.
|
||||
|
||||
use std::collections::HashSet;
|
||||
use std::collections::{BTreeMap, HashSet};
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
use std::sync::Mutex;
|
||||
|
||||
@@ -340,13 +340,84 @@ fn publish_snapshot(download: bool) {
|
||||
if download && !missing.is_empty() {
|
||||
crate::spawn(async move {
|
||||
crate::artwork_qt::download_missing(missing).await;
|
||||
// download=false: the covers are on disk now, so this pass can
|
||||
// only attach — it can never spawn another download round.
|
||||
publish_snapshot(false);
|
||||
// Was a second `publish_snapshot(false)`. Republishing for artwork
|
||||
// hands QML a brand-new array, and `QQuickItemView::setModel()`
|
||||
// then resets each rail's scroll offset and tears down its
|
||||
// QQmlDelegateModel — see the note at line 273 and the identical
|
||||
// reasoning in `HomeView.qml:161`. Every landing batch therefore
|
||||
// rebuilt all nine rails' delegates. The covers now travel as a
|
||||
// url-keyed PATCH, exactly as `foryou_qt::emit_foryou_art` does
|
||||
// for the For You rails and `myqbz_bridge` does for MyQBZ; the
|
||||
// document is left alone.
|
||||
//
|
||||
// The download itself is untouched: this changes only how a
|
||||
// resolved path is ANNOUNCED, never what gets fetched or cached,
|
||||
// so every other consumer of the shared image cache (the now-
|
||||
// playing bar, the queue, the `recent` rails) sees exactly what it
|
||||
// saw before.
|
||||
emit_reco_art(art_patch());
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/// Every (art_url -> art_path) this tab's rows can resolve from the disk cache
|
||||
/// right now. Attaches into a THROWAWAY copy of the sections, so it can never
|
||||
/// mutate or republish the live document, and never downloads.
|
||||
fn art_patch() -> BTreeMap<String, String> {
|
||||
let rows = rows_snapshot();
|
||||
let mut secs = sections(&rows);
|
||||
let _ = crate::artwork_qt::attach_cached(&mut secs);
|
||||
let mut patch: BTreeMap<String, String> = BTreeMap::new();
|
||||
for section in secs.iter() {
|
||||
for card in section.items.iter() {
|
||||
if !card.art_url.is_empty() && !card.art_path.is_empty() {
|
||||
patch.insert(card.art_url.clone(), card.art_path.clone());
|
||||
}
|
||||
}
|
||||
}
|
||||
patch
|
||||
}
|
||||
|
||||
/// ONE emit per batch — the receiving QML map is a `var`, so it only notifies
|
||||
/// on a new object reference, and an emit per url would copy the growing map
|
||||
/// and invalidate every cover binding once per cover. Nothing paints later for
|
||||
/// it: `download_missing().await` settles the whole batch first.
|
||||
fn emit_reco_art(patch: BTreeMap<String, String>) {
|
||||
if patch.is_empty() {
|
||||
return;
|
||||
}
|
||||
let json = serde_json::to_string(&patch).unwrap_or_else(|_| "{}".to_string());
|
||||
crate::home_bridge::ui(move |mut b| {
|
||||
b.as_mut().reco_art_ready(QString::from(json.as_str()));
|
||||
});
|
||||
}
|
||||
|
||||
/// Re-hand the resolved covers to a freshly mounted HomeView. HomeView is
|
||||
/// destroyed and rebuilt on every navigation while this document is published
|
||||
/// once per session, so without this the covers that arrived as a PATCH (i.e.
|
||||
/// everything that was not already on disk at publish time) would come back
|
||||
/// blank for the rest of the session — and the "still pending" probe would
|
||||
/// stay true forever and pin the skeleton pulse Timer on. The exact trap
|
||||
/// `foryou_qt::resolved_art_patch` exists to close.
|
||||
///
|
||||
/// Never downloads, never republishes; returns "" when nothing is resolved, so
|
||||
/// a cold first mount emits nothing and cannot clear a map an in-flight
|
||||
/// download is about to fill.
|
||||
pub(crate) fn resolved_art_patch() -> String {
|
||||
if !LOADED.load(Ordering::SeqCst) {
|
||||
return String::new();
|
||||
}
|
||||
let patch = art_patch();
|
||||
if patch.is_empty() {
|
||||
return String::new();
|
||||
}
|
||||
log::debug!(
|
||||
"[qbz-qt] reco art re-handed to a fresh HomeView: {} covers",
|
||||
patch.len()
|
||||
);
|
||||
serde_json::to_string(&patch).unwrap_or_else(|_| "{}".to_string())
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Entry points
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
Reference in new issue
Block a user