From 65cff4639f28d09276db9090b4dda3267361a845 Mon Sep 17 00:00:00 2001 From: Jessey Fransen Date: Wed, 19 Aug 2026 12:26:28 +0200 Subject: [PATCH 1/3] fix(renderer): recover from oversized wallpapers --- CHANGELOG.md | 2 + docs/architecture.md | 4 +- wallr-core/src/animated/mod.rs | 31 ++- wallr-core/src/daemon/mod.rs | 392 ++++++++++++++++++++++++------- wallr-core/src/preview/mod.rs | 82 +++++-- wallr-core/src/renderer/mod.rs | 219 ++++++++++++++++- wallr-core/src/video/mod.rs | 2 +- wallr-core/src/video/playback.rs | 67 +++++- 8 files changed, 679 insertions(+), 120 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d62e7d4..0e6e591 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ - **Fix dead FPS pacer in video playback**: the `--max-fps` pacing loop in the video path declared `last_present` without a type annotation and never assigned it, so the limit silently never activated and the crate failed to compile in a fresh package build. The variable is now typed and stamped after each presented frame. +- **Prevent oversized wallpaper restart loops**: static images are reduced to render-appropriate output dimensions before GPU upload, texture allocations are validated against the adapter limit, and wallpaper state is persisted only after a successful commit. Startup restoration can fall back to the previous valid wallpaper instead of repeatedly aborting on poisoned state. + ## 0.3.0 - **Stabilize explicit-sync and NVDEC lifecycles** — merged in PR #13 from @Luquatic. diff --git a/docs/architecture.md b/docs/architecture.md index 2bd4c04..af7f06e 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -40,13 +40,13 @@ - **Backend**: Vulkan / OpenGL / Metal (via `wgpu` abstraction) - **Shader Pipeline**: Single-pass WGSL shader (`effects.wgsl`) - **Uniform Buffer**: Tracks separate old/new image aspect ratios, screen resolution, animation progress (`0.0..1.0`), active effect type index (`fade`, `blur`, `wipe`, `slide`, `zoom`, `pixelate`, `ripple`, `dissolve`, `wave`, `grow`, `outer`), effect parameters (`param_a` to `param_d`), effect origin (`origin`), travel direction (`direction`), easing mode (`easing`: `0` linear, `1` ease-in, `2` ease-out, `3` ease-in-out), and scaling mode (`scaling_mode`: `0` fill, `1` fit, `2` stretch, `3` center, `4` tile). Struct is 80 bytes (`Vec2`-aligned, size padded for WGSL uniform layout). -- **Aspect Correction**: Computes aspect-ratio scaling directly inside the fragment shader, avoiding CPU-side cropping or image scaling overhead. Five scaling modes are supported: `fill` (cover, crops to fill screen), `fit` (contain, letterbox/pillarbox), `stretch` (ignores aspect ratio), `center` (1:1 centered), and `tile` (repeat). The mode is passed to the GPU via a uniform and applied per-pixel in the `scale_uv` function. Circular effects (`grow`, `outer`, `ripple`) additionally convert UVs into pixel-aspect-corrected space before taking `distance()`, so expanding rings are true circles on any monitor, never ovals. +- **Aspect Correction**: Static images larger than their useful render size are reduced once before upload while preserving enough pixels for the selected output and scaling mode. The fragment shader still owns the final aspect-ratio mapping: `fill` covers and crops, `fit` contains with letterboxing/pillarboxing, `stretch` ignores aspect ratio, `center` remains 1:1, and `tile` repeats. Center/tile sources retain native dimensions and return a clear error when they exceed the adapter limit rather than silently changing their pixel-sensitive scale. Circular effects (`grow`, `outer`, `ripple`) additionally convert UVs into pixel-aspect-corrected space before taking `distance()`, so expanding rings are true circles on any monitor, never ovals. - **Stable Image Registration**: The old and new source textures each keep their own immutable `fill` crop for the whole transition. Effects animate blend values and reveal masks in screen space; they do not translate or rescale the wallpaper texture. This prevents the visible “jump” that occurs when images with different source dimensions are changed mid-transition. - **Smoothness**: Every transition is eased with a configurable curve (`linear` / `ease_in` / `ease_out` / `ease_in_out`, default smoothstep cubic ease-in-out) and rendered one frame per vsync. The daemon presents with `PresentMode::Fifo`, so `get_current_texture()` blocks until the previous frame is displayed, pacing animations to the monitor refresh rate. Progress is derived from wall-clock time rather than a frame counter, so a transition lasts exactly its configured `duration` on any refresh rate (frame-count pacing would run too fast on high-refresh panels and too slow on low ones). The ease-in-out tail keeps visible motion almost to the last frame, so a blur radius or crossfade never appears to stagger to a halt before the transition finishes. - **Non-blocking transitions**: The daemon commits the new wallpaper state immediately and renders the visual transition on a detached background task, serialized by a render lock. `wallr set` returns as soon as the image is committed and themed, never waiting on GPU presents. If the compositor stops presenting (monitor off, suspend), the render task parks inside the present without freezing the IPC loop, and later transitions simply queue behind it. - **Live wallpapers**: when the committed file is an animated GIF, `AnimatedImage` decodes every frame once at load time. If the raw RGBA total fits the 256MB budget, frames are stored as-is and playback is a memcpy per frame; larger animations are stored as zstd-compressed streams (roughly 30:1) and decompressed through a persistent `zstd::bulk::Decompressor` context, so neither path ever re-decodes the source file during looping playback. The first frame becomes the transition's incoming texture, and when the transition ends the render task switches to playback: it computes the absolute wall-clock boundary of the next frame (`frame_start(index+1)` plus whole-loop offsets, so pacing survives animation wrap-around) and presents at that deadline, uploading the next frame into an idle double-buffered texture during the sleep via a mapped staging buffer (copied with `copy_buffer_to_texture`; unaligned widths fall back to `update_texture`). A playback generation counter is bumped on every commit, so a queued playback loop stops itself the moment a newer wallpaper supersedes it. GIF playback respects `wallr ipc pause/resume` commands and preserves timeline position when paused by tracking accumulated pause time. Static images skip playback entirely and present a single frame, and the preview window follows the same flow. - **Video playback**: `video::VideoPlayback` decodes MP4/WebM/MKV with FFmpeg. Hardware acceleration can be set to `auto` (tries all backends in priority order: NVDEC, VAAPI, VideoToolbox), a specific backend (tries that backend then falls back to software), or `software` (software-only, no hardware attempts). The active decoder backend is reported immediately after successful initialization. Frames are delivered on PTS timing through a small bounded queue and uploaded with the same texture pipeline; `wallpaper.loop_video` restarts the stream at EOF for seamless looping. `wallr ipc pause/resume/seek/info` control playback, and the video path is disabled for static images. -- **Previous-frame compositing**: The daemon keeps the previous decoded wallpaper texture as the outgoing source and reveals the new texture over it. The persisted last wallpaper is restored at daemon startup, so a restart also has a real outgoing frame. The preview window reads the same persisted path and uses the last applied wallpaper as its outgoing frame, falling back to a solid black frame only when nothing was ever applied (or when the outgoing image is the same file as the incoming one). +- **Previous-frame compositing**: The daemon keeps the previous decoded wallpaper texture as the outgoing source and reveals the new texture over it. Per-output state is atomically persisted only after the replacement commits, rotating the former path into a previous-wallpaper slot. Startup retries explicitly transient failures and falls back to that previous valid path when restoration still fails. The preview window reads the persisted path and uses the last applied wallpaper as its outgoing frame, falling back to a solid black frame only when nothing was ever applied (or when the outgoing image is the same file as the incoming one). ### 2b. Animation → Uniforms Path Every transition (from a YAML package, `wallr set --effect ...`, or the preview window) resolves to a single `animation::Effect` value, which `compute_effect_uniforms(effect, progress)` converts into an `EffectUniforms` struct (effect type, eased progress, `param_a` to `param_d`, origin, direction, easing mode). The daemon and preview both feed this into `Renderer::render_frame`, so CLI flags, YAML packages, and previews share one identical code path: diff --git a/wallr-core/src/animated/mod.rs b/wallr-core/src/animated/mod.rs index 90200c1..4f16588 100644 --- a/wallr-core/src/animated/mod.rs +++ b/wallr-core/src/animated/mod.rs @@ -14,6 +14,7 @@ use gif::DisposalMethod; /// Maximum bytes of decoded frame data kept in RAM. Frames beyond this are /// still decoded on demand, but not cached across loop wraps. const CACHE_BUDGET: usize = 256 * 1024 * 1024; +const MAX_GIF_WORKING_SET: usize = 512 * 1024 * 1024; /// A cached frame: raw RGBA8 or zstd-compressed RGBA8. The whole animation /// uses one representation, chosen at decode time: raw when the full decoded @@ -109,9 +110,8 @@ impl AnimatedImage { return Ok(None); } let total = info.delays.iter().copied().sum(); - let pixels = (info.width * info.height) as usize; + let (pixels, raw_size) = gif_allocation_sizes(info.width, info.height, info.delays.len())?; let frame_count = info.delays.len(); - let raw_size = pixels * 4 * frame_count; let raw_cache = raw_size <= CACHE_BUDGET; if !raw_cache { tracing::debug!( @@ -349,6 +349,27 @@ impl AnimatedImage { } } +fn gif_allocation_sizes( + width: u32, + height: u32, + frame_count: usize, +) -> anyhow::Result<(usize, usize)> { + let pixels = usize::try_from(u64::from(width) * u64::from(height))?; + let frame_bytes = pixels + .checked_mul(4) + .ok_or_else(|| anyhow::anyhow!("GIF frame size overflow for {width}x{height}"))?; + let working_set = frame_bytes + .checked_mul(3) + .ok_or_else(|| anyhow::anyhow!("GIF working-set overflow for {width}x{height}"))?; + anyhow::ensure!( + working_set <= MAX_GIF_WORKING_SET, + "GIF {width}x{height} requires approximately {:.1} MiB of decode working memory, exceeding the {} MiB safety limit", + working_set as f64 / (1024.0 * 1024.0), + MAX_GIF_WORKING_SET / 1024 / 1024 + ); + Ok((pixels, frame_bytes.saturating_mul(frame_count))) +} + /// Parses GIF header blocks (screen descriptor, graphic control extensions, /// image descriptors) without decoding pixel data, returning the timeline. fn scan_gif(bytes: &[u8]) -> anyhow::Result> { @@ -516,4 +537,10 @@ mod tests { assert!(scan_gif(b"not a gif at all").unwrap().is_none()); assert!(scan_gif(&[0u8; 100]).unwrap().is_none()); } + + #[test] + fn rejects_gif_working_sets_before_allocating_canvases() { + assert!(gif_allocation_sizes(7_680, 4_320, 2).is_ok()); + assert!(gif_allocation_sizes(16_384, 16_384, 1).is_err()); + } } diff --git a/wallr-core/src/daemon/mod.rs b/wallr-core/src/daemon/mod.rs index 7631e2b..b07e5b0 100644 --- a/wallr-core/src/daemon/mod.rs +++ b/wallr-core/src/daemon/mod.rs @@ -245,7 +245,10 @@ fn viewport_destination(configured: (u32, u32), physical: (u32, u32)) -> Option< #[cfg(test)] mod viewport_tests { - use super::{VideoPresentAction, video_present_action, viewport_destination}; + use super::{ + VideoPresentAction, is_transient_wallpaper_error, persist_wallpaper_at, + read_wallpaper_state, video_present_action, viewport_destination, write_wallpaper_state, + }; use crate::renderer::FrameStatus; #[test] @@ -284,6 +287,63 @@ mod viewport_tests { VideoPresentAction::Reconfigure ); } + + #[test] + fn rotates_wallpaper_state_only_after_successful_persistence() { + let temporary = tempfile::tempdir().expect("temporary directory"); + let first = temporary.path().join("first.jpg"); + let second = temporary.path().join("second.jpg"); + std::fs::write(&first, b"first").expect("first wallpaper"); + std::fs::write(&second, b"second").expect("second wallpaper"); + + persist_wallpaper_at(temporary.path(), "DP-1", &first).expect("persist first"); + assert_eq!( + read_wallpaper_state(temporary.path(), "last_wallpaper", "DP-1"), + Some(first.clone()) + ); + assert_eq!( + read_wallpaper_state(temporary.path(), "previous_wallpaper", "DP-1"), + None + ); + + persist_wallpaper_at(temporary.path(), "DP-1", &second).expect("persist second"); + assert_eq!( + read_wallpaper_state(temporary.path(), "last_wallpaper", "DP-1"), + Some(second) + ); + assert_eq!( + read_wallpaper_state(temporary.path(), "previous_wallpaper", "DP-1"), + Some(first) + ); + + let missing = temporary.path().join("missing.jpg"); + write_wallpaper_state(temporary.path(), "last_wallpaper", "DP-1", &missing) + .expect("persist missing path for restore test"); + assert_eq!( + read_wallpaper_state(temporary.path(), "last_wallpaper", "DP-1"), + Some(missing) + ); + } + + #[test] + fn retries_only_explicitly_transient_errors() { + let timed_out = anyhow::Error::new(std::io::Error::new( + std::io::ErrorKind::TimedOut, + "temporary timeout", + )); + let missing = anyhow::Error::new(std::io::Error::new( + std::io::ErrorKind::NotFound, + "missing wallpaper", + )); + let recoverable_video = anyhow::Error::new(crate::video::VideoError::QueueFull); + + assert!(is_transient_wallpaper_error(&timed_out)); + assert!(is_transient_wallpaper_error(&recoverable_video)); + assert!(!is_transient_wallpaper_error(&missing)); + assert!(!is_transient_wallpaper_error(&anyhow::anyhow!( + "invalid dimensions" + ))); + } } impl LayerShellHandler for WaylandState { @@ -710,8 +770,8 @@ impl RenderState { duration_ms: u32, scaling_mode: u32, ) -> anyhow::Result<()> { - self.scaling_mode = scaling_mode; let commit = self.commit_wallpaper(path, scaling_mode)?; + self.scaling_mode = scaling_mode; self.spawn_transition(commit, effect, duration_ms); // Update last_wallpaper after successful commit self.last_wallpaper = Some(path.to_path_buf()); @@ -726,37 +786,36 @@ impl RenderState { path: &std::path::Path, scaling_mode: u32, ) -> anyhow::Result { - use image::ImageReader; + use image::{ImageDecoder, ImageReader}; // Check if this is a video file FIRST if crate::video::VideoDecoder::is_video_file(path) { tracing::info!("Video file detected: {:?}", path); - // Invalidate any in-flight video render task BEFORE touching the - // shared decoder: the old task checks the generation on every - // iteration, so bumping first makes it exit (or skip the upload) - // before it could pull a frame of the new resolution from the - // freshly started decoder. - let generation = self.playback_gen.fetch_add(1, Ordering::SeqCst) + 1; - self.pacer.notify(); - - // Start video playback (replaces any previous playback and joins - // its decode thread, releasing the old decoder's buffers). - let metadata = - self.video_playback - .start(path, self.hw_accel, self.preload_frames, generation)?; - - // Wait for the first frame so the transition's incoming image is - // the real first frame, not a black placeholder. - let first_frame = self - .video_playback - .wait_first_frame(std::time::Duration::from_millis(1000)); + let generation = self.playback_gen.load(Ordering::SeqCst).wrapping_add(1); + let renderer = self.renderer.clone(); + // Prepare and validate the new decoder before replacing active + // playback. A failed video therefore leaves the old wallpaper and + // decoder untouched. + let mut prepared = crate::video::VideoPlayback::prepare( + path, + self.hw_accel, + self.preload_frames, + std::time::Duration::from_millis(1000), + move |metadata| { + renderer + .validate_video_texture(metadata.width, metadata.height) + .map_err(crate::video::VideoError::GpuResourceCreation) + }, + )?; + let metadata = prepared.metadata().clone(); + let first_frame = prepared.take_first_frame(); let (tex_width, tex_height) = first_frame .as_ref() .map(|frame| (frame.width, frame.height)) .unwrap_or((metadata.width, metadata.height)); - let video_texture = self.renderer.create_video_texture(tex_width, tex_height); + let video_texture = self.renderer.create_video_texture(tex_width, tex_height)?; let (img_width, img_height) = if let Some(frame) = first_frame { self.renderer .update_video_texture(&video_texture, &frame.data)?; @@ -766,6 +825,9 @@ impl RenderState { tracing::warn!("No first frame available, using black texture"); (metadata.width, metadata.height) }; + self.video_playback.commit(prepared, generation); + self.playback_gen.store(generation, Ordering::SeqCst); + self.pacer.notify(); let new_tex = video_texture.texture().clone(); let new_bind = video_texture.bind_group().clone(); @@ -802,28 +864,32 @@ impl RenderState { }); } - // A static image or GIF supersedes any video: release the video - // decoder and its buffers immediately (the generation bump also stops - // the video render task on its next vsync). - self.video_playback.stop(); - // Stream animated frames (GIF) on demand during playback; the // transition's incoming texture is the GIF's first frame. let mut animated = crate::animated::AnimatedImage::decode(path)?; let (new_tex, new_bind, img_width, img_height) = if let Some(anim) = animated.as_mut() { let (w, h) = (anim.width, anim.height); - let (tex, bind) = self.renderer.create_texture(w, h); + let (tex, bind) = self.renderer.create_texture(w, h)?; let first = anim.first_frame(); if !first.is_empty() { self.renderer.update_texture(&tex, first, w, h); } (tex, bind, w, h) } else { - let new_img = ImageReader::open(path)?.decode()?; - let (tex, bind) = self.renderer.load_texture(&new_img)?; - (tex, bind, new_img.width(), new_img.height()) + let decoder = ImageReader::open(path)?.into_decoder()?; + let (source_width, source_height) = decoder.dimensions(); + Renderer::validate_static_decode(source_width, source_height, decoder.total_bytes())?; + let new_img = image::DynamicImage::from_decoder(decoder)?; + let (tex, bind, width, height) = + self.renderer + .load_texture(&new_img, self.width, self.height, scaling_mode)?; + (tex, bind, width, height) }; + // Only supersede active video playback after the replacement has + // decoded and allocated successfully. + self.video_playback.stop(); + let old_bind = self.current_bind.take(); let (old_img_width, old_img_height) = if old_bind.is_some() { (self.current_width.max(1), self.current_height.max(1)) @@ -902,24 +968,173 @@ impl RenderState { } async fn restore_cached_wallpaper(name: &str, render_state: &Arc>) { - let state_path = dirs::cache_dir() - .unwrap_or_else(|| std::path::PathBuf::from("/tmp")) - .join(format!("wallr/last_wallpaper/{name}")); - let Ok(path_str) = std::fs::read_to_string(state_path) else { + let state_root = wallpaper_state_root(); + let Some(path) = read_wallpaper_state(&state_root, "last_wallpaper", name) else { return; }; - let path = std::path::Path::new(path_str.trim()); - if !path.exists() { - return; - } - let mut state = render_state.lock().await; let effect = crate::animation::Effect::Fade(crate::animation::FadeParams::default()); - if let Err(err) = state.set_wallpaper(path, &effect, 1000, 0).await { - tracing::warn!("Failed to restore wallpaper for {name}: {err}"); + if let Err(err) = set_wallpaper_with_retry(render_state, &path, &effect, 1000, 0).await { + tracing::warn!("Failed to restore wallpaper for {name} from {path:?}: {err}"); + let Some(previous) = read_wallpaper_state(&state_root, "previous_wallpaper", name) else { + return; + }; + match set_wallpaper_with_retry(render_state, &previous, &effect, 1000, 0).await { + Ok(()) => { + if let Err(persist_err) = + write_wallpaper_state(&state_root, "last_wallpaper", name, &previous) + { + tracing::warn!( + "Restored previous wallpaper for {name}, but failed to update state: {persist_err}" + ); + } else { + tracing::warn!("Restored previous wallpaper for {name} after {path:?} failed"); + } + } + Err(previous_err) => tracing::warn!( + "Failed to restore previous wallpaper for {name} from {previous:?}: {previous_err}" + ), + } } } +const WALLPAPER_RETRY_ATTEMPTS: usize = 3; +const WALLPAPER_RETRY_DELAY: std::time::Duration = std::time::Duration::from_millis(150); + +async fn set_wallpaper_with_retry( + render_state: &Arc>, + path: &std::path::Path, + effect: &crate::animation::Effect, + duration_ms: u32, + scaling_mode: u32, +) -> anyhow::Result<()> { + let mut attempt = 1; + loop { + let result = { + let mut state = render_state.lock().await; + state + .set_wallpaper(path, effect, duration_ms, scaling_mode) + .await + }; + match result { + Ok(()) => return Ok(()), + Err(err) + if attempt < WALLPAPER_RETRY_ATTEMPTS && is_transient_wallpaper_error(&err) => + { + tracing::warn!( + "Transient wallpaper error for {path:?} (attempt {attempt}/{WALLPAPER_RETRY_ATTEMPTS}): {err}" + ); + attempt += 1; + tokio::time::sleep(WALLPAPER_RETRY_DELAY).await; + } + Err(err) => return Err(err), + } + } +} + +fn is_transient_wallpaper_error(err: &anyhow::Error) -> bool { + err.chain().any(|cause| { + cause + .downcast_ref::() + .is_some_and(|io_err| { + matches!( + io_err.kind(), + std::io::ErrorKind::Interrupted + | std::io::ErrorKind::WouldBlock + | std::io::ErrorKind::TimedOut + ) + }) + || cause + .downcast_ref::() + .is_some_and(crate::video::VideoError::is_recoverable) + }) +} + +fn wallpaper_state_root() -> std::path::PathBuf { + dirs::cache_dir() + .unwrap_or_else(|| std::path::PathBuf::from("/tmp")) + .join("wallr") +} + +fn wallpaper_state_path(root: &std::path::Path, state_dir: &str, name: &str) -> std::path::PathBuf { + root.join(state_dir).join(name) +} + +fn read_wallpaper_state( + root: &std::path::Path, + state_dir: &str, + name: &str, +) -> Option { + let path = std::fs::read_to_string(wallpaper_state_path(root, state_dir, name)).ok()?; + let wallpaper = std::path::PathBuf::from(path.trim()); + (!wallpaper.as_os_str().is_empty()).then_some(wallpaper) +} + +fn write_wallpaper_state( + root: &std::path::Path, + state_dir: &str, + name: &str, + wallpaper: &std::path::Path, +) -> std::io::Result<()> { + use std::io::Write; + use std::sync::atomic::{AtomicU64, Ordering}; + + static TEMP_ID: AtomicU64 = AtomicU64::new(0); + + let state_path = wallpaper_state_path(root, state_dir, name); + let parent = state_path + .parent() + .ok_or_else(|| std::io::Error::other("wallpaper state path has no parent"))?; + std::fs::create_dir_all(parent)?; + let file_name = state_path + .file_name() + .and_then(|value| value.to_str()) + .unwrap_or("wallpaper"); + + loop { + let id = TEMP_ID.fetch_add(1, Ordering::Relaxed); + let temporary_path = parent.join(format!(".{file_name}.{}.{}.tmp", std::process::id(), id)); + match std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&temporary_path) + { + Ok(mut temporary) => { + if let Err(err) = temporary + .write_all(wallpaper.as_os_str().as_encoded_bytes()) + .and_then(|()| temporary.sync_all()) + .and_then(|()| std::fs::rename(&temporary_path, &state_path)) + { + let _ = std::fs::remove_file(&temporary_path); + return Err(err); + } + return Ok(()); + } + Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => continue, + Err(err) => return Err(err), + } + } +} + +fn persist_wallpaper(name: &str, wallpaper: &std::path::Path) -> std::io::Result<()> { + let root = wallpaper_state_root(); + persist_wallpaper_at(&root, name, wallpaper) +} + +fn persist_wallpaper_at( + root: &std::path::Path, + name: &str, + wallpaper: &std::path::Path, +) -> std::io::Result<()> { + let previous = read_wallpaper_state(root, "last_wallpaper", name) + .filter(|current| current.exists() && current != wallpaper); + write_wallpaper_state(root, "last_wallpaper", name, wallpaper)?; + if let Some(previous) = previous { + write_wallpaper_state(root, "previous_wallpaper", name, &previous)?; + } + Ok(()) +} + /// Presents one frame per vsync until the wall-clock duration elapses. With /// PresentMode::Fifo, `get_current_texture` blocks until the previous frame /// is presented, so this loop is paced to the monitor refresh rate, and the @@ -1025,8 +1240,22 @@ fn play_live( gif_paused: &std::sync::Arc, per_output_uniforms: &crate::renderer::PerOutputUniforms, ) { - let (tex_a, bind_a) = renderer.create_texture(animated.width, animated.height); - let (tex_b, bind_b) = renderer.create_texture(animated.width, animated.height); + let Ok((tex_a, bind_a)) = renderer.create_texture(animated.width, animated.height) else { + tracing::warn!( + "GIF dimensions {}x{} exceed the GPU texture limit", + animated.width, + animated.height + ); + return; + }; + let Ok((tex_b, bind_b)) = renderer.create_texture(animated.width, animated.height) else { + tracing::warn!( + "GIF dimensions {}x{} exceed the GPU texture limit", + animated.width, + animated.height + ); + return; + }; let (frame_w, frame_h) = (animated.width, animated.height); let (bytes_per_row, rows) = (frame_w * 4, frame_h); let frame_bytes = bytes_per_row as u64 * rows as u64; @@ -1389,6 +1618,25 @@ async fn resolve_targets( } } +fn resolve_named_targets( + render_states: &std::collections::HashMap< + String, + std::sync::Arc>, + >, + monitor: Option<&str>, +) -> Vec<(String, std::sync::Arc>)> { + match monitor { + Some(name) => render_states + .get(name) + .map(|state| vec![(name.to_string(), state.clone())]) + .unwrap_or_default(), + None => render_states + .iter() + .map(|(name, state)| (name.clone(), state.clone())) + .collect(), + } +} + pub struct Daemon { config: WallrConfig, paused: Arc, @@ -1688,7 +1936,7 @@ impl Daemon { } // Resolve targets: unknown monitor = error, no monitor = all outputs. - let targets = resolve_targets(&render_states, monitor.as_deref()).await; + let targets = resolve_named_targets(&render_states, monitor.as_deref()); if targets.is_empty() { return IpcResponse { success: false, @@ -1699,23 +1947,6 @@ impl Daemon { }; } - // Persist per-output last wallpaper for all targeted outputs. - { - let persist_names: Vec<&str> = match &monitor { - Some(name) => vec![name.as_str()], - None => render_states.keys().map(|s| s.as_str()).collect(), - }; - for name in persist_names { - let state_path = dirs::cache_dir() - .unwrap_or_else(|| std::path::PathBuf::from("/tmp")) - .join(format!("wallr/last_wallpaper/{name}")); - if let Some(parent) = state_path.parent() { - let _ = std::fs::create_dir_all(parent); - } - let _ = std::fs::write(&state_path, &path); - } - } - let effect = effect.unwrap_or_else(|| { crate::animation::Effect::Fade(crate::animation::FadeParams::default()) }); @@ -1734,31 +1965,32 @@ impl Daemon { }; let mut last_err = None; - for rs in &targets { + for (name, rs) in &targets { let rs_clone = rs.clone(); let p_clone = p.clone(); let effect_clone = effect.clone(); let result = tokio::task::spawn_blocking(move || { let rt = tokio::runtime::Handle::current(); - rt.block_on(async { - let mut lock = rs_clone.lock().await; - lock.set_wallpaper( - &p_clone, - &effect_clone, - duration, - scaling_mode_u32, - ) - .await - }) + rt.block_on(set_wallpaper_with_retry( + &rs_clone, + &p_clone, + &effect_clone, + duration, + scaling_mode_u32, + )) }) .await; - if let Err(e) = result { - last_err = Some(format!("Task spawn failed: {e}")); - continue; - } - if let Err(e) = result.unwrap() { - last_err = Some(format!("Render failed: {e}")); + match result { + Err(e) => last_err = Some(format!("Task spawn failed: {e}")), + Ok(Err(e)) => last_err = Some(format!("Render failed: {e}")), + Ok(Ok(())) => { + if let Err(err) = persist_wallpaper(name, &p) { + tracing::warn!( + "Wallpaper changed on {name}, but state persistence failed: {err}" + ); + } + } } } diff --git a/wallr-core/src/preview/mod.rs b/wallr-core/src/preview/mod.rs index 243d53d..09b2a8f 100644 --- a/wallr-core/src/preview/mod.rs +++ b/wallr-core/src/preview/mod.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use std::time::Instant; use image::GenericImageView; +use image::ImageDecoder; use tracing::info; use winit::application::ApplicationHandler; use winit::dpi::LogicalSize; @@ -202,16 +203,29 @@ impl ApplicationHandler for PreviewApp { // window. Images and GIFs keep the transition-based preview path. if crate::video::VideoDecoder::is_video_file(&self.target_path) { let playback = crate::video::VideoPlayback::new(); - match playback.start( + match crate::video::VideoPlayback::prepare( &self.target_path, crate::video::HwAccel::from_config("auto"), crate::config::VideoConfig::default().preload_frames, - 0, + std::time::Duration::from_millis(2000), + |metadata| { + renderer + .validate_video_texture(metadata.width, metadata.height) + .map_err(crate::video::VideoError::GpuResourceCreation) + }, ) { - Ok(meta) => { - let first = playback.wait_first_frame(std::time::Duration::from_millis(2000)); + Ok(mut prepared) => { + let meta = prepared.metadata().clone(); + let first = prepared.take_first_frame(); let (w, h) = (meta.width, meta.height); - let texture = renderer.create_video_texture(w, h); + let texture = match renderer.create_video_texture(w, h) { + Ok(texture) => texture, + Err(e) => { + eprintln!("failed to create video texture: {e}"); + event_loop.exit(); + return; + } + }; if let Some(frame) = first { if let Err(e) = renderer.update_video_texture(&texture, &frame.data) { eprintln!("failed to upload first video frame: {e}"); @@ -219,6 +233,7 @@ impl ApplicationHandler for PreviewApp { return; } } + playback.commit(prepared, 0); self.video = Some(playback); self.video_texture = Some(texture); self.video_size = (w, h); @@ -239,8 +254,25 @@ impl ApplicationHandler for PreviewApp { } let img = match image::ImageReader::open(&self.target_path) { - Ok(reader) => match reader.decode() { - Ok(img) => img, + Ok(reader) => match reader.into_decoder() { + Ok(decoder) => { + let (width, height) = decoder.dimensions(); + if let Err(e) = + Renderer::validate_static_decode(width, height, decoder.total_bytes()) + { + eprintln!("failed to decode image: {e}"); + event_loop.exit(); + return; + } + match image::DynamicImage::from_decoder(decoder) { + Ok(img) => img, + Err(e) => { + eprintln!("failed to decode image: {e}"); + event_loop.exit(); + return; + } + } + } Err(e) => { eprintln!("failed to decode image: {e}"); event_loop.exit(); @@ -254,16 +286,15 @@ impl ApplicationHandler for PreviewApp { } }; - let (new_tex, new_bind) = match renderer.load_texture(&img) { - Ok(t) => t, - Err(e) => { - eprintln!("failed to upload image: {e}"); - event_loop.exit(); - return; - } - }; - - let (w, h) = img.dimensions(); + let (new_tex, new_bind, w, h) = + match renderer.load_texture(&img, size.width, size.height, 0) { + Ok(t) => t, + Err(e) => { + eprintln!("failed to upload image: {e}"); + event_loop.exit(); + return; + } + }; // Fade in over the last applied wallpaper (persisted by the daemon), // so the preview shows a real transition like on the desktop. Fall @@ -273,9 +304,9 @@ impl ApplicationHandler for PreviewApp { let (bg_tex, bg_bind, old_size) = match load_last_wallpaper(&self.target_path).and_then(|bg| { renderer - .load_texture(&bg) + .load_texture(&bg, size.width, size.height, 0) .ok() - .map(|(tex, bind)| (tex, bind, bg.dimensions())) + .map(|(tex, bind, width, height)| (tex, bind, (width, height))) }) { Some((tex, bind, size)) => (tex, bind, size), None => { @@ -284,8 +315,8 @@ impl ApplicationHandler for PreviewApp { h.max(1), image::Rgba([0, 0, 0, 255]), )); - match renderer.load_texture(&black) { - Ok((tex, bind)) => (tex, bind, (w, h)), + match renderer.load_texture(&black, size.width, size.height, 0) { + Ok((tex, bind, width, height)) => (tex, bind, (width, height)), Err(e) => { eprintln!("failed to upload background: {e}"); event_loop.exit(); @@ -303,7 +334,14 @@ impl ApplicationHandler for PreviewApp { .flatten(); if let Some(anim) = self.animated.as_mut() { let (w, h) = (anim.width, anim.height); - let (tex, bind) = renderer.create_texture(w, h); + let (tex, bind) = match renderer.create_texture(w, h) { + Ok(texture) => texture, + Err(e) => { + eprintln!("failed to create GIF texture: {e}"); + event_loop.exit(); + return; + } + }; let first = anim.first_frame(); if !first.is_empty() { renderer.update_texture(&tex, first, w, h); diff --git a/wallr-core/src/renderer/mod.rs b/wallr-core/src/renderer/mod.rs index 9082c31..68e0ed3 100644 --- a/wallr-core/src/renderer/mod.rs +++ b/wallr-core/src/renderer/mod.rs @@ -3,6 +3,9 @@ use wgpu::util::DeviceExt; use crate::video::{VideoFrameData, YuvColorInfo, YuvMatrix, YuvRange}; +const MAX_TEXTURE_BYTES: u64 = 256 * 1024 * 1024; +const MAX_STATIC_DECODE_BYTES: u64 = 512 * 1024 * 1024; + pub struct Renderer { pub instance: wgpu::Instance, pub adapter: wgpu::Adapter, @@ -156,12 +159,18 @@ impl Renderer { .await .ok_or_else(|| anyhow::anyhow!("Failed to find suitable adapter"))?; + let adapter_limits = adapter.limits(); + let required_limits = wgpu::Limits { + max_texture_dimension_2d: adapter_limits.max_texture_dimension_2d, + ..wgpu::Limits::default() + }; + let (device, queue) = adapter .request_device( &wgpu::DeviceDescriptor { label: None, required_features: wgpu::Features::empty(), - required_limits: wgpu::Limits::default(), + required_limits, memory_hints: wgpu::MemoryHints::default(), }, None, @@ -318,6 +327,24 @@ impl Renderer { }) } + pub fn validate_static_decode( + width: u32, + height: u32, + decoded_bytes: u64, + ) -> anyhow::Result<()> { + anyhow::ensure!( + width > 0 && height > 0, + "decoded image dimensions must be non-zero, got {width}x{height}" + ); + anyhow::ensure!( + decoded_bytes <= MAX_STATIC_DECODE_BYTES, + "decoded image for {width}x{height} requires approximately {:.1} MiB, exceeding the {:.0} MiB safety limit", + decoded_bytes as f64 / (1024.0 * 1024.0), + MAX_STATIC_DECODE_BYTES as f64 / (1024.0 * 1024.0) + ); + Ok(()) + } + /// Create a per-output uniform buffer and bind group so each output /// renders with its own GPU state, eliminating cross-output races. pub fn create_per_output_uniforms(&self) -> PerOutputUniforms { @@ -392,7 +419,13 @@ impl Renderer { pipeline } - pub fn create_texture(&self, width: u32, height: u32) -> (wgpu::Texture, wgpu::BindGroup) { + pub fn create_texture( + &self, + width: u32, + height: u32, + ) -> anyhow::Result<(wgpu::Texture, wgpu::BindGroup)> { + validate_texture_dimensions(width, height, self.device.limits().max_texture_dimension_2d)?; + validate_texture_memory(width, height, 4)?; let size = wgpu::Extent3d { width, height, @@ -435,7 +468,7 @@ impl Renderer { label: None, }); - (texture, bind_group) + Ok((texture, bind_group)) } pub fn update_texture(&self, texture: &wgpu::Texture, rgba: &[u8], width: u32, height: u32) { @@ -460,7 +493,8 @@ impl Renderer { ); } - pub fn create_video_texture(&self, width: u32, height: u32) -> VideoTexture { + pub fn create_video_texture(&self, width: u32, height: u32) -> anyhow::Result { + self.validate_video_texture(width, height)?; let plane_texture = |label, size, format| { self.device.create_texture(&wgpu::TextureDescriptor { label: Some(label), @@ -578,7 +612,7 @@ impl Renderer { ], }); - VideoTexture { + Ok(VideoTexture { output, output_view, effects_bind_group, @@ -588,7 +622,14 @@ impl Renderer { conversion_bind_group, width, height, - } + }) + } + + pub fn validate_video_texture(&self, width: u32, height: u32) -> anyhow::Result<()> { + validate_texture_dimensions(width, height, self.device.limits().max_texture_dimension_2d)?; + // RGBA output plus NV12 luma/chroma planes use about 5.5 bytes per + // pixel. Round up so all allocations stay within a conservative cap. + validate_texture_memory(width, height, 6) } pub fn update_video_texture( @@ -686,12 +727,28 @@ impl Renderer { pub fn load_texture( &self, image: &image::DynamicImage, - ) -> anyhow::Result<(wgpu::Texture, wgpu::BindGroup)> { - let rgba = image.to_rgba8(); - let (width, height) = image.dimensions(); - let (texture, bind_group) = self.create_texture(width, height); + output_width: u32, + output_height: u32, + scaling_mode: u32, + ) -> anyhow::Result<(wgpu::Texture, wgpu::BindGroup, u32, u32)> { + let (width, height) = prepared_image_dimensions( + image.width(), + image.height(), + output_width, + output_height, + scaling_mode, + self.device.limits().max_texture_dimension_2d, + ); + let rgba = if (width, height) == image.dimensions() { + image.to_rgba8() + } else { + image + .resize_exact(width, height, image::imageops::FilterType::Lanczos3) + .to_rgba8() + }; + let (texture, bind_group) = self.create_texture(width, height)?; self.update_texture(&texture, &rgba, width, height); - Ok((texture, bind_group)) + Ok((texture, bind_group, width, height)) } pub fn update_uniforms(&self, buffer: &wgpu::Buffer, uniforms: Uniforms) { @@ -791,6 +848,90 @@ pub enum FrameStatus { Lost, } +fn validate_texture_dimensions(width: u32, height: u32, limit: u32) -> anyhow::Result<()> { + anyhow::ensure!( + width > 0 && height > 0, + "texture dimensions must be non-zero, got {width}x{height}" + ); + anyhow::ensure!( + width <= limit && height <= limit, + "texture dimensions {width}x{height} exceed the GPU limit of {limit}" + ); + Ok(()) +} + +fn validate_texture_memory(width: u32, height: u32, bytes_per_pixel: u64) -> anyhow::Result<()> { + validate_image_memory( + width, + height, + bytes_per_pixel, + MAX_TEXTURE_BYTES, + "texture allocation", + ) +} + +fn validate_image_memory( + width: u32, + height: u32, + bytes_per_pixel: u64, + limit: u64, + label: &str, +) -> anyhow::Result<()> { + let bytes = u64::from(width) + .checked_mul(u64::from(height)) + .and_then(|pixels| pixels.checked_mul(bytes_per_pixel)) + .ok_or_else(|| anyhow::anyhow!("{label} size overflow for {width}x{height}"))?; + anyhow::ensure!( + bytes <= limit, + "{label} for {width}x{height} requires approximately {:.1} MiB, exceeding the {:.0} MiB safety limit", + bytes as f64 / (1024.0 * 1024.0), + limit as f64 / (1024.0 * 1024.0) + ); + Ok(()) +} + +fn prepared_image_dimensions( + image_width: u32, + image_height: u32, + output_width: u32, + output_height: u32, + scaling_mode: u32, + texture_limit: u32, +) -> (u32, u32) { + if image_width == 0 || image_height == 0 { + return (image_width, image_height); + } + + let limit_scale = (texture_limit as f64 / image_width as f64) + .min(texture_limit as f64 / image_height as f64) + .min(1.0); + let output_scale = match scaling_mode { + // Fill retains enough pixels to cover the output; fit retains enough + // to fit inside it. The shader still owns the final crop/letterbox. + 0 if output_width > 0 && output_height > 0 => (output_width as f64 / image_width as f64) + .max(output_height as f64 / image_height as f64) + .min(1.0), + 1 if output_width > 0 && output_height > 0 => (output_width as f64 / image_width as f64) + .min(output_height as f64 / image_height as f64) + .min(1.0), + 2 if output_width > 0 && output_height > 0 => { + return ( + image_width.min(output_width).min(texture_limit).max(1), + image_height.min(output_height).min(texture_limit).max(1), + ); + } + // Center and tile have pixel-sensitive semantics. Reject an impossible + // native texture later rather than silently changing its apparent size. + _ => return (image_width, image_height), + }; + let scale = limit_scale.min(output_scale); + + ( + ((image_width as f64 * scale).round() as u32).max(1), + ((image_height as f64 * scale).round() as u32).max(1), + ) +} + /// Everything needed to present one transition frame to a surface. pub struct FrameRequest<'a> { pub surface: &'a wgpu::Surface<'a>, @@ -834,4 +975,60 @@ mod tests { assert_eq!(full_2020.blue, [1.0, 1.8814, 0.0, 0.0]); assert_eq!(full_2020.range, [0.0, 1.0, 128.0 / 255.0, 1.0]); } + + #[test] + fn validates_texture_dimensions_before_wgpu() { + assert!(validate_texture_dimensions(8192, 8192, 8192).is_ok()); + assert!(validate_texture_dimensions(0, 1080, 8192).is_err()); + assert!(validate_texture_dimensions(8193, 1080, 8192).is_err()); + assert!(validate_texture_dimensions(1920, 8193, 8192).is_err()); + assert!(validate_texture_memory(7_680, 4_320, 6).is_ok()); + assert!(validate_texture_memory(16_384, 16_384, 4).is_err()); + assert!(Renderer::validate_static_decode(11_322, 6_192, 210_304_512).is_ok()); + assert!(Renderer::validate_static_decode(32_768, 32_768, u64::MAX).is_err()); + } + + #[test] + fn fill_reduces_large_images_to_cover_the_output() { + assert_eq!( + prepared_image_dimensions(11_322, 6_192, 3_840, 2_160, 0, 32_768), + (3_950, 2_160) + ); + } + + #[test] + fn fit_preserves_aspect_ratio_without_upscaling() { + assert_eq!( + prepared_image_dimensions(11_322, 6_192, 3_840, 2_160, 1, 32_768), + (3_840, 2_100) + ); + assert_eq!( + prepared_image_dimensions(1_920, 1_080, 3_840, 2_160, 1, 32_768), + (1_920, 1_080) + ); + } + + #[test] + fn pixel_sensitive_modes_preserve_native_dimensions() { + assert_eq!( + prepared_image_dimensions(11_322, 6_192, 3_840, 2_160, 3, 8_192), + (11_322, 6_192) + ); + assert_eq!( + prepared_image_dimensions(3_840, 2_160, 2_560, 1_440, 4, 8_192), + (3_840, 2_160) + ); + } + + #[test] + fn stretch_does_not_upscale_small_images() { + assert_eq!( + prepared_image_dimensions(1, 1, 3_840, 2_160, 2, 32_768), + (1, 1) + ); + assert_eq!( + prepared_image_dimensions(5_000, 1_000, 3_840, 2_160, 2, 32_768), + (3_840, 1_000) + ); + } } diff --git a/wallr-core/src/video/mod.rs b/wallr-core/src/video/mod.rs index 9732762..8932072 100644 --- a/wallr-core/src/video/mod.rs +++ b/wallr-core/src/video/mod.rs @@ -10,5 +10,5 @@ pub use decoder::{ }; pub use error::{VideoError, VideoResult}; pub use gpu::{GpuSelection, detect_adapters, select_adapter}; -pub use playback::VideoPlayback; +pub use playback::{PreparedVideoPlayback, VideoPlayback}; pub use scheduler::{FrameScheduler, ScheduledFrame}; diff --git a/wallr-core/src/video/playback.rs b/wallr-core/src/video/playback.rs index 53dbc29..3fa8ef6 100644 --- a/wallr-core/src/video/playback.rs +++ b/wallr-core/src/video/playback.rs @@ -13,6 +13,22 @@ pub struct VideoPlayback { generation: AtomicU64, } +pub struct PreparedVideoPlayback { + decoder: VideoDecoder, + metadata: VideoMetadata, + first_frame: Option, +} + +impl PreparedVideoPlayback { + pub fn metadata(&self) -> &VideoMetadata { + &self.metadata + } + + pub fn take_first_frame(&mut self) -> Option { + self.first_frame.take() + } +} + impl VideoPlayback { pub fn new() -> Self { Self { @@ -30,13 +46,60 @@ impl VideoPlayback { preload_frames: usize, generation: u64, ) -> Result { - self.stop(); + let prepared = Self::prepare( + path, + hw_accel, + preload_frames, + Duration::from_millis(0), + |_| Ok(()), + )?; + Ok(self.commit(prepared, generation)) + } + + pub fn prepare( + path: &Path, + hw_accel: HwAccel, + preload_frames: usize, + first_frame_timeout: Duration, + validate: F, + ) -> Result + where + F: FnOnce(&VideoMetadata) -> Result<(), crate::video::error::VideoError>, + { let decoder = VideoDecoder::with_preload(path, hw_accel, preload_frames)?; let metadata = decoder.metadata().clone(); + validate(&metadata)?; + let deadline = Instant::now() + first_frame_timeout; + let first_frame = loop { + if let Some(frame) = decoder.next_frame() { + break Some(frame); + } + if Instant::now() >= deadline { + tracing::warn!("Timed out waiting for first video frame"); + break None; + } + std::thread::sleep(Duration::from_millis(5)); + }; + + Ok(PreparedVideoPlayback { + decoder, + metadata, + first_frame, + }) + } + + pub fn commit(&self, prepared: PreparedVideoPlayback, generation: u64) -> VideoMetadata { + let PreparedVideoPlayback { + decoder, + metadata, + first_frame, + } = prepared; let scheduler = FrameScheduler::new(metadata.duration); + self.stop(); self.generation.store(generation, Ordering::Release); *self.lock_decoder() = Some(decoder); *self.lock_scheduler() = Some(scheduler); + *self.lock_pending() = first_frame; tracing::info!( "Video started: {}x{} @ {:.2} fps, {:?}", metadata.width, @@ -44,7 +107,7 @@ impl VideoPlayback { metadata.fps, metadata.duration ); - Ok(metadata) + metadata } pub fn stop(&self) { From a8e0895632103ca630950cf9442fef822e71a2b6 Mon Sep 17 00:00:00 2001 From: Jessey Fransen Date: Wed, 19 Aug 2026 12:42:28 +0200 Subject: [PATCH 2/3] fix(renderer): address wallpaper recovery review --- wallr-core/src/daemon/mod.rs | 210 ++++++++++++++++++---------------- wallr-core/src/preview/mod.rs | 3 +- 2 files changed, 115 insertions(+), 98 deletions(-) diff --git a/wallr-core/src/daemon/mod.rs b/wallr-core/src/daemon/mod.rs index b07e5b0..eebd9c8 100644 --- a/wallr-core/src/daemon/mod.rs +++ b/wallr-core/src/daemon/mod.rs @@ -325,6 +325,24 @@ mod viewport_tests { ); } + #[test] + fn wallpaper_state_preserves_non_utf8_and_whitespace() { + use std::os::unix::ffi::OsStringExt; + + let temporary = tempfile::tempdir().expect("temporary directory"); + let wallpaper = std::path::PathBuf::from(std::ffi::OsString::from_vec( + b" /tmp/wallpaper-\xff.jpg ".to_vec(), + )); + + write_wallpaper_state(temporary.path(), "last_wallpaper", "DP-1", &wallpaper) + .expect("persist wallpaper path"); + + assert_eq!( + read_wallpaper_state(temporary.path(), "last_wallpaper", "DP-1"), + Some(wallpaper) + ); + } + #[test] fn retries_only_explicitly_transient_errors() { let timed_out = anyhow::Error::new(std::io::Error::new( @@ -763,7 +781,7 @@ struct CommitData { } impl RenderState { - async fn set_wallpaper( + fn set_wallpaper( &mut self, path: &std::path::Path, effect: &crate::animation::Effect, @@ -1008,28 +1026,34 @@ async fn set_wallpaper_with_retry( duration_ms: u32, scaling_mode: u32, ) -> anyhow::Result<()> { - let mut attempt = 1; - loop { - let result = { - let mut state = render_state.lock().await; - state - .set_wallpaper(path, effect, duration_ms, scaling_mode) - .await - }; - match result { - Ok(()) => return Ok(()), - Err(err) - if attempt < WALLPAPER_RETRY_ATTEMPTS && is_transient_wallpaper_error(&err) => - { - tracing::warn!( - "Transient wallpaper error for {path:?} (attempt {attempt}/{WALLPAPER_RETRY_ATTEMPTS}): {err}" - ); - attempt += 1; - tokio::time::sleep(WALLPAPER_RETRY_DELAY).await; + let render_state = Arc::clone(render_state); + let path = path.to_path_buf(); + let effect = effect.clone(); + + tokio::task::spawn_blocking(move || { + let mut attempt = 1; + loop { + let result = { + let mut state = render_state.blocking_lock(); + state.set_wallpaper(&path, &effect, duration_ms, scaling_mode) + }; + match result { + Ok(()) => return Ok(()), + Err(err) + if attempt < WALLPAPER_RETRY_ATTEMPTS + && is_transient_wallpaper_error(&err) => + { + tracing::warn!( + "Transient wallpaper error for {path:?} (attempt {attempt}/{WALLPAPER_RETRY_ATTEMPTS}): {err}" + ); + attempt += 1; + std::thread::sleep(WALLPAPER_RETRY_DELAY); + } + Err(err) => return Err(err), } - Err(err) => return Err(err), } - } + }) + .await? } fn is_transient_wallpaper_error(err: &anyhow::Error) -> bool { @@ -1065,8 +1089,10 @@ fn read_wallpaper_state( state_dir: &str, name: &str, ) -> Option { - let path = std::fs::read_to_string(wallpaper_state_path(root, state_dir, name)).ok()?; - let wallpaper = std::path::PathBuf::from(path.trim()); + use std::os::unix::ffi::OsStringExt; + + let path = std::fs::read(wallpaper_state_path(root, state_dir, name)).ok()?; + let wallpaper = std::path::PathBuf::from(std::ffi::OsString::from_vec(path)); (!wallpaper.as_os_str().is_empty()).then_some(wallpaper) } @@ -1077,6 +1103,7 @@ fn write_wallpaper_state( wallpaper: &std::path::Path, ) -> std::io::Result<()> { use std::io::Write; + use std::os::unix::ffi::OsStrExt; use std::sync::atomic::{AtomicU64, Ordering}; static TEMP_ID: AtomicU64 = AtomicU64::new(0); @@ -1101,7 +1128,7 @@ fn write_wallpaper_state( { Ok(mut temporary) => { if let Err(err) = temporary - .write_all(wallpaper.as_os_str().as_encoded_bytes()) + .write_all(wallpaper.as_os_str().as_bytes()) .and_then(|()| temporary.sync_all()) .and_then(|()| std::fs::rename(&temporary_path, &state_path)) { @@ -1966,25 +1993,18 @@ impl Daemon { let mut last_err = None; for (name, rs) in &targets { - let rs_clone = rs.clone(); - let p_clone = p.clone(); - let effect_clone = effect.clone(); - let result = tokio::task::spawn_blocking(move || { - let rt = tokio::runtime::Handle::current(); - rt.block_on(set_wallpaper_with_retry( - &rs_clone, - &p_clone, - &effect_clone, - duration, - scaling_mode_u32, - )) - }) + let result = set_wallpaper_with_retry( + rs, + &p, + &effect, + duration, + scaling_mode_u32, + ) .await; match result { - Err(e) => last_err = Some(format!("Task spawn failed: {e}")), - Ok(Err(e)) => last_err = Some(format!("Render failed: {e}")), - Ok(Ok(())) => { + Err(e) => last_err = Some(format!("Render failed: {e}")), + Ok(()) => { if let Err(err) = persist_wallpaper(name, &p) { tracing::warn!( "Wallpaper changed on {name}, but state persistence failed: {err}" @@ -2257,28 +2277,34 @@ impl Daemon { crate::animation::Effect::Fade(crate::animation::FadeParams::default()) }); let duration = duration_ms.unwrap_or(800); + let tmp = std::env::temp_dir().join("wallr_blank.png"); + { + let img = + image::RgbaImage::from_pixel(1, 1, image::Rgba([0, 0, 0, 255])); + let _ = img.save(&tmp); + } - for (name, rs) in render_states.iter() { - if monitor.as_deref() != Some(name.as_str()) && monitor.is_some() { - continue; - } - let mut lock = rs.lock().await; - if lock.blanked { - continue; - } - lock.pre_blank = Some(( - lock.last_wallpaper.clone().unwrap_or_default(), - lock.scaling_mode, - )); - lock.blanked = true; - let tmp = std::env::temp_dir().join("wallr_blank.png"); - { - let img = - image::RgbaImage::from_pixel(1, 1, image::Rgba([0, 0, 0, 255])); - let _ = img.save(&tmp); + for rs in &targets { + let rs = Arc::clone(rs); + let tmp = tmp.clone(); + let black_effect = black_effect.clone(); + let blanked = tokio::task::spawn_blocking(move || { + let mut lock = rs.blocking_lock(); + if lock.blanked { + return false; + } + lock.pre_blank = Some(( + lock.last_wallpaper.clone().unwrap_or_default(), + lock.scaling_mode, + )); + lock.blanked = true; + let _ = lock.set_wallpaper(&tmp, &black_effect, duration, 0); + true + }) + .await; + if matches!(blanked, Ok(true)) { + blanked_count += 1; } - let _ = lock.set_wallpaper(&tmp, &black_effect, duration, 0).await; - blanked_count += 1; } IpcResponse { success: true, @@ -2311,41 +2337,35 @@ impl Daemon { if monitor.as_deref() != Some(name.as_str()) && monitor.is_some() { continue; } - let mut lock = rs.lock().await; - if !lock.blanked { - continue; - } - if let Some((ref path, scaling_mode)) = lock.pre_blank.clone() { - if !path.exists() { - errors - .push(format!("{}: wallpaper path no longer exists", name)); - lock.blanked = false; - lock.pre_blank = None; - continue; - } - match lock - .set_wallpaper( - std::path::Path::new(&path), - &restore_effect, - duration, - scaling_mode, - ) - .await - { - Ok(_) => { - lock.blanked = false; - lock.pre_blank = None; - restored_count += 1; - } - Err(e) => { - errors.push(format!("{}: restore failed: {}", name, e)); - lock.blanked = false; - lock.pre_blank = None; - } + let rs = Arc::clone(rs); + let restore_effect = restore_effect.clone(); + let result = tokio::task::spawn_blocking(move || { + let mut lock = rs.blocking_lock(); + if !lock.blanked { + return None; } - } else { - errors.push(format!("{}: no previous wallpaper to restore", name)); + let result = match lock.pre_blank.clone() { + Some((path, scaling_mode)) if path.exists() => lock + .set_wallpaper( + &path, + &restore_effect, + duration, + scaling_mode, + ) + .map_err(|e| format!("restore failed: {e}")), + Some(_) => Err("wallpaper path no longer exists".to_string()), + None => Err("no previous wallpaper to restore".to_string()), + }; lock.blanked = false; + lock.pre_blank = None; + Some(result) + }) + .await; + match result { + Ok(Some(Ok(()))) => restored_count += 1, + Ok(Some(Err(e))) => errors.push(format!("{name}: {e}")), + Ok(None) => {} + Err(e) => errors.push(format!("{name}: restore task failed: {e}")), } } if !errors.is_empty() { @@ -2459,11 +2479,9 @@ impl Daemon { let p = path.clone(); let name = name.clone(); tokio::spawn(async move { - let mut lock = rs.lock().await; let effect = crate::animation::Effect::Fade(crate::animation::FadeParams::default()); - let _ = lock.set_wallpaper(&p, &effect, 600, 0).await; - drop(lock); + let _ = set_wallpaper_with_retry(&rs, &p, &effect, 600, 0).await; let opts = SetOptions { no_theme: false, theme_provider: None, diff --git a/wallr-core/src/preview/mod.rs b/wallr-core/src/preview/mod.rs index 09b2a8f..b5e22c6 100644 --- a/wallr-core/src/preview/mod.rs +++ b/wallr-core/src/preview/mod.rs @@ -2,7 +2,6 @@ use std::path::PathBuf; use std::sync::Arc; use std::time::Instant; -use image::GenericImageView; use image::ImageDecoder; use tracing::info; use winit::application::ApplicationHandler; @@ -359,7 +358,7 @@ impl ApplicationHandler for PreviewApp { self.new_bind = Some(new_bind); self._bg_tex = Some(bg_tex); self._new_tex = Some(new_tex); - self.img_size = img.dimensions(); + self.img_size = (w, h); self.old_img_size = old_size; self.start = Some(Instant::now()); } From 427dfad9e32cb75e6363249d8ad5c573ae74325c Mon Sep 17 00:00:00 2001 From: Jessey Fransen Date: Wed, 19 Aug 2026 12:49:51 +0200 Subject: [PATCH 3/3] fix(daemon): harden wallpaper state transitions --- wallr-core/Cargo.toml | 4 +- wallr-core/src/daemon/mod.rs | 80 ++++++++++++++++++++++++++--------- wallr-core/src/preview/mod.rs | 5 ++- 3 files changed, 65 insertions(+), 24 deletions(-) diff --git a/wallr-core/Cargo.toml b/wallr-core/Cargo.toml index 2e7bb28..bec422e 100644 --- a/wallr-core/Cargo.toml +++ b/wallr-core/Cargo.toml @@ -62,6 +62,7 @@ notify.workspace = true dirs.workspace = true glob.workspace = true which.workspace = true +tempfile = "3" # Utilities sha2.workspace = true @@ -71,6 +72,3 @@ humansize.workspace = true # Video support ffmpeg-next.workspace = true crossbeam-channel.workspace = true - -[dev-dependencies] -tempfile = "3" diff --git a/wallr-core/src/daemon/mod.rs b/wallr-core/src/daemon/mod.rs index eebd9c8..82d5e4d 100644 --- a/wallr-core/src/daemon/mod.rs +++ b/wallr-core/src/daemon/mod.rs @@ -2262,7 +2262,7 @@ impl Daemon { effect, duration_ms, } => { - let targets = resolve_targets(&render_states, monitor.as_deref()).await; + let targets = resolve_named_targets(&render_states, monitor.as_deref()); if targets.is_empty() { return IpcResponse { success: false, @@ -2273,42 +2273,77 @@ impl Daemon { }; } let mut blanked_count = 0u32; + let mut errors = Vec::new(); let black_effect = effect.unwrap_or_else(|| { crate::animation::Effect::Fade(crate::animation::FadeParams::default()) }); let duration = duration_ms.unwrap_or(800); - let tmp = std::env::temp_dir().join("wallr_blank.png"); + let mut blank_file = match tempfile::Builder::new() + .prefix("wallr_blank-") + .suffix(".png") + .tempfile() + { + Ok(file) => file, + Err(e) => { + return IpcResponse { + success: false, + message: Some(format!("Failed to create blank image: {e}")), + }; + } + }; + let blank_path = blank_file.path().to_path_buf(); + let blank = image::DynamicImage::ImageRgba8( + image::RgbaImage::from_pixel(1, 1, image::Rgba([0, 0, 0, 255])), + ); + if let Err(e) = blank + .write_to(blank_file.as_file_mut(), image::ImageFormat::Png) { - let img = - image::RgbaImage::from_pixel(1, 1, image::Rgba([0, 0, 0, 255])); - let _ = img.save(&tmp); + return IpcResponse { + success: false, + message: Some(format!("Failed to write blank image: {e}")), + }; } - for rs in &targets { + for (name, rs) in &targets { let rs = Arc::clone(rs); - let tmp = tmp.clone(); + let blank_path = blank_path.clone(); let black_effect = black_effect.clone(); let blanked = tokio::task::spawn_blocking(move || { let mut lock = rs.blocking_lock(); if lock.blanked { - return false; + return Ok(false); } - lock.pre_blank = Some(( + let previous = ( lock.last_wallpaper.clone().unwrap_or_default(), lock.scaling_mode, - )); + ); + lock.set_wallpaper(&blank_path, &black_effect, duration, 0)?; + lock.pre_blank = Some(previous); lock.blanked = true; - let _ = lock.set_wallpaper(&tmp, &black_effect, duration, 0); - true + Ok::(true) }) .await; - if matches!(blanked, Ok(true)) { - blanked_count += 1; + match blanked { + Ok(Ok(true)) => blanked_count += 1, + Ok(Ok(false)) => {} + Ok(Err(e)) => errors.push(format!("{name}: blank failed: {e}")), + Err(e) => errors.push(format!("{name}: blank task failed: {e}")), } } - IpcResponse { - success: true, - message: Some(format!("Blanked {blanked_count} output(s)")), + if errors.is_empty() { + IpcResponse { + success: true, + message: Some(format!("Blanked {blanked_count} output(s)")), + } + } else { + IpcResponse { + success: false, + message: Some(format!( + "Blanked {blanked_count} output(s), {} error(s): {}", + errors.len(), + errors.join("; ") + )), + } } } IpcCommand::Restore { @@ -2356,8 +2391,10 @@ impl Daemon { Some(_) => Err("wallpaper path no longer exists".to_string()), None => Err("no previous wallpaper to restore".to_string()), }; - lock.blanked = false; - lock.pre_blank = None; + if result.is_ok() { + lock.blanked = false; + lock.pre_blank = None; + } Some(result) }) .await; @@ -2481,7 +2518,10 @@ impl Daemon { tokio::spawn(async move { let effect = crate::animation::Effect::Fade(crate::animation::FadeParams::default()); - let _ = set_wallpaper_with_retry(&rs, &p, &effect, 600, 0).await; + if let Err(err) = set_wallpaper_with_retry(&rs, &p, &effect, 600, 0).await { + tracing::warn!("Watcher wallpaper render failed for {p:?}: {err}"); + return; + } let opts = SetOptions { no_theme: false, theme_provider: None, diff --git a/wallr-core/src/preview/mod.rs b/wallr-core/src/preview/mod.rs index b5e22c6..7fc458a 100644 --- a/wallr-core/src/preview/mod.rs +++ b/wallr-core/src/preview/mod.rs @@ -70,7 +70,10 @@ fn load_last_wallpaper(target: &std::path::Path) -> Option if same { return None; } - image::ImageReader::open(&last).ok()?.decode().ok() + let decoder = image::ImageReader::open(&last).ok()?.into_decoder().ok()?; + let (width, height) = decoder.dimensions(); + Renderer::validate_static_decode(width, height, decoder.total_bytes()).ok()?; + image::DynamicImage::from_decoder(decoder).ok() } struct PreviewApp {