From 2dac4a4fc6f5fedd42665834991b19115b62088e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tau=20G=C3=A4rtli?= Date: Tue, 6 Aug 2024 20:17:51 +0200 Subject: [PATCH] Follow the System Theme in egui (#4860) * Some initial progress towards #4490 This PR just moves `Theme` and the "follow system theme" settings to egui and adds `RawInput.system_theme`. A follow-up PR can then introduce the two separate `dark_mode_style` and `light_mode_style` fields on `Options`. * [x] I have followed the instructions in the PR template ### Breaking changes The options `follow_system_theme` and `default_theme` has been moved from `eframe` into `egui::Options`, settable with `ctx.options_mut` --------- Co-authored-by: Emil Ernerfeldt --- crates/eframe/src/epi.rs | 62 ------------------- crates/eframe/src/native/epi_integration.rs | 22 +------ crates/eframe/src/native/glow_integration.rs | 7 +-- crates/eframe/src/native/wgpu_integration.rs | 6 +- crates/eframe/src/native/winit_integration.rs | 10 --- crates/eframe/src/web/app_runner.rs | 11 +--- crates/eframe/src/web/events.rs | 13 ++-- crates/eframe/src/web/mod.rs | 10 ++- crates/eframe/src/web/web_runner.rs | 6 -- crates/egui-winit/src/lib.rs | 19 +++++- crates/egui/src/data/input.rs | 15 ++++- crates/egui/src/lib.rs | 2 +- crates/egui/src/memory.rs | 49 ++++++++++++++- crates/egui/src/memory/theme.rs | 29 +++++++++ crates/egui_glow/examples/pure_glow.rs | 3 +- crates/egui_glow/src/winit.rs | 2 + 16 files changed, 129 insertions(+), 137 deletions(-) create mode 100644 crates/egui/src/memory/theme.rs diff --git a/crates/eframe/src/epi.rs b/crates/eframe/src/epi.rs index ac6d0f45a24..3b303dce1ee 100644 --- a/crates/eframe/src/epi.rs +++ b/crates/eframe/src/epi.rs @@ -297,21 +297,6 @@ pub struct NativeOptions { #[cfg(any(feature = "glow", feature = "wgpu"))] pub renderer: Renderer, - /// Try to detect and follow the system preferred setting for dark vs light mode. - /// - /// The theme will automatically change when the dark vs light mode preference is changed. - /// - /// Does not work on Linux (see ). - /// - /// See also [`Self::default_theme`]. - pub follow_system_theme: bool, - - /// Which theme to use in case [`Self::follow_system_theme`] is `false` - /// or eframe fails to detect the system theme. - /// - /// Default: [`Theme::Dark`]. - pub default_theme: Theme, - /// This controls what happens when you close the main eframe window. /// /// If `true`, execution will continue after the eframe window is closed. @@ -417,8 +402,6 @@ impl Default for NativeOptions { #[cfg(any(feature = "glow", feature = "wgpu"))] renderer: Renderer::default(), - follow_system_theme: cfg!(target_os = "macos") || cfg!(target_os = "windows"), - default_theme: Theme::Dark, run_and_return: true, #[cfg(any(feature = "glow", feature = "wgpu"))] @@ -449,19 +432,6 @@ impl Default for NativeOptions { /// Options when using `eframe` in a web page. #[cfg(target_arch = "wasm32")] pub struct WebOptions { - /// Try to detect and follow the system preferred setting for dark vs light mode. - /// - /// See also [`Self::default_theme`]. - /// - /// Default: `true`. - pub follow_system_theme: bool, - - /// Which theme to use in case [`Self::follow_system_theme`] is `false` - /// or system theme detection fails. - /// - /// Default: `Theme::Dark`. - pub default_theme: Theme, - /// Sets the number of bits in the depth buffer. /// /// `egui` doesn't need the depth buffer, so the default value is 0. @@ -492,8 +462,6 @@ pub struct WebOptions { impl Default for WebOptions { fn default() -> Self { Self { - follow_system_theme: true, - default_theme: Theme::Dark, depth_buffer: 0, #[cfg(feature = "glow")] @@ -509,31 +477,6 @@ impl Default for WebOptions { // ---------------------------------------------------------------------------- -/// Dark or Light theme. -#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] -#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] -pub enum Theme { - /// Dark mode: light text on a dark background. - Dark, - - /// Light mode: dark text on a light background. - Light, -} - -impl Theme { - /// Get the egui visuals corresponding to this theme. - /// - /// Use with [`egui::Context::set_visuals`]. - pub fn egui_visuals(self) -> egui::Visuals { - match self { - Self::Dark => egui::Visuals::dark(), - Self::Light => egui::Visuals::light(), - } - } -} - -// ---------------------------------------------------------------------------- - /// WebGL Context options #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] @@ -814,11 +757,6 @@ pub struct IntegrationInfo { #[cfg(target_arch = "wasm32")] pub web_info: WebInfo, - /// Does the OS use dark or light mode? - /// - /// `None` means "don't know". - pub system_theme: Option, - /// Seconds of cpu usage (in seconds) on the previous frame. /// /// This includes [`App::update`] as well as rendering (except for vsync waiting). diff --git a/crates/eframe/src/native/epi_integration.rs b/crates/eframe/src/native/epi_integration.rs index 5d02e2386e9..541dcdd34b7 100644 --- a/crates/eframe/src/native/epi_integration.rs +++ b/crates/eframe/src/native/epi_integration.rs @@ -10,7 +10,7 @@ use raw_window_handle::{HasDisplayHandle as _, HasWindowHandle as _}; use egui::{DeferredViewportUiCallback, NumExt as _, ViewportBuilder, ViewportId}; use egui_winit::{EventResponse, WindowSettings}; -use crate::{epi, Theme}; +use crate::epi; pub fn viewport_builder( egui_zoom_factor: f32, @@ -158,7 +158,6 @@ pub struct EpiIntegration { close: bool, can_drag_window: bool, - follow_system_theme: bool, #[cfg(feature = "persistence")] persist_window: bool, app_icon_setter: super::app_icon::AppTitleIconSetter, @@ -169,7 +168,6 @@ impl EpiIntegration { pub fn new( egui_ctx: egui::Context, window: &winit::window::Window, - system_theme: Option, app_name: &str, native_options: &crate::NativeOptions, storage: Option>, @@ -180,10 +178,7 @@ impl EpiIntegration { #[cfg(feature = "wgpu")] wgpu_render_state: Option, ) -> Self { let frame = epi::Frame { - info: epi::IntegrationInfo { - system_theme, - cpu_usage: None, - }, + info: epi::IntegrationInfo { cpu_usage: None }, storage, #[cfg(feature = "glow")] gl, @@ -217,7 +212,6 @@ impl EpiIntegration { pending_full_output: Default::default(), close: false, can_drag_window: false, - follow_system_theme: native_options.follow_system_theme, #[cfg(feature = "persistence")] persist_window: native_options.persist_window, app_icon_setter, @@ -251,11 +245,6 @@ impl EpiIntegration { state: ElementState::Pressed, .. } => self.can_drag_window = true, - WindowEvent::ThemeChanged(winit_theme) if self.follow_system_theme => { - let theme = theme_from_winit_theme(*winit_theme); - self.frame.info.system_theme = Some(theme); - self.egui_ctx.set_visuals(theme.egui_visuals()); - } _ => {} } @@ -398,10 +387,3 @@ pub fn load_egui_memory(_storage: Option<&dyn epi::Storage>) -> Option Theme { - match theme { - winit::window::Theme::Dark => Theme::Dark, - winit::window::Theme::Light => Theme::Light, - } -} diff --git a/crates/eframe/src/native/glow_integration.rs b/crates/eframe/src/native/glow_integration.rs index 366b71cfd7a..fcec15c35ee 100644 --- a/crates/eframe/src/native/glow_integration.rs +++ b/crates/eframe/src/native/glow_integration.rs @@ -228,14 +228,11 @@ impl GlowWinitApp { } } - let system_theme = - winit_integration::system_theme(&glutin.window(ViewportId::ROOT), &self.native_options); let painter = Rc::new(RefCell::new(painter)); let integration = EpiIntegration::new( egui_ctx, &glutin.window(ViewportId::ROOT), - system_theme, &self.app_name, &self.native_options, storage, @@ -281,9 +278,6 @@ impl GlowWinitApp { } } - let theme = system_theme.unwrap_or(self.native_options.default_theme); - integration.egui_ctx.set_visuals(theme.egui_visuals()); - if self .native_options .viewport @@ -1120,6 +1114,7 @@ impl GlutinWindowContext { viewport_id, event_loop, Some(window.scale_factor() as f32), + window.theme(), self.max_texture_side, ) }); diff --git a/crates/eframe/src/native/wgpu_integration.rs b/crates/eframe/src/native/wgpu_integration.rs index 74f4e957c70..2a4fae9532c 100644 --- a/crates/eframe/src/native/wgpu_integration.rs +++ b/crates/eframe/src/native/wgpu_integration.rs @@ -203,11 +203,9 @@ impl WgpuWinitApp { let wgpu_render_state = painter.render_state(); - let system_theme = winit_integration::system_theme(&window, &self.native_options); let integration = EpiIntegration::new( egui_ctx.clone(), &window, - system_theme, &self.app_name, &self.native_options, storage, @@ -243,6 +241,7 @@ impl WgpuWinitApp { ViewportId::ROOT, event_loop, Some(window.scale_factor() as f32), + window.theme(), painter.max_texture_side(), ); @@ -251,8 +250,6 @@ impl WgpuWinitApp { let event_loop_proxy = self.repaint_proxy.lock().clone(); egui_winit.init_accesskit(&window, event_loop_proxy); } - let theme = system_theme.unwrap_or(self.native_options.default_theme); - egui_ctx.set_visuals(theme.egui_visuals()); let app_creator = std::mem::take(&mut self.app_creator) .expect("Single-use AppCreator has unexpectedly already been taken"); @@ -872,6 +869,7 @@ impl Viewport { viewport_id, event_loop, Some(window.scale_factor() as f32), + window.theme(), painter.max_texture_side(), )); diff --git a/crates/eframe/src/native/winit_integration.rs b/crates/eframe/src/native/winit_integration.rs index 1489b0bf40d..049c90a63ca 100644 --- a/crates/eframe/src/native/winit_integration.rs +++ b/crates/eframe/src/native/winit_integration.rs @@ -118,16 +118,6 @@ pub enum EventResult { Exit, } -pub fn system_theme(window: &Window, options: &crate::NativeOptions) -> Option { - if options.follow_system_theme { - window - .theme() - .map(super::epi_integration::theme_from_winit_theme) - } else { - None - } -} - #[cfg(feature = "accesskit")] pub(crate) fn on_accesskit_window_event( egui_winit: &mut egui_winit::State, diff --git a/crates/eframe/src/web/app_runner.rs b/crates/eframe/src/web/app_runner.rs index 1a0c3f44c53..216fa16ecce 100644 --- a/crates/eframe/src/web/app_runner.rs +++ b/crates/eframe/src/web/app_runner.rs @@ -38,18 +38,11 @@ impl AppRunner { ) -> Result { let painter = super::ActiveWebPainter::new(canvas, &web_options).await?; - let system_theme = if web_options.follow_system_theme { - super::system_theme() - } else { - None - }; - let info = epi::IntegrationInfo { web_info: epi::WebInfo { user_agent: super::user_agent().unwrap_or_default(), location: super::web_location(), }, - system_theme, cpu_usage: None, }; let storage = LocalStorage::default(); @@ -68,9 +61,6 @@ impl AppRunner { o.zoom_factor = 1.0; }); - let theme = system_theme.unwrap_or(web_options.default_theme); - egui_ctx.set_visuals(theme.egui_visuals()); - let cc = epi::CreationContext { egui_ctx: egui_ctx.clone(), integration_info: info.clone(), @@ -132,6 +122,7 @@ impl AppRunner { .entry(egui::ViewportId::ROOT) .or_default() .native_pixels_per_point = Some(super::native_pixels_per_point()); + runner.input.raw.system_theme = super::system_theme(); Ok(runner) } diff --git a/crates/eframe/src/web/events.rs b/crates/eframe/src/web/events.rs index e3696057d04..b3854d17887 100644 --- a/crates/eframe/src/web/events.rs +++ b/crates/eframe/src/web/events.rs @@ -94,6 +94,7 @@ pub(crate) fn install_event_handlers(runner_ref: &WebRunner) -> Result<(), JsVal install_wheel(runner_ref, &canvas)?; install_drag_and_drop(runner_ref, &canvas)?; install_window_events(runner_ref, &window)?; + install_color_scheme_change_event(runner_ref, &window)?; Ok(()) } @@ -353,17 +354,17 @@ fn install_window_events(runner_ref: &WebRunner, window: &EventTarget) -> Result Ok(()) } -pub(crate) fn install_color_scheme_change_event(runner_ref: &WebRunner) -> Result<(), JsValue> { - let window = web_sys::window().unwrap(); - - if let Some(media_query_list) = prefers_color_scheme_dark(&window)? { +fn install_color_scheme_change_event( + runner_ref: &WebRunner, + window: &web_sys::Window, +) -> Result<(), JsValue> { + if let Some(media_query_list) = prefers_color_scheme_dark(window)? { runner_ref.add_event_listener::( &media_query_list, "change", |event, runner| { let theme = theme_from_dark_mode(event.matches()); - runner.frame.info.system_theme = Some(theme); - runner.egui_ctx().set_visuals(theme.egui_visuals()); + runner.input.raw.system_theme = Some(theme); runner.needs_repaint.repaint_asap(); }, )?; diff --git a/crates/eframe/src/web/mod.rs b/crates/eframe/src/web/mod.rs index 519c4052876..1a6fa6b447c 100644 --- a/crates/eframe/src/web/mod.rs +++ b/crates/eframe/src/web/mod.rs @@ -45,8 +45,6 @@ use web_sys::MediaQueryList; use input::*; -use crate::Theme; - // ---------------------------------------------------------------------------- pub(crate) fn string_from_js_value(value: &JsValue) -> String { @@ -103,7 +101,7 @@ pub fn native_pixels_per_point() -> f32 { /// Ask the browser about the preferred system theme. /// /// `None` means unknown. -pub fn system_theme() -> Option { +pub fn system_theme() -> Option { let dark_mode = prefers_color_scheme_dark(&web_sys::window()?) .ok()?? .matches(); @@ -114,11 +112,11 @@ fn prefers_color_scheme_dark(window: &web_sys::Window) -> Result Theme { +fn theme_from_dark_mode(dark_mode: bool) -> egui::Theme { if dark_mode { - Theme::Dark + egui::Theme::Dark } else { - Theme::Light + egui::Theme::Light } } diff --git a/crates/eframe/src/web/web_runner.rs b/crates/eframe/src/web/web_runner.rs index 12034089305..0e10a10826e 100644 --- a/crates/eframe/src/web/web_runner.rs +++ b/crates/eframe/src/web/web_runner.rs @@ -63,8 +63,6 @@ impl WebRunner { ) -> Result<(), JsValue> { self.destroy(); - let follow_system_theme = web_options.follow_system_theme; - let text_agent = TextAgent::attach(self)?; let runner = AppRunner::new(canvas, web_options, app_creator, text_agent).await?; @@ -83,10 +81,6 @@ impl WebRunner { { events::install_event_handlers(self)?; - if follow_system_theme { - events::install_color_scheme_change_event(self)?; - } - // The resize observer handles calling `request_animation_frame` to start the render loop. events::install_resize_observer(self)?; } diff --git a/crates/egui-winit/src/lib.rs b/crates/egui-winit/src/lib.rs index b7881e3ca84..10d79a3af03 100644 --- a/crates/egui-winit/src/lib.rs +++ b/crates/egui-winit/src/lib.rs @@ -14,7 +14,7 @@ pub use accesskit_winit; pub use egui; #[cfg(feature = "accesskit")] use egui::accesskit; -use egui::{Pos2, Rect, Vec2, ViewportBuilder, ViewportCommand, ViewportId, ViewportInfo}; +use egui::{Pos2, Rect, Theme, Vec2, ViewportBuilder, ViewportCommand, ViewportId, ViewportInfo}; pub use winit; pub mod clipboard; @@ -111,6 +111,7 @@ impl State { viewport_id: ViewportId, display_target: &dyn HasDisplayHandle, native_pixels_per_point: Option, + theme: Option, max_texture_side: Option, ) -> Self { crate::profile_function!(); @@ -150,6 +151,7 @@ impl State { .entry(ViewportId::ROOT) .or_default() .native_pixels_per_point = native_pixels_per_point; + slf.egui_input.system_theme = theme.map(to_egui_theme); if let Some(max_texture_side) = max_texture_side { slf.set_max_texture_side(max_texture_side); @@ -403,6 +405,13 @@ impl State { consumed: false, } } + WindowEvent::ThemeChanged(winit_theme) => { + self.egui_input.system_theme = Some(to_egui_theme(*winit_theme)); + EventResponse { + repaint: true, + consumed: false, + } + } WindowEvent::HoveredFile(path) => { self.egui_input.hovered_files.push(egui::HoveredFile { path: Some(path.clone()), @@ -462,7 +471,6 @@ impl State { | WindowEvent::Occluded(_) | WindowEvent::Resized(_) | WindowEvent::Moved(_) - | WindowEvent::ThemeChanged(_) | WindowEvent::TouchpadPressure { .. } | WindowEvent::CloseRequested => EventResponse { repaint: true, @@ -890,6 +898,13 @@ impl State { } } +fn to_egui_theme(theme: winit::window::Theme) -> Theme { + match theme { + winit::window::Theme::Dark => Theme::Dark, + winit::window::Theme::Light => Theme::Light, + } +} + pub fn inner_rect_in_points(window: &Window, pixels_per_point: f32) -> Option { let inner_pos_px = window.inner_position().ok()?; let inner_pos_px = egui::pos2(inner_pos_px.x as f32, inner_pos_px.y as f32); diff --git a/crates/egui/src/data/input.rs b/crates/egui/src/data/input.rs index 50f20b9d6f9..3bc88037426 100644 --- a/crates/egui/src/data/input.rs +++ b/crates/egui/src/data/input.rs @@ -2,7 +2,7 @@ use epaint::ColorImage; -use crate::{emath::*, Key, ViewportId, ViewportIdMap}; +use crate::{emath::*, Key, Theme, ViewportId, ViewportIdMap}; /// What the integrations provides to egui at the start of each frame. /// @@ -73,6 +73,11 @@ pub struct RawInput { /// /// False when the user alt-tab away from the application, for instance. pub focused: bool, + + /// Does the OS use dark or light mode? + /// + /// `None` means "don't know". + pub system_theme: Option, } impl Default for RawInput { @@ -89,6 +94,7 @@ impl Default for RawInput { hovered_files: Default::default(), dropped_files: Default::default(), focused: true, // integrations opt into global focus tracking + system_theme: None, } } } @@ -117,6 +123,7 @@ impl RawInput { hovered_files: self.hovered_files.clone(), dropped_files: std::mem::take(&mut self.dropped_files), focused: self.focused, + system_theme: self.system_theme, } } @@ -134,6 +141,7 @@ impl RawInput { mut hovered_files, mut dropped_files, focused, + system_theme, } = newer; self.viewport_id = viewport_ids; @@ -147,6 +155,7 @@ impl RawInput { self.hovered_files.append(&mut hovered_files); self.dropped_files.append(&mut dropped_files); self.focused = focused; + self.system_theme = system_theme; } } @@ -189,7 +198,7 @@ pub struct ViewportInfo { /// This should always be set, if known. /// /// On web this takes browser scaling into account, - /// and orresponds to [`window.devicePixelRatio`](https://developer.mozilla.org/en-US/docs/Web/API/Window/devicePixelRatio) in JavaScript. + /// and corresponds to [`window.devicePixelRatio`](https://developer.mozilla.org/en-US/docs/Web/API/Window/devicePixelRatio) in JavaScript. pub native_pixels_per_point: Option, /// Current monitor size in egui points. @@ -1044,6 +1053,7 @@ impl RawInput { hovered_files, dropped_files, focused, + system_theme, } = self; ui.label(format!("Active viwport: {viewport_id:?}")); @@ -1068,6 +1078,7 @@ impl RawInput { ui.label(format!("hovered_files: {}", hovered_files.len())); ui.label(format!("dropped_files: {}", dropped_files.len())); ui.label(format!("focused: {focused}")); + ui.label(format!("system_theme: {system_theme:?}")); ui.scope(|ui| { ui.set_min_height(150.0); ui.label(format!("events: {events:#?}")) diff --git a/crates/egui/src/lib.rs b/crates/egui/src/lib.rs index b4571c88a38..92436215f2f 100644 --- a/crates/egui/src/lib.rs +++ b/crates/egui/src/lib.rs @@ -460,7 +460,7 @@ pub use { layers::{LayerId, Order}, layout::*, load::SizeHint, - memory::{Memory, Options}, + memory::{Memory, Options, Theme}, painter::Painter, response::{InnerResponse, Response}, sense::Sense, diff --git a/crates/egui/src/memory.rs b/crates/egui/src/memory.rs index a09f0b57e31..86aa3526799 100644 --- a/crates/egui/src/memory.rs +++ b/crates/egui/src/memory.rs @@ -8,6 +8,9 @@ use crate::{ ViewportId, ViewportIdMap, ViewportIdSet, }; +mod theme; +pub use theme::Theme; + // ---------------------------------------------------------------------------- /// The data that egui persists between frames. @@ -169,6 +172,21 @@ pub struct Options { #[cfg_attr(feature = "serde", serde(skip))] pub(crate) style: std::sync::Arc