From c1fc5a397a0ccd3c943f312d9f3bae065323f44f Mon Sep 17 00:00:00 2001 From: msiglreith Date: Sun, 14 Nov 2021 13:49:16 +0100 Subject: [PATCH 1/3] Refactor window initialization by splitting NCCREATE and CREATE related tasks. Fixes issue with invisible owner windows. --- src/platform_impl/windows/event_loop.rs | 33 +-- src/platform_impl/windows/window.rs | 330 +++++++++++++----------- 2 files changed, 188 insertions(+), 175 deletions(-) diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index 0275f3fc9d..b287429638 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/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(window: HWND, userdata: &WindowData) { } #[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,19 @@ pub(super) unsafe extern "system" fn public_window_callback( let initdata = createstruct.lpCreateParams as LONG_PTR; let initdata = &mut *(initdata as *mut InitData<'_, T>); - let runner = initdata.event_loop.runner_shared.clone(); - if let Some((win, userdata)) = runner.catch_unwind(|| (initdata.post_init)(window)) { - initdata.window = Some(win); - let userdata = Box::into_raw(Box::new(userdata)); - winuser::SetWindowLongPtrW( - window, - winuser::GWL_USERDATA, - userdata as WindowLongPtr, - ); - userdata - } else { - return -1; - } + return initdata.on_nccreate(window, msg, wparam, lparam); } + // `userdata` is set in `WM_NCCREATE` appearing before `WM_CREATE`. // 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>); + + return initdata.on_create(window, msg, wparam, lparam); + } (0, _) => return winuser::DefWindowProcW(window, msg, wparam, lparam), _ => userdata as *mut WindowData, }; @@ -889,11 +885,6 @@ unsafe fn public_window_callback_inner( 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); diff --git a/src/platform_impl/windows/window.rs b/src/platform_impl/windows/window.rs index 7e002e1644..8959214f10 100644 --- a/src/platform_impl/windows/window.rs +++ b/src/platform_impl/windows/window.rs @@ -14,7 +14,7 @@ use std::{ use winapi::{ ctypes::c_int, shared::{ - minwindef::{HINSTANCE, LPARAM, UINT, WPARAM}, + minwindef::{HINSTANCE, LPARAM, LRESULT, UINT, WPARAM}, windef::{HWND, POINT, POINTS, RECT}, }, um::{ @@ -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, 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 { - 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), - } - }) - } + unsafe { init(w_attr, pl_attr, event_loop) } } pub fn set_title(&self, text: &str) { @@ -726,20 +676,184 @@ unsafe impl Send for WindowWrapper {} pub(super) struct InitData<'a, T: 'static> { // inputs pub event_loop: &'a EventLoopWindowTarget, - pub post_init: &'a dyn Fn(HWND) -> (Window, WindowData), + pub attributes: WindowAttributes, + pub pl_attribs: PlatformSpecificWindowBuilderAttributes, + pub window_flags: WindowFlags, // outputs pub window: Option, } -unsafe fn init( +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: &mut Window) -> event_loop::WindowData { + 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), + } + } + + pub unsafe fn on_nccreate( + &mut self, + window: HWND, + msg: UINT, + wparam: WPARAM, + lparam: LPARAM, + ) -> LRESULT { + 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) + }); + + if let Some((win, userdata)) = result { + self.window = Some(win); + let userdata = Box::into_raw(Box::new(userdata)); + winuser::SetWindowLongPtrW(window, winuser::GWL_USERDATA, userdata as _); + winuser::DefWindowProcW(window, msg, wparam, lparam) + } else { + -1 + } + } + + pub unsafe fn on_create( + &mut self, + window: HWND, + msg: UINT, + wparam: WPARAM, + lparam: LPARAM, + ) -> LRESULT { + 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); + } + + winuser::DefWindowProcW(window, msg, wparam, lparam) + } +} +unsafe fn init( attributes: WindowAttributes, pl_attribs: PlatformSpecificWindowBuilderAttributes, event_loop: &EventLoopWindowTarget, - create_window_data: F, ) -> Result where T: 'static, - F: Fn(&mut Window) -> WindowData, { let title = OsStr::new(&attributes.title) .encode_wide() @@ -780,17 +894,9 @@ where let mut initdata = InitData { event_loop, - post_init: &|hwnd| { - let mut window = post_init( - WindowWrapper(hwnd), - attributes.clone(), - pl_attribs.clone(), - window_flags, - event_loop, - ); - let window_data = create_window_data(&mut window); - (window, window_data) - }, + attributes, + pl_attribs: pl_attribs.clone(), + window_flags, window: None, }; @@ -810,7 +916,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 +930,6 @@ where Ok(initdata.window.unwrap()) } -unsafe fn post_init( - real_window: WindowWrapper, - attributes: WindowAttributes, - pl_attribs: PlatformSpecificWindowBuilderAttributes, - window_flags: WindowFlags, - event_loop: &EventLoopWindowTarget, -) -> 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( window_icon: &Option, taskbar_icon: &Option, From e559713925edbb58d50e1d24c0a961e18a0918fc Mon Sep 17 00:00:00 2001 From: msiglreith Date: Wed, 17 Nov 2021 18:07:55 +0100 Subject: [PATCH 2/3] address review comments --- src/platform_impl/windows/event_loop.rs | 13 +++++++-- src/platform_impl/windows/window.rs | 35 +++++++------------------ 2 files changed, 21 insertions(+), 27 deletions(-) diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index b287429638..6330a550b9 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -813,7 +813,15 @@ pub(super) unsafe extern "system" fn public_window_callback( let initdata = createstruct.lpCreateParams as LONG_PTR; let initdata = &mut *(initdata as *mut InitData<'_, T>); - return initdata.on_nccreate(window, msg, wparam, lparam); + let result = match initdata.on_nccreate(window) { + Some(userdata) => { + winuser::SetWindowLongPtrW(window, winuser::GWL_USERDATA, userdata as _); + winuser::DefWindowProcW(window, msg, wparam, lparam) + } + None => -1, // failed to create the window + }; + + return result; } // `userdata` is set in `WM_NCCREATE` appearing before `WM_CREATE`. // Getting here should quite frankly be impossible, @@ -824,7 +832,8 @@ pub(super) unsafe extern "system" fn public_window_callback( let initdata = createstruct.lpCreateParams as LONG_PTR; let initdata = &mut *(initdata as *mut InitData<'_, T>); - return initdata.on_create(window, msg, wparam, lparam); + initdata.on_create(); + return winuser::DefWindowProcW(window, msg, wparam, lparam); } (0, _) => return winuser::DefWindowProcW(window, msg, wparam, lparam), _ => userdata as *mut WindowData, diff --git a/src/platform_impl/windows/window.rs b/src/platform_impl/windows/window.rs index 8959214f10..e3dd723ca0 100644 --- a/src/platform_impl/windows/window.rs +++ b/src/platform_impl/windows/window.rs @@ -14,7 +14,7 @@ use std::{ use winapi::{ ctypes::c_int, shared::{ - minwindef::{HINSTANCE, LPARAM, LRESULT, UINT, WPARAM}, + minwindef::{HINSTANCE, LPARAM, UINT, WPARAM}, windef::{HWND, POINT, POINTS, RECT}, }, um::{ @@ -40,7 +40,7 @@ use crate::{ dark_mode::try_theme, dpi::{dpi_to_scale_factor, enable_non_client_dpi_scaling, hwnd_dpi}, drop_handler::FileDropHandler, - event_loop::{self, EventLoopWindowTarget, DESTROY_MSG_ID}, + event_loop::{self, EventLoopWindowTarget, WindowLongPtr, DESTROY_MSG_ID}, icon::{self, IconType}, monitor, util, window_state::{CursorFlags, SavedWindow, WindowFlags, WindowState}, @@ -739,7 +739,7 @@ impl<'a, T: 'static> InitData<'a, T> { } } - unsafe fn create_window_data(&self, win: &mut Window) -> event_loop::WindowData { + unsafe fn create_window_data(&self, win: &Window) -> event_loop::WindowData { let file_drop_handler = if self.pl_attribs.drag_and_drop { use winapi::shared::winerror::{OLE_E_WRONGCOMPOBJ, RPC_E_CHANGED_MODE, S_OK}; @@ -787,13 +787,9 @@ impl<'a, T: 'static> InitData<'a, T> { } } - pub unsafe fn on_nccreate( - &mut self, - window: HWND, - msg: UINT, - wparam: WPARAM, - lparam: LPARAM, - ) -> LRESULT { + // 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 { let runner = self.event_loop.runner_shared.clone(); let result = runner.catch_unwind(|| unsafe { let mut window = self.create_window(window); @@ -801,23 +797,14 @@ impl<'a, T: 'static> InitData<'a, T> { (window, window_data) }); - if let Some((win, userdata)) = result { + result.map(|(win, userdata)| { self.window = Some(win); let userdata = Box::into_raw(Box::new(userdata)); - winuser::SetWindowLongPtrW(window, winuser::GWL_USERDATA, userdata as _); - winuser::DefWindowProcW(window, msg, wparam, lparam) - } else { - -1 - } + userdata as _ + }) } - pub unsafe fn on_create( - &mut self, - window: HWND, - msg: UINT, - wparam: WPARAM, - lparam: LPARAM, - ) -> LRESULT { + pub unsafe fn on_create(&mut self) { let win = self.window.as_mut().expect("failed window creation"); let attributes = self.attributes.clone(); @@ -843,8 +830,6 @@ impl<'a, T: 'static> InitData<'a, T> { if let Some(position) = attributes.position { win.set_outer_position(position); } - - winuser::DefWindowProcW(window, msg, wparam, lparam) } } unsafe fn init( From 3d8b300cdb63be3a125fe89090a85e3856d0d218 Mon Sep 17 00:00:00 2001 From: Markus Siglreithmaier Date: Wed, 17 Nov 2021 18:14:20 +0100 Subject: [PATCH 3/3] Update src/platform_impl/windows/event_loop.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Markus Røyset --- src/platform_impl/windows/event_loop.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index 6330a550b9..b7bc1cd42b 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -823,7 +823,6 @@ pub(super) unsafe extern "system" fn public_window_callback( return result; } - // `userdata` is set in `WM_NCCREATE` appearing before `WM_CREATE`. // Getting here should quite frankly be impossible, // but we'll make window creation fail here just in case. (0, winuser::WM_CREATE) => return -1,