Windows: Split window initialization across NCCREATE and CREATE (#2062)

* Refactor window initialization by splitting NCCREATE and CREATE related tasks.

Fixes issue with invisible owner windows.

* address review comments

* Update src/platform_impl/windows/event_loop.rs

Co-authored-by: Markus Røyset <maroider@protonmail.com>

Co-authored-by: Markus Røyset <maroider@protonmail.com>

src/platform_impl/windows/event_loop.rs

@@ -38,7 +38,7 @@ use crate::{
     monitor::MonitorHandle as RootMonitorHandle,
     platform_impl::platform::{
         dark_mode::try_theme,
-        dpi::{become_dpi_aware, dpi_to_scale_factor, enable_non_client_dpi_scaling},
+        dpi::{become_dpi_aware, dpi_to_scale_factor},
         drop_handler::FileDropHandler,
         event::{self, handle_extended_keys, process_key_params, vkey_to_winit_vkey},
         monitor::{self, MonitorHandle},
@@ -789,9 +789,9 @@ fn update_modifiers<T>(window: HWND, userdata: &WindowData<T>) {
 }
 
 #[cfg(target_arch = "x86_64")]
-type WindowLongPtr = LONG_PTR;
+pub(crate) type WindowLongPtr = LONG_PTR;
 #[cfg(target_arch = "x86")]
-type WindowLongPtr = LONG;
+pub(crate) type WindowLongPtr = LONG;
 
 /// Any window whose callback is configured to this function will have its events propagated
 /// through the events loop of the thread the window was created in.
@@ -813,23 +813,27 @@ pub(super) unsafe extern "system" fn public_window_callback<T: 'static>(
             let initdata = createstruct.lpCreateParams as LONG_PTR;
             let initdata = &mut *(initdata as *mut InitData<'_, T>);
 
-            let runner = initdata.event_loop.runner_shared.clone();
+            let result = match initdata.on_nccreate(window) {
-            if let Some((win, userdata)) = runner.catch_unwind(|| (initdata.post_init)(window)) {
+                Some(userdata) => {
-                initdata.window = Some(win);
+                    winuser::SetWindowLongPtrW(window, winuser::GWL_USERDATA, userdata as _);
-                let userdata = Box::into_raw(Box::new(userdata));
+                    winuser::DefWindowProcW(window, msg, wparam, lparam)
-                winuser::SetWindowLongPtrW(
-                    window,
-                    winuser::GWL_USERDATA,
-                    userdata as WindowLongPtr,
-                );
-                userdata
-            } else {
-                return -1;
                 }
+                None => -1, // failed to create the window
+            };
+
+            return result;
         }
         // Getting here should quite frankly be impossible,
         // but we'll make window creation fail here just in case.
         (0, winuser::WM_CREATE) => return -1,
+        (_, winuser::WM_CREATE) => {
+            let createstruct = &mut *(lparam as *mut winuser::CREATESTRUCTW);
+            let initdata = createstruct.lpCreateParams as LONG_PTR;
+            let initdata = &mut *(initdata as *mut InitData<'_, T>);
+
+            initdata.on_create();
+            return winuser::DefWindowProcW(window, msg, wparam, lparam);
+        }
         (0, _) => return winuser::DefWindowProcW(window, msg, wparam, lparam),
         _ => userdata as *mut WindowData<T>,
     };
@@ -889,11 +893,6 @@ unsafe fn public_window_callback_inner<T: 'static>(
             0
         }
 
-        winuser::WM_NCCREATE => {
-            enable_non_client_dpi_scaling(window);
-            winuser::DefWindowProcW(window, msg, wparam, lparam)
-        }
-
         winuser::WM_NCLBUTTONDOWN => {
             if wparam == winuser::HTCAPTION as _ {
                 winuser::PostMessageW(window, winuser::WM_MOUSEMOVE, 0, lparam);

src/platform_impl/windows/window.rs

@@ -38,9 +38,9 @@ use crate::{
     monitor::MonitorHandle as RootMonitorHandle,
     platform_impl::platform::{
         dark_mode::try_theme,
-        dpi::{dpi_to_scale_factor, hwnd_dpi},
+        dpi::{dpi_to_scale_factor, enable_non_client_dpi_scaling, hwnd_dpi},
         drop_handler::FileDropHandler,
-        event_loop::{self, EventLoopWindowTarget, WindowData, DESTROY_MSG_ID},
+        event_loop::{self, EventLoopWindowTarget, WindowLongPtr, DESTROY_MSG_ID},
         icon::{self, IconType},
         monitor, util,
         window_state::{CursorFlags, SavedWindow, WindowFlags, WindowState},
@@ -71,57 +71,7 @@ impl Window {
         // First person to remove the need for cloning here gets a cookie!
         //
         // done. you owe me -- ossi
-        unsafe {
+        unsafe { init(w_attr, pl_attr, event_loop) }
-            let drag_and_drop = pl_attr.drag_and_drop;
-            init(w_attr, pl_attr, event_loop, |win| {
-                let file_drop_handler = if drag_and_drop {
-                    use winapi::shared::winerror::{OLE_E_WRONGCOMPOBJ, RPC_E_CHANGED_MODE, S_OK};
-
-                    let ole_init_result = ole2::OleInitialize(ptr::null_mut());
-                    // It is ok if the initialize result is `S_FALSE` because it might happen that
-                    // multiple windows are created on the same thread.
-                    if ole_init_result == OLE_E_WRONGCOMPOBJ {
-                        panic!("OleInitialize failed! Result was: `OLE_E_WRONGCOMPOBJ`");
-                    } else if ole_init_result == RPC_E_CHANGED_MODE {
-                        panic!(
-                            "OleInitialize failed! Result was: `RPC_E_CHANGED_MODE`. \
-                            Make sure other crates are not using multithreaded COM library \
-                            on the same thread or disable drag and drop support."
-                        );
-                    }
-
-                    let file_drop_runner = event_loop.runner_shared.clone();
-                    let file_drop_handler = FileDropHandler::new(
-                        win.window.0,
-                        Box::new(move |event| {
-                            if let Ok(e) = event.map_nonuser_event() {
-                                file_drop_runner.send_event(e)
-                            }
-                        }),
-                    );
-                    let handler_interface_ptr =
-                        &mut (*file_drop_handler.data).interface as LPDROPTARGET;
-
-                    assert_eq!(
-                        ole2::RegisterDragDrop(win.window.0, handler_interface_ptr),
-                        S_OK
-                    );
-                    Some(file_drop_handler)
-                } else {
-                    None
-                };
-
-                event_loop.runner_shared.register_window(win.window.0);
-
-                event_loop::WindowData {
-                    window_state: win.window_state.clone(),
-                    event_loop_runner: event_loop.runner_shared.clone(),
-                    _file_drop_handler: file_drop_handler,
-                    userdata_removed: Cell::new(false),
-                    recurse_depth: Cell::new(0),
-                }
-            })
-        }
     }
 
     pub fn set_title(&self, text: &str) {
@@ -726,20 +676,169 @@ unsafe impl Send for WindowWrapper {}
 pub(super) struct InitData<'a, T: 'static> {
     // inputs
     pub event_loop: &'a EventLoopWindowTarget<T>,
-    pub post_init: &'a dyn Fn(HWND) -> (Window, WindowData<T>),
+    pub attributes: WindowAttributes,
+    pub pl_attribs: PlatformSpecificWindowBuilderAttributes,
+    pub window_flags: WindowFlags,
     // outputs
     pub window: Option<Window>,
 }
 
-unsafe fn init<T, F>(
+impl<'a, T: 'static> InitData<'a, T> {
+    unsafe fn create_window(&self, window: HWND) -> Window {
+        // Register for touch events if applicable
+        {
+            let digitizer = winuser::GetSystemMetrics(winuser::SM_DIGITIZER) as u32;
+            if digitizer & winuser::NID_READY != 0 {
+                winuser::RegisterTouchWindow(window, winuser::TWF_WANTPALM);
+            }
+        }
+
+        let dpi = hwnd_dpi(window);
+        let scale_factor = dpi_to_scale_factor(dpi);
+
+        // making the window transparent
+        if self.attributes.transparent && !self.pl_attribs.no_redirection_bitmap {
+            // Empty region for the blur effect, so the window is fully transparent
+            let region = CreateRectRgn(0, 0, -1, -1);
+
+            let bb = dwmapi::DWM_BLURBEHIND {
+                dwFlags: dwmapi::DWM_BB_ENABLE | dwmapi::DWM_BB_BLURREGION,
+                fEnable: 1,
+                hRgnBlur: region,
+                fTransitionOnMaximized: 0,
+            };
+
+            dwmapi::DwmEnableBlurBehindWindow(window, &bb);
+            DeleteObject(region as _);
+        }
+
+        // If the system theme is dark, we need to set the window theme now
+        // before we update the window flags (and possibly show the
+        // window for the first time).
+        let current_theme = try_theme(window, self.pl_attribs.preferred_theme);
+
+        let window_state = {
+            let window_state = WindowState::new(
+                &self.attributes,
+                self.pl_attribs.taskbar_icon.clone(),
+                scale_factor,
+                current_theme,
+                self.pl_attribs.preferred_theme,
+            );
+            let window_state = Arc::new(Mutex::new(window_state));
+            WindowState::set_window_flags(window_state.lock(), window, |f| *f = self.window_flags);
+            window_state
+        };
+
+        enable_non_client_dpi_scaling(window);
+
+        Window {
+            window: WindowWrapper(window),
+            window_state,
+            thread_executor: self.event_loop.create_thread_executor(),
+        }
+    }
+
+    unsafe fn create_window_data(&self, win: &Window) -> event_loop::WindowData<T> {
+        let file_drop_handler = if self.pl_attribs.drag_and_drop {
+            use winapi::shared::winerror::{OLE_E_WRONGCOMPOBJ, RPC_E_CHANGED_MODE, S_OK};
+
+            let ole_init_result = ole2::OleInitialize(ptr::null_mut());
+            // It is ok if the initialize result is `S_FALSE` because it might happen that
+            // multiple windows are created on the same thread.
+            if ole_init_result == OLE_E_WRONGCOMPOBJ {
+                panic!("OleInitialize failed! Result was: `OLE_E_WRONGCOMPOBJ`");
+            } else if ole_init_result == RPC_E_CHANGED_MODE {
+                panic!(
+                    "OleInitialize failed! Result was: `RPC_E_CHANGED_MODE`. \
+                    Make sure other crates are not using multithreaded COM library \
+                    on the same thread or disable drag and drop support."
+                );
+            }
+
+            let file_drop_runner = self.event_loop.runner_shared.clone();
+            let file_drop_handler = FileDropHandler::new(
+                win.window.0,
+                Box::new(move |event| {
+                    if let Ok(e) = event.map_nonuser_event() {
+                        file_drop_runner.send_event(e)
+                    }
+                }),
+            );
+            let handler_interface_ptr = &mut (*file_drop_handler.data).interface as LPDROPTARGET;
+
+            assert_eq!(
+                ole2::RegisterDragDrop(win.window.0, handler_interface_ptr),
+                S_OK
+            );
+            Some(file_drop_handler)
+        } else {
+            None
+        };
+
+        self.event_loop.runner_shared.register_window(win.window.0);
+
+        event_loop::WindowData {
+            window_state: win.window_state.clone(),
+            event_loop_runner: self.event_loop.runner_shared.clone(),
+            _file_drop_handler: file_drop_handler,
+            userdata_removed: Cell::new(false),
+            recurse_depth: Cell::new(0),
+        }
+    }
+
+    // Returns a pointer to window user data on success.
+    // The user data will be registered for the window and can be accessed within the window event callback.
+    pub unsafe fn on_nccreate(&mut self, window: HWND) -> Option<WindowLongPtr> {
+        let runner = self.event_loop.runner_shared.clone();
+        let result = runner.catch_unwind(|| unsafe {
+            let mut window = self.create_window(window);
+            let window_data = self.create_window_data(&mut window);
+            (window, window_data)
+        });
+
+        result.map(|(win, userdata)| {
+            self.window = Some(win);
+            let userdata = Box::into_raw(Box::new(userdata));
+            userdata as _
+        })
+    }
+
+    pub unsafe fn on_create(&mut self) {
+        let win = self.window.as_mut().expect("failed window creation");
+        let attributes = self.attributes.clone();
+
+        // Set visible before setting the size to ensure the
+        // attribute is correctly applied.
+        win.set_visible(attributes.visible);
+
+        let dimensions = attributes
+            .inner_size
+            .unwrap_or_else(|| PhysicalSize::new(800, 600).into());
+        win.set_inner_size(dimensions);
+        if attributes.maximized {
+            // Need to set MAXIMIZED after setting `inner_size` as
+            // `Window::set_inner_size` changes MAXIMIZED to false.
+            win.set_maximized(true);
+        }
+
+        if attributes.fullscreen.is_some() {
+            win.set_fullscreen(attributes.fullscreen);
+            force_window_active(win.window.0);
+        }
+
+        if let Some(position) = attributes.position {
+            win.set_outer_position(position);
+        }
+    }
+}
+unsafe fn init<T>(
     attributes: WindowAttributes,
     pl_attribs: PlatformSpecificWindowBuilderAttributes,
     event_loop: &EventLoopWindowTarget<T>,
-    create_window_data: F,
 ) -> Result<Window, RootOsError>
 where
     T: 'static,
-    F: Fn(&mut Window) -> WindowData<T>,
 {
     let title = OsStr::new(&attributes.title)
         .encode_wide()
@@ -780,17 +879,9 @@ where
 
     let mut initdata = InitData {
         event_loop,
-        post_init: &|hwnd| {
+        attributes,
-            let mut window = post_init(
+        pl_attribs: pl_attribs.clone(),
-                WindowWrapper(hwnd),
-                attributes.clone(),
-                pl_attribs.clone(),
         window_flags,
-                event_loop,
-            );
-            let window_data = create_window_data(&mut window);
-            (window, window_data)
-        },
         window: None,
     };
 
@@ -810,7 +901,7 @@ where
         &mut initdata as *mut _ as *mut _,
     );
 
-    // If the `post_init` callback in `InitData` panicked, then should resume panicking here
+    // If the window creation in `InitData` panicked, then should resume panicking here
     if let Err(panic_error) = event_loop.runner_shared.take_panic_error() {
         panic::resume_unwind(panic_error)
     }
@@ -824,90 +915,6 @@ where
     Ok(initdata.window.unwrap())
 }
 
-unsafe fn post_init<T: 'static>(
-    real_window: WindowWrapper,
-    attributes: WindowAttributes,
-    pl_attribs: PlatformSpecificWindowBuilderAttributes,
-    window_flags: WindowFlags,
-    event_loop: &EventLoopWindowTarget<T>,
-) -> Window {
-    // Register for touch events if applicable
-    {
-        let digitizer = winuser::GetSystemMetrics(winuser::SM_DIGITIZER) as u32;
-        if digitizer & winuser::NID_READY != 0 {
-            winuser::RegisterTouchWindow(real_window.0, winuser::TWF_WANTPALM);
-        }
-    }
-
-    let dpi = hwnd_dpi(real_window.0);
-    let scale_factor = dpi_to_scale_factor(dpi);
-
-    // making the window transparent
-    if attributes.transparent && !pl_attribs.no_redirection_bitmap {
-        // Empty region for the blur effect, so the window is fully transparent
-        let region = CreateRectRgn(0, 0, -1, -1);
-
-        let bb = dwmapi::DWM_BLURBEHIND {
-            dwFlags: dwmapi::DWM_BB_ENABLE | dwmapi::DWM_BB_BLURREGION,
-            fEnable: 1,
-            hRgnBlur: region,
-            fTransitionOnMaximized: 0,
-        };
-
-        dwmapi::DwmEnableBlurBehindWindow(real_window.0, &bb);
-        DeleteObject(region as _);
-    }
-
-    // If the system theme is dark, we need to set the window theme now
-    // before we update the window flags (and possibly show the
-    // window for the first time).
-    let current_theme = try_theme(real_window.0, pl_attribs.preferred_theme);
-
-    let window_state = {
-        let window_state = WindowState::new(
-            &attributes,
-            pl_attribs.taskbar_icon,
-            scale_factor,
-            current_theme,
-            pl_attribs.preferred_theme,
-        );
-        let window_state = Arc::new(Mutex::new(window_state));
-        WindowState::set_window_flags(window_state.lock(), real_window.0, |f| *f = window_flags);
-        window_state
-    };
-
-    let win = Window {
-        window: real_window,
-        window_state,
-        thread_executor: event_loop.create_thread_executor(),
-    };
-
-    // Set visible before setting the size to ensure the
-    // attribute is correctly applied.
-    win.set_visible(attributes.visible);
-
-    let dimensions = attributes
-        .inner_size
-        .unwrap_or_else(|| PhysicalSize::new(800, 600).into());
-    win.set_inner_size(dimensions);
-    if attributes.maximized {
-        // Need to set MAXIMIZED after setting `inner_size` as
-        // `Window::set_inner_size` changes MAXIMIZED to false.
-        win.set_maximized(true);
-    }
-
-    if attributes.fullscreen.is_some() {
-        win.set_fullscreen(attributes.fullscreen);
-        force_window_active(win.window.0);
-    }
-
-    if let Some(position) = attributes.position {
-        win.set_outer_position(position);
-    }
-
-    win
-}
-
 unsafe fn register_window_class<T: 'static>(
     window_icon: &Option<Icon>,
     taskbar_icon: &Option<Icon>,