From 3d7d7661826b4af6727f13265b01bb5914b0e587 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Thu, 20 Jun 2024 16:05:34 +0200 Subject: [PATCH] macOS: set the theme on the NSWindow, instead of application-wide This new implementation uses: - The NSAppearanceCustomization protocol for retrieving the appearance of the window, instead of using the application-wide `-[NSApplication effectiveAppearance]`. - Key-Value observing for observing the `effectiveAppearance` to compute the `ThemeChanged` event, instead of using the undocumented `AppleInterfaceThemeChangedNotification` notification. This also fixes `WindowBuilder::with_theme` not having any effect, and the conversion between `Theme` and `NSAppearance` is made a bit more robust. --- Cargo.toml | 1 + examples/window.rs | 20 ++- src/changelog/unreleased.md | 5 + src/event.rs | 2 + src/platform_impl/macos/window_delegate.rs | 195 +++++++++++++-------- src/window.rs | 14 +- 6 files changed, 153 insertions(+), 84 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 97b8924a3c..c1ae8cf0af 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -125,6 +125,7 @@ features = [ "NSDictionary", "NSDistributedNotificationCenter", "NSEnumerator", + "NSKeyValueObserving", "NSNotification", "NSObjCRuntime", "NSPathUtilities", diff --git a/examples/window.rs b/examples/window.rs index 3a4bd61519..11f324713c 100644 --- a/examples/window.rs +++ b/examples/window.rs @@ -212,6 +212,12 @@ impl Application { Action::PrintHelp => self.print_help(), #[cfg(macos_platform)] Action::CycleOptionAsAlt => window.cycle_option_as_alt(), + Action::SetTheme(theme) => { + window.window.set_theme(theme); + // Get the resulting current theme to draw with + let actual_theme = theme.or_else(|| window.window.theme()).unwrap_or(Theme::Dark); + window.set_draw_theme(actual_theme); + }, #[cfg(macos_platform)] Action::CreateNewTab => { let tab_id = window.window.tabbing_identifier(); @@ -334,7 +340,7 @@ impl ApplicationHandler for Application { }, WindowEvent::ThemeChanged(theme) => { info!("Theme changed to {theme:?}"); - window.set_theme(theme); + window.set_draw_theme(theme); }, WindowEvent::RedrawRequested => { if let Err(err) = window.draw() { @@ -733,8 +739,8 @@ impl WindowState { self.window.request_redraw(); } - /// Change the theme. - fn set_theme(&mut self, theme: Theme) { + /// Change the theme that things are drawn in. + fn set_draw_theme(&mut self, theme: Theme) { self.theme = theme; self.window.request_redraw(); } @@ -884,6 +890,7 @@ enum Action { ShowWindowMenu, #[cfg(macos_platform)] CycleOptionAsAlt, + SetTheme(Option), #[cfg(macos_platform)] CreateNewTab, RequestResize, @@ -915,6 +922,9 @@ impl Action { Action::ShowWindowMenu => "Show window menu", #[cfg(macos_platform)] Action::CycleOptionAsAlt => "Cycle option as alt mode", + Action::SetTheme(None) => "Change to the system theme", + Action::SetTheme(Some(Theme::Light)) => "Change to a light theme", + Action::SetTheme(Some(Theme::Dark)) => "Change to a dark theme", #[cfg(macos_platform)] Action::CreateNewTab => "Create new tab", Action::RequestResize => "Request a resize", @@ -1059,6 +1069,10 @@ const KEY_BINDINGS: &[Binding<&'static str>] = &[ Action::AnimationCustomCursor, ), Binding::new("Z", ModifiersState::CONTROL, Action::ToggleCursorVisibility), + // K. + Binding::new("K", ModifiersState::empty(), Action::SetTheme(None)), + Binding::new("K", ModifiersState::SUPER, Action::SetTheme(Some(Theme::Light))), + Binding::new("K", ModifiersState::CONTROL, Action::SetTheme(Some(Theme::Dark))), #[cfg(macos_platform)] Binding::new("T", ModifiersState::SUPER, Action::CreateNewTab), #[cfg(macos_platform)] diff --git a/src/changelog/unreleased.md b/src/changelog/unreleased.md index a399e03468..6adb37b188 100644 --- a/src/changelog/unreleased.md +++ b/src/changelog/unreleased.md @@ -40,6 +40,11 @@ changelog entry. ## Unreleased +### Changed + +- On macOS, set the window theme on the `NSWindow` instead of application-wide. + ### Fixed - On X11, build on arm platforms. +- On macOS, fixed `WindowBuilder::with_theme` not having any effect on the window. diff --git a/src/event.rs b/src/event.rs index 5cd3877a26..30a71d6514 100644 --- a/src/event.rs +++ b/src/event.rs @@ -389,6 +389,8 @@ pub enum WindowEvent { /// Applications might wish to react to this to change the theme of the content of the window /// when the system changes the window theme. /// + /// This only reports a change if the window theme was not overridden by [`Window::set_theme`]. + /// /// ## Platform-specific /// /// - **iOS / Android / X11 / Wayland / Orbital:** Unsupported. diff --git a/src/platform_impl/macos/window_delegate.rs b/src/platform_impl/macos/window_delegate.rs index 1e3675f1ca..674088cf0a 100644 --- a/src/platform_impl/macos/window_delegate.rs +++ b/src/platform_impl/macos/window_delegate.rs @@ -1,6 +1,8 @@ #![allow(clippy::unnecessary_cast)] use std::cell::{Cell, RefCell}; use std::collections::VecDeque; +use std::ffi::c_void; +use std::ptr; use std::sync::{Arc, Mutex}; use core_graphics::display::{CGDisplay, CGPoint}; @@ -9,18 +11,20 @@ use objc2::rc::{autoreleasepool, Retained}; use objc2::runtime::{AnyObject, ProtocolObject}; use objc2::{declare_class, msg_send_id, mutability, sel, ClassType, DeclaredClass}; use objc2_app_kit::{ - NSAppKitVersionNumber, NSAppKitVersionNumber10_12, NSAppearance, NSApplication, - NSApplicationPresentationOptions, NSBackingStoreType, NSColor, NSDraggingDestination, - NSFilenamesPboardType, NSPasteboard, NSRequestUserAttentionType, NSScreen, NSView, - NSWindowButton, NSWindowDelegate, NSWindowFullScreenButton, NSWindowLevel, - NSWindowOcclusionState, NSWindowOrderingMode, NSWindowSharingType, NSWindowStyleMask, - NSWindowTabbingMode, NSWindowTitleVisibility, + NSAppKitVersionNumber, NSAppKitVersionNumber10_12, NSAppearance, NSAppearanceCustomization, + NSAppearanceNameAqua, NSApplication, NSApplicationPresentationOptions, NSBackingStoreType, + NSColor, NSDraggingDestination, NSFilenamesPboardType, NSPasteboard, + NSRequestUserAttentionType, NSScreen, NSView, NSWindowButton, NSWindowDelegate, + NSWindowFullScreenButton, NSWindowLevel, NSWindowOcclusionState, NSWindowOrderingMode, + NSWindowSharingType, NSWindowStyleMask, NSWindowTabbingMode, NSWindowTitleVisibility, }; use objc2_foundation::{ - ns_string, CGFloat, MainThreadMarker, NSArray, NSCopying, NSDistributedNotificationCenter, - NSObject, NSObjectNSDelayedPerforming, NSObjectNSThreadPerformAdditions, NSObjectProtocol, - NSPoint, NSRect, NSSize, NSString, + ns_string, CGFloat, MainThreadMarker, NSArray, NSCopying, NSDictionary, NSKeyValueChangeKey, + NSKeyValueChangeNewKey, NSKeyValueChangeOldKey, NSKeyValueObservingOptions, NSObject, + NSObjectNSDelayedPerforming, NSObjectNSKeyValueObserverRegistration, NSObjectProtocol, NSPoint, + NSRect, NSSize, NSString, }; +use tracing::{trace, warn}; use super::app_state::ApplicationDelegate; use super::cursor::cursor_from_icon; @@ -79,8 +83,6 @@ pub(crate) struct State { window: Retained, - current_theme: Cell>, - // During `windowDidResize`, we use this to only send Moved if the position changed. // // This is expressed in native screen coordinates. @@ -419,32 +421,66 @@ declare_class!( } } + // Key-Value Observing unsafe impl WindowDelegate { - // Observe theme change - #[method(effectiveAppearanceDidChange:)] - fn effective_appearance_did_change(&self, sender: Option<&AnyObject>) { - trace_scope!("effectiveAppearanceDidChange:"); - unsafe { - self.performSelectorOnMainThread_withObject_waitUntilDone( - sel!(effectiveAppearanceDidChangedOnMainThread:), - sender, - false, - ) - }; - } + #[method(observeValueForKeyPath:ofObject:change:context:)] + fn observe_value( + &self, + key_path: Option<&NSString>, + _object: Option<&AnyObject>, + change: Option<&NSDictionary>, + _context: *mut c_void, + ) { + trace_scope!("observeValueForKeyPath:ofObject:change:context:"); + // NOTE: We don't _really_ need to check the key path, as there should only be one, but + // in the future we might want to observe other key paths. + if key_path == Some(ns_string!("effectiveAppearance")) { + let change = change.expect("requested a change dictionary in `addObserver`, but none was provided"); + let old = change.get(unsafe { NSKeyValueChangeOldKey }).expect("requested change dictionary did not contain `NSKeyValueChangeOldKey`"); + let new = change.get(unsafe { NSKeyValueChangeNewKey }).expect("requested change dictionary did not contain `NSKeyValueChangeNewKey`"); + + // SAFETY: The value of `effectiveAppearance` is `NSAppearance` + let old: *const AnyObject = old; + let old: *const NSAppearance = old.cast(); + let old: &NSAppearance = unsafe { &*old }; + let new: *const AnyObject = new; + let new: *const NSAppearance = new.cast(); + let new: &NSAppearance = unsafe { &*new }; + + trace!(old = %unsafe { old.name() }, new = %unsafe { new.name() }, "effectiveAppearance changed"); + + // Ignore the change if the window's theme is customized by the user (since in that + // case the `effectiveAppearance` is only emitted upon said customization, and then + // it's triggered directly by a user action, and we don't want to emit the event). + if unsafe { self.window().appearance() }.is_some() { + return; + } - #[method(effectiveAppearanceDidChangedOnMainThread:)] - fn effective_appearance_did_changed_on_main_thread(&self, _: Option<&AnyObject>) { - let mtm = MainThreadMarker::from(self); - let theme = get_ns_theme(mtm); - let old_theme = self.ivars().current_theme.replace(Some(theme)); - if old_theme != Some(theme) { - self.queue_event(WindowEvent::ThemeChanged(theme)); + let old = appearance_to_theme(old); + let new = appearance_to_theme(new); + // Check that the theme changed in Winit's terms (the theme might have changed on + // other parameters, such as level of contrast, but the event should not be emitted + // in those cases). + if old == new { + return; + } + + self.queue_event(WindowEvent::ThemeChanged(new)); + } else { + panic!("unknown observed keypath {key_path:?}"); } } } ); +impl Drop for WindowDelegate { + fn drop(&mut self) { + unsafe { + self.window().removeObserver_forKeyPath(self, ns_string!("effectiveAppearance")); + } + } +} + fn new_window( app_delegate: &ApplicationDelegate, attrs: &WindowAttributes, @@ -668,15 +704,13 @@ impl WindowDelegate { let scale_factor = window.backingScaleFactor() as _; - let current_theme = match attrs.preferred_theme { - Some(theme) => Some(theme), - None => Some(get_ns_theme(mtm)), - }; + if let Some(appearance) = theme_to_appearance(attrs.preferred_theme) { + unsafe { window.setAppearance(Some(&appearance)) }; + } let delegate = mtm.alloc().set_ivars(State { app_delegate: app_delegate.retain(), window: window.retain(), - current_theme: Cell::new(current_theme), previous_position: Cell::new(None), previous_scale_factor: Cell::new(scale_factor), resize_increments: Cell::new(resize_increments), @@ -702,14 +736,16 @@ impl WindowDelegate { } window.setDelegate(Some(ProtocolObject::from_ref(&*delegate))); - // Enable theme change event - let notification_center = unsafe { NSDistributedNotificationCenter::defaultCenter() }; + // Listen for theme change event. + // + // SAFETY: The observer is un-registered in the `Drop` of the delegate. unsafe { - notification_center.addObserver_selector_name_object( + window.addObserver_forKeyPath_options_context( &delegate, - sel!(effectiveAppearanceDidChange:), - Some(ns_string!("AppleInterfaceThemeChangedNotification")), - None, + ns_string!("effectiveAppearance"), + NSKeyValueObservingOptions::NSKeyValueObservingOptionNew + | NSKeyValueObservingOptions::NSKeyValueObservingOptionOld, + ptr::null_mut(), ) }; @@ -1615,20 +1651,24 @@ impl WindowDelegate { } } - #[inline] - pub fn theme(&self) -> Option { - self.ivars().current_theme.get() - } - #[inline] pub fn has_focus(&self) -> bool { self.window().isKeyWindow() } + pub fn theme(&self) -> Option { + // Note: We could choose between returning the value of `effectiveAppearance` or + // `appearance`, depending on what the user is asking about: + // - "how should I render on this particular frame". + // - "what is the configuration for this window". + // + // We choose the latter for consistency with the `set_theme` call, though it might also be + // useful to expose the former. + Some(appearance_to_theme(unsafe { &*self.window().appearance()? })) + } + pub fn set_theme(&self, theme: Option) { - let mtm = MainThreadMarker::from(self); - set_ns_theme(theme, mtm); - self.ivars().current_theme.set(theme.or_else(|| Some(get_ns_theme(mtm)))); + unsafe { self.window().setAppearance(theme_to_appearance(theme).as_deref()) }; } #[inline] @@ -1787,34 +1827,39 @@ impl WindowExtMacOS for WindowDelegate { const DEFAULT_STANDARD_FRAME: NSRect = NSRect::new(NSPoint::new(50.0, 50.0), NSSize::new(800.0, 600.0)); -pub(super) fn get_ns_theme(mtm: MainThreadMarker) -> Theme { - let app = NSApplication::sharedApplication(mtm); - if !app.respondsToSelector(sel!(effectiveAppearance)) { - return Theme::Light; - } - let appearance = app.effectiveAppearance(); - let name = appearance - .bestMatchFromAppearancesWithNames(&NSArray::from_id_slice(&[ - NSString::from_str("NSAppearanceNameAqua"), - NSString::from_str("NSAppearanceNameDarkAqua"), - ])) - .unwrap(); - match &*name.to_string() { - "NSAppearanceNameDarkAqua" => Theme::Dark, - _ => Theme::Light, +fn dark_appearance_name() -> &'static NSString { + // Don't use the static `NSAppearanceNameDarkAqua` to allow linking on macOS < 10.14 + ns_string!("NSAppearanceNameDarkAqua") +} + +fn appearance_to_theme(appearance: &NSAppearance) -> Theme { + let best_match = appearance.bestMatchFromAppearancesWithNames(&NSArray::from_id_slice(&[ + unsafe { NSAppearanceNameAqua.copy() }, + dark_appearance_name().copy(), + ])); + if let Some(best_match) = best_match { + if *best_match == *dark_appearance_name() { + Theme::Dark + } else { + Theme::Light + } + } else { + warn!(?appearance, "failed to determine the theme of the appearance"); + // Default to light in this case + Theme::Light } } -fn set_ns_theme(theme: Option, mtm: MainThreadMarker) { - let app = NSApplication::sharedApplication(mtm); - if app.respondsToSelector(sel!(effectiveAppearance)) { - let appearance = theme.map(|t| { - let name = match t { - Theme::Dark => NSString::from_str("NSAppearanceNameDarkAqua"), - Theme::Light => NSString::from_str("NSAppearanceNameAqua"), - }; - NSAppearance::appearanceNamed(&name).unwrap() - }); - app.setAppearance(appearance.as_ref().map(|a| a.as_ref())); +fn theme_to_appearance(theme: Option) -> Option> { + let appearance = match theme? { + Theme::Light => unsafe { NSAppearance::appearanceNamed(NSAppearanceNameAqua) }, + Theme::Dark => NSAppearance::appearanceNamed(dark_appearance_name()), + }; + if let Some(appearance) = appearance { + Some(appearance) + } else { + warn!(?theme, "could not find appearance for theme"); + // Assume system appearance in this case + None } } diff --git a/src/window.rs b/src/window.rs index 76fa34e405..775b3f5352 100644 --- a/src/window.rs +++ b/src/window.rs @@ -393,7 +393,6 @@ impl WindowAttributes { /// /// ## Platform-specific /// - /// - **macOS:** This is an app-wide setting. /// - **Wayland:** This controls only CSD. When using `None` it'll try to use dbus to get the /// system preference. When explicit theme is used, this will avoid dbus all together. /// - **x11:** Build window with `_GTK_THEME_VARIANT` hint set to `dark` or `light`. @@ -1354,11 +1353,12 @@ impl Window { self.window.maybe_queue_on_main(move |w| w.request_user_attention(request_type)) } - /// Sets the current window theme. Use `None` to fallback to system default. + /// Set or override the window theme. + /// + /// Specify `None` to reset the theme to the system default. /// /// ## Platform-specific /// - /// - **macOS:** This is an app-wide setting. /// - **Wayland:** Sets the theme for the client side decorations. Using `None` will use dbus to /// get the system preference. /// - **X11:** Sets `_GTK_THEME_VARIANT` hint to `dark` or `light` and if `None` is used, it @@ -1374,12 +1374,14 @@ impl Window { self.window.maybe_queue_on_main(move |w| w.set_theme(theme)) } - /// Returns the current window theme. + /// Returns the current window theme override. + /// + /// Returns `None` if the current theme is set as the system default, or if it cannot be + /// determined on the current platform. /// /// ## Platform-specific /// - /// - **macOS:** This is an app-wide setting. - /// - **iOS / Android / Wayland / x11 / Orbital:** Unsupported. + /// - **iOS / Android / Wayland / x11 / Orbital:** Unsupported, returns `None`. #[inline] pub fn theme(&self) -> Option { let _span = tracing::debug_span!("winit::Window::theme",).entered();