perf(wallpapers): cap the bundled backgrounds at 3840px - #542
Merged
Conversation
They shipped at up to 7680x7680 for an app whose largest output is 4K. That is resolution nobody can see, paid for three times: on disk, in decode time, and in VRAM for as long as the texture stays resident. 17 of 18 resized to 3840 on the long edge, Lanczos, quality 92. Measured: | | before | after | |---|---|---| | on disk | 23.7 MB | 13.9 MB | | all 18 resident | 1774 MB | 772 MB | | a typical two-wallpaper session | 450 MB | 112 MB | | decoding all 18 | 3.31 s | 1.43 s | 3840 rather than something smaller because it is exactly 1:1 for a 4K output — below it, a 4K export would sample a background softer than its own frame. At 1080p the texture is still oversampled 2:1. So this is not a quality trade at any output size we support; it is removing detail that no pipeline could reach. The encoding loss is 41.3 to 52.1 dB PSNR against the raw Lanczos downscale, measured per file. Above 40 dB is visually transparent on photographic content, and the worst case is the most detailed image. ICC profiles are preserved byte for byte — five files carry one, and losing it would shift colours, which is a visible regression where the resolution is not. EXIF rides along untouched. Thumbnails are already 240x240 and unchanged. WHY THE ASSETS AND NOT THE CODE. A runtime cap was measured first and rejected: `image::imageops::resize` costs 217 to 730 ms per first use in release, two to four times the decode it follows, and the two strategies tried (one resize, successive halvings) cost the same. A GPU downscale would be cheap but `load_image_srv` runs mid-frame, so it would have to save and restore the render target and viewport across three back-ends — real hazard, marginal gain. Capping the assets costs nothing at runtime and shrinks the installer too. It does not cover wallpapers the user imports. Those stay bounded by the LRU budget from #537, which is the safety net this optimisation sits on top of rather than replaces. One consequence worth naming: `img_cache_stays_under_budget` now holds 11-13 entries resident instead of 3, which is the real win — far less eviction churn. Its anti-vacuous guard (unevicted total must exceed the budget) now reads 772 MB against 512. Still true, but closer to the floor: shrink these assets much further and that test will fail, which is exactly the signal it exists to give.
Contributor
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (17)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bundled wallpapers shipped at up to 7680×7680, for an app whose largest output is 4K. That is resolution nobody can see, paid for three times: on disk, in decode time, and in VRAM for as long as the texture stays resident.
17 of 18 resized to 3840 on the long edge (Lanczos, quality 92).
Why 3840, and why this is not a quality trade
3840 is exactly 1:1 for a 4K output — below it, a 4K export would sample a background softer than its own frame. At 1080p the texture is still oversampled 2:1. So nothing is lost at any output size we support; this removes detail no pipeline could reach.
Encoding loss measured per file: 41.3 to 52.1 dB PSNR against the raw Lanczos downscale. Above 40 dB is visually transparent on photographic content, and the worst case is the most detailed image.
ICC profiles are preserved byte for byte — five files carry one, and losing it would shift colours, which is a visible regression where the resolution is not. EXIF rides along untouched. Thumbnails are already 240×240 and unchanged.
Why the assets and not the code
A runtime cap was measured first, and rejected:
image::imageops::resizecosts 217–730 ms per first use in release — two to four times the decode it follows. Both strategies tried (one resize, successive halvings) cost the same. Adding up to 730 ms to picking a wallpaper to save memory is a bad trade.load_image_srvruns mid-frame, so it would have to save and restore the render target and viewport across three back-ends. Real hazard, marginal gain.Capping the assets costs nothing at runtime and shrinks the installer too.
It does not cover wallpapers the user imports. Those stay bounded by the LRU budget from #537 — the safety net this sits on top of rather than replaces.
One consequence worth naming
img_cache_stays_under_budgetnow holds 11–13 entries resident instead of 3, which is the real win: far less eviction churn. Its anti-vacuous guard (unevicted total must exceed the budget) now reads 772 MB against 512. Still true, but closer to the floor — shrink these assets much further and that test will fail, which is exactly the signal it exists to give.Verification
2208 JS tests, 151 Rust, docs check clean.
🤖 Generated with Claude Code