Skip to content

fix(wallpaper): skip ImageHandle::from_rgba when source and fit unchanged#2020

Open
rchennau wants to merge 1 commit into
pop-os:masterfrom
rchennau:fix/wallpaper-cache-display-image-unique-id-leak
Open

fix(wallpaper): skip ImageHandle::from_rgba when source and fit unchanged#2020
rchennau wants to merge 1 commit into
pop-os:masterfrom
rchennau:fix/wallpaper-cache-display-image-unique-id-leak

Conversation

@rchennau
Copy link
Copy Markdown

Problem

cosmic-settings desktop leaks ~9 GB/hour of RSS when running as a persistent background daemon with slideshow mode active. The process is eventually OOM-killed after exhausting swap, which starves the Wayland compositor of CPU time and freezes the display. This is a reproduction of #843.

Root Cause

cache_display_image() calls ImageHandle::from_rgba() on every Message::UpdateState event (fired once per wallpaper rotation by the inotify watcher on the cosmic-bg state file). from_rgba unconditionally calls Id::unique():

// pop-os/iced core/src/image.rs:216
static NEXT_ID: AtomicU64 = AtomicU64::new(0);
Self(_Id::Unique(NEXT_ID.fetch_add(1, atomic::Ordering::Relaxed)))

Every call produces a monotonically-increasing unique ID, making cache hits in the wgpu raster cache (wgpu/src/image/raster.rsFxHashMap<image::Id, Memory>) structurally impossible.

Eviction only happens inside trim() (raster.rs:78), which is only called from draw() (wgpu/src/lib.rs:162), which is only called from present(). When the Wayland compositor suppresses RedrawRequested between rotations (DPMS off, occlusion, throttling), present() never fires, trim() never runs, and the FxHashMap grows unbounded — one Memory entry per rotation interval.

This is the same trim()/present() gap described in iced-rs/iced#2659, triggered here by the slideshow rotation path.

Fix

Add cached_display_key: Option<DefaultKey> and cached_display_fit: Option<usize> to Page. At the start of cache_display_image(), resolve the effective DefaultKey for the current selection and short-circuit if it matches the previously rendered values:

if self.cached_display_handle.is_some()
    && resolved_key == self.cached_display_key
    && Some(self.selected_fit) == self.cached_display_fit
{
    return;  // source unchanged — existing handle is valid
}

ImageHandle::from_rgba is now called only when the wallpaper or fit mode actually changes, not on every state event. All three existing cached_display_handle = None sites are updated to clear the new fields so state stays coherent.

Tested

  • Pop!_OS 24.04, cosmic-settings v1.0.12, NVIDIA RTX 5060 Ti + Intel iGPU (hybrid display path)
  • Built from source, deployed, and monitored for 20 minutes
  • RSS stable at ~243 MB (previously grew at ~9.3 GB/hour)
  • Slideshow rotation continues to update the display preview correctly

Note on complementary fix

This prevents new unique handles from accumulating. A complementary fix in pop-os/iced — decoupling trim() from present() so stale entries are evicted even when the compositor suppresses redraws — would provide defence-in-depth (see iced-rs/iced#2659).

…or cache leak)

ImageHandle::from_rgba always calls Id::unique() (AtomicU64::fetch_add,
core/src/image.rs:216), so every UpdateState event during slideshow mode
inserted an un-evictable entry into the wgpu raster cache
(wgpu/src/image/raster.rs:FxHashMap<image::Id,Memory>), causing ~9 GB/hour
RSS growth.

Changes:
- Add cached_display_key: Option<DefaultKey> and cached_display_fit: Option<usize>
  to Page struct.
- cache_display_image() resolves the effective DefaultKey first, then short-circuits
  if both key and fit match the previously rendered values.
- Sync key/fit fields at all three existing cached_display_handle=None sites
  (on_leave, ColorAdd, ColorSelect) so state stays coherent.

Resolves: DEF-COSMIC-001 fix-path / pop-os#843
See also: pop-os/iced wgpu/src/image/raster.rs (Fix B: trim() vs present() gap)
@leviport
Copy link
Copy Markdown
Member

The PR template is mandatory

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.

2 participants