Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eframe: Automatically change theme when system dark/light mode changes #2750

Merged
merged 17 commits into from
Mar 29, 2023
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/eframe/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ web-sys = { version = "0.3.58", features = [
"KeyboardEvent",
"Location",
"MediaQueryList",
"MediaQueryListEvent",
"MouseEvent",
"Navigator",
"Performance",
Expand Down
2 changes: 2 additions & 0 deletions crates/eframe/src/epi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,8 @@ pub struct NativeOptions {
/// By default, this is `true` on Mac and Windows, but `false` on Linux
/// due to <https://github.com/frewsxcv/rust-dark-light/issues/17>.
///
/// On Mac and Windows the theme will automatically change when the dark vs light mode preference is changed.
///
/// See also [`Self::default_theme`].
pub follow_system_theme: bool,

Expand Down
22 changes: 21 additions & 1 deletion crates/eframe/src/native/epi_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ pub fn window_builder<E>(
// Restore pos/size from previous session
window_settings.clamp_to_sane_values(largest_monitor_point_size(event_loop));
#[cfg(windows)]
window_settings.clamp_window_to_sane_position(&event_loop);
window_settings.clamp_window_to_sane_position(event_loop);
window_builder = window_settings.initialize_window(window_builder);
window_settings.inner_size_points()
} else {
Expand Down Expand Up @@ -308,14 +308,18 @@ pub struct EpiIntegration {
close: bool,
can_drag_window: bool,
window_state: WindowState,
#[cfg(feature = "dark-light")]
follow_system_theme: bool,
}

impl EpiIntegration {
#[allow(clippy::too_many_arguments)]
pub fn new<E>(
event_loop: &EventLoopWindowTarget<E>,
max_texture_side: usize,
window: &winit::window::Window,
system_theme: Option<Theme>,
#[cfg(feature = "dark-light")] follow_system_theme: bool,
storage: Option<Box<dyn epi::Storage>>,
#[cfg(feature = "glow")] gl: Option<std::sync::Arc<glow::Context>>,
#[cfg(feature = "wgpu")] wgpu_render_state: Option<egui_wgpu::RenderState>,
Expand Down Expand Up @@ -363,6 +367,8 @@ impl EpiIntegration {
close: false,
can_drag_window: false,
window_state,
#[cfg(feature = "dark-light")]
follow_system_theme,
}
}

Expand Down Expand Up @@ -426,6 +432,12 @@ impl EpiIntegration {
WindowEvent::ScaleFactorChanged { scale_factor, .. } => {
self.frame.info.native_pixels_per_point = Some(*scale_factor as _);
}
#[cfg(feature = "dark-light")]
WindowEvent::ThemeChanged(winit_theme) if self.follow_system_theme => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any reason to limit this on the dark-light feature.

dark-light pulls in a specific dependency for detecting the system theme on start-up. Maybe we can do that via winit instead?

Copy link
Contributor Author

@bash bash Mar 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed #[cfg(feature = "dark-light")] from all places where it's not necessary.

Detecting the theme via winit's theme function works for Windows and macOS—I have tested the latest change on both my Windows 11 and my macOS devices.
The theme is read from the system preferences when the window is created.

The theme function always returns None on Linux though (for both X11 and Wayland), so there we still have to rely on dark-light.

The NativeOptions::system_theme function has become a bit of a liar, since it no longer corresponds with the effectively used system theme on Windows and macOS.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting! Sounds like we maybe don't need dark-light at all then? (since it already doesn't work well on Linux)

let theme = theme_from_winit_theme(*winit_theme);
self.frame.info.system_theme = Some(theme);
self.egui_ctx.set_visuals(theme.egui_visuals());
}
_ => {}
}

Expand Down Expand Up @@ -565,3 +577,11 @@ pub fn load_egui_memory(_storage: Option<&dyn epi::Storage>) -> Option<egui::Mem
#[cfg(not(feature = "persistence"))]
None
}

#[cfg(feature = "dark-light")]
pub(crate) fn theme_from_winit_theme(theme: winit::window::Theme) -> Theme {
match theme {
winit::window::Theme::Dark => Theme::Dark,
winit::window::Theme::Light => Theme::Light,
}
}
4 changes: 4 additions & 0 deletions crates/eframe/src/native/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,8 @@ mod glow_integration {
painter.max_texture_side(),
gl_window.window(),
system_theme,
#[cfg(feature = "dark-light")]
self.native_options.follow_system_theme,
storage,
Some(gl.clone()),
#[cfg(feature = "wgpu")]
Expand Down Expand Up @@ -1123,6 +1125,8 @@ mod wgpu_integration {
painter.max_texture_side().unwrap_or(2048),
&window,
system_theme,
#[cfg(feature = "dark-light")]
self.native_options.follow_system_theme,
storage,
#[cfg(feature = "glow")]
None,
Expand Down
9 changes: 7 additions & 2 deletions crates/eframe/src/web/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -530,15 +530,16 @@ pub async fn start(
tracing::warn!(
"eframe compiled without RUSTFLAGS='--cfg=web_sys_unstable_apis'. Copying text won't work."
);
let follow_system_theme = web_options.follow_system_theme;

let mut runner = AppRunner::new(canvas_id, web_options, app_creator).await?;
runner.warm_up()?;
start_runner(runner)
start_runner(runner, follow_system_theme)
}

/// Install event listeners to register different input events
/// and starts running the given [`AppRunner`].
fn start_runner(app_runner: AppRunner) -> Result<AppRunnerRef, JsValue> {
fn start_runner(app_runner: AppRunner, follow_system_theme: bool) -> Result<AppRunnerRef, JsValue> {
let mut runner_container = AppRunnerContainer {
runner: Arc::new(Mutex::new(app_runner)),
panicked: Arc::new(AtomicBool::new(false)),
Expand All @@ -549,6 +550,10 @@ fn start_runner(app_runner: AppRunner) -> Result<AppRunnerRef, JsValue> {
super::events::install_document_events(&mut runner_container)?;
text_agent::install_text_agent(&mut runner_container)?;

if follow_system_theme {
super::events::install_color_scheme_change_event(&mut runner_container)?;
}

super::events::paint_and_schedule(&runner_container.runner, runner_container.panicked.clone())?;

// Disable all event handlers on panic
Expand Down
21 changes: 21 additions & 0 deletions crates/eframe/src/web/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,27 @@ pub fn install_document_events(runner_container: &mut AppRunnerContainer) -> Res
Ok(())
}

pub fn install_color_scheme_change_event(
runner_container: &mut AppRunnerContainer,
) -> Result<(), JsValue> {
let window = web_sys::window().unwrap();

if let Some(media_query_list) = prefers_color_scheme_dark(&window)? {
runner_container.add_event_listener::<web_sys::MediaQueryListEvent>(
&media_query_list,
"change",
|event, mut runner_lock| {
let theme = theme_from_dark_mode(event.matches());
runner_lock.frame.info.system_theme = Some(theme);
runner_lock.egui_ctx().set_visuals(theme.egui_visuals());
runner_lock.needs_repaint.repaint_asap();
},
)?;
}

Ok(())
}

pub fn install_canvas_events(runner_container: &mut AppRunnerContainer) -> Result<(), JsValue> {
let canvas = canvas_element(runner_container.runner.lock().canvas_id()).unwrap();

Expand Down
19 changes: 15 additions & 4 deletions crates/eframe/src/web/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ use std::sync::{

use egui::Vec2;
use wasm_bindgen::prelude::*;
use web_sys::EventTarget;
use web_sys::{EventTarget, MediaQueryList};

use input::*;

Expand Down Expand Up @@ -75,11 +75,22 @@ pub fn native_pixels_per_point() -> f32 {
}

pub fn system_theme() -> Option<Theme> {
let dark_mode = web_sys::window()?
.match_media("(prefers-color-scheme: dark)")
let dark_mode = prefers_color_scheme_dark(&web_sys::window()?)
.ok()??
.matches();
Some(if dark_mode { Theme::Dark } else { Theme::Light })
Some(theme_from_dark_mode(dark_mode))
}

fn prefers_color_scheme_dark(window: &web_sys::Window) -> Result<Option<MediaQueryList>, JsValue> {
window.match_media("(prefers-color-scheme: dark)")
}

fn theme_from_dark_mode(dark_mode: bool) -> Theme {
if dark_mode {
Theme::Dark
} else {
Theme::Light
}
}

pub fn canvas_element(canvas_id: &str) -> Option<web_sys::HtmlCanvasElement> {
Expand Down