From d3e2e184c23be1778aa1eaca88110ab18d316b9a Mon Sep 17 00:00:00 2001 From: gyurix Date: Mon, 12 Oct 2026 10:18:29 +0200 Subject: [PATCH] Linux: the display theme asks the desktop, not winit (#298) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Linux: the display theme asks the desktop, not winit winit reports no system theme on Linux — `ActiveEventLoop::system_theme()` is `None` there on every desktop — so "Use system setting" never saw the desktop's choice and fell back to light. A new `desktop_theme` module asks the XDG Desktop Portal's Settings interface for `org.gnome.desktop.interface color-scheme` on a thread of its own, so no D-Bus call is ever made from the frame loop, and the app follows the answer, including a change made while it runs. The colour scheme is asked for under its GNOME schema name first, which GNOME's portal and XFCE's both answer, then under the older freedesktop one, which KDE answers. A desktop that answers neither leaves the user's own choice alone, exactly as before. macOS and Windows are untouched: winit answers there. * Review fixes for #298: one D-Bus connection; watcher started by the app - The watcher keeps one session-bus connection for its lifetime instead of opening a new one every 4 s; a connection whose calls all fail is dropped and reopened on the next poll. - WordApp::new no longer starts the watcher. The desktop app calls WordApp::watch_desktop_theme at start-up, so unit tests, kittest and ui_shot don't spawn D-Bus pollers and what they draw doesn't depend on the host's desktop theme (test: a_new_app_does_not_watch_the_desktop). - A change wakes the window (request_repaint), so an idle app follows a theme switch. Co-Authored-By: Claude Opus 5.5 * Test: name the empty Vec's type (a new dependency adds a PartialEq impl for char) --------- Co-authored-by: GyuriX Co-authored-by: Brandon Thomas Co-authored-by: Claude Opus 5.5 --- Cargo.lock | 1 + apps/wordcraft/src/main.rs | 1 + crates/ui-egui/Cargo.toml | 6 + crates/ui-egui/src/desktop_theme.rs | 171 ++++++++++++++++++++++++++++ crates/ui-egui/src/i18n/tests.rs | 2 +- crates/ui-egui/src/lib.rs | 47 +++++++- 6 files changed, 221 insertions(+), 7 deletions(-) create mode 100644 crates/ui-egui/src/desktop_theme.rs diff --git a/Cargo.lock b/Cargo.lock index 0dfdc22..36f1a7c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5249,6 +5249,7 @@ dependencies = [ "wordcraft-layout", "wordcraft-render", "wordcraft-zotero", + "zbus", "zip", ] diff --git a/apps/wordcraft/src/main.rs b/apps/wordcraft/src/main.rs index 6c39278..c5008e9 100644 --- a/apps/wordcraft/src/main.rs +++ b/apps/wordcraft/src/main.rs @@ -288,6 +288,7 @@ fn main() -> eframe::Result { let (file_dialog, dialogs) = file_dialogs::hook(&cc.egui_ctx); let mut app = WordApp::new(Session::new(doc), services(file_dialog, &cc.egui_ctx)); load_prefs(&mut app); + app.watch_desktop_theme(&cc.egui_ctx); app.ui.window = restored; app.integrated_titlebar = cfg!(target_os = "macos"); let mut key_file = None; diff --git a/crates/ui-egui/Cargo.toml b/crates/ui-egui/Cargo.toml index 54924d0..4f2e02d 100644 --- a/crates/ui-egui/Cargo.toml +++ b/crates/ui-egui/Cargo.toml @@ -25,6 +25,12 @@ image = { workspace = true } # The system's preferred interface languages (#8). Native: OS APIs; wasm: none (English). sys-locale = "0.3" +# Linux only, where winit reports no system theme: the desktop portal's light/dark answer (see +# `desktop_theme.rs`). Already in the tree, with these features, through egui's own accessibility +# bus; pinned to one version so nothing is built twice. +[target.'cfg(target_os = "linux")'.dependencies] +zbus = "=5.19.0" + [target.'cfg(target_arch = "wasm32")'.dependencies] # The browser clock (`SystemTime::now()`/`Instant::now()` panic on wasm32). js-sys = "0.3" diff --git a/crates/ui-egui/src/desktop_theme.rs b/crates/ui-egui/src/desktop_theme.rs new file mode 100644 index 0000000..7f9ed41 --- /dev/null +++ b/crates/ui-egui/src/desktop_theme.rs @@ -0,0 +1,171 @@ +//! The desktop's own light/dark choice, for an interface setting that follows it. +//! +//! winit reports no system theme on Linux — `ActiveEventLoop::system_theme` is `None` on every +//! desktop there — so "Use system setting" has to ask the desktop itself. The XDG Desktop Portal's +//! Settings interface answers on GNOME, KDE and XFCE alike, over X11 and Wayland alike. + +use std::sync::mpsc::{Receiver, TryRecvError, channel}; +use std::time::Duration; + +/// How often [`DesktopTheme::start`] re-reads the desktop's answer. The portal also publishes a +/// change signal, but subscribing needs a connection kept alive for the life of every window; one +/// round trip every few seconds is cheap and reacts soon enough after a theme switch. +const POLL: Duration = Duration::from_secs(4); + +/// Whether the desktop asks applications to draw dark. `None` when it says nothing, and when +/// there is no session bus or no portal to ask (a bare X server, a container), so the caller +/// keeps whatever it chose itself. +pub fn prefers_dark() -> Option { + read(&mut Bus::default()) +} + +/// Watches the desktop and reports its choice when it changes. [`prefers_dark`] covers the +/// answer at start-up; this covers the user switching theme while the app runs. +#[derive(Default)] +pub struct DesktopTheme { + /// `None` when no watcher thread could be started; the one-shot read still works. + changes: Option>>, +} + +impl DesktopTheme { + /// Start watching. Nothing runs before this is called, and calling it once is enough. The + /// first answer is sent straight away, so the caller does not have to ask for it separately. + /// `changed` runs on the watcher's thread after each answer is sent (the app asks for a + /// repaint there, so an idle window picks the change up). The app binary starts this; tests + /// and headless screenshots don't, so they never depend on the machine's desktop theme. + pub fn start(changed: impl Fn() + Send + 'static) -> Self { + let (tx, rx) = channel(); + let started = std::thread::Builder::new().name("wordcraft-desktop-theme".into()).spawn(move || { + // One session-bus connection for the life of the watcher, opened again only after + // it fails. + let mut bus = Bus::default(); + let mut last = read(&mut bus); + if tx.send(last).is_err() { + return; + } + changed(); + loop { + std::thread::sleep(POLL); + let now = read(&mut bus); + if now != last { + last = now; + if tx.send(now).is_err() { + return; + } + changed(); + } + } + }); + Self { changes: started.ok().map(|_| rx) } + } + + /// Whether a watcher thread is running. + pub fn watching(&self) -> bool { + self.changes.is_some() + } + + /// The desktop's new answer if it changed since the last call, to be polled once a frame. + /// Several changes in one frame collapse to the newest, and a desktop that stops answering + /// leaves the last answer in place. + pub fn take_change(&mut self) -> Option { + let Some(changes) = &self.changes else { return None }; + let mut newest = None; + loop { + match changes.try_recv() { + Ok(changed) => newest = changed, + Err(TryRecvError::Empty) | Err(TryRecvError::Disconnected) => return newest, + } + } + } +} + +/// The watcher's session-bus connection, kept between polls. +#[cfg(target_os = "linux")] +#[derive(Default)] +struct Bus(Option); + +#[cfg(not(target_os = "linux"))] +#[derive(Default)] +struct Bus; + +/// Ask the portal once, over `bus`'s connection (opened when there is none). D-Bus calls +/// block, so this belongs off the UI thread. A connection whose calls all fail is dropped, so +/// the next poll opens a fresh one (the bus or portal may have restarted). +#[cfg(target_os = "linux")] +fn read(bus: &mut Bus) -> Option { + const PORTAL: &str = "org.freedesktop.portal.Desktop"; + const SETTINGS: &str = "org.freedesktop.portal.Settings"; + const SCHEMAS: [(&str, &str); 2] = [("org.gnome.desktop.interface", "color-scheme"), ("org.freedesktop.interface.color-scheme", "color-scheme")]; + if bus.0.is_none() { + bus.0 = zbus::blocking::Connection::session().ok(); + } + let connection = bus.0.as_ref()?; + let mut answered = false; + for (schema, key) in SCHEMAS { + let Ok(reply) = connection.call_method(Some(PORTAL), "/org/freedesktop/portal/desktop", Some(SETTINGS), "ReadOne", &(schema, key)) else { + continue; + }; + answered = true; + let Ok((answer,)) = reply.body().deserialize::<(zbus::zvariant::OwnedValue,)>() else { continue }; + if let Ok(scheme) = String::try_from(&*answer) { + return parse(&scheme); + } + } + if !answered { + bus.0 = None; + } + None +} + +/// The portal's `color-scheme` answer. `default` means the desktop states no preference, so +/// neither does the application. +#[cfg(target_os = "linux")] +fn parse(scheme: &str) -> Option { + match scheme { + "prefer-dark" => Some(true), + "prefer-light" => Some(false), + _ => None, + } +} + +/// macOS and Windows answer through winit, which the caller already has. +#[cfg(not(target_os = "linux"))] +fn read(_bus: &mut Bus) -> Option { + None +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The portal answers with one of three strings; only the two preferences are a choice. + #[test] + #[cfg(target_os = "linux")] + fn the_portal_answer_reads_as_a_choice_or_as_none() { + assert_eq!(parse("prefer-dark"), Some(true)); + assert_eq!(parse("prefer-light"), Some(false)); + assert_eq!(parse("default"), None); + assert_eq!(parse(""), None); + } + + /// Off Linux the caller already has winit's answer, so this adds nothing. + #[test] + #[cfg(not(target_os = "linux"))] + fn other_platforms_answer_through_winit() { + assert_eq!(prefers_dark(), None); + } + + /// A watcher that was never started is inert rather than a panic. + #[test] + fn a_watcher_without_a_thread_is_inert() { + assert_eq!(DesktopTheme::default().take_change(), None); + } + + /// A new app doesn't watch the desktop (the app binary starts the watcher), so tests and + /// headless screenshots never depend on the host's theme or spawn D-Bus pollers. + #[test] + fn a_new_app_does_not_watch_the_desktop() { + let app = crate::WordApp::new(wordcraft_engine::Session::new(wordcraft_doc::Document::new()), Default::default()); + assert!(!app.desktop_theme.watching()); + } +} diff --git a/crates/ui-egui/src/i18n/tests.rs b/crates/ui-egui/src/i18n/tests.rs index 7cd37d8..7384efc 100644 --- a/crates/ui-egui/src/i18n/tests.rs +++ b/crates/ui-egui/src/i18n/tests.rs @@ -448,7 +448,7 @@ fn cjk_catalogs_are_covered_by_the_embedded_interface_faces() { assert!(missing.is_empty(), "{code}: no glyph in {font:?} for {missing}"); } // So the interface reads no installed CJK font for these languages. - assert_eq!(wordcraft_fonts::ui_uncovered_cjk(&[l.0.source, &names], &wordcraft_fonts::ui_cjk_fonts(l.prefers_hans())), []); + assert_eq!(wordcraft_fonts::ui_uncovered_cjk(&[l.0.source, &names], &wordcraft_fonts::ui_cjk_fonts(l.prefers_hans())), Vec::::new()); } } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 472235b..d29e1b8 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -5,6 +5,13 @@ //! the menus, ribbon, shortcuts, control channel and MCP all reach the same behaviour. #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] +/// What the operating system says about light and dark. macOS and Windows answer through winit; +/// winit reports nothing on Linux, where the desktop portal does ([`desktop_theme`]). `None` means +/// nobody answered, not a preference. +fn system_theme(ctx: &egui::Context, desktop_dark: Option) -> Option { + ctx.system_theme().or_else(|| desktop_dark.map(|dark| if dark { egui::Theme::Dark } else { egui::Theme::Light })) +} + /// An English UI string in the current interface language ([`i18n::t`]). #[macro_export] macro_rules! tl { @@ -23,6 +30,7 @@ pub mod colors; pub mod content_controls; pub mod control; pub mod credits; +pub mod desktop_theme; pub mod dialogs; pub mod dialogs_chart; pub mod dialogs_controls; @@ -311,8 +319,12 @@ pub struct WordApp { /// File ▸ Options: load the installed CJK fallback font if no embedded face covers it. pub(crate) want_system_cjk: bool, fonts_frames: u32, - /// The interface theme setting last installed in egui ([`theme::apply`]). + /// The effective interface theme last installed in egui ([`theme::apply`]). applied_theme: Option, + /// The desktop's light/dark choice, watched while the app runs (Linux has no winit answer). + desktop_theme: desktop_theme::DesktopTheme, + /// The last answer [`desktop_theme`] gave, so a frame that asks costs nothing. + desktop_dark: Option, /// The interface language whose editing language the session last got ([`i18n::Lang::editing_lang`]). applied_editing: Option, /// What the system last said about its animation effects ([`Services::animations_enabled`]). @@ -401,6 +413,9 @@ impl WordApp { want_system_cjk: false, fonts_frames: 0, applied_theme: None, + // Inert until the app binary calls `watch_desktop_theme`. + desktop_theme: desktop_theme::DesktopTheme::default(), + desktop_dark: None, applied_editing: None, system_animations: None, applied_animations: None, @@ -803,9 +818,17 @@ impl WordApp { } } + /// Follow the desktop's light/dark choice while the app runs (Linux, where winit reports no + /// system theme; see [`desktop_theme`]). The desktop app calls this once at start-up; unit + /// tests and `ui_shot` don't, so what they draw doesn't depend on the host's desktop theme. + pub fn watch_desktop_theme(&mut self, ctx: &egui::Context) { + let ctx = ctx.clone(); + self.desktop_theme = desktop_theme::DesktopTheme::start(move || ctx.request_repaint()); + } + /// Whether the interface theme setting currently resolves to dark (`System` asks the OS). pub fn ui_is_dark(&self) -> bool { - self.ui.theme.is_dark(self.ctx.as_ref().and_then(egui::Context::system_theme)) + self.ui.theme.is_dark(self.ctx.as_ref().and_then(|ctx| system_theme(ctx, self.desktop_dark))) } /// Whether animation effects are on: the Animation effects setting, where `System` follows @@ -1142,12 +1165,24 @@ impl WordApp { ctx.options_mut(|o| o.zoom_with_keyboard = false); self.styled = true; } + if let Some(answer) = self.desktop_theme.take_change() { + self.desktop_dark = Some(answer); + } // Both palettes are installed and egui picks one: with `System` it follows an OS appearance - // change live (it reports it and repaints, #311). The interface follows the Interface theme + // change live (winit reports it and repaints, #311). Where winit is silent (Linux) the + // desktop portal's answer fills in instead. The interface follows the Interface theme // setting alone; Dark page only changes how pages are drawn (#312). - if self.applied_theme != Some(self.ui.theme) { - theme::apply(ctx, self.ui.theme); - self.applied_theme = Some(self.ui.theme); + let effective = match self.ui.theme { + theme::Appearance::System if ctx.system_theme().is_none() => match self.desktop_dark { + Some(true) => theme::Appearance::Dark, + Some(false) => theme::Appearance::Light, + None => theme::Appearance::System, + }, + other => other, + }; + if self.applied_theme != Some(effective) { + theme::apply(ctx, effective); + self.applied_theme = Some(effective); } // Interface size (#475): egui's zoom factor multiplies the display scale the system reports // (native pixels per point), so it adds to it rather than replacing it. Keyboard zoom is