From 1f5bc965fe192be47ef30a384115b446a4b85073 Mon Sep 17 00:00:00 2001 From: coygeek <65363919+coygeek@users.noreply.github.com> Date: Fri, 9 Oct 2026 18:28:07 -0700 Subject: [PATCH] fix(ui): follow live system appearance Resolve System appearance from egui on every frame and fall back to Dark when the integration cannot report a theme. Keep receiving OS changes during manual modes and apply the selected palette to both widget style branches and the current root UI. Remove the cached detector and its unused dependency. Cover startup, live transitions, manual overrides, saved System preferences, and user colors with an application-frame regression. Add reproducible offscreen system-theme inputs and document the fallback. --- Cargo.lock | 37 -------- Cargo.toml | 1 - crates/ui-egui/Cargo.toml | 1 - crates/ui-egui/examples/ui_shot.rs | 33 +++++++- crates/ui-egui/src/lib.rs | 2 + crates/ui-egui/src/menus.rs | 3 - crates/ui-egui/src/theme.rs | 130 +++++++++++++++++++---------- docs/control-protocol.md | 9 +- 8 files changed, 125 insertions(+), 91 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 34de49ab..8b267585 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -837,23 +837,6 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f27ae1dd37df86211c42e150270f82743308803d90a6f6e6651cd730d5e1732f" -[[package]] -name = "dark-light" -version = "3.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0f66b71ac6b73812b0bf51cda8dd646a268fbb38623f45b2b7907593c323f745" -dependencies = [ - "futures-channel", - "futures-core", - "objc2 0.6.5", - "objc2-foundation 0.3.2", - "wasm-bindgen", - "web-sys", - "windows-sys 0.61.2", - "winreg", - "zbus", -] - [[package]] name = "dasp_sample" version = "0.11.0" @@ -1319,15 +1302,6 @@ dependencies = [ "percent-encoding", ] -[[package]] -name = "futures-channel" -version = "0.3.34" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b1f9e3d69d39e4862ffed03ed071a76f9a13ba1d9109d355b0f0aa6b15e393c4" -dependencies = [ - "futures-core", -] - [[package]] name = "futures-core" version = "0.3.34" @@ -3596,7 +3570,6 @@ dependencies = [ name = "soundcraft-ui-egui" version = "0.4.0" dependencies = [ - "dark-light", "egui", "egui_kittest", "image", @@ -5106,16 +5079,6 @@ dependencies = [ "memchr", ] -[[package]] -name = "winreg" -version = "0.56.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7d6f32a0ff4a9f6f01231eb2059cc85479330739333e0e58cadf03b6af2cca10" -dependencies = [ - "cfg-if", - "windows-sys 0.61.2", -] - [[package]] name = "winresource" version = "0.1.31" diff --git a/Cargo.toml b/Cargo.toml index fe55a4f5..95b9a8e6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -44,7 +44,6 @@ cpal = "0.16" egui = "0.36" eframe = { version = "0.36", default-features = false } egui_kittest = "0.36" -dark-light = "3" rfd = "0.17" image = { version = "0.25", default-features = false, features = ["png"] } diff --git a/crates/ui-egui/Cargo.toml b/crates/ui-egui/Cargo.toml index 0439b8b4..c91b324e 100644 --- a/crates/ui-egui/Cargo.toml +++ b/crates/ui-egui/Cargo.toml @@ -21,7 +21,6 @@ soundcraft-engine.workspace = true soundcraft-playback.workspace = true soundcraft-video.workspace = true egui.workspace = true -dark-light.workspace = true serde.workspace = true serde_json.workspace = true log.workspace = true diff --git a/crates/ui-egui/examples/ui_shot.rs b/crates/ui-egui/examples/ui_shot.rs index acb1d5a8..073b823d 100644 --- a/crates/ui-egui/examples/ui_shot.rs +++ b/crates/ui-egui/examples/ui_shot.rs @@ -1,9 +1,10 @@ //! Render the real UI offscreen (no window, no focus stealing) and save PNGs. //! -//! `cargo run -p soundcraft-ui-egui --example ui_shot -- out.png [script.jsonl] [--blank] [--size WxH]` +//! `cargo run -p soundcraft-ui-egui --example ui_shot -- out.png [script.jsonl] [--blank] [--size WxH] [--system-theme light|dark|none]` //! //! Script lines are control requests (`{"method": "...", "params": {...}}`), `{"shot": "path.png"}` -//! or `{"steps": n}`. The demo session is loaded unless `--blank`. +//! or `{"steps": n}`. `{"system_theme": "light"}` (also `"dark"` or `"none"`) changes the +//! simulated OS appearance for subsequent frames. The demo session is loaded unless `--blank`. use soundcraft_ui_egui::{ControlRequest, Services, SoundApp}; use std::sync::atomic::{AtomicBool, Ordering}; @@ -11,11 +12,29 @@ use std::sync::mpsc; static READY: AtomicBool = AtomicBool::new(false); +fn system_theme(value: &str) -> Option> { + match value { + "light" => Some(Some(egui::Theme::Light)), + "dark" => Some(Some(egui::Theme::Dark)), + "none" => Some(None), + _ => None, + } +} + fn main() { let args: Vec = std::env::args().skip(1).collect(); let out = args.first().cloned().unwrap_or_else(|| "soundcraft.png".into()); let script = args.get(1).filter(|a| !a.starts_with("--")).cloned(); let blank = args.iter().any(|a| a == "--blank"); + let appearance = if let Some(i) = args.iter().position(|a| a == "--system-theme") { + let Some(appearance) = args.get(i + 1).and_then(|value| system_theme(value)) else { + eprintln!("--system-theme requires light, dark or none"); + return; + }; + appearance + } else { + None + }; let (w, h) = args .iter() .position(|a| a == "--size") @@ -39,6 +58,7 @@ fn main() { app, ); harness.input_mut().max_texture_side = Some(8192); + harness.input_mut().system_theme = appearance; READY.store(true, Ordering::Relaxed); let step = |h: &mut egui_kittest::Harness| { let mut raw = std::mem::take(h.input_mut()); @@ -63,6 +83,15 @@ fn main() { eprintln!("bad line: {line}"); continue; }; + if let Some(value) = v.get("system_theme") { + let Some(appearance) = value.as_str().and_then(system_theme) else { + eprintln!("system_theme requires light, dark or none: {line}"); + continue; + }; + harness.input_mut().system_theme = appearance; + step(&mut harness); + continue; + } if let Some(p) = v.get("shot").and_then(|x| x.as_str()) { for _ in 0..3 { step(&mut harness); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index fc0685d2..1a28d99d 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -818,6 +818,8 @@ impl SoundApp { /// Lay out the whole window. pub fn ui(&mut self, ui: &mut egui::Ui) { let ctx = ui.ctx().clone(); + // The integration creates this Ui before logic resolves the current frame's palette. + ui.set_style(ctx.global_style()); if !self.fonts_ready { ctx.request_repaint(); return; diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index d8c40e7b..3fb49ce3 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -538,9 +538,6 @@ pub fn run_ui_command(app: &mut SoundApp, id: &str, p: &Value) -> Option bool { LIGHT_THEME.load(Ordering::Relaxed) } -/// Whether `mode` resolves to the Light palette right now. Never blocks. +/// Resolve each frame from the integration's current OS appearance. Unknown uses Dark. pub fn wants_light(ctx: &egui::Context, mode: ThemeMode) -> bool { match mode { ThemeMode::Light => true, ThemeMode::Dark => false, - ThemeMode::System => system_is_light(ctx), + ThemeMode::System => ctx.system_theme() == Some(egui::Theme::Light), } } -/// Forget the cached system appearance so the next System lookup asks again. -pub fn refresh_system() { - let _ = SYSTEM.compare_exchange(SYSTEM_DARK, SYSTEM_UNKNOWN, Ordering::Relaxed, Ordering::Relaxed); - let _ = SYSTEM.compare_exchange(SYSTEM_LIGHT, SYSTEM_UNKNOWN, Ordering::Relaxed, Ordering::Relaxed); -} - -/// The OS appearance. The query (a D-Bus call on Linux) runs once on a background thread; until it -/// answers this reports dark, and `ctx` is repainted when the answer arrives. -#[cfg(not(target_arch = "wasm32"))] -fn system_is_light(ctx: &egui::Context) -> bool { - if SYSTEM.compare_exchange(SYSTEM_UNKNOWN, SYSTEM_ASKING, Ordering::Relaxed, Ordering::Relaxed).is_ok() { - let ctx = ctx.clone(); - let spawned = std::thread::Builder::new().name("soundcraft-theme".into()).spawn(move || { - let light = matches!(dark_light::detect(), Ok(dark_light::Mode::Light)); - SYSTEM.store(if light { SYSTEM_LIGHT } else { SYSTEM_DARK }, Ordering::Relaxed); - ctx.request_repaint(); - }); - if spawned.is_err() { - SYSTEM.store(SYSTEM_DARK, Ordering::Relaxed); - } - } - SYSTEM.load(Ordering::Relaxed) == SYSTEM_LIGHT -} - -/// On the web the query is a synchronous `matchMedia` lookup (no threads there). -#[cfg(target_arch = "wasm32")] -fn system_is_light(_ctx: &egui::Context) -> bool { - if SYSTEM.compare_exchange(SYSTEM_UNKNOWN, SYSTEM_ASKING, Ordering::Relaxed, Ordering::Relaxed).is_ok() { - let light = matches!(dark_light::detect(), Ok(dark_light::Mode::Light)); - SYSTEM.store(if light { SYSTEM_LIGHT } else { SYSTEM_DARK }, Ordering::Relaxed); - } - SYSTEM.load(Ordering::Relaxed) == SYSTEM_LIGHT -} - /// Track colours tinted for clip bodies. pub fn clip_colors(rgb: [u8; 3], selected: bool) -> (Color32, Color32, Color32) { let [r, g, b] = rgb; @@ -430,8 +389,13 @@ pub fn apply(ctx: &egui::Context, light: bool) { v.widgets.inactive.fg_stroke.color = t.text; v.window_corner_radius = egui::CornerRadius::same(6); v.menu_corner_radius = egui::CornerRadius::same(4); - ctx.set_visuals(v); - ctx.global_style_mut(|s| { + // Keep receiving native OS appearance changes even when our palette is manual. On macOS, + // pinning egui's theme also pins the window appearance and hides later OS changes. + ctx.set_theme(egui::ThemePreference::System); + // Both egui style branches must use our resolved palette: egui can select either branch + // as the OS changes, including while SoundCraft's manual Light/Dark choice stays fixed. + ctx.all_styles_mut(|s| { + s.visuals = v.clone(); s.spacing.item_spacing = egui::vec2(6.0, 4.0); s.spacing.button_padding = egui::vec2(6.0, 2.0); s.interaction.tooltip_delay = 0.4; @@ -476,4 +440,78 @@ mod tests { assert_eq!(app.run("ui.theme", json!({"mode": "system"})), Ok(json!({"mode": "system"}))); assert!(crate::menus::UI_COMMANDS.iter().any(|c| c.0 == "ui.theme")); } + + #[test] + fn application_frames_follow_system_appearance_and_preserve_manual_palettes() { + // Keep all frame assertions in one test: custom-painted tokens share LIGHT_THEME. + fn frame(app: &mut SoundApp, ctx: &egui::Context, system_theme: Option, light: bool) { + let mode = app.ui.theme; + let colors = |app: &SoundApp| { + app.engine + .session() + .tracks + .iter() + .map(|track| { + (track.color, track.playlists.iter().flat_map(|playlist| &playlist.clips).map(|clip| clip.color).collect::>()) + }) + .collect::>() + }; + let before = colors(app); + let mut output = ctx.run_ui(egui::RawInput { system_theme, ..Default::default() }, |ui| { + let ctx = ui.ctx().clone(); + app.logic(&ctx); + let tokens = if light { Tokens::LIGHT } else { Tokens::DARK }; + assert_eq!(is_light(), light, "custom palette for {mode:?} with {system_theme:?}"); + assert_eq!(Tokens::current().panel_bg, tokens.panel_bg); + assert_eq!(ctx.options(|options| options.theme_preference), egui::ThemePreference::System); + for theme in [egui::Theme::Light, egui::Theme::Dark] { + let style = ctx.style_of(theme); + assert_eq!(style.visuals.dark_mode, !light); + assert_eq!(style.visuals.panel_fill, tokens.panel_bg); + assert_eq!(style.visuals.widgets.inactive.bg_fill, tokens.button); + assert_eq!(style.visuals.widgets.inactive.fg_stroke.color, tokens.text); + assert_eq!(style.spacing.button_padding, egui::vec2(6.0, 2.0)); + } + app.ui(ui); + assert_eq!(ui.visuals().panel_fill, tokens.panel_bg); + assert_eq!(ui.visuals().widgets.inactive.bg_fill, tokens.button); + }); + output.textures_delta.clear(); + assert_eq!(app.ui.theme, mode); + assert_eq!(serde_json::to_value(&app.ui).unwrap()["theme"], mode.id()); + assert_eq!(colors(app), before, "appearance must preserve track and clip colors"); + } + + let appearances = [Some(egui::Theme::Light), Some(egui::Theme::Dark), Some(egui::Theme::Light), None]; + for initial in [Some(egui::Theme::Light), Some(egui::Theme::Dark), None] { + let ctx = egui::Context::default(); + let mut app = SoundApp::new(soundcraft_engine::Engine::default(), None, Services::default()); + app.ui = serde_json::from_value(json!({"theme": "system"})).unwrap(); + frame(&mut app, &ctx, initial, initial == Some(egui::Theme::Light)); + } + + let ctx = egui::Context::default(); + let mut engine = soundcraft_engine::demo::demo_engine(); + let track = engine.session_mut().tracks.first_mut().unwrap(); + track.color = [19, 83, 147]; + track.playlists.iter_mut().flat_map(|playlist| &mut playlist.clips).next().unwrap().color = Some([211, 57, 99]); + let mut app = SoundApp::new(engine, None, Services::default()); + for mode in [ThemeMode::System, ThemeMode::Light, ThemeMode::Dark] { + app.run("ui.theme", json!({"mode": mode.id()})).unwrap(); + for appearance in appearances { + let light = mode == ThemeMode::Light || (mode == ThemeMode::System && appearance == Some(egui::Theme::Light)); + frame(&mut app, &ctx, appearance, light); + } + if mode != ThemeMode::System { + let latest = if mode == ThemeMode::Light { egui::Theme::Dark } else { egui::Theme::Light }; + frame(&mut app, &ctx, Some(latest), mode == ThemeMode::Light); + app.run("ui.theme", json!({"mode": "system"})).unwrap(); + // Resolve the current context immediately, without another OS appearance event. + app.logic(&ctx); + assert_eq!(is_light(), latest == egui::Theme::Light); + assert_eq!(ctx.global_style().visuals.dark_mode, latest != egui::Theme::Light); + frame(&mut app, &ctx, Some(latest), latest == egui::Theme::Light); + } + } + } } diff --git a/docs/control-protocol.md b/docs/control-protocol.md index ba7b0795..a64c4c85 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -64,4 +64,11 @@ soundcraft-cli app --port $P ui.menu.invoke '{"path":"Track > New..."}' Offscreen (no window): `cargo run -p soundcraft-ui-egui --example ui_shot -- out.png script.jsonl`, where each script line is a request (`{"method": …, "params": …}`), `{"shot": "path.png"}` or -`{"steps": n}`. +`{"steps": n}`. Use `--system-theme light|dark|none` to supply the initial OS appearance, and +script events such as `{"system_theme": "dark"}` to change it on subsequent frames. + +Appearance defaults to Dark. `ui.theme {"mode":"system"}` follows the appearance supplied by +the native or web integration at launch and on later frames, while the saved choice stays System. +When the integration cannot report an appearance, System uses Dark. Manual Light and Dark stay +fixed when the OS appearance changes. An offscreen render without `--system-theme` supplies no +OS appearance and therefore resolves System to Dark.