From df5a9c5094bfb925487303b632985487279505a1 Mon Sep 17 00:00:00 2001 From: Nathan Anderson Date: Fri, 12 Dec 2025 23:16:49 -0500 Subject: [PATCH 1/9] fix(queue): correct queue-size documentation guidance The documentation states the queue "only works when queue-size is bigger than available images." From my reading of the code, this appears backwards and may cause the issues reported in discussion #136. When queue-size approaches or exceeds available images, the random selection loop struggles to find unseen images after several attempts, falling back to recent duplicates. Navigation also appears to hit buffer boundaries, blocking next and previous commands. Updated guidance to recommend queue-size smaller than total images, ideally half or less. Added runtime warning when queue-size >= image count, matching the pattern used elsewhere for configuration validation. Extracted helpers (`get_queue_and_file_count`, `warn_if_queue_too_large`) to avoid repeating logic across initialization and update paths. Still learning Rust idioms, so feedback welcome on both the analysis and approach. Addresses GitHub discussion #136. --- README.md | 7 +++-- daemon/src/image_picker.rs | 60 +++++++++++++++++++++++++++++++++----- 2 files changed, 57 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 6db8366..7185a7e 100644 --- a/README.md +++ b/README.md @@ -104,8 +104,11 @@ calculate the next wallpaper accordingly. When `sorting` is set to `random`, it all the wallpapers shown in a queue, so that the commands `next` and `previous` can work as intended. -**Notice**: _the queue only works when `queue-size` setting (which defaults to `10`) is bigger -than the number of available images in the folder_. +**Notice**: _For best randomization, keep `queue-size` (which defaults to `10`) smaller than your +total number of images. A good rule of thumb: set `queue-size` to half your image count or less. +When `queue-size` equals or exceeds the total number of images, navigation through the queue +becomes blocked and duplicate-avoidance becomes ineffective. A warning will be logged if this +configuration is detected._ The cycling of images can also be paused/resumed by running the `pause` and `resume` commands, or just `toggle-pause`, using _wpaperctl_: diff --git a/daemon/src/image_picker.rs b/daemon/src/image_picker.rs index 98205bc..23694d7 100644 --- a/daemon/src/image_picker.rs +++ b/daemon/src/image_picker.rs @@ -155,6 +155,35 @@ impl Drop for GroupedRandom { } } +/// Helper function to warn if queue-size is >= available files +fn warn_if_queue_too_large(queue_size: usize, files_count: usize) { + if queue_size >= files_count && files_count > 0 { + warn!( + "queue-size ({}) is greater than or equal to available images ({}). \ + This degrades randomization and blocks navigation. \ + Recommended: reduce queue-size to {} or less (half of image count).", + queue_size, files_count, files_count / 2 + ); + } +} + +/// Extract queue size and file count from wallpaper info +fn get_queue_and_file_count( + wallpaper_info: &WallpaperInfo, + filelist_cache: &RefCell, +) -> (usize, usize) { + let queue_size = wallpaper_info.drawn_images_queue_size; + let files_count = filelist_cache + .borrow() + .get( + &wallpaper_info.path, + wallpaper_info.recursive.unwrap_or_default(), + ) + .len(); + + (queue_size, files_count) +} + enum ImagePickerSorting { Random(Queue), GroupedRandom(GroupedRandom), @@ -171,14 +200,18 @@ impl ImagePickerSorting { ) -> Self { match wallpaper_info.sorting { None | Some(Sorting::Random) => { - Self::new_random(wallpaper_info.drawn_images_queue_size) + let (queue_size, files_count) = get_queue_and_file_count(wallpaper_info, &filelist_cache); + warn_if_queue_too_large(queue_size, files_count); + Self::new_random(queue_size) } Some(Sorting::GroupedRandom { group }) => { + let (queue_size, files_count) = get_queue_and_file_count(wallpaper_info, &filelist_cache); + warn_if_queue_too_large(queue_size, files_count); ImagePickerSorting::GroupedRandom(GroupedRandom::new( groups, group, wl_surface, - wallpaper_info.drawn_images_queue_size, + queue_size, )) } Some(Sorting::Ascending) => { @@ -219,6 +252,7 @@ pub struct ImagePicker { impl ImagePicker { pub const DEFAULT_DRAWN_IMAGES_QUEUE_SIZE: usize = 10; + pub fn new( wallpaper_info: &WallpaperInfo, wl_surface: &WlSurface, @@ -462,22 +496,27 @@ impl ImagePicker { // The path has changed, use a new random sorting, otherwise we reuse the current // drawn_images (_, Sorting::Random) if path_changed => { - self.sorting = - ImagePickerSorting::new_random(wallpaper_info.drawn_images_queue_size); + let (queue_size, files_count) = get_queue_and_file_count(wallpaper_info, &self.filelist_cache); + warn_if_queue_too_large(queue_size, files_count); + self.sorting = ImagePickerSorting::new_random(queue_size); } (_, Sorting::Random) => { // if the path was not changed, use the current image as the first image of // the drawn_images - let mut queue = Queue::with_capacity(wallpaper_info.drawn_images_queue_size); + let (queue_size, files_count) = get_queue_and_file_count(wallpaper_info, &self.filelist_cache); + warn_if_queue_too_large(queue_size, files_count); + let mut queue = Queue::with_capacity(queue_size); queue.push(self.current_image()); self.sorting = ImagePickerSorting::Random(queue); } (_, Sorting::GroupedRandom { group }) if path_changed => { + let (queue_size, files_count) = get_queue_and_file_count(wallpaper_info, &self.filelist_cache); + warn_if_queue_too_large(queue_size, files_count); self.sorting = ImagePickerSorting::GroupedRandom(GroupedRandom::new( wallpaper_groups.clone(), group, wl_surface, - wallpaper_info.drawn_images_queue_size, + queue_size, )); } // If the group is the same @@ -486,11 +525,14 @@ impl ImagePicker { Sorting::GroupedRandom { group }, ) if grouped_random.group.borrow().group == group => {} (_, Sorting::GroupedRandom { group }) => { + let (queue_size, files_count) = get_queue_and_file_count(wallpaper_info, &self.filelist_cache); + warn_if_queue_too_large(queue_size, files_count); + let grouped_random = GroupedRandom::new( wallpaper_groups.clone(), group, wl_surface, - wallpaper_info.drawn_images_queue_size, + queue_size, ); let mut group = grouped_random.group.borrow_mut(); @@ -506,7 +548,9 @@ impl ImagePicker { } } } else { - self.sorting = ImagePickerSorting::new_random(wallpaper_info.drawn_images_queue_size); + let (queue_size, files_count) = get_queue_and_file_count(wallpaper_info, &self.filelist_cache); + warn_if_queue_too_large(queue_size, files_count); + self.sorting = ImagePickerSorting::new_random(queue_size); } } From ca2c3ab558e6162722ea6cfda76ff800f00659bc Mon Sep 17 00:00:00 2001 From: Nathan Anderson Date: Fri, 12 Dec 2025 23:31:12 -0500 Subject: [PATCH 2/9] chore: run cargo fmt on queue warning changes Apologies for missing this in the initial commit. I should have run `cargo fmt` before submitting the PR, I'm still getting used to the Rust workflow. --- daemon/src/image_picker.rs | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/daemon/src/image_picker.rs b/daemon/src/image_picker.rs index 23694d7..d1e35c4 100644 --- a/daemon/src/image_picker.rs +++ b/daemon/src/image_picker.rs @@ -162,7 +162,9 @@ fn warn_if_queue_too_large(queue_size: usize, files_count: usize) { "queue-size ({}) is greater than or equal to available images ({}). \ This degrades randomization and blocks navigation. \ Recommended: reduce queue-size to {} or less (half of image count).", - queue_size, files_count, files_count / 2 + queue_size, + files_count, + files_count / 2 ); } } @@ -200,18 +202,17 @@ impl ImagePickerSorting { ) -> Self { match wallpaper_info.sorting { None | Some(Sorting::Random) => { - let (queue_size, files_count) = get_queue_and_file_count(wallpaper_info, &filelist_cache); + let (queue_size, files_count) = + get_queue_and_file_count(wallpaper_info, &filelist_cache); warn_if_queue_too_large(queue_size, files_count); Self::new_random(queue_size) } Some(Sorting::GroupedRandom { group }) => { - let (queue_size, files_count) = get_queue_and_file_count(wallpaper_info, &filelist_cache); + let (queue_size, files_count) = + get_queue_and_file_count(wallpaper_info, &filelist_cache); warn_if_queue_too_large(queue_size, files_count); ImagePickerSorting::GroupedRandom(GroupedRandom::new( - groups, - group, - wl_surface, - queue_size, + groups, group, wl_surface, queue_size, )) } Some(Sorting::Ascending) => { @@ -496,21 +497,24 @@ impl ImagePicker { // The path has changed, use a new random sorting, otherwise we reuse the current // drawn_images (_, Sorting::Random) if path_changed => { - let (queue_size, files_count) = get_queue_and_file_count(wallpaper_info, &self.filelist_cache); + let (queue_size, files_count) = + get_queue_and_file_count(wallpaper_info, &self.filelist_cache); warn_if_queue_too_large(queue_size, files_count); self.sorting = ImagePickerSorting::new_random(queue_size); } (_, Sorting::Random) => { // if the path was not changed, use the current image as the first image of // the drawn_images - let (queue_size, files_count) = get_queue_and_file_count(wallpaper_info, &self.filelist_cache); + let (queue_size, files_count) = + get_queue_and_file_count(wallpaper_info, &self.filelist_cache); warn_if_queue_too_large(queue_size, files_count); let mut queue = Queue::with_capacity(queue_size); queue.push(self.current_image()); self.sorting = ImagePickerSorting::Random(queue); } (_, Sorting::GroupedRandom { group }) if path_changed => { - let (queue_size, files_count) = get_queue_and_file_count(wallpaper_info, &self.filelist_cache); + let (queue_size, files_count) = + get_queue_and_file_count(wallpaper_info, &self.filelist_cache); warn_if_queue_too_large(queue_size, files_count); self.sorting = ImagePickerSorting::GroupedRandom(GroupedRandom::new( wallpaper_groups.clone(), @@ -525,15 +529,12 @@ impl ImagePicker { Sorting::GroupedRandom { group }, ) if grouped_random.group.borrow().group == group => {} (_, Sorting::GroupedRandom { group }) => { - let (queue_size, files_count) = get_queue_and_file_count(wallpaper_info, &self.filelist_cache); + let (queue_size, files_count) = + get_queue_and_file_count(wallpaper_info, &self.filelist_cache); warn_if_queue_too_large(queue_size, files_count); - let grouped_random = GroupedRandom::new( - wallpaper_groups.clone(), - group, - wl_surface, - queue_size, - ); + let grouped_random = + GroupedRandom::new(wallpaper_groups.clone(), group, wl_surface, queue_size); let mut group = grouped_random.group.borrow_mut(); // If there are no other surfaces, we must reuse the current wallpaper @@ -548,7 +549,8 @@ impl ImagePicker { } } } else { - let (queue_size, files_count) = get_queue_and_file_count(wallpaper_info, &self.filelist_cache); + let (queue_size, files_count) = + get_queue_and_file_count(wallpaper_info, &self.filelist_cache); warn_if_queue_too_large(queue_size, files_count); self.sorting = ImagePickerSorting::new_random(queue_size); } From eec1e442373766044de6238ebefe669ea2f49ac6 Mon Sep 17 00:00:00 2001 From: Nathan Anderson Date: Fri, 12 Jun 2026 10:05:50 -0400 Subject: [PATCH 3/9] fix(queue): keep navigation in sync when an image is shown again A repeated image was silently dropped on push, leaving the cursor on a different image than the screen. From there the queue stopped recording history and navigation drifted. Record a repeat by moving it to the most recent slot; replays of history are left alone. --- daemon/src/image_picker.rs | 58 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 55 insertions(+), 3 deletions(-) diff --git a/daemon/src/image_picker.rs b/daemon/src/image_picker.rs index d1e35c4..a327751 100644 --- a/daemon/src/image_picker.rs +++ b/daemon/src/image_picker.rs @@ -80,10 +80,27 @@ impl Queue { } fn push(&mut self, p: PathBuf) { - // Avoid duplicates - if self.buffer.contains(&p) { + // Nothing to record if the cursor is already on this image: we + // got here by replaying history, and the queue already reflects + // what is on screen. + if self + .buffer + .get(self.current) + .map_or(false, |path| *path == p) + { return; - }; + } + + // The image is in our history but not under the cursor, so the + // random fallback showed it again. Move it to the most recent + // slot; navigation should walk the order images actually + // appeared, repeats included. + if let Some(index) = self.buffer.iter().position(|path| *path == p) { + self.buffer.remove(index); + self.buffer.push_back(p); + self.current = self.buffer.len() - 1; + return; + } if self.is_full() { self.buffer.pop_front(); @@ -678,6 +695,41 @@ mod tests { assert_eq!(None, queue.previous()); } + #[test] + fn test_push_redisplayed_image() { + // A queue larger than the image set forces the random fallback + // to repeat an image we have already shown. Recording that + // repeat moves it to the most recent slot, so previous walks + // the order things actually appeared on screen. + let mut queue = Queue::with_capacity(5); + queue.push(PathBuf::from("mypath")); + queue.push(PathBuf::from("mypath2")); + queue.push(PathBuf::from("mypath3")); + + queue.push(PathBuf::from("mypath")); + assert_eq!(Path::new("mypath"), queue.current()); + assert_eq!(Some((Path::new("mypath3"), 1)), queue.previous()); + assert_eq!(Some((Path::new("mypath2"), 0)), queue.previous()); + assert_eq!(None, queue.previous()); + } + + #[test] + fn test_push_replayed_image() { + // Walking back and then forward replays the queue in place. + // Pushing a replayed image must leave the buffer and cursor + // alone, or the replay would skip ahead of the user. + let mut queue = Queue::with_capacity(3); + queue.push(PathBuf::from("mypath")); + queue.push(PathBuf::from("mypath2")); + queue.push(PathBuf::from("mypath3")); + + assert_eq!(Some((Path::new("mypath2"), 1)), queue.previous()); + assert_eq!(Some((Path::new("mypath"), 0)), queue.previous()); + assert_eq!(Some((Path::new("mypath2"), 1)), queue.next()); + queue.push(PathBuf::from("mypath2")); + assert_eq!(Some((Path::new("mypath3"), 2)), queue.next()); + } + #[test] fn test_resize() { let mut queue = Queue::with_capacity(5); From cb4b77238e6663d09368774c4a1a02e0b3bfb230 Mon Sep 17 00:00:00 2001 From: Nathan Anderson Date: Fri, 12 Jun 2026 10:06:30 -0400 Subject: [PATCH 4/9] fix(queue): remove the queue-size runtime warning The warning steered users away from queue-size >= image count, yet that is the setup most people want. Fix the queue instead of warning about it; the selection fix lands next. --- daemon/src/image_picker.rs | 78 ++++++++------------------------------ 1 file changed, 16 insertions(+), 62 deletions(-) diff --git a/daemon/src/image_picker.rs b/daemon/src/image_picker.rs index a327751..5d72c0f 100644 --- a/daemon/src/image_picker.rs +++ b/daemon/src/image_picker.rs @@ -172,37 +172,6 @@ impl Drop for GroupedRandom { } } -/// Helper function to warn if queue-size is >= available files -fn warn_if_queue_too_large(queue_size: usize, files_count: usize) { - if queue_size >= files_count && files_count > 0 { - warn!( - "queue-size ({}) is greater than or equal to available images ({}). \ - This degrades randomization and blocks navigation. \ - Recommended: reduce queue-size to {} or less (half of image count).", - queue_size, - files_count, - files_count / 2 - ); - } -} - -/// Extract queue size and file count from wallpaper info -fn get_queue_and_file_count( - wallpaper_info: &WallpaperInfo, - filelist_cache: &RefCell, -) -> (usize, usize) { - let queue_size = wallpaper_info.drawn_images_queue_size; - let files_count = filelist_cache - .borrow() - .get( - &wallpaper_info.path, - wallpaper_info.recursive.unwrap_or_default(), - ) - .len(); - - (queue_size, files_count) -} - enum ImagePickerSorting { Random(Queue), GroupedRandom(GroupedRandom), @@ -219,17 +188,14 @@ impl ImagePickerSorting { ) -> Self { match wallpaper_info.sorting { None | Some(Sorting::Random) => { - let (queue_size, files_count) = - get_queue_and_file_count(wallpaper_info, &filelist_cache); - warn_if_queue_too_large(queue_size, files_count); - Self::new_random(queue_size) + Self::new_random(wallpaper_info.drawn_images_queue_size) } Some(Sorting::GroupedRandom { group }) => { - let (queue_size, files_count) = - get_queue_and_file_count(wallpaper_info, &filelist_cache); - warn_if_queue_too_large(queue_size, files_count); ImagePickerSorting::GroupedRandom(GroupedRandom::new( - groups, group, wl_surface, queue_size, + groups, + group, + wl_surface, + wallpaper_info.drawn_images_queue_size, )) } Some(Sorting::Ascending) => { @@ -270,7 +236,6 @@ pub struct ImagePicker { impl ImagePicker { pub const DEFAULT_DRAWN_IMAGES_QUEUE_SIZE: usize = 10; - pub fn new( wallpaper_info: &WallpaperInfo, wl_surface: &WlSurface, @@ -514,30 +479,22 @@ impl ImagePicker { // The path has changed, use a new random sorting, otherwise we reuse the current // drawn_images (_, Sorting::Random) if path_changed => { - let (queue_size, files_count) = - get_queue_and_file_count(wallpaper_info, &self.filelist_cache); - warn_if_queue_too_large(queue_size, files_count); - self.sorting = ImagePickerSorting::new_random(queue_size); + self.sorting = + ImagePickerSorting::new_random(wallpaper_info.drawn_images_queue_size); } (_, Sorting::Random) => { // if the path was not changed, use the current image as the first image of // the drawn_images - let (queue_size, files_count) = - get_queue_and_file_count(wallpaper_info, &self.filelist_cache); - warn_if_queue_too_large(queue_size, files_count); - let mut queue = Queue::with_capacity(queue_size); + let mut queue = Queue::with_capacity(wallpaper_info.drawn_images_queue_size); queue.push(self.current_image()); self.sorting = ImagePickerSorting::Random(queue); } (_, Sorting::GroupedRandom { group }) if path_changed => { - let (queue_size, files_count) = - get_queue_and_file_count(wallpaper_info, &self.filelist_cache); - warn_if_queue_too_large(queue_size, files_count); self.sorting = ImagePickerSorting::GroupedRandom(GroupedRandom::new( wallpaper_groups.clone(), group, wl_surface, - queue_size, + wallpaper_info.drawn_images_queue_size, )); } // If the group is the same @@ -546,12 +503,12 @@ impl ImagePicker { Sorting::GroupedRandom { group }, ) if grouped_random.group.borrow().group == group => {} (_, Sorting::GroupedRandom { group }) => { - let (queue_size, files_count) = - get_queue_and_file_count(wallpaper_info, &self.filelist_cache); - warn_if_queue_too_large(queue_size, files_count); - - let grouped_random = - GroupedRandom::new(wallpaper_groups.clone(), group, wl_surface, queue_size); + let grouped_random = GroupedRandom::new( + wallpaper_groups.clone(), + group, + wl_surface, + wallpaper_info.drawn_images_queue_size, + ); let mut group = grouped_random.group.borrow_mut(); // If there are no other surfaces, we must reuse the current wallpaper @@ -566,10 +523,7 @@ impl ImagePicker { } } } else { - let (queue_size, files_count) = - get_queue_and_file_count(wallpaper_info, &self.filelist_cache); - warn_if_queue_too_large(queue_size, files_count); - self.sorting = ImagePickerSorting::new_random(queue_size); + self.sorting = ImagePickerSorting::new_random(wallpaper_info.drawn_images_queue_size); } } From 9ffe1e1d59a8526dd1c08bc80a161a453d53de78 Mon Sep 17 00:00:00 2001 From: Nathan Anderson Date: Fri, 12 Jun 2026 10:07:48 -0400 Subject: [PATCH 5/9] fix(queue): never repeat a wallpaper until all have been shown Rejection sampling could repeat an image by bad luck, and always did once the queue held the whole collection. Sample directly from the images not yet shown this cycle; when none remain, start a new cycle that avoids only the image on screen. A queue at least as big as the collection now cycles exactly as the README always described. --- README.md | 9 ++- daemon/src/image_picker.rs | 134 +++++++++++++++++++++++++++++-------- 2 files changed, 111 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index 7185a7e..f662b71 100644 --- a/README.md +++ b/README.md @@ -104,11 +104,10 @@ calculate the next wallpaper accordingly. When `sorting` is set to `random`, it all the wallpapers shown in a queue, so that the commands `next` and `previous` can work as intended. -**Notice**: _For best randomization, keep `queue-size` (which defaults to `10`) smaller than your -total number of images. A good rule of thumb: set `queue-size` to half your image count or less. -When `queue-size` equals or exceeds the total number of images, navigation through the queue -becomes blocked and duplicate-avoidance becomes ineffective. A warning will be logged if this -configuration is detected._ +**Notice**: _wpaperd will not show a wallpaper again while it is still in the queue, whose size +can be set with `queue-size` (`10` by default). When `queue-size` is equal to or bigger than the +number of available images, no wallpaper is repeated until all of them have been shown; then a +new cycle starts._ The cycling of images can also be paused/resumed by running the `pause` and `resume` commands, or just `toggle-pause`, using _wpaperctl_: diff --git a/daemon/src/image_picker.rs b/daemon/src/image_picker.rs index 5d72c0f..8497d76 100644 --- a/daemon/src/image_picker.rs +++ b/daemon/src/image_picker.rs @@ -21,6 +21,10 @@ pub struct Queue { current: usize, tail: usize, size: usize, + // How many of the most recent entries belong to the current cycle, + // i.e. have been shown since the last time every available image + // had been seen. Random selection avoids these. + in_cycle: usize, } impl Queue { @@ -30,6 +34,7 @@ impl Queue { current: 0, tail: size - 1, size, + in_cycle: 0, } } @@ -69,8 +74,15 @@ impl Queue { self.buffer.len() == self.size } - fn contains(&self, p: &PathBuf) -> bool { - self.buffer.contains(p) + fn shown_in_current_cycle(&self, p: &PathBuf) -> bool { + self.buffer + .iter() + .skip(self.buffer.len() - self.in_cycle) + .any(|path| path == p) + } + + fn start_new_cycle(&mut self) { + self.in_cycle = 0; } fn set_current_to(&mut self, p: &Path) { @@ -91,11 +103,16 @@ impl Queue { return; } - // The image is in our history but not under the cursor, so the - // random fallback showed it again. Move it to the most recent - // slot; navigation should walk the order images actually - // appeared, repeats included. + // The image is in our history but not under the cursor, so a new + // cycle showed it again. Move it to the most recent slot; + // navigation should walk the order images actually appeared, + // repeats included. if let Some(index) = self.buffer.iter().position(|path| *path == p) { + // It can only have come from before the current cycle + // started, so it joins the cycle now + if index < self.buffer.len() - self.in_cycle { + self.in_cycle += 1; + } self.buffer.remove(index); self.buffer.push_back(p); self.current = self.buffer.len() - 1; @@ -109,6 +126,7 @@ impl Queue { self.buffer.push_back(p); self.current = self.buffer.len() - 1; } + self.in_cycle = (self.in_cycle + 1).min(self.buffer.len()); } fn has_reached_end(&self) -> bool { @@ -131,6 +149,7 @@ impl Queue { self.tail = new_size - 1; self.size = new_size; self.buffer = new_buf; + self.in_cycle = self.in_cycle.min(self.buffer.len()); } } } @@ -592,31 +611,25 @@ fn next_random_image( return (0, files[0].to_path_buf()); } - // Otherwise pick a new random image that has not been drawn before - // Try 5 times, then get a random image. We do this because it might happen - // that the queue is bigger than the amount of available wallpapers - let mut tries = 5; + // Pick uniformly among the images that have not been shown in the + // current cycle. Sampling the complement directly means a repeat + // can never happen by bad luck, only by exhaustion. + let available: Vec = (0..files.len()) + .filter(|index| !queue.shown_in_current_cycle(&files[*index])) + .collect(); + if !available.is_empty() { + let index = available[fastrand::usize(..available.len())]; + return (index, files[index].to_path_buf()); + } + + // Every image has been shown: start a new cycle. The only image we + // rule out is the one on screen, and we know there is more than one. + queue.start_new_cycle(); loop { let index = fastrand::usize(..files.len()); - // search for an image that has not been drawn yet - // fail after 5 tries - if !queue.contains(&files[index]) { + if files[index] != current_image { break (index, files[index].to_path_buf()); } - - // We have already tried a bunch of times - // We still need a new image, get the first one that is different than - // the current one. We also know that there is more than one image - if tries == 0 { - break loop { - let index = fastrand::usize(..files.len()); - if files[index] != current_image { - break (index, files[index].to_path_buf()); - } - }; - } - - tries -= 1; } } @@ -637,6 +650,8 @@ mod tests { // Note this useful idiom: importing names from outer (for mod tests) scope. use super::*; + use std::collections::HashSet; + #[test] fn test_push() { let mut queue = Queue::with_capacity(2); @@ -684,6 +699,71 @@ mod tests { assert_eq!(Some((Path::new("mypath3"), 2)), queue.next()); } + #[test] + fn test_push_keeps_cycle_count() { + // The cycle count follows where a re-shown image came from: one + // shown again within the current cycle does not grow it, one + // from before the cycle joins it. + let mut queue = Queue::with_capacity(5); + queue.push(PathBuf::from("mypath")); + queue.push(PathBuf::from("mypath2")); + queue.push(PathBuf::from("mypath3")); + assert_eq!(3, queue.in_cycle); + + // mypath2 is already part of the cycle; showing it again is + // not progress through the collection + queue.push(PathBuf::from("mypath2")); + assert_eq!(3, queue.in_cycle); + assert_eq!(Path::new("mypath2"), queue.current()); + + // A new cycle starts empty and the first image shown joins it + queue.start_new_cycle(); + assert_eq!(0, queue.in_cycle); + queue.push(PathBuf::from("mypath")); + assert_eq!(1, queue.in_cycle); + } + + #[test] + fn test_next_random_image_avoids_already_shown() { + // With one image left unseen, the selection must pick it; chance + // is not good enough. The loop guards against a regression to + // sampling that only mostly avoids the queue. + let files: Vec = ["a", "b", "c", "d"].iter().map(PathBuf::from).collect(); + for _ in 0..50 { + let mut queue = Queue::with_capacity(10); + queue.push(files[0].clone()); + queue.push(files[1].clone()); + queue.push(files[2].clone()); + + let (index, path) = next_random_image(&files[2], &mut queue, &files); + assert_eq!(3, index); + assert_eq!(files[3], path); + } + } + + #[test] + fn test_next_random_image_cycles_without_repeats() { + // Once every image has been shown, a new cycle starts: again no + // repeats until every image has been shown, and no image twice + // in a row across the boundary. + let files: Vec = ["a", "b", "c"].iter().map(PathBuf::from).collect(); + for _ in 0..20 { + let mut queue = Queue::with_capacity(10); + let mut current = PathBuf::new(); + for cycle in 0..3 { + let mut shown = HashSet::new(); + for _ in 0..files.len() { + let (_, path) = next_random_image(¤t, &mut queue, &files); + assert_ne!(current, path, "repeat across change in cycle {cycle}"); + assert!(shown.insert(path.clone()), "repeat within cycle {cycle}"); + queue.push(path.clone()); + current = path; + } + assert_eq!(files.len(), shown.len()); + } + } + } + #[test] fn test_resize() { let mut queue = Queue::with_capacity(5); From 8747bf1a3eaf2163601f74c4edd44a0159894266 Mon Sep 17 00:00:00 2001 From: Nathan Anderson Date: Fri, 12 Jun 2026 10:23:21 -0400 Subject: [PATCH 6/9] fix(queue): make resize correct in every direction Only shrinking a full buffer worked. Growing a buffer that was not full left tail stale, so walking previous could index out of bounds and crash; shrinking one never dropped entries, so eviction stopped for good. The buffer is stored oldest to newest, which makes resize a matter of dropping from the front until it fits. --- daemon/src/image_picker.rs | 100 +++++++++++++++++++++++++++++++------ 1 file changed, 84 insertions(+), 16 deletions(-) diff --git a/daemon/src/image_picker.rs b/daemon/src/image_picker.rs index 8497d76..c684a02 100644 --- a/daemon/src/image_picker.rs +++ b/daemon/src/image_picker.rs @@ -134,23 +134,23 @@ impl Queue { } fn resize(&mut self, new_size: usize) { - if !self.is_full() { - self.buffer.reserve_exact(new_size); - self.size = new_size; - } else { - let relative_current = (self.current + self.size - self.tail) % self.size; - self.current = (self.tail + self.size - new_size % self.size) % self.size; - let relative_current = (relative_current + self.size - self.current - 1) % new_size; - let mut new_buf = VecDeque::new(); - while let Some((prev, _)) = self.next() { - new_buf.push_back(prev.to_path_buf()); - } - self.current = relative_current; - self.tail = new_size - 1; - self.size = new_size; - self.buffer = new_buf; - self.in_cycle = self.in_cycle.min(self.buffer.len()); + // A queue cannot work with zero capacity; an automatically sized + // queue can ask for it when its folder is emptied. Keep the + // current size until there is a real one again. + if new_size == 0 { + return; + } + + // The buffer is stored oldest to newest, so shrinking means + // dropping from the front. The cursor follows the image it was + // on, or the oldest survivor if that image is dropped. + while self.buffer.len() > new_size { + self.buffer.pop_front(); + self.current = self.current.saturating_sub(1); } + self.size = new_size; + self.tail = new_size - 1; + self.in_cycle = self.in_cycle.min(self.buffer.len()); } } @@ -723,6 +723,27 @@ mod tests { assert_eq!(1, queue.in_cycle); } + #[test] + fn test_resize_evicts_cursor() { + // A live resize can fire while the user is anywhere in their + // history. When the cursor's image is evicted, the cursor lands + // on the oldest survivor. + let mut queue = Queue::with_capacity(5); + queue.push(PathBuf::from("mypath")); + queue.push(PathBuf::from("mypath2")); + queue.push(PathBuf::from("mypath3")); + queue.push(PathBuf::from("mypath4")); + queue.push(PathBuf::from("mypath5")); + assert_eq!(Some((Path::new("mypath4"), 3)), queue.previous()); + assert_eq!(Some((Path::new("mypath3"), 2)), queue.previous()); + assert_eq!(Some((Path::new("mypath2"), 1)), queue.previous()); + + queue.resize(2); + assert_eq!(Path::new("mypath4"), queue.current()); + assert_eq!(None, queue.previous()); + assert_eq!(Some((Path::new("mypath5"), 1)), queue.next()); + } + #[test] fn test_next_random_image_avoids_already_shown() { // With one image left unseen, the selection must pick it; chance @@ -764,6 +785,53 @@ mod tests { } } + #[test] + fn test_resize_grow() { + // Growing a queue that was not full left tail pointing at the + // old last slot, and walking backwards past the oldest entry + // indexed out of bounds. + let mut queue = Queue::with_capacity(3); + queue.push(PathBuf::from("mypath")); + queue.push(PathBuf::from("mypath2")); + queue.push(PathBuf::from("mypath3")); + + queue.resize(5); + assert_eq!(Path::new("mypath3"), queue.current()); + assert_eq!(Some((Path::new("mypath2"), 1)), queue.previous()); + assert_eq!(Some((Path::new("mypath"), 0)), queue.previous()); + assert_eq!(None, queue.previous()); + } + + #[test] + fn test_resize_shrink_not_full() { + // Shrinking below the number of buffered entries must drop the + // oldest ones; leaving the buffer bigger than the size means + // eviction never starts again and navigation wraps wrongly. + let mut queue = Queue::with_capacity(10); + queue.push(PathBuf::from("mypath")); + queue.push(PathBuf::from("mypath2")); + queue.push(PathBuf::from("mypath3")); + + queue.resize(2); + assert_eq!(Path::new("mypath3"), queue.current()); + assert_eq!(Some((Path::new("mypath2"), 0)), queue.previous()); + assert_eq!(None, queue.previous()); + } + + #[test] + fn test_resize_to_zero() { + // A queue cannot work with zero capacity; an automatically sized + // queue can ask for it when its folder is emptied. Ignore it. + let mut queue = Queue::with_capacity(2); + queue.push(PathBuf::from("mypath")); + queue.push(PathBuf::from("mypath2")); + + queue.resize(0); + assert_eq!(Path::new("mypath2"), queue.current()); + assert_eq!(Some((Path::new("mypath"), 0)), queue.previous()); + assert_eq!(None, queue.previous()); + } + #[test] fn test_resize() { let mut queue = Queue::with_capacity(5); From 4ef0d5b84ff85990861b1580706dff32a59546b5 Mon Sep 17 00:00:00 2001 From: Nathan Anderson Date: Fri, 12 Jun 2026 10:23:41 -0400 Subject: [PATCH 7/9] perf(queue): build the available set in one pass Checking every file against the cycle window was quadratic at the end of a cycle, which collection-sized queues hit on every change. Collect the cycle into a set once and filter against it. --- daemon/src/image_picker.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/daemon/src/image_picker.rs b/daemon/src/image_picker.rs index c684a02..175ed6c 100644 --- a/daemon/src/image_picker.rs +++ b/daemon/src/image_picker.rs @@ -1,6 +1,6 @@ use std::{ cell::RefCell, - collections::VecDeque, + collections::{HashSet, VecDeque}, path::{Path, PathBuf}, rc::Rc, }; @@ -74,11 +74,8 @@ impl Queue { self.buffer.len() == self.size } - fn shown_in_current_cycle(&self, p: &PathBuf) -> bool { - self.buffer - .iter() - .skip(self.buffer.len() - self.in_cycle) - .any(|path| path == p) + fn current_cycle(&self) -> impl Iterator { + self.buffer.iter().skip(self.buffer.len() - self.in_cycle) } fn start_new_cycle(&mut self) { @@ -613,9 +610,11 @@ fn next_random_image( // Pick uniformly among the images that have not been shown in the // current cycle. Sampling the complement directly means a repeat - // can never happen by bad luck, only by exhaustion. + // can never happen by bad luck, only by exhaustion. The set keeps + // this linear when the queue is sized to the whole collection. + let shown: HashSet<&PathBuf> = queue.current_cycle().collect(); let available: Vec = (0..files.len()) - .filter(|index| !queue.shown_in_current_cycle(&files[*index])) + .filter(|index| !shown.contains(&files[*index])) .collect(); if !available.is_empty() { let index = available[fastrand::usize(..available.len())]; From ded0920a577598bdbf055aba1369f1a814ba6dcb Mon Sep 17 00:00:00 2001 From: Nathan Anderson Date: Fri, 12 Jun 2026 10:26:23 -0400 Subject: [PATCH 8/9] feat(queue): size the queue to the collection by default An unset queue-size now means the number of available wallpapers, so nothing repeats until everything has been shown and there is no number to tune. An explicit queue-size keeps the old window behaviour. Single files and empty folders fall back to 10, and every size clamps to at least 1. --- daemon/src/config.rs | 6 +-- daemon/src/image_picker.rs | 80 +++++++++++++++++++++++++++++------- daemon/src/surface.rs | 3 +- daemon/src/wallpaper_info.rs | 8 ++-- 4 files changed, 74 insertions(+), 23 deletions(-) diff --git a/daemon/src/config.rs b/daemon/src/config.rs index 5984c52..9f66637 100644 --- a/daemon/src/config.rs +++ b/daemon/src/config.rs @@ -21,7 +21,6 @@ use serde::Deserialize; use smithay_client_toolkit::reexports::calloop::ping::Ping; use crate::{ - image_picker::ImagePicker, render::Transition, wallpaper_info::{BackgroundMode, Recursive, Sorting, WallpaperInfo}, }; @@ -209,8 +208,9 @@ impl SerializedWallpaperInfo { (None, None) => BackgroundMode::default(), }; let drawn_images_queue_size = match (&self.queue_size, &default.queue_size) { - (Some(size), _) | (None, Some(size)) => *size, - (None, None) => ImagePicker::DEFAULT_DRAWN_IMAGES_QUEUE_SIZE, + (Some(size), _) | (None, Some(size)) => Some(*size), + // None means the queue follows the number of available wallpapers + (None, None) => None, }; let initial_transition = match (&self.initial_transition, &default.initial_transition) { (Some(initial_transition), _) | (None, Some(initial_transition)) => *initial_transition, diff --git a/daemon/src/image_picker.rs b/daemon/src/image_picker.rs index 175ed6c..2d76af9 100644 --- a/daemon/src/image_picker.rs +++ b/daemon/src/image_picker.rs @@ -188,6 +188,38 @@ impl Drop for GroupedRandom { } } +/// An explicit queue-size always wins. Otherwise the queue is sized to +/// the collection itself, so that no wallpaper repeats until every one +/// has been shown. Single files and empty folders fall back to the +/// default; a queue needs room for at least one image. +fn effective_queue_size(explicit: Option, files_count: usize) -> usize { + explicit + .unwrap_or(if files_count == 0 { + ImagePicker::DEFAULT_DRAWN_IMAGES_QUEUE_SIZE + } else { + files_count + }) + .max(1) +} + +fn queue_size_for( + wallpaper_info: &WallpaperInfo, + filelist_cache: &RefCell, +) -> usize { + let files_count = if wallpaper_info.path.is_dir() { + filelist_cache + .borrow() + .get( + &wallpaper_info.path, + wallpaper_info.recursive.unwrap_or_default(), + ) + .len() + } else { + 0 + }; + effective_queue_size(wallpaper_info.drawn_images_queue_size, files_count) +} + enum ImagePickerSorting { Random(Queue), GroupedRandom(GroupedRandom), @@ -204,14 +236,14 @@ impl ImagePickerSorting { ) -> Self { match wallpaper_info.sorting { None | Some(Sorting::Random) => { - Self::new_random(wallpaper_info.drawn_images_queue_size) + Self::new_random(queue_size_for(wallpaper_info, &filelist_cache)) } Some(Sorting::GroupedRandom { group }) => { ImagePickerSorting::GroupedRandom(GroupedRandom::new( groups, group, wl_surface, - wallpaper_info.drawn_images_queue_size, + queue_size_for(wallpaper_info, &filelist_cache), )) } Some(Sorting::Ascending) => { @@ -495,13 +527,16 @@ impl ImagePicker { // The path has changed, use a new random sorting, otherwise we reuse the current // drawn_images (_, Sorting::Random) if path_changed => { - self.sorting = - ImagePickerSorting::new_random(wallpaper_info.drawn_images_queue_size); + self.sorting = ImagePickerSorting::new_random(queue_size_for( + wallpaper_info, + &self.filelist_cache, + )); } (_, Sorting::Random) => { // if the path was not changed, use the current image as the first image of // the drawn_images - let mut queue = Queue::with_capacity(wallpaper_info.drawn_images_queue_size); + let mut queue = + Queue::with_capacity(queue_size_for(wallpaper_info, &self.filelist_cache)); queue.push(self.current_image()); self.sorting = ImagePickerSorting::Random(queue); } @@ -510,7 +545,7 @@ impl ImagePicker { wallpaper_groups.clone(), group, wl_surface, - wallpaper_info.drawn_images_queue_size, + queue_size_for(wallpaper_info, &self.filelist_cache), )); } // If the group is the same @@ -523,7 +558,7 @@ impl ImagePicker { wallpaper_groups.clone(), group, wl_surface, - wallpaper_info.drawn_images_queue_size, + queue_size_for(wallpaper_info, &self.filelist_cache), ); let mut group = grouped_random.group.borrow_mut(); @@ -539,7 +574,10 @@ impl ImagePicker { } } } else { - self.sorting = ImagePickerSorting::new_random(wallpaper_info.drawn_images_queue_size); + self.sorting = ImagePickerSorting::new_random(queue_size_for( + wallpaper_info, + &self.filelist_cache, + )); } } @@ -554,18 +592,15 @@ impl ImagePicker { } } - pub fn update_queue_size(&mut self, drawn_images_queue_size: usize) { + pub fn update_queue_size(&mut self, wallpaper_info: &WallpaperInfo) { + let queue_size = queue_size_for(wallpaper_info, &self.filelist_cache); match &mut self.sorting { ImagePickerSorting::Random(queue) => { - queue.resize(drawn_images_queue_size); + queue.resize(queue_size); } ImagePickerSorting::Ascending(_) | ImagePickerSorting::Descending(_) => {} ImagePickerSorting::GroupedRandom(group) => { - group - .group - .borrow_mut() - .queue - .resize(drawn_images_queue_size); + group.group.borrow_mut().queue.resize(queue_size); } } } @@ -784,6 +819,21 @@ mod tests { } } + #[test] + fn test_effective_queue_size() { + // An explicit queue-size always wins + assert_eq!(7, effective_queue_size(Some(7), 100)); + // Without one, the queue covers the whole collection + assert_eq!(100, effective_queue_size(None, 100)); + // Single files and empty folders fall back to the default + assert_eq!( + ImagePicker::DEFAULT_DRAWN_IMAGES_QUEUE_SIZE, + effective_queue_size(None, 0) + ); + // A queue needs room for at least one image + assert_eq!(1, effective_queue_size(Some(0), 100)); + } + #[test] fn test_resize_grow() { // Growing a queue that was not full left tail pointing at the diff --git a/daemon/src/surface.rs b/daemon/src/surface.rs index 14d0f1c..7d8f245 100644 --- a/daemon/src/surface.rs +++ b/daemon/src/surface.rs @@ -529,8 +529,7 @@ impl Surface { } } if self.wallpaper_info.drawn_images_queue_size != wallpaper_info.drawn_images_queue_size { - self.image_picker - .update_queue_size(self.wallpaper_info.drawn_images_queue_size); + self.image_picker.update_queue_size(&self.wallpaper_info); } if self.wallpaper_info.transition_time != wallpaper_info.transition_time { let transition_time = self.wallpaper_info.transition_time; diff --git a/daemon/src/wallpaper_info.rs b/daemon/src/wallpaper_info.rs index 5538dd9..bb039c5 100644 --- a/daemon/src/wallpaper_info.rs +++ b/daemon/src/wallpaper_info.rs @@ -2,7 +2,7 @@ use std::{path::PathBuf, time::Duration}; use serde::Deserialize; -use crate::{image_picker::ImagePicker, render::Transition}; +use crate::render::Transition; #[derive(Debug, PartialEq, Default, Ord, Eq, PartialOrd, Clone, Copy)] pub enum Recursive { @@ -28,7 +28,9 @@ pub struct WallpaperInfo { pub apply_shadow: bool, pub sorting: Option, pub mode: BackgroundMode, - pub drawn_images_queue_size: usize, + /// The queue size set in the configuration; None means the queue is + /// automatically sized to the number of available wallpapers + pub drawn_images_queue_size: Option, pub transition_time: u32, /// Determines if we should show the transition between black and first @@ -54,7 +56,7 @@ impl Default for WallpaperInfo { apply_shadow: false, sorting: None, mode: BackgroundMode::default(), - drawn_images_queue_size: ImagePicker::DEFAULT_DRAWN_IMAGES_QUEUE_SIZE, + drawn_images_queue_size: None, transition_time: Transition::Fade {}.default_transition_time(), initial_transition: true, transition: Transition::Fade {}, From 106c181309bd216e34b8cf579738182f813dc441 Mon Sep 17 00:00:00 2001 From: Nathan Anderson Date: Fri, 12 Jun 2026 10:30:13 -0400 Subject: [PATCH 9/9] feat(queue): follow the folder as it changes hotwatch already repopulates the file list when a wallpaper folder changes; automatically sized queues now resize in the same event. New images join the current cycle, removals drop the oldest history first, and explicit sizes are left alone. --- README.md | 10 +++++----- daemon/src/filelist_cache.rs | 4 ++++ daemon/src/surface.rs | 8 ++++++++ 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index f662b71..c273835 100644 --- a/README.md +++ b/README.md @@ -104,10 +104,10 @@ calculate the next wallpaper accordingly. When `sorting` is set to `random`, it all the wallpapers shown in a queue, so that the commands `next` and `previous` can work as intended. -**Notice**: _wpaperd will not show a wallpaper again while it is still in the queue, whose size -can be set with `queue-size` (`10` by default). When `queue-size` is equal to or bigger than the -number of available images, no wallpaper is repeated until all of them have been shown; then a -new cycle starts._ +**Notice**: _by default the queue is sized to the number of available images and follows it as +the folder changes, so no wallpaper is repeated until all of them have been shown; then a new +cycle starts. Set `queue-size` to limit how many recently shown wallpapers are remembered +instead._ The cycling of images can also be paused/resumed by running the `pause` and `resume` commands, or just `toggle-pause`, using _wpaperctl_: @@ -145,7 +145,7 @@ represents a different display and can contain the following keys: - `offset`, offset the image on the screen, with a value from `0.0` to `1.0`. (_Optional_, `0.0` by default for `tile` mode and `0.5` for all the other modes) - `queue-size`, decide how big the queue should be when `path` is set a directory and `sorting` is - set to `random`. (_Optional_, `10` by default) + set to `random`. (_Optional_, sized to the number of available images by default) - `initial-transition`, enable the initial transition at wpaperd startup. (_Optional_, true by default) - `recursive`, recursively iterate the directory `path` when looking for available wallpapers; it is only valid when `path` points to a directory. (_Optional_, true by default) diff --git a/daemon/src/filelist_cache.rs b/daemon/src/filelist_cache.rs index e843829..d945056 100644 --- a/daemon/src/filelist_cache.rs +++ b/daemon/src/filelist_cache.rs @@ -78,6 +78,10 @@ impl FilelistCache { event_loop_handle .insert_source(ping_source, move |_, _, wpaperd| { wpaperd.filelist_cache.borrow_mut().update_cache(); + // Queues sized to the collection follow it as it changes + for surface in &mut wpaperd.surfaces { + surface.update_automatic_queue_size(); + } }) .map_err(|e| eyre!("{e:?}")) .wrap_err("Failed to insert the filelist cache watcher in the event loop")?; diff --git a/daemon/src/surface.rs b/daemon/src/surface.rs index 7d8f245..0935341 100644 --- a/daemon/src/surface.rs +++ b/daemon/src/surface.rs @@ -539,6 +539,14 @@ impl Surface { } } + /// A queue without an explicit queue-size follows the number of + /// available wallpapers; explicitly sized queues are left alone + pub fn update_automatic_queue_size(&mut self) { + if self.wallpaper_info.drawn_images_queue_size.is_none() { + self.image_picker.update_queue_size(&self.wallpaper_info); + } + } + fn handle_new_duration( &mut self, wallpaper_info: &WallpaperInfo,