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

@@ -125,6 +125,7 @@ features = [
     "NSDictionary",
     "NSDistributedNotificationCenter",
     "NSEnumerator",
+    "NSKeyValueObserving",
     "NSNotification",
     "NSObjCRuntime",
     "NSPathUtilities",

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<UserEvent> 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.
+    /// Change the theme that things are drawn in.
-    fn set_theme(&mut self, theme: Theme) {
+    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<Theme>),
     #[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)]

src/changelog/unreleased.md

@@ -45,6 +45,7 @@ changelog entry.
 - On Web, let events wake up event loop immediately when using
   `ControlFlow::Poll`.
 - Bump MSRV from `1.70` to `1.73`.
+- On macOS, set the window theme on the `NSWindow` instead of application-wide.
 
 ### Removed
 
@@ -55,3 +56,4 @@ changelog entry.
 ### Fixed
 
 - On X11, build on arm platforms.
+- On macOS, fixed `WindowBuilder::with_theme` not having any effect on the window.

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.

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,
+    NSAppKitVersionNumber, NSAppKitVersionNumber10_12, NSAppearance, NSAppearanceCustomization,
-    NSApplicationPresentationOptions, NSBackingStoreType, NSColor, NSDraggingDestination,
+    NSAppearanceNameAqua, NSApplication, NSApplicationPresentationOptions, NSBackingStoreType,
-    NSFilenamesPboardType, NSPasteboard, NSRequestUserAttentionType, NSScreen, NSView,
+    NSColor, NSDraggingDestination, NSFilenamesPboardType, NSPasteboard,
-    NSWindowButton, NSWindowDelegate, NSWindowFullScreenButton, NSWindowLevel,
+    NSRequestUserAttentionType, NSScreen, NSView, NSWindowButton, NSWindowDelegate,
-    NSWindowOcclusionState, NSWindowOrderingMode, NSWindowSharingType, NSWindowStyleMask,
+    NSWindowFullScreenButton, NSWindowLevel, NSWindowOcclusionState, NSWindowOrderingMode,
-    NSWindowTabbingMode, NSWindowTitleVisibility,
+    NSWindowSharingType, NSWindowStyleMask, NSWindowTabbingMode, NSWindowTitleVisibility,
 };
 use objc2_foundation::{
-    ns_string, CGFloat, MainThreadMarker, NSArray, NSCopying, NSDistributedNotificationCenter,
+    ns_string, CGFloat, MainThreadMarker, NSArray, NSCopying, NSDictionary, NSKeyValueChangeKey,
-    NSObject, NSObjectNSDelayedPerforming, NSObjectNSThreadPerformAdditions, NSObjectProtocol,
+    NSKeyValueChangeNewKey, NSKeyValueChangeOldKey, NSKeyValueObservingOptions, NSObject,
-    NSPoint, NSRect, NSSize, NSString,
+    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<WinitWindow>,
 
-    current_theme: Cell<Option<Theme>>,
-
     // 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(observeValueForKeyPath:ofObject:change:context:)]
-        #[method(effectiveAppearanceDidChange:)]
+        fn observe_value(
-        fn effective_appearance_did_change(&self, sender: Option<&AnyObject>) {
+            &self,
-            trace_scope!("effectiveAppearanceDidChange:");
+            key_path: Option<&NSString>,
-            unsafe {
+            _object: Option<&AnyObject>,
-                self.performSelectorOnMainThread_withObject_waitUntilDone(
+            change: Option<&NSDictionary<NSKeyValueChangeKey, AnyObject>>,
-                    sel!(effectiveAppearanceDidChangedOnMainThread:),
+            _context: *mut c_void,
-                    sender,
+        ) {
-                    false,
+            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`");
 
-        #[method(effectiveAppearanceDidChangedOnMainThread:)]
+                // SAFETY: The value of `effectiveAppearance` is `NSAppearance`
-        fn effective_appearance_did_changed_on_main_thread(&self, _: Option<&AnyObject>) {
+                let old: *const AnyObject = old;
-            let mtm = MainThreadMarker::from(self);
+                let old: *const NSAppearance = old.cast();
-            let theme = get_ns_theme(mtm);
+                let old: &NSAppearance = unsafe { &*old };
-            let old_theme = self.ivars().current_theme.replace(Some(theme));
+                let new: *const AnyObject = new;
-            if old_theme != Some(theme) {
+                let new: *const NSAppearance = new.cast();
-                self.queue_event(WindowEvent::ThemeChanged(theme));
+                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;
+                }
+
+                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 {
+        if let Some(appearance) = theme_to_appearance(attrs.preferred_theme) {
-            Some(theme) => Some(theme),
+            unsafe { window.setAppearance(Some(&appearance)) };
-            None => Some(get_ns_theme(mtm)),
+        }
-        };
 
         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
+        // Listen for theme change event.
-        let notification_center = unsafe { NSDistributedNotificationCenter::defaultCenter() };
+        //
+        // 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:),
+                ns_string!("effectiveAppearance"),
-                Some(ns_string!("AppleInterfaceThemeChangedNotification")),
+                NSKeyValueObservingOptions::NSKeyValueObservingOptionNew
-                None,
+                    | NSKeyValueObservingOptions::NSKeyValueObservingOptionOld,
+                ptr::null_mut(),
             )
         };
 
@@ -1615,20 +1651,24 @@ impl WindowDelegate {
         }
     }
 
-    #[inline]
-    pub fn theme(&self) -> Option<Theme> {
-        self.ivars().current_theme.get()
-    }
-
     #[inline]
     pub fn has_focus(&self) -> bool {
         self.window().isKeyWindow()
     }
 
+    pub fn theme(&self) -> Option<Theme> {
+        // 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<Theme>) {
-        let mtm = MainThreadMarker::from(self);
+        unsafe { self.window().setAppearance(theme_to_appearance(theme).as_deref()) };
-        set_ns_theme(theme, mtm);
-        self.ivars().current_theme.set(theme.or_else(|| Some(get_ns_theme(mtm))));
     }
 
     #[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 {
+fn dark_appearance_name() -> &'static NSString {
-    let app = NSApplication::sharedApplication(mtm);
+    // Don't use the static `NSAppearanceNameDarkAqua` to allow linking on macOS < 10.14
-    if !app.respondsToSelector(sel!(effectiveAppearance)) {
+    ns_string!("NSAppearanceNameDarkAqua")
-        return Theme::Light;
+}
-    }
+
-    let appearance = app.effectiveAppearance();
+fn appearance_to_theme(appearance: &NSAppearance) -> Theme {
-    let name = appearance
+    let best_match = appearance.bestMatchFromAppearancesWithNames(&NSArray::from_id_slice(&[
-        .bestMatchFromAppearancesWithNames(&NSArray::from_id_slice(&[
+        unsafe { NSAppearanceNameAqua.copy() },
-            NSString::from_str("NSAppearanceNameAqua"),
+        dark_appearance_name().copy(),
-            NSString::from_str("NSAppearanceNameDarkAqua"),
+    ]));
-        ]))
+    if let Some(best_match) = best_match {
-        .unwrap();
+        if *best_match == *dark_appearance_name() {
-    match &*name.to_string() {
+            Theme::Dark
-        "NSAppearanceNameDarkAqua" => Theme::Dark,
+        } else {
-        _ => Theme::Light,
+            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<Theme>, mtm: MainThreadMarker) {
+fn theme_to_appearance(theme: Option<Theme>) -> Option<Retained<NSAppearance>> {
-    let app = NSApplication::sharedApplication(mtm);
+    let appearance = match theme? {
-    if app.respondsToSelector(sel!(effectiveAppearance)) {
+        Theme::Light => unsafe { NSAppearance::appearanceNamed(NSAppearanceNameAqua) },
-        let appearance = theme.map(|t| {
+        Theme::Dark => NSAppearance::appearanceNamed(dark_appearance_name()),
-            let name = match t {
+    };
-                Theme::Dark => NSString::from_str("NSAppearanceNameDarkAqua"),
+    if let Some(appearance) = appearance {
-                Theme::Light => NSString::from_str("NSAppearanceNameAqua"),
+        Some(appearance)
-            };
+    } else {
-            NSAppearance::appearanceNamed(&name).unwrap()
+        warn!(?theme, "could not find appearance for theme");
-        });
+        // Assume system appearance in this case
-        app.setAppearance(appearance.as_ref().map(|a| a.as_ref()));
+        None
     }
 }

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, returns `None`.
-    /// - **iOS / Android / Wayland / x11 / Orbital:** Unsupported.
     #[inline]
     pub fn theme(&self) -> Option<Theme> {
         let _span = tracing::debug_span!("winit::Window::theme",).entered();