From a4a219dec57c9131678eca4df10c774b571801c0 Mon Sep 17 00:00:00 2001 From: Eatgrapes Date: Sat, 26 Sep 2026 18:11:50 +0800 Subject: [PATCH 01/13] refactor: add pollkit for polled background jobs and throttles The app loop hand-rolled the same pieces in many places: mpsc receivers polled with try_recv for background results, Instant fields compared against fixed intervals, and retry deadlines. pollkit wraps them as Job, Every and Cooldown so each call site says what it wants instead of how to track it. --- Cargo.lock | 8 ++ Cargo.toml | 10 +- crates/pollkit/Cargo.toml | 10 ++ crates/pollkit/src/lib.rs | 253 +++++++++++++++++++++++++++++++++++++ src/core/audio.rs | 9 +- src/core/smtc.rs | 18 +-- src/core/smtc/worker.rs | 10 +- src/main.rs | 1 + src/plugin/manager.rs | 22 +--- src/window/app.rs | 42 +++--- src/window/app/frame.rs | 111 ++++++---------- src/window/app/system.rs | 56 +++----- src/window/settings/mod.rs | 56 +++----- 13 files changed, 407 insertions(+), 199 deletions(-) create mode 100644 crates/pollkit/Cargo.toml create mode 100644 crates/pollkit/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 73058b9b..71c51b8a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -16,6 +16,7 @@ dependencies = [ "libloading 0.9.0", "log", "lrc", + "pollkit", "pulldown-cmark", "quick-xml", "realfft", @@ -3039,6 +3040,13 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "pollkit" +version = "0.1.0" +dependencies = [ + "log", +] + [[package]] name = "pollster" version = "0.4.0" diff --git a/Cargo.toml b/Cargo.toml index 9793a6b3..943a0e74 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,14 @@ version.workspace = true edition = "2024" [workspace] -members = ["crates/winisland-core", "crates/winisland-plugin-api", "crates/winisland-render", "crates/winisland-platform", "crates/winisland-platform-windows"] +members = [ + "crates/pollkit", + "crates/winisland-core", + "crates/winisland-plugin-api", + "crates/winisland-render", + "crates/winisland-platform", + "crates/winisland-platform-windows", +] [workspace.package] version = "1.3.9" @@ -38,6 +45,7 @@ winisland-core = { path = "crates/winisland-core" } winisland-render = { path = "crates/winisland-render" } winisland-platform = { path = "crates/winisland-platform" } winisland-platform-windows = { path = "crates/winisland-platform-windows" } +pollkit = { path = "crates/pollkit" } zip = "8.6.0" serde_yaml = "0.9" pulldown-cmark = { version = "0.13.4", default-features = false } diff --git a/crates/pollkit/Cargo.toml b/crates/pollkit/Cargo.toml new file mode 100644 index 00000000..e39a9bdd --- /dev/null +++ b/crates/pollkit/Cargo.toml @@ -0,0 +1,10 @@ +[package] +name = "pollkit" +version = "0.1.0" +edition = "2024" +publish = false +description = "Background jobs, interval throttles and cooldowns for synchronous polled event loops" +license = "MIT" + +[dependencies] +log = "0.4" diff --git a/crates/pollkit/src/lib.rs b/crates/pollkit/src/lib.rs new file mode 100644 index 00000000..776295d1 --- /dev/null +++ b/crates/pollkit/src/lib.rs @@ -0,0 +1,253 @@ +//! Helpers for loops that poll instead of await: a [`Job`] that runs work in the +//! background and hands back its result on a later tick, an [`Every`] throttle for +//! "do this at most once per interval", and a [`Cooldown`] for "not again until then". + +use std::fmt; +use std::sync::mpsc::{self, Receiver, TryRecvError}; +use std::sync::{Arc, Mutex, OnceLock, PoisonError}; +use std::time::{Duration, Instant}; + +static WAKE_HOOK: OnceLock = OnceLock::new(); + +/// Registers a function that runs on the worker thread whenever a job finishes, +/// so the polling loop can wake up early instead of waiting for its next tick. +pub fn set_wake_hook(hook: fn()) { + let _ = WAKE_HOOK.set(hook); +} + +fn wake() { + if let Some(hook) = WAKE_HOOK.get() { + hook(); + } +} + +#[derive(Default)] +struct CancelSlot(Mutex>>); + +impl CancelSlot { + fn set(&self, action: impl FnOnce() + Send + 'static) { + *self.0.lock().unwrap_or_else(PoisonError::into_inner) = Some(Box::new(action)); + } + + fn fire(&self) { + let action = self.0.lock().unwrap_or_else(PoisonError::into_inner).take(); + if let Some(action) = action { + action(); + } + } +} + +/// The worker went away without producing a result: its thread panicked, failed to +/// spawn, or dropped its [`Done`] handle. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Aborted; + +impl fmt::Display for Aborted { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("background job ended without a result") + } +} + +impl std::error::Error for Aborted {} + +/// Completion handle for a [`Job`]. Move it into whatever runs the work and call +/// [`Done::send`] exactly once. +pub struct Done { + sender: mpsc::Sender, + cancel: Arc, +} + +impl Done { + /// Delivers the result and wakes the polling loop. + pub fn send(self, value: T) { + if self.sender.send(value).is_ok() { + wake(); + } + } + + /// Sets what [`Job::cancel`] does. Calling it again replaces the previous action, + /// which lets a worker keep the action pointed at its current inner operation. + pub fn on_cancel(&self, action: impl FnOnce() + Send + 'static) { + self.cancel.set(action); + } +} + +/// A unit of background work polled from a synchronous loop. +/// +/// A job is either idle or running. [`Job::poll`] returns the result once and the job +/// goes back to idle. Dropping or replacing a running job runs its cancel action. +pub struct Job { + receiver: Option>, + cancel: Arc, +} + +impl Default for Job { + fn default() -> Self { + Self::idle() + } +} + +impl Job { + pub fn idle() -> Self { + Self { + receiver: None, + cancel: Arc::default(), + } + } + + /// A running job plus the handle that completes it. Use this when the work is + /// driven by something other than a plain thread, such as an async runtime. + pub fn pending() -> (Self, Done) { + let (sender, receiver) = mpsc::channel(); + let cancel = Arc::new(CancelSlot::default()); + let job = Self { + receiver: Some(receiver), + cancel: Arc::clone(&cancel), + }; + (job, Done { sender, cancel }) + } + + pub fn is_running(&self) -> bool { + self.receiver.is_some() + } + + /// Takes the result if the job has finished. Returns `None` while idle or running. + pub fn poll(&mut self) -> Option> { + let outcome = match self.receiver.as_ref()?.try_recv() { + Ok(value) => Ok(value), + Err(TryRecvError::Empty) => return None, + Err(TryRecvError::Disconnected) => Err(Aborted), + }; + self.finish(); + Some(outcome) + } + + /// Like [`Job::poll`], treating an aborted job the same as one still running. + pub fn poll_ok(&mut self) -> Option { + self.poll()?.ok() + } + + /// Runs the cancel action (if the worker registered one) and returns to idle. + pub fn cancel(&mut self) { + if self.receiver.take().is_some() { + self.cancel.fire(); + self.cancel = Arc::default(); + } + } + + fn finish(&mut self) { + self.receiver = None; + self.cancel = Arc::default(); + } +} + +impl Job { + /// Runs `work` on a new thread. + pub fn spawn(work: impl FnOnce() -> T + Send + 'static) -> Self { + Self::start(std::thread::Builder::new(), work) + } + + /// Runs `work` on a new thread with the given name. + pub fn spawn_named(name: &str, work: impl FnOnce() -> T + Send + 'static) -> Self { + Self::start(std::thread::Builder::new().name(name.to_string()), work) + } + + fn start(builder: std::thread::Builder, work: impl FnOnce() -> T + Send + 'static) -> Self { + let (job, done) = Self::pending(); + if let Err(error) = builder.spawn(move || done.send(work())) { + log::warn!("Failed to spawn job thread: {error}"); + } + job + } +} + +impl Drop for Job { + fn drop(&mut self) { + self.cancel(); + } +} + +/// Rate limits a recurring action: [`Every::due`] is true at most once per interval. +#[derive(Debug, Clone, Copy)] +pub struct Every { + interval: Duration, + last: Option, +} + +impl Every { + /// First due one interval from now. + pub fn new(interval: Duration) -> Self { + Self { + interval, + last: Some(Instant::now()), + } + } + + pub fn secs(secs: u64) -> Self { + Self::new(Duration::from_secs(secs)) + } + + pub fn millis(millis: u64) -> Self { + Self::new(Duration::from_millis(millis)) + } + + /// True when an interval has passed since the last time this returned true. + /// Put it last in a condition chain so it only ticks when the action runs. + pub fn due(&mut self, now: Instant) -> bool { + if self + .last + .is_some_and(|last| now.saturating_duration_since(last) < self.interval) + { + return false; + } + self.last = Some(now); + true + } + + pub fn due_now(&mut self) -> bool { + self.due(Instant::now()) + } + + /// Makes the next [`Every::due`] call succeed immediately. + pub fn expire(&mut self) { + self.last = None; + } + + /// Counts `now` as a tick, as if the action had just run. + pub fn restart(&mut self, now: Instant) { + self.last = Some(now); + } +} + +/// A one-shot wait: [`Cooldown::start`] it after something happens and check +/// [`Cooldown::is_ready`] before doing it again. Starts out ready. +#[derive(Debug, Clone, Copy, Default)] +pub struct Cooldown { + until: Option, +} + +impl Cooldown { + pub const fn ready() -> Self { + Self { until: None } + } + + pub fn start(&mut self, now: Instant, duration: Duration) { + self.until = Some(now + duration); + } + + pub fn start_now(&mut self, duration: Duration) { + self.start(Instant::now(), duration); + } + + pub fn is_ready(&self, now: Instant) -> bool { + self.until.is_none_or(|until| now >= until) + } + + pub fn is_ready_now(&self) -> bool { + self.is_ready(Instant::now()) + } + + pub fn clear(&mut self) { + self.until = None; + } +} diff --git a/src/core/audio.rs b/src/core/audio.rs index 738d8da1..e29e33f1 100644 --- a/src/core/audio.rs +++ b/src/core/audio.rs @@ -1,5 +1,6 @@ use cpal::traits::{DeviceTrait, HostTrait, StreamTrait}; use cpal::{FromSample, Sample, SampleFormat, Stream, StreamConfig}; +use pollkit::Cooldown; use realfft::RealFftPlanner; use std::sync::atomic::{AtomicBool, AtomicU32, Ordering}; use std::sync::{Arc, Mutex, RwLock}; @@ -394,7 +395,7 @@ impl AudioProcessor { tokio::task::spawn_blocking(move || { let mut active_process_id = 0; let mut unavailable_process_id = None; - let mut retry_after = Instant::now(); + let mut retry = Cooldown::ready(); let mut analyzer = SpectrumAnalyzer::new(LOOPBACK_SAMPLE_RATE as u32); while !context.cancel.is_cancelled() && context.is_current() { @@ -409,17 +410,17 @@ impl AudioProcessor { if process_id != active_process_id { active_process_id = process_id; unavailable_process_id = None; - retry_after = Instant::now(); + retry.clear(); } - if Instant::now() < retry_after { + if !retry.is_ready_now() { std::thread::sleep(Duration::from_millis(100)); continue; } if let Err(error) = capture_process_audio(process_id, &context, &mut analyzer) { context.set_process_capture_active(false); - retry_after = Instant::now() + PROCESS_CAPTURE_RETRY_INTERVAL; + retry.start_now(PROCESS_CAPTURE_RETRY_INTERVAL); if unavailable_process_id != Some(process_id) { unavailable_process_id = Some(process_id); log::warn!( diff --git a/src/core/smtc.rs b/src/core/smtc.rs index 830c6552..c83b71e5 100644 --- a/src/core/smtc.rs +++ b/src/core/smtc.rs @@ -1,6 +1,7 @@ use std::sync::Arc; use std::time::{Duration, Instant}; +use pollkit::Job; use tokio::sync::{mpsc, watch}; use tokio_util::sync::CancellationToken; @@ -14,19 +15,10 @@ mod worker; const SEEK_GUARD_DURATION: Duration = Duration::from_secs(4); -pub(crate) fn detect_active_apps_async() -> std::sync::mpsc::Receiver> { - let (tx, rx) = std::sync::mpsc::channel(); - let spawn_result = std::thread::Builder::new() - .name("winisland-smtc-settings".to_string()) - .spawn(move || { - let apps = crate::platform::media().detect_active_apps(); - let _ = tx.send(apps); - crate::platform::wake(); - }); - if let Err(error) = spawn_result { - log::warn!("Failed to start settings media app scan: {error}"); - } - rx +pub(crate) fn detect_active_apps_async() -> Job> { + Job::spawn_named("winisland-smtc-settings", || { + crate::platform::media().detect_active_apps() + }) } #[derive(Clone, Debug)] diff --git a/src/core/smtc/worker.rs b/src/core/smtc/worker.rs index 48eb4f5f..6f384409 100644 --- a/src/core/smtc/worker.rs +++ b/src/core/smtc/worker.rs @@ -1,5 +1,6 @@ use std::time::{Duration, Instant}; +use pollkit::Every; use tokio::sync::{mpsc, watch}; use tokio_util::sync::CancellationToken; use winisland_platform::{MediaCommand, MediaContext}; @@ -105,7 +106,7 @@ pub(super) fn smtc_poll_loop(channels: WorkerChannels, cancel: CancellationToken } } - let mut last_regular_update = Instant::now(); + let mut regular_update = Every::millis(300); let mut regular_poll_count = 0u32; while !cancel.is_cancelled() { @@ -145,7 +146,7 @@ pub(super) fn smtc_poll_loop(channels: WorkerChannels, cancel: CancellationToken enabled = next_enabled; if enabled { media_state.thumbnail_fetcher = ThumbnailFetcher::new(info_tx.clone()); - last_regular_update = Instant::now() - Duration::from_millis(301); + regular_update.expire(); media_state.last_session_seen = Instant::now(); media_state.timeline_cache.clear(); log::info!("SMTC: listener enabled"); @@ -204,10 +205,10 @@ pub(super) fn smtc_poll_loop(channels: WorkerChannels, cancel: CancellationToken current_lyrics_local_dir.as_deref(), true, ); - last_regular_update = Instant::now(); + regular_update.restart(Instant::now()); } - if last_regular_update.elapsed() > Duration::from_millis(300) { + if regular_update.due_now() { regular_poll_count += 1; let do_auto_allow = regular_poll_count.is_multiple_of(10); media_state.update( @@ -218,7 +219,6 @@ pub(super) fn smtc_poll_loop(channels: WorkerChannels, cancel: CancellationToken current_lyrics_local_dir.as_deref(), do_auto_allow, ); - last_regular_update = Instant::now(); } let _ = wake_rx.recv_timeout(Duration::from_millis(300)); diff --git a/src/main.rs b/src/main.rs index e93d0d88..efe9ac4c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -77,6 +77,7 @@ fn main() { utils::updater::start_update_checker(); + pollkit::set_wake_hook(platform::wake); let mut app = App::default(); platform::window().run(&mut app).unwrap(); if let Some(probe) = capability_probe { diff --git a/src/plugin/manager.rs b/src/plugin/manager.rs index d561132f..0e9107a5 100644 --- a/src/plugin/manager.rs +++ b/src/plugin/manager.rs @@ -3,7 +3,7 @@ use std::collections::{HashMap, HashSet}; use std::ffi::c_void; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicU64, Ordering}; -use std::sync::{Arc, Mutex, MutexGuard, OnceLock, mpsc}; +use std::sync::{Arc, Mutex, MutexGuard, OnceLock}; use super::loader::NativePlugin; use super::types::{ @@ -22,6 +22,7 @@ use super::types::{ context_from_ffi, read_c_str, widget_from_ffi, }; use super::zip_loader::{self, PluginManifest}; +use pollkit::Job; use skia_safe::{Canvas, Color, ColorType, ISize, ImageInfo, Paint, Rect}; use winisland_render::plugin_v1_backend as skia_safe; @@ -2426,22 +2427,13 @@ impl PluginManager { collect_installed_plugins(&self.plugin_dir, &disabled, self.loaded_plugin_snapshot()) } - pub fn installed_plugins_async(&self) -> mpsc::Receiver> { + pub fn installed_plugins_async(&self) -> Job> { let plugin_dir = self.plugin_dir.clone(); let loaded_plugins = self.loaded_plugin_snapshot(); - let (tx, rx) = mpsc::channel(); - let spawn_result = std::thread::Builder::new() - .name("winisland-plugin-scan".to_string()) - .spawn(move || { - let disabled = disabled_plugin_ids(&plugin_dir); - let plugins = collect_installed_plugins(&plugin_dir, &disabled, loaded_plugins); - let _ = tx.send(plugins); - crate::platform::wake(); - }); - if let Err(error) = spawn_result { - log::warn!("Failed to start plugin scan: {error}"); - } - rx + Job::spawn_named("winisland-plugin-scan", move || { + let disabled = disabled_plugin_ids(&plugin_dir); + collect_installed_plugins(&plugin_dir, &disabled, loaded_plugins) + }) } fn loaded_plugin_snapshot(&self) -> Vec { diff --git a/src/window/app.rs b/src/window/app.rs index 963c602b..26e86207 100644 --- a/src/window/app.rs +++ b/src/window/app.rs @@ -7,8 +7,8 @@ use crate::plugin::marketplace::MarketplaceCatalog; use crate::plugin::zip_loader::PluginManifest; use crate::ui::compact::CompactOverlay; use crate::window::settings::SettingsApp; +use pollkit::{Cooldown, Every, Job}; use std::path::PathBuf; -use std::sync::mpsc; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use winisland_core::config::{AppConfig, LyricTransitionAnimation, LyricTransitionMode}; use winisland_core::context::ContextManager; @@ -72,7 +72,7 @@ pub struct App { last_media_title: String, lyrics: LyricState, idle_timer: Instant, - last_effect_refresh: Instant, + effect_refresh: Every, hide: HideState, is_dragging: bool, dismissing_notification: bool, @@ -83,13 +83,13 @@ pub struct App { drag_axis: Option, last_update_time: Instant, last_render_time: Instant, - last_topmost_check: Instant, + topmost_check: Every, renderer_retry_at: Option, - last_fullscreen_check: Instant, - last_config_check: Instant, - last_monitor_check: Instant, - last_working_set_trim: Instant, - compact_widget_refresh_at: Instant, + fullscreen_check: Every, + config_check: Every, + monitor_check: Every, + working_set_trim: Every, + compact_widget_refresh: Cooldown, last_config_modified: Option, next_frame_deadline: Instant, animation_frame_interval: Duration, @@ -110,10 +110,10 @@ pub struct App { plugin_mgr: PluginManager, plugin_media_source: Option, is_light_theme: bool, - pending_install: Option>, + pending_install: Job, marketplace_catalog: Option, - pending_marketplace_catalog: Option>, - pending_marketplace_download: Option>, + pending_marketplace_catalog: Job, + pending_marketplace_download: Job, right_press_cursor: Option<(i32, i32)>, is_right_dragging: bool, right_drag_start_offset: Option<(i32, i32)>, @@ -178,7 +178,7 @@ impl Default for App { last_media_title: String::new(), lyrics: LyricState::default(), idle_timer: Instant::now(), - last_effect_refresh: Instant::now(), + effect_refresh: Every::secs(1), hide: HideState::default(), is_dragging: false, dismissing_notification: false, @@ -189,13 +189,13 @@ impl Default for App { drag_axis: None, last_update_time: Instant::now(), last_render_time: Instant::now(), - last_topmost_check: Instant::now(), + topmost_check: Every::secs(1), renderer_retry_at: None, - last_fullscreen_check: Instant::now(), - last_config_check: Instant::now(), - last_monitor_check: Instant::now(), - last_working_set_trim: Instant::now(), - compact_widget_refresh_at: Instant::now(), + fullscreen_check: Every::millis(100), + config_check: Every::millis(500), + monitor_check: Every::secs(1), + working_set_trim: Every::new(frame::WORKING_SET_TRIM_INTERVAL), + compact_widget_refresh: Cooldown::ready(), last_config_modified, next_frame_deadline: Instant::now(), animation_frame_interval: DEFAULT_ANIMATION_FRAME_INTERVAL, @@ -216,10 +216,10 @@ impl Default for App { plugin_mgr: PluginManager::default(), plugin_media_source: None, is_light_theme: false, - pending_install: None, + pending_install: Job::idle(), marketplace_catalog: None, - pending_marketplace_catalog: None, - pending_marketplace_download: None, + pending_marketplace_catalog: Job::idle(), + pending_marketplace_download: Job::idle(), right_press_cursor: None, is_right_dragging: false, right_drag_start_offset: None, diff --git a/src/window/app/frame.rs b/src/window/app/frame.rs index 22baf12d..719e87a7 100644 --- a/src/window/app/frame.rs +++ b/src/window/app/frame.rs @@ -1,4 +1,3 @@ -use std::sync::mpsc; use std::time::{Duration, Instant}; use winisland_platform::WindowPosition; @@ -20,7 +19,7 @@ const INTERACTIVE_FRAME_INTERVAL: Duration = Duration::from_millis(20); const PLAYBACK_FRAME_INTERVAL: Duration = Duration::from_millis(20); const IDLE_FRAME_INTERVAL: Duration = Duration::from_millis(50); const HIDDEN_FRAME_INTERVAL: Duration = Duration::from_millis(100); -const WORKING_SET_TRIM_INTERVAL: Duration = Duration::from_secs(20); +pub(super) const WORKING_SET_TRIM_INTERVAL: Duration = Duration::from_secs(20); const RENDERER_RECOVERY_INTERVAL: Duration = Duration::from_secs(1); const LYRIC_TRANSITION_STEP: f32 = 0.05; const LYRIC_TRANSITION_LEAD_MS: u64 = (1000.0 / (60.0 * LYRIC_TRANSITION_STEP as f64)) as u64; @@ -60,9 +59,8 @@ impl App { self.geom.win_y = self.geom.configured_y; window.set_outer_position(WindowPosition::new(self.geom.win_x, self.geom.win_y)); } - if now.duration_since(self.last_topmost_check) >= Duration::from_secs(1) { + if self.topmost_check.due(now) { Self::enforce_overlay_window(&window); - self.last_topmost_check = now; } self.handle_tray_events(&window); self.reload_config_if_changed(&window); @@ -103,7 +101,7 @@ impl App { let (music_active, media_is_playing) = self.poll_media_info(&window); - if now.duration_since(self.last_fullscreen_check) >= Duration::from_millis(100) { + if self.fullscreen_check.due(now) { self.update_fullscreen_suppression(&window, now); } @@ -219,11 +217,8 @@ impl App { } fn poll_pending_plugin_install(&mut self) { - let Some(rx) = self.pending_install.take() else { - return; - }; - match rx.try_recv() { - Ok(Ok((manifest, staging))) => { + match self.pending_install.poll() { + Some(Ok(Ok((manifest, staging)))) => { if let Err(error) = self.plugin_mgr.activate_staged_plugin(&manifest, &staging) { let _ = std::fs::remove_dir_all(staging); Self::show_toast("Plugin Error", &error); @@ -240,8 +235,7 @@ impl App { ); if let Some(settings) = self.settings.as_mut() { settings.finish_marketplace_install(); - settings - .set_plugin_inventory_receiver(self.plugin_mgr.installed_plugins_async()); + settings.set_plugin_inventory_scan(self.plugin_mgr.installed_plugins_async()); settings.set_plugin_status( winisland_core::i18n::tr("plugin_install_success"), false, @@ -249,7 +243,7 @@ impl App { } log::info!("Plugin '{}' installed via drop", manifest.name); } - Ok(Err(e)) => { + Some(Ok(Err(e))) => { Self::show_toast("Plugin Error", &e); self.set_plugin_install_error(winisland_core::i18n::tr_args( "plugin_install_failed", @@ -257,10 +251,7 @@ impl App { )); log::error!("Failed to install plugin from drop: {e}"); } - Err(mpsc::TryRecvError::Empty) => { - self.pending_install = Some(rx); - } - Err(mpsc::TryRecvError::Disconnected) => { + Some(Err(_)) => { Self::show_toast("Plugin Error", "Installation thread crashed"); log::error!("Plugin installation thread disconnected unexpectedly"); self.set_plugin_install_error(winisland_core::i18n::tr_args( @@ -268,68 +259,59 @@ impl App { &["installation thread crashed"], )); } + None => {} } } fn poll_pending_plugin_marketplace(&mut self) { - if let Some(rx) = self.pending_marketplace_catalog.take() { - match rx.try_recv() { - Ok(Ok(catalog)) => { - self.marketplace_catalog = Some(catalog.clone()); - if let Some(settings) = self.settings.as_mut() { - settings.set_marketplace_catalog(catalog); - } - } - Ok(Err(error)) => { - log::error!("Failed to load plugin marketplace: {error}"); - if self.marketplace_catalog.is_none() - && let Some(settings) = self.settings.as_mut() - { - settings.set_marketplace_error(error); - } + match self.pending_marketplace_catalog.poll() { + Some(Ok(Ok(catalog))) => { + self.marketplace_catalog = Some(catalog.clone()); + if let Some(settings) = self.settings.as_mut() { + settings.set_marketplace_catalog(catalog); } - Err(mpsc::TryRecvError::Empty) => { - self.pending_marketplace_catalog = Some(rx); + } + Some(Ok(Err(error))) => { + log::error!("Failed to load plugin marketplace: {error}"); + if self.marketplace_catalog.is_none() + && let Some(settings) = self.settings.as_mut() + { + settings.set_marketplace_error(error); } - Err(mpsc::TryRecvError::Disconnected) => { - if self.marketplace_catalog.is_none() - && let Some(settings) = self.settings.as_mut() - { - settings.set_marketplace_error( - "The marketplace task stopped unexpectedly".into(), - ); - } + } + Some(Err(_)) => { + if self.marketplace_catalog.is_none() + && let Some(settings) = self.settings.as_mut() + { + settings + .set_marketplace_error("The marketplace task stopped unexpectedly".into()); } } + None => {} } - let Some(rx) = self.pending_marketplace_download.take() else { - return; - }; - match rx.try_recv() { - Ok(Ok(path)) => { + match self.pending_marketplace_download.poll() { + Some(Ok(Ok(path))) => { if let Some(settings) = self.settings.as_mut() { settings .set_plugin_status(winisland_core::i18n::tr("plugin_installing"), false); } self.install_zip_drop(&path); } - Ok(Err(error)) => { + Some(Ok(Err(error))) => { log::error!("Failed to download marketplace plugin: {error}"); self.set_plugin_install_error(winisland_core::i18n::tr_args( "plugin_install_failed", &[&error], )); } - Err(mpsc::TryRecvError::Empty) => { - self.pending_marketplace_download = Some(rx); - } - Err(mpsc::TryRecvError::Disconnected) => { + Some(Err(_)) => { self.set_plugin_install_error(winisland_core::i18n::tr_args( "plugin_install_failed", &["the marketplace download task stopped unexpectedly"], )); } + None => {} } } @@ -371,7 +353,6 @@ impl App { } fn update_fullscreen_suppression(&mut self, window: &WindowRef, now: Instant) { - self.last_fullscreen_check = now; let prev_fullscreen = self.is_fullscreen_suppressed; self.is_fullscreen_suppressed = is_foreground_fullscreen( self.geom.monitor_pos.0, @@ -881,30 +862,24 @@ impl App { || self.is_hidden() || self.compact_overlay.is_visible() { - self.compact_widget_refresh_at = now; + self.compact_widget_refresh.clear(); return; } let Some(delay) = crate::ui::widget::compact::next_refresh_delay(&self.config.compact_widget_layout) else { - self.compact_widget_refresh_at = now; + self.compact_widget_refresh.clear(); return; }; - if now >= self.compact_widget_refresh_at { + if self.compact_widget_refresh.is_ready(now) { window.request_redraw(); - self.compact_widget_refresh_at = now + delay; + self.compact_widget_refresh.start(now, delay); } } fn periodic_effect_redraw_due(&mut self) -> bool { let is_dynamic = self.config.island_style == "dynamic"; - let due = !self.is_hidden() - && self.last_effect_refresh.elapsed().as_millis() >= 1000 - && (is_dynamic || self.expanded); - if due { - self.last_effect_refresh = Instant::now(); - } - due + !self.is_hidden() && (is_dynamic || self.expanded) && self.effect_refresh.due_now() } fn schedule_next_frame(&mut self, window: &WindowRef, now: Instant, pacing: FramePacing) { @@ -938,12 +913,10 @@ impl App { && !interactive_active && !resource_usage_active && (!self.expanded || (!playback_active && !dynamic_effect_active)) - && self.last_working_set_trim.elapsed() >= WORKING_SET_TRIM_INTERVAL + && self.working_set_trim.due(now) + && let Err(error) = crate::platform::metrics().trim_working_set() { - if let Err(error) = crate::platform::metrics().trim_working_set() { - log::warn!("Working set trim failed: {error}"); - } - self.last_working_set_trim = now; + log::warn!("Working set trim failed: {error}"); } let frame_interval = if transition_active { self.animation_frame_interval diff --git a/src/window/app/system.rs b/src/window/app/system.rs index 021e4d93..ed80a561 100644 --- a/src/window/app/system.rs +++ b/src/window/app/system.rs @@ -1,7 +1,7 @@ use std::path::Path; -use std::sync::mpsc; use std::time::{Duration, Instant}; +use pollkit::Job; use winisland_platform::{HostBackdropParams, TrayAction, WindowSize}; use crate::core::persistence::{get_config_path, load_config}; @@ -67,8 +67,8 @@ impl App { if let Some(settings) = self.settings.as_mut() { match result { Ok(()) => { - if let Some(receiver) = plugin_inventory { - settings.set_plugin_inventory_receiver(receiver); + if let Some(scan) = plugin_inventory { + settings.set_plugin_inventory_scan(scan); } settings.set_plugin_status( winisland_core::i18n::tr("plugin_state_restart"), @@ -94,8 +94,8 @@ impl App { match result { Ok(()) => { settings.set_plugin_widgets(self.widget_mgr.configurable_widgets()); - if let Some(receiver) = plugin_inventory { - settings.set_plugin_inventory_receiver(receiver); + if let Some(scan) = plugin_inventory { + settings.set_plugin_inventory_scan(scan); } settings.set_plugin_status( winisland_core::i18n::tr("plugin_uninstalled"), @@ -219,7 +219,7 @@ impl App { } pub(super) fn install_zip_drop(&mut self, path: &Path) { - if self.pending_install.is_some() || self.pending_marketplace_download.is_some() { + if self.pending_install.is_running() || self.pending_marketplace_download.is_running() { Self::show_toast("Plugin Info", "Another installation is already in progress"); if let Some(settings) = self.settings.as_mut() { settings.set_plugin_status( @@ -235,31 +235,21 @@ impl App { let plugin_dir = self.plugin_mgr.plugin_dir.clone(); let zip_path = path.to_path_buf(); - let (tx, rx) = mpsc::channel(); - - std::thread::spawn(move || { - let result = zip_loader::extract_plugin(&zip_path, &plugin_dir); - let _ = tx.send(result); - }); - - self.pending_install = Some(rx); + self.pending_install = + Job::spawn(move || zip_loader::extract_plugin(&zip_path, &plugin_dir)); log::info!("Plugin extraction started in background thread"); } fn load_plugin_marketplace(&mut self) { - if self.pending_marketplace_catalog.is_some() { + if self.pending_marketplace_catalog.is_running() { return; } if let Some(settings) = self.settings.as_mut() { settings.set_marketplace_loading(); } - let (tx, rx) = mpsc::channel(); - tokio::spawn(async move { - let result = marketplace::load_catalog().await; - let _ = tx.send(result); - crate::platform::wake(); - }); - self.pending_marketplace_catalog = Some(rx); + let (job, done) = Job::pending(); + tokio::spawn(async move { done.send(marketplace::load_catalog().await) }); + self.pending_marketplace_catalog = job; } fn install_marketplace_plugin(&mut self, plugin: MarketplacePlugin) { @@ -273,7 +263,7 @@ impl App { } return; } - if self.pending_install.is_some() || self.pending_marketplace_download.is_some() { + if self.pending_install.is_running() || self.pending_marketplace_download.is_running() { if let Some(settings) = self.settings.as_mut() { settings.finish_marketplace_install(); settings.set_plugin_status( @@ -292,13 +282,9 @@ impl App { false, ); } - let (tx, rx) = mpsc::channel(); - tokio::spawn(async move { - let result = marketplace::download_plugin(&plugin).await; - let _ = tx.send(result); - crate::platform::wake(); - }); - self.pending_marketplace_download = Some(rx); + let (job, done) = Job::pending(); + tokio::spawn(async move { done.send(marketplace::download_plugin(&plugin).await) }); + self.pending_marketplace_download = job; } pub(super) fn open_settings(&mut self) { @@ -333,7 +319,7 @@ impl App { return; }; settings.create_window(renderer, target_monitor); - settings.set_plugin_inventory_receiver(self.plugin_mgr.installed_plugins_async()); + settings.set_plugin_inventory_scan(self.plugin_mgr.installed_plugins_async()); if let Some(catalog) = self.marketplace_catalog.clone() { settings.set_marketplace_catalog(catalog); } @@ -360,7 +346,7 @@ impl App { if let Err(error) = crate::platform::metrics().trim_working_set() { log::warn!("Working set trim failed: {error}"); } - self.last_working_set_trim = Instant::now(); + self.working_set_trim.restart(Instant::now()); } log::info!("Settings window closed and resources released"); } @@ -409,8 +395,7 @@ impl App { pub(super) fn reload_config_if_changed(&mut self, window_ref: &WindowRef) { let now = Instant::now(); - if now.duration_since(self.last_config_check) >= Duration::from_millis(500) { - self.last_config_check = now; + if self.config_check.due(now) { let modified = std::fs::metadata(get_config_path()) .and_then(|metadata| metadata.modified()) .ok(); @@ -539,10 +524,9 @@ impl App { } } - if now.duration_since(self.last_monitor_check) < Duration::from_secs(1) { + if !self.monitor_check.due(now) { return; } - self.last_monitor_check = now; if let Some(monitor) = Self::get_target_monitor(window_ref, self.config.monitor_index) { if let Some(settings) = self.settings.as_mut() { settings.set_target_monitor(monitor.clone()); diff --git a/src/window/settings/mod.rs b/src/window/settings/mod.rs index 23191c53..e3a89b47 100644 --- a/src/window/settings/mod.rs +++ b/src/window/settings/mod.rs @@ -8,7 +8,7 @@ use crate::utils::settings_ui::{ SwitchAnimator, WidgetDropAnimation, WidgetEditorHover, WidgetEditorMode, WidgetEditorSlot, WidgetSource, }; -use std::sync::mpsc; +use pollkit::Job; use std::time::{Duration, Instant}; use winisland_core::anim::AnimPool; use winisland_core::config::AppConfig; @@ -202,7 +202,7 @@ pub struct SettingsApp { pub(crate) last_frame_time: Instant, pub(crate) next_frame_deadline: Instant, pub(crate) detected_apps: Vec, - detected_apps_rx: Option>>, + detected_apps_scan: Job>, pub(crate) sidebar_hover: i32, pub(crate) popup: Option, pub(crate) number_input: Option, @@ -235,7 +235,7 @@ pub struct SettingsApp { pub(crate) resource_editor_open: bool, pub(crate) plugin_widgets: Vec, pub(crate) plugins: Vec, - plugin_inventory_rx: Option>>, + plugin_inventory_scan: Job>, pub(crate) plugin_page_tab: PluginPageTab, pub(crate) marketplace_state: MarketplaceViewState, pub(crate) marketplace_installing_id: Option, @@ -333,7 +333,7 @@ impl SettingsApp { last_frame_time: Instant::now(), next_frame_deadline: Instant::now(), detected_apps, - detected_apps_rx: None, + detected_apps_scan: Job::idle(), sidebar_hover: -1, popup: None, number_input: None, @@ -366,7 +366,7 @@ impl SettingsApp { resource_editor_open: false, plugin_widgets, plugins, - plugin_inventory_rx: None, + plugin_inventory_scan: Job::idle(), plugin_page_tab: PluginPageTab::Installed, marketplace_state: MarketplaceViewState::NotLoaded, marketplace_installing_id: None, @@ -478,42 +478,31 @@ impl SettingsApp { if changed { self.items_dirty = true; } - if self.detected_apps_rx.is_none() { - self.detected_apps_rx = Some(crate::core::smtc::detect_active_apps_async()); + if !self.detected_apps_scan.is_running() { + self.detected_apps_scan = crate::core::smtc::detect_active_apps_async(); } } fn poll_detected_apps(&mut self) { - let Some(rx) = self.detected_apps_rx.take() else { + let Some(apps) = self.detected_apps_scan.poll_ok() else { return; }; - match rx.try_recv() { - Ok(apps) => { - let mut changed = false; - for app in apps { - if !self.detected_apps.contains(&app) { - self.detected_apps.push(app); - changed = true; - } - } - if changed { - self.items_dirty = true; - self.request_redraw(); - } + let mut changed = false; + for app in apps { + if !self.detected_apps.contains(&app) { + self.detected_apps.push(app); + changed = true; } - Err(mpsc::TryRecvError::Empty) => self.detected_apps_rx = Some(rx), - Err(mpsc::TryRecvError::Disconnected) => {} + } + if changed { + self.items_dirty = true; + self.request_redraw(); } } fn poll_plugin_inventory(&mut self) { - let Some(rx) = self.plugin_inventory_rx.take() else { - return; - }; - match rx.try_recv() { - Ok(plugins) => self.set_plugins(plugins), - Err(mpsc::TryRecvError::Empty) => self.plugin_inventory_rx = Some(rx), - Err(mpsc::TryRecvError::Disconnected) => {} + if let Some(plugins) = self.plugin_inventory_scan.poll_ok() { + self.set_plugins(plugins); } } } @@ -1339,11 +1328,8 @@ impl SettingsApp { self.request_redraw(); } - pub(crate) fn set_plugin_inventory_receiver( - &mut self, - receiver: mpsc::Receiver>, - ) { - self.plugin_inventory_rx = Some(receiver); + pub(crate) fn set_plugin_inventory_scan(&mut self, scan: Job>) { + self.plugin_inventory_scan = scan; } pub(crate) fn set_plugin_widgets(&mut self, plugin_widgets: Vec) { From 555abde3a361df62516159318416bbca2708db2d Mon Sep 17 00:00:00 2001 From: Eatgrapes Date: Sat, 26 Sep 2026 18:58:07 +0800 Subject: [PATCH 02/13] refactor(plugin): add c-result so host calls can use ? Every plugin host call returned PluginResultC directly, so each fallible step was spelled out as a match or if-let that converted the error by hand, plus a lock_runtime_or_return macro for the lock. c-result is an attribute macro that lets these functions return a Rust Result and use ?, and converts to the C type at the boundary. The conversion is a From impl on PluginResultC in the plugin API, so plugins can use it for their own callbacks as well. --- Cargo.lock | 9 + Cargo.toml | 2 + crates/c-result/Cargo.toml | 14 + crates/c-result/src/lib.rs | 30 ++ crates/winisland-plugin-api/src/types/mod.rs | 10 + src/plugin/manager.rs | 382 ++++++++----------- 6 files changed, 214 insertions(+), 233 deletions(-) create mode 100644 crates/c-result/Cargo.toml create mode 100644 crates/c-result/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 71c51b8a..302ad759 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7,6 +7,7 @@ name = "WinIsland" version = "1.3.9" dependencies = [ "base64 0.23.1", + "c-result", "cpal", "dirs", "ed25519-dalek 3.0.0", @@ -470,6 +471,14 @@ dependencies = [ "libbz2-rs-sys", ] +[[package]] +name = "c-result" +version = "0.1.0" +dependencies = [ + "quote", + "syn 2.0.119", +] + [[package]] name = "cairo-rs" version = "0.18.5" diff --git a/Cargo.toml b/Cargo.toml index 943a0e74..a3f1729d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,6 +5,7 @@ edition = "2024" [workspace] members = [ + "crates/c-result", "crates/pollkit", "crates/winisland-core", "crates/winisland-plugin-api", @@ -46,6 +47,7 @@ winisland-render = { path = "crates/winisland-render" } winisland-platform = { path = "crates/winisland-platform" } winisland-platform-windows = { path = "crates/winisland-platform-windows" } pollkit = { path = "crates/pollkit" } +c-result = { path = "crates/c-result" } zip = "8.6.0" serde_yaml = "0.9" pulldown-cmark = { version = "0.13.4", default-features = false } diff --git a/crates/c-result/Cargo.toml b/crates/c-result/Cargo.toml new file mode 100644 index 00000000..152bf453 --- /dev/null +++ b/crates/c-result/Cargo.toml @@ -0,0 +1,14 @@ +[package] +name = "c-result" +version = "0.1.0" +edition = "2024" +publish = false +description = "Write extern \"C\" functions with Result and ? while exporting a C result type" +license = "MIT" + +[lib] +proc-macro = true + +[dependencies] +quote = "1" +syn = { version = "2", features = ["full"] } diff --git a/crates/c-result/src/lib.rs b/crates/c-result/src/lib.rs new file mode 100644 index 00000000..a98c78d6 --- /dev/null +++ b/crates/c-result/src/lib.rs @@ -0,0 +1,30 @@ +use proc_macro::TokenStream; +use quote::quote; +use syn::{ItemFn, ReturnType, Type, parse_macro_input, parse_quote}; + +/// `#[c_result(CType)]` lets an `extern "C"` function be written against a Rust `Result` +/// and use `?`, while the exported symbol returns `CType`. The value is converted with +/// `From> for CType`, so the error type only needs whatever that impl accepts. +#[proc_macro_attribute] +pub fn c_result(attr: TokenStream, item: TokenStream) -> TokenStream { + let c_type = parse_macro_input!(attr as Type); + let ItemFn { + attrs, + vis, + mut sig, + block, + } = parse_macro_input!(item as ItemFn); + let ReturnType::Type(_, result_type) = sig.output.clone() else { + return syn::Error::new_spanned(sig.fn_token, "c_result needs a function returning Result") + .to_compile_error() + .into(); + }; + sig.output = parse_quote!(-> #c_type); + quote! { + #(#attrs)* + #vis #sig { + <#c_type as ::core::convert::From<#result_type>>::from((move || -> #result_type #block)()) + } + } + .into() +} diff --git a/crates/winisland-plugin-api/src/types/mod.rs b/crates/winisland-plugin-api/src/types/mod.rs index 65ac4331..f3bd08c5 100644 --- a/crates/winisland-plugin-api/src/types/mod.rs +++ b/crates/winisland-plugin-api/src/types/mod.rs @@ -111,6 +111,16 @@ impl PluginResultC { } } +/// Build a result from a Rust `Result`, using the error's `Display` text as the message. +impl From> for PluginResultC { + fn from(result: Result<(), E>) -> Self { + match result { + Ok(()) => Self::ok(), + Err(error) => Self::err(&error.to_string()), + } + } +} + /// Fill a fixed-size byte buffer with a string, zeroing the rest. /// /// Useful for initialising `#[repr(C)]` struct fields with a diff --git a/src/plugin/manager.rs b/src/plugin/manager.rs index 0e9107a5..86c1bdc6 100644 --- a/src/plugin/manager.rs +++ b/src/plugin/manager.rs @@ -22,10 +22,13 @@ use super::types::{ context_from_ffi, read_c_str, widget_from_ffi, }; use super::zip_loader::{self, PluginManifest}; +use c_result::c_result; use pollkit::Job; use skia_safe::{Canvas, Color, ColorType, ISize, ImageInfo, Paint, Rect}; use winisland_render::plugin_v1_backend as skia_safe; +type HostResult = Result<(), &'static str>; + const MAX_COVER_BYTES: u32 = 16 * 1024 * 1024; const MAX_CONTEXTS_PER_PLUGIN: usize = 64; const MAX_MEDIA_SOURCES_PER_PLUGIN: usize = 4; @@ -243,19 +246,10 @@ fn release_runtime(state: MutexGuard<'static, RuntimeState>) { crate::platform::wake(); } -macro_rules! lock_runtime_or_return { - () => { - match lock_runtime() { - Ok(state) => state, - Err(error) => return PluginResultC::err(error), - } - }; -} - macro_rules! require_output { ($ptr:expr, $message:literal) => { if $ptr.is_null() { - return PluginResultC::err($message); + return Err($message); } }; } @@ -719,26 +713,20 @@ unsafe fn read_utf8(value: Utf8SliceV1, max_len: u32) -> Result PluginResultC { +) -> HostResult { require_output!(out_id, "resource output pointer is null"); // SAFETY: read_widget_data validates and copies the versioned widget structure. - let data = match unsafe { read_struct(data) } { - Ok(data) => data, - Err(error) => return PluginResultC::err(error), - }; - if let Err(error) = validate_context_data(&data) { - return PluginResultC::err(error); - } - let mut state = lock_runtime_or_return!(); - if let Err(error) = require_capability(&state, token, CAPABILITY_CONTEXT) { - return PluginResultC::err(error); - } + let data = unsafe { read_struct(data) }?; + validate_context_data(&data)?; + let mut state = lock_runtime()?; + require_capability(&state, token, CAPABILITY_CONTEXT)?; if resource_count(&state, token, ResourceKind::Context) >= MAX_CONTEXTS_PER_PLUGIN { - return PluginResultC::err("context resource limit reached"); + return Err("context resource limit reached"); } let id = next_id(&NEXT_RESOURCE_ID); let context = context_from_ffi(id, &data); @@ -757,28 +745,22 @@ unsafe extern "C" fn context_create( // SAFETY: out_id was checked non-null and belongs to the caller. unsafe { out_id.write(id) }; release_runtime(state); - PluginResultC::ok() + Ok(()) } +#[c_result(PluginResultC)] unsafe extern "C" fn context_update( token: PluginToken, id: ResourceId, data: *const ContextDataV1, -) -> PluginResultC { +) -> HostResult { // SAFETY: read_widget_data validates and copies the versioned widget structure. - let data = match unsafe { read_struct(data) } { - Ok(data) => data, - Err(error) => return PluginResultC::err(error), - }; - if let Err(error) = validate_context_data(&data) { - return PluginResultC::err(error); - } + let data = unsafe { read_struct(data) }?; + validate_context_data(&data)?; let context = context_from_ffi(id, &data); let size_bytes = context.title.len() + context.body.len() + context.compact_text.len(); - let mut state = lock_runtime_or_return!(); - if let Err(error) = require_resource(&state, token, id, ResourceKind::Context) { - return PluginResultC::err(error); - } + let mut state = lock_runtime()?; + require_resource(&state, token, id, ResourceKind::Context)?; if let Some(owner) = state.resources.get_mut(&id) { owner.size_bytes = size_bytes; } @@ -786,21 +768,20 @@ unsafe extern "C" fn context_update( .context_events .insert(id, ContextEvent::Upsert(context)); release_runtime(state); - PluginResultC::ok() + Ok(()) } -unsafe extern "C" fn context_release(token: PluginToken, id: ResourceId) -> PluginResultC { - let mut state = lock_runtime_or_return!(); - if let Err(error) = require_resource(&state, token, id, ResourceKind::Context) { - return PluginResultC::err(error); - } +#[c_result(PluginResultC)] +unsafe extern "C" fn context_release(token: PluginToken, id: ResourceId) -> HostResult { + let mut state = lock_runtime()?; + require_resource(&state, token, id, ResourceKind::Context)?; state.resources.remove(&id); state.context_events.remove(&id); if state.visible_contexts.remove(&id) { state.context_events.insert(id, ContextEvent::Remove(id)); } release_runtime(state); - PluginResultC::ok() + Ok(()) } fn copy_media(data: &MediaSourceDataV1, id: ResourceId) -> Result { @@ -851,34 +832,27 @@ fn copy_media(data: &MediaSourceDataV1, id: ResourceId) -> Result PluginResultC { +) -> HostResult { require_output!(out_id, "resource output pointer is null"); // SAFETY: Validation is performed by read_struct before the value is used. - let data = match unsafe { read_struct(data) } { - Ok(data) => data, - Err(error) => return PluginResultC::err(error), - }; + let data = unsafe { read_struct(data) }?; let id = next_id(&NEXT_RESOURCE_ID); - let mut media = match copy_media(&data, id) { - Ok(media) => media, - Err(error) => return PluginResultC::err(error), - }; - let mut state = lock_runtime_or_return!(); - if let Err(error) = require_capability(&state, token, CAPABILITY_MEDIA) { - return PluginResultC::err(error); - } + let mut media = copy_media(&data, id)?; + let mut state = lock_runtime()?; + require_capability(&state, token, CAPABILITY_MEDIA)?; if resource_count(&state, token, ResourceKind::Media) >= MAX_MEDIA_SOURCES_PER_PLUGIN { - return PluginResultC::err("media resource limit reached"); + return Err("media resource limit reached"); } if resource_bytes(&state, token, ResourceKind::Media, None) .saturating_add(media.data.cover_data.len()) > MAX_MEDIA_BYTES_PER_PLUGIN { - return PluginResultC::err("media resources exceed the 32 MiB limit"); + return Err("media resources exceed the 32 MiB limit"); } state.media_sequence = state.media_sequence.wrapping_add(1); media.sequence = state.media_sequence; @@ -895,39 +869,32 @@ unsafe extern "C" fn media_create( // SAFETY: out_id was checked non-null and belongs to the caller. unsafe { out_id.write(id) }; release_runtime(state); - PluginResultC::ok() + Ok(()) } +#[c_result(PluginResultC)] unsafe extern "C" fn media_update( token: PluginToken, id: ResourceId, data: *const MediaSourceDataV1, -) -> PluginResultC { +) -> HostResult { // SAFETY: Validation is performed by read_struct before the value is used. - let data = match unsafe { read_struct(data) } { - Ok(data) => data, - Err(error) => return PluginResultC::err(error), - }; - let mut media = match copy_media(&data, id) { - Ok(media) => media, - Err(error) => return PluginResultC::err(error), - }; - let mut state = lock_runtime_or_return!(); - if let Err(error) = require_resource(&state, token, id, ResourceKind::Media) { - return PluginResultC::err(error); - } + let data = unsafe { read_struct(data) }?; + let mut media = copy_media(&data, id)?; + let mut state = lock_runtime()?; + require_resource(&state, token, id, ResourceKind::Media)?; if state .media .get(&id) .is_some_and(|media| media.in_flight != 0) { - return PluginResultC::err("media callback is in progress"); + return Err("media callback is in progress"); } if resource_bytes(&state, token, ResourceKind::Media, Some(id)) .saturating_add(media.data.cover_data.len()) > MAX_MEDIA_BYTES_PER_PLUGIN { - return PluginResultC::err("media resources exceed the 32 MiB limit"); + return Err("media resources exceed the 32 MiB limit"); } state.media_sequence = state.media_sequence.wrapping_add(1); media.sequence = state.media_sequence; @@ -937,86 +904,75 @@ unsafe extern "C" fn media_update( state.media.insert(id, media); state.media_dirty = true; release_runtime(state); - PluginResultC::ok() + Ok(()) } -unsafe extern "C" fn media_release(token: PluginToken, id: ResourceId) -> PluginResultC { - let mut state = lock_runtime_or_return!(); - if let Err(error) = require_resource(&state, token, id, ResourceKind::Media) { - return PluginResultC::err(error); - } +#[c_result(PluginResultC)] +unsafe extern "C" fn media_release(token: PluginToken, id: ResourceId) -> HostResult { + let mut state = lock_runtime()?; + require_resource(&state, token, id, ResourceKind::Media)?; if state .media .get(&id) .is_some_and(|media| media.in_flight != 0) { - return PluginResultC::err("media callback is in progress"); + return Err("media callback is in progress"); } state.resources.remove(&id); state.media.remove(&id); state.media_dirty = true; release_runtime(state); - PluginResultC::ok() + Ok(()) } +#[c_result(PluginResultC)] unsafe extern "C" fn i18n_register_bundle( token: PluginToken, language: Utf8SliceV1, pairs: *const TranslationPairV1, count: u32, out_id: *mut ResourceId, -) -> PluginResultC { +) -> HostResult { require_output!(out_id, "resource output pointer is null"); if count == 0 || count > MAX_TRANSLATION_PAIRS || pairs.is_null() { - return PluginResultC::err("translation bundle is empty or too large"); + return Err("translation bundle is empty or too large"); } // SAFETY: The plugin owns the borrowed language bytes for this call. - let language = match unsafe { read_utf8(language, 64) } { - Ok(language) if !language.is_empty() => language, - Ok(_) => return PluginResultC::err("language is empty"), - Err(error) => return PluginResultC::err(error), - }; + let language = unsafe { read_utf8(language, 64) }?; + if language.is_empty() { + return Err("language is empty"); + } // SAFETY: count is bounded and the plugin guarantees this borrowed array is valid. let pairs = unsafe { std::slice::from_raw_parts(pairs, count as usize) }; let mut copied = Vec::with_capacity(pairs.len()); let mut total_bytes = 0usize; for pair in pairs { // SAFETY: Translation strings are borrowed for this call and copied immediately. - let key = match unsafe { read_utf8(pair.key, MAX_TRANSLATION_STRING_BYTES) } { - Ok(key) if !key.is_empty() => key, - Ok(_) => return PluginResultC::err("translation key is empty"), - Err(error) => return PluginResultC::err(error), - }; + let key = unsafe { read_utf8(pair.key, MAX_TRANSLATION_STRING_BYTES) }?; + if key.is_empty() { + return Err("translation key is empty"); + } // SAFETY: Translation strings are borrowed for this call and copied immediately. - let value = match unsafe { read_utf8(pair.value, MAX_TRANSLATION_STRING_BYTES) } { - Ok(value) => value, - Err(error) => return PluginResultC::err(error), - }; + let value = unsafe { read_utf8(pair.value, MAX_TRANSLATION_STRING_BYTES) }?; total_bytes = match total_bytes.checked_add(key.len() + value.len()) { Some(total) if total <= MAX_TRANSLATION_BUNDLE_BYTES => total, - _ => return PluginResultC::err("translation bundle exceeds 1 MiB"), + _ => return Err("translation bundle exceeds 1 MiB"), }; copied.push((key, value)); } - let mut state = lock_runtime_or_return!(); - if let Err(error) = require_capability(&state, token, CAPABILITY_I18N) { - return PluginResultC::err(error); - } + let mut state = lock_runtime()?; + require_capability(&state, token, CAPABILITY_I18N)?; if resource_count(&state, token, ResourceKind::I18n) >= MAX_I18N_BUNDLES_PER_PLUGIN { - return PluginResultC::err("translation bundle limit reached"); + return Err("translation bundle limit reached"); } if resource_bytes(&state, token, ResourceKind::I18n, None).saturating_add(total_bytes) > MAX_I18N_BYTES_PER_PLUGIN { - return PluginResultC::err("translation bundles exceed the 4 MiB limit"); + return Err("translation bundles exceed the 4 MiB limit"); } let id = next_id(&NEXT_RESOURCE_ID); - if let Err(error) = - winisland_core::i18n::register_plugin_translation_bundle(id, language, copied) - { - return PluginResultC::err(error); - } + winisland_core::i18n::register_plugin_translation_bundle(id, language, copied)?; state.resources.insert( id, ResourceOwner { @@ -1028,69 +984,58 @@ unsafe extern "C" fn i18n_register_bundle( // SAFETY: out_id was checked non-null and belongs to the caller. unsafe { out_id.write(id) }; release_runtime(state); - PluginResultC::ok() + Ok(()) } -unsafe extern "C" fn i18n_release_bundle(token: PluginToken, id: ResourceId) -> PluginResultC { - let mut state = lock_runtime_or_return!(); - if let Err(error) = require_resource(&state, token, id, ResourceKind::I18n) { - return PluginResultC::err(error); - } - if let Err(error) = winisland_core::i18n::release_plugin_translation_bundle(id) { - return PluginResultC::err(error); - } +#[c_result(PluginResultC)] +unsafe extern "C" fn i18n_release_bundle(token: PluginToken, id: ResourceId) -> HostResult { + let mut state = lock_runtime()?; + require_resource(&state, token, id, ResourceKind::I18n)?; + winisland_core::i18n::release_plugin_translation_bundle(id)?; state.resources.remove(&id); release_runtime(state); - PluginResultC::ok() + Ok(()) } -unsafe extern "C" fn host_state_get( - token: PluginToken, - out_state: *mut HostStateV1, -) -> PluginResultC { +#[c_result(PluginResultC)] +unsafe extern "C" fn host_state_get(token: PluginToken, out_state: *mut HostStateV1) -> HostResult { if out_state.is_null() { - return PluginResultC::err("host state output pointer is null"); + return Err("host state output pointer is null"); } // SAFETY: HostStateV1 begins with struct_size, which is readable by contract. let struct_size = unsafe { std::ptr::read_unaligned(out_state.cast::()) }; if struct_size < std::mem::size_of::() as u32 { - return PluginResultC::err("host state output struct is truncated"); - } - let state = lock_runtime_or_return!(); - if let Err(error) = require_capability(&state, token, CAPABILITY_HOST_STATE) { - return PluginResultC::err(error); + return Err("host state output struct is truncated"); } + let state = lock_runtime()?; + require_capability(&state, token, CAPABILITY_HOST_STATE)?; let snapshot = HostStateV1::from(&state.host_state); // SAFETY: The size check proves the caller provided a complete v1 output struct. unsafe { out_state.write(snapshot) }; - PluginResultC::ok() + Ok(()) } +#[c_result(PluginResultC)] unsafe extern "C" fn lyrics_transform_register( token: PluginToken, data: *const LyricsTransformerDataV1, out_id: *mut ResourceId, -) -> PluginResultC { +) -> HostResult { require_output!(out_id, "resource output pointer is null"); // SAFETY: Validation is performed by read_struct before the value is used. - let data = match unsafe { read_struct(data) } { - Ok(data) => data, - Err(error) => return PluginResultC::err(error), - }; + let data = unsafe { read_struct(data) }?; if data.flags != 0 { - return PluginResultC::err("lyrics transformer contains unknown flags"); + return Err("lyrics transformer contains unknown flags"); } let Some(on_transform) = data.on_transform else { - return PluginResultC::err("lyrics transform callback is required"); + return Err("lyrics transform callback is required"); }; - let mut state = lock_runtime_or_return!(); - if let Err(error) = require_capability(&state, token, CAPABILITY_LYRICS_TRANSFORM) { - return PluginResultC::err(error); - } + let mut state = lock_runtime()?; + require_capability(&state, token, CAPABILITY_LYRICS_TRANSFORM)?; if resource_count(&state, token, ResourceKind::LyricsTransform) >= MAX_LYRICS_TRANSFORMERS_PER_PLUGIN { - return PluginResultC::err("lyrics transformer resource limit reached"); + return Err("lyrics transformer resource limit reached"); } let id = next_id(&NEXT_RESOURCE_ID); state.lyrics_transformer_sequence = state.lyrics_transformer_sequence.wrapping_add(1); @@ -1114,24 +1059,23 @@ unsafe extern "C" fn lyrics_transform_register( ); // SAFETY: out_id was checked non-null and belongs to the caller. unsafe { out_id.write(id) }; - PluginResultC::ok() + Ok(()) } -unsafe extern "C" fn lyrics_transform_release(token: PluginToken, id: ResourceId) -> PluginResultC { - let mut state = lock_runtime_or_return!(); - if let Err(error) = require_resource(&state, token, id, ResourceKind::LyricsTransform) { - return PluginResultC::err(error); - } +#[c_result(PluginResultC)] +unsafe extern "C" fn lyrics_transform_release(token: PluginToken, id: ResourceId) -> HostResult { + let mut state = lock_runtime()?; + require_resource(&state, token, id, ResourceKind::LyricsTransform)?; if state .lyrics_transformers .get(&id) .is_some_and(|transformer| transformer.in_flight != 0) { - return PluginResultC::err("lyrics transform callback is in progress"); + return Err("lyrics transform callback is in progress"); } state.resources.remove(&id); state.lyrics_transformers.remove(&id); - PluginResultC::ok() + Ok(()) } fn ctx_ref<'a>(ctx: *const WidgetDrawContextV1) -> Option<&'a WidgetDrawContextV1> { @@ -1479,45 +1423,36 @@ unsafe extern "C" fn ffi_translate(ctx: *const WidgetDrawContextV1, dx: f32, dy: }); } +#[c_result(PluginResultC)] unsafe extern "C" fn widget_create( token: PluginToken, data: *const WidgetDataV1, out_id: *mut ResourceId, -) -> PluginResultC { +) -> HostResult { require_output!(out_id, "widget output pointer is null"); // SAFETY: Validation is performed by read_struct before the value is used. - let data = match unsafe { read_widget_data(data) } { - Ok(data) => data, - Err(error) => return PluginResultC::err(error), - }; - if let Err(error) = validate_widget_data(&data) { - return PluginResultC::err(error); - } - let mut state = lock_runtime_or_return!(); - if let Err(error) = require_capability(&state, token, CAPABILITY_WIDGET) { - return PluginResultC::err(error); - } + let data = unsafe { read_widget_data(data) }?; + validate_widget_data(&data)?; + let mut state = lock_runtime()?; + require_capability(&state, token, CAPABILITY_WIDGET)?; if resource_count(&state, token, ResourceKind::Widget) >= MAX_WIDGETS_PER_PLUGIN { - return PluginResultC::err("widget resource limit reached"); + return Err("widget resource limit reached"); } let plugin_id = match state.plugins.get(&token) { Some(plugin) => plugin.id.clone(), - None => return PluginResultC::err("invalid plugin token"), - }; - let key = match validate_widget_key(&data.key) { - Ok(key) => key, - Err(error) => return PluginResultC::err(error), + None => return Err("invalid plugin token"), }; + let key = validate_widget_key(&data.key)?; if key .as_ref() .is_some_and(|key| state.widget_keys.contains_key(&(token, key.to_string()))) { - return PluginResultC::err("widget key is already registered by this plugin"); + return Err("widget key is already registered by this plugin"); } let id = next_id(&NEXT_RESOURCE_ID); let widget = widget_from_ffi(&plugin_id, id, &data); let Some(on_draw) = data.on_draw else { - return PluginResultC::err("widget render callback is required"); + return Err("widget render callback is required"); }; let size_bytes = widget.title.len() + widget.body.len(); state.resources.insert( @@ -1543,30 +1478,21 @@ unsafe extern "C" fn widget_create( // SAFETY: out_id was checked non-null and belongs to the caller. unsafe { out_id.write(id) }; release_runtime(state); - PluginResultC::ok() + Ok(()) } +#[c_result(PluginResultC)] unsafe extern "C" fn widget_update( token: PluginToken, id: ResourceId, data: *const WidgetDataV1, -) -> PluginResultC { +) -> HostResult { // SAFETY: Validation is performed by read_struct before the value is used. - let data = match unsafe { read_widget_data(data) } { - Ok(data) => data, - Err(error) => return PluginResultC::err(error), - }; - if let Err(error) = validate_widget_data(&data) { - return PluginResultC::err(error); - } - let mut state = lock_runtime_or_return!(); - if let Err(error) = require_resource(&state, token, id, ResourceKind::Widget) { - return PluginResultC::err(error); - } - let key = match validate_widget_key(&data.key) { - Ok(key) => key, - Err(error) => return PluginResultC::err(error), - }; + let data = unsafe { read_widget_data(data) }?; + validate_widget_data(&data)?; + let mut state = lock_runtime()?; + require_resource(&state, token, id, ResourceKind::Widget)?; + let key = validate_widget_key(&data.key)?; let existing_key = state .widget_keys .iter() @@ -1574,18 +1500,18 @@ unsafe extern "C" fn widget_update( (*owner == token && *resource == id).then_some(key.as_str()) }); if existing_key != key.as_deref() { - return PluginResultC::err("widget key cannot change after creation"); + return Err("widget key cannot change after creation"); } if state .widgets .get(&id) .is_some_and(|widget| widget.in_flight != 0) { - return PluginResultC::err("widget render callback is in progress"); + return Err("widget render callback is in progress"); } let plugin_id = match state.plugins.get(&token) { Some(plugin) => plugin.id.clone(), - None => return PluginResultC::err("invalid plugin token"), + None => return Err("invalid plugin token"), }; let widget = widget_from_ffi(&plugin_id, id, &data); let size_bytes = widget.title.len() + widget.body.len(); @@ -1593,7 +1519,7 @@ unsafe extern "C" fn widget_update( owner.size_bytes = size_bytes; } let Some(on_draw) = data.on_draw else { - return PluginResultC::err("widget render callback is required"); + return Err("widget render callback is required"); }; state.widgets.insert( id, @@ -1605,20 +1531,19 @@ unsafe extern "C" fn widget_update( ); state.widget_events.insert(id, WidgetEvent::Upsert(widget)); release_runtime(state); - PluginResultC::ok() + Ok(()) } -unsafe extern "C" fn widget_release(token: PluginToken, id: ResourceId) -> PluginResultC { - let mut state = lock_runtime_or_return!(); - if let Err(error) = require_resource(&state, token, id, ResourceKind::Widget) { - return PluginResultC::err(error); - } +#[c_result(PluginResultC)] +unsafe extern "C" fn widget_release(token: PluginToken, id: ResourceId) -> HostResult { + let mut state = lock_runtime()?; + require_resource(&state, token, id, ResourceKind::Widget)?; if state .widgets .get(&id) .is_some_and(|widget| widget.in_flight != 0) { - return PluginResultC::err("widget render callback is in progress"); + return Err("widget render callback is in progress"); } state.resources.remove(&id); state.widgets.remove(&id); @@ -1628,39 +1553,35 @@ unsafe extern "C" fn widget_release(token: PluginToken, id: ResourceId) -> Plugi state.widget_events.remove(&id); state.widget_events.insert(id, WidgetEvent::Remove(id)); release_runtime(state); - PluginResultC::ok() + Ok(()) } +#[c_result(PluginResultC)] unsafe extern "C" fn settings_create( token: PluginToken, data: *const SettingsPageDataV1, out_id: *mut ResourceId, -) -> PluginResultC { +) -> HostResult { require_output!(out_id, "settings page output pointer is null"); - let mut state = lock_runtime_or_return!(); - if let Err(error) = require_capability(&state, token, CAPABILITY_SETTINGS) { - return PluginResultC::err(error); - } + let mut state = lock_runtime()?; + require_capability(&state, token, CAPABILITY_SETTINGS)?; if resource_count(&state, token, ResourceKind::Settings) >= MAX_SETTINGS_PAGES_PER_PLUGIN { - return PluginResultC::err("settings page limit reached"); + return Err("settings page limit reached"); } let id = next_id(&NEXT_RESOURCE_ID); let sequence = state.settings_sequence.wrapping_add(1); // SAFETY: The plugin keeps the page and nested borrowed data valid for this call. - let (resource, size_bytes) = match unsafe { copy_settings_page(id, sequence, data) } { - Ok(resource) => resource, - Err(error) => return PluginResultC::err(error), - }; + let (resource, size_bytes) = unsafe { copy_settings_page(id, sequence, data) }?; if state .settings_keys .contains_key(&(token, resource.page.key.clone())) { - return PluginResultC::err("settings page key is already registered by this plugin"); + return Err("settings page key is already registered by this plugin"); } if resource_bytes(&state, token, ResourceKind::Settings, None).saturating_add(size_bytes) > MAX_SETTINGS_BYTES_PER_PLUGIN { - return PluginResultC::err("settings page exceeds the 2 MiB limit"); + return Err("settings page exceeds the 2 MiB limit"); } state.settings_sequence = sequence; state.resources.insert( @@ -1679,36 +1600,32 @@ unsafe extern "C" fn settings_create( // SAFETY: out_id was checked non-null and belongs to the caller. unsafe { out_id.write(id) }; release_runtime(state); - PluginResultC::ok() + Ok(()) } +#[c_result(PluginResultC)] unsafe extern "C" fn settings_update( token: PluginToken, id: ResourceId, data: *const SettingsPageDataV1, -) -> PluginResultC { - let mut state = lock_runtime_or_return!(); - if let Err(error) = require_resource(&state, token, id, ResourceKind::Settings) { - return PluginResultC::err(error); - } +) -> HostResult { + let mut state = lock_runtime()?; + require_resource(&state, token, id, ResourceKind::Settings)?; let Some(existing) = state.settings.get(&id) else { - return PluginResultC::err("settings page was not found"); + return Err("settings page was not found"); }; let existing_key = existing.page.key.clone(); let sequence = existing.page.sequence; let in_flight = existing.in_flight; // SAFETY: The plugin keeps the page and nested borrowed data valid for this call. - let (mut resource, size_bytes) = match unsafe { copy_settings_page(id, sequence, data) } { - Ok(resource) => resource, - Err(error) => return PluginResultC::err(error), - }; + let (mut resource, size_bytes) = unsafe { copy_settings_page(id, sequence, data) }?; if resource.page.key != existing_key { - return PluginResultC::err("settings page key cannot change after creation"); + return Err("settings page key cannot change after creation"); } if resource_bytes(&state, token, ResourceKind::Settings, Some(id)).saturating_add(size_bytes) > MAX_SETTINGS_BYTES_PER_PLUGIN { - return PluginResultC::err("settings page exceeds the 2 MiB limit"); + return Err("settings page exceeds the 2 MiB limit"); } resource.in_flight = in_flight; if let Some(owner) = state.resources.get_mut(&id) { @@ -1717,20 +1634,19 @@ unsafe extern "C" fn settings_update( state.settings.insert(id, resource); state.settings_dirty = true; release_runtime(state); - PluginResultC::ok() + Ok(()) } -unsafe extern "C" fn settings_release(token: PluginToken, id: ResourceId) -> PluginResultC { - let mut state = lock_runtime_or_return!(); - if let Err(error) = require_resource(&state, token, id, ResourceKind::Settings) { - return PluginResultC::err(error); - } +#[c_result(PluginResultC)] +unsafe extern "C" fn settings_release(token: PluginToken, id: ResourceId) -> HostResult { + let mut state = lock_runtime()?; + require_resource(&state, token, id, ResourceKind::Settings)?; if state .settings .get(&id) .is_some_and(|settings| settings.in_flight != 0) { - return PluginResultC::err("settings callback is in progress"); + return Err("settings callback is in progress"); } state.resources.remove(&id); state.settings.remove(&id); @@ -1739,7 +1655,7 @@ unsafe extern "C" fn settings_release(token: PluginToken, id: ResourceId) -> Plu .retain(|_, resource_id| *resource_id != id); state.settings_dirty = true; release_runtime(state); - PluginResultC::ok() + Ok(()) } pub fn update_host_state(state: HostState) { From c4d9ede893a8146a14ac648a63573c663f4ab350 Mon Sep 17 00:00:00 2001 From: Eatgrapes Date: Sat, 26 Sep 2026 19:29:27 +0800 Subject: [PATCH 03/13] refactor(windows): use windows-rs handles, strings and registry The platform crate built NUL-terminated UTF-16 buffers by hand for every string argument, closed handles manually on each exit path, and drove the Run key through raw Reg* calls. The app user model ID lookup was also written twice, once for media activation and once for audio capture. windows-rs already ships these pieces: HSTRING and the w!/h! literals for string parameters, Owned for handles, and the windows-registry crate, which was already in the lock file. Use them and move the shared process lookup into a process module. As a side effect, reveal_path now passes the path without a lossy UTF-8 round trip. --- Cargo.lock | 1 + crates/winisland-platform-windows/Cargo.toml | 1 + .../src/audio/capture.rs | 41 ++--------- .../winisland-platform-windows/src/display.rs | 5 +- crates/winisland-platform-windows/src/lib.rs | 1 + .../winisland-platform-windows/src/metrics.rs | 5 +- .../src/notify/toast.rs | 16 ++--- .../winisland-platform-windows/src/process.rs | 32 +++++++++ .../src/shell/activate.rs | 52 +++----------- .../src/shell/autostart.rs | 70 +++++-------------- .../src/shell/instance.rs | 25 +++---- .../src/shell/links.rs | 22 ++---- .../src/shell/mod.rs | 6 +- 13 files changed, 94 insertions(+), 183 deletions(-) create mode 100644 crates/winisland-platform-windows/src/process.rs diff --git a/Cargo.lock b/Cargo.lock index 302ad759..df4b11db 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5230,6 +5230,7 @@ dependencies = [ "windows", "windows-core", "windows-numerics", + "windows-registry", "winisland-platform", "winit", ] diff --git a/crates/winisland-platform-windows/Cargo.toml b/crates/winisland-platform-windows/Cargo.toml index c31ae0d8..f307ddbd 100644 --- a/crates/winisland-platform-windows/Cargo.toml +++ b/crates/winisland-platform-windows/Cargo.toml @@ -6,6 +6,7 @@ edition = "2024" [dependencies] winisland-platform = { path = "../winisland-platform" } windows-core = "0.62.2" +windows-registry = "0.6.1" windows = { version = "0.62.2", features = ["Win32_Foundation", "Win32_System_Com", "Win32_System_Ole", "Win32_System_Wmi", "Win32_System_Variant", "Win32_Globalization", "Win32_System_Registry", "Win32_System_Threading", "Win32_System_SystemInformation", "Win32_System_LibraryLoader", "Win32_System_EventLog", "Win32_System_WinRT", "Win32_System_WinRT_Composition", "Win32_Media_Audio", "Win32_Media_Audio_Endpoints", "Win32_UI_WindowsAndMessaging", "Win32_UI_Shell", "Win32_UI_Input_KeyboardAndMouse", "Win32_Storage_Packaging_Appx", "Win32_Graphics_Dxgi", "Win32_Graphics_Dxgi_Common", "Win32_Graphics_Gdi", "Win32_Graphics_Dwm", "Win32_NetworkManagement_IpHelper", "Win32_NetworkManagement_Ndis", "Win32_Storage_FileSystem", "ApplicationModel", "Foundation", "Foundation_Collections", "Storage_Streams", "UI_Notifications", "UI_Notifications_Management", "UI_Composition", "UI_Composition_Desktop", "Media_Control", "Graphics_Imaging", "Data_Xml_Dom", "System"] } dirs = "6.0" log = { version = "0.4", features = ["std"] } diff --git a/crates/winisland-platform-windows/src/audio/capture.rs b/crates/winisland-platform-windows/src/audio/capture.rs index 74f7f3e7..c9d3768e 100644 --- a/crates/winisland-platform-windows/src/audio/capture.rs +++ b/crates/winisland-platform-windows/src/audio/capture.rs @@ -1,18 +1,17 @@ use wasapi::{ AudioCaptureClient, AudioClient, Direction, Handle, SampleType, StreamMode, WaveFormat, }; -use windows::Win32::Foundation::{CloseHandle, ERROR_INSUFFICIENT_BUFFER, S_OK}; +use windows::Win32::Foundation::S_OK; use windows::Win32::Media::Audio::{ Endpoints::IAudioMeterInformation, IAudioSessionControl2, IAudioSessionManager2, IMMDeviceEnumerator, MMDeviceEnumerator, eConsole, eRender, }; -use windows::Win32::Storage::Packaging::Appx::GetApplicationUserModelId; use windows::Win32::System::Com::{CLSCTX_ALL, CoCreateInstance}; -use windows::Win32::System::Threading::{OpenProcess, PROCESS_QUERY_LIMITED_INFORMATION}; -use windows::core::{Interface, PWSTR}; +use windows::core::Interface; use winisland_platform::{AudioMeter, PlatformError, ProcessCapture}; use crate::com::ComGuard; +use crate::process; const LOOPBACK_SAMPLE_RATE: usize = 48_000; const LOOPBACK_CHANNELS: usize = 2; @@ -74,7 +73,8 @@ impl AudioMeter for WindowsAudioMeter { continue; }; if process_id != 0 - && process_app_user_model_id(process_id) + && process::open(process_id) + .and_then(|process| process::app_user_model_id(*process)) .is_some_and(|id| id.eq_ignore_ascii_case(app_id)) { return Some(process_id); @@ -116,37 +116,6 @@ impl AudioMeter for WindowsAudioMeter { } } -fn process_app_user_model_id(process_id: u32) -> Option { - // SAFETY: The PID comes from an audio session and the requested access only reads identity. - let process = - unsafe { OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, false, process_id).ok()? }; - let mut length = 0; - // SAFETY: A null output buffer requests the required UTF-16 length for this valid handle. - let first = unsafe { GetApplicationUserModelId(process, &mut length, None) }; - if first != ERROR_INSUFFICIENT_BUFFER || length == 0 { - // SAFETY: This branch owns the process handle and closes it exactly once. - unsafe { - let _ = CloseHandle(process); - } - return None; - } - let mut app_id = vec![0u16; length as usize]; - // SAFETY: The buffer has the length returned by Windows and stays allocated for this call. - let result = unsafe { - GetApplicationUserModelId(process, &mut length, Some(PWSTR(app_id.as_mut_ptr()))) - }; - // SAFETY: The process handle is no longer used after this point. - unsafe { - let _ = CloseHandle(process); - } - if result.0 != 0 { - return None; - } - String::from_utf16(&app_id) - .ok() - .map(|id| id.trim_end_matches('\0').to_string()) -} - pub(super) struct WindowsProcessCapture { client: AudioClient, capture: AudioCaptureClient, diff --git a/crates/winisland-platform-windows/src/display.rs b/crates/winisland-platform-windows/src/display.rs index 6e6846b6..f7b1712f 100644 --- a/crates/winisland-platform-windows/src/display.rs +++ b/crates/winisland-platform-windows/src/display.rs @@ -14,7 +14,7 @@ use windows::Win32::UI::WindowsAndMessaging::{ GetForegroundWindow, GetWindowRect, GetWindowThreadProcessId, IsIconic, SW_RESTORE, SetForegroundWindow, ShowWindow, }; -use windows::core::{BOOL, PCWSTR}; +use windows::core::{BOOL, HSTRING}; use winisland_platform::{ DisplayProvider, GpuProfile, MonitorId, MonitorInfo, PlatformError, Point, Rect, }; @@ -184,10 +184,9 @@ fn foreground_fullscreen(target: Rect) -> bool { } fn find_window(title: &str) -> Option { - let wide: Vec = title.encode_utf16().chain(Some(0)).collect(); // SAFETY: The title is NUL-terminated and remains live for the call. unsafe { - FindWindowW(None, PCWSTR(wide.as_ptr())) + FindWindowW(None, &HSTRING::from(title)) .ok() .filter(|hwnd| !hwnd.is_invalid()) } diff --git a/crates/winisland-platform-windows/src/lib.rs b/crates/winisland-platform-windows/src/lib.rs index 2053171c..2f835cc5 100644 --- a/crates/winisland-platform-windows/src/lib.rs +++ b/crates/winisland-platform-windows/src/lib.rs @@ -10,6 +10,7 @@ mod input; mod media; mod metrics; mod notify; +mod process; mod shell; pub mod window; diff --git a/crates/winisland-platform-windows/src/metrics.rs b/crates/winisland-platform-windows/src/metrics.rs index 52e72496..c19a0587 100644 --- a/crates/winisland-platform-windows/src/metrics.rs +++ b/crates/winisland-platform-windows/src/metrics.rs @@ -13,7 +13,7 @@ use windows::Win32::System::SystemInformation::{GlobalMemoryStatusEx, MEMORYSTAT use windows::Win32::System::Threading::{ GetCurrentProcess, GetSystemTimes, SetProcessWorkingSetSize, }; -use windows::core::{Interface, PCWSTR}; +use windows::core::{HSTRING, Interface}; use winisland_platform::{MetricSelection, PlatformError, SystemMetrics, SystemSample}; const IF_TYPE_SOFTWARE_LOOPBACK: u32 = 24; @@ -123,13 +123,12 @@ fn network() -> Option<(u64, u64)> { fn disk() -> Option<(u64, u64)> { let root = std::env::var("SystemDrive").unwrap_or_else(|_| "C:".to_string()) + "\\"; - let wide: Vec = root.encode_utf16().chain(Some(0)).collect(); let mut total = 0u64; let mut free = 0u64; // SAFETY: The root is NUL-terminated and total and free are writable outputs. unsafe { GetDiskFreeSpaceExW( - PCWSTR(wide.as_ptr()), + &HSTRING::from(root), None, Some(&mut total), Some(&mut free), diff --git a/crates/winisland-platform-windows/src/notify/toast.rs b/crates/winisland-platform-windows/src/notify/toast.rs index fc946801..2d864935 100644 --- a/crates/winisland-platform-windows/src/notify/toast.rs +++ b/crates/winisland-platform-windows/src/notify/toast.rs @@ -1,19 +1,15 @@ use windows::ApplicationModel::Package; use windows::UI::Notifications::{ToastNotification, ToastNotificationManager, ToastTemplateType}; use windows::Win32::UI::Shell::SetCurrentProcessExplicitAppUserModelID; -use windows::core::{HSTRING, PCWSTR}; +use windows::core::{HSTRING, h, w}; pub(super) fn set_app_identity() { if Package::Current().is_ok() { return; } - let wide: Vec = "WinIsland.PluginManager" - .encode_utf16() - .chain(Some(0)) - .collect(); - // SAFETY: The UTF-16 buffer is NUL-terminated and remains live for this synchronous call. + // SAFETY: The app ID is a static NUL-terminated literal. unsafe { - let _ = SetCurrentProcessExplicitAppUserModelID(PCWSTR::from_raw(wide.as_ptr())); + let _ = SetCurrentProcessExplicitAppUserModelID(w!("WinIsland.PluginManager")); } } @@ -26,7 +22,7 @@ pub(super) fn show(title: &str, message: &str) { return; } }; - if let Ok(nodes) = tmpl.SelectNodes(&HSTRING::from("//text")) { + if let Ok(nodes) = tmpl.SelectNodes(h!("//text")) { if let Ok(node) = nodes.Item(0) { let _ = node.SetInnerText(&HSTRING::from(title)); } @@ -44,9 +40,7 @@ pub(super) fn show(title: &str, message: &str) { let notifier_result = if Package::Current().is_ok() { ToastNotificationManager::CreateToastNotifier() } else { - ToastNotificationManager::CreateToastNotifierWithId(&HSTRING::from( - "WinIsland.PluginManager", - )) + ToastNotificationManager::CreateToastNotifierWithId(h!("WinIsland.PluginManager")) }; let notifier = match notifier_result { Ok(notifier) => notifier, diff --git a/crates/winisland-platform-windows/src/process.rs b/crates/winisland-platform-windows/src/process.rs new file mode 100644 index 00000000..192de4b8 --- /dev/null +++ b/crates/winisland-platform-windows/src/process.rs @@ -0,0 +1,32 @@ +use windows::Win32::Foundation::{ERROR_INSUFFICIENT_BUFFER, ERROR_SUCCESS, HANDLE}; +use windows::Win32::Storage::Packaging::Appx::GetApplicationUserModelId; +use windows::Win32::System::Threading::{OpenProcess, PROCESS_QUERY_LIMITED_INFORMATION}; +use windows::core::{Owned, PWSTR}; + +pub(crate) fn open(process_id: u32) -> Option> { + // SAFETY: The requested access only queries identity, and the new handle is owned by the guard. + unsafe { + OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, false, process_id) + .ok() + .map(|process| Owned::new(process)) + } +} + +pub(crate) fn app_user_model_id(process: HANDLE) -> Option { + let mut length = 0; + // SAFETY: The first call only queries the required buffer length. + if unsafe { GetApplicationUserModelId(process, &mut length, None) } != ERROR_INSUFFICIENT_BUFFER + || length == 0 + { + return None; + } + let mut buffer = vec![0u16; length as usize]; + // SAFETY: The buffer has the length reported by the first call. + if unsafe { GetApplicationUserModelId(process, &mut length, Some(PWSTR(buffer.as_mut_ptr()))) } + != ERROR_SUCCESS + { + return None; + } + buffer.truncate(length.saturating_sub(1) as usize); + String::from_utf16(&buffer).ok() +} diff --git a/crates/winisland-platform-windows/src/shell/activate.rs b/crates/winisland-platform-windows/src/shell/activate.rs index c50837db..5880e3af 100644 --- a/crates/winisland-platform-windows/src/shell/activate.rs +++ b/crates/winisland-platform-windows/src/shell/activate.rs @@ -1,13 +1,8 @@ use std::path::Path; -use windows::Win32::Foundation::{ - CloseHandle, ERROR_INSUFFICIENT_BUFFER, ERROR_SUCCESS, HANDLE, HWND, LPARAM, -}; -use windows::Win32::Storage::Packaging::Appx::GetApplicationUserModelId; +use windows::Win32::Foundation::{HANDLE, HWND, LPARAM}; use windows::Win32::System::Com::{CLSCTX_LOCAL_SERVER, CoCreateInstance}; -use windows::Win32::System::Threading::{ - OpenProcess, PROCESS_NAME_WIN32, PROCESS_QUERY_LIMITED_INFORMATION, QueryFullProcessImageNameW, -}; +use windows::Win32::System::Threading::{PROCESS_NAME_WIN32, QueryFullProcessImageNameW}; use windows::Win32::UI::Shell::{ ACTIVATEOPTIONS, ApplicationActivationManager, IApplicationActivationManager, }; @@ -15,10 +10,11 @@ use windows::Win32::UI::WindowsAndMessaging::{ EnumWindows, GW_OWNER, GetWindow, GetWindowThreadProcessId, IsWindowVisible, SW_RESTORE, SetForegroundWindow, ShowWindow, }; -use windows::core::{BOOL, PCWSTR, PWSTR}; +use windows::core::{BOOL, HSTRING, PCWSTR, PWSTR}; use winisland_platform::PlatformError; use crate::com::ComGuard; +use crate::process; pub(super) fn application(app_user_model_id: &str) -> Result { if app_user_model_id.is_empty() { @@ -32,17 +28,13 @@ pub(super) fn application(app_user_model_id: &str) -> Result Result { let _com = ComGuard::sta()?; - let app_id: Vec = app_user_model_id.encode_utf16().chain(Some(0)).collect(); - // SAFETY: The COM apartment is initialized here; both UTF-16 buffers live through the call. + let app_id = HSTRING::from(app_user_model_id); + // SAFETY: The COM apartment is initialized here, and the app ID outlives the call. let result = unsafe { let manager: IApplicationActivationManager = CoCreateInstance(&ApplicationActivationManager, None, CLSCTX_LOCAL_SERVER) .map_err(|error| PlatformError::Backend(error.to_string()))?; - manager.ActivateApplication( - PCWSTR(app_id.as_ptr()), - PCWSTR::null(), - ACTIVATEOPTIONS::default(), - ) + manager.ActivateApplication(&app_id, PCWSTR::null(), ACTIVATEOPTIONS::default()) }; match result { Ok(process_id) => { @@ -108,18 +100,13 @@ unsafe extern "system" fn find_media_window(hwnd: HWND, lparam: LPARAM) -> BOOL if process_id == 0 || process_id == std::process::id() { return true.into(); } - // SAFETY: OpenProcess only queries a process handle by the ID returned above. - let Ok(process) = - (unsafe { OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, false, process_id) }) - else { + let Some(process) = process::open(process_id) else { return true.into(); }; - let matches = process_application_user_model_id(process) + let matches = process::app_user_model_id(*process) .is_some_and(|id| id.eq_ignore_ascii_case(&search.source_app_id)) - || process_executable_name(process) + || process_executable_name(*process) .is_some_and(|name| name.eq_ignore_ascii_case(&search.executable_name)); - // SAFETY: process is the one owned handle returned by OpenProcess. - let _ = unsafe { CloseHandle(process) }; if matches { search.window = Some(hwnd); false.into() @@ -128,25 +115,6 @@ unsafe extern "system" fn find_media_window(hwnd: HWND, lparam: LPARAM) -> BOOL } } -fn process_application_user_model_id(process: HANDLE) -> Option { - let mut length = 0; - // SAFETY: The first call only queries the required buffer length. - if unsafe { GetApplicationUserModelId(process, &mut length, None) } != ERROR_INSUFFICIENT_BUFFER - || length == 0 - { - return None; - } - let mut buffer = vec![0u16; length as usize]; - // SAFETY: The buffer has the length reported by the first call. - if unsafe { GetApplicationUserModelId(process, &mut length, Some(PWSTR(buffer.as_mut_ptr()))) } - != ERROR_SUCCESS - { - return None; - } - buffer.truncate(length.saturating_sub(1) as usize); - String::from_utf16(&buffer).ok() -} - fn process_executable_name(process: HANDLE) -> Option { let mut buffer = vec![0u16; 32_768]; let mut length = buffer.len() as u32; diff --git a/crates/winisland-platform-windows/src/shell/autostart.rs b/crates/winisland-platform-windows/src/shell/autostart.rs index 1b94ace7..b206b619 100644 --- a/crates/winisland-platform-windows/src/shell/autostart.rs +++ b/crates/winisland-platform-windows/src/shell/autostart.rs @@ -1,65 +1,31 @@ use std::env; use windows::Win32::Foundation::ERROR_FILE_NOT_FOUND; -use windows::Win32::System::Registry::{ - HKEY, HKEY_CURRENT_USER, KEY_READ, KEY_WRITE, REG_OPTION_NON_VOLATILE, REG_SZ, RegCloseKey, - RegCreateKeyExW, RegDeleteValueW, RegOpenKeyExW, RegQueryValueExW, RegSetValueExW, -}; -use windows::core::w; +use windows_registry::CURRENT_USER; use winisland_platform::PlatformError; +const RUN_KEY: &str = "Software\\Microsoft\\Windows\\CurrentVersion\\Run"; +const VALUE_NAME: &str = "WinIsland"; + pub(super) fn enabled() -> Result { - let mut key = HKEY::default(); - // SAFETY: Static NUL-terminated registry names and a valid output handle are supplied. - unsafe { - RegOpenKeyExW( - HKEY_CURRENT_USER, - w!("Software\\Microsoft\\Windows\\CurrentVersion\\Run"), - Some(0), - KEY_READ, - &mut key, - ) - .ok() + let key = CURRENT_USER + .open(RUN_KEY) .map_err(|error| PlatformError::Backend(error.to_string()))?; - let result = RegQueryValueExW(key, w!("WinIsland"), None, None, None, None).is_ok(); - let _ = RegCloseKey(key); - Ok(result) - } + Ok(key.get_type(VALUE_NAME).is_ok()) } pub(super) fn set(enabled: bool) -> Result<(), PlatformError> { - let mut key = HKEY::default(); - // SAFETY: Static NUL-terminated registry names and an initialized output handle are supplied. - unsafe { - RegCreateKeyExW( - HKEY_CURRENT_USER, - w!("Software\\Microsoft\\Windows\\CurrentVersion\\Run"), - Some(0), - None, - REG_OPTION_NON_VOLATILE, - KEY_WRITE, - None, - &mut key, - None, - ) - .ok() + let key = CURRENT_USER + .create(RUN_KEY) .map_err(|error| PlatformError::Backend(error.to_string()))?; - let result = if enabled { - let exe = - env::current_exe().map_err(|error| PlatformError::Backend(error.to_string()))?; - let quoted = format!("\"{}\"", exe.display()); - let wide: Vec = quoted.encode_utf16().chain(Some(0)).collect(); - let bytes = std::slice::from_raw_parts(wide.as_ptr().cast::(), wide.len() * 2); - RegSetValueExW(key, w!("WinIsland"), Some(0), REG_SZ, Some(bytes)) - } else { - RegDeleteValueW(key, w!("WinIsland")) - }; - let _ = RegCloseKey(key); - if !enabled && result == ERROR_FILE_NOT_FOUND { - return Ok(()); - } - result - .ok() - .map_err(|error| PlatformError::Backend(error.to_string())) + let result = if enabled { + let exe = env::current_exe().map_err(|error| PlatformError::Backend(error.to_string()))?; + key.set_string(VALUE_NAME, format!("\"{}\"", exe.display())) + } else { + key.remove_value(VALUE_NAME) + }; + match result { + Err(error) if !enabled && error.code() == ERROR_FILE_NOT_FOUND.to_hresult() => Ok(()), + result => result.map_err(|error| PlatformError::Backend(error.to_string())), } } diff --git a/crates/winisland-platform-windows/src/shell/instance.rs b/crates/winisland-platform-windows/src/shell/instance.rs index 23529c7e..5d785502 100644 --- a/crates/winisland-platform-windows/src/shell/instance.rs +++ b/crates/winisland-platform-windows/src/shell/instance.rs @@ -1,30 +1,23 @@ -use windows::Win32::Foundation::{CloseHandle, ERROR_ALREADY_EXISTS, GetLastError, HANDLE}; +use windows::Win32::Foundation::{ERROR_ALREADY_EXISTS, GetLastError, HANDLE}; use windows::Win32::System::Threading::CreateMutexW; -use windows::core::PCWSTR; +use windows::core::{HSTRING, Owned}; use winisland_platform::{InstanceLock, PlatformError}; -struct WindowsInstanceLock(HANDLE); +struct WindowsInstanceLock { + _mutex: Owned, +} impl InstanceLock for WindowsInstanceLock {} -impl Drop for WindowsInstanceLock { - fn drop(&mut self) { - // SAFETY: This is the one owned handle returned by CreateMutexW. - let _ = unsafe { CloseHandle(self.0) }; - } -} - pub(super) fn acquire(key: &str) -> Result>, PlatformError> { - let key: Vec = key.encode_utf16().chain(Some(0)).collect(); - // SAFETY: The key is NUL-terminated and lives for the entire call. - let handle = unsafe { CreateMutexW(None, true, PCWSTR(key.as_ptr())) } + let key = HSTRING::from(key); + // SAFETY: The key outlives the call, and the new mutex handle is owned by the guard. + let mutex = unsafe { CreateMutexW(None, true, &key).map(|mutex| Owned::new(mutex)) } .map_err(|error| PlatformError::Backend(error.to_string()))?; // SAFETY: GetLastError is read on the same thread immediately after CreateMutexW. if unsafe { GetLastError() } == ERROR_ALREADY_EXISTS { - // SAFETY: This handle is owned but not retained when the mutex already exists. - let _ = unsafe { CloseHandle(handle) }; Ok(None) } else { - Ok(Some(Box::new(WindowsInstanceLock(handle)))) + Ok(Some(Box::new(WindowsInstanceLock { _mutex: mutex }))) } } diff --git a/crates/winisland-platform-windows/src/shell/links.rs b/crates/winisland-platform-windows/src/shell/links.rs index a140255e..2d714779 100644 --- a/crates/winisland-platform-windows/src/shell/links.rs +++ b/crates/winisland-platform-windows/src/shell/links.rs @@ -2,22 +2,12 @@ use std::path::Path; use windows::Win32::UI::Shell::ShellExecuteW; use windows::Win32::UI::WindowsAndMessaging::SW_SHOWNORMAL; -use windows::core::PCWSTR; +use windows::core::HSTRING; use winisland_platform::PlatformError; -fn open(target: &str) -> Result<(), PlatformError> { - let target: Vec = target.encode_utf16().chain(Some(0)).collect(); - // SAFETY: The UTF-16 target is NUL-terminated and remains live for the synchronous call. - let result = unsafe { - ShellExecuteW( - None, - None, - PCWSTR(target.as_ptr()), - None, - None, - SW_SHOWNORMAL, - ) - }; +fn open(target: &HSTRING) -> Result<(), PlatformError> { + // SAFETY: The target is NUL-terminated and remains live for the synchronous call. + let result = unsafe { ShellExecuteW(None, None, target, None, None, SW_SHOWNORMAL) }; if result.0 as isize > 32 { Ok(()) } else { @@ -29,9 +19,9 @@ fn open(target: &str) -> Result<(), PlatformError> { } pub(super) fn open_url(url: &str) -> Result<(), PlatformError> { - open(url) + open(&url.into()) } pub(super) fn reveal_path(path: &Path) -> Result<(), PlatformError> { - open(&path.to_string_lossy()) + open(&path.into()) } diff --git a/crates/winisland-platform-windows/src/shell/mod.rs b/crates/winisland-platform-windows/src/shell/mod.rs index 1e3de7d4..b36880cd 100644 --- a/crates/winisland-platform-windows/src/shell/mod.rs +++ b/crates/winisland-platform-windows/src/shell/mod.rs @@ -15,7 +15,7 @@ use windows::Win32::UI::WindowsAndMessaging::{ IDOK, IDYES, MB_ICONERROR, MB_ICONINFORMATION, MB_OKCANCEL, MB_SETFOREGROUND, MB_TOPMOST, MESSAGEBOX_STYLE, MessageBoxW, }; -use windows::core::PCWSTR; +use windows::core::HSTRING; use winisland_platform::{ InstanceLock, LocalDateTime, PlatformError, ShellIntegration, TrayAction, TrayLabels, TrayTheme, }; @@ -118,8 +118,6 @@ fn show_dialog( body: &str, style: MESSAGEBOX_STYLE, ) -> windows::Win32::UI::WindowsAndMessaging::MESSAGEBOX_RESULT { - let title: Vec = title.encode_utf16().chain(Some(0)).collect(); - let body: Vec = body.encode_utf16().chain(Some(0)).collect(); // SAFETY: Both strings remain live and NUL-terminated for the synchronous call. - unsafe { MessageBoxW(None, PCWSTR(body.as_ptr()), PCWSTR(title.as_ptr()), style) } + unsafe { MessageBoxW(None, &HSTRING::from(body), &HSTRING::from(title), style) } } From 7f287ba5200381e0e6da34a5559d2c7a553daea3 Mon Sep 17 00:00:00 2001 From: Eatgrapes Date: Sat, 26 Sep 2026 19:58:34 +0800 Subject: [PATCH 04/13] refactor: use parking_lot locks instead of poison recovery Every one of these std locks was unwrapped with PoisonError::into_inner, so poisoning was already ignored. parking_lot locks never poison, which removes that boilerplate and lets try_lock return an Option. Locks that do react to poisoning (plugin manager, logger, i18n) are left unchanged. --- Cargo.lock | 49 +++++++++++++++++++ Cargo.toml | 1 + crates/winisland-platform-windows/Cargo.toml | 1 + .../src/display/brightness.rs | 12 ++--- .../winisland-platform-windows/src/input.rs | 11 ++--- .../src/notify/mod.rs | 15 ++---- src/core/audio.rs | 40 +++++---------- src/core/smtc/properties.rs | 19 ++----- src/platform.rs | 12 ++--- src/ui/compact/volume.rs | 26 +++------- 10 files changed, 87 insertions(+), 99 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index df4b11db..e7116fd2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -17,6 +17,7 @@ dependencies = [ "libloading 0.9.0", "log", "lrc", + "parking_lot", "pollkit", "pulldown-cmark", "quick-xml", @@ -2262,6 +2263,15 @@ version = "0.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "47d9d19d1d6efa0109d2f65ff4c85cddd50bd572e5a00127ab10987290bcefae" +[[package]] +name = "lock_api" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "224399e74b87b5f3557511d98dff8b14089b3dadafcab6bb93eab67d3aace965" +dependencies = [ + "scopeguard", +] + [[package]] name = "log" version = "0.4.33" @@ -2937,6 +2947,29 @@ dependencies = [ "system-deps", ] +[[package]] +name = "parking_lot" +version = "0.12.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93857453250e3077bd71ff98b6a65ea6621a19bb0f559a85248955ac12c45a1a" +dependencies = [ + "lock_api", + "parking_lot_core", +] + +[[package]] +name = "parking_lot_core" +version = "0.9.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2621685985a2ebf1c516881c026032ac7deafcda1a2c9b7850dc81e3dfcb64c1" +dependencies = [ + "cfg-if", + "libc", + "redox_syscall 0.5.18", + "smallvec", + "windows-link", +] + [[package]] name = "paste" version = "1.0.15" @@ -3507,6 +3540,15 @@ dependencies = [ "bitflags 1.3.2", ] +[[package]] +name = "redox_syscall" +version = "0.5.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed2bf2547551a7053d6fdfafda3f938979645c44812fbfcda098faae3f1a362d" +dependencies = [ + "bitflags 2.13.1", +] + [[package]] name = "redox_syscall" version = "0.9.2" @@ -3809,6 +3851,12 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e1cf6437eb19a8f4a6cc0f7dca544973b0b78843adbfeb3683d1a94a0024a294" +[[package]] +name = "scopeguard" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" + [[package]] name = "sctk-adwaita" version = "0.10.1" @@ -5224,6 +5272,7 @@ dependencies = [ "image", "libloading 0.9.0", "log", + "parking_lot", "tokio", "tray-icon", "wasapi", diff --git a/Cargo.toml b/Cargo.toml index a3f1729d..d375ae76 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,6 +41,7 @@ rfd = "0.17.2" libloading = "0.9" log = { version = "0.4", features = ["std"] } lrc = "0.2.0" +parking_lot = "0.12" winisland-plugin-api = { path = "crates/winisland-plugin-api" } winisland-core = { path = "crates/winisland-core" } winisland-render = { path = "crates/winisland-render" } diff --git a/crates/winisland-platform-windows/Cargo.toml b/crates/winisland-platform-windows/Cargo.toml index f307ddbd..049d1d42 100644 --- a/crates/winisland-platform-windows/Cargo.toml +++ b/crates/winisland-platform-windows/Cargo.toml @@ -10,6 +10,7 @@ windows-registry = "0.6.1" windows = { version = "0.62.2", features = ["Win32_Foundation", "Win32_System_Com", "Win32_System_Ole", "Win32_System_Wmi", "Win32_System_Variant", "Win32_Globalization", "Win32_System_Registry", "Win32_System_Threading", "Win32_System_SystemInformation", "Win32_System_LibraryLoader", "Win32_System_EventLog", "Win32_System_WinRT", "Win32_System_WinRT_Composition", "Win32_Media_Audio", "Win32_Media_Audio_Endpoints", "Win32_UI_WindowsAndMessaging", "Win32_UI_Shell", "Win32_UI_Input_KeyboardAndMouse", "Win32_Storage_Packaging_Appx", "Win32_Graphics_Dxgi", "Win32_Graphics_Dxgi_Common", "Win32_Graphics_Gdi", "Win32_Graphics_Dwm", "Win32_NetworkManagement_IpHelper", "Win32_NetworkManagement_Ndis", "Win32_Storage_FileSystem", "ApplicationModel", "Foundation", "Foundation_Collections", "Storage_Streams", "UI_Notifications", "UI_Notifications_Management", "UI_Composition", "UI_Composition_Desktop", "Media_Control", "Graphics_Imaging", "Data_Xml_Dom", "System"] } dirs = "6.0" log = { version = "0.4", features = ["std"] } +parking_lot = "0.12" tray-icon = "0.24.1" image = "0.25" wasapi = "0.24.0" diff --git a/crates/winisland-platform-windows/src/display/brightness.rs b/crates/winisland-platform-windows/src/display/brightness.rs index 2612eb94..25bd5360 100644 --- a/crates/winisland-platform-windows/src/display/brightness.rs +++ b/crates/winisland-platform-windows/src/display/brightness.rs @@ -1,7 +1,8 @@ +use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::mpsc::{self, SyncSender}; -use std::sync::{Arc, Mutex}; +use parking_lot::Mutex; use windows::Win32::System::Com::{CLSCTX_INPROC_SERVER, CoCreateInstance}; use windows::Win32::System::Variant::{VARIANT, VT_I4, VT_UI1, VT_UI4}; use windows::Win32::System::Wmi::{ISWbemLocator, ISWbemObject, ISWbemServices, SWbemLocator}; @@ -44,10 +45,7 @@ impl WindowsBrightnessFeed { impl BrightnessFeed for WindowsBrightnessFeed { fn snapshot(&self) -> BrightnessSnapshot { - *self - .snapshot - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) + *self.snapshot.lock() } fn set_level(&self, level: f32) { @@ -205,9 +203,7 @@ unsafe fn set_brightness(object: &ISWbemObject, level: u8) -> windows::core::Res } fn publish(snapshot: &Mutex, level: f32, notify: bool) { - let mut current = snapshot - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); + let mut current = snapshot.lock(); if (current.level - level).abs() > 0.001 && notify { current.revision = current.revision.wrapping_add(1); } diff --git a/crates/winisland-platform-windows/src/input.rs b/crates/winisland-platform-windows/src/input.rs index 8c51c687..cee62e47 100644 --- a/crates/winisland-platform-windows/src/input.rs +++ b/crates/winisland-platform-windows/src/input.rs @@ -1,7 +1,7 @@ use std::cell::RefCell; -use std::sync::Mutex; use std::time::Duration; +use parking_lot::Mutex; use windows::Win32::Foundation::{HINSTANCE, LPARAM, LRESULT, WPARAM}; use windows::Win32::System::LibraryLoader::GetModuleHandleW; use windows::Win32::UI::Input::KeyboardAndMouse::{ @@ -56,17 +56,13 @@ impl InputHooks for WindowsInput { *slot.borrow_mut() = Some(HookGuard(hook)); log::info!("Volume keys are handled by WinIsland"); } - *VOLUME_CALLBACK - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) = Some(callback); + *VOLUME_CALLBACK.lock() = Some(callback); Ok(()) }) } fn uninstall_volume_keys(&self) -> Result<(), PlatformError> { - *VOLUME_CALLBACK - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) = None; + *VOLUME_CALLBACK.lock() = None; VOLUME_HOOK.with(|slot| { if slot.borrow_mut().take().is_some() { log::info!("Native volume flyout replacement disabled"); @@ -115,7 +111,6 @@ unsafe extern "system" fn volume_keyboard_hook( }; let handled = VOLUME_CALLBACK .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) .as_ref() .is_some_and(|callback| callback(event)); if handled { diff --git a/crates/winisland-platform-windows/src/notify/mod.rs b/crates/winisland-platform-windows/src/notify/mod.rs index 5af01536..10df770d 100644 --- a/crates/winisland-platform-windows/src/notify/mod.rs +++ b/crates/winisland-platform-windows/src/notify/mod.rs @@ -8,10 +8,11 @@ use winisland_platform::{ }; use std::collections::HashSet; +use std::sync::Arc; use std::sync::mpsc::{self, Receiver}; -use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; +use parking_lot::Mutex; use windows::ApplicationModel::AppDisplayInfo; use windows::Foundation::Size; use windows::UI::Notifications::Management::{ @@ -40,19 +41,11 @@ struct CancelSlot(Mutex>>); impl CancelSlot { fn set(&self, cancel: impl Fn() + Send + Sync + 'static) { - *self - .0 - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) = Some(Box::new(cancel)); + *self.0.lock() = Some(Box::new(cancel)); } fn cancel(&self) { - if let Some(cancel) = self - .0 - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .take() - { + if let Some(cancel) = self.0.lock().take() { cancel(); } } diff --git a/src/core/audio.rs b/src/core/audio.rs index e29e33f1..515f76eb 100644 --- a/src/core/audio.rs +++ b/src/core/audio.rs @@ -1,9 +1,10 @@ use cpal::traits::{DeviceTrait, HostTrait, StreamTrait}; use cpal::{FromSample, Sample, SampleFormat, Stream, StreamConfig}; +use parking_lot::{Mutex, RwLock}; use pollkit::Cooldown; use realfft::RealFftPlanner; +use std::sync::Arc; use std::sync::atomic::{AtomicBool, AtomicU32, Ordering}; -use std::sync::{Arc, Mutex, RwLock}; use std::time::{Duration, Instant}; use tokio_util::sync::CancellationToken; @@ -88,7 +89,7 @@ impl SpectrumAnalyzer { gate_override: &AtomicF32, ) { if !analysis_enabled(gate, gate_override) { - if let Ok(mut spectrum) = spectrum.try_lock() { + if let Some(mut spectrum) = spectrum.try_lock() { *spectrum = [0.0; SPECTRUM_BAND_COUNT]; } return; @@ -119,7 +120,7 @@ impl SpectrumAnalyzer { for (output_band, (input_band, gain)) in SPECTRUM_OUTPUT_MAPPING.iter().enumerate() { final_bins[output_band] = raw_bins[*input_band] * gain; } - if let Ok(mut spectrum) = spectrum.try_lock() { + if let Some(mut spectrum) = spectrum.try_lock() { *spectrum = final_bins; } } @@ -215,10 +216,7 @@ impl AudioProcessor { self.stop_workers(); return [0.0; SPECTRUM_BAND_COUNT]; } - *self - .spectrum - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) + *self.spectrum.lock() } pub fn set_gate_override(&self, value: bool) { @@ -227,10 +225,7 @@ impl AudioProcessor { pub fn set_target_app_id(&self, app_id: &str) { let changed = { - let mut target_app_id = self - .target_app_id - .write() - .unwrap_or_else(std::sync::PoisonError::into_inner); + let mut target_app_id = self.target_app_id.write(); if *target_app_id == app_id { false } else { @@ -254,10 +249,7 @@ impl AudioProcessor { return; } let cancel = { - let mut workers = self - .workers - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); + let mut workers = self.workers.lock(); if workers.is_some() { return; } @@ -276,21 +268,14 @@ impl AudioProcessor { } fn stop_workers(&self) { - let cancel = self - .workers - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .take(); + let cancel = self.workers.lock().take(); if let Some(cancel) = cancel { self.worker_generation.fetch_add(1, Ordering::AcqRel); cancel.cancel(); self.target_process_id.store(0, Ordering::Relaxed); self.process_capture_active.store(false, Ordering::Release); self.gate.set(0.0); - *self - .spectrum - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) = [0.0; SPECTRUM_BAND_COUNT]; + *self.spectrum.lock() = [0.0; SPECTRUM_BAND_COUNT]; log::info!("Audio media ended, stopping capture workers"); } } @@ -340,10 +325,7 @@ impl AudioProcessor { } } - let requested_app_id = target_app_id - .read() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .clone(); + let requested_app_id = target_app_id.read().clone(); if requested_app_id != current_target_app_id || now >= next_target_refresh { current_target_app_id = requested_app_id; current_target_process_id = meter @@ -696,7 +678,7 @@ fn analysis_enabled(gate: &AtomicF32, gate_override: &AtomicF32) -> bool { fn reset_spectrum(analyzer: &mut SpectrumAnalyzer, spectrum: &Mutex<[f32; SPECTRUM_BAND_COUNT]>) { analyzer.input_len = 0; - if let Ok(mut spectrum) = spectrum.try_lock() { + if let Some(mut spectrum) = spectrum.try_lock() { *spectrum = [0.0; SPECTRUM_BAND_COUNT]; } } diff --git a/src/core/smtc/properties.rs b/src/core/smtc/properties.rs index 4393e5bb..7ef28061 100644 --- a/src/core/smtc/properties.rs +++ b/src/core/smtc/properties.rs @@ -1,8 +1,9 @@ use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; -use std::sync::{Arc, Condvar, Mutex}; +use std::sync::Arc; use std::time::{Duration, Instant}; +use parking_lot::{Condvar, Mutex}; use tokio::sync::watch; use winisland_platform::{MediaSessionHandle, PlatformError, ThumbnailError, TrackInfo}; @@ -60,19 +61,13 @@ impl ThumbnailFetcher { .name("winisland-smtc-thumbnail".to_string()) .spawn(move || { loop { - let mut pending = worker_state - .request - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); + let mut pending = worker_state.request.lock(); while pending.is_none() && !worker_state .closed .load(std::sync::atomic::Ordering::Acquire) { - pending = worker_state - .changed - .wait(pending) - .unwrap_or_else(std::sync::PoisonError::into_inner); + worker_state.changed.wait(&mut pending); } if worker_state .closed @@ -108,11 +103,7 @@ impl ThumbnailFetcher { artist: String, is_song_change: bool, ) { - let mut request = self - .state - .request - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); + let mut request = self.state.request.lock(); *request = Some(ThumbnailFetchRequest { session: Arc::clone(session), source_app_id, diff --git a/src/platform.rs b/src/platform.rs index 284bee04..bf1c6d0c 100644 --- a/src/platform.rs +++ b/src/platform.rs @@ -1,4 +1,4 @@ -use std::sync::Mutex; +use parking_lot::Mutex; use winisland_platform::{ AudioProvider, Capabilities, DisplayProvider, HitRegion, InputHooks, MediaProvider, @@ -20,17 +20,11 @@ static CAPABILITIES: Mutex = Mutex::new(Capabilities { }); pub(crate) fn capabilities() -> Capabilities { - *CAPABILITIES - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) + *CAPABILITIES.lock() } pub(crate) fn update_capabilities(update: impl FnOnce(&mut Capabilities)) { - update( - &mut CAPABILITIES - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner), - ); + update(&mut CAPABILITIES.lock()); } pub(crate) fn shell() -> &'static dyn ShellIntegration { diff --git a/src/ui/compact/volume.rs b/src/ui/compact/volume.rs index cfb3c127..1d6478e8 100644 --- a/src/ui/compact/volume.rs +++ b/src/ui/compact/volume.rs @@ -1,6 +1,7 @@ +use parking_lot::Mutex; +use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::mpsc::{self, Receiver, SyncSender}; -use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; use tokio_util::sync::CancellationToken; use winisland_platform::{VolumeCommand, VolumeKey, VolumeKeyEvent}; @@ -75,11 +76,7 @@ impl VolumeMonitor { } pub(super) fn snapshot(&self) -> VolumeSnapshot { - *self - .state - .snapshot - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) + *self.state.snapshot.lock() } pub(super) fn set_key_handling_enabled(&self, enabled: bool) { @@ -92,11 +89,7 @@ impl VolumeMonitor { pub(super) fn set_level(&self, level: f32) { if self.can_set_level() && level.is_finite() { - *self - .state - .pending_level - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) = Some(level.clamp(0.0, 1.0)); + *self.state.pending_level.lock() = Some(level.clamp(0.0, 1.0)); } } @@ -197,11 +190,7 @@ fn spawn_volume_monitor( } let mut command_handled = false; - let pending_level = state - .pending_level - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .take(); + let pending_level = state.pending_level.lock().take(); for command in command_receiver .try_iter() .chain(pending_level.map(VolumeCommand::SetLevel)) @@ -243,10 +232,7 @@ fn publish_volume_snapshot( (last.level - current.level).abs() > VOLUME_CHANGE_THRESHOLD || last.muted != current.muted }); - let mut snapshot = state - .snapshot - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); + let mut snapshot = state.snapshot.lock(); let revision = if changed { snapshot.revision.wrapping_add(1) } else { From a54ba2f64f705ec598c3842caf0046e872546b5c Mon Sep 17 00:00:00 2001 From: Eatgrapes Date: Sat, 26 Sep 2026 20:35:00 +0800 Subject: [PATCH 05/13] refactor(core): derive config enum strings with strum DockPosition and LyricTransitionMode each spelled out their config strings in several hand-written matches (as_str, FromStr and the String conversion). strum now derives them from one snake_case rule, so a new variant needs no extra match arms. Saved values stay byte-identical and unknown strings still fall back to the default. --- Cargo.lock | 22 ++++++++++ crates/winisland-core/Cargo.toml | 1 + crates/winisland-core/src/config/lyrics.rs | 34 ++-------------- crates/winisland-core/src/config/visual.rs | 47 +++------------------- src/window/settings/pages/music.rs | 2 +- 5 files changed, 33 insertions(+), 73 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e7116fd2..7f0076a3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4176,6 +4176,27 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6637bab7722d379c8b41ba849228d680cc12d0a45ba1fa2b48f2a30577a06731" +[[package]] +name = "strum" +version = "0.28.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9628de9b8791db39ceda2b119bbe13134770b56c138ec1d3af810d045c04f9bd" +dependencies = [ + "strum_macros", +] + +[[package]] +name = "strum_macros" +version = "0.28.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ab85eea0270ee17587ed4156089e10b9e6880ee688791d45a905f5b1ca36f664" +dependencies = [ + "heck 0.5.0", + "proc-macro2", + "quote", + "syn 2.0.119", +] + [[package]] name = "subtle" version = "2.6.1" @@ -5257,6 +5278,7 @@ dependencies = [ "reqwest", "serde", "serde_json", + "strum", "toml 1.1.4+spec-1.1.0", ] diff --git a/crates/winisland-core/Cargo.toml b/crates/winisland-core/Cargo.toml index 0c8536e5..381214b8 100644 --- a/crates/winisland-core/Cargo.toml +++ b/crates/winisland-core/Cargo.toml @@ -14,4 +14,5 @@ quick-xml = "0.41.0" reqwest = { version = "0.13.4", features = ["json"] } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0.149" +strum = { version = "0.28", features = ["derive"] } toml = "1.1.2" diff --git a/crates/winisland-core/src/config/lyrics.rs b/crates/winisland-core/src/config/lyrics.rs index b07abb0b..7782c3bf 100644 --- a/crates/winisland-core/src/config/lyrics.rs +++ b/crates/winisland-core/src/config/lyrics.rs @@ -1,7 +1,10 @@ use serde::{Deserialize, Serialize}; +use strum::{Display, EnumString, IntoStaticStr}; #[derive(Serialize, Deserialize, Clone, Copy, Debug, Default, PartialEq, Eq)] -#[serde(from = "String", into = "String")] +#[serde(from = "String", into = "&'static str")] +#[derive(Display, EnumString, IntoStaticStr)] +#[strum(serialize_all = "snake_case")] pub enum LyricTransitionMode { Random, #[default] @@ -13,15 +16,6 @@ pub enum LyricTransitionMode { impl LyricTransitionMode { pub const ALL: [Self; 4] = [Self::Random, Self::Blur, Self::Slide, Self::Fade]; - pub const fn as_str(self) -> &'static str { - match self { - Self::Random => "random", - Self::Blur => "blur", - Self::Slide => "slide", - Self::Fade => "fade", - } - } - pub const fn animation(self, random_value: u64) -> LyricTransitionAnimation { match self { Self::Random => match random_value % 3 { @@ -36,32 +30,12 @@ impl LyricTransitionMode { } } -impl std::str::FromStr for LyricTransitionMode { - type Err = (); - - fn from_str(value: &str) -> Result { - match value { - "random" => Ok(Self::Random), - "blur" => Ok(Self::Blur), - "slide" => Ok(Self::Slide), - "fade" => Ok(Self::Fade), - _ => Err(()), - } - } -} - impl From for LyricTransitionMode { fn from(value: String) -> Self { value.parse().unwrap_or_default() } } -impl From for String { - fn from(value: LyricTransitionMode) -> Self { - value.as_str().to_string() - } -} - #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum LyricTransitionAnimation { Blur, diff --git a/crates/winisland-core/src/config/visual.rs b/crates/winisland-core/src/config/visual.rs index 3cc60d25..45e201d9 100644 --- a/crates/winisland-core/src/config/visual.rs +++ b/crates/winisland-core/src/config/visual.rs @@ -1,4 +1,5 @@ use serde::{Deserialize, Serialize}; +use strum::{Display, EnumString, IntoStaticStr}; pub const TOP_OFFSET: i32 = 10; pub const PADDING: f32 = 80.0; @@ -6,9 +7,10 @@ pub const MIN_HIDDEN_WIDTH: f32 = 0.0; pub const MAX_HIDDEN_WIDTH: f32 = 400.0; pub const MAX_LYRIC_WIDTH: f32 = 700.0; -#[derive(Serialize, Deserialize, Clone, Copy, Debug, PartialEq, Eq)] -#[serde(from = "String", into = "String")] -#[derive(Default)] +#[derive(Serialize, Deserialize, Clone, Copy, Debug, Default, PartialEq, Eq)] +#[serde(from = "String", into = "&'static str")] +#[derive(Display, EnumString, IntoStaticStr)] +#[strum(serialize_all = "snake_case")] pub enum DockPosition { #[default] TopCenter, @@ -34,39 +36,6 @@ impl DockPosition { pub const fn is_right(self) -> bool { matches!(self, Self::TopRight | Self::BottomRight) } - - pub const fn as_str(self) -> &'static str { - match self { - Self::TopCenter => "top_center", - Self::TopLeft => "top_left", - Self::TopRight => "top_right", - Self::BottomCenter => "bottom_center", - Self::BottomLeft => "bottom_left", - Self::BottomRight => "bottom_right", - } - } -} - -impl std::fmt::Display for DockPosition { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str((*self).as_str()) - } -} - -impl std::str::FromStr for DockPosition { - type Err = (); - - fn from_str(s: &str) -> Result { - match s { - "top_center" => Ok(Self::TopCenter), - "top_left" => Ok(Self::TopLeft), - "top_right" => Ok(Self::TopRight), - "bottom_center" => Ok(Self::BottomCenter), - "bottom_left" => Ok(Self::BottomLeft), - "bottom_right" => Ok(Self::BottomRight), - _ => Err(()), - } - } } impl From for DockPosition { @@ -74,9 +43,3 @@ impl From for DockPosition { value.parse().unwrap_or_default() } } - -impl From for String { - fn from(value: DockPosition) -> Self { - value.as_str().to_string() - } -} diff --git a/src/window/settings/pages/music.rs b/src/window/settings/pages/music.rs index 499a5595..70e5c4d1 100644 --- a/src/window/settings/pages/music.rs +++ b/src/window/settings/pages/music.rs @@ -346,7 +346,7 @@ impl SettingsApp { lyric_transition_labels(), LyricTransitionMode::ALL .into_iter() - .map(|mode| mode.as_str().to_string()) + .map(|mode| mode.to_string()) .collect(), LyricTransitionMode::ALL .iter() From da1d00632fb1aec7625e3561b8428b8c1f4c18ca Mon Sep 17 00:00:00 2001 From: Eatgrapes Date: Sat, 26 Sep 2026 23:05:14 +0800 Subject: [PATCH 06/13] refactor(settings): derive a schema for plain config rows Every switch, stepper and select on the general and music pages repeated one pattern: an action variant, a hand-built row, a click arm and a set_* or select_* helper. The settings-schema crates derive the field list, labels, limits and choices from #[setting(..)] on AppConfig, and one generic handler builds those rows and applies clicks, typed numbers and popup choices. Rows with side effects or runtime options (autostart, language, monitor, island style, font, lyrics folder, media apps) stay hand-written. Typed numbers are now rounded to the precision the row displays. --- Cargo.lock | 19 ++ Cargo.toml | 3 + crates/settings-schema-derive/Cargo.toml | 16 + crates/settings-schema-derive/src/lib.rs | 310 +++++++++++++++++ crates/settings-schema/Cargo.toml | 10 + crates/settings-schema/src/lib.rs | 99 ++++++ crates/winisland-core/Cargo.toml | 1 + crates/winisland-core/src/config/lyrics.rs | 2 - crates/winisland-core/src/config/mod.rs | 54 ++- src/window/settings/mod.rs | 4 +- .../settings/pages/general/appearance.rs | 268 ++------------- src/window/settings/pages/general/behavior.rs | 232 ++----------- src/window/settings/pages/general/effects.rs | 118 +------ src/window/settings/pages/mod.rs | 30 +- src/window/settings/pages/music.rs | 311 ++---------------- src/window/settings/pages/schema.rs | 123 +++++++ 16 files changed, 753 insertions(+), 847 deletions(-) create mode 100644 crates/settings-schema-derive/Cargo.toml create mode 100644 crates/settings-schema-derive/src/lib.rs create mode 100644 crates/settings-schema/Cargo.toml create mode 100644 crates/settings-schema/src/lib.rs create mode 100644 src/window/settings/pages/schema.rs diff --git a/Cargo.lock b/Cargo.lock index 7f0076a3..8989c334 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -27,6 +27,7 @@ dependencies = [ "serde", "serde_json", "serde_yaml", + "settings-schema", "sha2 0.11.0", "thiserror 2.0.20", "tokio", @@ -3973,6 +3974,23 @@ dependencies = [ "unsafe-libyaml", ] +[[package]] +name = "settings-schema" +version = "0.1.0" +dependencies = [ + "settings-schema-derive", +] + +[[package]] +name = "settings-schema-derive" +version = "0.1.0" +dependencies = [ + "heck 0.5.0", + "proc-macro2", + "quote", + "syn 2.0.119", +] + [[package]] name = "sha1" version = "0.11.0" @@ -5278,6 +5296,7 @@ dependencies = [ "reqwest", "serde", "serde_json", + "settings-schema", "strum", "toml 1.1.4+spec-1.1.0", ] diff --git a/Cargo.toml b/Cargo.toml index d375ae76..16db3038 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,6 +7,8 @@ edition = "2024" members = [ "crates/c-result", "crates/pollkit", + "crates/settings-schema", + "crates/settings-schema-derive", "crates/winisland-core", "crates/winisland-plugin-api", "crates/winisland-render", @@ -49,6 +51,7 @@ winisland-platform = { path = "crates/winisland-platform" } winisland-platform-windows = { path = "crates/winisland-platform-windows" } pollkit = { path = "crates/pollkit" } c-result = { path = "crates/c-result" } +settings-schema = { path = "crates/settings-schema" } zip = "8.6.0" serde_yaml = "0.9" pulldown-cmark = { version = "0.13.4", default-features = false } diff --git a/crates/settings-schema-derive/Cargo.toml b/crates/settings-schema-derive/Cargo.toml new file mode 100644 index 00000000..51120cd6 --- /dev/null +++ b/crates/settings-schema-derive/Cargo.toml @@ -0,0 +1,16 @@ +[package] +name = "settings-schema-derive" +version = "0.1.0" +edition = "2024" +publish = false +description = "Derive macro for settings-schema" +license = "MIT" + +[lib] +proc-macro = true + +[dependencies] +heck = "0.5" +proc-macro2 = "1" +quote = "1" +syn = "2" diff --git a/crates/settings-schema-derive/src/lib.rs b/crates/settings-schema-derive/src/lib.rs new file mode 100644 index 00000000..cbf393be --- /dev/null +++ b/crates/settings-schema-derive/src/lib.rs @@ -0,0 +1,310 @@ +use heck::ToUpperCamelCase; +use proc_macro::TokenStream; +use proc_macro2::TokenStream as TokenStream2; +use quote::{format_ident, quote}; +use syn::ext::IdentExt; +use syn::meta::ParseNestedMeta; +use syn::parse::{Parse, ParseStream}; +use syn::punctuated::Punctuated; +use syn::{Data, DeriveInput, Expr, Field, Fields, Ident, Lit, LitInt, LitStr, Token, Type}; + +/// Derives `settings_schema::Settings` for a struct with named fields and generates a +/// `Field` enum with one variant per field marked `#[setting(..)]`. Each marked +/// field takes exactly one kind and an optional label: +/// +/// ```text +/// #[setting(toggle)] bool fields +/// #[setting(number(min = 0.0, max = 1.0, step = 0.05, precision = 2))] +/// #[setting(choice("stable" => "channel_stable", 30 => "30 FPS"))] +/// #[setting(toggle, label = "start_on_boot")] +/// ``` +/// +/// Number fields are converted with `as`, so any primitive numeric type works; bounds, +/// step and precision are optional and accept constant expressions. Choice fields go +/// through `Display` and `FromStr`. +#[proc_macro_derive(Settings, attributes(setting))] +pub fn derive_settings(input: TokenStream) -> TokenStream { + let input = syn::parse_macro_input!(input as DeriveInput); + expand(&input) + .unwrap_or_else(syn::Error::into_compile_error) + .into() +} + +struct Setting<'a> { + ident: &'a Ident, + ty: &'a Type, + label: Option, + kind: Kind, +} + +enum Kind { + Toggle, + Number(Box), + Choice(Vec), +} + +#[derive(Default)] +struct NumberArgs { + min: Option, + max: Option, + step: Option, + precision: Option, +} + +struct ChoiceArg { + value: String, + label: LitStr, +} + +impl Parse for ChoiceArg { + fn parse(input: ParseStream) -> syn::Result { + let value = match input.parse()? { + Lit::Str(value) => value.value(), + Lit::Int(value) => value.base10_digits().to_string(), + other => { + return Err(syn::Error::new_spanned( + other, + "expected a string or integer value", + )); + } + }; + input.parse::]>()?; + Ok(Self { + value, + label: input.parse()?, + }) + } +} + +fn expand(input: &DeriveInput) -> syn::Result { + let name = &input.ident; + let Data::Struct(data) = &input.data else { + return Err(syn::Error::new_spanned( + name, + "Settings can only be derived for structs", + )); + }; + let Fields::Named(fields) = &data.fields else { + return Err(syn::Error::new_spanned( + name, + "Settings needs a struct with named fields", + )); + }; + let settings = fields + .named + .iter() + .filter_map(|field| parse_setting(field).transpose()) + .collect::>>()?; + if settings.is_empty() { + return Err(syn::Error::new_spanned( + name, + "no field is marked with #[setting(..)]", + )); + } + + let vis = &input.vis; + let field_enum = format_ident!("{}Field", name); + let (impl_generics, type_generics, where_clause) = input.generics.split_for_impl(); + let variants: Vec = settings + .iter() + .map(|setting| { + Ident::new( + &setting.ident.unraw().to_string().to_upper_camel_case(), + setting.ident.span(), + ) + }) + .collect(); + let count = settings.len(); + let schemas = settings.iter().map(schema_tokens); + let get_arms = settings.iter().zip(&variants).map(|(setting, variant)| { + let ident = setting.ident; + let value = match setting.kind { + Kind::Toggle => quote!(::settings_schema::Value::Bool(self.#ident)), + Kind::Number(_) => quote!(::settings_schema::Value::Number(self.#ident as f64)), + Kind::Choice(_) => quote! { + ::settings_schema::Value::Text(::std::string::ToString::to_string(&self.#ident)) + }, + }; + quote!(#field_enum::#variant => #value) + }); + let set_arms = settings.iter().zip(&variants).map(|(setting, variant)| { + let ident = setting.ident; + let ty = setting.ty; + match setting.kind { + Kind::Toggle => quote! { + (#field_enum::#variant, ::settings_schema::Value::Bool(value)) => self.#ident = value + }, + Kind::Number(_) => quote! { + (#field_enum::#variant, ::settings_schema::Value::Number(value)) => { + self.#ident = value as #ty + } + }, + Kind::Choice(_) => quote! { + (#field_enum::#variant, ::settings_schema::Value::Text(value)) => { + match value.parse::<#ty>() { + ::core::result::Result::Ok(value) => self.#ident = value, + ::core::result::Result::Err(_) => return false, + } + } + }, + } + }); + + Ok(quote! { + #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] + #vis enum #field_enum { + #(#variants,)* + } + + impl #impl_generics ::settings_schema::Settings for #name #type_generics #where_clause { + type Field = #field_enum; + + const FIELDS: &'static [#field_enum] = &[#(#field_enum::#variants),*]; + + fn schema(field: #field_enum) -> &'static ::settings_schema::Schema { + static SCHEMAS: [::settings_schema::Schema; #count] = [#(#schemas),*]; + &SCHEMAS[field as usize] + } + + fn get(&self, field: #field_enum) -> ::settings_schema::Value { + match field { + #(#get_arms,)* + } + } + + fn set(&mut self, field: #field_enum, value: ::settings_schema::Value) -> bool { + let ::core::option::Option::Some(value) = Self::schema(field).kind.normalize(value) + else { + return false; + }; + match (field, value) { + #(#set_arms,)* + _ => return false, + } + true + } + } + }) +} + +fn parse_setting(field: &Field) -> syn::Result>> { + let Some(ident) = &field.ident else { + return Ok(None); + }; + let mut attrs = field + .attrs + .iter() + .filter(|attr| attr.path().is_ident("setting")) + .peekable(); + let Some(first) = attrs.peek().copied() else { + return Ok(None); + }; + let mut label = None; + let mut kind = None; + for attr in attrs { + attr.parse_nested_meta(|meta| { + if meta.path.is_ident("label") { + label = Some(meta.value()?.parse()?); + return Ok(()); + } + let parsed = if meta.path.is_ident("toggle") { + Kind::Toggle + } else if meta.path.is_ident("number") { + Kind::Number(Box::new(parse_number(&meta)?)) + } else if meta.path.is_ident("choice") { + let content; + syn::parenthesized!(content in meta.input); + let choices = Punctuated::::parse_terminated(&content)?; + Kind::Choice(choices.into_iter().collect()) + } else { + return Err( + meta.error("expected `toggle`, `number(..)`, `choice(..)` or `label = \"..\"`") + ); + }; + if kind.replace(parsed).is_some() { + return Err(meta.error("a setting takes exactly one kind")); + } + Ok(()) + })?; + } + let Some(kind) = kind else { + return Err(syn::Error::new_spanned( + first, + "a setting needs `toggle`, `number(..)` or `choice(..)`", + )); + }; + Ok(Some(Setting { + ident, + ty: &field.ty, + label, + kind, + })) +} + +fn parse_number(meta: &ParseNestedMeta) -> syn::Result { + let mut args = NumberArgs::default(); + meta.parse_nested_meta(|arg| { + if arg.path.is_ident("min") { + args.min = Some(arg.value()?.parse()?); + } else if arg.path.is_ident("max") { + args.max = Some(arg.value()?.parse()?); + } else if arg.path.is_ident("step") { + args.step = Some(arg.value()?.parse()?); + } else if arg.path.is_ident("precision") { + args.precision = Some(arg.value()?.parse()?); + } else { + return Err(arg.error("expected `min`, `max`, `step` or `precision`")); + } + Ok(()) + })?; + Ok(args) +} + +fn schema_tokens(setting: &Setting) -> TokenStream2 { + let key = setting.ident.unraw().to_string(); + let label = setting + .label + .as_ref() + .map_or_else(|| key.clone(), LitStr::value); + let kind = match &setting.kind { + Kind::Toggle => quote!(::settings_schema::Kind::Toggle), + Kind::Number(args) => { + let bound = |value: &Option, default: TokenStream2| { + value + .as_ref() + .map_or(default, |value| quote!((#value) as f64)) + }; + let min = bound(&args.min, quote!(f64::NEG_INFINITY)); + let max = bound(&args.max, quote!(f64::INFINITY)); + let step = bound(&args.step, quote!(1.0)); + let precision = args + .precision + .as_ref() + .map_or_else(|| quote!(0), |precision| quote!(#precision)); + quote! { + ::settings_schema::Kind::Number(::settings_schema::Number { + min: #min, + max: #max, + step: #step, + precision: #precision, + }) + } + } + Kind::Choice(choices) => { + let values = choices.iter().map(|choice| &choice.value); + let labels = choices.iter().map(|choice| &choice.label); + quote! { + ::settings_schema::Kind::Choice(&[ + #(::settings_schema::Choice { value: #values, label: #labels }),* + ]) + } + } + }; + quote! { + ::settings_schema::Schema { + key: #key, + label: #label, + kind: #kind, + } + } +} diff --git a/crates/settings-schema/Cargo.toml b/crates/settings-schema/Cargo.toml new file mode 100644 index 00000000..1bcdab96 --- /dev/null +++ b/crates/settings-schema/Cargo.toml @@ -0,0 +1,10 @@ +[package] +name = "settings-schema" +version = "0.1.0" +edition = "2024" +publish = false +description = "Derive a runtime schema for settings structs and read or write their fields through it" +license = "MIT" + +[dependencies] +settings-schema-derive = { path = "../settings-schema-derive" } diff --git a/crates/settings-schema/src/lib.rs b/crates/settings-schema/src/lib.rs new file mode 100644 index 00000000..928225ce --- /dev/null +++ b/crates/settings-schema/src/lib.rs @@ -0,0 +1,99 @@ +//! Field-level reflection for settings structs. `#[derive(Settings)]` lists the fields +//! marked `#[setting(..)]` in a generated `Field` enum and describes each one +//! with a [`Schema`]: its key, its label, and whether it is a toggle, a bounded number +//! or a choice from a list. Code that presents settings walks the schema and reads or +//! writes any field through [`Settings::get`] and [`Settings::set`] instead of carrying +//! glue for every field. Nothing here draws or depends on a UI toolkit. + +pub use settings_schema_derive::Settings; + +pub trait Settings { + /// The generated `Field` enum, with one variant per `#[setting]` field. + type Field: Copy + Eq + 'static; + + /// Every setting field in declaration order. + const FIELDS: &'static [Self::Field]; + + fn schema(field: Self::Field) -> &'static Schema; + + fn get(&self, field: Self::Field) -> Value; + + /// Stores `value` once [`Kind::normalize`] accepts it and, for a choice, once it + /// parses into the field type. Returns whether the field was written. + fn set(&mut self, field: Self::Field, value: Value) -> bool; +} + +#[derive(Debug)] +pub struct Schema { + /// The field name. + pub key: &'static str, + /// The `label = ".."` argument, or the field name when there is none. + pub label: &'static str, + pub kind: Kind, +} + +#[derive(Debug)] +pub enum Kind { + Toggle, + Number(Number), + Choice(&'static [Choice]), +} + +impl Kind { + /// Returns the value a field of this kind would store: numbers are rounded to their + /// precision and clamped, while a value of another kind or an unlisted choice is + /// rejected. + pub fn normalize(&self, value: Value) -> Option { + match (self, value) { + (Self::Toggle, Value::Bool(on)) => Some(Value::Bool(on)), + (Self::Number(number), Value::Number(value)) if value.is_finite() => { + Some(Value::Number(number.normalize(value))) + } + (Self::Choice(choices), Value::Text(text)) + if choices.iter().any(|choice| choice.value == text) => + { + Some(Value::Text(text)) + } + _ => None, + } + } +} + +/// Limits of a numeric setting. Omitted bounds are infinite, and `precision` is the +/// number of decimals that are kept and shown. +#[derive(Clone, Copy, Debug, PartialEq)] +pub struct Number { + pub min: f64, + pub max: f64, + pub step: f64, + pub precision: u8, +} + +impl Number { + pub fn normalize(&self, value: f64) -> f64 { + let scale = 10f64.powi(i32::from(self.precision)); + ((value * scale).round() / scale).clamp(self.min, self.max) + } + + /// Moves `value` by `steps` steps, negative to go down, and normalizes the result. + pub fn stepped(&self, value: f64, steps: i32) -> f64 { + self.normalize(value + self.step * f64::from(steps)) + } + + pub fn format(&self, value: f64) -> String { + format!("{value:.*}", usize::from(self.precision)) + } +} + +#[derive(Debug)] +pub struct Choice { + pub value: &'static str, + pub label: &'static str, +} + +#[derive(Clone, Debug, PartialEq)] +pub enum Value { + Bool(bool), + Number(f64), + Text(String), +} diff --git a/crates/winisland-core/Cargo.toml b/crates/winisland-core/Cargo.toml index 381214b8..b1473f61 100644 --- a/crates/winisland-core/Cargo.toml +++ b/crates/winisland-core/Cargo.toml @@ -14,5 +14,6 @@ quick-xml = "0.41.0" reqwest = { version = "0.13.4", features = ["json"] } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0.149" +settings-schema = { path = "../settings-schema" } strum = { version = "0.28", features = ["derive"] } toml = "1.1.2" diff --git a/crates/winisland-core/src/config/lyrics.rs b/crates/winisland-core/src/config/lyrics.rs index 7782c3bf..7c720590 100644 --- a/crates/winisland-core/src/config/lyrics.rs +++ b/crates/winisland-core/src/config/lyrics.rs @@ -14,8 +14,6 @@ pub enum LyricTransitionMode { } impl LyricTransitionMode { - pub const ALL: [Self; 4] = [Self::Random, Self::Blur, Self::Slide, Self::Fade]; - pub const fn animation(self, random_value: u64) -> LyricTransitionAnimation { match self { Self::Random => match random_value % 3 { diff --git a/crates/winisland-core/src/config/mod.rs b/crates/winisland-core/src/config/mod.rs index bc3fc107..aa87cda4 100644 --- a/crates/winisland-core/src/config/mod.rs +++ b/crates/winisland-core/src/config/mod.rs @@ -5,6 +5,7 @@ mod widget_layout; mod widgets; use serde::{Deserialize, Serialize}; +use settings_schema::Settings; pub use lyrics::*; pub use migrate::*; @@ -17,23 +18,39 @@ pub const APP_AUTHOR: &str = "Eatgrapes"; pub const APP_HOMEPAGE: &str = "https://github.com/WinIslandProject/WinIsland"; pub const WINDOW_TITLE: &str = "WinIsland"; -#[derive(Serialize, Deserialize, Clone, Debug, PartialEq)] +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Settings)] pub struct AppConfig { #[serde(alias = "global_scale")] + #[setting(number(min = 0.5, max = 5.0, step = 0.05, precision = 2))] pub compact_scale: f32, #[serde(default = "default_expanded_scale")] + #[setting(number(min = 0.5, max = 5.0, step = 0.05, precision = 2))] pub expanded_scale: f32, + #[setting(number(min = 40.0, max = 400.0, step = 5.0))] pub base_width: f32, + #[setting(number(min = 15.0, max = 200.0, step = 2.0))] pub base_height: f32, + #[setting(number(min = 200.0, max = 2000.0, step = 10.0))] pub expanded_width: f32, + #[setting(number(min = 100.0, max = 1000.0, step = 10.0))] pub expanded_height: f32, + #[setting(toggle)] pub motion_blur: bool, #[serde(default = "default_animation_fps")] + #[setting(choice( + 30 => "30 FPS", + 60 => "60 FPS", + 90 => "90 FPS", + 120 => "120 FPS", + 0 => "frame_rate_native", + ))] pub animation_fps: u32, #[serde(default = "default_expanded_idle_fps")] + #[setting(choice(30 => "30 FPS", 45 => "45 FPS", 60 => "60 FPS", 90 => "90 FPS"))] pub expanded_idle_fps: u32, #[serde(default = "default_island_style")] pub island_style: String, + #[setting(toggle, label = "smtc_control")] pub smtc_enabled: bool, #[serde(default)] pub music_notice_acknowledged: bool, @@ -41,10 +58,13 @@ pub struct AppConfig { #[serde(default)] pub smtc_known_apps: Vec, #[serde(default = "default_show_lyrics")] + #[setting(toggle)] pub show_lyrics: bool, #[serde(default)] + #[setting(toggle)] pub show_secondary_lyrics: bool, #[serde(default = "default_lyrics_mode")] + #[setting(choice("online" => "lyrics_mode_online", "lrc" => "lyrics_mode_lrc"))] pub lyrics_mode: String, #[serde(default)] pub lyrics_local_dir: Option, @@ -53,34 +73,59 @@ pub struct AppConfig { #[serde(default)] pub auto_start: bool, #[serde(default)] + #[setting(toggle)] pub auto_hide: bool, #[serde(default)] + #[setting(toggle)] pub fullscreen_auto_hide: bool, #[serde(default = "default_auto_hide_delay")] + #[setting(number(min = 1.0, max = 60.0, step = 1.0), label = "hide_delay")] pub auto_hide_delay: f32, #[serde(default = "default_hidden_width")] + #[setting(number(min = MIN_HIDDEN_WIDTH, max = MAX_HIDDEN_WIDTH, step = 1.0))] pub hidden_width: f32, #[serde(default = "default_check_for_updates")] + #[setting(toggle, label = "check_updates")] pub check_for_updates: bool, #[serde(default = "default_update_check_interval")] + #[setting(number(min = 1.0, max = 24.0, step = 1.0), label = "update_interval")] pub update_check_interval: f32, #[serde(default = "default_language")] pub language: String, #[serde(default = "default_lyrics_source")] + #[setting(choice( + "163" => "lyrics_source_163", + "qq" => "lyrics_source_qq", + "kugou" => "lyrics_source_kugou", + "lrclib" => "lyrics_source_lrclib", + "amll" => "AMLL", + ))] pub lyrics_source: String, #[serde(default)] + #[setting(number(min = -10.0, max = 10.0, step = 0.1, precision = 1))] pub lyrics_delay: f64, #[serde(default)] + #[setting(toggle)] pub lyrics_scroll: bool, #[serde(default = "default_lyrics_scroll_max_width")] + #[setting(number(min = 100.0, max = 500.0, step = 10.0))] pub lyrics_scroll_max_width: f32, #[serde(default = "default_lyrics_side_gap")] + #[setting(number(min = 0.0, max = 32.0, step = 1.0))] pub lyrics_side_gap: f32, #[serde(default)] + #[setting(choice( + "random" => "lyrics_transition_random", + "blur" => "lyrics_transition_blur", + "slide" => "lyrics_transition_slide", + "fade" => "lyrics_transition_fade", + ))] pub lyrics_transition_animation: LyricTransitionMode, #[serde(default)] + #[setting(number(step = 5.0))] pub position_x_offset: i32, #[serde(default)] + #[setting(number(step = 5.0))] pub position_y_offset: i32, #[serde( rename = "dock_position", @@ -91,8 +136,10 @@ pub struct AppConfig { #[serde(default)] pub monitor_index: i32, #[serde(default)] + #[setting(number(min = 0.0, max = 30.0, step = 1.0))] pub font_size: f32, #[serde(default = "default_settings_theme")] + #[setting(choice("system" => "theme_system", "light" => "theme_light", "dark" => "theme_dark"))] pub settings_theme: String, #[serde(default = "default_mini_cover_shape")] pub mini_cover_shape: String, @@ -101,14 +148,19 @@ pub struct AppConfig { #[serde(default = "default_cover_rotate")] pub cover_rotate: bool, #[serde(default = "default_update_channel")] + #[setting(choice("stable" => "channel_stable", "beta" => "channel_beta"))] pub update_channel: String, #[serde(default)] + #[setting(toggle)] pub right_click_drag: bool, #[serde(default)] + #[setting(toggle)] pub notification_display: bool, #[serde(default = "default_replace_native_volume_flyout")] + #[setting(toggle)] pub replace_native_volume_flyout: bool, #[serde(default = "default_brightness_overlay_enabled")] + #[setting(toggle, label = "brightness_overlay")] pub brightness_overlay_enabled: bool, #[serde(default = "default_widget_layout")] pub widget_layout: Vec, diff --git a/src/window/settings/mod.rs b/src/window/settings/mod.rs index e3a89b47..1fa3f0b5 100644 --- a/src/window/settings/mod.rs +++ b/src/window/settings/mod.rs @@ -11,7 +11,7 @@ use crate::utils::settings_ui::{ use pollkit::Job; use std::time::{Duration, Instant}; use winisland_core::anim::AnimPool; -use winisland_core::config::AppConfig; +use winisland_core::config::{AppConfig, AppConfigField}; use winisland_core::widgets::PluginWidget; use winisland_platform::{ CursorKind, InputState, Key, LogicalWindowSize, MouseButton, MouseWheelDelta, PlatformEvent, @@ -248,6 +248,7 @@ pub struct SettingsApp { pub(crate) plugin_settings_pages: Vec, pub(crate) plugin_settings_error: Option<(u64, String)>, pub(crate) pending_plugin_setting: Option, + pub(crate) pending_setting: Option, plugin_request: Option, close_requested: bool, } @@ -379,6 +380,7 @@ impl SettingsApp { plugin_settings_pages, plugin_settings_error: None, pending_plugin_setting: None, + pending_setting: None, plugin_request: None, close_requested: false, } diff --git a/src/window/settings/pages/general/appearance.rs b/src/window/settings/pages/general/appearance.rs index ef4ea39c..bb519187 100644 --- a/src/window/settings/pages/general/appearance.rs +++ b/src/window/settings/pages/general/appearance.rs @@ -1,5 +1,6 @@ -use crate::utils::settings_ui::{ClickResult, StepDirection}; -use crate::window::settings::{NumberInputHandler, PopupState}; +use crate::utils::settings_ui::ClickResult; +use crate::window::settings::PopupState; +use winisland_core::config::AppConfigField; use winisland_core::i18n::tr; use super::super::{PageInput, SettingsPage}; @@ -7,15 +8,6 @@ use super::SettingsApp; #[derive(Clone, Copy)] pub(super) enum AppearanceAction { - CompactScale, - ExpandedScale, - BaseWidth, - BaseHeight, - ExpandedWidth, - ExpandedHeight, - PositionX, - PositionY, - FontSize, Monitor, } @@ -24,60 +16,15 @@ impl SettingsApp { let mut page = SettingsPage::new(); page.section(tr("section_appearance")); page.group_start(); - page.row_stepper( - tr("compact_scale"), - format!("{:.2}", self.config.compact_scale), - true, - AppearanceAction::CompactScale, - ); - page.row_stepper( - tr("expanded_scale"), - format!("{:.2}", self.config.expanded_scale), - true, - AppearanceAction::ExpandedScale, - ); - page.row_stepper( - tr("base_width"), - self.config.base_width.to_string(), - true, - AppearanceAction::BaseWidth, - ); - page.row_stepper( - tr("base_height"), - self.config.base_height.to_string(), - true, - AppearanceAction::BaseHeight, - ); - page.row_stepper( - tr("expanded_width"), - self.config.expanded_width.to_string(), - true, - AppearanceAction::ExpandedWidth, - ); - page.row_stepper( - tr("expanded_height"), - self.config.expanded_height.to_string(), - true, - AppearanceAction::ExpandedHeight, - ); - page.row_stepper( - tr("position_x_offset"), - self.config.position_x_offset.to_string(), - true, - AppearanceAction::PositionX, - ); - page.row_stepper( - tr("position_y_offset"), - self.config.position_y_offset.to_string(), - true, - AppearanceAction::PositionY, - ); - page.row_stepper( - tr("font_size"), - format!("{:.0}", self.config.font_size), - true, - AppearanceAction::FontSize, - ); + page.setting(&self.config, AppConfigField::CompactScale, true); + page.setting(&self.config, AppConfigField::ExpandedScale, true); + page.setting(&self.config, AppConfigField::BaseWidth, true); + page.setting(&self.config, AppConfigField::BaseHeight, true); + page.setting(&self.config, AppConfigField::ExpandedWidth, true); + page.setting(&self.config, AppConfigField::ExpandedHeight, true); + page.setting(&self.config, AppConfigField::PositionXOffset, true); + page.setting(&self.config, AppConfigField::PositionYOffset, true); + page.setting(&self.config, AppConfigField::FontSize, true); let monitors = Self::get_monitor_list(); let selected_monitor = @@ -99,191 +46,32 @@ impl SettingsApp { pub(super) fn handle_appearance_click(&mut self, input: PageInput) { let page = self.build_appearance_page(); let result = input.hit_test(&page); - let Some(action) = page.action(&result).copied() else { - return; - }; - - if let ClickResult::StepperValue(item_index) = &result { - let (value, on_commit): (String, NumberInputHandler) = match action { - AppearanceAction::CompactScale => ( - format!("{:.2}", self.config.compact_scale), - set_compact_scale, - ), - AppearanceAction::ExpandedScale => ( - format!("{:.2}", self.config.expanded_scale), - set_expanded_scale, - ), - AppearanceAction::BaseWidth => (self.config.base_width.to_string(), set_base_width), - AppearanceAction::BaseHeight => { - (self.config.base_height.to_string(), set_base_height) - } - AppearanceAction::ExpandedWidth => { - (self.config.expanded_width.to_string(), set_expanded_width) - } - AppearanceAction::ExpandedHeight => { - (self.config.expanded_height.to_string(), set_expanded_height) - } - AppearanceAction::PositionX => { - (self.config.position_x_offset.to_string(), set_position_x) - } - AppearanceAction::PositionY => { - (self.config.position_y_offset.to_string(), set_position_y) - } - AppearanceAction::FontSize => { - (format!("{:.0}", self.config.font_size), set_font_size) - } - AppearanceAction::Monitor => return, - }; - self.begin_number_input( - input.stepper_value_rect(&page, *item_index, self.scroll_y), - value, - on_commit, - ); + if self.handle_setting_click(&page, &result, input) { return; } - - if let Some(direction) = result.step_direction() { - match action { - AppearanceAction::CompactScale => { - self.config.compact_scale = - (step(self.config.compact_scale, direction, 0.05, 0.5, 5.0) * 100.0) - .round() - / 100.0; - } - AppearanceAction::ExpandedScale => { - self.config.expanded_scale = - (step(self.config.expanded_scale, direction, 0.05, 0.5, 5.0) * 100.0) - .round() - / 100.0; - } - AppearanceAction::BaseWidth => { - self.config.base_width = - step(self.config.base_width, direction, 5.0, 40.0, 400.0); - } - AppearanceAction::BaseHeight => { - self.config.base_height = - step(self.config.base_height, direction, 2.0, 15.0, 200.0); - } - AppearanceAction::ExpandedWidth => { - self.config.expanded_width = - step(self.config.expanded_width, direction, 10.0, 200.0, 2000.0); - } - AppearanceAction::ExpandedHeight => { - self.config.expanded_height = - step(self.config.expanded_height, direction, 10.0, 100.0, 1000.0); - } - AppearanceAction::PositionX => { - self.config.position_x_offset += signed_step(direction, 5); - } - AppearanceAction::PositionY => { - self.config.position_y_offset += signed_step(direction, 5); - } - AppearanceAction::FontSize => { - self.config.font_size = step(self.config.font_size, direction, 1.0, 0.0, 30.0); - } - AppearanceAction::Monitor => return, - } - self.persist_settings_change(); - return; - } - - let ClickResult::SourceButton(item_index) = result else { + let (Some(AppearanceAction::Monitor), ClickResult::SourceButton(item_index)) = + (page.action(&result), result) + else { return; }; let button_rect = input.popup_button_rect(&page, item_index, self.scroll_y); let (win_w, win_h) = self.logical_window_size(); - let popup = match action { - AppearanceAction::Monitor => { - let monitors = Self::get_monitor_list(); - let selected = - (self.config.monitor_index as usize).min(monitors.len().saturating_sub(1)); - let values = (0..monitors.len()).map(|index| index.to_string()).collect(); - PopupState::new( - select_monitor, - button_rect, - monitors, - values, - selected, - win_w, - win_h, - ) - } - _ => return, - }; - self.show_popup(popup); - } -} - -fn step(value: f32, direction: StepDirection, amount: f32, min: f32, max: f32) -> f32 { - match direction { - StepDirection::Decrement => value - amount, - StepDirection::Increment => value + amount, - } - .clamp(min, max) -} - -fn signed_step(direction: StepDirection, amount: i32) -> i32 { - match direction { - StepDirection::Decrement => -amount, - StepDirection::Increment => amount, + let monitors = Self::get_monitor_list(); + let selected = (self.config.monitor_index as usize).min(monitors.len().saturating_sub(1)); + let values = (0..monitors.len()).map(|index| index.to_string()).collect(); + self.show_popup(PopupState::new( + select_monitor, + button_rect, + monitors, + values, + selected, + win_w, + win_h, + )); } } fn select_monitor(app: &mut SettingsApp, value: &str) { app.config.monitor_index = value.parse().unwrap_or(0); } - -fn set_compact_scale(app: &mut SettingsApp, value: &str) { - if let Ok(value) = value.parse::() { - app.config.compact_scale = value.clamp(0.5, 5.0); - } -} - -fn set_expanded_scale(app: &mut SettingsApp, value: &str) { - if let Ok(value) = value.parse::() { - app.config.expanded_scale = value.clamp(0.5, 5.0); - } -} - -fn set_base_width(app: &mut SettingsApp, value: &str) { - if let Ok(value) = value.parse::() { - app.config.base_width = value.clamp(40.0, 400.0); - } -} - -fn set_base_height(app: &mut SettingsApp, value: &str) { - if let Ok(value) = value.parse::() { - app.config.base_height = value.clamp(15.0, 200.0); - } -} - -fn set_expanded_width(app: &mut SettingsApp, value: &str) { - if let Ok(value) = value.parse::() { - app.config.expanded_width = value.clamp(200.0, 2000.0); - } -} - -fn set_expanded_height(app: &mut SettingsApp, value: &str) { - if let Ok(value) = value.parse::() { - app.config.expanded_height = value.clamp(100.0, 1000.0); - } -} - -fn set_position_x(app: &mut SettingsApp, value: &str) { - if let Ok(value) = value.parse::() { - app.config.position_x_offset = value; - } -} - -fn set_position_y(app: &mut SettingsApp, value: &str) { - if let Ok(value) = value.parse::() { - app.config.position_y_offset = value; - } -} - -fn set_font_size(app: &mut SettingsApp, value: &str) { - if let Ok(value) = value.parse::() { - app.config.font_size = value.clamp(0.0, 30.0); - } -} diff --git a/src/window/settings/pages/general/behavior.rs b/src/window/settings/pages/general/behavior.rs index f8a39fcf..8bf837e2 100644 --- a/src/window/settings/pages/general/behavior.rs +++ b/src/window/settings/pages/general/behavior.rs @@ -1,6 +1,7 @@ -use crate::utils::settings_ui::{ClickResult, StepDirection}; -use crate::window::settings::{NumberInputHandler, PopupState}; -use winisland_core::config::{AppConfig, MAX_HIDDEN_WIDTH, MIN_HIDDEN_WIDTH}; +use crate::utils::settings_ui::ClickResult; +use crate::utils::settings_ui::items::SettingsItem; +use crate::window::settings::PopupState; +use winisland_core::config::{AppConfig, AppConfigField, MAX_HIDDEN_WIDTH}; use winisland_core::i18n::{available_langs, current_lang, init_i18n, set_lang, tr}; use winisland_render::text::FontManager; @@ -10,18 +11,7 @@ use super::SettingsApp; #[derive(Clone, Copy)] pub(super) enum BehaviorAction { AutoStart, - AutoHide, - FullscreenAutoHide, - HiddenWidth, - RightClickDrag, - NotificationDisplay, - ReplaceNativeVolumeFlyout, - BrightnessOverlay, - HideDelay, Language, - CheckForUpdates, - UpdateChannel, - UpdateInterval, CheckUpdatesNow, ResetDefaults, HideIsland, @@ -40,59 +30,38 @@ impl SettingsApp { caps.autostart, BehaviorAction::AutoStart, ); - page.row_switch( - tr("auto_hide"), - self.config.auto_hide, - true, - BehaviorAction::AutoHide, - ); - page.row_switch( - tr("fullscreen_auto_hide"), - self.config.fullscreen_auto_hide, - true, - BehaviorAction::FullscreenAutoHide, - ); + page.setting(&self.config, AppConfigField::AutoHide, true); + page.setting(&self.config, AppConfigField::FullscreenAutoHide, true); if self.config.auto_hide { - page.row_stepper( - tr("hide_delay"), - format!("{:.0}", self.config.auto_hide_delay), - true, - BehaviorAction::HideDelay, - ); + page.setting(&self.config, AppConfigField::AutoHideDelay, true); } - page.row_stepper( - tr("hidden_width"), - if self.config.hidden_width >= MAX_HIDDEN_WIDTH { - tr("hidden_width_off") - } else { - format!("{:.0}", self.config.hidden_width) + page.push_setting( + SettingsItem::RowStepper { + label: tr("hidden_width"), + value: if self.config.hidden_width >= MAX_HIDDEN_WIDTH { + tr("hidden_width_off") + } else { + format!("{:.0}", self.config.hidden_width) + }, + enabled: true, }, - true, - BehaviorAction::HiddenWidth, - ); - page.row_switch( - tr("right_click_drag"), - self.config.right_click_drag, - true, - BehaviorAction::RightClickDrag, + AppConfigField::HiddenWidth, ); - page.row_switch( - tr("notification_display"), - self.config.notification_display, + page.setting(&self.config, AppConfigField::RightClickDrag, true); + page.setting( + &self.config, + AppConfigField::NotificationDisplay, caps.toast_events, - BehaviorAction::NotificationDisplay, ); - page.row_switch( - tr("replace_native_volume_flyout"), - self.config.replace_native_volume_flyout, + page.setting( + &self.config, + AppConfigField::ReplaceNativeVolumeFlyout, caps.input_hooks && caps.volume_control, - BehaviorAction::ReplaceNativeVolumeFlyout, ); - page.row_switch( - tr("brightness_overlay"), - self.config.brightness_overlay_enabled, + page.setting( + &self.config, + AppConfigField::BrightnessOverlayEnabled, caps.brightness_control, - BehaviorAction::BrightnessOverlay, ); if !caps.autostart || !caps.toast_events || !caps.input_hooks || !caps.brightness_control { page.row_label(tr("platform_unavailable")); @@ -120,28 +89,10 @@ impl SettingsApp { page.group_end(); page.section(tr("section_updates")); page.group_start(); - page.row_switch( - tr("check_updates"), - self.config.check_for_updates, - true, - BehaviorAction::CheckForUpdates, - ); + page.setting(&self.config, AppConfigField::CheckForUpdates, true); if self.config.check_for_updates { - page.row_source( - tr("update_channel"), - vec![ - (tr("channel_stable"), self.config.update_channel == "stable"), - (tr("channel_beta"), self.config.update_channel == "beta"), - ], - true, - BehaviorAction::UpdateChannel, - ); - page.row_stepper( - tr("update_interval"), - format!("{:.0}", self.config.update_check_interval), - true, - BehaviorAction::UpdateInterval, - ); + page.setting(&self.config, AppConfigField::UpdateChannel, true); + page.setting(&self.config, AppConfigField::UpdateCheckInterval, true); } page.row_button( tr("check_updates_manual"), @@ -162,33 +113,13 @@ impl SettingsApp { pub(super) fn handle_behavior_click(&mut self, input: PageInput) { let page = self.build_behavior_page(); let result = input.hit_test(&page); + if self.handle_setting_click(&page, &result, input) { + return; + } let Some(action) = page.action(&result).copied() else { return; }; - if let ClickResult::StepperValue(item_index) = &result { - let (value, on_commit): (String, NumberInputHandler) = match action { - BehaviorAction::HideDelay => ( - format!("{:.0}", self.config.auto_hide_delay), - set_hide_delay, - ), - BehaviorAction::HiddenWidth => { - (format!("{:.0}", self.config.hidden_width), set_hidden_width) - } - BehaviorAction::UpdateInterval => ( - format!("{:.0}", self.config.update_check_interval), - set_update_interval, - ), - _ => return, - }; - self.begin_number_input( - input.stepper_value_rect(&page, *item_index, self.scroll_y), - value, - on_commit, - ); - return; - } - let changed = match (action, &result) { (BehaviorAction::AutoStart, ClickResult::Switch(_)) => { let enabled = !self.config.auto_start; @@ -213,64 +144,6 @@ impl SettingsApp { self.plugin_request = Some(crate::window::settings::PluginSettingsRequest::Exit); false } - (BehaviorAction::AutoHide, ClickResult::Switch(_)) => { - self.config.auto_hide = !self.config.auto_hide; - true - } - (BehaviorAction::FullscreenAutoHide, ClickResult::Switch(_)) => { - self.config.fullscreen_auto_hide = !self.config.fullscreen_auto_hide; - true - } - (BehaviorAction::RightClickDrag, ClickResult::Switch(_)) => { - self.config.right_click_drag = !self.config.right_click_drag; - true - } - (BehaviorAction::NotificationDisplay, ClickResult::Switch(_)) => { - self.config.notification_display = !self.config.notification_display; - true - } - (BehaviorAction::ReplaceNativeVolumeFlyout, ClickResult::Switch(_)) => { - self.config.replace_native_volume_flyout = - !self.config.replace_native_volume_flyout; - true - } - (BehaviorAction::BrightnessOverlay, ClickResult::Switch(_)) => { - self.config.brightness_overlay_enabled = !self.config.brightness_overlay_enabled; - true - } - (BehaviorAction::CheckForUpdates, ClickResult::Switch(_)) => { - self.config.check_for_updates = !self.config.check_for_updates; - true - } - (BehaviorAction::HideDelay, _) => { - let Some(direction) = result.step_direction() else { - return; - }; - self.config.auto_hide_delay = - step(self.config.auto_hide_delay, direction, 1.0, 1.0, 60.0); - true - } - (BehaviorAction::HiddenWidth, _) => { - let Some(direction) = result.step_direction() else { - return; - }; - self.config.hidden_width = step( - self.config.hidden_width, - direction, - 1.0, - MIN_HIDDEN_WIDTH, - MAX_HIDDEN_WIDTH, - ); - true - } - (BehaviorAction::UpdateInterval, _) => { - let Some(direction) = result.step_direction() else { - return; - }; - self.config.update_check_interval = - step(self.config.update_check_interval, direction, 1.0, 1.0, 24.0); - true - } (BehaviorAction::ResetDefaults, ClickResult::CenterLink(_)) => { self.config = AppConfig::default(); init_i18n(&self.config.language); @@ -312,53 +185,14 @@ impl SettingsApp { win_h, ) } - BehaviorAction::UpdateChannel => PopupState::new( - select_update_channel, - button_rect, - vec![tr("channel_stable"), tr("channel_beta")], - vec!["stable".to_string(), "beta".to_string()], - usize::from(self.config.update_channel == "beta"), - win_w, - win_h, - ), _ => return, }; self.show_popup(popup); } } -fn step(value: f32, direction: StepDirection, amount: f32, min: f32, max: f32) -> f32 { - match direction { - StepDirection::Decrement => value - amount, - StepDirection::Increment => value + amount, - } - .clamp(min, max) -} - fn select_language(app: &mut SettingsApp, value: &str) { app.config.language = value.to_string(); set_lang(value); crate::ui::widget::expanded::calendar::clear_calendar_text_cache(); } - -fn select_update_channel(app: &mut SettingsApp, value: &str) { - app.config.update_channel = value.to_string(); -} - -fn set_hide_delay(app: &mut SettingsApp, value: &str) { - if let Ok(value) = value.parse::() { - app.config.auto_hide_delay = value.clamp(1.0, 60.0); - } -} - -fn set_hidden_width(app: &mut SettingsApp, value: &str) { - if let Ok(value) = value.parse::() { - app.config.hidden_width = value.clamp(MIN_HIDDEN_WIDTH, MAX_HIDDEN_WIDTH); - } -} - -fn set_update_interval(app: &mut SettingsApp, value: &str) { - if let Ok(value) = value.parse::() { - app.config.update_check_interval = value.clamp(1.0, 24.0); - } -} diff --git a/src/window/settings/pages/general/effects.rs b/src/window/settings/pages/general/effects.rs index 8f8980a9..ae82ef32 100644 --- a/src/window/settings/pages/general/effects.rs +++ b/src/window/settings/pages/general/effects.rs @@ -1,5 +1,6 @@ use crate::utils::settings_ui::ClickResult; use crate::window::settings::PopupState; +use winisland_core::config::AppConfigField; use winisland_core::i18n::tr; use winisland_render::text::FontManager; @@ -8,10 +9,6 @@ use super::SettingsApp; #[derive(Clone, Copy)] pub(super) enum EffectsAction { - SettingsTheme, - MotionBlur, - AnimationFps, - ExpandedIdleFps, IslandStyle, CustomFont, } @@ -21,44 +18,10 @@ impl SettingsApp { let mut page = SettingsPage::new(); page.section(tr("section_effects")); page.group_start(); - page.row_source( - tr("settings_theme"), - vec![ - (tr("theme_system"), self.config.settings_theme == "system"), - (tr("theme_light"), self.config.settings_theme == "light"), - (tr("theme_dark"), self.config.settings_theme == "dark"), - ], - true, - EffectsAction::SettingsTheme, - ); - page.row_switch( - tr("motion_blur"), - self.config.motion_blur, - true, - EffectsAction::MotionBlur, - ); - page.row_source( - tr("animation_fps"), - [30, 60, 90, 120] - .into_iter() - .map(|fps| (format!("{fps} FPS"), self.config.animation_fps == fps)) - .chain(std::iter::once(( - tr("frame_rate_native"), - self.config.animation_fps == 0, - ))) - .collect(), - true, - EffectsAction::AnimationFps, - ); - page.row_source( - tr("expanded_idle_fps"), - [30, 45, 60, 90] - .into_iter() - .map(|fps| (format!("{fps} FPS"), self.config.expanded_idle_fps == fps)) - .collect(), - true, - EffectsAction::ExpandedIdleFps, - ); + page.setting(&self.config, AppConfigField::SettingsTheme, true); + page.setting(&self.config, AppConfigField::MotionBlur, true); + page.setting(&self.config, AppConfigField::AnimationFps, true); + page.setting(&self.config, AppConfigField::ExpandedIdleFps, true); page.group_end(); page.group_start(); let host_backdrop = crate::platform::capabilities().host_backdrop; @@ -92,15 +55,14 @@ impl SettingsApp { pub(super) fn handle_effects_click(&mut self, input: PageInput) { let page = self.build_effects_page(); let result = input.hit_test(&page); + if self.handle_setting_click(&page, &result, input) { + return; + } let Some(action) = page.action(&result).copied() else { return; }; let changed = match (action, &result) { - (EffectsAction::MotionBlur, ClickResult::Switch(_)) => { - self.config.motion_blur = !self.config.motion_blur; - true - } (EffectsAction::CustomFont, ClickResult::FontSelect(_)) => { let Some(path) = rfd::FileDialog::new() .add_filter("Fonts", &["ttf", "otf"]) @@ -130,23 +92,6 @@ impl SettingsApp { let button_rect = input.popup_button_rect(&page, item_index, self.scroll_y); let (win_w, win_h) = self.logical_window_size(); let popup = match action { - EffectsAction::SettingsTheme => PopupState::new( - select_theme, - button_rect, - vec![tr("theme_system"), tr("theme_light"), tr("theme_dark")], - vec![ - "system".to_string(), - "light".to_string(), - "dark".to_string(), - ], - match self.config.settings_theme.as_str() { - "light" => 1, - "dark" => 2, - _ => 0, - }, - win_w, - win_h, - ), EffectsAction::IslandStyle => { let host_backdrop = crate::platform::capabilities().host_backdrop; let mut labels = vec![tr("style_default")]; @@ -171,47 +116,12 @@ impl SettingsApp { win_h, ) } - EffectsAction::AnimationFps => PopupState::new( - select_animation_fps, - button_rect, - vec![ - "30 FPS".to_string(), - "60 FPS".to_string(), - "90 FPS".to_string(), - "120 FPS".to_string(), - tr("frame_rate_native"), - ], - [30, 60, 90, 120, 0].map(|fps| fps.to_string()).to_vec(), - [30, 60, 90, 120, 0] - .iter() - .position(|fps| *fps == self.config.animation_fps) - .unwrap_or(2), - win_w, - win_h, - ), - EffectsAction::ExpandedIdleFps => PopupState::new( - select_expanded_idle_fps, - button_rect, - [30, 45, 60, 90].map(|fps| format!("{fps} FPS")).to_vec(), - [30, 45, 60, 90].map(|fps| fps.to_string()).to_vec(), - [30, 45, 60, 90] - .iter() - .position(|fps| *fps == self.config.expanded_idle_fps) - .unwrap_or(2), - win_w, - win_h, - ), _ => return, }; self.show_popup(popup); } } -fn select_theme(app: &mut SettingsApp, value: &str) { - app.config.settings_theme = value.to_string(); - app.update_theme(); -} - fn select_island_style(app: &mut SettingsApp, value: &str) { app.config.island_style = if value == "glass" && !crate::platform::capabilities().host_backdrop { @@ -220,15 +130,3 @@ fn select_island_style(app: &mut SettingsApp, value: &str) { value.to_string() }; } - -fn select_animation_fps(app: &mut SettingsApp, value: &str) { - if let Ok(fps) = value.parse() { - app.config.animation_fps = fps; - } -} - -fn select_expanded_idle_fps(app: &mut SettingsApp, value: &str) { - if let Ok(fps) = value.parse() { - app.config.expanded_idle_fps = fps; - } -} diff --git a/src/window/settings/pages/mod.rs b/src/window/settings/pages/mod.rs index df24ca7b..b772f202 100644 --- a/src/window/settings/pages/mod.rs +++ b/src/window/settings/pages/mod.rs @@ -1,3 +1,4 @@ +use winisland_core::config::AppConfigField; use winisland_render::{Image, Rect, Rgba}; use crate::utils::settings_ui::items::{ @@ -13,11 +14,17 @@ pub mod general; pub mod music; pub mod plugin_settings; pub mod plugins; +mod schema; pub mod widgets; pub(crate) struct SettingsPage { items: Vec, - actions: Vec>, + actions: Vec>>, +} + +enum RowAction { + Page(A), + Setting(AppConfigField), } impl SettingsPage { @@ -35,7 +42,12 @@ impl SettingsPage { pub(crate) fn push_action(&mut self, item: SettingsItem, action: A) { self.items.push(item); - self.actions.push(Some(action)); + self.actions.push(Some(RowAction::Page(action))); + } + + pub(crate) fn push_setting(&mut self, item: SettingsItem, field: AppConfigField) { + self.items.push(item); + self.actions.push(Some(RowAction::Setting(field))); } pub(crate) fn section(&mut self, label: String) { @@ -187,6 +199,20 @@ impl SettingsPage { } pub(crate) fn action(&self, result: &ClickResult) -> Option<&A> { + match self.row_action(result)? { + RowAction::Page(action) => Some(action), + RowAction::Setting(_) => None, + } + } + + pub(crate) fn setting_field(&self, result: &ClickResult) -> Option { + match self.row_action(result)? { + RowAction::Setting(field) => Some(*field), + RowAction::Page(_) => None, + } + } + + fn row_action(&self, result: &ClickResult) -> Option<&RowAction> { result .item_index() .and_then(|index| self.actions.get(index)) diff --git a/src/window/settings/pages/music.rs b/src/window/settings/pages/music.rs index 70e5c4d1..5f2544aa 100644 --- a/src/window/settings/pages/music.rs +++ b/src/window/settings/pages/music.rs @@ -1,10 +1,10 @@ +use crate::utils::settings_ui::ClickResult; use crate::utils::settings_ui::items::SettingsItem; -use crate::utils::settings_ui::{ClickResult, StepDirection}; -use winisland_core::config::LyricTransitionMode; +use winisland_core::config::AppConfigField; use winisland_core::i18n::{current_lang, tr}; use winisland_render::{Point, Rect}; -use super::super::{NumberInputHandler, PopupState, SettingsApp}; +use super::super::SettingsApp; use super::{PageInput, SettingsPage}; pub(super) const MUSIC_NOTICE_HEIGHT: f32 = 150.0; @@ -15,16 +15,6 @@ pub(crate) fn music_notice_button_rect(width: f32, start_y: f32) -> Rect { #[derive(Clone)] enum MusicAction { - SmtcEnabled, - LyricsMode, - ShowLyrics, - ShowSecondaryLyrics, - LyricsSource, - LyricsDelay, - LyricsScroll, - LyricsScrollWidth, - LyricsSideGap, - LyricsTransitionAnimation, LyricsFolder, App(String), } @@ -45,12 +35,7 @@ impl SettingsApp { } page.section(tr("section_playback")); page.group_start(); - page.row_switch( - tr("smtc_control"), - self.config.smtc_enabled, - media_available, - MusicAction::SmtcEnabled, - ); + page.setting(&self.config, AppConfigField::SmtcEnabled, media_available); if !media_available { page.row_label(tr("platform_unavailable")); } @@ -59,18 +44,7 @@ impl SettingsApp { if self.config.smtc_enabled && media_available { page.section(tr("section_lyrics")); page.group_start(); - page.row_source( - tr("lyrics_mode"), - vec![ - ( - tr("lyrics_mode_online"), - self.config.lyrics_mode == "online", - ), - (tr("lyrics_mode_lrc"), self.config.lyrics_mode == "lrc"), - ], - true, - MusicAction::LyricsMode, - ); + page.setting(&self.config, AppConfigField::LyricsMode, true); if self.config.lyrics_mode == "lrc" { page.row_folder( tr("lyrics_local_dir"), @@ -87,78 +61,25 @@ impl SettingsApp { true, MusicAction::LyricsFolder, ); - page.row_switch( - tr("show_secondary_lyrics"), - self.config.show_secondary_lyrics, - true, - MusicAction::ShowSecondaryLyrics, - ); + page.setting(&self.config, AppConfigField::ShowSecondaryLyrics, true); } else { - page.row_switch( - tr("show_lyrics"), - show_lyrics, - true, - MusicAction::ShowLyrics, - ); + page.setting(&self.config, AppConfigField::ShowLyrics, true); if show_lyrics { - page.row_switch( - tr("show_secondary_lyrics"), - self.config.show_secondary_lyrics, - true, - MusicAction::ShowSecondaryLyrics, - ); - page.row_source( - tr("lyrics_source"), - vec![ - (tr("lyrics_source_163"), self.config.lyrics_source == "163"), - (tr("lyrics_source_qq"), self.config.lyrics_source == "qq"), - ( - tr("lyrics_source_kugou"), - self.config.lyrics_source == "kugou", - ), - ( - tr("lyrics_source_lrclib"), - self.config.lyrics_source == "lrclib", - ), - ("AMLL".to_string(), self.config.lyrics_source == "amll"), - ], - true, - MusicAction::LyricsSource, - ); - page.row_stepper( - tr("lyrics_delay"), - format!("{:.1}", self.config.lyrics_delay), - true, - MusicAction::LyricsDelay, - ); - page.row_switch( - tr("lyrics_scroll"), - self.config.lyrics_scroll, - true, - MusicAction::LyricsScroll, - ); + page.setting(&self.config, AppConfigField::ShowSecondaryLyrics, true); + page.setting(&self.config, AppConfigField::LyricsSource, true); + page.setting(&self.config, AppConfigField::LyricsDelay, true); + page.setting(&self.config, AppConfigField::LyricsScroll, true); if self.config.lyrics_scroll { - page.row_stepper( - tr("lyrics_scroll_max_width"), - (self.config.lyrics_scroll_max_width as i32).to_string(), - true, - MusicAction::LyricsScrollWidth, - ); + page.setting(&self.config, AppConfigField::LyricsScrollMaxWidth, true); } } } if show_lyrics { - page.row_stepper( - tr("lyrics_side_gap"), - (self.config.lyrics_side_gap as i32).to_string(), - true, - MusicAction::LyricsSideGap, - ); - page.row_source( - tr("lyrics_transition_animation"), - lyric_transition_options(self.config.lyrics_transition_animation), + page.setting(&self.config, AppConfigField::LyricsSideGap, true); + page.setting( + &self.config, + AppConfigField::LyricsTransitionAnimation, true, - MusicAction::LyricsTransitionAnimation, ); } page.group_end(); @@ -197,81 +118,14 @@ impl SettingsApp { } let page = self.build_music_page(); let result = input.hit_test(&page); + if self.handle_setting_click(&page, &result, input) { + return; + } let Some(action) = page.action(&result).cloned() else { return; }; - if let ClickResult::StepperValue(item_index) = &result { - let (value, on_commit): (String, NumberInputHandler) = match action { - MusicAction::LyricsDelay => { - (format!("{:.1}", self.config.lyrics_delay), set_lyrics_delay) - } - MusicAction::LyricsScrollWidth => ( - (self.config.lyrics_scroll_max_width as i32).to_string(), - set_lyrics_scroll_width, - ), - MusicAction::LyricsSideGap => ( - (self.config.lyrics_side_gap as i32).to_string(), - set_lyrics_side_gap, - ), - _ => return, - }; - self.begin_number_input( - input.stepper_value_rect(&page, *item_index, self.scroll_y), - value, - on_commit, - ); - return; - } - let changed = match (&action, &result) { - (MusicAction::SmtcEnabled, ClickResult::Switch(_)) => { - self.config.smtc_enabled = !self.config.smtc_enabled; - true - } - (MusicAction::ShowLyrics, ClickResult::Switch(_)) => { - self.config.show_lyrics = !self.config.show_lyrics; - true - } - (MusicAction::ShowSecondaryLyrics, ClickResult::Switch(_)) => { - self.config.show_secondary_lyrics = !self.config.show_secondary_lyrics; - true - } - (MusicAction::LyricsScroll, ClickResult::Switch(_)) => { - self.config.lyrics_scroll = !self.config.lyrics_scroll; - true - } - (MusicAction::LyricsDelay, _) => { - let Some(direction) = result.step_direction() else { - return; - }; - self.config.lyrics_delay = - (step_f64(self.config.lyrics_delay, direction, 0.1, -10.0, 10.0) * 10.0) - .round() - / 10.0; - true - } - (MusicAction::LyricsScrollWidth, _) => { - let Some(direction) = result.step_direction() else { - return; - }; - self.config.lyrics_scroll_max_width = step( - self.config.lyrics_scroll_max_width, - direction, - 10.0, - 100.0, - 500.0, - ); - true - } - (MusicAction::LyricsSideGap, _) => { - let Some(direction) = result.step_direction() else { - return; - }; - self.config.lyrics_side_gap = - step(self.config.lyrics_side_gap, direction, 1.0, 0.0, 32.0); - true - } (MusicAction::LyricsFolder, ClickResult::FolderSelect(_)) => { let Some(path) = rfd::FileDialog::new().pick_folder() else { return; @@ -298,133 +152,6 @@ impl SettingsApp { }; if changed { self.persist_settings_change(); - return; } - - let ClickResult::SourceButton(item_index) = result else { - return; - }; - let (win_w, win_h) = self.logical_window_size(); - let button_rect = input.popup_button_rect(&page, item_index, self.scroll_y); - let popup = match action { - MusicAction::LyricsMode => PopupState::new( - select_lyrics_mode, - button_rect, - vec![tr("lyrics_mode_online"), tr("lyrics_mode_lrc")], - vec!["online".to_string(), "lrc".to_string()], - usize::from(self.config.lyrics_mode == "lrc"), - win_w, - win_h, - ), - MusicAction::LyricsSource => PopupState::new( - select_lyrics_source, - button_rect, - vec![ - tr("lyrics_source_163"), - tr("lyrics_source_qq"), - tr("lyrics_source_kugou"), - tr("lyrics_source_lrclib"), - "AMLL".to_string(), - ], - vec![ - "163".to_string(), - "qq".to_string(), - "kugou".to_string(), - "lrclib".to_string(), - "amll".to_string(), - ], - ["163", "qq", "kugou", "lrclib", "amll"] - .iter() - .position(|source| *source == self.config.lyrics_source) - .unwrap_or_default(), - win_w, - win_h, - ), - MusicAction::LyricsTransitionAnimation => PopupState::new( - select_lyrics_transition_animation, - button_rect, - lyric_transition_labels(), - LyricTransitionMode::ALL - .into_iter() - .map(|mode| mode.to_string()) - .collect(), - LyricTransitionMode::ALL - .iter() - .position(|mode| *mode == self.config.lyrics_transition_animation) - .unwrap_or_default(), - win_w, - win_h, - ), - _ => return, - }; - self.show_popup(popup); - } -} - -fn lyric_transition_labels() -> Vec { - vec![ - tr("lyrics_transition_random"), - tr("lyrics_transition_blur"), - tr("lyrics_transition_slide"), - tr("lyrics_transition_fade"), - ] -} - -fn lyric_transition_options(selected: LyricTransitionMode) -> Vec<(String, bool)> { - lyric_transition_labels() - .into_iter() - .zip(LyricTransitionMode::ALL) - .map(|(label, mode)| (label, mode == selected)) - .collect() -} - -fn step(value: f32, direction: StepDirection, amount: f32, min: f32, max: f32) -> f32 { - match direction { - StepDirection::Decrement => value - amount, - StepDirection::Increment => value + amount, - } - .clamp(min, max) -} - -fn step_f64(value: f64, direction: StepDirection, amount: f64, min: f64, max: f64) -> f64 { - match direction { - StepDirection::Decrement => value - amount, - StepDirection::Increment => value + amount, - } - .clamp(min, max) -} - -fn select_lyrics_source(app: &mut SettingsApp, value: &str) { - app.config.lyrics_source = value.to_string(); -} - -fn select_lyrics_mode(app: &mut SettingsApp, value: &str) { - app.config.lyrics_mode = value.to_string(); - if value == "lrc" { - app.config.show_lyrics = true; - } -} - -fn select_lyrics_transition_animation(app: &mut SettingsApp, value: &str) { - app.config.lyrics_transition_animation = value.to_string().into(); -} - -fn set_lyrics_delay(app: &mut SettingsApp, value: &str) { - if let Ok(value) = value.parse::() { - app.config.lyrics_delay = value.clamp(-10.0, 10.0); - } -} - -fn set_lyrics_scroll_width(app: &mut SettingsApp, value: &str) { - if let Ok(value) = value.parse::() { - app.config.lyrics_scroll_max_width = value.clamp(100.0, 500.0); - } -} - -fn set_lyrics_side_gap(app: &mut SettingsApp, value: &str) { - if let Ok(value) = value.parse::() - && value.is_finite() - { - app.config.lyrics_side_gap = value.round().clamp(0.0, 32.0); } } diff --git a/src/window/settings/pages/schema.rs b/src/window/settings/pages/schema.rs new file mode 100644 index 00000000..01d2b4b1 --- /dev/null +++ b/src/window/settings/pages/schema.rs @@ -0,0 +1,123 @@ +use settings_schema::{Kind, Settings, Value}; +use winisland_core::config::{AppConfig, AppConfigField}; +use winisland_core::i18n::tr; + +use crate::utils::settings_ui::ClickResult; +use crate::utils::settings_ui::items::SettingsItem; +use crate::window::settings::{PopupState, SettingsApp}; + +use super::{PageInput, SettingsPage}; + +impl SettingsPage { + pub(crate) fn setting(&mut self, config: &AppConfig, field: AppConfigField, enabled: bool) { + let schema = AppConfig::schema(field); + let label = tr(schema.label); + let item = match (&schema.kind, config.get(field)) { + (Kind::Toggle, Value::Bool(on)) => SettingsItem::RowSwitch { label, on, enabled }, + (Kind::Number(number), Value::Number(value)) => SettingsItem::RowStepper { + label, + value: number.format(value), + enabled, + }, + (Kind::Choice(choices), Value::Text(value)) => SettingsItem::RowSourceSelect { + label, + options: choices + .iter() + .map(|choice| (tr(choice.label), choice.value == value)) + .collect(), + enabled, + }, + _ => return, + }; + self.push_setting(item, field); + } +} + +impl SettingsApp { + pub(crate) fn handle_setting_click( + &mut self, + page: &SettingsPage, + result: &ClickResult, + input: PageInput, + ) -> bool { + let Some(field) = page.setting_field(result) else { + return false; + }; + match ( + &AppConfig::schema(field).kind, + self.config.get(field), + result, + ) { + (Kind::Toggle, Value::Bool(on), ClickResult::Switch(_)) => { + self.update_setting(field, Value::Bool(!on)); + } + (Kind::Number(number), Value::Number(value), ClickResult::StepperDec(_)) => { + self.update_setting(field, Value::Number(number.stepped(value, -1))); + } + (Kind::Number(number), Value::Number(value), ClickResult::StepperInc(_)) => { + self.update_setting(field, Value::Number(number.stepped(value, 1))); + } + (Kind::Number(number), Value::Number(value), ClickResult::StepperValue(index)) => { + self.pending_setting = Some(field); + let rect = input.stepper_value_rect(page, *index, self.scroll_y); + self.begin_number_input(rect, number.format(value), commit_setting_number); + } + (Kind::Choice(choices), Value::Text(value), ClickResult::SourceButton(index)) => { + self.pending_setting = Some(field); + let rect = input.popup_button_rect(page, *index, self.scroll_y); + let (width, height) = self.logical_window_size(); + self.show_popup(PopupState::new( + select_setting_choice, + rect, + choices.iter().map(|choice| tr(choice.label)).collect(), + choices + .iter() + .map(|choice| choice.value.to_string()) + .collect(), + choices + .iter() + .position(|choice| choice.value == value) + .unwrap_or_default(), + width, + height, + )); + } + _ => {} + } + true + } + + fn update_setting(&mut self, field: AppConfigField, value: Value) { + if self.apply_setting(field, value) { + self.persist_settings_change(); + } + } + + fn apply_setting(&mut self, field: AppConfigField, value: Value) -> bool { + if !self.config.set(field, value) { + return false; + } + match field { + AppConfigField::SettingsTheme => self.update_theme(), + AppConfigField::LyricsMode if self.config.lyrics_mode == "lrc" => { + self.config.show_lyrics = true; + } + _ => {} + } + true + } +} + +fn select_setting_choice(app: &mut SettingsApp, value: &str) { + if let Some(field) = app.pending_setting.take() { + app.apply_setting(field, Value::Text(value.to_string())); + } +} + +fn commit_setting_number(app: &mut SettingsApp, value: &str) { + if let Some(field) = app.pending_setting.take() + && let Ok(value) = value.parse() + { + app.apply_setting(field, Value::Number(value)); + } +} From 4ad02ce17b8f6eaa3d5b503a9f3230c7c43c620f Mon Sep 17 00:00:00 2001 From: Eatgrapes Date: Sat, 26 Sep 2026 23:26:37 +0800 Subject: [PATCH 07/13] refactor(platform): add PlatformError::backend for error mapping The Windows backend wrapped 42 foreign errors with the same closure, |error| PlatformError::Backend(error.to_string()). A From impl cannot cover them: winisland-platform takes no dependencies, the orphan rule keeps the impl out of winisland-platform-windows, and a blanket impl would overlap From for T. A generic constructor lets every site use map_err(PlatformError::backend) instead. --- .../src/audio/capture.rs | 18 +++++++-------- .../winisland-platform-windows/src/display.rs | 2 +- .../winisland-platform-windows/src/input.rs | 5 ++--- .../src/media/mod.rs | 22 ++++++------------- .../winisland-platform-windows/src/metrics.rs | 2 +- .../src/shell/activate.rs | 2 +- .../src/shell/autostart.rs | 10 ++++----- .../src/shell/instance.rs | 2 +- .../src/shell/mod.rs | 12 +++++----- .../src/shell/tray.rs | 21 +++++++----------- .../src/shell/update.rs | 2 +- .../src/window/loop.rs | 6 ++--- .../src/window/mod.rs | 8 +++---- crates/winisland-platform/src/error.rs | 6 +++++ 14 files changed, 51 insertions(+), 67 deletions(-) diff --git a/crates/winisland-platform-windows/src/audio/capture.rs b/crates/winisland-platform-windows/src/audio/capture.rs index c9d3768e..99f1e7aa 100644 --- a/crates/winisland-platform-windows/src/audio/capture.rs +++ b/crates/winisland-platform-windows/src/audio/capture.rs @@ -136,7 +136,7 @@ impl WindowsProcessCapture { None, ); let mut client = AudioClient::new_application_loopback_client(process_id, true) - .map_err(|error| PlatformError::Backend(error.to_string()))?; + .map_err(PlatformError::backend)?; client .initialize_client( &format, @@ -146,16 +146,14 @@ impl WindowsProcessCapture { buffer_duration_hns: 0, }, ) - .map_err(|error| PlatformError::Backend(error.to_string()))?; + .map_err(PlatformError::backend)?; let event = client .set_get_eventhandle() - .map_err(|error| PlatformError::Backend(error.to_string()))?; + .map_err(PlatformError::backend)?; let capture = client .get_audiocaptureclient() - .map_err(|error| PlatformError::Backend(error.to_string()))?; - client - .start_stream() - .map_err(|error| PlatformError::Backend(error.to_string()))?; + .map_err(PlatformError::backend)?; + client.start_stream().map_err(PlatformError::backend)?; Ok(Self { client, capture, @@ -174,19 +172,19 @@ impl ProcessCapture for WindowsProcessCapture { while let Some(frame_count) = self .capture .get_next_packet_size() - .map_err(|error| PlatformError::Backend(error.to_string()))? + .map_err(PlatformError::backend)? .filter(|count| *count > 0) { self.bytes.resize(frame_count as usize * BYTES_PER_FRAME, 0); let (frames_read, _) = self .capture .read_from_device(&mut self.bytes) - .map_err(|error| PlatformError::Backend(error.to_string()))?; + .map_err(PlatformError::backend)?; captured = true; let pending = self .capture .get_next_packet_size() - .map_err(|error| PlatformError::Backend(error.to_string()))? + .map_err(PlatformError::backend)? .is_some_and(|count| count > 0); if !pending { samples.clear(); diff --git a/crates/winisland-platform-windows/src/display.rs b/crates/winisland-platform-windows/src/display.rs index f7b1712f..31c0cad6 100644 --- a/crates/winisland-platform-windows/src/display.rs +++ b/crates/winisland-platform-windows/src/display.rs @@ -43,7 +43,7 @@ impl DisplayProvider for WindowsDisplay { ) } .ok() - .map_err(|error| PlatformError::Backend(error.to_string()))?; + .map_err(PlatformError::backend)?; Ok(monitors) } diff --git a/crates/winisland-platform-windows/src/input.rs b/crates/winisland-platform-windows/src/input.rs index cee62e47..e331be2d 100644 --- a/crates/winisland-platform-windows/src/input.rs +++ b/crates/winisland-platform-windows/src/input.rs @@ -43,15 +43,14 @@ impl InputHooks for WindowsInput { if slot.borrow().is_none() { // SAFETY: The current module contains the static callback; the caller pumps messages. let hook = unsafe { - let module = GetModuleHandleW(None) - .map_err(|error| PlatformError::Backend(error.to_string()))?; + let module = GetModuleHandleW(None).map_err(PlatformError::backend)?; SetWindowsHookExW( WH_KEYBOARD_LL, Some(volume_keyboard_hook), Some(HINSTANCE(module.0)), 0, ) - .map_err(|error| PlatformError::Backend(error.to_string()))? + .map_err(PlatformError::backend)? }; *slot.borrow_mut() = Some(HookGuard(hook)); log::info!("Volume keys are handled by WinIsland"); diff --git a/crates/winisland-platform-windows/src/media/mod.rs b/crates/winisland-platform-windows/src/media/mod.rs index 46136bef..deceb4d2 100644 --- a/crates/winisland-platform-windows/src/media/mod.rs +++ b/crates/winisland-platform-windows/src/media/mod.rs @@ -22,8 +22,7 @@ struct WinRtGuard; impl WinRtGuard { fn new() -> Result { // SAFETY: Each successful initialization is balanced by this guard on the same thread. - unsafe { RoInitialize(RO_INIT_MULTITHREADED) } - .map_err(|error| PlatformError::Backend(error.to_string()))?; + unsafe { RoInitialize(RO_INIT_MULTITHREADED) }.map_err(PlatformError::backend)?; Ok(Self) } } @@ -47,7 +46,7 @@ impl WindowsMediaContext { let guard = WinRtGuard::new()?; let manager = GlobalSystemMediaTransportControlsSessionManager::RequestAsync() .and_then(|operation| operation.join()) - .map_err(|error| PlatformError::Backend(error.to_string()))?; + .map_err(PlatformError::backend)?; let (event_tx, event_rx) = mpsc::sync_channel(1); let handler = TypedEventHandler::new(move |_manager, _args| { let _ = event_tx.try_send(()); @@ -131,13 +130,8 @@ impl MediaSessionHandle for WindowsMediaSession { } fn playback(&self) -> Result { - let info = self - .0 - .GetPlaybackInfo() - .map_err(|error| PlatformError::Backend(error.to_string()))?; - let status = info - .PlaybackStatus() - .map_err(|error| PlatformError::Backend(error.to_string()))?; + let info = self.0.GetPlaybackInfo().map_err(PlatformError::backend)?; + let status = info.PlaybackStatus().map_err(PlatformError::backend)?; Ok(status == GlobalSystemMediaTransportControlsSessionPlaybackStatus::Playing) } @@ -146,7 +140,7 @@ impl MediaSessionHandle for WindowsMediaSession { .0 .TryGetMediaPropertiesAsync() .and_then(|operation| operation.join()) - .map_err(|error| PlatformError::Backend(error.to_string()))?; + .map_err(PlatformError::backend)?; Ok(read_track(&props)) } @@ -190,12 +184,10 @@ impl MediaSessionHandle for WindowsMediaSession { .0 .TryChangePlaybackPositionAsync(ticks) .and_then(|operation| operation.join()) - .map_err(|error| PlatformError::Backend(error.to_string())); + .map_err(PlatformError::backend); } }; - operation - .map(|_| true) - .map_err(|error| PlatformError::Backend(error.to_string())) + operation.map(|_| true).map_err(PlatformError::backend) } fn thumbnail(&self, expected_title: &str) -> Result, ThumbnailError> { diff --git a/crates/winisland-platform-windows/src/metrics.rs b/crates/winisland-platform-windows/src/metrics.rs index c19a0587..43ad5d48 100644 --- a/crates/winisland-platform-windows/src/metrics.rs +++ b/crates/winisland-platform-windows/src/metrics.rs @@ -64,7 +64,7 @@ impl SystemMetrics for WindowsMetrics { fn trim_working_set(&self) -> Result<(), PlatformError> { // SAFETY: GetCurrentProcess returns a valid pseudo-handle; maximum limits request a trim. unsafe { SetProcessWorkingSetSize(GetCurrentProcess(), usize::MAX, usize::MAX) } - .map_err(|error| PlatformError::Backend(error.to_string())) + .map_err(PlatformError::backend) } } diff --git a/crates/winisland-platform-windows/src/shell/activate.rs b/crates/winisland-platform-windows/src/shell/activate.rs index 5880e3af..f04d1bf0 100644 --- a/crates/winisland-platform-windows/src/shell/activate.rs +++ b/crates/winisland-platform-windows/src/shell/activate.rs @@ -33,7 +33,7 @@ fn activate_on_sta(app_user_model_id: &str) -> Result { let result = unsafe { let manager: IApplicationActivationManager = CoCreateInstance(&ApplicationActivationManager, None, CLSCTX_LOCAL_SERVER) - .map_err(|error| PlatformError::Backend(error.to_string()))?; + .map_err(PlatformError::backend)?; manager.ActivateApplication(&app_id, PCWSTR::null(), ACTIVATEOPTIONS::default()) }; match result { diff --git a/crates/winisland-platform-windows/src/shell/autostart.rs b/crates/winisland-platform-windows/src/shell/autostart.rs index b206b619..35493db5 100644 --- a/crates/winisland-platform-windows/src/shell/autostart.rs +++ b/crates/winisland-platform-windows/src/shell/autostart.rs @@ -8,24 +8,22 @@ const RUN_KEY: &str = "Software\\Microsoft\\Windows\\CurrentVersion\\Run"; const VALUE_NAME: &str = "WinIsland"; pub(super) fn enabled() -> Result { - let key = CURRENT_USER - .open(RUN_KEY) - .map_err(|error| PlatformError::Backend(error.to_string()))?; + let key = CURRENT_USER.open(RUN_KEY).map_err(PlatformError::backend)?; Ok(key.get_type(VALUE_NAME).is_ok()) } pub(super) fn set(enabled: bool) -> Result<(), PlatformError> { let key = CURRENT_USER .create(RUN_KEY) - .map_err(|error| PlatformError::Backend(error.to_string()))?; + .map_err(PlatformError::backend)?; let result = if enabled { - let exe = env::current_exe().map_err(|error| PlatformError::Backend(error.to_string()))?; + let exe = env::current_exe().map_err(PlatformError::backend)?; key.set_string(VALUE_NAME, format!("\"{}\"", exe.display())) } else { key.remove_value(VALUE_NAME) }; match result { Err(error) if !enabled && error.code() == ERROR_FILE_NOT_FOUND.to_hresult() => Ok(()), - result => result.map_err(|error| PlatformError::Backend(error.to_string())), + result => result.map_err(PlatformError::backend), } } diff --git a/crates/winisland-platform-windows/src/shell/instance.rs b/crates/winisland-platform-windows/src/shell/instance.rs index 5d785502..06cba9ec 100644 --- a/crates/winisland-platform-windows/src/shell/instance.rs +++ b/crates/winisland-platform-windows/src/shell/instance.rs @@ -13,7 +13,7 @@ pub(super) fn acquire(key: &str) -> Result>, Platfo let key = HSTRING::from(key); // SAFETY: The key outlives the call, and the new mutex handle is owned by the guard. let mutex = unsafe { CreateMutexW(None, true, &key).map(|mutex| Owned::new(mutex)) } - .map_err(|error| PlatformError::Backend(error.to_string()))?; + .map_err(PlatformError::backend)?; // SAFETY: GetLastError is read on the same thread immediately after CreateMutexW. if unsafe { GetLastError() } == ERROR_ALREADY_EXISTS { Ok(None) diff --git a/crates/winisland-platform-windows/src/shell/mod.rs b/crates/winisland-platform-windows/src/shell/mod.rs index b36880cd..eb699ff9 100644 --- a/crates/winisland-platform-windows/src/shell/mod.rs +++ b/crates/winisland-platform-windows/src/shell/mod.rs @@ -71,13 +71,11 @@ impl ShellIntegration for WindowsShell { instance::acquire(key) } fn restart(&self, args: &[String]) -> Result<(), PlatformError> { - std::process::Command::new( - std::env::current_exe().map_err(|error| PlatformError::Backend(error.to_string()))?, - ) - .args(args) - .spawn() - .map(|_| ()) - .map_err(|error| PlatformError::Backend(error.to_string())) + std::process::Command::new(std::env::current_exe().map_err(PlatformError::backend)?) + .args(args) + .spawn() + .map(|_| ()) + .map_err(PlatformError::backend) } fn fatal_dialog(&self, title: &str, body: &str) { show_dialog(title, body, MB_ICONERROR | MB_SETFOREGROUND | MB_TOPMOST); diff --git a/crates/winisland-platform-windows/src/shell/tray.rs b/crates/winisland-platform-windows/src/shell/tray.rs index fe9941dc..faea60ec 100644 --- a/crates/winisland-platform-windows/src/shell/tray.rs +++ b/crates/winisland-platform-windows/src/shell/tray.rs @@ -23,11 +23,10 @@ fn icon(theme: TrayTheme) -> Result { TrayTheme::Dark => include_bytes!("../../../../resources/icon.png"), }; let rgba = image::load_from_memory(bytes) - .map_err(|error| PlatformError::Backend(error.to_string()))? + .map_err(PlatformError::backend)? .to_rgba8(); let (width, height) = rgba.dimensions(); - Icon::from_rgba(rgba.into_raw(), width, height) - .map_err(|error| PlatformError::Backend(error.to_string())) + Icon::from_rgba(rgba.into_raw(), width, height).map_err(PlatformError::backend) } pub(super) fn install(theme: TrayTheme, labels: TrayLabels) -> Result<(), PlatformError> { @@ -36,20 +35,16 @@ pub(super) fn install(theme: TrayTheme, labels: TrayLabels) -> Result<(), Platfo let settings = MenuItem::new(&labels.settings, true, None); let restart = MenuItem::new(&labels.restart, true, None); let exit = MenuItem::new(&labels.exit, true, None); - menu.append(&toggle) - .map_err(|error| PlatformError::Backend(error.to_string()))?; - menu.append(&settings) - .map_err(|error| PlatformError::Backend(error.to_string()))?; - menu.append(&restart) - .map_err(|error| PlatformError::Backend(error.to_string()))?; - menu.append(&exit) - .map_err(|error| PlatformError::Backend(error.to_string()))?; + menu.append(&toggle).map_err(PlatformError::backend)?; + menu.append(&settings).map_err(PlatformError::backend)?; + menu.append(&restart).map_err(PlatformError::backend)?; + menu.append(&exit).map_err(PlatformError::backend)?; let tray = TrayIconBuilder::new() .with_tooltip(&labels.tooltip) .with_menu(Box::new(menu)) .with_icon(icon(theme)?) .build() - .map_err(|error| PlatformError::Backend(error.to_string()))?; + .map_err(PlatformError::backend)?; TRAY.with(|slot| { *slot.borrow_mut() = Some(WindowsTray { tray, @@ -70,7 +65,7 @@ pub(super) fn update(theme: TrayTheme, labels: TrayLabels) -> Result<(), Platfor if tray.theme != theme { tray.tray .set_icon(Some(icon(theme)?)) - .map_err(|error| PlatformError::Backend(error.to_string()))?; + .map_err(PlatformError::backend)?; tray.theme = theme; } tray.toggle.set_text(&labels.toggle); diff --git a/crates/winisland-platform-windows/src/shell/update.rs b/crates/winisland-platform-windows/src/shell/update.rs index 015bc9f3..7621ebec 100644 --- a/crates/winisland-platform-windows/src/shell/update.rs +++ b/crates/winisland-platform-windows/src/shell/update.rs @@ -22,5 +22,5 @@ pub(super) fn install(package: &Path) -> Result<(), PlatformError> { .args(["-WindowStyle", "Hidden", "-Command", &script]) .spawn() .map(|_| ()) - .map_err(|error| PlatformError::Backend(error.to_string())) + .map_err(PlatformError::backend) } diff --git a/crates/winisland-platform-windows/src/window/loop.rs b/crates/winisland-platform-windows/src/window/loop.rs index a6bfe8d1..1ca02c52 100644 --- a/crates/winisland-platform-windows/src/window/loop.rs +++ b/crates/winisland-platform-windows/src/window/loop.rs @@ -51,13 +51,11 @@ pub(super) fn run(handler: &mut dyn AppHandler) -> Result<(), PlatformError> { } false }); - let event_loop = builder - .build() - .map_err(|error| PlatformError::Backend(error.to_string()))?; + let event_loop = builder.build().map_err(PlatformError::backend)?; let _ = PROXY.set(event_loop.create_proxy()); event_loop .run_app(&mut EventAdapter { handler }) - .map_err(|error| PlatformError::Backend(error.to_string())) + .map_err(PlatformError::backend) } pub(super) fn exit() { diff --git a/crates/winisland-platform-windows/src/window/mod.rs b/crates/winisland-platform-windows/src/window/mod.rs index 3732506a..0ae5289d 100644 --- a/crates/winisland-platform-windows/src/window/mod.rs +++ b/crates/winisland-platform-windows/src/window/mod.rs @@ -190,7 +190,7 @@ impl WindowSystem for WindowsWindowSystem { let backdrop = Arc::new( event_loop .create_window(attrs::backdrop_attributes()) - .map_err(|error| PlatformError::Backend(error.to_string()))?, + .map_err(PlatformError::backend)?, ); let _ = backdrop.set_cursor_hittest(false); let owner = window_hwnd(&backdrop) @@ -199,7 +199,7 @@ impl WindowSystem for WindowsWindowSystem { let window = Arc::new( event_loop .create_window(attrs::overlay_attributes(spec, owner)) - .map_err(|error| PlatformError::Backend(error.to_string()))?, + .map_err(PlatformError::backend)?, ); let id = WindowId(u64::from(window.id())); WINDOWS.with(|windows| { @@ -227,7 +227,7 @@ impl WindowSystem for WindowsWindowSystem { let window = Arc::new( event_loop .create_window(attrs::settings_attributes(spec, monitor.as_ref())) - .map_err(|error| PlatformError::Backend(error.to_string()))?, + .map_err(PlatformError::backend)?, ); let id = WindowId(u64::from(window.id())); WINDOWS.with(|windows| { @@ -495,7 +495,7 @@ impl WindowSystem for WindowsWindowSystem { }; if let Err(error) = host_backdrop.update(params) { self.release_host_backdrop(id); - return Err(PlatformError::Backend(error.to_string())); + return Err(PlatformError::backend(error)); } Ok(true) } diff --git a/crates/winisland-platform/src/error.rs b/crates/winisland-platform/src/error.rs index 9f929c02..3205941c 100644 --- a/crates/winisland-platform/src/error.rs +++ b/crates/winisland-platform/src/error.rs @@ -7,6 +7,12 @@ pub enum PlatformError { Backend(String), } +impl PlatformError { + pub fn backend(error: impl fmt::Display) -> Self { + Self::Backend(error.to_string()) + } +} + impl fmt::Display for PlatformError { fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { match self { From dcd8aec0ecb0569b73c54a0198cf54a6d0612d9c Mon Sep 17 00:00:00 2001 From: Eatgrapes Date: Sat, 26 Sep 2026 23:51:29 +0800 Subject: [PATCH 08/13] refactor: move the remaining locks to parking_lot The i18n catalogue, the file logger and pollkit's cancel slot still used std locks with poison handling. Release builds use panic = abort, so a poisoned lock is never observed there and those branches never ran. The i18n bundle functions keep returning Result so their callers compile unchanged. The v1 plugin manager keeps its std mutex because the plugin ABI v2 work replaces that file. --- Cargo.lock | 2 ++ crates/pollkit/Cargo.toml | 1 + crates/pollkit/src/lib.rs | 8 +++++--- crates/winisland-core/Cargo.toml | 1 + crates/winisland-core/src/i18n/mod.rs | 23 ++++++++++++----------- src/utils/logger.rs | 12 ++++-------- 6 files changed, 25 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8989c334..846853da 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3088,6 +3088,7 @@ name = "pollkit" version = "0.1.0" dependencies = [ "log", + "parking_lot", ] [[package]] @@ -5292,6 +5293,7 @@ dependencies = [ "fuzzengine", "log", "lrc", + "parking_lot", "quick-xml", "reqwest", "serde", diff --git a/crates/pollkit/Cargo.toml b/crates/pollkit/Cargo.toml index e39a9bdd..c695fb73 100644 --- a/crates/pollkit/Cargo.toml +++ b/crates/pollkit/Cargo.toml @@ -8,3 +8,4 @@ license = "MIT" [dependencies] log = "0.4" +parking_lot = "0.12" diff --git a/crates/pollkit/src/lib.rs b/crates/pollkit/src/lib.rs index 776295d1..093879e4 100644 --- a/crates/pollkit/src/lib.rs +++ b/crates/pollkit/src/lib.rs @@ -4,9 +4,11 @@ use std::fmt; use std::sync::mpsc::{self, Receiver, TryRecvError}; -use std::sync::{Arc, Mutex, OnceLock, PoisonError}; +use std::sync::{Arc, OnceLock}; use std::time::{Duration, Instant}; +use parking_lot::Mutex; + static WAKE_HOOK: OnceLock = OnceLock::new(); /// Registers a function that runs on the worker thread whenever a job finishes, @@ -26,11 +28,11 @@ struct CancelSlot(Mutex>>); impl CancelSlot { fn set(&self, action: impl FnOnce() + Send + 'static) { - *self.0.lock().unwrap_or_else(PoisonError::into_inner) = Some(Box::new(action)); + *self.0.lock() = Some(Box::new(action)); } fn fire(&self) { - let action = self.0.lock().unwrap_or_else(PoisonError::into_inner).take(); + let action = self.0.lock().take(); if let Some(action) = action { action(); } diff --git a/crates/winisland-core/Cargo.toml b/crates/winisland-core/Cargo.toml index b1473f61..6681cacb 100644 --- a/crates/winisland-core/Cargo.toml +++ b/crates/winisland-core/Cargo.toml @@ -10,6 +10,7 @@ encoding_rs = "0.8" fuzzengine = "0.2.1" log = { version = "0.4", features = ["std"] } lrc = "0.2.0" +parking_lot = "0.12" quick-xml = "0.41.0" reqwest = { version = "0.13.4", features = ["json"] } serde = { version = "1.0", features = ["derive"] } diff --git a/crates/winisland-core/src/i18n/mod.rs b/crates/winisland-core/src/i18n/mod.rs index 3ab4775c..8fba96fb 100644 --- a/crates/winisland-core/src/i18n/mod.rs +++ b/crates/winisland-core/src/i18n/mod.rs @@ -1,6 +1,8 @@ use std::collections::{BTreeMap, HashMap}; use std::path::Path; -use std::sync::{Arc, LazyLock, OnceLock, RwLock}; +use std::sync::{Arc, LazyLock, OnceLock}; + +use parking_lot::RwLock; #[derive(Clone, Debug)] pub struct Language { @@ -222,28 +224,28 @@ pub fn init_i18n(config_lang: &str) { } else { config_lang.to_string() }; - I18N.write().unwrap().load(&target_lang); + I18N.write().load(&target_lang); } pub fn set_lang(lang: &str) { - I18N.write().unwrap().load(lang); + I18N.write().load(lang); } pub fn current_lang() -> String { - I18N.read().unwrap().current_lang.clone() + I18N.read().current_lang.clone() } pub fn tr(key: &str) -> String { - I18N.read().unwrap().get(key) + I18N.read().get(key) } pub fn tr_args(key: &str, args: &[&str]) -> String { - let template = I18N.read().unwrap().get(key); + let template = I18N.read().get(key); format_args(&template, args) } pub fn available_langs() -> Vec { - I18N.read().unwrap().available_languages.clone() + I18N.read().available_languages.clone() } pub fn register_plugin_translation_bundle( @@ -251,14 +253,13 @@ pub fn register_plugin_translation_bundle( language: String, pairs: Vec<(String, String)>, ) -> Result<(), &'static str> { - let mut i18n = I18N.write().map_err(|_| "i18n lock is poisoned")?; - i18n.register_plugin_translation_bundle(id, language, pairs); + I18N.write() + .register_plugin_translation_bundle(id, language, pairs); Ok(()) } pub fn release_plugin_translation_bundle(id: u64) -> Result<(), &'static str> { - let mut i18n = I18N.write().map_err(|_| "i18n lock is poisoned")?; - i18n.release_plugin_translation_bundle(id); + I18N.write().release_plugin_translation_bundle(id); Ok(()) } diff --git a/src/utils/logger.rs b/src/utils/logger.rs index 9f0a734f..94bed92d 100644 --- a/src/utils/logger.rs +++ b/src/utils/logger.rs @@ -1,9 +1,9 @@ use log::{Level, LevelFilter, Log, Metadata, Record, SetLoggerError}; +use parking_lot::Mutex; use std::fs::{self, File, OpenOptions}; use std::io::{BufWriter, Write}; use std::panic::{self, PanicHookInfo}; use std::path::{Path, PathBuf}; -use std::sync::Mutex; const LOG_FILE: &str = "winisland.log"; const MAX_LOG_SIZE: u64 = 1_024_000; // 1MB @@ -89,18 +89,14 @@ impl Log for FileLogger { record.target(), record.args() ); - if let Ok(mut state) = self.state.lock() - && state.write(msg.as_bytes()).is_ok() - && record.level() <= Level::Warn - { + let mut state = self.state.lock(); + if state.write(msg.as_bytes()).is_ok() && record.level() <= Level::Warn { let _ = state.flush(); } } fn flush(&self) { - if let Ok(mut state) = self.state.lock() { - let _ = state.flush(); - } + let _ = self.state.lock().flush(); } } From c72eafef10dd15412b1a506b910280035f480963 Mon Sep 17 00:00:00 2001 From: Eatgrapes Date: Sun, 27 Sep 2026 01:09:56 +0800 Subject: [PATCH 09/13] refactor(core): declare config defaults once with educe Every AppConfig default was written up to three times: in a default_* function, in a #[serde(default = ..)] attribute and in the hand-written Default impl. Each value is now declared once with #[educe(Default = ..)], and the struct-level #[serde(default)] fills missing keys from it. A config file that lacks a key now keeps its other values instead of being replaced by defaults. config_version keeps its field-level default of 0 so old files still migrate. --- Cargo.lock | 1 + crates/winisland-core/Cargo.toml | 1 + crates/winisland-core/src/config/mod.rs | 184 ++++---------------- crates/winisland-core/src/config/widgets.rs | 6 +- 4 files changed, 44 insertions(+), 148 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 846853da..7c31b186 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5289,6 +5289,7 @@ name = "winisland-core" version = "1.3.9" dependencies = [ "base64 0.23.1", + "educe", "encoding_rs", "fuzzengine", "log", diff --git a/crates/winisland-core/Cargo.toml b/crates/winisland-core/Cargo.toml index 6681cacb..4d445d3e 100644 --- a/crates/winisland-core/Cargo.toml +++ b/crates/winisland-core/Cargo.toml @@ -6,6 +6,7 @@ publish = false [dependencies] base64 = "0.23.1" +educe = { version = "0.7", default-features = false, features = ["Default"] } encoding_rs = "0.8" fuzzengine = "0.2.1" log = { version = "0.4", features = ["std"] } diff --git a/crates/winisland-core/src/config/mod.rs b/crates/winisland-core/src/config/mod.rs index aa87cda4..b134c1a6 100644 --- a/crates/winisland-core/src/config/mod.rs +++ b/crates/winisland-core/src/config/mod.rs @@ -4,6 +4,7 @@ mod visual; mod widget_layout; mod widgets; +use educe::Educe; use serde::{Deserialize, Serialize}; use settings_schema::Settings; @@ -18,25 +19,33 @@ pub const APP_AUTHOR: &str = "Eatgrapes"; pub const APP_HOMEPAGE: &str = "https://github.com/WinIslandProject/WinIsland"; pub const WINDOW_TITLE: &str = "WinIsland"; -#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Settings)] +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Educe, Settings)] +#[educe(Default)] +#[serde(default)] pub struct AppConfig { #[serde(alias = "global_scale")] + #[educe(Default = 1.0)] #[setting(number(min = 0.5, max = 5.0, step = 0.05, precision = 2))] pub compact_scale: f32, - #[serde(default = "default_expanded_scale")] + #[educe(Default = 1.0)] #[setting(number(min = 0.5, max = 5.0, step = 0.05, precision = 2))] pub expanded_scale: f32, + #[educe(Default = 120.0)] #[setting(number(min = 40.0, max = 400.0, step = 5.0))] pub base_width: f32, + #[educe(Default = 27.0)] #[setting(number(min = 15.0, max = 200.0, step = 2.0))] pub base_height: f32, + #[educe(Default = 360.0)] #[setting(number(min = 200.0, max = 2000.0, step = 10.0))] pub expanded_width: f32, + #[educe(Default = 200.0)] #[setting(number(min = 100.0, max = 1000.0, step = 10.0))] pub expanded_height: f32, + #[educe(Default = true)] #[setting(toggle)] pub motion_blur: bool, - #[serde(default = "default_animation_fps")] + #[educe(Default = 90)] #[setting(choice( 30 => "30 FPS", 60 => "60 FPS", @@ -45,54 +54,47 @@ pub struct AppConfig { 0 => "frame_rate_native", ))] pub animation_fps: u32, - #[serde(default = "default_expanded_idle_fps")] + #[educe(Default = 60)] #[setting(choice(30 => "30 FPS", 45 => "45 FPS", 60 => "60 FPS", 90 => "90 FPS"))] pub expanded_idle_fps: u32, - #[serde(default = "default_island_style")] + #[educe(Default = "default")] pub island_style: String, + #[educe(Default = true)] #[setting(toggle, label = "smtc_control")] pub smtc_enabled: bool, - #[serde(default)] pub music_notice_acknowledged: bool, pub smtc_apps: Vec, - #[serde(default)] pub smtc_known_apps: Vec, - #[serde(default = "default_show_lyrics")] + #[educe(Default = true)] #[setting(toggle)] pub show_lyrics: bool, - #[serde(default)] #[setting(toggle)] pub show_secondary_lyrics: bool, - #[serde(default = "default_lyrics_mode")] + #[educe(Default = "online")] #[setting(choice("online" => "lyrics_mode_online", "lrc" => "lyrics_mode_lrc"))] pub lyrics_mode: String, - #[serde(default)] pub lyrics_local_dir: Option, - #[serde(default)] pub custom_font_path: Option, - #[serde(default)] pub auto_start: bool, - #[serde(default)] #[setting(toggle)] pub auto_hide: bool, - #[serde(default)] #[setting(toggle)] pub fullscreen_auto_hide: bool, - #[serde(default = "default_auto_hide_delay")] + #[educe(Default = 5.0)] #[setting(number(min = 1.0, max = 60.0, step = 1.0), label = "hide_delay")] pub auto_hide_delay: f32, - #[serde(default = "default_hidden_width")] + #[educe(Default = 5.0)] #[setting(number(min = MIN_HIDDEN_WIDTH, max = MAX_HIDDEN_WIDTH, step = 1.0))] pub hidden_width: f32, - #[serde(default = "default_check_for_updates")] + #[educe(Default = true)] #[setting(toggle, label = "check_updates")] pub check_for_updates: bool, - #[serde(default = "default_update_check_interval")] + #[educe(Default = 4.0)] #[setting(number(min = 1.0, max = 24.0, step = 1.0), label = "update_interval")] pub update_check_interval: f32, - #[serde(default = "default_language")] + #[educe(Default = "auto")] pub language: String, - #[serde(default = "default_lyrics_source")] + #[educe(Default = "163")] #[setting(choice( "163" => "lyrics_source_163", "qq" => "lyrics_source_qq", @@ -101,19 +103,16 @@ pub struct AppConfig { "amll" => "AMLL", ))] pub lyrics_source: String, - #[serde(default)] #[setting(number(min = -10.0, max = 10.0, step = 0.1, precision = 1))] pub lyrics_delay: f64, - #[serde(default)] #[setting(toggle)] pub lyrics_scroll: bool, - #[serde(default = "default_lyrics_scroll_max_width")] + #[educe(Default = 300.0)] #[setting(number(min = 100.0, max = 500.0, step = 10.0))] pub lyrics_scroll_max_width: f32, - #[serde(default = "default_lyrics_side_gap")] + #[educe(Default = 6.0)] #[setting(number(min = 0.0, max = 32.0, step = 1.0))] pub lyrics_side_gap: f32, - #[serde(default)] #[setting(choice( "random" => "lyrics_transition_random", "blur" => "lyrics_transition_blur", @@ -121,156 +120,49 @@ pub struct AppConfig { "fade" => "lyrics_transition_fade", ))] pub lyrics_transition_animation: LyricTransitionMode, - #[serde(default)] #[setting(number(step = 5.0))] pub position_x_offset: i32, - #[serde(default)] #[setting(number(step = 5.0))] pub position_y_offset: i32, - #[serde( - rename = "dock_position", - default, - skip_serializing_if = "Option::is_none" - )] + #[serde(rename = "dock_position", skip_serializing_if = "Option::is_none")] pub legacy_dock_position: Option, - #[serde(default)] pub monitor_index: i32, - #[serde(default)] #[setting(number(min = 0.0, max = 30.0, step = 1.0))] pub font_size: f32, - #[serde(default = "default_settings_theme")] + #[educe(Default = "system")] #[setting(choice("system" => "theme_system", "light" => "theme_light", "dark" => "theme_dark"))] pub settings_theme: String, - #[serde(default = "default_mini_cover_shape")] + #[educe(Default = "square")] pub mini_cover_shape: String, - #[serde(default = "default_expanded_cover_shape")] + #[educe(Default = "square")] pub expanded_cover_shape: String, - #[serde(default = "default_cover_rotate")] pub cover_rotate: bool, - #[serde(default = "default_update_channel")] + #[educe(Default = "stable")] #[setting(choice("stable" => "channel_stable", "beta" => "channel_beta"))] pub update_channel: String, - #[serde(default)] #[setting(toggle)] pub right_click_drag: bool, - #[serde(default)] #[setting(toggle)] pub notification_display: bool, - #[serde(default = "default_replace_native_volume_flyout")] + #[educe(Default = true)] #[setting(toggle)] pub replace_native_volume_flyout: bool, - #[serde(default = "default_brightness_overlay_enabled")] + #[educe(Default = true)] #[setting(toggle, label = "brightness_overlay")] pub brightness_overlay_enabled: bool, - #[serde(default = "default_widget_layout")] + #[educe(Default(expression = default_widget_layout()))] pub widget_layout: Vec, - #[serde(default)] pub plugin_widget_layout: Vec, - #[serde(default)] pub compact_widget_layout: Vec, - #[serde(default = "default_resource_metrics")] + #[educe(Default(expression = default_resource_metrics()))] pub resource_metrics: Vec, - #[serde(default = "default_resource_metrics")] + #[educe(Default(expression = default_resource_metrics()))] pub compact_resource_metrics: Vec, - #[serde(default = "default_resource_widget_columns")] + #[educe(Default = 2)] pub resource_widget_columns: usize, - #[serde(default = "default_resource_widget_rows")] + #[educe(Default = 1)] pub resource_widget_rows: usize, #[serde(default)] + #[educe(Default(expression = CONFIG_VERSION))] pub config_version: u32, } - -macro_rules! defaults { - ($($name:ident: $ty:ty = $value:expr),* $(,)?) => { - $(fn $name() -> $ty { $value })* - }; -} - -defaults! { - default_true: bool = true, - default_resource_widget_columns: usize = 2, - default_resource_widget_rows: usize = 1, - default_expanded_scale: f32 = 1.0, - default_animation_fps: u32 = 90, - default_expanded_idle_fps: u32 = 60, - default_replace_native_volume_flyout: bool = true, - default_brightness_overlay_enabled: bool = true, - default_island_style: String = "default".to_string(), - default_show_lyrics: bool = true, - default_lyrics_mode: String = "online".to_string(), - default_auto_hide_delay: f32 = 5.0, - default_hidden_width: f32 = 5.0, - default_check_for_updates: bool = true, - default_update_check_interval: f32 = 4.0, - default_language: String = "auto".to_string(), - default_lyrics_source: String = "163".to_string(), - default_lyrics_scroll_max_width: f32 = 300.0, - default_lyrics_side_gap: f32 = 6.0, - default_settings_theme: String = "system".to_string(), - default_mini_cover_shape: String = "square".to_string(), - default_expanded_cover_shape: String = "square".to_string(), - default_cover_rotate: bool = false, - default_update_channel: String = "stable".to_string(), -} - -impl Default for AppConfig { - fn default() -> Self { - Self { - compact_scale: 1.0, - expanded_scale: default_expanded_scale(), - base_width: 120.0, - base_height: 27.0, - expanded_width: 360.0, - expanded_height: 200.0, - motion_blur: true, - animation_fps: default_animation_fps(), - expanded_idle_fps: default_expanded_idle_fps(), - island_style: default_island_style(), - smtc_enabled: true, - music_notice_acknowledged: false, - smtc_apps: Vec::new(), - smtc_known_apps: Vec::new(), - show_lyrics: default_show_lyrics(), - show_secondary_lyrics: false, - lyrics_mode: default_lyrics_mode(), - lyrics_local_dir: None, - custom_font_path: None, - auto_start: false, - auto_hide: false, - fullscreen_auto_hide: false, - auto_hide_delay: default_auto_hide_delay(), - hidden_width: default_hidden_width(), - check_for_updates: default_check_for_updates(), - update_check_interval: default_update_check_interval(), - language: default_language(), - lyrics_source: default_lyrics_source(), - lyrics_delay: 0.0, - lyrics_scroll: false, - lyrics_scroll_max_width: default_lyrics_scroll_max_width(), - lyrics_side_gap: default_lyrics_side_gap(), - lyrics_transition_animation: LyricTransitionMode::default(), - position_x_offset: 0, - position_y_offset: 0, - legacy_dock_position: None, - monitor_index: 0, - font_size: 0.0, - settings_theme: default_settings_theme(), - mini_cover_shape: default_mini_cover_shape(), - expanded_cover_shape: default_expanded_cover_shape(), - cover_rotate: default_cover_rotate(), - update_channel: default_update_channel(), - right_click_drag: false, - notification_display: false, - replace_native_volume_flyout: default_replace_native_volume_flyout(), - brightness_overlay_enabled: default_brightness_overlay_enabled(), - widget_layout: default_widget_layout(), - plugin_widget_layout: Vec::new(), - compact_widget_layout: Vec::new(), - resource_metrics: default_resource_metrics(), - compact_resource_metrics: default_resource_metrics(), - resource_widget_columns: default_resource_widget_columns(), - resource_widget_rows: default_resource_widget_rows(), - config_version: CONFIG_VERSION, - } - } -} diff --git a/crates/winisland-core/src/config/widgets.rs b/crates/winisland-core/src/config/widgets.rs index eb43421d..dfe67b11 100644 --- a/crates/winisland-core/src/config/widgets.rs +++ b/crates/winisland-core/src/config/widgets.rs @@ -1,8 +1,6 @@ use serde::{Deserialize, Serialize}; use std::sync::atomic::{AtomicU8, Ordering}; -use super::default_true; - #[derive(Serialize, Deserialize, Clone, Copy, Debug, PartialEq, Eq)] #[serde(rename_all = "snake_case")] pub enum WidgetKind { @@ -103,6 +101,10 @@ pub struct ResourceMetricConfig { pub color: u32, } +fn default_true() -> bool { + true +} + pub fn default_resource_metrics() -> Vec { [ (ResourceMetricKind::Cpu, 0x32bef6, true), From 2b2f26d7ea6415f7caf5499208fddf8a1bb402dc Mon Sep 17 00:00:00 2001 From: Eatgrapes Date: Sun, 27 Sep 2026 01:40:10 +0800 Subject: [PATCH 10/13] refactor(windows): run notification async reads as pollkit jobs The notification feed tracked three WinRT async operations with a receiver, a hand-written cancel slot and a finish function that reset both on every branch. pollkit's Job already bundles the channel, the cancel action and the wake-up, so each operation is now one Job field and its finish function is a plain match on poll(). Finished jobs wake the loop through pollkit's hook, which the app sets to the same platform wake as the feed's waker. --- Cargo.lock | 1 + crates/winisland-platform-windows/Cargo.toml | 1 + .../src/notify/mod.rs | 154 ++++-------------- .../src/notify/read.rs | 9 +- 4 files changed, 42 insertions(+), 123 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7c31b186..c715c6d9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5317,6 +5317,7 @@ dependencies = [ "libloading 0.9.0", "log", "parking_lot", + "pollkit", "tokio", "tray-icon", "wasapi", diff --git a/crates/winisland-platform-windows/Cargo.toml b/crates/winisland-platform-windows/Cargo.toml index 049d1d42..286a3a87 100644 --- a/crates/winisland-platform-windows/Cargo.toml +++ b/crates/winisland-platform-windows/Cargo.toml @@ -11,6 +11,7 @@ windows = { version = "0.62.2", features = ["Win32_Foundation", "Win32_System_Co dirs = "6.0" log = { version = "0.4", features = ["std"] } parking_lot = "0.12" +pollkit = { path = "../pollkit" } tray-icon = "0.24.1" image = "0.25" wasapi = "0.24.0" diff --git a/crates/winisland-platform-windows/src/notify/mod.rs b/crates/winisland-platform-windows/src/notify/mod.rs index 10df770d..87de76ff 100644 --- a/crates/winisland-platform-windows/src/notify/mod.rs +++ b/crates/winisland-platform-windows/src/notify/mod.rs @@ -9,10 +9,9 @@ use winisland_platform::{ use std::collections::HashSet; use std::sync::Arc; -use std::sync::mpsc::{self, Receiver}; use std::time::{Duration, Instant}; -use parking_lot::Mutex; +use pollkit::Job; use windows::ApplicationModel::AppDisplayInfo; use windows::Foundation::Size; use windows::UI::Notifications::Management::{ @@ -36,21 +35,6 @@ struct NotificationRecord { creation_time: i64, } -#[derive(Default)] -struct CancelSlot(Mutex>>); - -impl CancelSlot { - fn set(&self, cancel: impl Fn() + Send + Sync + 'static) { - *self.0.lock() = Some(Box::new(cancel)); - } - - fn cancel(&self) { - if let Some(cancel) = self.0.lock().take() { - cancel(); - } - } -} - enum NotificationReadResult { Notifications(Vec), Failed(HRESULT), @@ -61,12 +45,9 @@ pub struct WindowsNotificationFeed { listener: Option, event_subscription: Option, subscription_status: Option, - access_receiver: Option>, - access_cancel: Option>, - read_receiver: Option>, - read_cancel: Option>, - icon_receiver: Option)>>, - icon_cancel: Option>, + access: Job, + read: Job<(NotificationReadKind, NotificationReadResult)>, + icon: Job<(u32, Option)>, access_attempted: bool, retry_after: Option, seen_notifications: HashSet<(u32, i64)>, @@ -84,7 +65,7 @@ impl WindowsNotificationFeed { self.finish_access_request(); if self.listener.is_none() - && self.access_receiver.is_none() + && !self.access.is_running() && !self.access_attempted && self .retry_after @@ -113,23 +94,18 @@ impl WindowsNotificationFeed { self.schedule_retry(); return; }; - let (sender, receiver) = mpsc::sync_channel(1); + let (job, done) = Job::pending(); let cancel_operation = operation.clone(); - let cancel = Arc::new(CancelSlot::default()); - cancel.set(move || { + done.on_cancel(move || { let _ = cancel_operation.Cancel(); }); - self.access_cancel = Some(cancel); tokio::task::spawn_blocking(move || { - let granted = matches!( + done.send(matches!( operation.join(), Ok(UserNotificationListenerAccessStatus::Allowed) - ); - if sender.send(granted).is_ok() { - wake(); - } + )); }); - self.access_receiver = Some(receiver); + self.access = job; } Ok(status) => log::warn!("Notification access was not granted: {status:?}"), Err(error) => { @@ -140,14 +116,8 @@ impl WindowsNotificationFeed { } fn finish_access_request(&mut self) { - let result = self - .access_receiver - .as_ref() - .map(std::sync::mpsc::Receiver::try_recv); - match result { + match self.access.poll() { Some(Ok(true)) => { - self.access_receiver = None; - self.access_cancel = None; let Ok(listener) = UserNotificationListener::Current() else { log::warn!("Notification listener is unavailable after access was granted"); self.schedule_retry(); @@ -155,18 +125,12 @@ impl WindowsNotificationFeed { }; self.start_monitor(listener); } - Some(Ok(false)) => { - self.access_receiver = None; - self.access_cancel = None; - log::warn!("Notification access was not granted"); - } - Some(Err(mpsc::TryRecvError::Disconnected)) => { - self.access_receiver = None; - self.access_cancel = None; + Some(Ok(false)) => log::warn!("Notification access was not granted"), + Some(Err(_)) => { log::warn!("Notification access request ended unexpectedly"); self.schedule_retry(); } - Some(Err(mpsc::TryRecvError::Empty)) | None => {} + None => {} } } @@ -182,29 +146,19 @@ impl WindowsNotificationFeed { } fn finish_notification_read(&mut self) { - let result = self - .read_receiver - .as_ref() - .map(std::sync::mpsc::Receiver::try_recv); - match result { + match self.read.poll() { Some(Ok((kind, NotificationReadResult::Notifications(notifications)))) => { - self.read_receiver = None; - self.read_cancel = None; self.handle_notification_snapshot(kind, notifications); } Some(Ok((_, NotificationReadResult::Failed(error)))) => { - self.read_receiver = None; - self.read_cancel = None; log::warn!("Notification history could not be read: {error:?}"); self.restart_monitor(); } - Some(Err(mpsc::TryRecvError::Disconnected)) => { - self.read_receiver = None; - self.read_cancel = None; + Some(Err(_)) => { log::warn!("Notification history request ended unexpectedly"); self.restart_monitor(); } - Some(Err(mpsc::TryRecvError::Empty)) | None => {} + None => {} } } @@ -271,7 +225,7 @@ impl WindowsNotificationFeed { fn read_notifications_when_signaled(&mut self) { if self.event_subscription.is_none() - || self.read_receiver.is_some() + || self.read.is_running() || !notification_event::take_delivery() { return; @@ -289,13 +243,11 @@ impl WindowsNotificationFeed { log::warn!("Notification history request could not be started"); return false; }; - let (sender, receiver) = mpsc::sync_channel(1); + let (job, done) = Job::pending(); let cancel_operation = operation.clone(); - let cancel = Arc::new(CancelSlot::default()); - cancel.set(move || { + done.on_cancel(move || { let _ = cancel_operation.Cancel(); }); - self.read_cancel = Some(cancel); tokio::task::spawn_blocking(move || { let result = operation.join().map(|notifications| { let mut records = Vec::new(); @@ -321,30 +273,18 @@ impl WindowsNotificationFeed { Ok(notifications) => NotificationReadResult::Notifications(notifications), Err(error) => NotificationReadResult::Failed(error.code()), }; - if sender.send((kind, result)).is_ok() { - wake(); - } + done.send((kind, result)); }); - self.read_receiver = Some(receiver); + self.read = job; true } fn stop(&mut self) { - for cancel in [ - self.access_cancel.take(), - self.read_cancel.take(), - self.icon_cancel.take(), - ] - .into_iter() - .flatten() - { - cancel.cancel(); - } + self.access.cancel(); + self.read.cancel(); + self.icon.cancel(); self.listener = None; self.event_subscription = None; - self.access_receiver = None; - self.read_receiver = None; - self.icon_receiver = None; self.seen_notifications.clear(); self.pending_notification_id = None; notification_event::reset_signals(); @@ -374,10 +314,7 @@ impl WindowsNotificationFeed { } fn start_icon_read(&mut self, notification_id: u32, display: AppDisplayInfo) { - if let Some(cancel) = self.icon_cancel.take() { - cancel.cancel(); - } - self.icon_receiver = None; + self.icon.cancel(); let Some(operation) = display .GetLogo(Size { Width: 64.0, @@ -388,46 +325,27 @@ impl WindowsNotificationFeed { else { return; }; - let (sender, receiver) = mpsc::sync_channel(1); + let (job, done) = Job::pending(); let cancel_operation = operation.clone(); - let cancel = Arc::new(CancelSlot::default()); - cancel.set(move || { + done.on_cancel(move || { let _ = cancel_operation.Cancel(); }); - self.icon_cancel = Some(Arc::clone(&cancel)); tokio::task::spawn_blocking(move || { let icon = operation .join() .ok() - .and_then(|stream| read_app_icon(stream, &cancel)); - if sender.send((notification_id, icon)).is_ok() { - wake(); - } + .and_then(|stream| read_app_icon(stream, &done)); + done.send((notification_id, icon)); }); - self.icon_receiver = Some(receiver); + self.icon = job; } fn finish_icon_read(&mut self) -> Option { - let result = self - .icon_receiver - .as_ref() - .map(std::sync::mpsc::Receiver::try_recv); - match result { - Some(Ok((notification_id, icon))) => { - self.icon_receiver = None; - self.icon_cancel = None; - icon.map(|icon| NotificationMonitorUpdate::Icon { - notification_id, - icon, - }) - } - Some(Err(mpsc::TryRecvError::Disconnected)) => { - self.icon_receiver = None; - self.icon_cancel = None; - None - } - Some(Err(mpsc::TryRecvError::Empty)) | None => None, - } + let (notification_id, icon) = self.icon.poll_ok()?; + icon.map(|icon| NotificationMonitorUpdate::Icon { + notification_id, + icon, + }) } pub(super) fn remove_notification(&self, notification_id: u32) { diff --git a/crates/winisland-platform-windows/src/notify/read.rs b/crates/winisland-platform-windows/src/notify/read.rs index 8a72a45c..592718b2 100644 --- a/crates/winisland-platform-windows/src/notify/read.rs +++ b/crates/winisland-platform-windows/src/notify/read.rs @@ -1,11 +1,10 @@ +use pollkit::Done; use windows::ApplicationModel::AppDisplayInfo; use windows::Storage::Streams::{DataReader, IRandomAccessStreamWithContentType}; use windows::UI::Notifications::Management::UserNotificationListener; use windows::UI::Notifications::{KnownNotificationBindings, UserNotification}; use winisland_platform::{IconBounds, NotificationIconData, NotificationPayload}; -use super::CancelSlot; - const MAX_ICON_BYTES: u64 = 2 * 1024 * 1024; pub(super) fn read_notification( listener: &UserNotificationListener, @@ -83,9 +82,9 @@ fn read_notification_text(notification: &UserNotification) -> (String, String) { ) } -pub(super) fn read_app_icon( +pub(super) fn read_app_icon( stream: IRandomAccessStreamWithContentType, - cancel: &CancelSlot, + done: &Done, ) -> Option { let size = stream.Size().ok()?; if size == 0 || size > MAX_ICON_BYTES { @@ -94,7 +93,7 @@ pub(super) fn read_app_icon( let reader = DataReader::CreateDataReader(&stream).ok()?; let operation = reader.LoadAsync(size as u32).ok()?; let cancel_operation = operation.clone(); - cancel.set(move || { + done.on_cancel(move || { let _ = cancel_operation.Cancel(); }); operation.join().ok()?; From a302408703d444d4f46a70aa237da31bb13b1d6e Mon Sep 17 00:00:00 2001 From: Eatgrapes Date: Sun, 27 Sep 2026 01:46:22 +0800 Subject: [PATCH 11/13] refactor(windows): share WMI connection and window registration The brightness probe and monitor connected to ROOT\WMI with the same eight-argument call and repeated both query strings, and the overlay and settings windows were created and registered the same way. Each now goes through one helper. --- .../src/display/brightness.rs | 79 +++++++------------ .../src/window/mod.rs | 73 ++++++++--------- 2 files changed, 63 insertions(+), 89 deletions(-) diff --git a/crates/winisland-platform-windows/src/display/brightness.rs b/crates/winisland-platform-windows/src/display/brightness.rs index 25bd5360..a16f5fc9 100644 --- a/crates/winisland-platform-windows/src/display/brightness.rs +++ b/crates/winisland-platform-windows/src/display/brightness.rs @@ -11,6 +11,10 @@ use winisland_platform::{BrightnessFeed, BrightnessSnapshot}; use crate::com::ComGuard; +const BRIGHTNESS_QUERY: &str = + "SELECT CurrentBrightness FROM WmiMonitorBrightness WHERE Active = TRUE"; +const METHODS_QUERY: &str = "SELECT * FROM WmiMonitorBrightnessMethods WHERE Active = TRUE"; + pub(super) struct WindowsBrightnessFeed { snapshot: Arc>, sender: SyncSender, @@ -68,36 +72,12 @@ pub(super) fn available() -> bool { let Ok(_com) = ComGuard::mta() else { return false; }; - // SAFETY: The WMI locator, service, and queried objects remain on this COM thread. + // SAFETY: The WMI service and queried objects remain on this COM thread. unsafe { - let Ok(locator) = - CoCreateInstance::<_, ISWbemLocator>(&SWbemLocator, None, CLSCTX_INPROC_SERVER) - else { - return false; - }; - let empty = BSTR::new(); - let Ok(services) = locator.ConnectServer( - &empty, - &BSTR::from("ROOT\\WMI"), - &empty, - &empty, - &empty, - &empty, - 0, - None, - ) else { - return false; - }; - query_first( - &services, - "SELECT CurrentBrightness FROM WmiMonitorBrightness WHERE Active = TRUE", - ) - .is_ok() - && query_first( - &services, - "SELECT * FROM WmiMonitorBrightnessMethods WHERE Active = TRUE", - ) - .is_ok() + connect_wmi().is_ok_and(|services| { + query_first(&services, BRIGHTNESS_QUERY).is_ok() + && query_first(&services, METHODS_QUERY).is_ok() + }) } } @@ -108,28 +88,11 @@ fn run_monitor( ) -> windows::core::Result<()> { // SAFETY: All WMI objects stay on this COM-initialized worker thread. unsafe { - let locator: ISWbemLocator = CoCreateInstance(&SWbemLocator, None, CLSCTX_INPROC_SERVER)?; - let empty = BSTR::new(); - let services = locator.ConnectServer( - &empty, - &BSTR::from("ROOT\\WMI"), - &empty, - &empty, - &empty, - &empty, - 0, - None, - )?; - let initial = query_first( - &services, - "SELECT CurrentBrightness FROM WmiMonitorBrightness WHERE Active = TRUE", - )?; + let services = connect_wmi()?; + let initial = query_first(&services, BRIGHTNESS_QUERY)?; let level = property_u32(&initial, "CurrentBrightness")?.min(100) as f32 / 100.0; publish(snapshot, level, false); - let setter = query_first( - &services, - "SELECT * FROM WmiMonitorBrightnessMethods WHERE Active = TRUE", - )?; + let setter = query_first(&services, METHODS_QUERY)?; let events = services.ExecNotificationQuery( &BSTR::from("SELECT * FROM WmiMonitorBrightnessEvent WHERE Active = TRUE"), &BSTR::from("WQL"), @@ -152,6 +115,24 @@ fn run_monitor( Ok(()) } +unsafe fn connect_wmi() -> windows::core::Result { + // SAFETY: Caller runs on a COM-initialized thread and keeps the service on it. + unsafe { + let locator: ISWbemLocator = CoCreateInstance(&SWbemLocator, None, CLSCTX_INPROC_SERVER)?; + let empty = BSTR::new(); + locator.ConnectServer( + &empty, + &BSTR::from("ROOT\\WMI"), + &empty, + &empty, + &empty, + &empty, + 0, + None, + ) + } +} + unsafe fn query_first( services: &ISWbemServices, query: &str, diff --git a/crates/winisland-platform-windows/src/window/mod.rs b/crates/winisland-platform-windows/src/window/mod.rs index 0ae5289d..bade64c7 100644 --- a/crates/winisland-platform-windows/src/window/mod.rs +++ b/crates/winisland-platform-windows/src/window/mod.rs @@ -14,10 +14,11 @@ use winisland_platform::{ WindowPosition, WindowSize, WindowSystem, }; use winit::dpi::{PhysicalPosition, PhysicalSize}; +use winit::event_loop::ActiveEventLoop; use winit::monitor::MonitorHandle; use winit::platform::windows::{MonitorHandleExtWindows, WindowExtWindows}; use winit::raw_window_handle::{HasWindowHandle, RawWindowHandle}; -use winit::window::{CursorIcon, Theme as WinitTheme, Window}; +use winit::window::{CursorIcon, Theme as WinitTheme, Window, WindowAttributes}; use crate::backdrop::HostBackdrop; use crate::display::WindowsDisplay; @@ -56,6 +57,29 @@ fn with_window(id: WindowId, f: impl FnOnce(&Window) -> T) -> Option { Some(f(&window)) } +fn create_window( + event_loop: &ActiveEventLoop, + attributes: WindowAttributes, +) -> Result, PlatformError> { + event_loop + .create_window(attributes) + .map(Arc::new) + .map_err(PlatformError::backend) +} + +fn register_window(window: Arc, backdrop: Option>) -> WindowId { + let id = WindowId(u64::from(window.id())); + let record = WindowRecord { + host_backdrop: None, + window, + backdrop, + }; + WINDOWS.with(|windows| { + windows.borrow_mut().insert(id, record); + }); + id +} + fn monitor_id(monitor: &MonitorHandle) -> MonitorId { MonitorId(monitor.hmonitor() as usize as u64) } @@ -187,32 +211,13 @@ impl WindowSystem for WindowsWindowSystem { fn create_overlay(&self, spec: OverlaySpec) -> Result { r#loop::with_active_event_loop(|event_loop| { - let backdrop = Arc::new( - event_loop - .create_window(attrs::backdrop_attributes()) - .map_err(PlatformError::backend)?, - ); + let backdrop = create_window(event_loop, attrs::backdrop_attributes())?; let _ = backdrop.set_cursor_hittest(false); let owner = window_hwnd(&backdrop) .ok_or(PlatformError::Unavailable("Win32 backdrop window handle"))? as isize; - let window = Arc::new( - event_loop - .create_window(attrs::overlay_attributes(spec, owner)) - .map_err(PlatformError::backend)?, - ); - let id = WindowId(u64::from(window.id())); - WINDOWS.with(|windows| { - windows.borrow_mut().insert( - id, - WindowRecord { - host_backdrop: None, - window, - backdrop: Some(backdrop), - }, - ); - }); - Ok(id) + let window = create_window(event_loop, attrs::overlay_attributes(spec, owner))?; + Ok(register_window(window, Some(backdrop))) }) .ok_or(PlatformError::Unavailable("active event loop"))? } @@ -224,23 +229,11 @@ impl WindowSystem for WindowsWindowSystem { .available_monitors() .find(|monitor| monitor_id(monitor) == id) }); - let window = Arc::new( - event_loop - .create_window(attrs::settings_attributes(spec, monitor.as_ref())) - .map_err(PlatformError::backend)?, - ); - let id = WindowId(u64::from(window.id())); - WINDOWS.with(|windows| { - windows.borrow_mut().insert( - id, - WindowRecord { - host_backdrop: None, - window, - backdrop: None, - }, - ); - }); - Ok(id) + let attributes = attrs::settings_attributes(spec, monitor.as_ref()); + Ok(register_window( + create_window(event_loop, attributes)?, + None, + )) }) .ok_or(PlatformError::Unavailable("active event loop"))? } From ce841a7ec9cf6c42e5b09ad3d73f8fdae52af266 Mon Sep 17 00:00:00 2001 From: Eatgrapes Date: Sun, 27 Sep 2026 01:53:21 +0800 Subject: [PATCH 12/13] refactor(settings): share the row offset of popup and stepper rects Both rect helpers summed the heights of the rows above the clicked item on their own. They now read it from one row_top helper. --- src/window/settings/pages/mod.rs | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/src/window/settings/pages/mod.rs b/src/window/settings/pages/mod.rs index b772f202..8b51557e 100644 --- a/src/window/settings/pages/mod.rs +++ b/src/window/settings/pages/mod.rs @@ -237,21 +237,24 @@ impl PageInput { hit_test(page.items(), self.x, self.y, self.start_y, self.width) } + fn row_top(&self, page: &SettingsPage, item_index: usize, scroll_y: f32) -> f32 { + let above: f32 = page + .items() + .iter() + .take(item_index) + .map(SettingsItem::height) + .sum(); + self.start_y + above - scroll_y + } + pub(crate) fn popup_button_rect( &self, page: &SettingsPage, item_index: usize, scroll_y: f32, ) -> Rect { - let item_y = self.start_y - + page - .items() - .iter() - .take(item_index) - .map(SettingsItem::height) - .sum::(); let button_x = SIDEBAR_W + CONTENT_PADDING + self.width - GROUP_INNER_PAD - POPUP_BTN_W; - let button_y = item_y + (ROW_HEIGHT - POPUP_BTN_H) / 2.0 - scroll_y; + let button_y = self.row_top(page, item_index, scroll_y) + (ROW_HEIGHT - POPUP_BTN_H) / 2.0; Rect::from_xywh(button_x, button_y, POPUP_BTN_W, POPUP_BTN_H) } @@ -261,17 +264,11 @@ impl PageInput { item_index: usize, scroll_y: f32, ) -> Rect { - let item_y = self.start_y - + page - .items() - .iter() - .take(item_index) - .map(SettingsItem::height) - .sum::(); let content_w = self.width - CONTENT_PADDING * 2.0; let button_x = CONTENT_PADDING + content_w - GROUP_INNER_PAD - STEPPER_BTN_SIZE; let value_x = button_x - STEPPER_GAP - STEPPER_VALUE_W; - let value_y = item_y + (ROW_HEIGHT - STEPPER_BTN_SIZE) / 2.0 - scroll_y; + let value_y = + self.row_top(page, item_index, scroll_y) + (ROW_HEIGHT - STEPPER_BTN_SIZE) / 2.0; Rect::from_xywh( SIDEBAR_W + value_x, value_y, From 5055bed7a03956eb48891eec75fa82472bc2e8b8 Mon Sep 17 00:00:00 2001 From: Eatgrapes Date: Sun, 27 Sep 2026 01:59:06 +0800 Subject: [PATCH 13/13] refactor(ui): drop two copy-pasted blocks in lyrics and sidebar draw_highlighted_lyric built the same text draw three times with only the colour changing, and the settings sidebar ran the same row hit test for clicks and for hover. Each now goes through one closure or helper. --- src/ui/island/mini.rs | 42 +++++++++++------------------------- src/window/settings/input.rs | 32 +++++++++++---------------- 2 files changed, 26 insertions(+), 48 deletions(-) diff --git a/src/ui/island/mini.rs b/src/ui/island/mini.rs index e69e85e7..7f7140c8 100644 --- a/src/ui/island/mini.rs +++ b/src/ui/island/mini.rs @@ -477,12 +477,7 @@ fn draw_highlighted_lyric( blur: Option, highlight: Option, ) { - let Some(highlight) = highlight.filter(|highlight| { - highlight.start_byte <= highlight.end_byte - && highlight.end_byte <= text.len() - && text.is_char_boundary(highlight.start_byte) - && text.is_char_boundary(highlight.end_byte) - }) else { + let draw = |color: Rgba| { draw_text_cached(DrawTextCachedParams { painter, text, @@ -490,28 +485,26 @@ fn draw_highlighted_lyric( y, size, bold: false, - color: active_color, + color, blur, }); + }; + let Some(highlight) = highlight.filter(|highlight| { + highlight.start_byte <= highlight.end_byte + && highlight.end_byte <= text.len() + && text.is_char_boundary(highlight.start_byte) + && text.is_char_boundary(highlight.end_byte) + }) else { + draw(active_color); return; }; - let pending_color = Rgba::from_argb( + draw(Rgba::from_argb( active_color.a(), PENDING_LYRIC_CHANNEL, PENDING_LYRIC_CHANNEL, PENDING_LYRIC_CHANNEL, - ); - draw_text_cached(DrawTextCachedParams { - painter, - text, - x, - y, - size, - bold: false, - color: pending_color, - blur, - }); + )); let font_manager = FontManager::global(); let style = winisland_render::FontStyle::normal(); @@ -533,16 +526,7 @@ fn draw_highlighted_lyric( clip_right, y + size * 0.5, )); - draw_text_cached(DrawTextCachedParams { - painter, - text, - x, - y, - size, - bold: false, - color, - blur, - }); + draw(color); painter.restore(); }; draw_layer(active_color, x, x + completed_width); diff --git a/src/window/settings/input.rs b/src/window/settings/input.rs index 45d45e5d..41e6cfb7 100644 --- a/src/window/settings/input.rs +++ b/src/window/settings/input.rs @@ -53,15 +53,8 @@ impl SettingsApp { } if mouse_x < SIDEBAR_W { - for page in 0..self.sidebar_page_count() { - let row_y = SIDEBAR_START_Y + page as f32 * (SIDEBAR_ROW_H + SIDEBAR_ROW_GAP); - if mouse_y >= row_y - && mouse_y <= row_y + SIDEBAR_ROW_H - && (SIDEBAR_PAD..=SIDEBAR_W - SIDEBAR_PAD).contains(&mouse_x) - { - self.visit_page(page); - return; - } + if let Some(page) = self.sidebar_page_at(mouse_x, mouse_y) { + self.visit_page(page); } return; } @@ -152,16 +145,7 @@ impl SettingsApp { } if mouse_x < SIDEBAR_W { - for page in 0..self.sidebar_page_count() { - let row_y = SIDEBAR_START_Y + page as f32 * (SIDEBAR_ROW_H + SIDEBAR_ROW_GAP); - if mouse_y >= row_y - && mouse_y <= row_y + SIDEBAR_ROW_H - && (SIDEBAR_PAD..=SIDEBAR_W - SIDEBAR_PAD).contains(&mouse_x) - { - return true; - } - } - return false; + return self.sidebar_page_at(mouse_x, mouse_y).is_some(); } let content_width = self.content_width(); @@ -188,6 +172,16 @@ impl SettingsApp { ) } + fn sidebar_page_at(&self, x: f32, y: f32) -> Option { + if !(SIDEBAR_PAD..=SIDEBAR_W - SIDEBAR_PAD).contains(&x) { + return None; + } + (0..self.sidebar_page_count()).find(|&page| { + let row_y = SIDEBAR_START_Y + page as f32 * (SIDEBAR_ROW_H + SIDEBAR_ROW_GAP); + (row_y..=row_y + SIDEBAR_ROW_H).contains(&y) + }) + } + pub(super) fn page_navigation_at(mouse_x: f32, mouse_y: f32) -> Option { if !(PAGE_NAV_Y..=PAGE_NAV_Y + PAGE_NAV_HEIGHT).contains(&mouse_y) { return None;