Add WindowBuilder::with_parent_window (#2548)

* On macOS, add `WindowBuilderExtMacOS::with_parent_window`

* Replace Parent with Option<Id<NSWindow, Shared>>

* Add addChildWindow method on NSWindow instead

* Update with_parent_window to be unsafe fn

* Add unified `with_parent_window`

* Remove `WindowBuilderExtUnix::with_parent`

* Remove `WindowBuilderExtWindows::with_parent_window`

* Clean up CI warnings

* Update CHANGELOG.md

It's `WindowBuilderExtX11` rather than `WindowBuilderExtUnix`

* Rename parent to owner

* Make with_parent_window unsafe and update its doc

* Add another way to get window on mac

* Add more documentations

* Add match arm and panic on invalid varients

* Add Xcb arm

* Update child_window example to make it safer and work in i686

* Remove duplicate entry in CHANGELOG.md

* Propogate error instead of expect

* Replace unreachable to panic

* Add platform note to X11

Co-authored-by: Wu Yu Wei <wusyong9104@gmail.com>

CHANGELOG.md

@@ -8,6 +8,7 @@ And please only add new entries to the top of this list, right below the `# Unre
 
 # Unreleased
 
+- **Breaking:** Removed platform specific `WindowBuilder::with_parent` API in favor of `WindowBuilder::with_parent_window`.
 - On Windows, retain `WS_MAXIMIZE` window style when un-minimizing a maximized window.
 - On Windows, fix left mouse button release event not being sent after `Window::drag_window`.
 - On macOS, run most actions on the main thread, which is strictly more correct, but might make multithreaded applications block slightly more.

examples/child_window.rs

@@ -1,37 +1,39 @@
-#[cfg(all(target_os = "linux", feature = "x11"))]
-use std::collections::HashMap;
-
-#[cfg(all(target_os = "linux", feature = "x11"))]
-use winit::{
-    dpi::{LogicalPosition, LogicalSize, Position},
-    event::{ElementState, Event, KeyboardInput, WindowEvent},
-    event_loop::{ControlFlow, EventLoop, EventLoopWindowTarget},
-    platform::x11::{WindowBuilderExtX11, WindowExtX11},
-    window::{Window, WindowBuilder, WindowId},
-};
-
-#[cfg(all(target_os = "linux", feature = "x11"))]
-fn spawn_child_window(
-    parent: u32,
-    event_loop: &EventLoopWindowTarget<()>,
-    windows: &mut HashMap<u32, Window>,
-) {
-    let child_window = WindowBuilder::new()
-        .with_parent(WindowId::from(parent as u64))
-        .with_title("child window")
-        .with_inner_size(LogicalSize::new(200.0f32, 200.0f32))
-        .with_position(Position::Logical(LogicalPosition::new(0.0, 0.0)))
-        .with_visible(true)
-        .build(event_loop)
-        .unwrap();
-
-    let id = child_window.xlib_window().unwrap() as u32;
-    windows.insert(id, child_window);
-    println!("child window created with id: {}", id);
-}
-
-#[cfg(all(target_os = "linux", feature = "x11"))]
+#[cfg(any(
+    all(target_os = "linux", feature = "x11"),
+    target_os = "macos",
+    target_os = "windows"
+))]
 fn main() {
+    use std::collections::HashMap;
+
+    use raw_window_handle::HasRawWindowHandle;
+    use winit::{
+        dpi::{LogicalPosition, LogicalSize, Position},
+        event::{ElementState, Event, KeyboardInput, WindowEvent},
+        event_loop::{ControlFlow, EventLoop, EventLoopWindowTarget},
+        window::{Window, WindowBuilder, WindowId},
+    };
+
+    fn spawn_child_window(
+        parent: &Window,
+        event_loop: &EventLoopWindowTarget<()>,
+        windows: &mut HashMap<WindowId, Window>,
+    ) {
+        let parent = parent.raw_window_handle();
+        let mut builder = WindowBuilder::new()
+            .with_title("child window")
+            .with_inner_size(LogicalSize::new(200.0f32, 200.0f32))
+            .with_position(Position::Logical(LogicalPosition::new(0.0, 0.0)))
+            .with_visible(true);
+        // `with_parent_window` is unsafe. Parent window must a valid window.
+        builder = unsafe { builder.with_parent_window(Some(parent)) };
+        let child_window = builder.build(event_loop).unwrap();
+
+        let id = child_window.id();
+        windows.insert(id, child_window);
+        println!("child window created with id: {:?}", id);
+    }
+
     let mut windows = HashMap::new();
 
     let event_loop: EventLoop<()> = EventLoop::new();
@@ -42,8 +44,8 @@ fn main() {
         .build(&event_loop)
         .unwrap();
 
-    let root = parent_window.xlib_window().unwrap() as u32;
-    println!("parent window id: {})", root);
+    let root = parent_window;
+    println!("parent window: {:?})", root);
 
     event_loop.run(move |event: Event<'_, ()>, event_loop, control_flow| {
         *control_flow = ControlFlow::Wait;
@@ -55,7 +57,7 @@ fn main() {
                     *control_flow = ControlFlow::Exit;
                 }
                 WindowEvent::CursorEntered { device_id: _ } => {
-                    // println when the cursor entered in a window even if the child window is created
+                    // On x11, println when the cursor entered in a window even if the child window is created
                     // by some key inputs.
                     // the child windows are always placed at (0, 0) with size (200, 200) in the parent window,
                     // so we also can see this log when we move the cursor arround (200, 200) in parent window.
@@ -69,7 +71,7 @@ fn main() {
                         },
                     ..
                 } => {
-                    spawn_child_window(root, event_loop, &mut windows);
+                    spawn_child_window(&root, event_loop, &mut windows);
                 }
                 _ => (),
             }
@@ -77,7 +79,11 @@ fn main() {
     })
 }
 
-#[cfg(not(all(target_os = "linux", feature = "x11")))]
+#[cfg(not(any(
+    all(target_os = "linux", feature = "x11"),
+    target_os = "macos",
+    target_os = "windows"
+)))]
 fn main() {
-    panic!("This example is supported only on x11.");
+    panic!("This example is supported only on x11, macOS, and Windows.");
 }

src/platform/windows.rs

@@ -5,7 +5,7 @@ use crate::{
     event::DeviceId,
     event_loop::EventLoopBuilder,
     monitor::MonitorHandle,
-    platform_impl::{Parent, WinIcon},
+    platform_impl::WinIcon,
     window::{BadIcon, Icon, Window, WindowBuilder},
 };
 
@@ -179,14 +179,8 @@ impl WindowExtWindows for Window {
 
 /// Additional methods on `WindowBuilder` that are specific to Windows.
 pub trait WindowBuilderExtWindows {
-    /// Sets a parent to the window to be created.
-    ///
-    /// A child window has the WS_CHILD style and is confined to the client area of its parent window.
-    ///
-    /// For more information, see <https://docs.microsoft.com/en-us/windows/win32/winmsg/window-features#child-windows>
-    fn with_parent_window(self, parent: HWND) -> WindowBuilder;
-
     /// Set an owner to the window to be created. Can be used to create a dialog box, for example.
+    /// This only works when [`WindowBuilder::with_parent_window`] isn't called or set to `None`.
     /// Can be used in combination with [`WindowExtWindows::set_enable(false)`](WindowExtWindows::set_enable)
     /// on the owner window to create a modal dialog box.
     ///
@@ -235,15 +229,9 @@ pub trait WindowBuilderExtWindows {
 }
 
 impl WindowBuilderExtWindows for WindowBuilder {
-    #[inline]
-    fn with_parent_window(mut self, parent: HWND) -> WindowBuilder {
-        self.platform_specific.parent = Parent::ChildOf(parent);
-        self
-    }
-
     #[inline]
     fn with_owner_window(mut self, parent: HWND) -> WindowBuilder {
-        self.platform_specific.parent = Parent::OwnedBy(parent);
+        self.platform_specific.owner = Some(parent);
         self
     }
 

src/platform/x11.rs

@@ -1,7 +1,6 @@
 use std::os::raw;
 use std::{ptr, sync::Arc};
 
-use crate::window::WindowId;
 use crate::{
     event_loop::{EventLoopBuilder, EventLoopWindowTarget},
     monitor::MonitorHandle,
@@ -172,8 +171,6 @@ pub trait WindowBuilderExtX11 {
     fn with_x11_visual<T>(self, visual_infos: *const T) -> Self;
 
     fn with_x11_screen(self, screen_id: i32) -> Self;
-    /// Build window with parent window.
-    fn with_parent(self, parent_id: WindowId) -> Self;
 
     /// Build window with the given `general` and `instance` names.
     ///
@@ -227,12 +224,6 @@ impl WindowBuilderExtX11 for WindowBuilder {
         self
     }
 
-    #[inline]
-    fn with_parent(mut self, parent_id: WindowId) -> Self {
-        self.platform_specific.parent_id = Some(parent_id.0);
-        self
-    }
-
     #[inline]
     fn with_override_redirect(mut self, override_redirect: bool) -> Self {
         self.platform_specific.override_redirect = override_redirect;

src/platform_impl/linux/mod.rs

@@ -96,8 +96,6 @@ pub struct PlatformSpecificWindowBuilderAttributes {
     #[cfg(feature = "x11")]
     pub screen_id: Option<i32>,
     #[cfg(feature = "x11")]
-    pub parent_id: Option<WindowId>,
-    #[cfg(feature = "x11")]
     pub base_size: Option<Size>,
     #[cfg(feature = "x11")]
     pub override_redirect: bool,
@@ -114,8 +112,6 @@ impl Default for PlatformSpecificWindowBuilderAttributes {
             #[cfg(feature = "x11")]
             screen_id: None,
             #[cfg(feature = "x11")]
-            parent_id: None,
-            #[cfg(feature = "x11")]
             base_size: None,
             #[cfg(feature = "x11")]
             override_redirect: false,

src/platform_impl/linux/x11/window.rs

@@ -10,7 +10,7 @@ use std::{
 
 use libc;
 use raw_window_handle::{RawDisplayHandle, RawWindowHandle, XlibDisplayHandle, XlibWindowHandle};
-use x11_dl::xlib::TrueColor;
+use x11_dl::xlib::{TrueColor, XID};
 
 use crate::{
     dpi::{PhysicalPosition, PhysicalSize, Position, Size},
@@ -122,11 +122,11 @@ impl UnownedWindow {
         pl_attribs: PlatformSpecificWindowBuilderAttributes,
     ) -> Result<UnownedWindow, RootOsError> {
         let xconn = &event_loop.xconn;
-        let root = if let Some(id) = pl_attribs.parent_id {
-            // WindowId is XID under the hood which doesn't exceed u32, so this conversion is lossless
-            u64::from(id) as _
-        } else {
-            event_loop.root
+        let root = match window_attrs.parent_window {
+            Some(RawWindowHandle::Xlib(handle)) => handle.window,
+            Some(RawWindowHandle::Xcb(handle)) => handle.window as XID,
+            Some(raw) => unreachable!("Invalid raw window handle {raw:?} on X11"),
+            None => event_loop.root,
         };
 
         let mut monitors = xconn.available_monitors();

src/platform_impl/macos/appkit/mod.rs

@@ -54,7 +54,7 @@ pub(crate) use self::version::NSAppKitVersion;
 pub(crate) use self::view::{NSTrackingRectTag, NSView};
 pub(crate) use self::window::{
     NSBackingStoreType, NSWindow, NSWindowButton, NSWindowLevel, NSWindowOcclusionState,
-    NSWindowSharingType, NSWindowStyleMask, NSWindowTitleVisibility,
+    NSWindowOrderingMode, NSWindowSharingType, NSWindowStyleMask, NSWindowTitleVisibility,
 };
 
 #[link(name = "AppKit", kind = "framework")]

src/platform_impl/macos/appkit/view.rs

@@ -7,7 +7,7 @@ use objc2::rc::{Id, Shared};
 use objc2::runtime::Object;
 use objc2::{extern_class, extern_methods, msg_send_id, ClassType};
 
-use super::{NSCursor, NSResponder, NSTextInputContext};
+use super::{NSCursor, NSResponder, NSTextInputContext, NSWindow};
 
 extern_class!(
     #[derive(Debug, PartialEq, Eq, Hash)]
@@ -52,6 +52,10 @@ extern_methods!(
 
         #[sel(convertPoint:fromView:)]
         pub fn convertPoint_fromView(&self, point: NSPoint, view: Option<&NSView>) -> NSPoint;
+
+        pub fn window(&self) -> Option<Id<NSWindow, Shared>> {
+            unsafe { msg_send_id![self, window] }
+        }
     }
 
     unsafe impl NSView {

src/platform_impl/macos/appkit/window.rs

@@ -213,6 +213,9 @@ extern_methods!(
 
         #[sel(sendEvent:)]
         pub unsafe fn sendEvent(&self, event: &NSEvent);
+
+        #[sel(addChildWindow:ordered:)]
+        pub unsafe fn addChildWindow(&self, child: &NSWindow, ordered: NSWindowOrderingMode);
     }
 );
 
@@ -379,3 +382,16 @@ pub enum NSWindowSharingType {
 unsafe impl Encode for NSWindowSharingType {
     const ENCODING: Encoding = NSUInteger::ENCODING;
 }
+
+#[allow(dead_code)]
+#[repr(isize)] // NSInteger
+#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
+pub enum NSWindowOrderingMode {
+    NSWindowAbove = 1,
+    NSWindowBelow = -1,
+    NSWindowOut = 0,
+}
+
+unsafe impl Encode for NSWindowOrderingMode {
+    const ENCODING: Encoding = NSInteger::ENCODING;
+}

src/platform_impl/macos/window.rs

@@ -21,6 +21,7 @@ use crate::{
     platform::macos::WindowExtMacOS,
     platform_impl::platform::{
         app_state::AppState,
+        appkit::NSWindowOrderingMode,
         ffi,
         monitor::{self, MonitorHandle, VideoMode},
         util,
@@ -45,7 +46,7 @@ use objc2::{declare_class, msg_send, msg_send_id, sel, ClassType};
 use super::appkit::{
     NSApp, NSAppKitVersion, NSAppearance, NSApplicationPresentationOptions, NSBackingStoreType,
     NSColor, NSCursor, NSFilenamesPboardType, NSRequestUserAttentionType, NSResponder, NSScreen,
-    NSWindow, NSWindowButton, NSWindowLevel, NSWindowSharingType, NSWindowStyleMask,
+    NSView, NSWindow, NSWindowButton, NSWindowLevel, NSWindowSharingType, NSWindowStyleMask,
     NSWindowTitleVisibility,
 };
 
@@ -369,6 +370,38 @@ impl WinitWindow {
         })
         .ok_or_else(|| os_error!(OsError::CreationError("Couldn't create `NSWindow`")))?;
 
+        match attrs.parent_window {
+            Some(RawWindowHandle::AppKit(handle)) => {
+                // SAFETY: Caller ensures the pointer is valid or NULL
+                let parent: Id<NSWindow, Shared> =
+                    match unsafe { Id::retain(handle.ns_window.cast()) } {
+                        Some(window) => window,
+                        None => {
+                            // SAFETY: Caller ensures the pointer is valid or NULL
+                            let parent_view: Id<NSView, Shared> =
+                                match unsafe { Id::retain(handle.ns_view.cast()) } {
+                                    Some(view) => view,
+                                    None => {
+                                        return Err(os_error!(OsError::CreationError(
+                                            "raw window handle should be non-empty"
+                                        )))
+                                    }
+                                };
+                            parent_view.window().ok_or_else(|| {
+                                os_error!(OsError::CreationError(
+                                    "parent view should be installed in a window"
+                                ))
+                            })?
+                        }
+                    };
+                // SAFETY: We know that there are no parent -> child -> parent cycles since the only place in `winit`
+                // where we allow making a window a child window is right here, just after it's been created.
+                unsafe { parent.addChildWindow(&this, NSWindowOrderingMode::NSWindowAbove) };
+            }
+            Some(raw) => panic!("Invalid raw window handle {raw:?} on macOS"),
+            None => (),
+        }
+
         let view = WinitView::new(&this, pl_attrs.accepts_first_mouse);
 
         // The default value of `setWantsBestResolutionOpenGLSurface:` was `false` until

src/platform_impl/windows/mod.rs

@@ -20,16 +20,9 @@ pub(self) use crate::platform_impl::Fullscreen;
 use crate::event::DeviceId as RootDeviceId;
 use crate::icon::Icon;
 
-#[derive(Clone)]
-pub enum Parent {
-    None,
-    ChildOf(HWND),
-    OwnedBy(HWND),
-}
-
 #[derive(Clone)]
 pub struct PlatformSpecificWindowBuilderAttributes {
-    pub parent: Parent,
+    pub owner: Option<HWND>,
     pub menu: Option<HMENU>,
     pub taskbar_icon: Option<Icon>,
     pub no_redirection_bitmap: bool,
@@ -41,7 +34,7 @@ pub struct PlatformSpecificWindowBuilderAttributes {
 impl Default for PlatformSpecificWindowBuilderAttributes {
     fn default() -> Self {
         Self {
-            parent: Parent::None,
+            owner: None,
             menu: None,
             taskbar_icon: None,
             no_redirection_bitmap: false,

src/platform_impl/windows/window.rs

@@ -69,7 +69,7 @@ use crate::{
         monitor::{self, MonitorHandle},
         util,
         window_state::{CursorFlags, SavedWindow, WindowFlags, WindowState},
-        Fullscreen, Parent, PlatformSpecificWindowBuilderAttributes, WindowId,
+        Fullscreen, PlatformSpecificWindowBuilderAttributes, WindowId,
     },
     window::{
         CursorGrabMode, CursorIcon, Theme, UserAttentionType, WindowAttributes, WindowButtons,
@@ -1062,22 +1062,25 @@ where
     // so the diffing later can work.
     window_flags.set(WindowFlags::CLOSABLE, true);
 
-    let parent = match pl_attribs.parent {
-        Parent::ChildOf(parent) => {
+    let parent = match attributes.parent_window {
+        Some(RawWindowHandle::Win32(handle)) => {
             window_flags.set(WindowFlags::CHILD, true);
             if pl_attribs.menu.is_some() {
                 warn!("Setting a menu on a child window is unsupported");
             }
-            Some(parent)
-        }
-        Parent::OwnedBy(parent) => {
-            window_flags.set(WindowFlags::POPUP, true);
-            Some(parent)
-        }
-        Parent::None => {
-            window_flags.set(WindowFlags::ON_TASKBAR, true);
-            None
+            Some(handle.hwnd as HWND)
         }
+        Some(raw) => unreachable!("Invalid raw window handle {raw:?} on Windows"),
+        None => match pl_attribs.owner {
+            Some(parent) => {
+                window_flags.set(WindowFlags::POPUP, true);
+                Some(parent)
+            }
+            None => {
+                window_flags.set(WindowFlags::ON_TASKBAR, true);
+                None
+            }
+        },
     };
 
     let mut initdata = InitData {

src/window.rs

@@ -138,6 +138,7 @@ pub(crate) struct WindowAttributes {
     pub resize_increments: Option<Size>,
     pub content_protected: bool,
     pub window_level: WindowLevel,
+    pub parent_window: Option<RawWindowHandle>,
 }
 
 impl Default for WindowAttributes {
@@ -161,6 +162,7 @@ impl Default for WindowAttributes {
             preferred_theme: None,
             resize_increments: None,
             content_protected: false,
+            parent_window: None,
         }
     }
 }
@@ -401,6 +403,27 @@ impl WindowBuilder {
         self
     }
 
+    /// Build window with parent window.
+    ///
+    /// The default is `None`.
+    ///
+    /// ## Safety
+    ///
+    /// `parent_window` must be a valid window handle.
+    ///
+    /// ## Platform-specific
+    ///
+    /// - **Windows** : A child window has the WS_CHILD style and is confined
+    /// to the client area of its parent window. For more information, see
+    /// <https://docs.microsoft.com/en-us/windows/win32/winmsg/window-features#child-windows>
+    /// - **X11**: A child window is confined to the client area of its parent window.
+    /// - **Android / iOS / Wayland:** Unsupported.
+    #[inline]
+    pub unsafe fn with_parent_window(mut self, parent_window: Option<RawWindowHandle>) -> Self {
+        self.window.parent_window = parent_window;
+        self
+    }
+
     /// Builds the window.
     ///
     /// Possible causes of error include denied permission, incompatible system, and lack of memory.