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.
-    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<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

@@ -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.

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,
-    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<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(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<NSKeyValueChangeKey, AnyObject>>,
+            _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<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);
-        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<Theme>, 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<Theme>) -> Option<Retained<NSAppearance>> {
+    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
     }
 }