From dd127463c555430cec487af7676bbb04dc41f7d9 Mon Sep 17 00:00:00 2001 From: nerditation Date: Thu, 4 Jan 2024 23:47:07 +0800 Subject: [PATCH] Windows: Make `EventLoopWindowTarget` independent of UserEvent type (#3061) * make `EventLoopWindowTarget` independent of UserEvent type the `EventLoopWindowTarget` is needed for window creation. conceptually, only `EventLoop` and `EventLoopProxy` need to be parameterized, and all other parts of the backend should be agnostic about the user event type, parallel to how `Event` is parameterized, but `WindowEvent` is not. this change removes the dependency on the type of user events from the `EventLoopWindowTarget` for the Windows backend, but keep a phantom data to keep the API intact. to achieve this, I moved the `Receiver` end of the mpsc channel from `ThreadMsgTargetData` into `EventLoop` itself, so the `UserEvent` is only passed between `EventLoop` and `EventLoopProxy`, all other part of the backend just use unit type as a placeholder for user events. it's similar to the macos backend where an erased `EventHandler` trait object is used so all component except `EventLoop` and `EventLoopProxy` need to be parameterized. however `EventLoop` of the Windows backend already use an `Box` to wrap the user provided event handler callback, so no need for an dedicated trait object, I just modified the wrapper to replace the placeholder user event with real value pulled from the channel. I find this is the approach which need minimum change to be made to existing code. but it does the job and could serve as a starting point to future Windows backend re-works. * fix CI clippy failure. * make UserEventPlaceholder a new type instead of alias * invariance is maintained by top-level EventLoopWindowTarget this field is transitional and her to keep API compatibility only. the correct variance and such is already ensured by the top-level `EventLoopWindowTarget`, just use `PhantomData` here. --- src/event_loop.rs | 2 +- src/platform_impl/windows/event_loop.rs | 134 ++++++++++++------ .../windows/event_loop/runner.rs | 2 +- src/platform_impl/windows/window.rs | 2 +- 4 files changed, 94 insertions(+), 46 deletions(-) diff --git a/src/event_loop.rs b/src/event_loop.rs index 6dfa96b1e1..b719e1d996 100644 --- a/src/event_loop.rs +++ b/src/event_loop.rs @@ -50,7 +50,7 @@ pub struct EventLoop { /// `&EventLoop`. pub struct EventLoopWindowTarget { pub(crate) p: platform_impl::EventLoopWindowTarget, - pub(crate) _marker: PhantomData<*mut ()>, // Not Send nor Sync + pub(crate) _marker: PhantomData<*mut T>, // Not Send nor Sync + invariant over T } /// Object that allows building the event loop. diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index e9b35bcc7a..3f3344ef44 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -98,17 +98,38 @@ use self::runner::RunnerState; use super::{window::set_skip_taskbar, SelectedCursor}; -pub(crate) struct WindowData { +/// some backends like macos uses an uninhabited `Never` type, +/// on windows, `UserEvent`s are also dispatched through the +/// WNDPROC callback, and due to the re-entrant nature of the +/// callback, recursively delivered events must be queued in a +/// buffer, the current implementation put this queue in +/// `EventLoopRunner`, which is shared between the event pumping +/// loop and the callback. because it's hard to decide from the +/// outside whether a event needs to be buffered, I decided not +/// use `Event` for the shared runner state, but use unit +/// as a placeholder so user events can be buffered as usual, +/// the real `UserEvent` is pulled from the mpsc channel directly +/// when the placeholder event is delivered to the event handler +pub(crate) struct UserEventPlaceholder; + +// here below, the generic `EventLoopRunnerShared` is replaced with +// `EventLoopRunnerShared` so we can get rid +// of the generic parameter T in types which don't depend on T. +// this is the approach which requires minimum changes to current +// backend implementation. it should be considered transitional +// and should be refactored and cleaned up eventually, I hope. + +pub(crate) struct WindowData { pub window_state: Arc>, - pub event_loop_runner: EventLoopRunnerShared, + pub event_loop_runner: EventLoopRunnerShared, pub key_event_builder: KeyEventBuilder, pub _file_drop_handler: Option, pub userdata_removed: Cell, pub recurse_depth: Cell, } -impl WindowData { - fn send_event(&self, event: Event) { +impl WindowData { + fn send_event(&self, event: Event) { self.event_loop_runner.send_event(event); } @@ -117,13 +138,12 @@ impl WindowData { } } -struct ThreadMsgTargetData { - event_loop_runner: EventLoopRunnerShared, - user_event_receiver: Receiver, +struct ThreadMsgTargetData { + event_loop_runner: EventLoopRunnerShared, } -impl ThreadMsgTargetData { - fn send_event(&self, event: Event) { +impl ThreadMsgTargetData { + fn send_event(&self, event: Event) { self.event_loop_runner.send_event(event); } } @@ -136,7 +156,8 @@ pub(crate) enum ProcResult { } pub struct EventLoop { - thread_msg_sender: Sender, + user_event_sender: Sender, + user_event_receiver: Receiver, window_target: RootELW, msg_hook: Option bool + 'static>>, } @@ -160,7 +181,8 @@ impl Default for PlatformSpecificEventLoopAttributes { pub struct EventLoopWindowTarget { thread_id: u32, thread_msg_target: HWND, - pub(crate) runner_shared: EventLoopRunnerShared, + pub(crate) runner_shared: EventLoopRunnerShared, + _marker: PhantomData, } impl EventLoop { @@ -182,24 +204,26 @@ impl EventLoop { become_dpi_aware(); } - let thread_msg_target = create_event_target_window::(); + let thread_msg_target = create_event_target_window(); let runner_shared = Rc::new(EventLoopRunner::new(thread_msg_target)); - let thread_msg_sender = - insert_event_target_window_data::(thread_msg_target, runner_shared.clone()); + let (user_event_sender, user_event_receiver) = mpsc::channel(); + insert_event_target_window_data(thread_msg_target, runner_shared.clone()); raw_input::register_all_mice_and_keyboards_for_raw_input( thread_msg_target, Default::default(), ); Ok(EventLoop { - thread_msg_sender, + user_event_sender, + user_event_receiver, window_target: RootELW { p: EventLoopWindowTarget { thread_id, thread_msg_target, runner_shared, + _marker: PhantomData, }, _marker: PhantomData, }, @@ -229,11 +253,28 @@ impl EventLoop { } let event_loop_windows_ref = &self.window_target; + let user_event_receiver = &self.user_event_receiver; // # Safety // We make sure to call runner.clear_event_handler() before // returning unsafe { - runner.set_event_handler(move |event| event_handler(event, event_loop_windows_ref)); + runner.set_event_handler(move |event| { + // the shared `EventLoopRunner` is not parameterized + // `EventLoopProxy::send_event()` calls `PostMessage` + // to wakeup and dispatch a placeholder `UserEvent`, + // when we received the placeholder event here, the + // real UserEvent(T) should already be put in the + // mpsc channel and ready to be pulled + let event = match event.map_nonuser_event() { + Ok(non_user_event) => non_user_event, + Err(_user_event_placeholder) => Event::UserEvent( + user_event_receiver + .try_recv() + .expect("user event signaled but not received"), + ), + }; + event_handler(event, event_loop_windows_ref) + }); } } @@ -273,6 +314,7 @@ impl EventLoop { { let runner = &self.window_target.p.runner_shared; let event_loop_windows_ref = &self.window_target; + let user_event_receiver = &self.user_event_receiver; // # Safety // We make sure to call runner.clear_event_handler() before @@ -282,7 +324,17 @@ impl EventLoop { // to leave the runner in an unsound state with an associated // event handler. unsafe { - runner.set_event_handler(move |event| event_handler(event, event_loop_windows_ref)); + runner.set_event_handler(move |event| { + let event = match event.map_nonuser_event() { + Ok(non_user_event) => non_user_event, + Err(_user_event_placeholder) => Event::UserEvent( + user_event_receiver + .recv() + .expect("user event signaled but not received"), + ), + }; + event_handler(event, event_loop_windows_ref) + }); runner.wakeup(); } } @@ -468,7 +520,7 @@ impl EventLoop { pub fn create_proxy(&self) -> EventLoopProxy { EventLoopProxy { target_window: self.window_target.p.thread_msg_target, - event_send: self.thread_msg_sender.clone(), + event_send: self.user_event_sender.clone(), } } @@ -779,14 +831,14 @@ static THREAD_EVENT_TARGET_WINDOW_CLASS: Lazy> = /// pub static TASKBAR_CREATED: LazyMessageId = LazyMessageId::new("TaskbarCreated\0"); -fn create_event_target_window() -> HWND { +fn create_event_target_window() -> HWND { use windows_sys::Win32::UI::WindowsAndMessaging::CS_HREDRAW; use windows_sys::Win32::UI::WindowsAndMessaging::CS_VREDRAW; unsafe { let class = WNDCLASSEXW { cbSize: mem::size_of::() as u32, style: CS_HREDRAW | CS_VREDRAW, - lpfnWndProc: Some(thread_event_target_callback::), + lpfnWndProc: Some(thread_event_target_callback), cbClsExtra: 0, cbWndExtra: 0, hInstance: util::get_instance_handle(), @@ -839,21 +891,14 @@ fn create_event_target_window() -> HWND { } } -fn insert_event_target_window_data( +fn insert_event_target_window_data( thread_msg_target: HWND, - event_loop_runner: EventLoopRunnerShared, -) -> Sender { - let (tx, rx) = mpsc::channel(); - - let userdata = ThreadMsgTargetData { - event_loop_runner, - user_event_receiver: rx, - }; + event_loop_runner: EventLoopRunnerShared, +) { + let userdata = ThreadMsgTargetData { event_loop_runner }; let input_ptr = Box::into_raw(Box::new(userdata)); unsafe { super::set_window_long(thread_msg_target, GWL_USERDATA, input_ptr as isize) }; - - tx } /// Capture mouse input, allowing `window` to receive mouse events when the cursor is outside of @@ -883,7 +928,7 @@ fn normalize_pointer_pressure(pressure: u32) -> Option { /// Emit a `ModifiersChanged` event whenever modifiers have changed. /// Returns the current modifier state -fn update_modifiers(window: HWND, userdata: &WindowData) { +fn update_modifiers(window: HWND, userdata: &WindowData) { use crate::event::WindowEvent::ModifiersChanged; let modifiers = { @@ -905,7 +950,7 @@ fn update_modifiers(window: HWND, userdata: &WindowData) { } } -unsafe fn gain_active_focus(window: HWND, userdata: &WindowData) { +unsafe fn gain_active_focus(window: HWND, userdata: &WindowData) { use crate::event::WindowEvent::Focused; update_modifiers(window, userdata); @@ -916,7 +961,7 @@ unsafe fn gain_active_focus(window: HWND, userdata: &WindowData) { }); } -unsafe fn lose_active_focus(window: HWND, userdata: &WindowData) { +unsafe fn lose_active_focus(window: HWND, userdata: &WindowData) { use crate::event::WindowEvent::{Focused, ModifiersChanged}; userdata.window_state_lock().modifiers_state = ModifiersState::empty(); @@ -973,7 +1018,7 @@ pub(super) unsafe extern "system" fn public_window_callback( return DefWindowProcW(window, msg, wparam, lparam); }, (0, _) => return unsafe { DefWindowProcW(window, msg, wparam, lparam) }, - _ => userdata as *mut WindowData, + _ => userdata as *mut WindowData, }; let (result, userdata_removed, recurse_depth) = { @@ -997,12 +1042,12 @@ pub(super) unsafe extern "system" fn public_window_callback( result } -unsafe fn public_window_callback_inner( +unsafe fn public_window_callback_inner( window: HWND, msg: u32, wparam: WPARAM, lparam: LPARAM, - userdata: &WindowData, + userdata: &WindowData, ) -> LRESULT { let mut result = ProcResult::DefWindowProc(wparam); @@ -2299,14 +2344,14 @@ unsafe fn public_window_callback_inner( } } -unsafe extern "system" fn thread_event_target_callback( +unsafe extern "system" fn thread_event_target_callback( window: HWND, msg: u32, wparam: WPARAM, lparam: LPARAM, ) -> LRESULT { let userdata_ptr = - unsafe { super::get_window_long(window, GWL_USERDATA) } as *mut ThreadMsgTargetData; + unsafe { super::get_window_long(window, GWL_USERDATA) } as *mut ThreadMsgTargetData; if userdata_ptr.is_null() { // `userdata_ptr` will always be null for the first `WM_GETMINMAXINFO`, as well as `WM_NCCREATE` and // `WM_CREATE`. @@ -2359,9 +2404,12 @@ unsafe extern "system" fn thread_event_target_callback( } _ if msg == USER_EVENT_MSG_ID.get() => { - if let Ok(event) = userdata.user_event_receiver.recv() { - userdata.send_event(Event::UserEvent(event)); - } + // synthesis a placeholder UserEvent, so that if the callback is + // re-entered it can be buffered for later delivery. the real + // user event is still in the mpsc channel and will be pulled + // once the placeholder event is delivered to the wrapper + // `event_handler` + userdata.send_event(Event::UserEvent(UserEventPlaceholder)); 0 } _ if msg == EXEC_MSG_ID.get() => { @@ -2384,7 +2432,7 @@ unsafe extern "system" fn thread_event_target_callback( result } -unsafe fn handle_raw_input(userdata: &ThreadMsgTargetData, data: RAWINPUT) { +unsafe fn handle_raw_input(userdata: &ThreadMsgTargetData, data: RAWINPUT) { use crate::event::{ DeviceEvent::{Button, Key, Motion, MouseMotion, MouseWheel}, ElementState::{Pressed, Released}, diff --git a/src/platform_impl/windows/event_loop/runner.rs b/src/platform_impl/windows/event_loop/runner.rs index 7486528de5..4254c41ce8 100644 --- a/src/platform_impl/windows/event_loop/runner.rs +++ b/src/platform_impl/windows/event_loop/runner.rs @@ -416,7 +416,7 @@ impl BufferedEvent { let window_flags = unsafe { let userdata = - get_window_long(window_id.0.into(), GWL_USERDATA) as *mut WindowData; + get_window_long(window_id.0.into(), GWL_USERDATA) as *mut WindowData; (*userdata).window_state_lock().window_flags }; window_flags.set_size((window_id.0).0, inner_size); diff --git a/src/platform_impl/windows/window.rs b/src/platform_impl/windows/window.rs index 823e6e6e7a..b4995ca4a0 100644 --- a/src/platform_impl/windows/window.rs +++ b/src/platform_impl/windows/window.rs @@ -1127,7 +1127,7 @@ impl<'a, T: 'static> InitData<'a, T> { } } - unsafe fn create_window_data(&self, win: &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 { let ole_init_result = unsafe { OleInitialize(ptr::null_mut()) }; // It is ok if the initialize result is `S_FALSE` because it might happen that