mirror of
https://github.com/Psychotoxical/psysonic.git
synced 2026-07-21 23:05:46 +00:00
fix(cover): restore full-resolution album and artist covers (#1205)
* fix(cover): build cover tiers from the full-resolution source Derive the larger tiers from the decoded download instead of re-reading the just-written smaller tier (resize never upscales, so 512/800 were stored at the small resolution). Resolve the full-res 2000 tier exactly so it is actually downloaded and cached. Bump the cache layout stamp to drop already-poisoned tiers. * fix(cover): open the cover lightbox at full resolution Require the exact requested tier for the full-res helper so a smaller warmed tier no longer pins the lightbox to a downscaled image. Race the 2000 fetch against a 500ms opening window, show the 800 tier meanwhile, and persist 2000 for the next open. Adds an opening animation (reduced-motion aware) and tests. * docs(changelog): note full-resolution cover fix (#1205) * fix(cover): keep full-res peek exact in peek_batch and the grid seeder Review follow-up: ensure-path peek alone left a hole — cover_cache_peek_batch still laddered a 2000 request down to a smaller tier, and the in-memory grid seeder wrote that smaller path under the 2000 key, so Hero/fullscreen/lightbox surfaces (which peek 2000 before ensure) still showed a downscaled cover. Share one exact-2000 rule (peek_plain_cover_tier) across ensure and peek_batch, and never seed the full-res key from a smaller tier file.
This commit is contained in:
@@ -339,6 +339,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||||||
* History rows from other servers resolve album/cover metadata per server so Now Playing artwork loads when replaying cross-server plays.
|
* History rows from other servers resolve album/cover metadata per server so Now Playing artwork loads when replaying cross-server plays.
|
||||||
* Cross-server queue switches now send `playbackReport` **stopped** to the previous server so its Who is listening entry clears promptly.
|
* Cross-server queue switches now send `playbackReport` **stopped** to the previous server so its Who is listening entry clears promptly.
|
||||||
|
|
||||||
|
### Album and artist covers — full resolution restored
|
||||||
|
|
||||||
|
**By [@Psychotoxical](https://github.com/Psychotoxical), PR [#1205](https://github.com/Psychotoxical/psysonic/pull/1205)**
|
||||||
|
|
||||||
|
* Album and artist covers — and the full-size view when you click a cover — could appear small and low-quality even though the source image was large, depending on how you reached the album. Root cause: the cache built its larger sizes from a smaller already-saved size instead of the full-resolution download, so they were stored downscaled. Covers are now built from the full-resolution image, and the full-size view opens at full resolution. The cover cache refreshes once on update. Reported by users on Discord.
|
||||||
|
|
||||||
## Under the Hood
|
## Under the Hood
|
||||||
|
|
||||||
### ESLint setup and a strict lint pass over the frontend
|
### ESLint setup and a strict lint pass over the frontend
|
||||||
|
|||||||
@@ -17,7 +17,7 @@
|
|||||||
use std::path::{Path, PathBuf};
|
use std::path::{Path, PathBuf};
|
||||||
|
|
||||||
/// Written to `{cover_root}/.storage-layout` — mismatch triggers cache reset.
|
/// Written to `{cover_root}/.storage-layout` — mismatch triggers cache reset.
|
||||||
pub const LAYOUT_STAMP: &str = "canonical-segment-v4";
|
pub const LAYOUT_STAMP: &str = "canonical-segment-v5";
|
||||||
|
|
||||||
/// True for ids that are only valid as `getCoverArt` targets, not library entity keys.
|
/// True for ids that are only valid as `getCoverArt` targets, not library entity keys.
|
||||||
pub fn is_fetch_only_cover_id(id: &str) -> bool {
|
pub fn is_fetch_only_cover_id(id: &str) -> bool {
|
||||||
|
|||||||
@@ -59,6 +59,12 @@ pub struct CoverCacheEnsureResult {
|
|||||||
pub tier: u32,
|
pub tier: u32,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Result of the foreground tier-encode pass: whether the requested tier was
|
||||||
|
/// written, the freshly written `(tier, path)` pairs, and the full-resolution
|
||||||
|
/// decoded source kept for deriving the larger tiers (None on the bulk/quiet
|
||||||
|
/// path, which writes every tier up front).
|
||||||
|
type EncodeTiersOutcome = Result<(bool, Vec<(u32, PathBuf)>, Option<DynamicImage>), String>;
|
||||||
|
|
||||||
#[derive(Debug, Clone, Serialize)]
|
#[derive(Debug, Clone, Serialize)]
|
||||||
#[serde(rename_all = "camelCase")]
|
#[serde(rename_all = "camelCase")]
|
||||||
pub struct CoverCacheStatsDto {
|
pub struct CoverCacheStatsDto {
|
||||||
@@ -264,7 +270,7 @@ impl CoverCacheState {
|
|||||||
) -> Result<CoverCacheEnsureResult, String> {
|
) -> Result<CoverCacheEnsureResult, String> {
|
||||||
let this = state.lock().await;
|
let this = state.lock().await;
|
||||||
let dir = cover_dir_for_args(&this.root, args);
|
let dir = cover_dir_for_args(&this.root, args);
|
||||||
if let Some(path) = external_ensure::peek_cover_path(&dir, args.tier, args) {
|
if let Some(path) = ensure_peek(&dir, args.tier, args) {
|
||||||
return Ok(CoverCacheEnsureResult {
|
return Ok(CoverCacheEnsureResult {
|
||||||
hit: true,
|
hit: true,
|
||||||
path: path.to_string_lossy().into_owned(),
|
path: path.to_string_lossy().into_owned(),
|
||||||
@@ -362,7 +368,16 @@ impl CoverCacheState {
|
|||||||
Bytes(Vec<u8>),
|
Bytes(Vec<u8>),
|
||||||
}
|
}
|
||||||
|
|
||||||
let source = if let Some(img) = load_image_from_disk(&dir) {
|
// Full-res must come from the network: the largest on-disk derive tier is
|
||||||
|
// 800, so reusing a disk tier as the source would store a `2000.webp` that
|
||||||
|
// is only 800px (resize never upscales). Smaller tiers may reuse a disk
|
||||||
|
// source.
|
||||||
|
let disk_source = if args.tier >= 2000 {
|
||||||
|
None
|
||||||
|
} else {
|
||||||
|
load_image_from_disk(&dir)
|
||||||
|
};
|
||||||
|
let source = if let Some(img) = disk_source {
|
||||||
CoverSource::Image(img)
|
CoverSource::Image(img)
|
||||||
} else {
|
} else {
|
||||||
let http_registry = app
|
let http_registry = app
|
||||||
@@ -390,8 +405,8 @@ impl CoverCacheState {
|
|||||||
.acquire_owned()
|
.acquire_owned()
|
||||||
.await
|
.await
|
||||||
.map_err(|e| e.to_string())?;
|
.map_err(|e| e.to_string())?;
|
||||||
let (mut wrote_requested, fresh_tiers) = tauri::async_runtime::spawn_blocking(
|
let (mut wrote_requested, fresh_tiers, derive_source) = tauri::async_runtime::spawn_blocking(
|
||||||
move || -> Result<(bool, Vec<(u32, PathBuf)>), String> {
|
move || -> EncodeTiersOutcome {
|
||||||
let _cpu_permit = cpu_permit;
|
let _cpu_permit = cpu_permit;
|
||||||
let img = match source {
|
let img = match source {
|
||||||
CoverSource::Image(i) => i,
|
CoverSource::Image(i) => i,
|
||||||
@@ -403,7 +418,8 @@ impl CoverCacheState {
|
|||||||
if quiet {
|
if quiet {
|
||||||
disk::write_derived_webp_tiers(&dir_bg, &img, requested)?;
|
disk::write_derived_webp_tiers(&dir_bg, &img, requested)?;
|
||||||
wrote_requested = tier_exists(&dir_bg, requested).is_some();
|
wrote_requested = tier_exists(&dir_bg, requested).is_some();
|
||||||
} else {
|
return Ok((wrote_requested, fresh, None));
|
||||||
|
}
|
||||||
for tier in tiers_bg {
|
for tier in tiers_bg {
|
||||||
if tier_exists(&dir_bg, tier).is_some() {
|
if tier_exists(&dir_bg, tier).is_some() {
|
||||||
if tier == requested {
|
if tier == requested {
|
||||||
@@ -418,8 +434,13 @@ impl CoverCacheState {
|
|||||||
wrote_requested = true;
|
wrote_requested = true;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
// Hand the full-resolution decoded source back so the larger tiers
|
||||||
Ok((wrote_requested, fresh))
|
// are derived from it directly. Re-reading the largest tier off
|
||||||
|
// disk would pick the just-written `requested` tier (≤ requested);
|
||||||
|
// because `resize_tier` never upscales, deriving 512/800 from that
|
||||||
|
// smaller tier stored them at the small resolution — the
|
||||||
|
// "cover preview stays small" bug (e.g. an 800.webp that is 256×256).
|
||||||
|
Ok((wrote_requested, fresh, Some(img)))
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
@@ -442,7 +463,7 @@ impl CoverCacheState {
|
|||||||
// the Performance Probe "on-demand (ui)" throughput.
|
// the Performance Probe "on-demand (ui)" throughput.
|
||||||
note_ui_cover_produced(args);
|
note_ui_cover_produced(args);
|
||||||
if !quiet {
|
if !quiet {
|
||||||
if let Some(img) = load_image_from_disk(&dir) {
|
if let Some(img) = derive_source {
|
||||||
spawn_derive_remaining_tiers(
|
spawn_derive_remaining_tiers(
|
||||||
app.clone(),
|
app.clone(),
|
||||||
state.clone(),
|
state.clone(),
|
||||||
@@ -876,7 +897,9 @@ pub async fn cover_cache_peek_batch(
|
|||||||
&item.cache_kind,
|
&item.cache_kind,
|
||||||
&item.cache_entity_id,
|
&item.cache_entity_id,
|
||||||
);
|
);
|
||||||
let path = peek_tier_path(&dir, item.tier);
|
// Plain-cover peek (no surface in the batch DTO): full-res is exact-only,
|
||||||
|
// so a 2000 request never returns a smaller tier to seed the grid cache.
|
||||||
|
let path = peek_plain_cover_tier(&dir, item.tier);
|
||||||
if let Some(p) = path {
|
if let Some(p) = path {
|
||||||
out.insert(item.storage_key, p.to_string_lossy().into_owned());
|
out.insert(item.storage_key, p.to_string_lossy().into_owned());
|
||||||
}
|
}
|
||||||
@@ -909,6 +932,28 @@ fn peek_tier_path(dir: &Path, want: u32) -> Option<PathBuf> {
|
|||||||
None
|
None
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Disk peek for a plain (non-surface) cover request — shared by the ensure path
|
||||||
|
/// AND `cover_cache_peek_batch`. Full-res (≥2000) is **exact-only**: a smaller
|
||||||
|
/// peek-ladder fallback would both serve a downscaled image and short-circuit the
|
||||||
|
/// download, and (via the frontend grid seeder) poison the full-res in-memory key
|
||||||
|
/// for Hero / fullscreen / artist-hero surfaces, which peek 2000 before ensure.
|
||||||
|
/// Smaller display tiers keep the normal ladder peek.
|
||||||
|
fn peek_plain_cover_tier(dir: &Path, tier: u32) -> Option<PathBuf> {
|
||||||
|
if tier >= 2000 {
|
||||||
|
return tier_exists(dir, tier);
|
||||||
|
}
|
||||||
|
peek_tier_path(dir, tier)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Peek used by the ensure path. External surfaces keep their own ladder; plain
|
||||||
|
/// covers go through [`peek_plain_cover_tier`] (full-res exact).
|
||||||
|
fn ensure_peek(dir: &Path, tier: u32, args: &CoverCacheEnsureArgs) -> Option<PathBuf> {
|
||||||
|
if args.surface_kind.is_some() {
|
||||||
|
return external_ensure::peek_cover_path(dir, tier, args);
|
||||||
|
}
|
||||||
|
peek_plain_cover_tier(dir, tier)
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
#[tauri::command]
|
#[tauri::command]
|
||||||
pub async fn cover_cache_ensure(
|
pub async fn cover_cache_ensure(
|
||||||
@@ -1371,8 +1416,8 @@ mod tests {
|
|||||||
use super::decode_image_bytes;
|
use super::decode_image_bytes;
|
||||||
use super::disk::{cover_dir, tier_path};
|
use super::disk::{cover_dir, tier_path};
|
||||||
use super::{
|
use super::{
|
||||||
count_cached_cover_ids, is_safe_index_key, merge_cover_bucket, purge_external_files,
|
count_cached_cover_ids, ensure_peek, is_safe_index_key, merge_cover_bucket,
|
||||||
rename_bucket_inner,
|
peek_plain_cover_tier, purge_external_files, rename_bucket_inner, CoverCacheEnsureArgs,
|
||||||
};
|
};
|
||||||
use psysonic_core::cover_cache_layout::CANONICAL_PROGRESS_TIER;
|
use psysonic_core::cover_cache_layout::CANONICAL_PROGRESS_TIER;
|
||||||
use std::fs;
|
use std::fs;
|
||||||
@@ -1597,4 +1642,75 @@ mod tests {
|
|||||||
assert!(!entity.join(".miss-banner").exists());
|
assert!(!entity.join(".miss-banner").exists());
|
||||||
let _ = fs::remove_dir_all(&root);
|
let _ = fs::remove_dir_all(&root);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn test_ensure_args(tier: u32, surface_kind: Option<&str>) -> CoverCacheEnsureArgs {
|
||||||
|
CoverCacheEnsureArgs {
|
||||||
|
server_index_key: "srv".into(),
|
||||||
|
cache_kind: "album".into(),
|
||||||
|
cache_entity_id: "al-1".into(),
|
||||||
|
cover_art_id: "al-1".into(),
|
||||||
|
tier,
|
||||||
|
rest_base_url: "http://x".into(),
|
||||||
|
username: "u".into(),
|
||||||
|
password: "p".into(),
|
||||||
|
library_bulk: false,
|
||||||
|
library_server_id: None,
|
||||||
|
external_artwork_enabled: false,
|
||||||
|
surface_kind: surface_kind.map(String::from),
|
||||||
|
artist_name: None,
|
||||||
|
album_title: None,
|
||||||
|
external_artwork_byok: None,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn ensure_peek_fullres_requires_exact_tier() {
|
||||||
|
let root = fresh_tmpdir("ensure-peek-fullres");
|
||||||
|
let dir = root.join("album").join("al-1");
|
||||||
|
fs::create_dir_all(&dir).unwrap();
|
||||||
|
// A smaller tier is on disk (from a grid/hero load) but the full-res tier
|
||||||
|
// is not. A 2000 request must NOT accept the smaller fallback, or the
|
||||||
|
// download is skipped and the 2000 tier is never built/cached.
|
||||||
|
fs::write(tier_path(&dir, 512), b"x").unwrap();
|
||||||
|
let args = test_ensure_args(2000, None);
|
||||||
|
assert!(ensure_peek(&dir, 2000, &args).is_none());
|
||||||
|
// Once the exact tier exists it is a hit.
|
||||||
|
fs::write(tier_path(&dir, 2000), b"y").unwrap();
|
||||||
|
assert_eq!(ensure_peek(&dir, 2000, &args), Some(tier_path(&dir, 2000)));
|
||||||
|
let _ = fs::remove_dir_all(&root);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn peek_plain_cover_fullres_is_exact_but_display_tiers_ladder() {
|
||||||
|
let root = fresh_tmpdir("peek-plain-fullres");
|
||||||
|
let dir = root.join("album").join("al-1");
|
||||||
|
fs::create_dir_all(&dir).unwrap();
|
||||||
|
// Only a smaller tier on disk (grid/hero load). A full-res (2000) peek —
|
||||||
|
// used by both ensure AND cover_cache_peek_batch — must NOT ladder down to
|
||||||
|
// it, or the frontend grid seeder writes the 512 path under the 2000 key
|
||||||
|
// and Hero/fullscreen surfaces show a downscaled image.
|
||||||
|
fs::write(tier_path(&dir, 512), b"x").unwrap();
|
||||||
|
assert!(peek_plain_cover_tier(&dir, 2000).is_none());
|
||||||
|
// A display tier still ladders to a larger warmed tier.
|
||||||
|
assert_eq!(peek_plain_cover_tier(&dir, 256), Some(tier_path(&dir, 512)));
|
||||||
|
// Exact full-res is a hit.
|
||||||
|
fs::write(tier_path(&dir, 2000), b"y").unwrap();
|
||||||
|
assert_eq!(peek_plain_cover_tier(&dir, 2000), Some(tier_path(&dir, 2000)));
|
||||||
|
let _ = fs::remove_dir_all(&root);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn ensure_peek_display_tier_keeps_ladder_fallback() {
|
||||||
|
let root = fresh_tmpdir("ensure-peek-ladder");
|
||||||
|
let dir = root.join("album").join("al-1");
|
||||||
|
fs::create_dir_all(&dir).unwrap();
|
||||||
|
fs::write(tier_path(&dir, 800), b"x").unwrap();
|
||||||
|
// A 256 display request still accepts a larger warmed tier (grid behaviour
|
||||||
|
// is unchanged — only the full-res tier is exact-only).
|
||||||
|
assert_eq!(
|
||||||
|
ensure_peek(&dir, 256, &test_ensure_args(256, None)),
|
||||||
|
Some(tier_path(&dir, 800)),
|
||||||
|
);
|
||||||
|
let _ = fs::remove_dir_all(&root);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -55,3 +55,26 @@ describe('rememberDiskSrcLadder', () => {
|
|||||||
expect(keys).toContain('srv:cover:album:al-1:800');
|
expect(keys).toContain('srv:cover:album:al-1:800');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('full-res (2000) seed guard', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.mocked(rememberDiskSrc).mockClear();
|
||||||
|
vi.mocked(rememberDiskSrc).mockReturnValue('asset://x');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('never seeds the 2000 key from a smaller tier file', () => {
|
||||||
|
const ref = albumCoverRef('al-1', 'al-1');
|
||||||
|
rememberGridDiskSrc(ref, 2000, '/data/512.webp');
|
||||||
|
const keys = vi.mocked(rememberDiskSrc).mock.calls.map(c => c[0]);
|
||||||
|
expect(keys.some(k => k.endsWith(':2000'))).toBe(false);
|
||||||
|
// Smaller display keys are still seeded.
|
||||||
|
expect(keys.some(k => k.endsWith(':512'))).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('seeds the 2000 key from a real 2000 file', () => {
|
||||||
|
const ref = albumCoverRef('al-1', 'al-1');
|
||||||
|
rememberGridDiskSrc(ref, 2000, '/data/2000.webp');
|
||||||
|
const keys = vi.mocked(rememberDiskSrc).mock.calls.map(c => c[0]);
|
||||||
|
expect(keys.some(k => k.endsWith(':2000'))).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -3,6 +3,25 @@ import { hasCoverDiskReadyListeners, notifyCoverDiskReady } from './diskHandoff'
|
|||||||
import { coverStorageKeyFromRef } from './storageKeys';
|
import { coverStorageKeyFromRef } from './storageKeys';
|
||||||
import type { CoverArtRef, CoverArtTier } from './types';
|
import type { CoverArtRef, CoverArtTier } from './types';
|
||||||
|
|
||||||
|
/** Tier embedded in a cover file path (`…/512.webp`, `…/2000-fanart.webp`). */
|
||||||
|
function coverPathTier(fsPath: string): number | null {
|
||||||
|
const m = /(\d+)(?:-[a-z0-9]+)?\.webp$/i.exec(fsPath);
|
||||||
|
return m ? Number(m[1]) : null;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Never seed the full-res (≥2000) key from a smaller tier's file. The grid lookup
|
||||||
|
* order intentionally cross-seeds smaller display keys, but pinning a downscaled
|
||||||
|
* image under the 2000 key would make Hero / fullscreen / the lightbox show a
|
||||||
|
* small cover (they read the 2000 key before running ensure). Mirrors the Rust
|
||||||
|
* `peek_plain_cover_tier` exact-only rule for full-res.
|
||||||
|
*/
|
||||||
|
function skipFullResSeedTier(tier: CoverArtTier, fsPath: string): boolean {
|
||||||
|
if (tier < 2000) return false;
|
||||||
|
const src = coverPathTier(fsPath);
|
||||||
|
return src == null || src < 2000;
|
||||||
|
}
|
||||||
|
|
||||||
/** Dense grids: prefer a larger on-disk tier (800) before tiny thumbs when the ideal tier is missing. */
|
/** Dense grids: prefer a larger on-disk tier (800) before tiny thumbs when the ideal tier is missing. */
|
||||||
export function gridDiskSrcLookupOrder(want: CoverArtTier): CoverArtTier[] {
|
export function gridDiskSrcLookupOrder(want: CoverArtTier): CoverArtTier[] {
|
||||||
const out: CoverArtTier[] = [want];
|
const out: CoverArtTier[] = [want];
|
||||||
@@ -30,6 +49,7 @@ export function seedGridDiskSrcCache(ref: CoverArtRef, wantTier: CoverArtTier, f
|
|||||||
if (!fsPath) return false;
|
if (!fsPath) return false;
|
||||||
let hit = false;
|
let hit = false;
|
||||||
for (const tier of gridDiskSrcLookupOrder(wantTier)) {
|
for (const tier of gridDiskSrcLookupOrder(wantTier)) {
|
||||||
|
if (skipFullResSeedTier(tier, fsPath)) continue;
|
||||||
if (rememberDiskSrc(coverStorageKeyFromRef(ref, tier), fsPath)) hit = true;
|
if (rememberDiskSrc(coverStorageKeyFromRef(ref, tier), fsPath)) hit = true;
|
||||||
}
|
}
|
||||||
return hit;
|
return hit;
|
||||||
@@ -58,6 +78,7 @@ export function rememberDiskSrcLadder(
|
|||||||
if (!serverIndexKey || !ref.cacheEntityId || !fsPath) return false;
|
if (!serverIndexKey || !ref.cacheEntityId || !fsPath) return false;
|
||||||
let hit = false;
|
let hit = false;
|
||||||
for (const tier of gridDiskSrcLookupOrder(wantTier)) {
|
for (const tier of gridDiskSrcLookupOrder(wantTier)) {
|
||||||
|
if (skipFullResSeedTier(tier, fsPath)) continue;
|
||||||
const key = `${serverIndexKey}:cover:${ref.cacheKind}:${ref.cacheEntityId}:${tier}`;
|
const key = `${serverIndexKey}:cover:${ref.cacheKind}:${ref.cacheEntityId}:${tier}`;
|
||||||
if (rememberDiskSrc(key, fsPath)) hit = true;
|
if (rememberDiskSrc(key, fsPath)) hit = true;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,59 @@
|
|||||||
|
import { renderHook, act } from '@testing-library/react';
|
||||||
|
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||||
|
import { albumCoverRef } from './ref';
|
||||||
|
|
||||||
|
vi.mock('./resolveDisk', () => ({ ensureCoverTierDiskSrc: vi.fn() }));
|
||||||
|
vi.mock('./diskSrcLookup', () => ({ getDiskSrcForGrid: vi.fn(() => '') }));
|
||||||
|
vi.mock('./fetchUrl', () => ({ buildCoverArtFetchUrl: vi.fn(() => 'net://2000') }));
|
||||||
|
vi.mock('./imgSrc', () => ({ coverImgSrc: (s: string) => s }));
|
||||||
|
vi.mock('../components/CoverLightbox', () => ({ default: () => null }));
|
||||||
|
|
||||||
|
import { useCoverLightboxSrc } from './lightbox';
|
||||||
|
import { ensureCoverTierDiskSrc } from './resolveDisk';
|
||||||
|
import { getDiskSrcForGrid } from './diskSrcLookup';
|
||||||
|
|
||||||
|
const ref = albumCoverRef('al-1', 'al-1');
|
||||||
|
|
||||||
|
describe('useCoverLightboxSrc — full-res opening race', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
vi.mocked(ensureCoverTierDiskSrc).mockReset();
|
||||||
|
vi.mocked(getDiskSrcForGrid).mockReset().mockReturnValue('');
|
||||||
|
});
|
||||||
|
afterEach(() => {
|
||||||
|
vi.useRealTimers();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('shows full-res 2000 when the ensure resolves within the window', async () => {
|
||||||
|
vi.mocked(ensureCoverTierDiskSrc).mockResolvedValue('asset://2000');
|
||||||
|
const { result } = renderHook(() => useCoverLightboxSrc(ref));
|
||||||
|
act(() => result.current.open());
|
||||||
|
await act(async () => {
|
||||||
|
await vi.advanceTimersByTimeAsync(0);
|
||||||
|
});
|
||||||
|
expect(result.current.src).toBe('asset://2000');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('falls back to the warm 800 tier when 2000 is not ready within 500ms', async () => {
|
||||||
|
// 2000 ensure never resolves in time.
|
||||||
|
vi.mocked(ensureCoverTierDiskSrc).mockReturnValue(new Promise<string>(() => {}));
|
||||||
|
vi.mocked(getDiskSrcForGrid).mockReturnValue('asset://800');
|
||||||
|
const { result } = renderHook(() => useCoverLightboxSrc(ref));
|
||||||
|
act(() => result.current.open());
|
||||||
|
await act(async () => {
|
||||||
|
await vi.advanceTimersByTimeAsync(500);
|
||||||
|
});
|
||||||
|
expect(result.current.src).toBe('asset://800');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('falls back to the network full-res url when no warm tier is cached', async () => {
|
||||||
|
vi.mocked(ensureCoverTierDiskSrc).mockReturnValue(new Promise<string>(() => {}));
|
||||||
|
vi.mocked(getDiskSrcForGrid).mockReturnValue('');
|
||||||
|
const { result } = renderHook(() => useCoverLightboxSrc(ref));
|
||||||
|
act(() => result.current.open());
|
||||||
|
await act(async () => {
|
||||||
|
await vi.advanceTimersByTimeAsync(500);
|
||||||
|
});
|
||||||
|
expect(result.current.src).toBe('net://2000');
|
||||||
|
});
|
||||||
|
});
|
||||||
+18
-4
@@ -2,9 +2,13 @@ import { useCallback, useEffect, useState, type ReactNode } from 'react';
|
|||||||
import CoverLightbox from '../components/CoverLightbox';
|
import CoverLightbox from '../components/CoverLightbox';
|
||||||
import { buildCoverArtFetchUrl } from './fetchUrl';
|
import { buildCoverArtFetchUrl } from './fetchUrl';
|
||||||
import { coverImgSrc } from './imgSrc';
|
import { coverImgSrc } from './imgSrc';
|
||||||
|
import { getDiskSrcForGrid } from './diskSrcLookup';
|
||||||
import { ensureCoverTierDiskSrc } from './resolveDisk';
|
import { ensureCoverTierDiskSrc } from './resolveDisk';
|
||||||
import type { CoverArtRef } from './types';
|
import type { CoverArtRef } from './types';
|
||||||
|
|
||||||
|
/** Opening window: wait this long for the full-res 2000 tier before showing 800. */
|
||||||
|
const LIGHTBOX_FULLRES_WINDOW_MS = 500;
|
||||||
|
|
||||||
export function useCoverLightboxSrc(
|
export function useCoverLightboxSrc(
|
||||||
ref: CoverArtRef | null,
|
ref: CoverArtRef | null,
|
||||||
opts?: { alt?: string },
|
opts?: { alt?: string },
|
||||||
@@ -20,12 +24,22 @@ export function useCoverLightboxSrc(
|
|||||||
// eslint-disable-next-line react-hooks/set-state-in-effect
|
// eslint-disable-next-line react-hooks/set-state-in-effect
|
||||||
setLoading(true);
|
setLoading(true);
|
||||||
void (async () => {
|
void (async () => {
|
||||||
const diskSrc = await ensureCoverTierDiskSrc(ref, 2000);
|
// Kick the full-res (2000) ensure — Rust downloads + stores `2000.webp`. Do
|
||||||
|
// not block the open on it: race it against a short opening window. If the
|
||||||
|
// 2000 lands in time, show it; otherwise show the warm 800 tier now and let
|
||||||
|
// the 2000 finish + persist in the background, so the next open is full-res.
|
||||||
|
const fullSrc = ensureCoverTierDiskSrc(ref, 2000);
|
||||||
|
const winner = await Promise.race([
|
||||||
|
fullSrc,
|
||||||
|
new Promise<''>(resolve => {
|
||||||
|
setTimeout(() => resolve(''), LIGHTBOX_FULLRES_WINDOW_MS);
|
||||||
|
}),
|
||||||
|
]);
|
||||||
if (cancelled) return;
|
if (cancelled) return;
|
||||||
if (diskSrc) {
|
if (winner) {
|
||||||
setSrc(diskSrc);
|
setSrc(winner);
|
||||||
} else {
|
} else {
|
||||||
setSrc(buildCoverArtFetchUrl(ref, 2000));
|
setSrc(getDiskSrcForGrid(ref, 800) || buildCoverArtFetchUrl(ref, 2000));
|
||||||
}
|
}
|
||||||
setLoading(false);
|
setLoading(false);
|
||||||
})();
|
})();
|
||||||
|
|||||||
@@ -0,0 +1,52 @@
|
|||||||
|
import { describe, expect, it, vi, beforeEach } from 'vitest';
|
||||||
|
import { albumCoverRef } from './ref';
|
||||||
|
|
||||||
|
vi.mock('@tauri-apps/api/core', () => ({ isTauri: () => true }));
|
||||||
|
vi.mock('../api/coverCache', () => ({ coverCacheEnsure: vi.fn() }));
|
||||||
|
vi.mock('../utils/imageCache', () => ({ invalidateCacheKey: vi.fn() }));
|
||||||
|
vi.mock('./diskSrcCache', () => ({
|
||||||
|
getDiskSrc: vi.fn(() => ''),
|
||||||
|
rememberDiskSrc: vi.fn((_key: string, path: string) => `asset://${path}`),
|
||||||
|
}));
|
||||||
|
|
||||||
|
import { ensureCoverTierDiskSrc } from './resolveDisk';
|
||||||
|
import { coverCacheEnsure } from '../api/coverCache';
|
||||||
|
import { getDiskSrc, rememberDiskSrc } from './diskSrcCache';
|
||||||
|
|
||||||
|
const ref = albumCoverRef('al-1', 'al-1');
|
||||||
|
|
||||||
|
describe('ensureCoverTierDiskSrc — full-res exact-tier guard', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.mocked(coverCacheEnsure).mockReset();
|
||||||
|
vi.mocked(getDiskSrc).mockReset().mockReturnValue('');
|
||||||
|
vi.mocked(rememberDiskSrc).mockReset().mockImplementation((_k, p) => `asset://${p}`);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects a backend hit that served a smaller tier than requested', async () => {
|
||||||
|
// The Rust peek can report a hit with a smaller tier's file for a full-res
|
||||||
|
// request — the lightbox must treat that as a miss so it can fetch full-res.
|
||||||
|
vi.mocked(coverCacheEnsure).mockResolvedValue({
|
||||||
|
hit: true,
|
||||||
|
path: 'C:/cc/srv/album/al-1/512.webp',
|
||||||
|
tier: 2000,
|
||||||
|
});
|
||||||
|
expect(await ensureCoverTierDiskSrc(ref, 2000)).toBe('');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('accepts an exact-tier hit on either path separator', async () => {
|
||||||
|
vi.mocked(coverCacheEnsure).mockResolvedValue({
|
||||||
|
hit: true,
|
||||||
|
path: 'C:\\cc\\srv\\album\\al-1\\2000.webp',
|
||||||
|
tier: 2000,
|
||||||
|
});
|
||||||
|
expect(await ensureCoverTierDiskSrc(ref, 2000)).toBe(
|
||||||
|
'asset://C:\\cc\\srv\\album\\al-1\\2000.webp',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns an exact in-memory hit without calling the backend', async () => {
|
||||||
|
vi.mocked(getDiskSrc).mockReturnValue('asset://cached-2000');
|
||||||
|
expect(await ensureCoverTierDiskSrc(ref, 2000)).toBe('asset://cached-2000');
|
||||||
|
expect(coverCacheEnsure).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -1,13 +1,20 @@
|
|||||||
import { isTauri } from '@tauri-apps/api/core';
|
import { isTauri } from '@tauri-apps/api/core';
|
||||||
import { coverCacheEnsure } from '../api/coverCache';
|
import { coverCacheEnsure } from '../api/coverCache';
|
||||||
import { invalidateCacheKey } from '../utils/imageCache';
|
import { invalidateCacheKey } from '../utils/imageCache';
|
||||||
import { getDiskSrcForGrid } from './diskSrcLookup';
|
|
||||||
import { getDiskSrc, rememberDiskSrc } from './diskSrcCache';
|
import { getDiskSrc, rememberDiskSrc } from './diskSrcCache';
|
||||||
import { coverStorageKeyFromRef } from './storageKeys';
|
import { coverStorageKeyFromRef } from './storageKeys';
|
||||||
import type { CoverArtRef, CoverArtTier } from './types';
|
import type { CoverArtRef, CoverArtTier } from './types';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Full-res / lightbox — Rust WebP on disk (`cover-cache/…/2000.webp`), not IndexedDB.
|
* Full-res / lightbox — Rust WebP on disk (`cover-cache/…/2000.webp`), not IndexedDB.
|
||||||
|
*
|
||||||
|
* The exact requested tier is mandatory here: the grid disk-src ladder
|
||||||
|
* (`getDiskSrcForGrid`) and the Rust cover peek both fall back to a SMALLER
|
||||||
|
* already-warmed tier (e.g. a browsed 800) for an unmet request. That is correct
|
||||||
|
* for a grid cell, but for the lightbox it pins a downscaled image and never
|
||||||
|
* loads full-res — the "cover preview stays small after the first open" bug. So
|
||||||
|
* we only accept an exact-tier in-memory hit, and reject a backend hit whose path
|
||||||
|
* is a smaller tier, letting the caller fall back to the network full-res URL.
|
||||||
*/
|
*/
|
||||||
export async function ensureCoverTierDiskSrc(
|
export async function ensureCoverTierDiskSrc(
|
||||||
ref: CoverArtRef,
|
ref: CoverArtRef,
|
||||||
@@ -16,11 +23,12 @@ export async function ensureCoverTierDiskSrc(
|
|||||||
if (!ref.fetchCoverArtId || !isTauri()) return '';
|
if (!ref.fetchCoverArtId || !isTauri()) return '';
|
||||||
|
|
||||||
const storageKey = coverStorageKeyFromRef(ref, tier);
|
const storageKey = coverStorageKeyFromRef(ref, tier);
|
||||||
const cached = getDiskSrcForGrid(ref, tier) || getDiskSrc(storageKey);
|
const cached = getDiskSrc(storageKey);
|
||||||
if (cached) return cached;
|
if (cached) return cached;
|
||||||
|
|
||||||
const result = await coverCacheEnsure(ref, tier, 'high');
|
const result = await coverCacheEnsure(ref, tier, 'high');
|
||||||
if (!result.hit || !result.path) return '';
|
const exactTier = new RegExp(`[\\\\/]${tier}\\.webp$`).test(result.path);
|
||||||
|
if (!result.hit || !result.path || !exactTier) return '';
|
||||||
|
|
||||||
const src = rememberDiskSrc(storageKey, result.path);
|
const src = rememberDiskSrc(storageKey, result.path);
|
||||||
if (src) {
|
if (src) {
|
||||||
|
|||||||
@@ -11,6 +11,7 @@
|
|||||||
box-sizing: border-box;
|
box-sizing: border-box;
|
||||||
cursor: zoom-out;
|
cursor: zoom-out;
|
||||||
backdrop-filter: blur(6px);
|
backdrop-filter: blur(6px);
|
||||||
|
animation: cover-lightbox-overlay-in 180ms ease-out;
|
||||||
}
|
}
|
||||||
|
|
||||||
.cover-lightbox-img {
|
.cover-lightbox-img {
|
||||||
@@ -20,6 +21,24 @@
|
|||||||
border-radius: var(--radius-lg);
|
border-radius: var(--radius-lg);
|
||||||
box-shadow: 0 32px 80px rgba(0, 0, 0, 0.8);
|
box-shadow: 0 32px 80px rgba(0, 0, 0, 0.8);
|
||||||
cursor: default;
|
cursor: default;
|
||||||
|
animation: cover-lightbox-img-in 220ms cubic-bezier(0.2, 0, 0, 1);
|
||||||
|
}
|
||||||
|
|
||||||
|
@keyframes cover-lightbox-overlay-in {
|
||||||
|
from { opacity: 0; }
|
||||||
|
to { opacity: 1; }
|
||||||
|
}
|
||||||
|
|
||||||
|
@keyframes cover-lightbox-img-in {
|
||||||
|
from { opacity: 0; transform: scale(0.94); }
|
||||||
|
to { opacity: 1; transform: scale(1); }
|
||||||
|
}
|
||||||
|
|
||||||
|
@media (prefers-reduced-motion: reduce) {
|
||||||
|
.cover-lightbox-overlay,
|
||||||
|
.cover-lightbox-img {
|
||||||
|
animation: none;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
.cover-lightbox-close {
|
.cover-lightbox-close {
|
||||||
|
|||||||
Reference in New Issue
Block a user