Skip to content

perf(images): request TMDB backdrops at a sane size - #65

Merged
Thunderhawkk merged 6 commits into
Harborofficial:beta-branchfrom
kalashnikxvxiii:fix/backdrop-size
Nov 6, 2026
Merged

Thunderhawkk merged 6 commits into
Harborofficial:beta-branchfrom
kalashnikxvxiii:fix/backdrop-size

Conversation

@kalashnikxvxiii

@kalashnikxvxiii kalashnikxvxiii commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The detail hero and the stream picker asked TMDB for /t/p/original/, which returns whatever the uploader pushed — backdrops at 7922x4594 live in my WebKit cache from those two code paths. They are drawn into a ~1900x1000 window, and the picker blurs its copy to 40px.

Both call sites now go through one helper, src/lib/tmdb-backdrop-size.ts, and pick a size instead of always asking for the master:

path size requested
detail hero, heroFullQuality on (default) original
detail hero, heroFullQuality off w1280
picker, backdrop blur on (default) w780
picker, blur off + heroFullQuality on original
picker, blur off + heroFullQuality off w1280

The blurred picker layer is capped at w780 regardless of heroFullQuality: at a 40px blur the master is not distinguishable from w780, so there is no quality choice to respect there.

No markup or behavior changes.

Measurements, and what they apply to

The numbers below come from the first revision of this branch, which requested w1280 for the detail hero unconditionally. In today's implementation that is the heroFullQuality = off path. Linux/NVIDIA, 1 Hz sampler over a fixed tour (home, scroll, detail page, scroll, source picker, scroll, music, scroll, back to home):

original everywhere (before) w1280 hero + w780 blurred picker
peak VRAM 3296 MiB 817 MiB
at end of tour 3143 MiB 453 MiB
largest single jump +1337 MiB (opening the picker) +149 MiB

RSS was flat at 1450-1650 MiB in both runs, so this was purely GPU-side. The home page by itself never moved off ~400 MiB even while scrolling; the entire climb came from those two navigations, and before the change the curve only ratcheted up — a revisit to home read 743 MiB against 216 MiB on its first visit.

What this means for the shipped defaults, to be explicit rather than to imply the table carries over:

  • heroFullQuality defaults to on, so the detail hero still requests original out of the box. That part of the reduction applies only once the toggle is off.
  • streamBackdropBlur defaults to on, so the picker is capped at w780 by default. The largest single jump in the table above (+1337 MiB, opening the picker) is that path, and it is covered without changing any setting.

Re-run on the current revision

Same machine, same sampler, same tour, both runs on a release build of e37252de (tauri:build:linux-system) and the same title (Cyberpunk: Edgerunners). Per-process sampling over Harbor's whole process tree, so the rest of the desktop is not counted.

defaults (heroFullQuality on) heroFullQuality off
home at rest 266 MiB 393 MiB *
peak 972 MiB 775 MiB
at end of tour 550 MiB 488 MiB
largest single jump +191 MiB +177 MiB
RSS range 1186-2472 MiB 1252-1715 MiB

* the second run's idle figure is high because the hero art was already resident from the trip into Settings to flip the toggle; it is not a like-for-like baseline.

In both runs the peak lands in the picker phase, and in both the curve falls back afterwards instead of ratcheting: the end-of-tour figure is below the peak, which is the behavior the old original-everywhere build never showed. Turning the toggle off takes another ~200 MiB off the peak, which is the detail hero dropping from the master file to w1280.

So the shape of the original finding holds — both configurations sit far below the 3296 MiB peak and 3143 MiB tail of the unpatched build — but the bulk of it is already there at the defaults, and the toggle is worth roughly a fifth of the peak on top.

Caveats, since the tour is driven by hand: the first run took 510s and the second 341s, so the two are not step-for-step identical in dwell time, and the absolute numbers move with which titles the catalogs happen to serve.

w1280 is the bucket cinema-hero.tsx, peek-hero.tsx and critics-pick/utils.ts already downgrade original to, and hero.tsx reaches for original only above a 2000px-tall screen with heroFullQuality on. These two call sites were the outliers in that they never looked at the setting at all.

Verification

  • pnpm run typecheck — clean.
  • vp check on the changed files — clean. (The repo-wide pnpm run check fails while parsing src-tauri/vendor/librqbit/webui/vite.config.ts, which this branch does not touch.)
  • tests/tmdb-backdrop-quality.test.ts — 5 tests over the helper, both components rendered at every blur/quality combination, and the detail wiring. Passing.
  • Built with pnpm run tauri:build:linux-system and ran the tour twice, patched and unpatched, against the same profile and the same titles, for the first table; re-ran it on the current revision, once at the defaults and once with heroFullQuality off, for the second.

Platform Impact

Measured on Linux (Wayland/Hyprland, NVIDIA, WebKitGTK 2.24.5). The code path only changes a URL, so Windows and macOS behave the same. TV build unaffected; big-picture keeps its own fullQuality handling.

UI Changes

None at the defaults: the detail hero still loads original while heroFullQuality is on, and the picker layer is blurred to 40px where w780 and the master are indistinguishable.

With heroFullQuality off, the detail hero is softer on displays above ~1280px of hero width — which is what that toggle is for. Its description in library-panel/home-tab.tsx now says it covers the detail hero and the unblurred picker background, and that the cost is GPU memory as well as bandwidth.

Checklist

  • This pull request is focused and contains no unrelated refactors.
  • I ran vp check for the changed files.
  • I ran vp run typecheck after TypeScript changes.
  • I ran the relevant Cargo checks after Rust changes.
  • I tested affected platforms when the change is platform-specific.
  • I preserved playback, navigation, and configured hotkey behavior where applicable.
  • I added or updated tests for behavior changes.
  • I removed secrets, tokens, private URLs, and personal data from logs and screenshots.

The detail hero and the stream picker both asked for /t/p/original/,
which is whatever the uploader pushed: this machine's cache holds
7922x4594 and 6480x3840 backdrops. They are drawn into a ~1900x1000
window, and the picker blurs its copy to 40px, so WebKit decodes
~36 Mpx and keeps the filter surfaces alive.

Measured on Linux/NVIDIA over the same tour: opening a detail page
cost +877 MiB of VRAM and opening the source picker +1337 MiB, with
the session settling at ~3150 MiB. After the change the run peaks at
817 MiB and falls back to 453.

w1280 is the bucket cinema-hero, peek-hero and critics-pick already
downgrade to.
CI runs vp fmt --check on changed files and vp does not pick up
.oxfmtrc.json ("No config found, using defaults"), so it rewraps two
spots that were already there before this branch. Keeping it in its
own commit so the actual fix stays a four line diff.
@kalashnikxvxiii

Copy link
Copy Markdown
Contributor Author

The failing Frontend check here is not from this change.

The Test step fails on tests/watched-sync-western-cartoon.test.ts, which asserts on the source text of src/lib/anime-detect.ts — a file this branch does not touch (git diff --name-only beta-branch... lists four files, all under src/views and src/lib/use-stable-asset.ts). The assertion went stale in 40390495, when the Kitsu fallback gate moved into a needsMapping const. It fails on beta-branch on its own; #66 fixes that one.

More broadly, pnpm test on beta-branch at de076024 reports 123 failures out of 6997 with nothing applied, so the Test job is red for every PR open right now, not just this one. The Check changed files step is green here after the second commit, which only rewraps two spots the default vp formatter dislikes — vp reports "No config found, using defaults" in CI, so it does not pick up .oxfmtrc.json, and it flags pre-existing formatting in any file a PR happens to touch.

@Thunderhawkk

Copy link
Copy Markdown
Contributor

Thanks for the updates. I re-reviewed PR #65 at 750ee6e3, and the quality-aware image handling looks good. The previous issue where the stream picker's low-resolution placeholder could still load an original-resolution TMDB image has been fixed.

All 35 focused tests passed locally, including the new regression tests. The latest CI run also passed TypeScript and the changed-file checks.

There are two remaining points I'd like addressed:

  1. Update the Full quality hero image setting description.

    In src/views/settings/library-panel/home-tab.tsx, the description currently says:

    Load the highest-resolution artwork for the featured hero. Uses more bandwidth.

    With this PR, heroFullQuality also controls detail-page hero artwork and unblurred stream-picker backgrounds. The description should reflect this expanded scope and mention the potential GPU memory usage, not just bandwidth.

  2. Clarify the performance results in the PR description.

    The original VRAM measurements were collected when the detail hero always requested w1280. The latest implementation now respects heroFullQuality, which is enabled by default.

    This means detail-page heroes will still request original-resolution TMDB images under the default configuration. The stream picker remains optimized when backdrop blur is enabled, but the overall VRAM reductions previously reported may not apply to the current defaults.

    Please update the PR description to clarify which settings the measurements apply to. If possible, benchmark the revised implementation with the default settings and with full-quality hero images disabled.

Other than these points, I haven't found any additional issues with the remote PR changes. The remaining frontend CI failures appear consistent with the existing test-suite failures and are unrelated to the new backdrop tests.

The toggle now also gates the detail hero and the unblurred picker
backdrop, and the cost is GPU memory as much as bandwidth.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@kalashnikxvxiii

Copy link
Copy Markdown
Contributor Author

Both points are in at e37252d, and the re-benchmark is in the PR description.

  1. The toggle description now reads "Load the highest-resolution artwork for the featured hero, the detail page hero, and unblurred stream picker backgrounds. Uses more bandwidth and video memory.", with the source key in i18n/en.json moved along with it.
  2. Re-ran the same tour on a release build of this revision, per-process VRAM sampling over Harbor's process tree, same title in both runs: defaults peak 972 MiB / 550 MiB at the end of the tour, heroFullQuality off peak 775 MiB / 488 MiB. Against the unpatched 3296 MiB peak and 3143 MiB tail, the bulk of the reduction is already there at the defaults - the blurred picker is capped at w780 because backdrop blur is on by default - and the toggle is worth about another 200 MiB of peak on top. In both runs the curve now falls back after the picker instead of ratcheting. The tour is hand-driven, so dwell times differ between the two runs (510s vs 341s); the old table is still in the description, labelled as what it actually measured.

@comet776

comet776 commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Reviewed PR #65 at e37252de and also tested it merged onto the current beta-branch.

I found two remaining issues:

  1. [P2] tmdbBackdropAtSize() can rewrite non-TMDB URLs.
    The helper only checks for /t/p/<size>/ in the path and does not verify the hostname. A URL like https://cdn.example/t/p/w500/image.jpg would be incorrectly rewritten. Since detail artwork can come from sources other than TMDB, this should be gated to confirmed TMDB URLs. The existing “other host” test does not catch this because its test URL has no /t/p/.../ path.

  2. [P3] The updated setting description loses localization.
    The source string was changed, but the existing locale catalogs still use the old key. Those languages now fall back to English for this setting description.

Aside from those, the implementation looks good. The backdrop sizing behavior is consistent, the focused tests pass, and the PR merges cleanly onto the current beta-branch.

Requesting changes for the non-TMDB URL rewriting issue. I’d also fix the translation regression before merge.

tmdbBackdropAtSize() matched the /t/p/<size>/ path alone, so any other
host serving that shape got rewritten too. Match on the hostname as
well, the way artworkIdentity() and largestArtwork() already do.
The source string changed, but the catalogs kept the old key, so every
translated locale fell back to English for this setting. Re-keyed with
the wider scope and the video-memory note; i18n/*.json matches what
scripts/i18n-crowdin.mjs export produces.
@kalashnikxvxiii

Copy link
Copy Markdown
Contributor Author

Both fixed - the host check especially, that one would have hit real artwork since detail backdrops don't all come from TMDB.

tmdbBackdropAtSize() now parses the URL and bails unless the hostname is image.tmdb.org, with the parse wrapped so an unparsable value comes back untouched. Same shape as artworkIdentity() in poster-backdrop-expansion.ts and largestArtwork() in music/artwork-save.ts, which already gate on the host. Your exact case and a relative /t/p/w500/... path are both in the test now, and it drops to 4/5 if I put the ungated replace back.

On the host list: image.tmdb.org is hardcoded in tmdb-client.ts and there's no mirror or proxy setting anywhere, so an exact match doesn't cost any real URL the resize.

You were right about the catalogs, and for a better reason than I realised when I left them to Crowdin: i18n/*.json is generated from the TS catalogs by scripts/i18n-crowdin.mjs export, not the other way round, so leaving the old key in src/lib/i18n/locales/ meant Crowdin would never see the new one. All 15 languages are re-keyed with the wider scope and the video-memory note, and the exported JSON matches. id had the key in two files; audit.ts is the one that wins in the barrel, so that's the value the export carries.

One sibling worth a look, outside this PR: rewriteWideArtworkRung() in poster-backdrop-expansion.ts does the same unguarded /t/p/[^/]+/ replace, and expanding-card-artwork.ts feeds it card artwork from mixed sources. Happy to do it here if you'd rather keep the two consistent, otherwise I'll open a separate one.

I left the formatter alone on the catalog files - running it there rewraps a few hundred pre-existing lines that have nothing to do with this change, so the diff is four lines per file. Checks: typecheck clean, focused tests 5/5, check-i18n identical to the branch baseline, check:search clean, check:settings at its usual 11 pre-existing failures.

@Thunderhawkk
Thunderhawkk merged commit 5361c5c into Harborofficial:beta-branch Nov 6, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants