Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Begin transition to a trait-based system #3386

Closed
wants to merge 8 commits into from
Next Next commit
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<T>` 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<dyn FnMut>` 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.
  • Loading branch information
nerditation authored and madsmtm committed Dec 23, 2023
commit 5683787781bb0da573289c5ee2131c22ec513f13
133 changes: 92 additions & 41 deletions src/platform_impl/windows/event_loop.rs
Original file line number Diff line number Diff line change
@@ -98,17 +98,38 @@ use self::runner::RunnerState;

use super::{window::set_skip_taskbar, SelectedCursor};

pub(crate) struct WindowData<T: 'static> {
/// 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<Never>` 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
type UserEventPlaceholder = ();

// here below, the generic `EventLoopRunnerShared<T>` is replaced with
// `EventLoopRunnerShared<UserEventPlaceholder>` 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<Mutex<WindowState>>,
pub event_loop_runner: EventLoopRunnerShared<T>,
pub event_loop_runner: EventLoopRunnerShared<UserEventPlaceholder>,
pub key_event_builder: KeyEventBuilder,
pub _file_drop_handler: Option<FileDropHandler>,
pub userdata_removed: Cell<bool>,
pub recurse_depth: Cell<u32>,
}

impl<T> WindowData<T> {
fn send_event(&self, event: Event<T>) {
impl WindowData {
fn send_event(&self, event: Event<UserEventPlaceholder>) {
self.event_loop_runner.send_event(event);
}

@@ -117,13 +138,12 @@ impl<T> WindowData<T> {
}
}

struct ThreadMsgTargetData<T: 'static> {
event_loop_runner: EventLoopRunnerShared<T>,
user_event_receiver: Receiver<T>,
struct ThreadMsgTargetData {
event_loop_runner: EventLoopRunnerShared<UserEventPlaceholder>,
}

impl<T> ThreadMsgTargetData<T> {
fn send_event(&self, event: Event<T>) {
impl ThreadMsgTargetData {
fn send_event(&self, event: Event<UserEventPlaceholder>) {
self.event_loop_runner.send_event(event);
}
}
@@ -136,7 +156,8 @@ pub(crate) enum ProcResult {
}

pub struct EventLoop<T: 'static> {
thread_msg_sender: Sender<T>,
user_event_sender: Sender<T>,
user_event_receiver: Receiver<T>,
window_target: RootELW<T>,
msg_hook: Option<Box<dyn FnMut(*const c_void) -> bool + 'static>>,
}
@@ -160,7 +181,11 @@ impl Default for PlatformSpecificEventLoopAttributes {
pub struct EventLoopWindowTarget<T: 'static> {
thread_id: u32,
thread_msg_target: HWND,
pub(crate) runner_shared: EventLoopRunnerShared<T>,
pub(crate) runner_shared: EventLoopRunnerShared<UserEventPlaceholder>,
// TODO
// eventually should be removed after all the backends refactored,
// but for now should this be invariant or contra-variant to T?
_marker: PhantomData<*mut T>,
}

impl<T: 'static> EventLoop<T> {
@@ -186,20 +211,22 @@ impl<T: 'static> EventLoop<T> {

let runner_shared = Rc::new(EventLoopRunner::new(thread_msg_target));

let thread_msg_sender =
insert_event_target_window_data::<T>(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 +256,28 @@ impl<T: 'static> EventLoop<T> {
}

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 +317,7 @@ impl<T: 'static> EventLoop<T> {
{
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 +327,17 @@ impl<T: 'static> EventLoop<T> {
// 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 +523,7 @@ impl<T: 'static> EventLoop<T> {
pub fn create_proxy(&self) -> EventLoopProxy<T> {
EventLoopProxy {
target_window: self.window_target.p.thread_msg_target,
event_send: self.thread_msg_sender.clone(),
event_send: self.user_event_sender.clone(),
}
}

@@ -782,7 +837,7 @@ fn create_event_target_window<T: 'static>() -> HWND {
let class = WNDCLASSEXW {
cbSize: mem::size_of::<WNDCLASSEXW>() as u32,
style: CS_HREDRAW | CS_VREDRAW,
lpfnWndProc: Some(thread_event_target_callback::<T>),
lpfnWndProc: Some(thread_event_target_callback),
cbClsExtra: 0,
cbWndExtra: 0,
hInstance: util::get_instance_handle(),
@@ -835,21 +890,14 @@ fn create_event_target_window<T: 'static>() -> HWND {
}
}

fn insert_event_target_window_data<T>(
fn insert_event_target_window_data(
thread_msg_target: HWND,
event_loop_runner: EventLoopRunnerShared<T>,
) -> Sender<T> {
let (tx, rx) = mpsc::channel();

let userdata = ThreadMsgTargetData {
event_loop_runner,
user_event_receiver: rx,
};
event_loop_runner: EventLoopRunnerShared<UserEventPlaceholder>,
) {
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
@@ -879,7 +927,7 @@ fn normalize_pointer_pressure(pressure: u32) -> Option<Force> {

/// Emit a `ModifiersChanged` event whenever modifiers have changed.
/// Returns the current modifier state
fn update_modifiers<T>(window: HWND, userdata: &WindowData<T>) {
fn update_modifiers(window: HWND, userdata: &WindowData) {
use crate::event::WindowEvent::ModifiersChanged;

let modifiers = {
@@ -901,7 +949,7 @@ fn update_modifiers<T>(window: HWND, userdata: &WindowData<T>) {
}
}

unsafe fn gain_active_focus<T>(window: HWND, userdata: &WindowData<T>) {
unsafe fn gain_active_focus(window: HWND, userdata: &WindowData) {
use crate::event::WindowEvent::Focused;

update_modifiers(window, userdata);
@@ -912,7 +960,7 @@ unsafe fn gain_active_focus<T>(window: HWND, userdata: &WindowData<T>) {
});
}

unsafe fn lose_active_focus<T>(window: HWND, userdata: &WindowData<T>) {
unsafe fn lose_active_focus(window: HWND, userdata: &WindowData) {
use crate::event::WindowEvent::{Focused, ModifiersChanged};

userdata.window_state_lock().modifiers_state = ModifiersState::empty();
@@ -969,7 +1017,7 @@ pub(super) unsafe extern "system" fn public_window_callback<T: 'static>(
return DefWindowProcW(window, msg, wparam, lparam);
},
(0, _) => return unsafe { DefWindowProcW(window, msg, wparam, lparam) },
_ => userdata as *mut WindowData<T>,
_ => userdata as *mut WindowData,
};

let (result, userdata_removed, recurse_depth) = {
@@ -993,12 +1041,12 @@ pub(super) unsafe extern "system" fn public_window_callback<T: 'static>(
result
}

unsafe fn public_window_callback_inner<T: 'static>(
unsafe fn public_window_callback_inner(
window: HWND,
msg: u32,
wparam: WPARAM,
lparam: LPARAM,
userdata: &WindowData<T>,
userdata: &WindowData,
) -> LRESULT {
let mut result = ProcResult::DefWindowProc(wparam);

@@ -2295,14 +2343,14 @@ unsafe fn public_window_callback_inner<T: 'static>(
}
}

unsafe extern "system" fn thread_event_target_callback<T: 'static>(
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<T>;
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`.
@@ -2355,9 +2403,12 @@ unsafe extern "system" fn thread_event_target_callback<T: 'static>(
}

_ 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(()));
0
}
_ if msg == EXEC_MSG_ID.get() => {
@@ -2380,7 +2431,7 @@ unsafe extern "system" fn thread_event_target_callback<T: 'static>(
result
}

unsafe fn handle_raw_input<T: 'static>(userdata: &ThreadMsgTargetData<T>, data: RAWINPUT) {
unsafe fn handle_raw_input(userdata: &ThreadMsgTargetData, data: RAWINPUT) {
use crate::event::{
DeviceEvent::{Button, Key, Motion, MouseMotion, MouseWheel},
ElementState::{Pressed, Released},
2 changes: 1 addition & 1 deletion src/platform_impl/windows/event_loop/runner.rs
Original file line number Diff line number Diff line change
@@ -412,7 +412,7 @@ impl<T> BufferedEvent<T> {

let window_flags = unsafe {
let userdata =
get_window_long(window_id.0.into(), GWL_USERDATA) as *mut WindowData<T>;
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);
2 changes: 1 addition & 1 deletion src/platform_impl/windows/window.rs
Original file line number Diff line number Diff line change
@@ -1122,7 +1122,7 @@ impl<'a, T: 'static> InitData<'a, T> {
}
}

unsafe fn create_window_data(&self, win: &Window) -> event_loop::WindowData<T> {
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