From 37be8ee8ddb6f52f8bd6bf4c13cf0f0bae7fa0b0 Mon Sep 17 00:00:00 2001 From: Jason Perlow Date: Fri, 4 Sep 2026 22:57:00 -0400 Subject: [PATCH 01/11] feat(wallpaper): extract collection registry parsing into a testable core class --- meson.build | 6 ++ src/core/wallpaper_collections.vala | 102 ++++++++++++++++++++++++++ tests/wallpaper_collections_test.vala | 101 +++++++++++++++++++++++++ 3 files changed, 209 insertions(+) create mode 100644 src/core/wallpaper_collections.vala create mode 100644 tests/wallpaper_collections_test.vala diff --git a/meson.build b/meson.build index 13374ff..9b87a1d 100644 --- a/meson.build +++ b/meson.build @@ -442,3 +442,9 @@ safe_mode_test = executable('safe-mode-test', dependencies: [dependency('gobject-2.0')], ) test('safe-mode', safe_mode_test) + +wallpaper_collections_test = executable('wallpaper-collections-test', + sources: ['src/core/wallpaper_collections.vala', 'tests/wallpaper_collections_test.vala'], + dependencies: [dependency('gobject-2.0'), dependency('glib-2.0'), dependency('gio-2.0'), gee_dep], +) +test('wallpaper-collections', wallpaper_collections_test) diff --git a/src/core/wallpaper_collections.vala b/src/core/wallpaper_collections.vala new file mode 100644 index 0000000..1cf03b6 --- /dev/null +++ b/src/core/wallpaper_collections.vala @@ -0,0 +1,102 @@ +using GLib; +using Gee; + +namespace Singularity { + + public class WallpaperCollectionInfo : Object { + // Plain public fields, not GObject properties: Vala's property + // system rejects a property literally named "type" ("error: + // Property 'type' not allowed", collides with GObject's own type + // machinery). Plain fields sidestep that and still match the + // interface this class is documented to expose -- "public fields: + // string id, string name, string artist, string dir, string type". + public string id; + public string name; + public string artist; + public string dir; + public string type; + + public WallpaperCollectionInfo(string id, string name, string artist, string dir, string type) { + this.id = id; + this.name = name; + this.artist = artist; + this.dir = dir; + this.type = type; + } + } + + // Parses the .collection registry (INI-shaped KeyFiles, one per pack or + // provider) into a list of WallpaperCollectionInfo, in the priority order + // the search roots are given -- a later root's file for the same Id is + // ignored, matching "first root wins" so callers pass roots most-specific + // (e.g. per-user) LAST if they want a user override to win, or FIRST if + // they want the shipped default to win. desktop_page.vala's caller passes + // system dirs then the user dir, so a user's own collection can override + // one bundled with the OS. + // + // Callers pass explicit search_roots (not read from GLib.Environment + // here) so this class stays testable against a temp directory with no + // real filesystem layout assumptions. + public class WallpaperCollections : Object { + public static Gee.ArrayList parse(string[] search_roots) { + var results = new Gee.ArrayList(); + var seen_ids = new Gee.HashSet(); + + foreach (string root in search_roots) { + try { + var dir = File.new_for_path(root); + if (!dir.query_exists()) continue; + var en = dir.enumerate_children("standard::name", FileQueryInfoFlags.NONE, null); + FileInfo info; + while ((info = en.next_file(null)) != null) { + string filename = info.get_name(); + if (!filename.has_suffix(".collection")) continue; + + var kf = new GLib.KeyFile(); + try { + kf.load_from_file(GLib.Path.build_filename(root, filename), GLib.KeyFileFlags.NONE); + } catch (Error e) { + continue; // malformed file, skip it + } + + string collection_dir; + try { + collection_dir = kf.get_string("Collection", "Dir"); + } catch (Error e) { + continue; // Dir-less collection, skip it + } + if (collection_dir == null || collection_dir == "") continue; + + string id; + try { + id = kf.get_string("Collection", "Id"); + } catch (Error e) { + id = filename.substring(0, filename.length - ".collection".length); + } + if (id == null || id == "") { + id = filename.substring(0, filename.length - ".collection".length); + } + if (!seen_ids.add(id)) continue; // first root wins + + string name; + try { name = kf.get_string("Collection", "Name"); } + catch (Error e) { name = id; } + + string artist; + try { artist = kf.get_string("Collection", "Artist"); } + catch (Error e) { artist = ""; } + + string type; + try { type = kf.get_string("Collection", "Type"); } + catch (Error e) { type = "static"; } + + results.add(new WallpaperCollectionInfo(id, name, artist, collection_dir, type)); + } + } catch (Error e) { + continue; + } + } + return results; + } + } +} diff --git a/tests/wallpaper_collections_test.vala b/tests/wallpaper_collections_test.vala new file mode 100644 index 0000000..9fdd19d --- /dev/null +++ b/tests/wallpaper_collections_test.vala @@ -0,0 +1,101 @@ +using GLib; +using Gee; +using Singularity; + +private string make_tmp_dir() { + string path = GLib.DirUtils.make_tmp("wpcollections-XXXXXX"); + return path; +} + +private void write_collection(string dir, string filename, string contents) { + string path = GLib.Path.build_filename(dir, filename); + try { + FileUtils.set_contents(path, contents); + } catch (Error e) { + error("test setup failed: %s", e.message); + } +} + +private void test_parses_id_name_artist_dir() { + string root = make_tmp_dir(); + write_collection(root, "brandon.collection", + "[Collection]\n" + + "Id=brandon-perlow\n" + + "Name=Brandon Perlow\n" + + "Artist=Brandon Perlow\n" + + "Type=static\n" + + "Dir=/usr/share/backgrounds/ncz/brandon-perlow\n"); + + var result = WallpaperCollections.parse({ root }); + + assert(result.size == 1); + assert(result[0].id == "brandon-perlow"); + assert(result[0].name == "Brandon Perlow"); + assert(result[0].artist == "Brandon Perlow"); + assert(result[0].dir == "/usr/share/backgrounds/ncz/brandon-perlow"); + assert(result[0].type == "static"); +} + +private void test_id_falls_back_to_filename_stem() { + string root = make_tmp_dir(); + write_collection(root, "ncz.collection", + "[Collection]\n" + + "Name=NCZ-OS\n" + + "Dir=/usr/share/backgrounds/ncz\n"); + + var result = WallpaperCollections.parse({ root }); + + assert(result.size == 1); + assert(result[0].id == "ncz"); +} + +private void test_skips_dir_less_collection() { + string root = make_tmp_dir(); + write_collection(root, "broken.collection", + "[Collection]\n" + + "Id=broken\n" + + "Name=Broken\n"); + write_collection(root, "good.collection", + "[Collection]\n" + + "Id=good\n" + + "Name=Good\n" + + "Dir=/some/dir\n"); + + var result = WallpaperCollections.parse({ root }); + + assert(result.size == 1); + assert(result[0].id == "good"); +} + +private void test_ignores_non_collection_files_and_missing_dirs() { + string root = make_tmp_dir(); + write_collection(root, "notes.txt", "not a collection\n"); + + var result = WallpaperCollections.parse({ root, "/definitely/does/not/exist" }); + + assert(result.size == 0); +} + +private void test_dedupes_by_id_first_root_wins() { + string root_a = make_tmp_dir(); + string root_b = make_tmp_dir(); + write_collection(root_a, "ncz.collection", + "[Collection]\nId=ncz\nName=System\nDir=/system/ncz\n"); + write_collection(root_b, "ncz.collection", + "[Collection]\nId=ncz\nName=User Override\nDir=/user/ncz\n"); + + var result = WallpaperCollections.parse({ root_a, root_b }); + + assert(result.size == 1); + assert(result[0].name == "System"); +} + +public int main(string[] args) { + Test.init(ref args); + Test.add_func("/wallpaper-collections/parses-id-name-artist-dir", test_parses_id_name_artist_dir); + Test.add_func("/wallpaper-collections/id-falls-back-to-filename-stem", test_id_falls_back_to_filename_stem); + Test.add_func("/wallpaper-collections/skips-dir-less-collection", test_skips_dir_less_collection); + Test.add_func("/wallpaper-collections/ignores-non-collection-files-and-missing-dirs", test_ignores_non_collection_files_and_missing_dirs); + Test.add_func("/wallpaper-collections/dedupes-by-id-first-root-wins", test_dedupes_by_id_first_root_wins); + return Test.run(); +} From 2423de82ba4e1e820eba52ec52091d4baf981d62 Mon Sep 17 00:00:00 2001 From: Jason Perlow Date: Fri, 4 Sep 2026 23:39:02 -0400 Subject: [PATCH 02/11] feat(wallpaper): add rotation/selection state file helpers shared with the rotator daemon (cherry picked from commit 50db4b389610f4da04d7e65e5c2e3eb634c7d1a8) --- meson.build | 6 ++ src/core/wallpaper_rotation_state.vala | 78 +++++++++++++++++++++++ tests/wallpaper_rotation_state_test.vala | 79 ++++++++++++++++++++++++ 3 files changed, 163 insertions(+) create mode 100644 src/core/wallpaper_rotation_state.vala create mode 100644 tests/wallpaper_rotation_state_test.vala diff --git a/meson.build b/meson.build index 9b87a1d..e0903b2 100644 --- a/meson.build +++ b/meson.build @@ -448,3 +448,9 @@ wallpaper_collections_test = executable('wallpaper-collections-test', dependencies: [dependency('gobject-2.0'), dependency('glib-2.0'), dependency('gio-2.0'), gee_dep], ) test('wallpaper-collections', wallpaper_collections_test) + +wallpaper_rotation_state_test = executable('wallpaper-rotation-state-test', + sources: ['src/core/wallpaper_rotation_state.vala', 'tests/wallpaper_rotation_state_test.vala'], + dependencies: [dependency('gobject-2.0'), dependency('glib-2.0'), dependency('gio-2.0')], +) +test('wallpaper-rotation-state', wallpaper_rotation_state_test) diff --git a/src/core/wallpaper_rotation_state.vala b/src/core/wallpaper_rotation_state.vala new file mode 100644 index 0000000..a09313f --- /dev/null +++ b/src/core/wallpaper_rotation_state.vala @@ -0,0 +1,78 @@ +using GLib; + +namespace Singularity { + + // Reads and writes the plain-text state files + // cix-installer/post-install/45-wallpaper-rotator.sh's ncz-wallpaper-rotate + // and ncz-wallpaper-daemon shell scripts already poll every rotation cycle + // -- this class is the UI's side of that same shared state, not a new + // mechanism. config_dir is injected (rather than read from + // GLib.Environment here) so it's testable against a temp directory. + public class WallpaperRotationState : Object { + private const int DEFAULT_INTERVAL_SECONDS = 600; + private const int MIN_INTERVAL_SECONDS = 30; + + private string config_dir; + + public WallpaperRotationState(string config_dir) { + this.config_dir = config_dir; + } + + private string path_for(string filename) { + return GLib.Path.build_filename(config_dir, filename); + } + + private string? read_trimmed(string filename) { + string path = path_for(filename); + if (!FileUtils.test(path, FileTest.EXISTS)) return null; + string contents; + try { + FileUtils.get_contents(path, out contents); + } catch (Error e) { + return null; + } + return contents.strip(); + } + + private void write(string filename, string contents) { + GLib.DirUtils.create_with_parents(config_dir, 0700); + try { + FileUtils.set_contents(path_for(filename), contents); + } catch (Error e) { + warning("wallpaper rotation state: could not write %s: %s", filename, e.message); + } + } + + public string get_selected_collection(string default_id) { + string? value = read_trimmed("collection"); + return (value == null || value == "") ? default_id : value; + } + + public void set_selected_collection(string id) { + write("collection", id); + } + + public bool get_rotate_enabled() { + string? value = read_trimmed("rotate-enabled"); + return value != "0"; + } + + public void set_rotate_enabled(bool enabled) { + write("rotate-enabled", enabled ? "1" : "0"); + } + + public int get_rotate_interval_seconds() { + string? value = read_trimmed("rotate-interval"); + if (value == null) return DEFAULT_INTERVAL_SECONDS; + int64 parsed; + if (!int64.try_parse(value, out parsed)) return DEFAULT_INTERVAL_SECONDS; + int seconds = (int) parsed; + return seconds < MIN_INTERVAL_SECONDS ? MIN_INTERVAL_SECONDS : seconds; + } + + public void set_rotate_interval_seconds(int seconds) { + int clamped = seconds < MIN_INTERVAL_SECONDS ? MIN_INTERVAL_SECONDS : seconds; + write("rotate-interval", clamped.to_string()); + } + } +} \ No newline at end of file diff --git a/tests/wallpaper_rotation_state_test.vala b/tests/wallpaper_rotation_state_test.vala new file mode 100644 index 0000000..4bc8262 --- /dev/null +++ b/tests/wallpaper_rotation_state_test.vala @@ -0,0 +1,79 @@ +using GLib; +using Singularity; + +private string make_tmp_dir() { + return GLib.DirUtils.make_tmp("wprotation-XXXXXX"); +} + +private void test_selected_collection_defaults_when_unset() { + var state = new WallpaperRotationState(make_tmp_dir()); + assert(state.get_selected_collection("ncz") == "ncz"); +} + +private void test_selected_collection_roundtrips() { + var state = new WallpaperRotationState(make_tmp_dir()); + state.set_selected_collection("brandon-perlow"); + assert(state.get_selected_collection("ncz") == "brandon-perlow"); +} + +private void test_selected_collection_strips_whitespace() { + string dir = make_tmp_dir(); + try { + FileUtils.set_contents(GLib.Path.build_filename(dir, "collection"), " bing \n"); + } catch (Error e) { error("test setup failed: %s", e.message); } + var state = new WallpaperRotationState(dir); + assert(state.get_selected_collection("ncz") == "bing"); +} + +private void test_rotate_enabled_defaults_true() { + var state = new WallpaperRotationState(make_tmp_dir()); + assert(state.get_rotate_enabled() == true); +} + +private void test_rotate_enabled_roundtrips_false() { + var state = new WallpaperRotationState(make_tmp_dir()); + state.set_rotate_enabled(false); + assert(state.get_rotate_enabled() == false); + state.set_rotate_enabled(true); + assert(state.get_rotate_enabled() == true); +} + +private void test_rotate_interval_defaults_to_600() { + var state = new WallpaperRotationState(make_tmp_dir()); + assert(state.get_rotate_interval_seconds() == 600); +} + +private void test_rotate_interval_roundtrips() { + var state = new WallpaperRotationState(make_tmp_dir()); + state.set_rotate_interval_seconds(1800); + assert(state.get_rotate_interval_seconds() == 1800); +} + +private void test_rotate_interval_clamps_to_30_minimum() { + var state = new WallpaperRotationState(make_tmp_dir()); + state.set_rotate_interval_seconds(5); + assert(state.get_rotate_interval_seconds() == 30); +} + +private void test_rotate_interval_garbage_on_disk_reads_as_default() { + string dir = make_tmp_dir(); + try { + FileUtils.set_contents(GLib.Path.build_filename(dir, "rotate-interval"), "not-a-number\n"); + } catch (Error e) { error("test setup failed: %s", e.message); } + var state = new WallpaperRotationState(dir); + assert(state.get_rotate_interval_seconds() == 600); +} + +public int main(string[] args) { + Test.init(ref args); + Test.add_func("/wallpaper-rotation-state/selected-collection-defaults-when-unset", test_selected_collection_defaults_when_unset); + Test.add_func("/wallpaper-rotation-state/selected-collection-roundtrips", test_selected_collection_roundtrips); + Test.add_func("/wallpaper-rotation-state/selected-collection-strips-whitespace", test_selected_collection_strips_whitespace); + Test.add_func("/wallpaper-rotation-state/rotate-enabled-defaults-true", test_rotate_enabled_defaults_true); + Test.add_func("/wallpaper-rotation-state/rotate-enabled-roundtrips-false", test_rotate_enabled_roundtrips_false); + Test.add_func("/wallpaper-rotation-state/rotate-interval-defaults-to-600", test_rotate_interval_defaults_to_600); + Test.add_func("/wallpaper-rotation-state/rotate-interval-roundtrips", test_rotate_interval_roundtrips); + Test.add_func("/wallpaper-rotation-state/rotate-interval-clamps-to-30-minimum", test_rotate_interval_clamps_to_30_minimum); + Test.add_func("/wallpaper-rotation-state/rotate-interval-garbage-on-disk-reads-as-default", test_rotate_interval_garbage_on_disk_reads_as_default); + return Test.run(); +} \ No newline at end of file From a83037d0680112a39072c261d6fd4b0e154541ba Mon Sep 17 00:00:00 2001 From: Jason Perlow Date: Sat, 5 Sep 2026 23:48:51 -0400 Subject: [PATCH 03/11] feat(wallpaper): add source selector, grid scoping, and rotation controls Implements Tasks 3-4 of docs/superpowers/plans/2026-09-04-wallpaper-source-selector-rotation.md (Tasks 1-2 already landed as 36bcc12/50db4b3). - Wire WallpaperCollections.parse() and WallpaperRotationState into desktop_page.vala: a Wallpaper Source selector row lists every installed .collection, and populate_grid() now scans only the selected collections directory instead of every backgrounds path plus every collection at once. - Remove the now-superseded private collection_dirs() helper, fully replaced by WallpaperCollections.parse(). - Add Rotate Wallpapers (on/off) and Rotation Interval rows sharing the same ~/.config/ncz-wallpaper/{collection,rotate-enabled,rotate-interval} state files the shipped ncz-wallpaper-rotate/-daemon scripts already poll -- no daemon change, no new IPC. - meson.build: register wallpaper_collections.vala and wallpaper_rotation_state.vala in the main app source list. Tasks 1-2 only wired them into their own standalone test executables; desktop_page.vala could not resolve the types without this. Verified via a real container build (SINGULARITY_SOURCE_DIR override, build-singularity.sh): singularity-desktop links clean, and the shipped binary carries both the new symbols (singularity_wallpaper_collections_parse, singularity_wallpaper_rotation_state_get_selected_collection, etc.) and the literal UI strings ("Wallpaper Source", "Rotate Wallpapers", "Rotation Interval", "Every 10 minutes") -- checked directly in the binary, not inferred from build success. Not yet done: manual on-hardware verification of the actual UI (open Desktop settings, switch sources, toggle rotation) -- this commit verifies the code builds and ships, not that the UX behaves correctly end to end. (cherry picked from commit 8bd1f5e2d3bc5d65b1e9650cc743b25f1811cc2e) --- meson.build | 2 + .../sidebar/pages/desktop_page.vala | 142 ++++++++++-------- 2 files changed, 84 insertions(+), 60 deletions(-) diff --git a/meson.build b/meson.build index e0903b2..4444b81 100644 --- a/meson.build +++ b/meson.build @@ -282,6 +282,8 @@ singularity_core_sources = files( 'src/core/preview_cache.vala', 'src/core/extreme_mode_manager.vala', 'src/core/wallpaper_manager.vala', + 'src/core/wallpaper_collections.vala', + 'src/core/wallpaper_rotation_state.vala', 'src/core/wayland_gamma_backend.vala', 'src/core/shortcut_manager.vala', 'src/core/ush_portal.vala', diff --git a/src/components/sidebar/pages/desktop_page.vala b/src/components/sidebar/pages/desktop_page.vala index 52576af..4a38423 100644 --- a/src/components/sidebar/pages/desktop_page.vala +++ b/src/components/sidebar/pages/desktop_page.vala @@ -31,6 +31,9 @@ namespace Singularity { private SelectionRow? decorations_side_row; private WallpaperPreviewWidget preview_widget; private FlowBox wallpaper_grid; + private Gee.ArrayList wallpaper_collections = new Gee.ArrayList(); + private WallpaperRotationState rotation_state = new WallpaperRotationState( + GLib.Path.build_filename(GLib.Environment.get_user_config_dir(), "ncz-wallpaper")); private int wallpaper_grid_generation = 0; private int wallpaper_accent_generation = 0; private string cached_wallpaper_accent = "#3584e4"; @@ -178,6 +181,37 @@ namespace Singularity { preview_group.add_row(preview_row); add_group(preview_group); var grid_group = new PreferencesGroup(_("Wallpapers")); + + var collection_roots = new Gee.ArrayList(); + foreach (unowned string d in GLib.Environment.get_system_data_dirs()) + collection_roots.add(GLib.Path.build_filename(d, "ncz-wallpapers", "collections")); + collection_roots.add(GLib.Path.build_filename( + GLib.Environment.get_user_data_dir(), "ncz-wallpapers", "collections")); + wallpaper_collections = WallpaperCollections.parse(collection_roots.to_array()); + + var source_options = new Gee.ArrayList(); + foreach (var collection in wallpaper_collections) { + string label = (collection.artist != null && collection.artist != "" && collection.artist != collection.name) + ? "%s — %s".printf(collection.name, collection.artist) + : collection.name; + source_options.add(new Singularity.Core.AppSettingOption() { + id = collection.id, label = label + }); + } + string initial_collection_id = rotation_state.get_selected_collection("ncz"); + bool have_initial = false; + foreach (var opt in source_options) if (opt.id == initial_collection_id) have_initial = true; + if (!have_initial && source_options.size > 0) initial_collection_id = source_options[0].id; + + var source_row = new SelectionRow.with_options( + _("Wallpaper Source"), source_options, initial_collection_id); + source_row.subtitle = _("Which installed collection the gallery below shows"); + source_row.selected.connect((id) => { + rotation_state.set_selected_collection(id); + populate_grid(); + }); + grid_group.add_row(source_row); + wallpaper_grid = new FlowBox(); wallpaper_grid.add_css_class("wallpaper-gallery"); wallpaper_grid.valign = Align.START; @@ -195,6 +229,40 @@ namespace Singularity { var grid_row = new PreferencesRow(); grid_row.set_child(wallpaper_grid); grid_group.add_row(grid_row); + + var rotate_row = new SwitchRow(_("Rotate Wallpapers"), + _("Automatically change the wallpaper on a timer"), + rotation_state.get_rotate_enabled()); + grid_group.add_row(rotate_row); + + var interval_options = new Gee.ArrayList(); + interval_options.add(new Singularity.Core.AppSettingOption() { id = "600", label = _("Every 10 minutes") }); + interval_options.add(new Singularity.Core.AppSettingOption() { id = "1800", label = _("Every 30 minutes") }); + interval_options.add(new Singularity.Core.AppSettingOption() { id = "3600", label = _("Every hour") }); + interval_options.add(new Singularity.Core.AppSettingOption() { id = "14400", label = _("Every 4 hours") }); + interval_options.add(new Singularity.Core.AppSettingOption() { id = "86400", label = _("Every day") }); + + int current_interval = rotation_state.get_rotate_interval_seconds(); + string current_interval_id = current_interval.to_string(); + bool have_interval_match = false; + foreach (var opt in interval_options) if (opt.id == current_interval_id) have_interval_match = true; + if (!have_interval_match) current_interval_id = "600"; // a custom/legacy value collapses to the closest preset shown + + var interval_row = new SelectionRow.with_options( + _("Rotation Interval"), interval_options, current_interval_id); + interval_row.visible = rotation_state.get_rotate_enabled(); + interval_row.selected.connect((id) => { + int seconds; + if (int.try_parse(id, out seconds)) rotation_state.set_rotate_interval_seconds(seconds); + }); + grid_group.add_row(interval_row); + + rotate_row.switch_btn.notify["active"].connect(() => { + bool enabled = rotate_row.switch_btn.active; + rotation_state.set_rotate_enabled(enabled); + interval_row.visible = enabled; + }); + add_group(grid_group); GLib.Idle.add(() => { populate_grid(); return GLib.Source.REMOVE; }); refresh_wallpaper_accent_async(); @@ -1835,51 +1903,6 @@ namespace Singularity { // a deep tree should not stall the picker. private const int WALLPAPER_SCAN_MAX_DEPTH = 3; - // Directories declared by installed wallpaper packs. - // - // Reading the registry rather than guessing paths is what surfaces the - // Bing provider at all: its Dir= is /var/cache/ncz-wallpapers/bing, - // which is not under any backgrounds path and is unreachable by - // directory walking alone. - // - // .collection is the current on-disk format (KeyFile). The design in - // docs/WALLPAPER-PACKS.md moves to .pack.json and accepts both for one - // release; when that lands, parse *.pack.json here too rather than - // replacing this, or packs installed by the older deb disappear from - // the picker on upgrade. - private static Gee.ArrayList collection_dirs() { - var dirs = new ArrayList(); - var roots = new ArrayList(); - foreach (unowned string d in GLib.Environment.get_system_data_dirs()) - roots.add(GLib.Path.build_filename(d, "ncz-wallpapers", "collections")); - roots.add(GLib.Path.build_filename(GLib.Environment.get_user_data_dir(), - "ncz-wallpapers", "collections")); - - foreach (string root in roots) { - try { - var dir = File.new_for_path(root); - if (!dir.query_exists()) continue; - var en = dir.enumerate_children("standard::name", FileQueryInfoFlags.NONE, null); - FileInfo info; - while ((info = en.next_file(null)) != null) { - if (!info.get_name().has_suffix(".collection")) continue; - var kf = new GLib.KeyFile(); - try { - kf.load_from_file(GLib.Path.build_filename(root, info.get_name()), - GLib.KeyFileFlags.NONE); - string d = kf.get_string("Collection", "Dir"); - if (d != null && d != "" && !dirs.contains(d)) dirs.add(d); - } catch (Error e) { - // A malformed or Dir-less collection is skipped, not - // fatal: one bad pack must not empty the picker. - } - } - } catch (Error e) { - } - } - return dirs; - } - // Walk one scan root, collecting images. // // The previous implementation enumerated a single level and kept only @@ -1965,22 +1988,21 @@ namespace Singularity { var seen = new HashSet(); foreach (string uri in recent) seen.add(uri); - var path_list = new ArrayList(); - foreach (unowned string d in GLib.Environment.get_system_data_dirs()) - path_list.add(GLib.Path.build_filename(d, "backgrounds", "singularity")); - path_list.add(GLib.Path.build_filename(GLib.Environment.get_user_data_dir(), "backgrounds", "singularity")); - foreach (unowned string d in GLib.Environment.get_system_data_dirs()) - path_list.add(GLib.Path.build_filename(d, "backgrounds")); - path_list.add(GLib.Path.build_filename(GLib.Environment.get_user_data_dir(), "backgrounds")); - - // Packs declare their own directory, and it need not live under any - // backgrounds path. The Bing provider caches into - // /var/cache/ncz-wallpapers/bing, which nothing above would ever - // reach, so the registry is the only way those images are found. - foreach (string dir in collection_dirs()) - path_list.add(dir); + string selected_id = rotation_state.get_selected_collection("ncz"); + string? scan_dir = null; + foreach (var collection in wallpaper_collections) { + if (collection.id == selected_id) { scan_dir = collection.dir; break; } + } + // A selection with no matching collection (deleted pack, stale + // state file) must not empty the grid silently -- fall back to + // whatever the first known collection is, same "never leave the + // desktop with no wallpaper" principle the rotator script itself + // follows. + if (scan_dir == null && wallpaper_collections.size > 0) { + scan_dir = wallpaper_collections[0].dir; + } - string[] scan_paths = path_list.to_array(); + string[] scan_paths = (scan_dir == null) ? new string[0] : new string[] { scan_dir }; new GLib.Thread("wallpaper-scan", () => { var candidates = new ArrayList(); var thread_seen = new HashSet(); From 9417e0f9a58c19fdeddff0edf5bde5500a27bc9f Mon Sep 17 00:00:00 2001 From: Jason Perlow Date: Sun, 6 Sep 2026 01:38:06 -0400 Subject: [PATCH 04/11] fix(wallpaper): enforce gallery source boundaries Admit recent images only when the selected collection scan contains them. Exclude separately registered nested packs while retaining provider subdirectories. Exercise the production scanner with mixed history and a live-state probe. (cherry picked from commit bd9dd7a8de0ce3ef1da7dcf638b1f60f5f003f14) --- meson.build | 14 +++ scripts/wallpaper-gallery-probe.vala | 28 +++++ .../sidebar/pages/desktop_page.vala | 110 +----------------- src/core/wallpaper_gallery.vala | 108 +++++++++++++++++ tests/wallpaper_gallery_test.vala | 81 +++++++++++++ 5 files changed, 236 insertions(+), 105 deletions(-) create mode 100644 scripts/wallpaper-gallery-probe.vala create mode 100644 src/core/wallpaper_gallery.vala create mode 100644 tests/wallpaper_gallery_test.vala diff --git a/meson.build b/meson.build index 4444b81..ad77c93 100644 --- a/meson.build +++ b/meson.build @@ -283,6 +283,7 @@ singularity_core_sources = files( 'src/core/extreme_mode_manager.vala', 'src/core/wallpaper_manager.vala', 'src/core/wallpaper_collections.vala', + 'src/core/wallpaper_gallery.vala', 'src/core/wallpaper_rotation_state.vala', 'src/core/wayland_gamma_backend.vala', 'src/core/shortcut_manager.vala', @@ -456,3 +457,16 @@ wallpaper_rotation_state_test = executable('wallpaper-rotation-state-test', dependencies: [dependency('gobject-2.0'), dependency('glib-2.0'), dependency('gio-2.0')], ) test('wallpaper-rotation-state', wallpaper_rotation_state_test) + +wallpaper_gallery_test = executable('wallpaper-gallery-test', + sources: ['src/core/wallpaper_gallery.vala', 'tests/wallpaper_gallery_test.vala'], + dependencies: [dependency('gobject-2.0'), dependency('gio-2.0'), gee_dep], +) +test('wallpaper-gallery', wallpaper_gallery_test) + +executable('wallpaper-gallery-probe', + sources: ['src/core/wallpaper_gallery.vala', 'src/core/wallpaper_collections.vala', + 'src/core/wallpaper_rotation_state.vala', 'scripts/wallpaper-gallery-probe.vala'], + dependencies: [dependency('gobject-2.0'), dependency('gio-2.0'), gee_dep], + install: false, +) diff --git a/scripts/wallpaper-gallery-probe.vala b/scripts/wallpaper-gallery-probe.vala new file mode 100644 index 0000000..f668304 --- /dev/null +++ b/scripts/wallpaper-gallery-probe.vala @@ -0,0 +1,28 @@ +// Run in the desktop user's environment to report the production scanner's +// candidates against the installed registry and current GSettings history. +using GLib; +using Gee; +using Singularity; + +int main(string[] args) { + var roots = new ArrayList(); + foreach (unowned string dir in Environment.get_system_data_dirs()) + roots.add(Path.build_filename(dir, "ncz-wallpapers", "collections")); + roots.add(Path.build_filename(Environment.get_user_data_dir(), "ncz-wallpapers", "collections")); + var collections = WallpaperCollections.parse(roots.to_array()); + var state = new WallpaperRotationState(Path.build_filename(Environment.get_user_config_dir(), "ncz-wallpaper")); + string selected = state.get_selected_collection("ncz"); + string? root = null; + var dirs = new ArrayList(); + foreach (var collection in collections) { + dirs.add(collection.dir); + if (collection.id == selected) root = collection.dir; + } + if (root == null && collections.size > 0) root = collections[0].dir; + var settings = new GLib.Settings("dev.sinty.desktop"); + var candidates = WallpaperGallery.scan(root, dirs.to_array(), settings.get_strv("recent-wallpapers")); + stdout.printf("source=%s root=%s images=%d\n", selected, root ?? "(none)", candidates.size); + foreach (var candidate in candidates) + stdout.printf("%s\t%s\n", candidate.is_recent ? "recent" : "scan", candidate.uri); + return 0; +} diff --git a/src/components/sidebar/pages/desktop_page.vala b/src/components/sidebar/pages/desktop_page.vala index 4a38423..425cb1b 100644 --- a/src/components/sidebar/pages/desktop_page.vala +++ b/src/components/sidebar/pages/desktop_page.vala @@ -5,16 +5,6 @@ using Singularity.Widgets; namespace Singularity { - internal class WallpaperCandidate : Object { - public string uri { get; private set; } - public bool is_recent { get; private set; } - - public WallpaperCandidate(string uri, bool is_recent) { - this.uri = uri; - this.is_recent = is_recent; - } - } - public class DesktopPage : SettingsPage { private GLib.Settings settings; private GLib.Settings? wm_settings; @@ -1896,97 +1886,10 @@ namespace Singularity { }); } - // How deep to walk below a scan root. /usr/share/backgrounds holds - // ncz/, and a pack sits one further down (ncz/brandon-perlow), so two - // levels is what the shipped layout needs. The bound exists because - // $XDG_DATA_HOME/backgrounds is user-writable: someone who points it at - // a deep tree should not stall the picker. - private const int WALLPAPER_SCAN_MAX_DEPTH = 3; - - // Walk one scan root, collecting images. - // - // The previous implementation enumerated a single level and kept only - // entries whose content-type began with image/. /usr/share/backgrounds - // contains no images at all -- only ncz/ and singularity/ -- and a - // directory's content-type is inode/directory, so every shipped - // wallpaper was silently skipped. The picker had never displayed them. - private static void scan_wallpaper_dir(string path, - ArrayList candidates, - HashSet thread_seen, - HashSet visited_dirs, - int depth) { - if (depth > WALLPAPER_SCAN_MAX_DEPTH) return; - // The scan roots overlap by construction (/usr/share/backgrounds and - // /usr/share/backgrounds/singularity are both roots) and a pack may - // declare a Dir already reachable from one of them. Without this, - // those directories are walked more than once. - if (visited_dirs.contains(path)) return; - visited_dirs.add(path); - - try { - var dir = File.new_for_path(path); - if (!dir.query_exists()) return; - var enumerator = dir.enumerate_children( - "standard::name,standard::content-type,standard::type,standard::is-symlink,standard::symlink-target", - FileQueryInfoFlags.NONE, null); - FileInfo info; - while ((info = enumerator.next_file(null)) != null) { - var child = dir.get_child(info.get_name()); - - if (info.get_file_type() == FileType.DIRECTORY) { - // Not followed as a directory either: a symlinked - // directory is the easy way to walk in a circle. - if (info.get_is_symlink()) continue; - scan_wallpaper_dir(child.get_path(), candidates, thread_seen, - visited_dirs, depth + 1); - continue; - } - - // default.jpg is a symlink the rotator repoints at whichever - // wallpaper is current, at a target enumerated in this same - // directory -- following it would list one image twice, once - // under its own name and once as "default". Only elide a - // same-directory pointer like that one: a pack that ships an - // image as a symlink to a shared asset OUTSIDE this directory - // is real content, and the previous scanner listed it fine - // (content-type resolves through the link either way, since - // enumerate_children above passes no NOFOLLOW flag). - if (info.get_is_symlink()) { - string? target = info.get_symlink_target(); - if (target != null) { - string resolved = Path.is_absolute(target) - ? target - : Path.build_filename(path, target); - if (Path.get_dirname(resolved) == path) continue; - } - } - - string mime = info.get_content_type(); - if (mime == null || !mime.has_prefix("image/")) continue; - - string uri = child.get_uri(); - if (thread_seen.contains(uri)) continue; - thread_seen.add(uri); - candidates.add(new WallpaperCandidate(uri, false)); - } - } catch (Error e) { - } - } - private void populate_grid() { int gen = ++wallpaper_grid_generation; wallpaper_grid.remove_all(); - var uris = new ArrayList(); string[] recent = settings.get_strv("recent-wallpapers"); - foreach (string uri in recent) { - if (!uris.contains(uri)) { - uris.add(uri); - add_wallpaper_card(uri, true); - } - } - - var seen = new HashSet(); - foreach (string uri in recent) seen.add(uri); string selected_id = rotation_state.get_selected_collection("ncz"); string? scan_dir = null; @@ -2002,18 +1905,15 @@ namespace Singularity { scan_dir = wallpaper_collections[0].dir; } - string[] scan_paths = (scan_dir == null) ? new string[0] : new string[] { scan_dir }; + var collection_dirs = new ArrayList(); + foreach (var collection in wallpaper_collections) collection_dirs.add(collection.dir); new GLib.Thread("wallpaper-scan", () => { - var candidates = new ArrayList(); - var thread_seen = new HashSet(); - foreach (string uri in seen) thread_seen.add(uri); - - var visited_dirs = new HashSet(); - foreach (string path in scan_paths) - scan_wallpaper_dir(path, candidates, thread_seen, visited_dirs, 0); + var candidates = WallpaperGallery.scan(scan_dir, collection_dirs.to_array(), recent); GLib.Idle.add(() => { if (gen != wallpaper_grid_generation) return GLib.Source.REMOVE; + debug("Wallpaper gallery: source=%s root=%s images=%d", + selected_id, scan_dir ?? "(none)", candidates.size); append_wallpaper_candidates(candidates, gen, 0); return GLib.Source.REMOVE; }); diff --git a/src/core/wallpaper_gallery.vala b/src/core/wallpaper_gallery.vala new file mode 100644 index 0000000..26d1ca3 --- /dev/null +++ b/src/core/wallpaper_gallery.vala @@ -0,0 +1,108 @@ +using GLib; +using Gee; + +namespace Singularity { + internal class WallpaperCandidate : Object { + public string uri { get; private set; } + public bool is_recent { get; private set; } + + public WallpaperCandidate(string uri, bool is_recent) { + this.uri = uri; + this.is_recent = is_recent; + } + } + + internal class WallpaperGallery : Object { + // Membership comes from the selected scan. History can reorder its + // members, but must never introduce images from another source. + public static ArrayList scan(string? selected_dir, + string[] collection_dirs, + string[] recent) { + var scanned = new ArrayList(); + var members = new HashSet(); + var excluded = new HashSet(); + var result = new ArrayList(); + if (selected_dir == null) return result; + string root = File.new_for_path(selected_dir).get_path(); + foreach (string dir in collection_dirs) { + string other = File.new_for_path(dir).get_path(); + if (other != root) excluded.add(other); + } + scan_wallpaper_dir(root, scanned, members, new HashSet(), excluded, 0); + var added = new HashSet(); + foreach (string uri in recent) { + if (members.contains(uri) && added.add(uri)) + result.add(new WallpaperCandidate(uri, true)); + } + foreach (var candidate in scanned) { + if (added.add(candidate.uri)) result.add(candidate); + } + return result; + } + + // Bound traversal of user-controlled collection directories. + private const int WALLPAPER_SCAN_MAX_DEPTH = 3; + + // Other registered roots are separate sources, even when nested. + private static void scan_wallpaper_dir(string path, + ArrayList candidates, + HashSet thread_seen, + HashSet visited_dirs, + HashSet excluded_dirs, + int depth) { + if (depth > WALLPAPER_SCAN_MAX_DEPTH || excluded_dirs.contains(path)) return; + if (visited_dirs.contains(path)) return; + visited_dirs.add(path); + + try { + var dir = File.new_for_path(path); + if (!dir.query_exists()) return; + var enumerator = dir.enumerate_children( + "standard::name,standard::content-type,standard::type,standard::is-symlink,standard::symlink-target", + FileQueryInfoFlags.NONE, null); + FileInfo info; + while ((info = enumerator.next_file(null)) != null) { + var child = dir.get_child(info.get_name()); + + if (info.get_file_type() == FileType.DIRECTORY) { + // Not followed as a directory either: a symlinked + // directory is the easy way to walk in a circle. + if (info.get_is_symlink()) continue; + scan_wallpaper_dir(child.get_path(), candidates, thread_seen, + visited_dirs, excluded_dirs, depth + 1); + continue; + } + + // default.jpg is a symlink the rotator repoints at whichever + // wallpaper is current, at a target enumerated in this same + // directory -- following it would list one image twice, once + // under its own name and once as "default". Only elide a + // same-directory pointer like that one: a pack that ships an + // image as a symlink to a shared asset OUTSIDE this directory + // is real content, and the previous scanner listed it fine + // (content-type resolves through the link either way, since + // enumerate_children above passes no NOFOLLOW flag). + if (info.get_is_symlink()) { + string? target = info.get_symlink_target(); + if (target != null) { + string resolved = Path.is_absolute(target) + ? target + : Path.build_filename(path, target); + if (Path.get_dirname(resolved) == path) continue; + } + } + + string mime = info.get_content_type(); + if (mime == null || !mime.has_prefix("image/")) continue; + + string uri = child.get_uri(); + if (thread_seen.contains(uri)) continue; + thread_seen.add(uri); + candidates.add(new WallpaperCandidate(uri, false)); + } + } catch (Error e) { + } + } + + } +} diff --git a/tests/wallpaper_gallery_test.vala b/tests/wallpaper_gallery_test.vala new file mode 100644 index 0000000..e301e9b --- /dev/null +++ b/tests/wallpaper_gallery_test.vala @@ -0,0 +1,81 @@ +using GLib; +using Gee; +using Singularity; + +private string fixture_root; +private string ncz; +private string artist; +private string bing; + +private string image_file(string dir, string name) { + string path = Path.build_filename(dir, name + ".svg"); + try { + DirUtils.create_with_parents(dir, 0700); + FileUtils.set_contents(path, ""); + } catch (Error e) { error("fixture: %s", e.message); } + return File.new_for_path(path).get_uri(); +} + +private void test_recent_membership() { + string base_uri = image_file(ncz, "base"); + string art_uri = image_file(artist, "art"); + string bing_uri = image_file(bing, "daily"); + var result = WallpaperGallery.scan(artist, {ncz, artist, bing}, + {bing_uri, base_uri, art_uri, art_uri}); + assert(result.size == 1); + assert(result[0].uri == art_uri); + assert(result[0].is_recent); +} + +private void test_nested_collection_boundary() { + string base_uri = image_file(ncz, "base"); + string art_uri = image_file(artist, "art"); + var result = WallpaperGallery.scan(ncz + "/./", {ncz, artist + "/", bing}, {art_uri}); + assert(result.size == 1); + assert(result[0].uri == base_uri); + assert(!result[0].is_recent); +} + +private void test_provider_subdirectories() { + string daily = image_file(Path.build_filename(bing, "en-US"), "nested"); + var result = WallpaperGallery.scan(bing, {ncz, artist, bing}, {}); + assert(result.size == 2); + bool found = false; + foreach (var candidate in result) if (candidate.uri == daily) found = true; + assert(found); +} + +private void test_missing_collection() { + string uri = image_file(ncz, "base"); + assert(WallpaperGallery.scan(null, {ncz}, {uri}).size == 0); + assert(WallpaperGallery.scan(fixture_root + "/missing", {ncz}, {uri}).size == 0); +} + +private void test_default_alias_and_directory_loop() { + try { + File.new_for_path(ncz + "/default.svg").make_symbolic_link("base.svg"); + File.new_for_path(ncz + "/loop").make_symbolic_link(ncz); + } catch (Error e) { error("fixture: %s", e.message); } + string alias = File.new_for_path(ncz + "/default.svg").get_uri(); + var result = WallpaperGallery.scan(ncz, {ncz, artist, bing}, {alias}); + assert(result.size == 1); + assert(!result[0].is_recent); +} + +public int main(string[] args) { + Test.init(ref args); + try { fixture_root = DirUtils.make_tmp("wallpaper-gallery-XXXXXX"); } + catch (Error e) { error("fixture: %s", e.message); } + ncz = fixture_root + "/ncz"; + artist = ncz + "/artist"; + bing = fixture_root + "/bing"; + image_file(ncz, "base"); + image_file(artist, "art"); + image_file(bing, "daily"); + Test.add_func("/wallpaper-gallery/recent-membership", test_recent_membership); + Test.add_func("/wallpaper-gallery/nested-boundary", test_nested_collection_boundary); + Test.add_func("/wallpaper-gallery/provider-subdirectories", test_provider_subdirectories); + Test.add_func("/wallpaper-gallery/missing-collection", test_missing_collection); + Test.add_func("/wallpaper-gallery/default-alias-loop", test_default_alias_and_directory_loop); + return Test.run(); +} From 6a650a48b662f7cdff87ba942b8fd9bb037e5398 Mon Sep 17 00:00:00 2001 From: Jason Perlow Date: Sun, 6 Sep 2026 12:09:22 -0400 Subject: [PATCH 05/11] chore(wallpaper): drop dev-only gallery probe executable Not something an upstream maintainer should receive as part of the feature -- it was a local live-state debugging aid, not a test or a build artifact anything else depends on. --- meson.build | 7 ------- scripts/wallpaper-gallery-probe.vala | 28 ---------------------------- 2 files changed, 35 deletions(-) delete mode 100644 scripts/wallpaper-gallery-probe.vala diff --git a/meson.build b/meson.build index ad77c93..03d85c1 100644 --- a/meson.build +++ b/meson.build @@ -463,10 +463,3 @@ wallpaper_gallery_test = executable('wallpaper-gallery-test', dependencies: [dependency('gobject-2.0'), dependency('gio-2.0'), gee_dep], ) test('wallpaper-gallery', wallpaper_gallery_test) - -executable('wallpaper-gallery-probe', - sources: ['src/core/wallpaper_gallery.vala', 'src/core/wallpaper_collections.vala', - 'src/core/wallpaper_rotation_state.vala', 'scripts/wallpaper-gallery-probe.vala'], - dependencies: [dependency('gobject-2.0'), dependency('gio-2.0'), gee_dep], - install: false, -) diff --git a/scripts/wallpaper-gallery-probe.vala b/scripts/wallpaper-gallery-probe.vala deleted file mode 100644 index f668304..0000000 --- a/scripts/wallpaper-gallery-probe.vala +++ /dev/null @@ -1,28 +0,0 @@ -// Run in the desktop user's environment to report the production scanner's -// candidates against the installed registry and current GSettings history. -using GLib; -using Gee; -using Singularity; - -int main(string[] args) { - var roots = new ArrayList(); - foreach (unowned string dir in Environment.get_system_data_dirs()) - roots.add(Path.build_filename(dir, "ncz-wallpapers", "collections")); - roots.add(Path.build_filename(Environment.get_user_data_dir(), "ncz-wallpapers", "collections")); - var collections = WallpaperCollections.parse(roots.to_array()); - var state = new WallpaperRotationState(Path.build_filename(Environment.get_user_config_dir(), "ncz-wallpaper")); - string selected = state.get_selected_collection("ncz"); - string? root = null; - var dirs = new ArrayList(); - foreach (var collection in collections) { - dirs.add(collection.dir); - if (collection.id == selected) root = collection.dir; - } - if (root == null && collections.size > 0) root = collections[0].dir; - var settings = new GLib.Settings("dev.sinty.desktop"); - var candidates = WallpaperGallery.scan(root, dirs.to_array(), settings.get_strv("recent-wallpapers")); - stdout.printf("source=%s root=%s images=%d\n", selected, root ?? "(none)", candidates.size); - foreach (var candidate in candidates) - stdout.printf("%s\t%s\n", candidate.is_recent ? "recent" : "scan", candidate.uri); - return 0; -} From 62f6eec016b26f843c7c7c1eafec35f09e9950a4 Mon Sep 17 00:00:00 2001 From: Jason Perlow Date: Sun, 6 Sep 2026 12:09:31 -0400 Subject: [PATCH 06/11] fix(wallpaper): address adversarial review findings Five real correctness issues found by an adversarial review of the prior four commits before opening this as an upstream PR: - WallpaperRotationState.write(): write-then-rename instead of a direct set_contents, so the rotator daemon (which polls these files on its own timer, independent of this UI) can never observe a partial write. - get_rotate_enabled(): accept "false"/"off" as disabled, not only the literal "0" the UI itself writes -- a daemon written to a slightly different convention would otherwise read as always-on. - WallpaperCollections.parse(): trim KeyFile string values (Dir/Id/Name/ Artist/Type). GLib.KeyFile permits "Key = value" with surrounding whitespace; an untrimmed Dir would silently fail every path comparison downstream, and an untrimmed Id would defeat duplicate-id detection. - WallpaperGallery.scan(): canonicalize paths (Posix.realpath) before comparing collection roots for the source-boundary exclusion, so a pack installed through a symlinked directory is still recognized as the same root instead of leaking into another source's scan. - desktop_page.vala populate_grid(): when the persisted selection doesn't match any installed collection, persist the fallback (rather than re-deriving and re-logging the same mismatch on every refresh). Verified via a real container build (SINGULARITY_SOURCE_DIR override, build-singularity.sh) on ULTRA. --- .../sidebar/pages/desktop_page.vala | 6 ++++- src/core/wallpaper_collections.vala | 22 ++++++++++--------- src/core/wallpaper_gallery.vala | 14 ++++++++++-- src/core/wallpaper_rotation_state.vala | 15 +++++++++++-- 4 files changed, 42 insertions(+), 15 deletions(-) diff --git a/src/components/sidebar/pages/desktop_page.vala b/src/components/sidebar/pages/desktop_page.vala index 425cb1b..ac94a06 100644 --- a/src/components/sidebar/pages/desktop_page.vala +++ b/src/components/sidebar/pages/desktop_page.vala @@ -1900,9 +1900,13 @@ namespace Singularity { // state file) must not empty the grid silently -- fall back to // whatever the first known collection is, same "never leave the // desktop with no wallpaper" principle the rotator script itself - // follows. + // follows. Persist the fallback so the source row and the state + // file agree with what's actually on screen instead of re-falling + // back (and re-logging the same mismatch) on every refresh. if (scan_dir == null && wallpaper_collections.size > 0) { + selected_id = wallpaper_collections[0].id; scan_dir = wallpaper_collections[0].dir; + rotation_state.set_selected_collection(selected_id); } var collection_dirs = new ArrayList(); diff --git a/src/core/wallpaper_collections.vala b/src/core/wallpaper_collections.vala index 1cf03b6..4f2061c 100644 --- a/src/core/wallpaper_collections.vala +++ b/src/core/wallpaper_collections.vala @@ -61,34 +61,36 @@ namespace Singularity { string collection_dir; try { - collection_dir = kf.get_string("Collection", "Dir"); + collection_dir = kf.get_string("Collection", "Dir").strip(); } catch (Error e) { continue; // Dir-less collection, skip it } - if (collection_dir == null || collection_dir == "") continue; + if (collection_dir == "") continue; string id; try { - id = kf.get_string("Collection", "Id"); + id = kf.get_string("Collection", "Id").strip(); } catch (Error e) { - id = filename.substring(0, filename.length - ".collection".length); + id = ""; } - if (id == null || id == "") { + if (id == "") { id = filename.substring(0, filename.length - ".collection".length); } if (!seen_ids.add(id)) continue; // first root wins string name; - try { name = kf.get_string("Collection", "Name"); } - catch (Error e) { name = id; } + try { name = kf.get_string("Collection", "Name").strip(); } + catch (Error e) { name = ""; } + if (name == "") name = id; string artist; - try { artist = kf.get_string("Collection", "Artist"); } + try { artist = kf.get_string("Collection", "Artist").strip(); } catch (Error e) { artist = ""; } string type; - try { type = kf.get_string("Collection", "Type"); } - catch (Error e) { type = "static"; } + try { type = kf.get_string("Collection", "Type").strip(); } + catch (Error e) { type = ""; } + if (type == "") type = "static"; results.add(new WallpaperCollectionInfo(id, name, artist, collection_dir, type)); } diff --git a/src/core/wallpaper_gallery.vala b/src/core/wallpaper_gallery.vala index 26d1ca3..6271ab4 100644 --- a/src/core/wallpaper_gallery.vala +++ b/src/core/wallpaper_gallery.vala @@ -23,9 +23,9 @@ namespace Singularity { var excluded = new HashSet(); var result = new ArrayList(); if (selected_dir == null) return result; - string root = File.new_for_path(selected_dir).get_path(); + string root = canonical_path(selected_dir); foreach (string dir in collection_dirs) { - string other = File.new_for_path(dir).get_path(); + string other = canonical_path(dir); if (other != root) excluded.add(other); } scan_wallpaper_dir(root, scanned, members, new HashSet(), excluded, 0); @@ -43,6 +43,16 @@ namespace Singularity { // Bound traversal of user-controlled collection directories. private const int WALLPAPER_SCAN_MAX_DEPTH = 3; + // Dir= values across .collection files may point at the same + // directory through different symlinks (a pack install living + // outside /usr/share is a common layout) -- resolve to the real + // path before comparing, or the source-boundary exclusion above + // silently fails to recognize them as the same root. + private static string canonical_path(string path) { + string? real = Posix.realpath(path, null); + return real ?? File.new_for_path(path).get_path(); + } + // Other registered roots are separate sources, even when nested. private static void scan_wallpaper_dir(string path, ArrayList candidates, diff --git a/src/core/wallpaper_rotation_state.vala b/src/core/wallpaper_rotation_state.vala index a09313f..dddafd5 100644 --- a/src/core/wallpaper_rotation_state.vala +++ b/src/core/wallpaper_rotation_state.vala @@ -36,8 +36,17 @@ namespace Singularity { private void write(string filename, string contents) { GLib.DirUtils.create_with_parents(config_dir, 0700); + string dest = path_for(filename); + string tmp = dest + ".tmp"; try { - FileUtils.set_contents(path_for(filename), contents); + // Write-then-rename: the rotator daemon polls these files on + // its own timer, so a partial write it reads mid-flush would + // be picked up as-is. rename(2) within the same directory is + // atomic, so the daemon only ever sees a complete write. + FileUtils.set_contents(tmp, contents); + if (FileUtils.rename(tmp, dest) != 0) { + warning("wallpaper rotation state: could not rename %s into place", filename); + } } catch (Error e) { warning("wallpaper rotation state: could not write %s: %s", filename, e.message); } @@ -54,7 +63,9 @@ namespace Singularity { public bool get_rotate_enabled() { string? value = read_trimmed("rotate-enabled"); - return value != "0"; + if (value == null) return true; + string lowered = value.down(); + return lowered != "0" && lowered != "false" && lowered != "off"; } public void set_rotate_enabled(bool enabled) { From 213d0bef5dbbc89f60939fda65416730099621fd Mon Sep 17 00:00:00 2001 From: Jason Perlow Date: Sat, 12 Sep 2026 14:58:57 -0400 Subject: [PATCH 07/11] fix(wallpaper): decouple rotation state and pack registry from NCZ Replaces the NCZ-specific paths and default id that leaked into the wallpaper source selector / rotation feature with Singularity-owned equivalents, addressing Mirko's review on this PR: - Rotation state directory moves from ~/.config/ncz-wallpaper to ~/.config/singularity/wallpaper-rotation, matching the singularity/ convention already used by search_manager.vala (singularity/search-providers) and display_manager.vala (singularity/displays.json). - The pack registry search roots move from /ncz-wallpapers/ collections to /singularity/wallpaper-collections, so any downstream OS or pack installer -- not just NCZ -- can drop a .collection file there and have it appear in the picker. - The hardcoded "ncz" default collection id is replaced with "", which the existing "no matching collection" fallback (already present in both call sites) turns into "use whatever collection is actually installed" instead of assuming one named ncz exists. - WallpaperRotationState's doc comment no longer names cix-installer's ncz-wallpaper-rotate/ncz-wallpaper-daemon scripts; it now documents the three state files (collection, rotate-enabled, rotate-interval) as a plain, project-owned contract that any rotation daemon can implement, since that's what this class actually is regardless of which daemon reads it. - Test fixtures that used "ncz" purely as a sample id/dir name are renamed to generic placeholders (vendor, system, default) for consistency; WallpaperCollections and WallpaperGallery were already fully generic and needed no logic changes. No behavior changes for existing installs beyond the config paths moving -- the fallback-to-first-available-collection logic already present in populate_grid() and the initial-selection code is what now does the full job the "ncz" default used to do. Verified: full singularity-desktop build (meson setup + ninja) against libsingularity built from the same tree, and `meson test` -- all 8 suites pass, including the 19 wallpaper-collections/-gallery/ -rotation-state tests. --- .../sidebar/pages/desktop_page.vala | 18 ++++++++--- src/core/wallpaper_rotation_state.vala | 16 ++++++---- tests/wallpaper_collections_test.vala | 20 ++++++------ tests/wallpaper_gallery_test.vala | 32 +++++++++---------- tests/wallpaper_rotation_state_test.vala | 6 ++-- 5 files changed, 52 insertions(+), 40 deletions(-) diff --git a/src/components/sidebar/pages/desktop_page.vala b/src/components/sidebar/pages/desktop_page.vala index ac94a06..c6161cc 100644 --- a/src/components/sidebar/pages/desktop_page.vala +++ b/src/components/sidebar/pages/desktop_page.vala @@ -23,7 +23,7 @@ namespace Singularity { private FlowBox wallpaper_grid; private Gee.ArrayList wallpaper_collections = new Gee.ArrayList(); private WallpaperRotationState rotation_state = new WallpaperRotationState( - GLib.Path.build_filename(GLib.Environment.get_user_config_dir(), "ncz-wallpaper")); + GLib.Path.build_filename(GLib.Environment.get_user_config_dir(), "singularity", "wallpaper-rotation")); private int wallpaper_grid_generation = 0; private int wallpaper_accent_generation = 0; private string cached_wallpaper_accent = "#3584e4"; @@ -172,11 +172,16 @@ namespace Singularity { add_group(preview_group); var grid_group = new PreferencesGroup(_("Wallpapers")); + // "singularity/wallpaper-collections" is a project-owned registry + // location, not a specific vendor's: any downstream OS or pack + // installer can drop a .collection file here to have its wallpapers + // appear in this picker (see WallpaperCollections' class doc for the + // file format). var collection_roots = new Gee.ArrayList(); foreach (unowned string d in GLib.Environment.get_system_data_dirs()) - collection_roots.add(GLib.Path.build_filename(d, "ncz-wallpapers", "collections")); + collection_roots.add(GLib.Path.build_filename(d, "singularity", "wallpaper-collections")); collection_roots.add(GLib.Path.build_filename( - GLib.Environment.get_user_data_dir(), "ncz-wallpapers", "collections")); + GLib.Environment.get_user_data_dir(), "singularity", "wallpaper-collections")); wallpaper_collections = WallpaperCollections.parse(collection_roots.to_array()); var source_options = new Gee.ArrayList(); @@ -188,7 +193,10 @@ namespace Singularity { id = collection.id, label = label }); } - string initial_collection_id = rotation_state.get_selected_collection("ncz"); + // "" is not a real collection id -- it's just a value guaranteed + // not to match anything, so the fallback below always picks the + // first discovered collection when no prior selection is on disk. + string initial_collection_id = rotation_state.get_selected_collection(""); bool have_initial = false; foreach (var opt in source_options) if (opt.id == initial_collection_id) have_initial = true; if (!have_initial && source_options.size > 0) initial_collection_id = source_options[0].id; @@ -1891,7 +1899,7 @@ namespace Singularity { wallpaper_grid.remove_all(); string[] recent = settings.get_strv("recent-wallpapers"); - string selected_id = rotation_state.get_selected_collection("ncz"); + string selected_id = rotation_state.get_selected_collection(""); string? scan_dir = null; foreach (var collection in wallpaper_collections) { if (collection.id == selected_id) { scan_dir = collection.dir; break; } diff --git a/src/core/wallpaper_rotation_state.vala b/src/core/wallpaper_rotation_state.vala index dddafd5..bfba474 100644 --- a/src/core/wallpaper_rotation_state.vala +++ b/src/core/wallpaper_rotation_state.vala @@ -2,12 +2,16 @@ using GLib; namespace Singularity { - // Reads and writes the plain-text state files - // cix-installer/post-install/45-wallpaper-rotator.sh's ncz-wallpaper-rotate - // and ncz-wallpaper-daemon shell scripts already poll every rotation cycle - // -- this class is the UI's side of that same shared state, not a new - // mechanism. config_dir is injected (rather than read from - // GLib.Environment here) so it's testable against a temp directory. + // Reads and writes the plain-text rotation-state files under + // $XDG_CONFIG_HOME/singularity/wallpaper-rotation/: "collection" (the + // active collection id), "rotate-enabled" ("1"/"0") and + // "rotate-interval" (seconds). This is the documented, project-owned + // contract for wallpaper rotation -- any background daemon that wants to + // actually change the desktop wallpaper on a timer polls these files and + // this class is only the shell UI's side of that same shared state, not + // a new or vendor-specific mechanism. config_dir is injected (rather + // than read from GLib.Environment here) so it's testable against a temp + // directory. public class WallpaperRotationState : Object { private const int DEFAULT_INTERVAL_SECONDS = 600; private const int MIN_INTERVAL_SECONDS = 30; diff --git a/tests/wallpaper_collections_test.vala b/tests/wallpaper_collections_test.vala index 9fdd19d..8c65a69 100644 --- a/tests/wallpaper_collections_test.vala +++ b/tests/wallpaper_collections_test.vala @@ -24,7 +24,7 @@ private void test_parses_id_name_artist_dir() { "Name=Brandon Perlow\n" + "Artist=Brandon Perlow\n" + "Type=static\n" + - "Dir=/usr/share/backgrounds/ncz/brandon-perlow\n"); + "Dir=/usr/share/backgrounds/vendor/brandon-perlow\n"); var result = WallpaperCollections.parse({ root }); @@ -32,21 +32,21 @@ private void test_parses_id_name_artist_dir() { assert(result[0].id == "brandon-perlow"); assert(result[0].name == "Brandon Perlow"); assert(result[0].artist == "Brandon Perlow"); - assert(result[0].dir == "/usr/share/backgrounds/ncz/brandon-perlow"); + assert(result[0].dir == "/usr/share/backgrounds/vendor/brandon-perlow"); assert(result[0].type == "static"); } private void test_id_falls_back_to_filename_stem() { string root = make_tmp_dir(); - write_collection(root, "ncz.collection", + write_collection(root, "vendor.collection", "[Collection]\n" + - "Name=NCZ-OS\n" + - "Dir=/usr/share/backgrounds/ncz\n"); + "Name=Vendor OS\n" + + "Dir=/usr/share/backgrounds/vendor\n"); var result = WallpaperCollections.parse({ root }); assert(result.size == 1); - assert(result[0].id == "ncz"); + assert(result[0].id == "vendor"); } private void test_skips_dir_less_collection() { @@ -79,10 +79,10 @@ private void test_ignores_non_collection_files_and_missing_dirs() { private void test_dedupes_by_id_first_root_wins() { string root_a = make_tmp_dir(); string root_b = make_tmp_dir(); - write_collection(root_a, "ncz.collection", - "[Collection]\nId=ncz\nName=System\nDir=/system/ncz\n"); - write_collection(root_b, "ncz.collection", - "[Collection]\nId=ncz\nName=User Override\nDir=/user/ncz\n"); + write_collection(root_a, "vendor.collection", + "[Collection]\nId=vendor\nName=System\nDir=/system/vendor\n"); + write_collection(root_b, "vendor.collection", + "[Collection]\nId=vendor\nName=User Override\nDir=/user/vendor\n"); var result = WallpaperCollections.parse({ root_a, root_b }); diff --git a/tests/wallpaper_gallery_test.vala b/tests/wallpaper_gallery_test.vala index e301e9b..98fa47d 100644 --- a/tests/wallpaper_gallery_test.vala +++ b/tests/wallpaper_gallery_test.vala @@ -3,7 +3,7 @@ using Gee; using Singularity; private string fixture_root; -private string ncz; +private string system_dir; private string artist; private string bing; @@ -17,10 +17,10 @@ private string image_file(string dir, string name) { } private void test_recent_membership() { - string base_uri = image_file(ncz, "base"); + string base_uri = image_file(system_dir, "base"); string art_uri = image_file(artist, "art"); string bing_uri = image_file(bing, "daily"); - var result = WallpaperGallery.scan(artist, {ncz, artist, bing}, + var result = WallpaperGallery.scan(artist, {system_dir, artist, bing}, {bing_uri, base_uri, art_uri, art_uri}); assert(result.size == 1); assert(result[0].uri == art_uri); @@ -28,9 +28,9 @@ private void test_recent_membership() { } private void test_nested_collection_boundary() { - string base_uri = image_file(ncz, "base"); + string base_uri = image_file(system_dir, "base"); string art_uri = image_file(artist, "art"); - var result = WallpaperGallery.scan(ncz + "/./", {ncz, artist + "/", bing}, {art_uri}); + var result = WallpaperGallery.scan(system_dir + "/./", {system_dir, artist + "/", bing}, {art_uri}); assert(result.size == 1); assert(result[0].uri == base_uri); assert(!result[0].is_recent); @@ -38,7 +38,7 @@ private void test_nested_collection_boundary() { private void test_provider_subdirectories() { string daily = image_file(Path.build_filename(bing, "en-US"), "nested"); - var result = WallpaperGallery.scan(bing, {ncz, artist, bing}, {}); + var result = WallpaperGallery.scan(bing, {system_dir, artist, bing}, {}); assert(result.size == 2); bool found = false; foreach (var candidate in result) if (candidate.uri == daily) found = true; @@ -46,18 +46,18 @@ private void test_provider_subdirectories() { } private void test_missing_collection() { - string uri = image_file(ncz, "base"); - assert(WallpaperGallery.scan(null, {ncz}, {uri}).size == 0); - assert(WallpaperGallery.scan(fixture_root + "/missing", {ncz}, {uri}).size == 0); + string uri = image_file(system_dir, "base"); + assert(WallpaperGallery.scan(null, {system_dir}, {uri}).size == 0); + assert(WallpaperGallery.scan(fixture_root + "/missing", {system_dir}, {uri}).size == 0); } private void test_default_alias_and_directory_loop() { try { - File.new_for_path(ncz + "/default.svg").make_symbolic_link("base.svg"); - File.new_for_path(ncz + "/loop").make_symbolic_link(ncz); + File.new_for_path(system_dir + "/default.svg").make_symbolic_link("base.svg"); + File.new_for_path(system_dir + "/loop").make_symbolic_link(system_dir); } catch (Error e) { error("fixture: %s", e.message); } - string alias = File.new_for_path(ncz + "/default.svg").get_uri(); - var result = WallpaperGallery.scan(ncz, {ncz, artist, bing}, {alias}); + string alias = File.new_for_path(system_dir + "/default.svg").get_uri(); + var result = WallpaperGallery.scan(system_dir, {system_dir, artist, bing}, {alias}); assert(result.size == 1); assert(!result[0].is_recent); } @@ -66,10 +66,10 @@ public int main(string[] args) { Test.init(ref args); try { fixture_root = DirUtils.make_tmp("wallpaper-gallery-XXXXXX"); } catch (Error e) { error("fixture: %s", e.message); } - ncz = fixture_root + "/ncz"; - artist = ncz + "/artist"; + system_dir = fixture_root + "/system"; + artist = system_dir + "/artist"; bing = fixture_root + "/bing"; - image_file(ncz, "base"); + image_file(system_dir, "base"); image_file(artist, "art"); image_file(bing, "daily"); Test.add_func("/wallpaper-gallery/recent-membership", test_recent_membership); diff --git a/tests/wallpaper_rotation_state_test.vala b/tests/wallpaper_rotation_state_test.vala index 4bc8262..7938ebc 100644 --- a/tests/wallpaper_rotation_state_test.vala +++ b/tests/wallpaper_rotation_state_test.vala @@ -7,13 +7,13 @@ private string make_tmp_dir() { private void test_selected_collection_defaults_when_unset() { var state = new WallpaperRotationState(make_tmp_dir()); - assert(state.get_selected_collection("ncz") == "ncz"); + assert(state.get_selected_collection("default") == "default"); } private void test_selected_collection_roundtrips() { var state = new WallpaperRotationState(make_tmp_dir()); state.set_selected_collection("brandon-perlow"); - assert(state.get_selected_collection("ncz") == "brandon-perlow"); + assert(state.get_selected_collection("default") == "brandon-perlow"); } private void test_selected_collection_strips_whitespace() { @@ -22,7 +22,7 @@ private void test_selected_collection_strips_whitespace() { FileUtils.set_contents(GLib.Path.build_filename(dir, "collection"), " bing \n"); } catch (Error e) { error("test setup failed: %s", e.message); } var state = new WallpaperRotationState(dir); - assert(state.get_selected_collection("ncz") == "bing"); + assert(state.get_selected_collection("default") == "bing"); } private void test_rotate_enabled_defaults_true() { From 9a118556e19c891be2833b3e3dca5d96058b7007 Mon Sep 17 00:00:00 2001 From: Jason Perlow Date: Sat, 12 Sep 2026 17:25:29 -0400 Subject: [PATCH 08/11] feat(wallpaper): wire rotation settings to a real rotator The rotation state files under $XDG_CONFIG_HOME/singularity/ wallpaper-rotation had no reader, so "Rotate Wallpapers" and "Rotation Interval" persisted a preference nothing acted on. WallpaperRotator is that reader, in the shell process rather than a separate daemon or a new IPC surface: the shell already owns the wallpaper through WallpaperManager and already repaints on a background-picture-uri change, so a rotation is a timer plus the same settings write the gallery makes, and the crossfade, rescaling, preview and accent extraction come along unchanged. A separate binary would need its own copy of the collection parsing, its own schema lookup against the shell's prefix, and a session unit that reliably starts. - rotate-enabled decides whether a timer exists at all - rotate-interval is read when the timer is armed, and a FileMonitor on the state directory re-arms on any write, so a change applies immediately rather than after the current period - collection scopes the pick to that registry entry only - the pick is random but never the wallpaper already showing - an empty pack, a provider that fetched nothing or a stale collection id leaves the current wallpaper alone - suppressed in safe mode with the other optional startup work The scan runs on a worker thread, as populate_grid() already does for the identical walk -- it is filesystem I/O and this main loop is the compositor's. The registry roots and the state directory each had their path spelled out inline in the settings page; both now come from one accessor, so the writer and the reader cannot drift apart. Tests (13 cases) cover the pick (deterministic under an injected roll), collection scoping, the stale-id and empty-pack fallbacks, the threaded rotation hand-off, and that the switch and interval govern the armed timer. Checked by mutation: making pick() ignore the current wallpaper, reschedule() ignore rotate-enabled, and rotate_async() never announce, each fails its own test. meson test: 12/12 suites pass on aarch64. Assisted-by: Claude Code:claude-opus-5 AI-Scope: generated wallpaper_rotator.vala, its test, and the start_rotation() wiring, from a brief to build a real consumer for the existing rotation-state files using the shell's current display path. --- meson.build | 9 + .../sidebar/pages/desktop_page.vala | 12 +- src/core/main.vala | 9 + src/core/wallpaper_collections.vala | 15 + src/core/wallpaper_manager.vala | 23 ++ src/core/wallpaper_rotation_state.vala | 11 + src/core/wallpaper_rotator.vala | 253 +++++++++++++++++ tests/wallpaper_rotator_test.vala | 259 ++++++++++++++++++ 8 files changed, 584 insertions(+), 7 deletions(-) create mode 100644 src/core/wallpaper_rotator.vala create mode 100644 tests/wallpaper_rotator_test.vala diff --git a/meson.build b/meson.build index 03d85c1..19510fd 100644 --- a/meson.build +++ b/meson.build @@ -285,6 +285,7 @@ singularity_core_sources = files( 'src/core/wallpaper_collections.vala', 'src/core/wallpaper_gallery.vala', 'src/core/wallpaper_rotation_state.vala', + 'src/core/wallpaper_rotator.vala', 'src/core/wayland_gamma_backend.vala', 'src/core/shortcut_manager.vala', 'src/core/ush_portal.vala', @@ -463,3 +464,11 @@ wallpaper_gallery_test = executable('wallpaper-gallery-test', dependencies: [dependency('gobject-2.0'), dependency('gio-2.0'), gee_dep], ) test('wallpaper-gallery', wallpaper_gallery_test) + +wallpaper_rotator_test = executable('wallpaper-rotator-test', + sources: ['src/core/wallpaper_collections.vala', 'src/core/wallpaper_gallery.vala', + 'src/core/wallpaper_rotation_state.vala', 'src/core/wallpaper_rotator.vala', + 'tests/wallpaper_rotator_test.vala'], + dependencies: [dependency('gobject-2.0'), dependency('glib-2.0'), dependency('gio-2.0'), gee_dep], +) +test('wallpaper-rotator', wallpaper_rotator_test) diff --git a/src/components/sidebar/pages/desktop_page.vala b/src/components/sidebar/pages/desktop_page.vala index c6161cc..d1007d1 100644 --- a/src/components/sidebar/pages/desktop_page.vala +++ b/src/components/sidebar/pages/desktop_page.vala @@ -22,8 +22,10 @@ namespace Singularity { private WallpaperPreviewWidget preview_widget; private FlowBox wallpaper_grid; private Gee.ArrayList wallpaper_collections = new Gee.ArrayList(); + // Same directory WallpaperRotator reads: these controls write the + // rotation state and the rotator in the shell process acts on it. private WallpaperRotationState rotation_state = new WallpaperRotationState( - GLib.Path.build_filename(GLib.Environment.get_user_config_dir(), "singularity", "wallpaper-rotation")); + WallpaperRotationState.default_config_dir()); private int wallpaper_grid_generation = 0; private int wallpaper_accent_generation = 0; private string cached_wallpaper_accent = "#3584e4"; @@ -177,12 +179,8 @@ namespace Singularity { // installer can drop a .collection file here to have its wallpapers // appear in this picker (see WallpaperCollections' class doc for the // file format). - var collection_roots = new Gee.ArrayList(); - foreach (unowned string d in GLib.Environment.get_system_data_dirs()) - collection_roots.add(GLib.Path.build_filename(d, "singularity", "wallpaper-collections")); - collection_roots.add(GLib.Path.build_filename( - GLib.Environment.get_user_data_dir(), "singularity", "wallpaper-collections")); - wallpaper_collections = WallpaperCollections.parse(collection_roots.to_array()); + wallpaper_collections = WallpaperCollections.parse( + WallpaperCollections.default_search_roots()); var source_options = new Gee.ArrayList(); foreach (var collection in wallpaper_collections) { diff --git a/src/core/main.vala b/src/core/main.vala index e206cb9..dd3d7bf 100644 --- a/src/core/main.vala +++ b/src/core/main.vala @@ -314,6 +314,15 @@ public class SingularityApp : Singularity.ShellApplication, Singularity.Shell.Sh // connection is established at login, not on the first search. Singularity.SearchManager.get_default(); Singularity.NowPlayingCache.get_default(); + // Wallpaper rotation: the shell is the runtime consumer of the + // rotation-state files the Desktop settings page writes, so the + // "Rotate Wallpapers" switch and interval act on something. + // Suppressed in safe mode with the other optional startup + // features -- a timer that changes persisted session state is + // not what a machine recovering from a crash loop needs. + if (Singularity.SafeMode.get_default().allows( + Singularity.SafeFeature.AUTOSTART)) + Singularity.WallpaperManager.get_default().start_rotation(); // Once the startup allocation storm (GL shader compile, icon and // theme loading) has settled, hand the freed pages back to the OS. GLib.Timeout.add_seconds(10, () => { diff --git a/src/core/wallpaper_collections.vala b/src/core/wallpaper_collections.vala index 4f2061c..46e3735 100644 --- a/src/core/wallpaper_collections.vala +++ b/src/core/wallpaper_collections.vala @@ -38,6 +38,21 @@ namespace Singularity { // here) so this class stays testable against a temp directory with no // real filesystem layout assumptions. public class WallpaperCollections : Object { + // The registry roots, in the priority order parse() documents above: + // system data dirs first, the user's own dir last so a user-installed + // collection can override one bundled with the OS. Shared by the + // settings page and the rotator -- two copies of this list drift, and + // a rotator that cannot see a pack the gallery shows is exactly the + // shape that bug takes. + public static string[] default_search_roots() { + string[] roots = {}; + foreach (unowned string d in GLib.Environment.get_system_data_dirs()) + roots += GLib.Path.build_filename(d, "singularity", "wallpaper-collections"); + roots += GLib.Path.build_filename( + GLib.Environment.get_user_data_dir(), "singularity", "wallpaper-collections"); + return roots; + } + public static Gee.ArrayList parse(string[] search_roots) { var results = new Gee.ArrayList(); var seen_ids = new Gee.HashSet(); diff --git a/src/core/wallpaper_manager.vala b/src/core/wallpaper_manager.vala index 5579b2b..8cc5bc3 100644 --- a/src/core/wallpaper_manager.vala +++ b/src/core/wallpaper_manager.vala @@ -15,6 +15,7 @@ namespace Singularity { private string? _cached_path = null; private int _load_serial = 0; private Mutex _mutex = Mutex (); + private WallpaperRotator? rotator = null; public signal void wallpaper_changed(); @@ -33,6 +34,28 @@ namespace Singularity { reload(); } + // Turns the Desktop page's rotation controls into actual behaviour: + // the rotator decides what to show and when, and this is the single + // place that decision is applied. It goes through the same + // background-picture-uri key the gallery writes, so a rotation takes + // the ordinary path -- reload() below, the crossfade in Background, + // the settings preview and the accent extraction -- rather than a + // second, parallel way to put an image on screen. + public void start_rotation() { + if (rotator != null) return; + rotator = WallpaperRotator.get_default(); + rotator.current_uri = settings.get_string("background-picture-uri"); + rotator.wallpaper_selected.connect((uri) => { + settings.set_string("background-picture-uri", uri); + }); + // A wallpaper picked by hand (or by anything else) is now the + // current one, so the next rotation must not "change" to it. + settings.changed["background-picture-uri"].connect(() => { + rotator.current_uri = settings.get_string("background-picture-uri"); + }); + rotator.start(); + } + public void reload() { string custom_uri = settings.get_string("background-picture-uri"); string? path = resolve_path(custom_uri); diff --git a/src/core/wallpaper_rotation_state.vala b/src/core/wallpaper_rotation_state.vala index bfba474..18c9be4 100644 --- a/src/core/wallpaper_rotation_state.vala +++ b/src/core/wallpaper_rotation_state.vala @@ -22,6 +22,17 @@ namespace Singularity { this.config_dir = config_dir; } + // The one place this location is spelled out. The settings page (which + // writes the files) and the rotator (which reads them) are in the same + // process but were reached through separate code paths; a second + // literal here is a silent disagreement about where the contract + // lives, with a UI that appears to save and a rotator that never sees + // the change. + public static string default_config_dir() { + return GLib.Path.build_filename( + GLib.Environment.get_user_config_dir(), "singularity", "wallpaper-rotation"); + } + private string path_for(string filename) { return GLib.Path.build_filename(config_dir, filename); } diff --git a/src/core/wallpaper_rotator.vala b/src/core/wallpaper_rotator.vala new file mode 100644 index 0000000..acac8ca --- /dev/null +++ b/src/core/wallpaper_rotator.vala @@ -0,0 +1,253 @@ +using GLib; +using Gee; + +namespace Singularity { + + // The runtime consumer of the rotation-state files the Desktop settings + // page writes (see WallpaperRotationState for the file format). Without + // something on this side actually reading them, "Rotate Wallpapers" and + // "Rotation Interval" are inert controls: they persist a preference + // nothing acts on. + // + // WHY THIS LIVES IN THE SHELL PROCESS RATHER THAN A SEPARATE DAEMON. + // The shell is already running for the whole session, already owns the + // wallpaper through WallpaperManager, and already repaints it on a + // GSettings change -- so rotation here is a timer plus a settings write, + // and the crossfade, rescaling and accent extraction are reused + // unchanged. A separate binary would need a second copy of the + // collection parsing, its own GSettings schema lookup against whatever + // prefix the shell was installed into, and a session unit or autostart + // entry that actually gets started -- which is the part that tends to + // fail silently, leaving a rotator that is installed, correct, and has + // never once run. None of that buys anything the shell cannot already + // do, so it is not a daemon or a new IPC surface. + // + // Policy only: this class decides WHICH image and WHEN, and announces it + // through wallpaper_selected. It never touches GSettings, GTK or Gdk + // itself -- WallpaperManager.start_rotation() is the one place the + // decision is turned into an applied wallpaper, which is also what keeps + // this testable against temporary directories with no session at all. + public class WallpaperRotator : Object { + private static WallpaperRotator? _instance = null; + + // The chosen wallpaper, as a file:// URI. Connect to apply it. + public signal void wallpaper_selected(string uri); + + // What is on screen right now, so a rotation does not "change" the + // wallpaper to the one already showing. Kept in sync by whoever + // applies the signal, because the wallpaper can also be changed from + // the gallery or another application while the timer is armed. + public string? current_uri { get; set; default = null; } + + // What the armed timer is actually set to, or 0 when rotation is off. + // Observable so "the switch is off" and "the interval changed" are + // assertable without a test that waits out a real rotation period. + public int armed_interval_seconds { get; private set; default = 0; } + + private WallpaperRotationState state; + private string[] collection_roots; + private string config_dir; + private uint tick_id = 0; + private uint restart_id = 0; + private FileMonitor? state_monitor = null; + + public static WallpaperRotator get_default() { + if (_instance == null) { + _instance = new WallpaperRotator( + WallpaperRotationState.default_config_dir(), + WallpaperCollections.default_search_roots()); + } + return _instance; + } + + // config_dir and collection_roots are injected rather than read from + // GLib.Environment here, matching WallpaperRotationState and + // WallpaperCollections, so the rotation policy is testable against a + // temp directory. + public WallpaperRotator(string config_dir, string[] collection_roots) { + this.config_dir = config_dir; + this.collection_roots = collection_roots; + this.state = new WallpaperRotationState(config_dir); + } + + // Arms the timer and starts watching the state files. Safe to call + // more than once; a second call just re-reads the state. + public void start() { + watch_state_dir(); + reschedule(); + } + + public void stop() { + cancel_tick(); + if (restart_id != 0) { + Source.remove(restart_id); + restart_id = 0; + } + if (state_monitor != null) { + // Cancel, not just drop: the monitor's "changed" handler holds + // a reference back to this object, so releasing the field is + // not on its own enough to stop events arriving. + state_monitor.cancel(); + state_monitor = null; + } + } + + private void cancel_tick() { + if (tick_id != 0) { + Source.remove(tick_id); + tick_id = 0; + } + armed_interval_seconds = 0; + } + + // Re-read rotate-enabled / rotate-interval and arm (or cancel) the + // timer accordingly. The interval is read at arm time rather than + // cached at startup, so a user who changes it does not have to log + // out for the new value to take effect. + public void reschedule() { + cancel_tick(); + if (!state.get_rotate_enabled()) return; + int interval = state.get_rotate_interval_seconds(); + armed_interval_seconds = interval; + tick_id = Timeout.add_seconds(interval, () => { + tick_id = 0; + armed_interval_seconds = 0; + // Re-check enabled at fire time as well: the file can have + // been written after this timer was armed by something that + // does not go through the settings page. + if (state.get_rotate_enabled()) rotate_async(); + reschedule(); + return Source.REMOVE; + }); + } + + // What the timer fires. choose_next() walks the collection directory + // and queries a content type per file, which is filesystem I/O this + // process must not do on the main loop -- it is the compositor's, and + // a stall there drops frames. populate_grid() in the settings page + // threads the identical scan for the same reason. Public because this, + // not rotate_now(), is the path every real rotation takes, and a path + // no test can reach is a path nothing checks. + public void rotate_async() { + // Snapshot on the main thread: current_uri is written here when + // the wallpaper changes, and the worker must not read the + // property concurrently. + string? current = current_uri; + new Thread("wallpaper-rotate", () => { + string? uri = choose_next_for(current); + if (uri == null) return; + Idle.add(() => { + current_uri = uri; + wallpaper_selected(uri); + return Source.REMOVE; + }); + }); + } + + // Pick an image from the selected collection and announce it, on the + // calling thread. Public so a future "Next wallpaper" action has + // something to call, and so the tests can drive one rotation without + // a timer or a main loop. + public void rotate_now() { + string? uri = choose_next(); + if (uri == null) return; + current_uri = uri; + wallpaper_selected(uri); + } + + // The whole decision, with no side effects, so it can be asserted on + // directly: which collection, which images are in it, and which one + // is next. Returns null when there is nothing to rotate to -- an + // empty pack, a provider that fetched nothing, a stale collection id + // -- in which case the desktop keeps the wallpaper it has rather + // than being left with none. + public string? choose_next() { + return choose_next_for(current_uri); + } + + // The wallpaper to avoid is passed in rather than read from the + // property, so the worker thread in rotate_async() works from a + // main-thread snapshot instead of racing a concurrent write to it. + public string? choose_next_for(string? current) { + var collections = WallpaperCollections.parse(collection_roots); + if (collections.size == 0) return null; + + string selected_id = state.get_selected_collection(""); + string? scan_dir = null; + foreach (var collection in collections) { + if (collection.id == selected_id) { scan_dir = collection.dir; break; } + } + // Same fallback the gallery makes for a deleted pack or a stale + // state file: use the first known collection rather than doing + // nothing. Not persisted here -- the settings page owns that + // file, and a background timer quietly rewriting the user's + // selection is not the rotator's business. + if (scan_dir == null) scan_dir = collections[0].dir; + + var all_dirs = new ArrayList(); + foreach (var collection in collections) all_dirs.add(collection.dir); + + // No "recent" ordering: recency is a gallery presentation + // concern, and passing it here would bias the rotation toward + // the images the user has most recently picked by hand. + var candidates = WallpaperGallery.scan(scan_dir, all_dirs.to_array(), {}); + if (candidates.size == 0) return null; + + var uris = new ArrayList(); + foreach (var candidate in candidates) uris.add(candidate.uri); + return pick(uris, current, Random.next_int()); + } + + // Random, but never the image already on screen when the collection + // has an alternative -- a rotation that lands on the current + // wallpaper looks like the feature is broken. Taking the roll as a + // parameter keeps this deterministic under test instead of making + // the suite depend on a random draw. + public static string? pick(Gee.List uris, string? current_uri, uint32 roll) { + if (uris.size == 0) return null; + if (uris.size == 1) return uris[0]; + + var choices = new ArrayList(); + foreach (string uri in uris) { + if (uri != current_uri) choices.add(uri); + } + // Every candidate equals the current one (a collection of + // duplicates): keep what is showing rather than return null, + // which the caller would read as "nothing to rotate to". + if (choices.size == 0) return uris[0]; + return choices[(int) (roll % choices.size)]; + } + + // The state files are a documented, project-owned contract, so the + // settings page is not assumed to be the only writer: watch the + // directory instead of having the UI call back into here. That also + // means an interval change applies immediately rather than after the + // current (possibly day-long) period elapses. + private void watch_state_dir() { + if (state_monitor != null) return; + DirUtils.create_with_parents(config_dir, 0700); + try { + var dir = File.new_for_path(config_dir); + state_monitor = dir.monitor_directory(FileMonitorFlags.NONE, null); + } catch (Error e) { + // Not fatal: without a monitor the timer still re-reads the + // state on every tick, so changes take effect one period + // late instead of immediately. + warning("wallpaper rotator: cannot watch %s: %s", config_dir, e.message); + return; + } + state_monitor.changed.connect((file, other, event) => { + // WallpaperRotationState writes through a temp file and + // renames it into place, so a single logical change arrives + // as several events. Coalesce them, or each write re-arms the + // timer two or three times. + if (restart_id != 0) return; + restart_id = Timeout.add(250, () => { + restart_id = 0; + reschedule(); + return Source.REMOVE; + }); + }); + } + } +} diff --git a/tests/wallpaper_rotator_test.vala b/tests/wallpaper_rotator_test.vala new file mode 100644 index 0000000..74d716f --- /dev/null +++ b/tests/wallpaper_rotator_test.vala @@ -0,0 +1,259 @@ +using GLib; +using Gee; +using Singularity; + +private string fixture_root; + +private string make_dir(string name) { + string path = Path.build_filename(fixture_root, name); + DirUtils.create_with_parents(path, 0700); + return path; +} + +private string image_file(string dir, string name) { + string path = Path.build_filename(dir, name + ".svg"); + try { + DirUtils.create_with_parents(dir, 0700); + FileUtils.set_contents(path, + ""); + } catch (Error e) { error("fixture: %s", e.message); } + return File.new_for_path(path).get_uri(); +} + +private void write_collection(string registry, string id, string dir) { + try { + DirUtils.create_with_parents(registry, 0700); + FileUtils.set_contents(Path.build_filename(registry, id + ".collection"), + "[Collection]\nId=%s\nName=%s\nDir=%s\n".printf(id, id, dir)); + } catch (Error e) { error("fixture: %s", e.message); } +} + +// ---- pick(): which image is next ------------------------------------------ + +private void test_pick_single_candidate() { + var uris = new ArrayList(); + uris.add("file:///a.png"); + // The only image in the pack is also the one on screen: keep showing it + // rather than report "nothing to rotate to". + assert(WallpaperRotator.pick(uris, "file:///a.png", 0) == "file:///a.png"); +} + +private void test_pick_is_deterministic_for_a_given_roll() { + var uris = new ArrayList(); + uris.add("file:///a.png"); + uris.add("file:///b.png"); + uris.add("file:///c.png"); + assert(WallpaperRotator.pick(uris, null, 0) == "file:///a.png"); + assert(WallpaperRotator.pick(uris, null, 1) == "file:///b.png"); + assert(WallpaperRotator.pick(uris, null, 5) == "file:///c.png"); +} + +private void test_pick_never_returns_the_current_wallpaper() { + var uris = new ArrayList(); + uris.add("file:///a.png"); + uris.add("file:///b.png"); + uris.add("file:///c.png"); + // A rotation that lands on the image already showing reads as a broken + // feature, so every roll must skip it. + for (uint32 roll = 0; roll < 12; roll++) { + assert(WallpaperRotator.pick(uris, "file:///b.png", roll) != "file:///b.png"); + } +} + +private void test_pick_on_empty_list() { + assert(WallpaperRotator.pick(new ArrayList(), null, 0) == null); +} + +// ---- choose_next(): which collection --------------------------------------- + +private void test_rotates_within_the_selected_collection_only() { + string registry = make_dir("registry-scoped"); + string alpha = make_dir("alpha"); + string beta = make_dir("beta"); + string a1 = image_file(alpha, "a1"); + string a2 = image_file(alpha, "a2"); + image_file(beta, "b1"); + write_collection(registry, "alpha", alpha); + write_collection(registry, "beta", beta); + + string config = make_dir("config-scoped"); + new WallpaperRotationState(config).set_selected_collection("alpha"); + + var rotator = new WallpaperRotator(config, { registry }); + for (int i = 0; i < 8; i++) { + string? chosen = rotator.choose_next(); + assert(chosen == a1 || chosen == a2); + } +} + +private void test_stale_collection_id_falls_back_to_the_first() { + string registry = make_dir("registry-stale"); + string alpha = make_dir("alpha-stale"); + string only = image_file(alpha, "only"); + write_collection(registry, "alpha", alpha); + + string config = make_dir("config-stale"); + // A pack the user had selected and has since uninstalled must not leave + // the rotator doing nothing forever. + new WallpaperRotationState(config).set_selected_collection("uninstalled-pack"); + + var rotator = new WallpaperRotator(config, { registry }); + assert(rotator.choose_next() == only); +} + +private void test_empty_collection_leaves_the_wallpaper_alone() { + string registry = make_dir("registry-empty"); + string empty = make_dir("empty-pack"); + write_collection(registry, "empty", empty); + + string config = make_dir("config-empty"); + new WallpaperRotationState(config).set_selected_collection("empty"); + + var rotator = new WallpaperRotator(config, { registry }); + assert(rotator.choose_next() == null); +} + +private void test_no_registry_at_all() { + var rotator = new WallpaperRotator(make_dir("config-none"), + { Path.build_filename(fixture_root, "no-such-registry") }); + assert(rotator.choose_next() == null); +} + +// ---- rotate_now(): the announcement ---------------------------------------- + +private void test_rotate_now_announces_and_records_the_choice() { + string registry = make_dir("registry-emit"); + string pack = make_dir("emit-pack"); + string only = image_file(pack, "only"); + write_collection(registry, "emit", pack); + + string config = make_dir("config-emit"); + new WallpaperRotationState(config).set_selected_collection("emit"); + + var rotator = new WallpaperRotator(config, { registry }); + string? announced = null; + rotator.wallpaper_selected.connect((uri) => { announced = uri; }); + rotator.rotate_now(); + + assert(announced == only); + // Recorded, so the next rotation knows what is already on screen. + assert(rotator.current_uri == only); +} + +private void test_rotate_now_announces_nothing_when_there_is_nothing() { + string config = make_dir("config-silent"); + var rotator = new WallpaperRotator(config, + { Path.build_filename(fixture_root, "no-such-registry") }); + bool announced = false; + rotator.wallpaper_selected.connect((uri) => { announced = true; }); + rotator.rotate_now(); + assert(!announced); +} + +private void test_choose_next_for_excludes_the_supplied_wallpaper() { + string registry = make_dir("registry-exclude"); + string pack = make_dir("exclude-pack"); + string a = image_file(pack, "a"); + string b = image_file(pack, "b"); + write_collection(registry, "exclude", pack); + + string config = make_dir("config-exclude"); + new WallpaperRotationState(config).set_selected_collection("exclude"); + + // The threaded rotation passes a main-thread snapshot rather than + // reading the property, so the exclusion has to hold for the argument. + var rotator = new WallpaperRotator(config, { registry }); + for (int i = 0; i < 8; i++) { + assert(rotator.choose_next_for(a) == b); + assert(rotator.choose_next_for(b) == a); + } +} + +// The path every real rotation takes: the scan happens on a worker thread and +// the result is announced back on the main loop. rotate_now() is the same +// decision made synchronously, so testing only that would leave the threaded +// hand-off -- the part that actually runs -- unexercised. +private void test_rotate_async_announces_on_the_main_loop() { + string registry = make_dir("registry-async"); + string pack = make_dir("async-pack"); + string only = image_file(pack, "only"); + write_collection(registry, "async", pack); + + string config = make_dir("config-async"); + new WallpaperRotationState(config).set_selected_collection("async"); + + var rotator = new WallpaperRotator(config, { registry }); + var loop = new MainLoop(); + string? announced = null; + rotator.wallpaper_selected.connect((uri) => { + announced = uri; + loop.quit(); + }); + // Fail on the assertion below rather than hang the suite if the hand-off + // never happens. The flag matters: once this fires the source is already + // gone, and removing it again is a GLib critical that would mask the real + // failure with "Source ID was not found". + bool timed_out = false; + uint bail = Timeout.add_seconds(10, () => { + timed_out = true; + loop.quit(); + return Source.REMOVE; + }); + + rotator.rotate_async(); + loop.run(); + if (!timed_out) Source.remove(bail); + + assert(!timed_out); + assert(announced == only); + assert(rotator.current_uri == only); +} + +// ---- reschedule(): the switch and the interval actually govern the timer ---- + +private void test_timer_follows_the_rotation_state() { + string config = make_dir("config-timer"); + var state = new WallpaperRotationState(config); + var rotator = new WallpaperRotator(config, { make_dir("registry-timer") }); + + // Default state (no files written yet): enabled, 600s. + rotator.reschedule(); + assert(rotator.armed_interval_seconds == 600); + + state.set_rotate_interval_seconds(3600); + rotator.reschedule(); + assert(rotator.armed_interval_seconds == 3600); + + // The switch is what decides whether a timer exists at all. + state.set_rotate_enabled(false); + rotator.reschedule(); + assert(rotator.armed_interval_seconds == 0); + + state.set_rotate_enabled(true); + rotator.reschedule(); + assert(rotator.armed_interval_seconds == 3600); + + rotator.stop(); + assert(rotator.armed_interval_seconds == 0); +} + +public int main(string[] args) { + Test.init(ref args); + try { fixture_root = DirUtils.make_tmp("wallpaper-rotator-XXXXXX"); } + catch (Error e) { error("fixture: %s", e.message); } + + Test.add_func("/wallpaper-rotator/pick-single-candidate", test_pick_single_candidate); + Test.add_func("/wallpaper-rotator/pick-deterministic-roll", test_pick_is_deterministic_for_a_given_roll); + Test.add_func("/wallpaper-rotator/pick-skips-current", test_pick_never_returns_the_current_wallpaper); + Test.add_func("/wallpaper-rotator/pick-empty", test_pick_on_empty_list); + Test.add_func("/wallpaper-rotator/scoped-to-selected-collection", test_rotates_within_the_selected_collection_only); + Test.add_func("/wallpaper-rotator/stale-id-falls-back", test_stale_collection_id_falls_back_to_the_first); + Test.add_func("/wallpaper-rotator/empty-collection-is-a-no-op", test_empty_collection_leaves_the_wallpaper_alone); + Test.add_func("/wallpaper-rotator/no-registry", test_no_registry_at_all); + Test.add_func("/wallpaper-rotator/rotate-now-announces", test_rotate_now_announces_and_records_the_choice); + Test.add_func("/wallpaper-rotator/rotate-now-silent-when-empty", test_rotate_now_announces_nothing_when_there_is_nothing); + Test.add_func("/wallpaper-rotator/choose-next-for-excludes", test_choose_next_for_excludes_the_supplied_wallpaper); + Test.add_func("/wallpaper-rotator/rotate-async-announces", test_rotate_async_announces_on_the_main_loop); + Test.add_func("/wallpaper-rotator/timer-follows-state", test_timer_follows_the_rotation_state); + return Test.run(); +} From 97a9455aa4b075b00f8cd1b6fa478cb04e574c16 Mon Sep 17 00:00:00 2001 From: Jason Perlow Date: Sat, 12 Sep 2026 17:31:18 -0400 Subject: [PATCH 09/11] fix(wallpaper): make rotation opt-in, not on by an absent file get_rotate_enabled() treated a missing rotate-enabled file as enabled. That was harmless while nothing read these files, but the previous commit gave them a runtime consumer -- so as written, every existing install would begin replacing the wallpaper its user had chosen, every ten minutes, from whichever collection sorts first in the registry, without anyone having touched the switch. The settings page only writes that file from the toggle handler, which is connected after the SwitchRow is built, so a user who never opened the page has no file at all. Absent now means off. The switch and the interval row read the same accessor, so an untouched install shows rotation off, which is also what it does. Also corrects the registry-root ordering comments. parse() is first-root-wins and default_search_roots() passes system dirs first, so a shipped collection beats a user file reusing the same Id -- the opposite of what both comments claimed. Ordering and behaviour are unchanged; only the description was wrong. Tests: rotate-enabled now defaults false, and a rotator started against an untouched config arms no timer. Both fail if the old default is restored. Assisted-by: Claude Code:claude-opus-5 AI-Scope: adversarial review of the preceding commit found the default-on regression; Claude Code wrote this fix and its tests. --- src/core/wallpaper_collections.vala | 23 ++++++++++++----------- src/core/wallpaper_rotation_state.vala | 9 ++++++++- tests/wallpaper_rotation_state_test.vala | 8 +++++--- tests/wallpaper_rotator_test.vala | 19 ++++++++++++++++++- 4 files changed, 43 insertions(+), 16 deletions(-) diff --git a/src/core/wallpaper_collections.vala b/src/core/wallpaper_collections.vala index 46e3735..955e1d0 100644 --- a/src/core/wallpaper_collections.vala +++ b/src/core/wallpaper_collections.vala @@ -28,22 +28,23 @@ namespace Singularity { // Parses the .collection registry (INI-shaped KeyFiles, one per pack or // provider) into a list of WallpaperCollectionInfo, in the priority order // the search roots are given -- a later root's file for the same Id is - // ignored, matching "first root wins" so callers pass roots most-specific - // (e.g. per-user) LAST if they want a user override to win, or FIRST if - // they want the shipped default to win. desktop_page.vala's caller passes - // system dirs then the user dir, so a user's own collection can override - // one bundled with the OS. + // ignored, matching "first root wins" -- so callers pass roots in the + // order they want honoured, most-specific FIRST if a user file should + // beat a shipped one. default_search_roots() below passes system dirs + // first, which means a shipped collection wins an Id collision against a + // user file reusing the same Id. // // Callers pass explicit search_roots (not read from GLib.Environment // here) so this class stays testable against a temp directory with no // real filesystem layout assumptions. public class WallpaperCollections : Object { - // The registry roots, in the priority order parse() documents above: - // system data dirs first, the user's own dir last so a user-installed - // collection can override one bundled with the OS. Shared by the - // settings page and the rotator -- two copies of this list drift, and - // a rotator that cannot see a pack the gallery shows is exactly the - // shape that bug takes. + // The registry roots: system data dirs, then the user's own. parse() + // is first-root-wins, so on an Id collision the system collection is + // the one kept and a user file reusing that Id is dropped -- worth + // knowing before relying on the order, and unchanged here from what + // the settings page built inline. Shared by the settings page and the + // rotator, because two copies of this list drift, and a rotator that + // cannot see a pack the gallery shows is the shape that bug takes. public static string[] default_search_roots() { string[] roots = {}; foreach (unowned string d in GLib.Environment.get_system_data_dirs()) diff --git a/src/core/wallpaper_rotation_state.vala b/src/core/wallpaper_rotation_state.vala index 18c9be4..d3c26a0 100644 --- a/src/core/wallpaper_rotation_state.vala +++ b/src/core/wallpaper_rotation_state.vala @@ -76,9 +76,16 @@ namespace Singularity { write("collection", id); } + // Absent means OFF, not on. While nothing read these files the + // default was inert either way; now that WallpaperRotator acts on + // them, defaulting an absent file to enabled would mean every + // existing install starts replacing the wallpaper its user chose, + // every ten minutes, from a collection they never picked, without + // anyone having touched the switch. Rotation is opt-in: the file + // exists once the user has turned it on. public bool get_rotate_enabled() { string? value = read_trimmed("rotate-enabled"); - if (value == null) return true; + if (value == null) return false; string lowered = value.down(); return lowered != "0" && lowered != "false" && lowered != "off"; } diff --git a/tests/wallpaper_rotation_state_test.vala b/tests/wallpaper_rotation_state_test.vala index 7938ebc..d86bb9c 100644 --- a/tests/wallpaper_rotation_state_test.vala +++ b/tests/wallpaper_rotation_state_test.vala @@ -25,9 +25,11 @@ private void test_selected_collection_strips_whitespace() { assert(state.get_selected_collection("default") == "bing"); } -private void test_rotate_enabled_defaults_true() { +private void test_rotate_enabled_defaults_false() { + // Opt-in: with no state file written, nothing should be rotating the + // wallpaper a user chose by hand. var state = new WallpaperRotationState(make_tmp_dir()); - assert(state.get_rotate_enabled() == true); + assert(state.get_rotate_enabled() == false); } private void test_rotate_enabled_roundtrips_false() { @@ -69,7 +71,7 @@ public int main(string[] args) { Test.add_func("/wallpaper-rotation-state/selected-collection-defaults-when-unset", test_selected_collection_defaults_when_unset); Test.add_func("/wallpaper-rotation-state/selected-collection-roundtrips", test_selected_collection_roundtrips); Test.add_func("/wallpaper-rotation-state/selected-collection-strips-whitespace", test_selected_collection_strips_whitespace); - Test.add_func("/wallpaper-rotation-state/rotate-enabled-defaults-true", test_rotate_enabled_defaults_true); + Test.add_func("/wallpaper-rotation-state/rotate-enabled-defaults-false", test_rotate_enabled_defaults_false); Test.add_func("/wallpaper-rotation-state/rotate-enabled-roundtrips-false", test_rotate_enabled_roundtrips_false); Test.add_func("/wallpaper-rotation-state/rotate-interval-defaults-to-600", test_rotate_interval_defaults_to_600); Test.add_func("/wallpaper-rotation-state/rotate-interval-roundtrips", test_rotate_interval_roundtrips); diff --git a/tests/wallpaper_rotator_test.vala b/tests/wallpaper_rotator_test.vala index 74d716f..9684b5a 100644 --- a/tests/wallpaper_rotator_test.vala +++ b/tests/wallpaper_rotator_test.vala @@ -211,12 +211,28 @@ private void test_rotate_async_announces_on_the_main_loop() { // ---- reschedule(): the switch and the interval actually govern the timer ---- +// Rotation must not start on its own. Before a runtime consumer existed the +// "absent means enabled" default was inert; now it would mean every install +// that never touched the switch starts replacing a hand-picked wallpaper. +private void test_untouched_install_does_not_rotate() { + string config = make_dir("config-untouched"); + var rotator = new WallpaperRotator(config, { make_dir("registry-untouched") }); + rotator.start(); + assert(rotator.armed_interval_seconds == 0); + rotator.stop(); +} + private void test_timer_follows_the_rotation_state() { string config = make_dir("config-timer"); var state = new WallpaperRotationState(config); var rotator = new WallpaperRotator(config, { make_dir("registry-timer") }); - // Default state (no files written yet): enabled, 600s. + // Nothing written yet: off, so no timer at all. + rotator.reschedule(); + assert(rotator.armed_interval_seconds == 0); + + // Turning it on is what starts it, at the default period. + state.set_rotate_enabled(true); rotator.reschedule(); assert(rotator.armed_interval_seconds == 600); @@ -254,6 +270,7 @@ public int main(string[] args) { Test.add_func("/wallpaper-rotator/rotate-now-silent-when-empty", test_rotate_now_announces_nothing_when_there_is_nothing); Test.add_func("/wallpaper-rotator/choose-next-for-excludes", test_choose_next_for_excludes_the_supplied_wallpaper); Test.add_func("/wallpaper-rotator/rotate-async-announces", test_rotate_async_announces_on_the_main_loop); + Test.add_func("/wallpaper-rotator/untouched-install-does-not-rotate", test_untouched_install_does_not_rotate); Test.add_func("/wallpaper-rotator/timer-follows-state", test_timer_follows_the_rotation_state); return Test.run(); } From 81269e8fcc7bdf1ed07f253ae9a030b0ac8358e8 Mon Sep 17 00:00:00 2001 From: Jason Perlow Date: Sun, 13 Sep 2026 13:21:49 -0400 Subject: [PATCH 10/11] style(wallpaper): trim explanatory comments, use ASCII punctuation --- .../sidebar/pages/desktop_page.vala | 20 +--- src/core/main.vala | 7 +- src/core/wallpaper_collections.vala | 27 +---- src/core/wallpaper_gallery.vala | 23 +--- src/core/wallpaper_manager.vala | 10 +- src/core/wallpaper_rotation_state.vala | 31 +---- src/core/wallpaper_rotator.vala | 111 ++---------------- tests/wallpaper_rotation_state_test.vala | 4 +- tests/wallpaper_rotator_test.vala | 32 +---- 9 files changed, 28 insertions(+), 237 deletions(-) diff --git a/src/components/sidebar/pages/desktop_page.vala b/src/components/sidebar/pages/desktop_page.vala index d1007d1..52901fc 100644 --- a/src/components/sidebar/pages/desktop_page.vala +++ b/src/components/sidebar/pages/desktop_page.vala @@ -22,8 +22,6 @@ namespace Singularity { private WallpaperPreviewWidget preview_widget; private FlowBox wallpaper_grid; private Gee.ArrayList wallpaper_collections = new Gee.ArrayList(); - // Same directory WallpaperRotator reads: these controls write the - // rotation state and the rotator in the shell process acts on it. private WallpaperRotationState rotation_state = new WallpaperRotationState( WallpaperRotationState.default_config_dir()); private int wallpaper_grid_generation = 0; @@ -174,26 +172,18 @@ namespace Singularity { add_group(preview_group); var grid_group = new PreferencesGroup(_("Wallpapers")); - // "singularity/wallpaper-collections" is a project-owned registry - // location, not a specific vendor's: any downstream OS or pack - // installer can drop a .collection file here to have its wallpapers - // appear in this picker (see WallpaperCollections' class doc for the - // file format). wallpaper_collections = WallpaperCollections.parse( WallpaperCollections.default_search_roots()); var source_options = new Gee.ArrayList(); foreach (var collection in wallpaper_collections) { string label = (collection.artist != null && collection.artist != "" && collection.artist != collection.name) - ? "%s — %s".printf(collection.name, collection.artist) + ? "%s - %s".printf(collection.name, collection.artist) : collection.name; source_options.add(new Singularity.Core.AppSettingOption() { id = collection.id, label = label }); } - // "" is not a real collection id -- it's just a value guaranteed - // not to match anything, so the fallback below always picks the - // first discovered collection when no prior selection is on disk. string initial_collection_id = rotation_state.get_selected_collection(""); bool have_initial = false; foreach (var opt in source_options) if (opt.id == initial_collection_id) have_initial = true; @@ -1902,13 +1892,7 @@ namespace Singularity { foreach (var collection in wallpaper_collections) { if (collection.id == selected_id) { scan_dir = collection.dir; break; } } - // A selection with no matching collection (deleted pack, stale - // state file) must not empty the grid silently -- fall back to - // whatever the first known collection is, same "never leave the - // desktop with no wallpaper" principle the rotator script itself - // follows. Persist the fallback so the source row and the state - // file agree with what's actually on screen instead of re-falling - // back (and re-logging the same mismatch) on every refresh. + // Persist stale-selection fallback so the UI and saved state agree. if (scan_dir == null && wallpaper_collections.size > 0) { selected_id = wallpaper_collections[0].id; scan_dir = wallpaper_collections[0].dir; diff --git a/src/core/main.vala b/src/core/main.vala index dd3d7bf..a525a44 100644 --- a/src/core/main.vala +++ b/src/core/main.vala @@ -314,12 +314,7 @@ public class SingularityApp : Singularity.ShellApplication, Singularity.Shell.Sh // connection is established at login, not on the first search. Singularity.SearchManager.get_default(); Singularity.NowPlayingCache.get_default(); - // Wallpaper rotation: the shell is the runtime consumer of the - // rotation-state files the Desktop settings page writes, so the - // "Rotate Wallpapers" switch and interval act on something. - // Suppressed in safe mode with the other optional startup - // features -- a timer that changes persisted session state is - // not what a machine recovering from a crash loop needs. + // Do not change persisted wallpaper state during safe-mode recovery. if (Singularity.SafeMode.get_default().allows( Singularity.SafeFeature.AUTOSTART)) Singularity.WallpaperManager.get_default().start_rotation(); diff --git a/src/core/wallpaper_collections.vala b/src/core/wallpaper_collections.vala index 955e1d0..50d5151 100644 --- a/src/core/wallpaper_collections.vala +++ b/src/core/wallpaper_collections.vala @@ -4,12 +4,7 @@ using Gee; namespace Singularity { public class WallpaperCollectionInfo : Object { - // Plain public fields, not GObject properties: Vala's property - // system rejects a property literally named "type" ("error: - // Property 'type' not allowed", collides with GObject's own type - // machinery). Plain fields sidestep that and still match the - // interface this class is documented to expose -- "public fields: - // string id, string name, string artist, string dir, string type". + // Vala rejects a GObject property named "type"; keep these as fields. public string id; public string name; public string artist; @@ -25,26 +20,8 @@ namespace Singularity { } } - // Parses the .collection registry (INI-shaped KeyFiles, one per pack or - // provider) into a list of WallpaperCollectionInfo, in the priority order - // the search roots are given -- a later root's file for the same Id is - // ignored, matching "first root wins" -- so callers pass roots in the - // order they want honoured, most-specific FIRST if a user file should - // beat a shipped one. default_search_roots() below passes system dirs - // first, which means a shipped collection wins an Id collision against a - // user file reusing the same Id. - // - // Callers pass explicit search_roots (not read from GLib.Environment - // here) so this class stays testable against a temp directory with no - // real filesystem layout assumptions. public class WallpaperCollections : Object { - // The registry roots: system data dirs, then the user's own. parse() - // is first-root-wins, so on an Id collision the system collection is - // the one kept and a user file reusing that Id is dropped -- worth - // knowing before relying on the order, and unchanged here from what - // the settings page built inline. Shared by the settings page and the - // rotator, because two copies of this list drift, and a rotator that - // cannot see a pack the gallery shows is the shape that bug takes. + // parse() is first-root-wins, so system collections take precedence. public static string[] default_search_roots() { string[] roots = {}; foreach (unowned string d in GLib.Environment.get_system_data_dirs()) diff --git a/src/core/wallpaper_gallery.vala b/src/core/wallpaper_gallery.vala index 6271ab4..0d5d8bc 100644 --- a/src/core/wallpaper_gallery.vala +++ b/src/core/wallpaper_gallery.vala @@ -13,8 +13,7 @@ namespace Singularity { } internal class WallpaperGallery : Object { - // Membership comes from the selected scan. History can reorder its - // members, but must never introduce images from another source. + // History may reorder results, but cannot add another source's images. public static ArrayList scan(string? selected_dir, string[] collection_dirs, string[] recent) { @@ -43,11 +42,7 @@ namespace Singularity { // Bound traversal of user-controlled collection directories. private const int WALLPAPER_SCAN_MAX_DEPTH = 3; - // Dir= values across .collection files may point at the same - // directory through different symlinks (a pack install living - // outside /usr/share is a common layout) -- resolve to the real - // path before comparing, or the source-boundary exclusion above - // silently fails to recognize them as the same root. + // Resolve symlinks before enforcing collection boundaries. private static string canonical_path(string path) { string? real = Posix.realpath(path, null); return real ?? File.new_for_path(path).get_path(); @@ -75,23 +70,15 @@ namespace Singularity { var child = dir.get_child(info.get_name()); if (info.get_file_type() == FileType.DIRECTORY) { - // Not followed as a directory either: a symlinked - // directory is the easy way to walk in a circle. + // Do not follow symlinked directories; they may form cycles. if (info.get_is_symlink()) continue; scan_wallpaper_dir(child.get_path(), candidates, thread_seen, visited_dirs, excluded_dirs, depth + 1); continue; } - // default.jpg is a symlink the rotator repoints at whichever - // wallpaper is current, at a target enumerated in this same - // directory -- following it would list one image twice, once - // under its own name and once as "default". Only elide a - // same-directory pointer like that one: a pack that ships an - // image as a symlink to a shared asset OUTSIDE this directory - // is real content, and the previous scanner listed it fine - // (content-type resolves through the link either way, since - // enumerate_children above passes no NOFOLLOW flag). + // Ignore same-directory aliases such as the mutable default.jpg, + // but retain symlinks to shared assets outside the collection. if (info.get_is_symlink()) { string? target = info.get_symlink_target(); if (target != null) { diff --git a/src/core/wallpaper_manager.vala b/src/core/wallpaper_manager.vala index 8cc5bc3..e147f67 100644 --- a/src/core/wallpaper_manager.vala +++ b/src/core/wallpaper_manager.vala @@ -34,13 +34,6 @@ namespace Singularity { reload(); } - // Turns the Desktop page's rotation controls into actual behaviour: - // the rotator decides what to show and when, and this is the single - // place that decision is applied. It goes through the same - // background-picture-uri key the gallery writes, so a rotation takes - // the ordinary path -- reload() below, the crossfade in Background, - // the settings preview and the accent extraction -- rather than a - // second, parallel way to put an image on screen. public void start_rotation() { if (rotator != null) return; rotator = WallpaperRotator.get_default(); @@ -48,8 +41,7 @@ namespace Singularity { rotator.wallpaper_selected.connect((uri) => { settings.set_string("background-picture-uri", uri); }); - // A wallpaper picked by hand (or by anything else) is now the - // current one, so the next rotation must not "change" to it. + // Track external changes so rotation does not reselect the current image. settings.changed["background-picture-uri"].connect(() => { rotator.current_uri = settings.get_string("background-picture-uri"); }); diff --git a/src/core/wallpaper_rotation_state.vala b/src/core/wallpaper_rotation_state.vala index d3c26a0..bb3b958 100644 --- a/src/core/wallpaper_rotation_state.vala +++ b/src/core/wallpaper_rotation_state.vala @@ -2,16 +2,6 @@ using GLib; namespace Singularity { - // Reads and writes the plain-text rotation-state files under - // $XDG_CONFIG_HOME/singularity/wallpaper-rotation/: "collection" (the - // active collection id), "rotate-enabled" ("1"/"0") and - // "rotate-interval" (seconds). This is the documented, project-owned - // contract for wallpaper rotation -- any background daemon that wants to - // actually change the desktop wallpaper on a timer polls these files and - // this class is only the shell UI's side of that same shared state, not - // a new or vendor-specific mechanism. config_dir is injected (rather - // than read from GLib.Environment here) so it's testable against a temp - // directory. public class WallpaperRotationState : Object { private const int DEFAULT_INTERVAL_SECONDS = 600; private const int MIN_INTERVAL_SECONDS = 30; @@ -22,12 +12,6 @@ namespace Singularity { this.config_dir = config_dir; } - // The one place this location is spelled out. The settings page (which - // writes the files) and the rotator (which reads them) are in the same - // process but were reached through separate code paths; a second - // literal here is a silent disagreement about where the contract - // lives, with a UI that appears to save and a rotator that never sees - // the change. public static string default_config_dir() { return GLib.Path.build_filename( GLib.Environment.get_user_config_dir(), "singularity", "wallpaper-rotation"); @@ -54,10 +38,7 @@ namespace Singularity { string dest = path_for(filename); string tmp = dest + ".tmp"; try { - // Write-then-rename: the rotator daemon polls these files on - // its own timer, so a partial write it reads mid-flush would - // be picked up as-is. rename(2) within the same directory is - // atomic, so the daemon only ever sees a complete write. + // Same-directory rename keeps state updates atomic for readers. FileUtils.set_contents(tmp, contents); if (FileUtils.rename(tmp, dest) != 0) { warning("wallpaper rotation state: could not rename %s into place", filename); @@ -76,13 +57,7 @@ namespace Singularity { write("collection", id); } - // Absent means OFF, not on. While nothing read these files the - // default was inert either way; now that WallpaperRotator acts on - // them, defaulting an absent file to enabled would mean every - // existing install starts replacing the wallpaper its user chose, - // every ten minutes, from a collection they never picked, without - // anyone having touched the switch. Rotation is opt-in: the file - // exists once the user has turned it on. + // Rotation is opt-in; an absent state file must remain off. public bool get_rotate_enabled() { string? value = read_trimmed("rotate-enabled"); if (value == null) return false; @@ -108,4 +83,4 @@ namespace Singularity { write("rotate-interval", clamped.to_string()); } } -} \ No newline at end of file +} diff --git a/src/core/wallpaper_rotator.vala b/src/core/wallpaper_rotator.vala index acac8ca..6b010f6 100644 --- a/src/core/wallpaper_rotator.vala +++ b/src/core/wallpaper_rotator.vala @@ -3,45 +3,14 @@ using Gee; namespace Singularity { - // The runtime consumer of the rotation-state files the Desktop settings - // page writes (see WallpaperRotationState for the file format). Without - // something on this side actually reading them, "Rotate Wallpapers" and - // "Rotation Interval" are inert controls: they persist a preference - // nothing acts on. - // - // WHY THIS LIVES IN THE SHELL PROCESS RATHER THAN A SEPARATE DAEMON. - // The shell is already running for the whole session, already owns the - // wallpaper through WallpaperManager, and already repaints it on a - // GSettings change -- so rotation here is a timer plus a settings write, - // and the crossfade, rescaling and accent extraction are reused - // unchanged. A separate binary would need a second copy of the - // collection parsing, its own GSettings schema lookup against whatever - // prefix the shell was installed into, and a session unit or autostart - // entry that actually gets started -- which is the part that tends to - // fail silently, leaving a rotator that is installed, correct, and has - // never once run. None of that buys anything the shell cannot already - // do, so it is not a daemon or a new IPC surface. - // - // Policy only: this class decides WHICH image and WHEN, and announces it - // through wallpaper_selected. It never touches GSettings, GTK or Gdk - // itself -- WallpaperManager.start_rotation() is the one place the - // decision is turned into an applied wallpaper, which is also what keeps - // this testable against temporary directories with no session at all. + // Selects rotation timing and images; WallpaperManager applies the signal. public class WallpaperRotator : Object { private static WallpaperRotator? _instance = null; - // The chosen wallpaper, as a file:// URI. Connect to apply it. public signal void wallpaper_selected(string uri); - // What is on screen right now, so a rotation does not "change" the - // wallpaper to the one already showing. Kept in sync by whoever - // applies the signal, because the wallpaper can also be changed from - // the gallery or another application while the timer is armed. public string? current_uri { get; set; default = null; } - // What the armed timer is actually set to, or 0 when rotation is off. - // Observable so "the switch is off" and "the interval changed" are - // assertable without a test that waits out a real rotation period. public int armed_interval_seconds { get; private set; default = 0; } private WallpaperRotationState state; @@ -60,18 +29,12 @@ namespace Singularity { return _instance; } - // config_dir and collection_roots are injected rather than read from - // GLib.Environment here, matching WallpaperRotationState and - // WallpaperCollections, so the rotation policy is testable against a - // temp directory. public WallpaperRotator(string config_dir, string[] collection_roots) { this.config_dir = config_dir; this.collection_roots = collection_roots; this.state = new WallpaperRotationState(config_dir); } - // Arms the timer and starts watching the state files. Safe to call - // more than once; a second call just re-reads the state. public void start() { watch_state_dir(); reschedule(); @@ -84,9 +47,7 @@ namespace Singularity { restart_id = 0; } if (state_monitor != null) { - // Cancel, not just drop: the monitor's "changed" handler holds - // a reference back to this object, so releasing the field is - // not on its own enough to stop events arriving. + // The signal handler retains this object until the monitor is cancelled. state_monitor.cancel(); state_monitor = null; } @@ -100,10 +61,6 @@ namespace Singularity { armed_interval_seconds = 0; } - // Re-read rotate-enabled / rotate-interval and arm (or cancel) the - // timer accordingly. The interval is read at arm time rather than - // cached at startup, so a user who changes it does not have to log - // out for the new value to take effect. public void reschedule() { cancel_tick(); if (!state.get_rotate_enabled()) return; @@ -112,26 +69,16 @@ namespace Singularity { tick_id = Timeout.add_seconds(interval, () => { tick_id = 0; armed_interval_seconds = 0; - // Re-check enabled at fire time as well: the file can have - // been written after this timer was armed by something that - // does not go through the settings page. + // Another process may disable rotation after this timer is armed. if (state.get_rotate_enabled()) rotate_async(); reschedule(); return Source.REMOVE; }); } - // What the timer fires. choose_next() walks the collection directory - // and queries a content type per file, which is filesystem I/O this - // process must not do on the main loop -- it is the compositor's, and - // a stall there drops frames. populate_grid() in the settings page - // threads the identical scan for the same reason. Public because this, - // not rotate_now(), is the path every real rotation takes, and a path - // no test can reach is a path nothing checks. + // Keep collection scanning off the compositor's main loop. public void rotate_async() { - // Snapshot on the main thread: current_uri is written here when - // the wallpaper changes, and the worker must not read the - // property concurrently. + // Snapshot before entering the worker to avoid a concurrent property read. string? current = current_uri; new Thread("wallpaper-rotate", () => { string? uri = choose_next_for(current); @@ -144,10 +91,6 @@ namespace Singularity { }); } - // Pick an image from the selected collection and announce it, on the - // calling thread. Public so a future "Next wallpaper" action has - // something to call, and so the tests can drive one rotation without - // a timer or a main loop. public void rotate_now() { string? uri = choose_next(); if (uri == null) return; @@ -155,19 +98,10 @@ namespace Singularity { wallpaper_selected(uri); } - // The whole decision, with no side effects, so it can be asserted on - // directly: which collection, which images are in it, and which one - // is next. Returns null when there is nothing to rotate to -- an - // empty pack, a provider that fetched nothing, a stale collection id - // -- in which case the desktop keeps the wallpaper it has rather - // than being left with none. public string? choose_next() { return choose_next_for(current_uri); } - // The wallpaper to avoid is passed in rather than read from the - // property, so the worker thread in rotate_async() works from a - // main-thread snapshot instead of racing a concurrent write to it. public string? choose_next_for(string? current) { var collections = WallpaperCollections.parse(collection_roots); if (collections.size == 0) return null; @@ -177,19 +111,13 @@ namespace Singularity { foreach (var collection in collections) { if (collection.id == selected_id) { scan_dir = collection.dir; break; } } - // Same fallback the gallery makes for a deleted pack or a stale - // state file: use the first known collection rather than doing - // nothing. Not persisted here -- the settings page owns that - // file, and a background timer quietly rewriting the user's - // selection is not the rotator's business. + // Do not let the background timer rewrite a stale user selection. if (scan_dir == null) scan_dir = collections[0].dir; var all_dirs = new ArrayList(); foreach (var collection in collections) all_dirs.add(collection.dir); - // No "recent" ordering: recency is a gallery presentation - // concern, and passing it here would bias the rotation toward - // the images the user has most recently picked by hand. + // Recency is gallery presentation state, not rotation weighting. var candidates = WallpaperGallery.scan(scan_dir, all_dirs.to_array(), {}); if (candidates.size == 0) return null; @@ -198,11 +126,7 @@ namespace Singularity { return pick(uris, current, Random.next_int()); } - // Random, but never the image already on screen when the collection - // has an alternative -- a rotation that lands on the current - // wallpaper looks like the feature is broken. Taking the roll as a - // parameter keeps this deterministic under test instead of making - // the suite depend on a random draw. + // Inject the random roll so selection remains deterministic in tests. public static string? pick(Gee.List uris, string? current_uri, uint32 roll) { if (uris.size == 0) return null; if (uris.size == 1) return uris[0]; @@ -211,18 +135,12 @@ namespace Singularity { foreach (string uri in uris) { if (uri != current_uri) choices.add(uri); } - // Every candidate equals the current one (a collection of - // duplicates): keep what is showing rather than return null, - // which the caller would read as "nothing to rotate to". + // A collection may contain duplicate URIs for the current image. if (choices.size == 0) return uris[0]; return choices[(int) (roll % choices.size)]; } - // The state files are a documented, project-owned contract, so the - // settings page is not assumed to be the only writer: watch the - // directory instead of having the UI call back into here. That also - // means an interval change applies immediately rather than after the - // current (possibly day-long) period elapses. + // State may be written outside the settings page, so watch the directory. private void watch_state_dir() { if (state_monitor != null) return; DirUtils.create_with_parents(config_dir, 0700); @@ -230,17 +148,12 @@ namespace Singularity { var dir = File.new_for_path(config_dir); state_monitor = dir.monitor_directory(FileMonitorFlags.NONE, null); } catch (Error e) { - // Not fatal: without a monitor the timer still re-reads the - // state on every tick, so changes take effect one period - // late instead of immediately. + // The timer still re-reads state if monitoring is unavailable. warning("wallpaper rotator: cannot watch %s: %s", config_dir, e.message); return; } state_monitor.changed.connect((file, other, event) => { - // WallpaperRotationState writes through a temp file and - // renames it into place, so a single logical change arrives - // as several events. Coalesce them, or each write re-arms the - // timer two or three times. + // Atomic replacement emits multiple events; coalesce them. if (restart_id != 0) return; restart_id = Timeout.add(250, () => { restart_id = 0; diff --git a/tests/wallpaper_rotation_state_test.vala b/tests/wallpaper_rotation_state_test.vala index d86bb9c..0842072 100644 --- a/tests/wallpaper_rotation_state_test.vala +++ b/tests/wallpaper_rotation_state_test.vala @@ -26,8 +26,6 @@ private void test_selected_collection_strips_whitespace() { } private void test_rotate_enabled_defaults_false() { - // Opt-in: with no state file written, nothing should be rotating the - // wallpaper a user chose by hand. var state = new WallpaperRotationState(make_tmp_dir()); assert(state.get_rotate_enabled() == false); } @@ -78,4 +76,4 @@ public int main(string[] args) { Test.add_func("/wallpaper-rotation-state/rotate-interval-clamps-to-30-minimum", test_rotate_interval_clamps_to_30_minimum); Test.add_func("/wallpaper-rotation-state/rotate-interval-garbage-on-disk-reads-as-default", test_rotate_interval_garbage_on_disk_reads_as_default); return Test.run(); -} \ No newline at end of file +} diff --git a/tests/wallpaper_rotator_test.vala b/tests/wallpaper_rotator_test.vala index 9684b5a..a96ac4b 100644 --- a/tests/wallpaper_rotator_test.vala +++ b/tests/wallpaper_rotator_test.vala @@ -28,13 +28,9 @@ private void write_collection(string registry, string id, string dir) { } catch (Error e) { error("fixture: %s", e.message); } } -// ---- pick(): which image is next ------------------------------------------ - private void test_pick_single_candidate() { var uris = new ArrayList(); uris.add("file:///a.png"); - // The only image in the pack is also the one on screen: keep showing it - // rather than report "nothing to rotate to". assert(WallpaperRotator.pick(uris, "file:///a.png", 0) == "file:///a.png"); } @@ -53,8 +49,6 @@ private void test_pick_never_returns_the_current_wallpaper() { uris.add("file:///a.png"); uris.add("file:///b.png"); uris.add("file:///c.png"); - // A rotation that lands on the image already showing reads as a broken - // feature, so every roll must skip it. for (uint32 roll = 0; roll < 12; roll++) { assert(WallpaperRotator.pick(uris, "file:///b.png", roll) != "file:///b.png"); } @@ -64,8 +58,6 @@ private void test_pick_on_empty_list() { assert(WallpaperRotator.pick(new ArrayList(), null, 0) == null); } -// ---- choose_next(): which collection --------------------------------------- - private void test_rotates_within_the_selected_collection_only() { string registry = make_dir("registry-scoped"); string alpha = make_dir("alpha"); @@ -93,8 +85,6 @@ private void test_stale_collection_id_falls_back_to_the_first() { write_collection(registry, "alpha", alpha); string config = make_dir("config-stale"); - // A pack the user had selected and has since uninstalled must not leave - // the rotator doing nothing forever. new WallpaperRotationState(config).set_selected_collection("uninstalled-pack"); var rotator = new WallpaperRotator(config, { registry }); @@ -119,8 +109,6 @@ private void test_no_registry_at_all() { assert(rotator.choose_next() == null); } -// ---- rotate_now(): the announcement ---------------------------------------- - private void test_rotate_now_announces_and_records_the_choice() { string registry = make_dir("registry-emit"); string pack = make_dir("emit-pack"); @@ -136,7 +124,6 @@ private void test_rotate_now_announces_and_records_the_choice() { rotator.rotate_now(); assert(announced == only); - // Recorded, so the next rotation knows what is already on screen. assert(rotator.current_uri == only); } @@ -160,8 +147,6 @@ private void test_choose_next_for_excludes_the_supplied_wallpaper() { string config = make_dir("config-exclude"); new WallpaperRotationState(config).set_selected_collection("exclude"); - // The threaded rotation passes a main-thread snapshot rather than - // reading the property, so the exclusion has to hold for the argument. var rotator = new WallpaperRotator(config, { registry }); for (int i = 0; i < 8; i++) { assert(rotator.choose_next_for(a) == b); @@ -169,10 +154,6 @@ private void test_choose_next_for_excludes_the_supplied_wallpaper() { } } -// The path every real rotation takes: the scan happens on a worker thread and -// the result is announced back on the main loop. rotate_now() is the same -// decision made synchronously, so testing only that would leave the threaded -// hand-off -- the part that actually runs -- unexercised. private void test_rotate_async_announces_on_the_main_loop() { string registry = make_dir("registry-async"); string pack = make_dir("async-pack"); @@ -189,10 +170,7 @@ private void test_rotate_async_announces_on_the_main_loop() { announced = uri; loop.quit(); }); - // Fail on the assertion below rather than hang the suite if the hand-off - // never happens. The flag matters: once this fires the source is already - // gone, and removing it again is a GLib critical that would mask the real - // failure with "Source ID was not found". + // Avoid removing a timeout source after its callback already removed it. bool timed_out = false; uint bail = Timeout.add_seconds(10, () => { timed_out = true; @@ -209,11 +187,6 @@ private void test_rotate_async_announces_on_the_main_loop() { assert(rotator.current_uri == only); } -// ---- reschedule(): the switch and the interval actually govern the timer ---- - -// Rotation must not start on its own. Before a runtime consumer existed the -// "absent means enabled" default was inert; now it would mean every install -// that never touched the switch starts replacing a hand-picked wallpaper. private void test_untouched_install_does_not_rotate() { string config = make_dir("config-untouched"); var rotator = new WallpaperRotator(config, { make_dir("registry-untouched") }); @@ -227,11 +200,9 @@ private void test_timer_follows_the_rotation_state() { var state = new WallpaperRotationState(config); var rotator = new WallpaperRotator(config, { make_dir("registry-timer") }); - // Nothing written yet: off, so no timer at all. rotator.reschedule(); assert(rotator.armed_interval_seconds == 0); - // Turning it on is what starts it, at the default period. state.set_rotate_enabled(true); rotator.reschedule(); assert(rotator.armed_interval_seconds == 600); @@ -240,7 +211,6 @@ private void test_timer_follows_the_rotation_state() { rotator.reschedule(); assert(rotator.armed_interval_seconds == 3600); - // The switch is what decides whether a timer exists at all. state.set_rotate_enabled(false); rotator.reschedule(); assert(rotator.armed_interval_seconds == 0); From 068872df7c789a59108719c83c11eb28313042e8 Mon Sep 17 00:00:00 2001 From: Jason Perlow Date: Sun, 13 Sep 2026 16:39:55 -0400 Subject: [PATCH 11/11] fix(wallpaper): show a custom rotate-interval's real value, not the nearest preset Mirko's review: a custom rotate-interval displayed as "Every 10 minutes" while WallpaperRotator kept using the actual stored value. The Rotation Interval selector only offers five fixed presets (10min/30min/ 1h/4h/day). When the stored interval didn't match any of them, the row silently fell back to the 10-minute id purely for display, while WallpaperRotationState.get_rotate_interval_seconds() -- read directly by WallpaperRotator -- kept returning the real stored value. Rotation itself was never affected, but the settings page actively misrepresented what was about to happen. Fixed by preserving the value instead of collapsing it: when the current interval has no matching preset, a synthesized entry showing the real duration ("Every 2 hours (custom)") is inserted into the option list in chronological order and selected, rather than substituting a different preset's id. Nothing is written back to disk by this change -- the row only writes when the user actually picks a different option, same as before. Verified: full ninja build (204/204 targets, 0 errors) and meson test (8/8 pass) in a debian:forky podman container on ULTRA. Assisted-by: Claude Code:claude-sonnet-5 AI-scope: root-caused the display/state mismatch from Mirko's report, authored the custom-interval preservation and its label formatter, and verified the build/test result quoted above. --- .../sidebar/pages/desktop_page.vala | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/src/components/sidebar/pages/desktop_page.vala b/src/components/sidebar/pages/desktop_page.vala index 52901fc..6127510 100644 --- a/src/components/sidebar/pages/desktop_page.vala +++ b/src/components/sidebar/pages/desktop_page.vala @@ -29,6 +29,21 @@ namespace Singularity { private string cached_wallpaper_accent = "#3584e4"; private static bool wallpaper_css_loaded = false; + // Label for a stored rotate-interval that doesn't match a fixed + // preset. Hours when it divides evenly, else minutes, else seconds + // (WallpaperRotationState.MIN_INTERVAL_SECONDS is 30, below a minute). + private static string format_custom_interval_label(int seconds) { + if (seconds % 3600 == 0) { + int hours = seconds / 3600; + return ngettext("Every %d hour (custom)", "Every %d hours (custom)", hours).printf(hours); + } + if (seconds >= 60) { + int minutes = seconds / 60; + return ngettext("Every %d minute (custom)", "Every %d minutes (custom)", minutes).printf(minutes); + } + return ngettext("Every %d second (custom)", "Every %d seconds (custom)", seconds).printf(seconds); + } + // Appends a rounded-rectangle sub-path to the Cairo context. private static void round_rect(Cairo.Context ctx, double x, double y, double w, double h, double r) { double PI = Math.PI; @@ -232,7 +247,27 @@ namespace Singularity { string current_interval_id = current_interval.to_string(); bool have_interval_match = false; foreach (var opt in interval_options) if (opt.id == current_interval_id) have_interval_match = true; - if (!have_interval_match) current_interval_id = "600"; // a custom/legacy value collapses to the closest preset shown + // A stored value outside the fixed presets (set by an older + // build, or a future settings surface) gets its own entry + // showing the real value, inserted in chronological order, + // instead of silently displaying the nearest preset while + // WallpaperRotator keeps using the actual stored interval. + if (!have_interval_match) { + var custom_option = new Singularity.Core.AppSettingOption() { + id = current_interval_id, + label = format_custom_interval_label(current_interval) + }; + int insert_at = interval_options.size; + for (int i = 0; i < interval_options.size; i++) { + int preset_seconds; + if (int.try_parse(interval_options[i].id, out preset_seconds) + && current_interval < preset_seconds) { + insert_at = i; + break; + } + } + interval_options.insert(insert_at, custom_option); + } var interval_row = new SelectionRow.with_options( _("Rotation Interval"), interval_options, current_interval_id);