From 87cfe24854cc8bc9bc6a2189fe4047dc2106650a Mon Sep 17 00:00:00 2001 From: Osspial Date: Thu, 5 Mar 2020 13:44:50 -0500 Subject: [PATCH 01/19] Start work on fixing everything more work progress Fix crash with re-entrant WM_PAINT calls --- examples/window_run_return.rs | 6 +- src/platform_impl/windows/event_loop.rs | 395 ++++++++----- .../windows/event_loop/runner.rs | 558 +++++++----------- src/platform_impl/windows/window_state.rs | 4 +- 4 files changed, 441 insertions(+), 522 deletions(-) diff --git a/examples/window_run_return.rs b/examples/window_run_return.rs index 0112eb3e5b..74b4e82be7 100644 --- a/examples/window_run_return.rs +++ b/examples/window_run_return.rs @@ -30,10 +30,10 @@ fn main() { event_loop.run_return(|event, _, control_flow| { *control_flow = ControlFlow::Wait; - if let Event::WindowEvent { event, .. } = &event { - // Print only Window events to reduce noise + // if let Event::WindowEvent { event, .. } = &event { + // // Print only Window events to reduce noise println!("{:?}", event); - } + // } match event { Event::WindowEvent { diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index e5a4d4fce8..723426ec85 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -1,17 +1,4 @@ #![allow(non_snake_case)] -//! An events loop on Win32 is a background thread. -//! -//! Creating an events loop spawns a thread and blocks it in a permanent Win32 events loop. -//! Destroying the events loop stops the thread. -//! -//! You can use the `execute_in_thread` method to execute some code in the background thread. -//! Since Win32 requires you to create a window in the right thread, you must use this method -//! to create a window. -//! -//! If you create a window whose class is set to `callback`, the window's events will be -//! propagated with `run_forever` and `poll_events`. -//! The closure passed to the `execute_in_thread` method takes an `Inserter` that you can use to -//! add a `WindowState` entry to a list of window to be used by the callback. mod runner; @@ -24,6 +11,7 @@ use std::{ mpsc::{self, Receiver, Sender}, Arc, }, + thread, time::{Duration, Instant}, }; use winapi::shared::basetsd::{DWORD_PTR, UINT_PTR}; @@ -41,7 +29,6 @@ use winapi::{ }, }; -use self::runner::{ELRShared, EventLoopRunnerShared}; use crate::{ dpi::{PhysicalPosition, PhysicalSize}, event::{DeviceEvent, Event, Force, KeyboardInput, Touch, TouchPhase, WindowEvent}, @@ -57,6 +44,7 @@ use crate::{ }, window::{Fullscreen, WindowId as RootWindowId}, }; +use runner::{EventLoopRunnerShared, ELRShared}; type GetPointerFrameInfoHistory = unsafe extern "system" fn( pointerId: UINT, @@ -160,9 +148,16 @@ impl EventLoop { pub fn new_dpi_unaware_any_thread() -> EventLoop { let thread_id = unsafe { processthreadsapi::GetCurrentThreadId() }; - let runner_shared = Rc::new(ELRShared::new()); - let (thread_msg_target, thread_msg_sender) = - thread_event_target_window(runner_shared.clone()); + + let thread_msg_target = create_event_target_window(); + + let send_thread_msg_target = thread_msg_target as usize; + thread::spawn(move || wait_thread(thread_id, send_thread_msg_target as HWND)); + let wait_thread_id = get_wait_thread_id(); + + let runner_shared = Rc::new(ELRShared::new(thread_msg_target, wait_thread_id)); + + let thread_msg_sender = subclass_event_target_window(thread_msg_target, runner_shared.clone()); raw_input::register_all_mice_and_keyboards_for_raw_input(thread_msg_target); EventLoop { @@ -200,87 +195,39 @@ impl EventLoop { self.window_target .p .runner_shared - .set_runner(self, move |event, control_flow| { + .set_event_handler(move |event, control_flow| { event_handler(event, event_loop_windows_ref, control_flow) - }) + }); } let runner = &self.window_target.p.runner_shared; unsafe { let mut msg = mem::zeroed(); - let mut unread_message_exists = false; + runner.poll(); 'main: loop { - if let Err(payload) = runner.take_panic_error() { - runner.destroy_runner(); - panic::resume_unwind(payload); + if 0 == winuser::GetMessageW(&mut msg, ptr::null_mut(), 0, 0) { + break 'main; } + winuser::TranslateMessage(&mut msg); + winuser::DispatchMessageW(&mut msg); - runner.new_events(); - loop { - if !unread_message_exists { - if 0 == winuser::PeekMessageW( - &mut msg, - ptr::null_mut(), - 0, - 0, - winuser::PM_REMOVE, - ) { - break; - } - } - winuser::TranslateMessage(&mut msg); - winuser::DispatchMessageW(&mut msg); - - unread_message_exists = false; - - if msg.message == winuser::WM_PAINT { - // An "external" redraw was requested. - // Note that the WM_PAINT has been dispatched and - // has caused the event loop to emit the MainEventsCleared event. - // See EventLoopRunner::process_event(). - // The call to main_events_cleared() below will do nothing. - break; - } + if let Err(payload) = runner.take_panic_error() { + runner.reset_runner(); + panic::resume_unwind(payload); } - // Make sure we emit the MainEventsCleared event if no WM_PAINT message was received. - runner.main_events_cleared(); - // Drain eventual WM_PAINT messages sent if user called request_redraw() - // during handling of MainEventsCleared. - loop { - if 0 == winuser::PeekMessageW( - &mut msg, - ptr::null_mut(), - winuser::WM_PAINT, - winuser::WM_PAINT, - winuser::PM_QS_PAINT | winuser::PM_REMOVE, - ) { - break; - } - winuser::TranslateMessage(&mut msg); - winuser::DispatchMessageW(&mut msg); - } - runner.redraw_events_cleared(); - match runner.control_flow() { - ControlFlow::Exit => break 'main, - ControlFlow::Wait => { - if 0 == winuser::GetMessageW(&mut msg, ptr::null_mut(), 0, 0) { - break 'main; - } - unread_message_exists = true; - } - ControlFlow::WaitUntil(resume_time) => { - wait_until_time_or_msg(resume_time); - } - ControlFlow::Poll => (), + if runner.control_flow() == ControlFlow::Exit && !runner.handling_events() { + break 'main; } } } - runner.destroy_loop(); - runner.destroy_runner(); + unsafe { + runner.call_event_handler(Event::LoopDestroyed); + } + runner.reset_runner(); } pub fn create_proxy(&self) -> EventLoopProxy { @@ -316,24 +263,84 @@ fn main_thread_id() -> DWORD { unsafe { MAIN_THREAD_ID } } -unsafe fn wait_until_time_or_msg(wait_until: Instant) { - let now = Instant::now(); - if now < wait_until { - // MsgWaitForMultipleObjects tends to overshoot just a little bit. We subtract 1 millisecond - // from the requested time and spinlock for the remainder to compensate for that. - let resume_reason = winuser::MsgWaitForMultipleObjectsEx( +fn get_wait_thread_id() -> DWORD { + unsafe { + let mut msg = mem::zeroed(); + let result = winuser::GetMessageW( + &mut msg, + -1 as _, + *SEND_WAIT_THREAD_ID_MSG_ID, + *SEND_WAIT_THREAD_ID_MSG_ID + ); + assert_eq!( + msg.message, + *SEND_WAIT_THREAD_ID_MSG_ID, + "this shouldn't be possible. please open an issue with Winit. error code: {}", + result + ); + msg.lParam as DWORD + } +} + +fn wait_thread(parent_thread_id: DWORD, msg_window_id: HWND) { + unsafe { + let mut msg = mem::zeroed(); + + let cur_thread_id = processthreadsapi::GetCurrentThreadId(); + winuser::PostThreadMessageW( + parent_thread_id, + *SEND_WAIT_THREAD_ID_MSG_ID, 0, - ptr::null(), - dur2timeout(wait_until - now).saturating_sub(1), - winuser::QS_ALLEVENTS, - winuser::MWMO_INPUTAVAILABLE, + cur_thread_id as LPARAM, ); - if resume_reason == winerror::WAIT_TIMEOUT { - let mut msg = mem::zeroed(); - while Instant::now() < wait_until { - if 0 != winuser::PeekMessageW(&mut msg, ptr::null_mut(), 0, 0, 0) { - break; + let mut wait_until_opt = None; + 'main: loop { + if 0 == winuser::GetMessageW(&mut msg, ptr::null_mut(), 0, 0) { + break 'main; + } + + winuser::TranslateMessage(&mut msg); + winuser::DispatchMessageW(&mut msg); + + if msg.message == *WAIT_UNTIL_MSG_ID { + wait_until_opt = Some(*WaitUntilInstantBox::from_raw(msg.lParam as *mut _)); + } else if msg.message == *CANCEL_WAIT_UNTIL_MSG_ID { + wait_until_opt = None; + } + + if let Some(wait_until) = wait_until_opt { + let now = Instant::now(); + if now < wait_until { + // MsgWaitForMultipleObjects tends to overshoot just a little bit. We subtract + // 1 millisecond from the requested time and spinlock for the remainder to + // compensate for that. + let resume_reason = winuser::MsgWaitForMultipleObjectsEx( + 0, + ptr::null(), + dur2timeout(wait_until - now).saturating_sub(1), + winuser::QS_ALLEVENTS, + winuser::MWMO_INPUTAVAILABLE, + ); + if resume_reason == winerror::WAIT_TIMEOUT { + winuser::PostMessageW( + msg_window_id, + *PROCESS_NEW_EVENTS_MSG_ID, + 0, + 0, + ); + wait_until_opt = None; + } + + // TODO: SPINLOCK ON MAIN THREAD + // if resume_reason == winerror::WAIT_TIMEOUT { + // let mut msg = mem::zeroed(); + // while Instant::now() < wait_until { + // if 0 != winuser::PeekMessageW(&mut msg, ptr::null_mut(), 0, 0, 0) { + // break; + // } + // } + // } } } } @@ -461,6 +468,8 @@ impl EventLoopProxy { } } +type WaitUntilInstantBox = Box; + lazy_static! { // Message sent by the `EventLoopProxy` when we want to wake up the thread. // WPARAM and LPARAM are unused. @@ -477,6 +486,29 @@ lazy_static! { winuser::RegisterWindowMessageA("Winit::ExecMsg\0".as_ptr() as *const i8) } }; + static ref PROCESS_NEW_EVENTS_MSG_ID: u32 = { + unsafe { + winuser::RegisterWindowMessageA("Winit::ProcessNewEvents\0".as_ptr() as *const i8) + } + }; + /// lparam is the wait thread's message id. + static ref SEND_WAIT_THREAD_ID_MSG_ID: u32 = { + unsafe { + winuser::RegisterWindowMessageA("Winit::SendWaitThreadId\0".as_ptr() as *const i8) + } + }; + /// lparam points to a `Box` signifying the time `PROCESS_NEW_EVENTS_MSG_ID` should + /// be sent. + static ref WAIT_UNTIL_MSG_ID: u32 = { + unsafe { + winuser::RegisterWindowMessageA("Winit::WaitUntil\0".as_ptr() as *const i8) + } + }; + static ref CANCEL_WAIT_UNTIL_MSG_ID: u32 = { + unsafe { + winuser::RegisterWindowMessageA("Winit::CancelWaitUntil\0".as_ptr() as *const i8) + } + }; // Message sent by a `Window` when it wants to be destroyed by the main thread. // WPARAM and LPARAM are unused. pub static ref DESTROY_MSG_ID: u32 = { @@ -519,7 +551,7 @@ lazy_static! { }; } -fn thread_event_target_window(event_loop_runner: EventLoopRunnerShared) -> (HWND, Sender) { +fn create_event_target_window() -> HWND { unsafe { let window = winuser::CreateWindowExW( winuser::WS_EX_NOACTIVATE | winuser::WS_EX_TRANSPARENT | winuser::WS_EX_LAYERED, @@ -543,7 +575,12 @@ fn thread_event_target_window(event_loop_runner: EventLoopRunnerShared) -> // the LAYERED style. (winuser::WS_VISIBLE | winuser::WS_POPUP) as _, ); + window + } +} +fn subclass_event_target_window(window: HWND, event_loop_runner: EventLoopRunnerShared) -> Sender { + unsafe { let (tx, rx) = mpsc::channel(); let subclass_input = ThreadMsgTargetSubclassInput { @@ -559,7 +596,7 @@ fn thread_event_target_window(event_loop_runner: EventLoopRunnerShared) -> ); assert_eq!(subclass_result, 1); - (window, tx) + tx } } @@ -582,6 +619,7 @@ unsafe fn release_mouse(window_state: &mut WindowState) { const WINDOW_SUBCLASS_ID: UINT_PTR = 0; const THREAD_EVENT_TARGET_SUBCLASS_ID: UINT_PTR = 1; pub(crate) fn subclass_window(window: HWND, subclass_input: SubclassInput) { + subclass_input.event_loop_runner.register_window(window); let input_ptr = Box::into_raw(Box::new(subclass_input)); let subclass_result = unsafe { commctrl::SetWindowSubclass( @@ -601,6 +639,56 @@ fn normalize_pointer_pressure(pressure: u32) -> Option { } } +unsafe fn flush_paint_messages(except: Option, runner: &ELRShared) -> bool { + if !runner.redrawing() { + runner.main_events_cleared(); + let mut msg = mem::zeroed(); + runner.owned_windows(|redraw_window| { + if Some(redraw_window) == except { + return; + } + + if 0 == winuser::PeekMessageW( + &mut msg, + redraw_window, + winuser::WM_PAINT, + winuser::WM_PAINT, + winuser::PM_REMOVE, + ) { + return; + } + + winuser::TranslateMessage(&mut msg); + winuser::DispatchMessageW(&mut msg); + }); + true + } else { + false + } +} + +unsafe fn process_control_flow(runner: &ELRShared) { + match runner.control_flow() { + ControlFlow::Poll => { + winuser::PostMessageW( + runner.thread_msg_target(), + *PROCESS_NEW_EVENTS_MSG_ID, + 0, 0 + ); + }, + ControlFlow::Wait => (), + ControlFlow::WaitUntil(until) => { + winuser::PostThreadMessageW( + runner.wait_thread_id(), + *WAIT_UNTIL_MSG_ID, + 0, + Box::into_raw(WaitUntilInstantBox::new(until)) as LPARAM + ); + }, + ControlFlow::Exit => (), + } +} + /// Emit a `ModifiersChanged` event whenever modifiers have changed. fn update_modifiers(window: HWND, subclass_input: &SubclassInput) { use crate::event::WindowEvent::ModifiersChanged; @@ -641,18 +729,19 @@ unsafe extern "system" fn public_window_callback( match msg { winuser::WM_ENTERSIZEMOVE => { - subclass_input.event_loop_runner.set_modal_loop(true); + subclass_input.window_state.lock().set_window_flags_in_place(|f| f.insert(WindowFlags::MARKER_IN_SIZE_MOVE)); 0 - } + }, + winuser::WM_EXITSIZEMOVE => { - subclass_input.event_loop_runner.set_modal_loop(false); + subclass_input.window_state.lock().set_window_flags_in_place(|f| f.remove(WindowFlags::MARKER_IN_SIZE_MOVE)); 0 - } + }, + winuser::WM_NCCREATE => { enable_non_client_dpi_scaling(window); commctrl::DefSubclassProc(window, msg, wparam, lparam) } - winuser::WM_NCLBUTTONDOWN => { if wparam == winuser::HTCAPTION as _ { winuser::PostMessageW(window, winuser::WM_MOUSEMOVE, 0, 0); @@ -676,6 +765,7 @@ unsafe extern "system" fn public_window_callback( window_id: RootWindowId(WindowId(window)), event: Destroyed, }); + subclass_input.event_loop_runner.remove_window(window); drop(subclass_input); Box::from_raw(subclass_input_ptr as *mut SubclassInput); @@ -683,9 +773,27 @@ unsafe extern "system" fn public_window_callback( } winuser::WM_PAINT => { - subclass_input.send_event(Event::RedrawRequested(RootWindowId(WindowId(window)))); + if subclass_input.event_loop_runner.should_buffer() { + // this branch can happen in response to `UpdateWindow`, if Windows decides to + // redraw the window outside the normal flow of the event loop. + winuser::RedrawWindow( + window, + ptr::null(), + ptr::null_mut(), + winuser::RDW_INTERNALPAINT, + ); + + } else { + let managing_redraw = flush_paint_messages(Some(window), &subclass_input.event_loop_runner); + subclass_input.send_event(Event::RedrawRequested(RootWindowId(WindowId(window)))); + if managing_redraw { + subclass_input.event_loop_runner.redraw_events_cleared(); + process_control_flow(&subclass_input.event_loop_runner); + } + } + commctrl::DefSubclassProc(window, msg, wparam, lparam) - } + }, winuser::WM_WINDOWPOSCHANGING => { let mut window_state = subclass_input.window_state.lock(); @@ -1583,11 +1691,17 @@ unsafe extern "system" fn public_window_callback( }, }); - // Unset maximized if we're changing the window's size. - if new_physical_inner_size != old_physical_inner_size { - WindowState::set_window_flags(subclass_input.window_state.lock(), window, |f| { - f.set(WindowFlags::MAXIMIZED, false) - }); + let dragging_window: bool; + + { + let window_state = subclass_input.window_state.lock(); + dragging_window = window_state.window_flags().contains(WindowFlags::MARKER_IN_SIZE_MOVE); + // Unset maximized if we're changing the window's size. + if new_physical_inner_size != old_physical_inner_size { + WindowState::set_window_flags(window_state, window, |f| { + f.set(WindowFlags::MAXIMIZED, false) + }); + } } let new_outer_rect: RECT; @@ -1612,9 +1726,8 @@ unsafe extern "system" fn public_window_callback( ) .unwrap_or(conservative_rect); - // If we're not dragging the window, offset the window so that the cursor's + // If we're dragging the window, offset the window so that the cursor's // relative horizontal position in the title bar is preserved. - let dragging_window = subclass_input.event_loop_runner.in_modal_loop(); if dragging_window { let bias = { let cursor_pos = { @@ -1764,52 +1877,11 @@ unsafe extern "system" fn thread_event_target_callback( // when the event queue has been emptied. See `process_event` for more details. winuser::WM_PAINT => { winuser::ValidateRect(window, ptr::null()); - let queue_call_again = || { - winuser::RedrawWindow( - window, - ptr::null(), - ptr::null_mut(), - winuser::RDW_INTERNALPAINT, - ); - }; - let in_modal_loop = subclass_input.event_loop_runner.in_modal_loop(); - if in_modal_loop { - let runner = &subclass_input.event_loop_runner; - runner.main_events_cleared(); - // Drain eventual WM_PAINT messages sent if user called request_redraw() - // during handling of MainEventsCleared. - let mut msg = mem::zeroed(); - loop { - if 0 == winuser::PeekMessageW( - &mut msg, - ptr::null_mut(), - winuser::WM_PAINT, - winuser::WM_PAINT, - winuser::PM_QS_PAINT | winuser::PM_REMOVE, - ) { - break; - } - if msg.hwnd != window { - winuser::TranslateMessage(&mut msg); - winuser::DispatchMessageW(&mut msg); - } - } - runner.redraw_events_cleared(); - match runner.control_flow() { - // Waiting is handled by the modal loop. - ControlFlow::Exit | ControlFlow::Wait => runner.new_events(), - ControlFlow::WaitUntil(resume_time) => { - wait_until_time_or_msg(resume_time); - runner.new_events(); - queue_call_again(); - } - ControlFlow::Poll => { - runner.new_events(); - queue_call_again(); - } - } - } + flush_paint_messages(None, &subclass_input.event_loop_runner); + subclass_input.event_loop_runner.redraw_events_cleared(); + process_control_flow(&subclass_input.event_loop_runner); + 0 } @@ -1940,6 +2012,15 @@ unsafe extern "system" fn thread_event_target_callback( function(); 0 } + _ if msg == *PROCESS_NEW_EVENTS_MSG_ID => { + winuser::PostThreadMessageW( + subclass_input.event_loop_runner.wait_thread_id(), + *CANCEL_WAIT_UNTIL_MSG_ID, + 0, 0 + ); + subclass_input.event_loop_runner.poll(); + 0 + } _ => commctrl::DefSubclassProc(window, msg, wparam, lparam), } } diff --git a/src/platform_impl/windows/event_loop/runner.rs b/src/platform_impl/windows/event_loop/runner.rs index e5c062b035..e1aa2258fa 100644 --- a/src/platform_impl/windows/event_loop/runner.rs +++ b/src/platform_impl/windows/event_loop/runner.rs @@ -1,33 +1,39 @@ -use std::{any::Any, cell::RefCell, collections::VecDeque, mem, panic, ptr, rc::Rc, time::Instant}; +use std::{any::Any, cell::{Cell, RefCell}, collections::{HashSet, VecDeque}, mem, ptr, panic, rc::Rc, time::Instant}; -use winapi::{shared::windef::HWND, um::winuser}; +use winapi::{um::winuser, shared::{minwindef::DWORD, windef::HWND}}; use crate::{ dpi::PhysicalSize, event::{Event, StartCause, WindowEvent}, event_loop::ControlFlow, - platform_impl::platform::event_loop::{util, EventLoop}, + platform_impl::platform::util, window::WindowId, }; pub(crate) type EventLoopRunnerShared = Rc>; pub(crate) struct ELRShared { - runner: RefCell>>, - buffer: RefCell>>, -} - -struct EventLoopRunner { - control_flow: ControlFlow, - runner_state: RunnerState, - modal_redraw_window: HWND, - in_modal_loop: bool, - event_handler: Box, &mut ControlFlow)>, - panic_error: Option, + thread_msg_target: HWND, + wait_thread_id: DWORD, + processing_events: Cell, + panic_error: Cell>, + control_flow: Cell, + last_events_cleared: Cell, + event_handler: Cell, &mut ControlFlow)>>>, + event_buffer: RefCell>>, + owned_windows: Cell>, } pub type PanicError = Box; -pub enum BufferedEvent { +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +enum ProcessingEvents { + Uninitialized, + NoEvents, + MainEvents, + RedrawEvents, +} + +enum BufferedEvent { Event(Event<'static, T>), ScaleFactorChanged(WindowId, f64, PhysicalSize), } @@ -69,407 +75,237 @@ impl BufferedEvent { } impl ELRShared { - pub(crate) fn new() -> ELRShared { + pub(crate) fn new(thread_msg_target: HWND, wait_thread_id: DWORD) -> ELRShared { ELRShared { - runner: RefCell::new(None), - buffer: RefCell::new(VecDeque::new()), + thread_msg_target, + wait_thread_id, + processing_events: Cell::new(ProcessingEvents::Uninitialized), + control_flow: Cell::new(ControlFlow::Poll), + panic_error: Cell::new(None), + last_events_cleared: Cell::new(Instant::now()), + event_handler: Cell::new(None), + event_buffer: RefCell::new(VecDeque::new()), + owned_windows: Cell::new(HashSet::new()) } } - pub(crate) unsafe fn set_runner(&self, event_loop: &EventLoop, f: F) + pub fn thread_msg_target(&self) -> HWND { + self.thread_msg_target + } + + pub fn wait_thread_id(&self) -> DWORD { + self.wait_thread_id + } + + pub(crate) unsafe fn set_event_handler(&self, f: F) where F: FnMut(Event<'_, T>, &mut ControlFlow), { - let mut runner = EventLoopRunner::new(event_loop, f); - { - let mut runner_ref = self.runner.borrow_mut(); - // Dispatch any events that were buffered during the creation of the window - self.dispatch_buffered_events(&mut runner); - *runner_ref = Some(runner); - } + let old_event_handler = self.event_handler.replace( + mem::transmute::< + Option, &mut ControlFlow)>>, + Option, &mut ControlFlow)>>, + >(Some(Box::new(f))), + ); + assert!(old_event_handler.is_none()); } - pub(crate) fn destroy_runner(&self) { - *self.runner.borrow_mut() = None; + pub(crate) fn reset_runner(&self) { + let ELRShared { + thread_msg_target: _, + wait_thread_id: _, + processing_events, + panic_error, + control_flow, + last_events_cleared: _, + event_handler, + event_buffer: _, + owned_windows: _, + } = self; + processing_events.set(ProcessingEvents::Uninitialized); + panic_error.set(None); + control_flow.set(ControlFlow::Poll); + event_handler.set(None); } - pub(crate) fn new_events(&self) { - let mut runner_ref = self.runner.borrow_mut(); - if let Some(ref mut runner) = *runner_ref { - runner.new_events(); - // Dispatch any events that were buffered during the call `new_events` - self.dispatch_buffered_events(runner); - } + pub(crate) unsafe fn poll(&self) { + self.move_state_to(ProcessingEvents::MainEvents); } - pub(crate) fn send_event(&self, event: Event<'_, T>) { - if let Err(event) = self.send_event_unbuffered(event) { - // If the runner is already borrowed, we're in the middle of an event loop invocation. - // Add the event to a buffer to be processed later. - if let Event::RedrawRequested(_) = event { - panic!("buffering RedrawRequested event"); + pub(crate) unsafe fn send_event(&self, event: Event<'_, T>) { + if let Event::RedrawRequested(_) = event { + self.move_state_to(ProcessingEvents::RedrawEvents); + self.call_event_handler(event); + } else { + if self.should_buffer() { + // If the runner is already borrowed, we're in the middle of an event loop invocation. Add + // the event to a buffer to be processed later. + self.event_buffer + .borrow_mut() + .push_back(BufferedEvent::from_event(event)) + } else { + self.move_state_to(ProcessingEvents::MainEvents); + self.call_event_handler(event); + self.dispatch_buffered_events(); } - self.buffer - .borrow_mut() - .push_back(BufferedEvent::from_event(event)); } } - fn send_event_unbuffered<'e>(&self, event: Event<'e, T>) -> Result<(), Event<'e, T>> { - if let Ok(mut runner_ref) = self.runner.try_borrow_mut() { - if let Some(ref mut runner) = *runner_ref { - runner.process_event(event); - // Dispatch any events that were buffered during the call to `process_event`. - self.dispatch_buffered_events(runner); - return Ok(()); - } - } - Err(event) + pub(crate) unsafe fn main_events_cleared(&self) { + self.move_state_to(ProcessingEvents::RedrawEvents); } - fn dispatch_buffered_events(&self, runner: &mut EventLoopRunner) { - // We do this instead of using a `while let` loop because if we use a `while let` - // loop the reference returned `borrow_mut()` doesn't get dropped until the end - // of the loop's body and attempts to add events to the event buffer while in - // `process_event` will fail. - loop { - let buffered_event_opt = self.buffer.borrow_mut().pop_front(); - match buffered_event_opt { - Some(e) => e.dispatch_event(|e| runner.process_event(e)), - None => break, - } - } + pub(crate) unsafe fn redraw_events_cleared(&self) { + self.move_state_to(ProcessingEvents::NoEvents); } - pub(crate) fn main_events_cleared(&self) { - let mut runner_ref = self.runner.borrow_mut(); - if let Some(ref mut runner) = *runner_ref { - runner.main_events_cleared(); - if !self.buffer.borrow().is_empty() { - warn!("Buffered events while dispatching MainEventsCleared"); - } - } + pub fn redrawing(&self) -> bool { + self.processing_events.get() == ProcessingEvents::RedrawEvents } - pub(crate) fn redraw_events_cleared(&self) { - let mut runner_ref = self.runner.borrow_mut(); - if let Some(ref mut runner) = *runner_ref { - runner.redraw_events_cleared(); - if !self.buffer.borrow().is_empty() { - warn!("Buffered events while dispatching RedrawEventsCleared"); - } - } - } + pub(crate) unsafe fn call_event_handler(&self, event: Event<'_, T>) { + let mut panic_error = self.panic_error.take(); + if panic_error.is_none() { + let mut control_flow = self.control_flow.take(); + let mut event_handler = self.event_handler.take() + .expect("either event handler is re-entrant (likely), or no event handler is registered (very unlikely)"); - pub(crate) fn destroy_loop(&self) { - if let Ok(mut runner_ref) = self.runner.try_borrow_mut() { - if let Some(ref mut runner) = *runner_ref { - runner.call_event_handler(Event::LoopDestroyed); - } + panic_error = panic::catch_unwind(panic::AssertUnwindSafe(|| { + if control_flow != ControlFlow::Exit { + event_handler(event, &mut control_flow); + } else { + event_handler(event, &mut ControlFlow::Exit); + } + })).err(); + + assert!(self.event_handler.replace(Some(event_handler)).is_none()); + self.control_flow.set(control_flow); } + self.panic_error.set(panic_error); } pub(crate) fn take_panic_error(&self) -> Result<(), PanicError> { - let mut runner_ref = self.runner.borrow_mut(); - if let Some(ref mut runner) = *runner_ref { - runner.take_panic_error() - } else { - Ok(()) + match self.panic_error.take() { + Some(err) => Err(err), + None => Ok(()), } } - pub(crate) fn set_modal_loop(&self, in_modal_loop: bool) { - let mut runner_ref = self.runner.borrow_mut(); - if let Some(ref mut runner) = *runner_ref { - runner.in_modal_loop = in_modal_loop; - if in_modal_loop { - // jumpstart the modal loop - unsafe { - winuser::RedrawWindow( - runner.modal_redraw_window, - ptr::null(), - ptr::null_mut(), - winuser::RDW_INTERNALPAINT, - ); - } - } - } + pub fn register_window(&self, window: HWND) { + let mut owned_windows = self.owned_windows.take(); + owned_windows.insert(window); + self.owned_windows.set(owned_windows); } - pub(crate) fn in_modal_loop(&self) -> bool { - let runner = self.runner.borrow(); - if let Some(ref runner) = *runner { - runner.in_modal_loop - } else { - false - } + pub fn remove_window(&self, window: HWND) { + let mut owned_windows = self.owned_windows.take(); + owned_windows.remove(&window); + self.owned_windows.set(owned_windows); } pub fn control_flow(&self) -> ControlFlow { - let runner_ref = self.runner.borrow(); - if let Some(ref runner) = *runner_ref { - runner.control_flow - } else { - ControlFlow::Exit - } + self.control_flow.get() } -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum RunnerState { - /// The event loop has just been created, and an `Init` event must be sent. - New, - /// The event loop is idling, and began idling at the given instant. - Idle(Instant), - /// The event loop has received a signal from the OS that the loop may resume, but no winit - /// events have been generated yet. We're waiting for an event to be processed or the events - /// to be marked as cleared to send `NewEvents`, depending on the current `ControlFlow`. - DeferredNewEvents(Instant), - /// The event loop is handling the OS's events and sending them to the user's callback. - /// `NewEvents` has been sent, and `MainEventsCleared` hasn't. - HandlingEvents, - /// The event loop is handling the redraw events and sending them to the user's callback. - /// `MainEventsCleared` has been sent, and `RedrawEventsCleared` hasn't. - HandlingRedraw, -} -impl EventLoopRunner { - unsafe fn new(event_loop: &EventLoop, f: F) -> EventLoopRunner - where - F: FnMut(Event<'_, T>, &mut ControlFlow), - { - EventLoopRunner { - control_flow: ControlFlow::default(), - runner_state: RunnerState::New, - in_modal_loop: false, - modal_redraw_window: event_loop.window_target.p.thread_msg_target, - event_handler: mem::transmute::< - Box, &mut ControlFlow)>, - Box, &mut ControlFlow)>, - >(Box::new(f)), - panic_error: None, - } + pub fn handling_events(&self) -> bool { + self.processing_events.get() != ProcessingEvents::NoEvents } - fn take_panic_error(&mut self) -> Result<(), PanicError> { - match self.panic_error.take() { - Some(err) => Err(err), - None => Ok(()), + pub fn owned_windows(&self, mut f: impl FnMut(HWND)) { + let mut owned_windows = self.owned_windows.take(); + for hwnd in &owned_windows { + f(*hwnd); } + let new_owned_windows = self.owned_windows.take(); + owned_windows.extend(&new_owned_windows); + self.owned_windows.set(owned_windows); } - fn new_events(&mut self) { - self.runner_state = match self.runner_state { - // If we're already handling events or have deferred `NewEvents`, we don't need to do - // do any processing. - RunnerState::HandlingEvents - | RunnerState::HandlingRedraw - | RunnerState::DeferredNewEvents(..) => self.runner_state, - - // Send the `Init` `NewEvents` and immediately move into event processing. - RunnerState::New => { - self.call_event_handler(Event::NewEvents(StartCause::Init)); - RunnerState::HandlingEvents - } - - // When `NewEvents` gets sent after an idle depends on the control flow... - // Some `NewEvents` are deferred because not all Windows messages trigger an event_loop event. - // So we defer the `NewEvents` to when we actually process an event. - RunnerState::Idle(wait_start) => { - match self.control_flow { - // If we're polling, send `NewEvents` and immediately move into event processing. - ControlFlow::Poll => { - self.call_event_handler(Event::NewEvents(StartCause::Poll)); - RunnerState::HandlingEvents - }, - // If the user was waiting until a specific time, the `NewEvents` call gets sent - // at varying times depending on the current time. - ControlFlow::WaitUntil(resume_time) => { - match Instant::now() >= resume_time { - // If the current time is later than the requested resume time, we can tell the - // user that the resume time has been reached with `NewEvents` and immdiately move - // into event processing. - true => { - self.call_event_handler(Event::NewEvents(StartCause::ResumeTimeReached { - start: wait_start, - requested_resume: resume_time, - })); - RunnerState::HandlingEvents - }, - // However, if the current time is EARLIER than the requested resume time, we - // don't want to send the `WaitCancelled` event until we know an event is being - // sent. Defer. - false => RunnerState::DeferredNewEvents(wait_start) - } - }, - // If we're waiting, `NewEvents` doesn't get sent until winit gets an event, so - // we defer. - ControlFlow::Wait | - // `Exit` shouldn't really ever get sent here, but if it does do something somewhat sane. - ControlFlow::Exit => RunnerState::DeferredNewEvents(wait_start), - } - } - }; + pub fn should_buffer(&self) -> bool { + let handler = self.event_handler.take(); + let should_buffer = handler.is_none(); + self.event_handler.set(handler); + should_buffer } - fn process_event(&mut self, event: Event<'_, T>) { - // If we're in the modal loop, we need to have some mechanism for finding when the event - // queue has been cleared so we can call `events_cleared`. Windows doesn't give any utilities - // for doing this, but it DOES guarantee that WM_PAINT will only occur after input events have - // been processed. So, we send WM_PAINT to a dummy window which calls `events_cleared` when - // the events queue has been emptied. - if self.in_modal_loop { - unsafe { - winuser::RedrawWindow( - self.modal_redraw_window, - ptr::null(), - ptr::null_mut(), - winuser::RDW_INTERNALPAINT, - ); - } - } - - // If new event processing has to be done (i.e. call NewEvents or defer), do it. If we're - // already in processing nothing happens with this call. - self.new_events(); - - // Now that an event has been received, we have to send any `NewEvents` calls that were - // deferred. - if let RunnerState::DeferredNewEvents(wait_start) = self.runner_state { - match self.control_flow { - ControlFlow::Exit | ControlFlow::Wait => { - self.call_event_handler(Event::NewEvents(StartCause::WaitCancelled { - start: wait_start, - requested_resume: None, - })) - } - ControlFlow::WaitUntil(resume_time) => { - let start_cause = match Instant::now() >= resume_time { - // If the current time is later than the requested resume time, the resume time - // has been reached. - true => StartCause::ResumeTimeReached { - start: wait_start, - requested_resume: resume_time, - }, - // Otherwise, the requested resume time HASN'T been reached and we send a WaitCancelled. - false => StartCause::WaitCancelled { - start: wait_start, - requested_resume: Some(resume_time), - }, - }; - self.call_event_handler(Event::NewEvents(start_cause)); - } - // This can be reached if the control flow is changed to poll during a `RedrawRequested` - // that was sent after `MainEventsCleared`. - ControlFlow::Poll => self.call_event_handler(Event::NewEvents(StartCause::Poll)), - } - self.runner_state = RunnerState::HandlingEvents; - } - - match (self.runner_state, &event) { - (RunnerState::HandlingEvents, Event::RedrawRequested(window_id)) => { - self.call_event_handler(Event::MainEventsCleared); - self.runner_state = RunnerState::HandlingRedraw; - self.call_event_handler(Event::RedrawRequested(*window_id)); - } - (RunnerState::HandlingRedraw, Event::RedrawRequested(window_id)) => { - self.call_event_handler(Event::RedrawRequested(*window_id)); - } - (RunnerState::HandlingRedraw, _) => { - warn!( - "non-redraw event in redraw phase: {:?}", - event.map_nonuser_event::<()>().ok() - ); - } - (_, _) => { - self.runner_state = RunnerState::HandlingEvents; - self.call_event_handler(event); + unsafe fn dispatch_buffered_events(&self) { + loop { + // We do this instead of using a `while let` loop because if we use a `while let` + // loop the reference returned `borrow_mut()` doesn't get dropped until the end + // of the loop's body and attempts to add events to the event buffer while in + // `process_event` will fail. + let buffered_event_opt = self.event_buffer.borrow_mut().pop_front(); + match buffered_event_opt { + Some(e) => e.dispatch_event(|e| self.call_event_handler(e)), + None => break, } } } - fn main_events_cleared(&mut self) { - match self.runner_state { - // If we were handling events, send the MainEventsCleared message. - RunnerState::HandlingEvents => { - self.call_event_handler(Event::MainEventsCleared); - self.runner_state = RunnerState::HandlingRedraw; - } + unsafe fn move_state_to(&self, processing_events: ProcessingEvents) { + use ProcessingEvents::{Uninitialized, NoEvents, MainEvents, RedrawEvents}; - // We already cleared the main events, we don't have to do anything. - // This happens when process_events() processed a RedrawRequested event. - RunnerState::HandlingRedraw => {} - - // If we *weren't* handling events, we don't have to do anything. - RunnerState::New | RunnerState::Idle(..) => (), - - // Some control flows require a NewEvents call even if no events were received. This - // branch handles those. - RunnerState::DeferredNewEvents(wait_start) => { - match self.control_flow { - // If we had deferred a Poll, send the Poll NewEvents and MainEventsCleared. - ControlFlow::Poll => { - self.call_event_handler(Event::NewEvents(StartCause::Poll)); - self.runner_state = RunnerState::HandlingEvents; - self.call_event_handler(Event::MainEventsCleared); - self.runner_state = RunnerState::HandlingRedraw; - } - // If we had deferred a WaitUntil and the resume time has since been reached, - // send the resume notification and MainEventsCleared event. - ControlFlow::WaitUntil(resume_time) => { - if Instant::now() >= resume_time { - self.call_event_handler(Event::NewEvents( - StartCause::ResumeTimeReached { - start: wait_start, - requested_resume: resume_time, - }, - )); - self.runner_state = RunnerState::HandlingEvents; - self.call_event_handler(Event::MainEventsCleared); - self.runner_state = RunnerState::HandlingRedraw; - } - } - // If we deferred a wait and no events were received, the user doesn't have to - // get an event. - ControlFlow::Wait | ControlFlow::Exit => (), - } - } - } - } + let probably_wrong = || warn!("Given winit's current design, the fact that this branch is getting hit \ + is probably indicates a bug. Please open an issue at \ + https://github.com/rust-windowing/winit"); + + match (self.processing_events.replace(processing_events), processing_events) { + (Uninitialized, Uninitialized) | + (NoEvents, NoEvents) | + (MainEvents, MainEvents) | + (RedrawEvents, RedrawEvents) => (), - fn redraw_events_cleared(&mut self) { - match self.runner_state { - // If we were handling redraws, send the RedrawEventsCleared message. - RunnerState::HandlingRedraw => { + (Uninitialized, MainEvents) => { + self.call_new_events(true); + self.call_event_handler(Event::NewEvents(StartCause::Init)); + }, + (Uninitialized, RedrawEvents) => { + self.call_new_events(true); + self.call_event_handler(Event::MainEventsCleared); + }, + (Uninitialized, NoEvents) => { + self.call_new_events(true); + self.call_event_handler(Event::MainEventsCleared); + self.call_event_handler(Event::RedrawEventsCleared); + }, + (_, Uninitialized) => panic!("cannot move state to Uninitialized"), + + (NoEvents, MainEvents) => { + self.call_new_events(false); + }, + (NoEvents, RedrawEvents) => { + self.call_new_events(false); + self.call_event_handler(Event::MainEventsCleared); + }, + (MainEvents, RedrawEvents) => { + self.call_event_handler(Event::MainEventsCleared); + }, + (MainEvents, NoEvents) => { + probably_wrong(); + self.call_event_handler(Event::MainEventsCleared); self.call_event_handler(Event::RedrawEventsCleared); - self.runner_state = RunnerState::Idle(Instant::now()); + }, + (RedrawEvents, NoEvents) => { + self.call_event_handler(Event::RedrawEventsCleared); + }, + (RedrawEvents, MainEvents) => { + probably_wrong(); + self.call_event_handler(Event::RedrawEventsCleared); + self.call_new_events(false); } - // No event was processed, we don't have to do anything. - RunnerState::DeferredNewEvents(_) => (), - // Should not happen. - _ => warn!( - "unexpected state in redraw_events_cleared: {:?}", - self.runner_state - ), } } - fn call_event_handler(&mut self, event: Event<'_, T>) { - if self.panic_error.is_none() { - let EventLoopRunner { - ref mut panic_error, - ref mut event_handler, - ref mut control_flow, - .. - } = self; - *panic_error = panic::catch_unwind(panic::AssertUnwindSafe(|| { - if *control_flow != ControlFlow::Exit { - (*event_handler)(event, control_flow); - } else { - (*event_handler)(event, &mut ControlFlow::Exit); - } - })) - .err(); - } + unsafe fn call_new_events(&self, init: bool) { + let start_cause = match (init, self.control_flow()) { + (true, _) => StartCause::Init, + (false, ControlFlow::Poll) => StartCause::Poll, + (false, ControlFlow::Exit) | + (false, ControlFlow::Wait) => StartCause::WaitCancelled{ requested_resume: None, start: self.last_events_cleared.get() }, + (false, ControlFlow::WaitUntil(requested_resume)) => StartCause::WaitCancelled{ requested_resume: Some(requested_resume), start: self.last_events_cleared.get() }, + }; + self.call_event_handler(Event::NewEvents(start_cause)); + self.dispatch_buffered_events(); + winuser::RedrawWindow(self.thread_msg_target, ptr::null(), ptr::null_mut(), winuser::RDW_INTERNALPAINT); } } diff --git a/src/platform_impl/windows/window_state.rs b/src/platform_impl/windows/window_state.rs index 11794821ad..124090813c 100644 --- a/src/platform_impl/windows/window_state.rs +++ b/src/platform_impl/windows/window_state.rs @@ -80,7 +80,9 @@ bitflags! { /// window's state to match our stored state. This controls whether to accept those changes. const MARKER_RETAIN_STATE_ON_SIZE = 1 << 10; - const MINIMIZED = 1 << 11; + const MARKER_IN_SIZE_MOVE = 1 << 11; + + const MINIMIZED = 1 << 12; const FULLSCREEN_AND_MASK = !( WindowFlags::DECORATIONS.bits | From cc5f80b7d49863f36da87f74357846b2bb5443c9 Mon Sep 17 00:00:00 2001 From: Osspial Date: Thu, 5 Mar 2020 22:37:33 -0500 Subject: [PATCH 02/19] Fix issue that caused too many redraw events --- examples/window_run_return.rs | 6 +++--- src/platform_impl/windows/event_loop.rs | 28 +++++++++++++++++++++---- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/examples/window_run_return.rs b/examples/window_run_return.rs index 74b4e82be7..0112eb3e5b 100644 --- a/examples/window_run_return.rs +++ b/examples/window_run_return.rs @@ -30,10 +30,10 @@ fn main() { event_loop.run_return(|event, _, control_flow| { *control_flow = ControlFlow::Wait; - // if let Event::WindowEvent { event, .. } = &event { - // // Print only Window events to reduce noise + if let Event::WindowEvent { event, .. } = &event { + // Print only Window events to reduce noise println!("{:?}", event); - // } + } match event { Event::WindowEvent { diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index 723426ec85..a41f9adee6 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -639,6 +639,19 @@ fn normalize_pointer_pressure(pressure: u32) -> Option { } } +/// Flush redraw events for Winit's windows. +/// +/// Winit's API guarantees that all redraw events will be clustered together and dispatched all at +/// once, but the standard Windows message loop doesn't always exhibit that behavior. If multiple +/// windows have had redraws scheduled, but an input event is pushed to the message queue between +/// the `WM_PAINT` call for the first window and the `WM_PAINT` call for the second window, Windows +/// will dispatch the input event immediately instead of flushing all the redraw events. This +/// function explicitly pulls all of Winit's redraw events out of the event queue so that they +/// always all get processed in one fell swoop. +/// +/// Returns `true` if this invocation flushed all the redraw events. If this function is re-entrant, +/// it won't flush the redraw events and will return `false`. +#[must_use] unsafe fn flush_paint_messages(except: Option, runner: &ELRShared) -> bool { if !runner.redrawing() { runner.main_events_cleared(); @@ -774,7 +787,7 @@ unsafe extern "system" fn public_window_callback( winuser::WM_PAINT => { if subclass_input.event_loop_runner.should_buffer() { - // this branch can happen in response to `UpdateWindow`, if Windows decides to + // this branch can happen in response to `UpdateWindow`, if win32 decides to // redraw the window outside the normal flow of the event loop. winuser::RedrawWindow( window, @@ -1878,9 +1891,16 @@ unsafe extern "system" fn thread_event_target_callback( winuser::WM_PAINT => { winuser::ValidateRect(window, ptr::null()); - flush_paint_messages(None, &subclass_input.event_loop_runner); - subclass_input.event_loop_runner.redraw_events_cleared(); - process_control_flow(&subclass_input.event_loop_runner); + // If the WM_PAINT handler in `public_window_callback` has already flushed the redraw + // events, `handling_events` will return false and we won't emit a second + // `RedrawEventsCleared` event. + if subclass_input.event_loop_runner.handling_events() { + // This WM_PAINT handler will never be re-entrant because `flush_paint_messages` + // doesn't call WM_PAINT for the thread event target (i.e. this window). + assert!(flush_paint_messages(None, &subclass_input.event_loop_runner)); + subclass_input.event_loop_runner.redraw_events_cleared(); + process_control_flow(&subclass_input.event_loop_runner); + } 0 } From 5cbccd58970425d34e1a2ffc3e225e8f3bbae9bb Mon Sep 17 00:00:00 2001 From: Osspial Date: Thu, 5 Mar 2020 22:38:22 -0500 Subject: [PATCH 03/19] Format --- src/platform_impl/windows/event_loop.rs | 66 +++++++------- .../windows/event_loop/runner.rs | 90 ++++++++++++------- 2 files changed, 94 insertions(+), 62 deletions(-) diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index a41f9adee6..1c47485564 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -44,7 +44,7 @@ use crate::{ }, window::{Fullscreen, WindowId as RootWindowId}, }; -use runner::{EventLoopRunnerShared, ELRShared}; +use runner::{ELRShared, EventLoopRunnerShared}; type GetPointerFrameInfoHistory = unsafe extern "system" fn( pointerId: UINT, @@ -157,7 +157,8 @@ impl EventLoop { let runner_shared = Rc::new(ELRShared::new(thread_msg_target, wait_thread_id)); - let thread_msg_sender = subclass_event_target_window(thread_msg_target, runner_shared.clone()); + let thread_msg_sender = + subclass_event_target_window(thread_msg_target, runner_shared.clone()); raw_input::register_all_mice_and_keyboards_for_raw_input(thread_msg_target); EventLoop { @@ -270,11 +271,10 @@ fn get_wait_thread_id() -> DWORD { &mut msg, -1 as _, *SEND_WAIT_THREAD_ID_MSG_ID, - *SEND_WAIT_THREAD_ID_MSG_ID + *SEND_WAIT_THREAD_ID_MSG_ID, ); assert_eq!( - msg.message, - *SEND_WAIT_THREAD_ID_MSG_ID, + msg.message, *SEND_WAIT_THREAD_ID_MSG_ID, "this shouldn't be possible. please open an issue with Winit. error code: {}", result ); @@ -323,12 +323,7 @@ fn wait_thread(parent_thread_id: DWORD, msg_window_id: HWND) { winuser::MWMO_INPUTAVAILABLE, ); if resume_reason == winerror::WAIT_TIMEOUT { - winuser::PostMessageW( - msg_window_id, - *PROCESS_NEW_EVENTS_MSG_ID, - 0, - 0, - ); + winuser::PostMessageW(msg_window_id, *PROCESS_NEW_EVENTS_MSG_ID, 0, 0); wait_until_opt = None; } @@ -579,7 +574,10 @@ fn create_event_target_window() -> HWND { } } -fn subclass_event_target_window(window: HWND, event_loop_runner: EventLoopRunnerShared) -> Sender { +fn subclass_event_target_window( + window: HWND, + event_loop_runner: EventLoopRunnerShared, +) -> Sender { unsafe { let (tx, rx) = mpsc::channel(); @@ -683,21 +681,17 @@ unsafe fn flush_paint_messages(except: Option, runner: &ELRSha unsafe fn process_control_flow(runner: &ELRShared) { match runner.control_flow() { ControlFlow::Poll => { - winuser::PostMessageW( - runner.thread_msg_target(), - *PROCESS_NEW_EVENTS_MSG_ID, - 0, 0 - ); - }, + winuser::PostMessageW(runner.thread_msg_target(), *PROCESS_NEW_EVENTS_MSG_ID, 0, 0); + } ControlFlow::Wait => (), ControlFlow::WaitUntil(until) => { winuser::PostThreadMessageW( runner.wait_thread_id(), *WAIT_UNTIL_MSG_ID, 0, - Box::into_raw(WaitUntilInstantBox::new(until)) as LPARAM + Box::into_raw(WaitUntilInstantBox::new(until)) as LPARAM, ); - }, + } ControlFlow::Exit => (), } } @@ -742,14 +736,20 @@ unsafe extern "system" fn public_window_callback( match msg { winuser::WM_ENTERSIZEMOVE => { - subclass_input.window_state.lock().set_window_flags_in_place(|f| f.insert(WindowFlags::MARKER_IN_SIZE_MOVE)); + subclass_input + .window_state + .lock() + .set_window_flags_in_place(|f| f.insert(WindowFlags::MARKER_IN_SIZE_MOVE)); 0 - }, + } winuser::WM_EXITSIZEMOVE => { - subclass_input.window_state.lock().set_window_flags_in_place(|f| f.remove(WindowFlags::MARKER_IN_SIZE_MOVE)); + subclass_input + .window_state + .lock() + .set_window_flags_in_place(|f| f.remove(WindowFlags::MARKER_IN_SIZE_MOVE)); 0 - }, + } winuser::WM_NCCREATE => { enable_non_client_dpi_scaling(window); @@ -795,9 +795,9 @@ unsafe extern "system" fn public_window_callback( ptr::null_mut(), winuser::RDW_INTERNALPAINT, ); - } else { - let managing_redraw = flush_paint_messages(Some(window), &subclass_input.event_loop_runner); + let managing_redraw = + flush_paint_messages(Some(window), &subclass_input.event_loop_runner); subclass_input.send_event(Event::RedrawRequested(RootWindowId(WindowId(window)))); if managing_redraw { subclass_input.event_loop_runner.redraw_events_cleared(); @@ -806,7 +806,7 @@ unsafe extern "system" fn public_window_callback( } commctrl::DefSubclassProc(window, msg, wparam, lparam) - }, + } winuser::WM_WINDOWPOSCHANGING => { let mut window_state = subclass_input.window_state.lock(); @@ -1708,7 +1708,9 @@ unsafe extern "system" fn public_window_callback( { let window_state = subclass_input.window_state.lock(); - dragging_window = window_state.window_flags().contains(WindowFlags::MARKER_IN_SIZE_MOVE); + dragging_window = window_state + .window_flags() + .contains(WindowFlags::MARKER_IN_SIZE_MOVE); // Unset maximized if we're changing the window's size. if new_physical_inner_size != old_physical_inner_size { WindowState::set_window_flags(window_state, window, |f| { @@ -1897,7 +1899,10 @@ unsafe extern "system" fn thread_event_target_callback( if subclass_input.event_loop_runner.handling_events() { // This WM_PAINT handler will never be re-entrant because `flush_paint_messages` // doesn't call WM_PAINT for the thread event target (i.e. this window). - assert!(flush_paint_messages(None, &subclass_input.event_loop_runner)); + assert!(flush_paint_messages( + None, + &subclass_input.event_loop_runner + )); subclass_input.event_loop_runner.redraw_events_cleared(); process_control_flow(&subclass_input.event_loop_runner); } @@ -2036,7 +2041,8 @@ unsafe extern "system" fn thread_event_target_callback( winuser::PostThreadMessageW( subclass_input.event_loop_runner.wait_thread_id(), *CANCEL_WAIT_UNTIL_MSG_ID, - 0, 0 + 0, + 0, ); subclass_input.event_loop_runner.poll(); 0 diff --git a/src/platform_impl/windows/event_loop/runner.rs b/src/platform_impl/windows/event_loop/runner.rs index e1aa2258fa..72b2638d21 100644 --- a/src/platform_impl/windows/event_loop/runner.rs +++ b/src/platform_impl/windows/event_loop/runner.rs @@ -1,6 +1,16 @@ -use std::{any::Any, cell::{Cell, RefCell}, collections::{HashSet, VecDeque}, mem, ptr, panic, rc::Rc, time::Instant}; +use std::{ + any::Any, + cell::{Cell, RefCell}, + collections::{HashSet, VecDeque}, + mem, panic, ptr, + rc::Rc, + time::Instant, +}; -use winapi::{um::winuser, shared::{minwindef::DWORD, windef::HWND}}; +use winapi::{ + shared::{minwindef::DWORD, windef::HWND}, + um::winuser, +}; use crate::{ dpi::PhysicalSize, @@ -85,7 +95,7 @@ impl ELRShared { last_events_cleared: Cell::new(Instant::now()), event_handler: Cell::new(None), event_buffer: RefCell::new(VecDeque::new()), - owned_windows: Cell::new(HashSet::new()) + owned_windows: Cell::new(HashSet::new()), } } @@ -101,12 +111,10 @@ impl ELRShared { where F: FnMut(Event<'_, T>, &mut ControlFlow), { - let old_event_handler = self.event_handler.replace( - mem::transmute::< - Option, &mut ControlFlow)>>, - Option, &mut ControlFlow)>>, - >(Some(Box::new(f))), - ); + let old_event_handler = self.event_handler.replace(mem::transmute::< + Option, &mut ControlFlow)>>, + Option, &mut ControlFlow)>>, + >(Some(Box::new(f)))); assert!(old_event_handler.is_none()); } @@ -176,7 +184,8 @@ impl ELRShared { } else { event_handler(event, &mut ControlFlow::Exit); } - })).err(); + })) + .err(); assert!(self.event_handler.replace(Some(event_handler)).is_none()); self.control_flow.set(control_flow); @@ -243,51 +252,58 @@ impl ELRShared { } unsafe fn move_state_to(&self, processing_events: ProcessingEvents) { - use ProcessingEvents::{Uninitialized, NoEvents, MainEvents, RedrawEvents}; - - let probably_wrong = || warn!("Given winit's current design, the fact that this branch is getting hit \ - is probably indicates a bug. Please open an issue at \ - https://github.com/rust-windowing/winit"); + use ProcessingEvents::{MainEvents, NoEvents, RedrawEvents, Uninitialized}; + + let probably_wrong = || { + warn!( + "Given winit's current design, the fact that this branch is getting hit \ + is probably indicates a bug. Please open an issue at \ + https://github.com/rust-windowing/winit" + ) + }; - match (self.processing_events.replace(processing_events), processing_events) { - (Uninitialized, Uninitialized) | - (NoEvents, NoEvents) | - (MainEvents, MainEvents) | - (RedrawEvents, RedrawEvents) => (), + match ( + self.processing_events.replace(processing_events), + processing_events, + ) { + (Uninitialized, Uninitialized) + | (NoEvents, NoEvents) + | (MainEvents, MainEvents) + | (RedrawEvents, RedrawEvents) => (), (Uninitialized, MainEvents) => { self.call_new_events(true); self.call_event_handler(Event::NewEvents(StartCause::Init)); - }, + } (Uninitialized, RedrawEvents) => { self.call_new_events(true); self.call_event_handler(Event::MainEventsCleared); - }, + } (Uninitialized, NoEvents) => { self.call_new_events(true); self.call_event_handler(Event::MainEventsCleared); self.call_event_handler(Event::RedrawEventsCleared); - }, + } (_, Uninitialized) => panic!("cannot move state to Uninitialized"), (NoEvents, MainEvents) => { self.call_new_events(false); - }, + } (NoEvents, RedrawEvents) => { self.call_new_events(false); self.call_event_handler(Event::MainEventsCleared); - }, + } (MainEvents, RedrawEvents) => { self.call_event_handler(Event::MainEventsCleared); - }, + } (MainEvents, NoEvents) => { probably_wrong(); self.call_event_handler(Event::MainEventsCleared); self.call_event_handler(Event::RedrawEventsCleared); - }, + } (RedrawEvents, NoEvents) => { self.call_event_handler(Event::RedrawEventsCleared); - }, + } (RedrawEvents, MainEvents) => { probably_wrong(); self.call_event_handler(Event::RedrawEventsCleared); @@ -300,12 +316,22 @@ impl ELRShared { let start_cause = match (init, self.control_flow()) { (true, _) => StartCause::Init, (false, ControlFlow::Poll) => StartCause::Poll, - (false, ControlFlow::Exit) | - (false, ControlFlow::Wait) => StartCause::WaitCancelled{ requested_resume: None, start: self.last_events_cleared.get() }, - (false, ControlFlow::WaitUntil(requested_resume)) => StartCause::WaitCancelled{ requested_resume: Some(requested_resume), start: self.last_events_cleared.get() }, + (false, ControlFlow::Exit) | (false, ControlFlow::Wait) => StartCause::WaitCancelled { + requested_resume: None, + start: self.last_events_cleared.get(), + }, + (false, ControlFlow::WaitUntil(requested_resume)) => StartCause::WaitCancelled { + requested_resume: Some(requested_resume), + start: self.last_events_cleared.get(), + }, }; self.call_event_handler(Event::NewEvents(start_cause)); self.dispatch_buffered_events(); - winuser::RedrawWindow(self.thread_msg_target, ptr::null(), ptr::null_mut(), winuser::RDW_INTERNALPAINT); + winuser::RedrawWindow( + self.thread_msg_target, + ptr::null(), + ptr::null_mut(), + winuser::RDW_INTERNALPAINT, + ); } } From 60ef522a641565056422f2dba8790e6e3c1510c2 Mon Sep 17 00:00:00 2001 From: Osspial Date: Fri, 6 Mar 2020 00:04:49 -0500 Subject: [PATCH 04/19] misc. fixes --- src/platform_impl/windows/event_loop.rs | 25 +++++++++++-------- .../windows/event_loop/runner.rs | 12 ++------- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index 1c47485564..995b3e3585 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -326,16 +326,6 @@ fn wait_thread(parent_thread_id: DWORD, msg_window_id: HWND) { winuser::PostMessageW(msg_window_id, *PROCESS_NEW_EVENTS_MSG_ID, 0, 0); wait_until_opt = None; } - - // TODO: SPINLOCK ON MAIN THREAD - // if resume_reason == winerror::WAIT_TIMEOUT { - // let mut msg = mem::zeroed(); - // while Instant::now() < wait_until { - // if 0 != winuser::PeekMessageW(&mut msg, ptr::null_mut(), 0, 0, 0) { - // break; - // } - // } - // } } } } @@ -664,7 +654,7 @@ unsafe fn flush_paint_messages(except: Option, runner: &ELRSha redraw_window, winuser::WM_PAINT, winuser::WM_PAINT, - winuser::PM_REMOVE, + winuser::PM_REMOVE | winuser::QS_PAINT, ) { return; } @@ -2044,6 +2034,19 @@ unsafe extern "system" fn thread_event_target_callback( 0, 0, ); + + // if the control_flow is WaitUntil, make sure the given moment has actually passed + // before emitting NewEvents + if let ControlFlow::WaitUntil(wait_until) = + subclass_input.event_loop_runner.control_flow() + { + let mut msg = mem::zeroed(); + while Instant::now() < wait_until { + if 0 != winuser::PeekMessageW(&mut msg, ptr::null_mut(), 0, 0, 0) { + break; + } + } + } subclass_input.event_loop_runner.poll(); 0 } diff --git a/src/platform_impl/windows/event_loop/runner.rs b/src/platform_impl/windows/event_loop/runner.rs index 72b2638d21..5bd9ef93fb 100644 --- a/src/platform_impl/windows/event_loop/runner.rs +++ b/src/platform_impl/windows/event_loop/runner.rs @@ -254,14 +254,6 @@ impl ELRShared { unsafe fn move_state_to(&self, processing_events: ProcessingEvents) { use ProcessingEvents::{MainEvents, NoEvents, RedrawEvents, Uninitialized}; - let probably_wrong = || { - warn!( - "Given winit's current design, the fact that this branch is getting hit \ - is probably indicates a bug. Please open an issue at \ - https://github.com/rust-windowing/winit" - ) - }; - match ( self.processing_events.replace(processing_events), processing_events, @@ -297,7 +289,7 @@ impl ELRShared { self.call_event_handler(Event::MainEventsCleared); } (MainEvents, NoEvents) => { - probably_wrong(); + warn!("RedrawEventsCleared emitted without explicit MainEventsCleared"); self.call_event_handler(Event::MainEventsCleared); self.call_event_handler(Event::RedrawEventsCleared); } @@ -305,7 +297,7 @@ impl ELRShared { self.call_event_handler(Event::RedrawEventsCleared); } (RedrawEvents, MainEvents) => { - probably_wrong(); + warn!("NewEvents emitted without explicit RedrawEventsCleared"); self.call_event_handler(Event::RedrawEventsCleared); self.call_new_events(false); } From 9d399db26e4b4ad59e6471f303d691c7fb8ce028 Mon Sep 17 00:00:00 2001 From: Osspial Date: Fri, 6 Mar 2020 00:11:23 -0500 Subject: [PATCH 05/19] Panic if UpdateWindow is called --- src/platform_impl/windows/event_loop.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index 995b3e3585..185d06245f 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -777,14 +777,15 @@ unsafe extern "system" fn public_window_callback( winuser::WM_PAINT => { if subclass_input.event_loop_runner.should_buffer() { - // this branch can happen in response to `UpdateWindow`, if win32 decides to - // redraw the window outside the normal flow of the event loop. - winuser::RedrawWindow( - window, - ptr::null(), - ptr::null_mut(), - winuser::RDW_INTERNALPAINT, - ); + panic!("InvalidateRgn should be used instead of UpdateWindow"); + // // this branch can happen in response to `UpdateWindow`, if win32 decides to + // // redraw the window outside the normal flow of the event loop. + // winuser::RedrawWindow( + // window, + // ptr::null(), + // ptr::null_mut(), + // winuser::RDW_INTERNALPAINT, + // ); } else { let managing_redraw = flush_paint_messages(Some(window), &subclass_input.event_loop_runner); From ef179ad0c07bb032291468f4a25184abb336bd7e Mon Sep 17 00:00:00 2001 From: Osspial Date: Fri, 6 Mar 2020 01:01:09 -0500 Subject: [PATCH 06/19] Format again --- src/platform_impl/windows/event_loop.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index 185d06245f..db999eaa8e 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -778,14 +778,14 @@ unsafe extern "system" fn public_window_callback( winuser::WM_PAINT => { if subclass_input.event_loop_runner.should_buffer() { panic!("InvalidateRgn should be used instead of UpdateWindow"); - // // this branch can happen in response to `UpdateWindow`, if win32 decides to - // // redraw the window outside the normal flow of the event loop. - // winuser::RedrawWindow( - // window, - // ptr::null(), - // ptr::null_mut(), - // winuser::RDW_INTERNALPAINT, - // ); + // // this branch can happen in response to `UpdateWindow`, if win32 decides to + // // redraw the window outside the normal flow of the event loop. + // winuser::RedrawWindow( + // window, + // ptr::null(), + // ptr::null_mut(), + // winuser::RDW_INTERNALPAINT, + // ); } else { let managing_redraw = flush_paint_messages(Some(window), &subclass_input.event_loop_runner); From b14933700292676e647b805bd1faa1e0282f4ff7 Mon Sep 17 00:00:00 2001 From: Osspial Date: Fri, 6 Mar 2020 14:20:14 -0500 Subject: [PATCH 07/19] Fix bugs filnet discovered in testing --- src/platform_impl/windows/event_loop.rs | 6 ++-- .../windows/event_loop/runner.rs | 33 +++++++++++++------ 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index db999eaa8e..da9b75c527 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -654,7 +654,7 @@ unsafe fn flush_paint_messages(except: Option, runner: &ELRSha redraw_window, winuser::WM_PAINT, winuser::WM_PAINT, - winuser::PM_REMOVE | winuser::QS_PAINT, + winuser::PM_REMOVE | winuser::PM_QS_PAINT, ) { return; } @@ -1882,8 +1882,6 @@ unsafe extern "system" fn thread_event_target_callback( // Because WM_PAINT comes after all other messages, we use it during modal loops to detect // when the event queue has been emptied. See `process_event` for more details. winuser::WM_PAINT => { - winuser::ValidateRect(window, ptr::null()); - // If the WM_PAINT handler in `public_window_callback` has already flushed the redraw // events, `handling_events` will return false and we won't emit a second // `RedrawEventsCleared` event. @@ -1898,7 +1896,7 @@ unsafe extern "system" fn thread_event_target_callback( process_control_flow(&subclass_input.event_loop_runner); } - 0 + commctrl::DefSubclassProc(window, msg, wparam, lparam) } winuser::WM_INPUT_DEVICE_CHANGE => { diff --git a/src/platform_impl/windows/event_loop/runner.rs b/src/platform_impl/windows/event_loop/runner.rs index 5bd9ef93fb..1db40b24dd 100644 --- a/src/platform_impl/windows/event_loop/runner.rs +++ b/src/platform_impl/windows/event_loop/runner.rs @@ -265,15 +265,14 @@ impl ELRShared { (Uninitialized, MainEvents) => { self.call_new_events(true); - self.call_event_handler(Event::NewEvents(StartCause::Init)); } (Uninitialized, RedrawEvents) => { self.call_new_events(true); - self.call_event_handler(Event::MainEventsCleared); + self.call_events_cleared(); } (Uninitialized, NoEvents) => { self.call_new_events(true); - self.call_event_handler(Event::MainEventsCleared); + self.call_events_cleared(); self.call_event_handler(Event::RedrawEventsCleared); } (_, Uninitialized) => panic!("cannot move state to Uninitialized"), @@ -283,14 +282,14 @@ impl ELRShared { } (NoEvents, RedrawEvents) => { self.call_new_events(false); - self.call_event_handler(Event::MainEventsCleared); + self.call_events_cleared(); } (MainEvents, RedrawEvents) => { - self.call_event_handler(Event::MainEventsCleared); + self.call_events_cleared(); } (MainEvents, NoEvents) => { warn!("RedrawEventsCleared emitted without explicit MainEventsCleared"); - self.call_event_handler(Event::MainEventsCleared); + self.call_events_cleared(); self.call_event_handler(Event::RedrawEventsCleared); } (RedrawEvents, NoEvents) => { @@ -312,10 +311,19 @@ impl ELRShared { requested_resume: None, start: self.last_events_cleared.get(), }, - (false, ControlFlow::WaitUntil(requested_resume)) => StartCause::WaitCancelled { - requested_resume: Some(requested_resume), - start: self.last_events_cleared.get(), - }, + (false, ControlFlow::WaitUntil(requested_resume)) => { + if Instant::now() < requested_resume { + StartCause::WaitCancelled { + requested_resume: Some(requested_resume), + start: self.last_events_cleared.get(), + } + } else { + StartCause::ResumeTimeReached { + requested_resume, + start: self.last_events_cleared.get(), + } + } + } }; self.call_event_handler(Event::NewEvents(start_cause)); self.dispatch_buffered_events(); @@ -326,4 +334,9 @@ impl ELRShared { winuser::RDW_INTERNALPAINT, ); } + + unsafe fn call_events_cleared(&self) { + self.last_events_cleared.set(Instant::now()); + self.call_event_handler(Event::MainEventsCleared); + } } From f7277ad3112ba130812e7707513fd861d14ac0ab Mon Sep 17 00:00:00 2001 From: Osspial Date: Sat, 7 Mar 2020 14:42:17 -0500 Subject: [PATCH 08/19] Clean up runner code and fix bugs discovered in cleanup --- .../windows/event_loop/runner.rs | 290 ++++++++++-------- 1 file changed, 155 insertions(+), 135 deletions(-) diff --git a/src/platform_impl/windows/event_loop/runner.rs b/src/platform_impl/windows/event_loop/runner.rs index 1db40b24dd..bef5f16022 100644 --- a/src/platform_impl/windows/event_loop/runner.rs +++ b/src/platform_impl/windows/event_loop/runner.rs @@ -22,25 +22,36 @@ use crate::{ pub(crate) type EventLoopRunnerShared = Rc>; pub(crate) struct ELRShared { + // The event loop's win32 handles thread_msg_target: HWND, wait_thread_id: DWORD, - processing_events: Cell, - panic_error: Cell>, + control_flow: Cell, + runner_state: Cell, last_events_cleared: Cell, + event_handler: Cell, &mut ControlFlow)>>>, event_buffer: RefCell>>, + owned_windows: Cell>, + + panic_error: Cell>, } pub type PanicError = Box; #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -enum ProcessingEvents { +enum RunnerState { + /// The event loop has just been created, and an `Init` event must be sent. Uninitialized, - NoEvents, - MainEvents, - RedrawEvents, + /// The event loop is idling. + Idle, + /// The event loop is handling the OS's events and sending them to the user's callback. + /// `NewEvents` has been sent, and `MainEventsCleared` hasn't. + HandlingMainEvents, + /// The event loop is handling the redraw events and sending them to the user's callback. + /// `MainEventsCleared` has been sent, and `RedrawEventsCleared` hasn't. + HandlingRedrawEvents, } enum BufferedEvent { @@ -48,48 +59,12 @@ enum BufferedEvent { ScaleFactorChanged(WindowId, f64, PhysicalSize), } -impl BufferedEvent { - pub fn from_event(event: Event<'_, T>) -> BufferedEvent { - match event { - Event::WindowEvent { - event: - WindowEvent::ScaleFactorChanged { - scale_factor, - new_inner_size, - }, - window_id, - } => BufferedEvent::ScaleFactorChanged(window_id, scale_factor, *new_inner_size), - event => BufferedEvent::Event(event.to_static().unwrap()), - } - } - - pub fn dispatch_event(self, dispatch: impl FnOnce(Event<'_, T>)) { - match self { - Self::Event(event) => dispatch(event), - Self::ScaleFactorChanged(window_id, scale_factor, mut new_inner_size) => { - dispatch(Event::WindowEvent { - window_id, - event: WindowEvent::ScaleFactorChanged { - scale_factor, - new_inner_size: &mut new_inner_size, - }, - }); - util::set_inner_size_physical( - (window_id.0).0, - new_inner_size.width as _, - new_inner_size.height as _, - ); - } - } - } -} - impl ELRShared { pub(crate) fn new(thread_msg_target: HWND, wait_thread_id: DWORD) -> ELRShared { ELRShared { thread_msg_target, wait_thread_id, - processing_events: Cell::new(ProcessingEvents::Uninitialized), + runner_state: Cell::new(RunnerState::Uninitialized), control_flow: Cell::new(ControlFlow::Poll), panic_error: Cell::new(None), last_events_cleared: Cell::new(Instant::now()), @@ -99,14 +74,6 @@ impl ELRShared { } } - pub fn thread_msg_target(&self) -> HWND { - self.thread_msg_target - } - - pub fn wait_thread_id(&self) -> DWORD { - self.wait_thread_id - } - pub(crate) unsafe fn set_event_handler(&self, f: F) where F: FnMut(Event<'_, T>, &mut ControlFlow), @@ -122,7 +89,7 @@ impl ELRShared { let ELRShared { thread_msg_target: _, wait_thread_id: _, - processing_events, + runner_state, panic_error, control_flow, last_events_cleared: _, @@ -130,19 +97,84 @@ impl ELRShared { event_buffer: _, owned_windows: _, } = self; - processing_events.set(ProcessingEvents::Uninitialized); + runner_state.set(RunnerState::Uninitialized); panic_error.set(None); control_flow.set(ControlFlow::Poll); event_handler.set(None); } +} + +/// State retrieval functions. +impl ELRShared { + pub fn thread_msg_target(&self) -> HWND { + self.thread_msg_target + } + + pub fn wait_thread_id(&self) -> DWORD { + self.wait_thread_id + } + + pub fn redrawing(&self) -> bool { + self.runner_state.get() == RunnerState::HandlingRedrawEvents + } + + pub fn take_panic_error(&self) -> Result<(), PanicError> { + match self.panic_error.take() { + Some(err) => Err(err), + None => Ok(()), + } + } + + pub fn control_flow(&self) -> ControlFlow { + self.control_flow.get() + } + + pub fn handling_events(&self) -> bool { + self.runner_state.get() != RunnerState::Idle + } + + pub fn should_buffer(&self) -> bool { + let handler = self.event_handler.take(); + let should_buffer = handler.is_none(); + self.event_handler.set(handler); + should_buffer + } +} + +/// Associated window functions. +impl ELRShared { + pub fn register_window(&self, window: HWND) { + let mut owned_windows = self.owned_windows.take(); + owned_windows.insert(window); + self.owned_windows.set(owned_windows); + } + + pub fn remove_window(&self, window: HWND) { + let mut owned_windows = self.owned_windows.take(); + owned_windows.remove(&window); + self.owned_windows.set(owned_windows); + } + + pub fn owned_windows(&self, mut f: impl FnMut(HWND)) { + let mut owned_windows = self.owned_windows.take(); + for hwnd in &owned_windows { + f(*hwnd); + } + let new_owned_windows = self.owned_windows.take(); + owned_windows.extend(&new_owned_windows); + self.owned_windows.set(owned_windows); + } +} +/// Event dispatch functions. +impl ELRShared { pub(crate) unsafe fn poll(&self) { - self.move_state_to(ProcessingEvents::MainEvents); + self.move_state_to(RunnerState::HandlingMainEvents); } pub(crate) unsafe fn send_event(&self, event: Event<'_, T>) { if let Event::RedrawRequested(_) = event { - self.move_state_to(ProcessingEvents::RedrawEvents); + self.move_state_to(RunnerState::HandlingRedrawEvents); self.call_event_handler(event); } else { if self.should_buffer() { @@ -152,7 +184,7 @@ impl ELRShared { .borrow_mut() .push_back(BufferedEvent::from_event(event)) } else { - self.move_state_to(ProcessingEvents::MainEvents); + self.move_state_to(RunnerState::HandlingMainEvents); self.call_event_handler(event); self.dispatch_buffered_events(); } @@ -160,15 +192,11 @@ impl ELRShared { } pub(crate) unsafe fn main_events_cleared(&self) { - self.move_state_to(ProcessingEvents::RedrawEvents); + self.move_state_to(RunnerState::HandlingRedrawEvents); } pub(crate) unsafe fn redraw_events_cleared(&self) { - self.move_state_to(ProcessingEvents::NoEvents); - } - - pub fn redrawing(&self) -> bool { - self.processing_events.get() == ProcessingEvents::RedrawEvents + self.move_state_to(RunnerState::Idle); } pub(crate) unsafe fn call_event_handler(&self, event: Event<'_, T>) { @@ -193,50 +221,6 @@ impl ELRShared { self.panic_error.set(panic_error); } - pub(crate) fn take_panic_error(&self) -> Result<(), PanicError> { - match self.panic_error.take() { - Some(err) => Err(err), - None => Ok(()), - } - } - - pub fn register_window(&self, window: HWND) { - let mut owned_windows = self.owned_windows.take(); - owned_windows.insert(window); - self.owned_windows.set(owned_windows); - } - - pub fn remove_window(&self, window: HWND) { - let mut owned_windows = self.owned_windows.take(); - owned_windows.remove(&window); - self.owned_windows.set(owned_windows); - } - - pub fn control_flow(&self) -> ControlFlow { - self.control_flow.get() - } - - pub fn handling_events(&self) -> bool { - self.processing_events.get() != ProcessingEvents::NoEvents - } - - pub fn owned_windows(&self, mut f: impl FnMut(HWND)) { - let mut owned_windows = self.owned_windows.take(); - for hwnd in &owned_windows { - f(*hwnd); - } - let new_owned_windows = self.owned_windows.take(); - owned_windows.extend(&new_owned_windows); - self.owned_windows.set(owned_windows); - } - - pub fn should_buffer(&self) -> bool { - let handler = self.event_handler.take(); - let should_buffer = handler.is_none(); - self.event_handler.set(handler); - should_buffer - } - unsafe fn dispatch_buffered_events(&self) { loop { // We do this instead of using a `while let` loop because if we use a `while let` @@ -251,53 +235,53 @@ impl ELRShared { } } - unsafe fn move_state_to(&self, processing_events: ProcessingEvents) { - use ProcessingEvents::{MainEvents, NoEvents, RedrawEvents, Uninitialized}; + unsafe fn move_state_to(&self, runner_state: RunnerState) { + use RunnerState::{HandlingMainEvents, Idle, HandlingRedrawEvents, Uninitialized}; match ( - self.processing_events.replace(processing_events), - processing_events, + self.runner_state.replace(runner_state), + runner_state, ) { (Uninitialized, Uninitialized) - | (NoEvents, NoEvents) - | (MainEvents, MainEvents) - | (RedrawEvents, RedrawEvents) => (), + | (Idle, Idle) + | (HandlingMainEvents, HandlingMainEvents) + | (HandlingRedrawEvents, HandlingRedrawEvents) => (), - (Uninitialized, MainEvents) => { + (Uninitialized, HandlingMainEvents) => { self.call_new_events(true); } - (Uninitialized, RedrawEvents) => { + (Uninitialized, HandlingRedrawEvents) => { self.call_new_events(true); - self.call_events_cleared(); + self.call_event_handler(Event::MainEventsCleared); } - (Uninitialized, NoEvents) => { + (Uninitialized, Idle) => { self.call_new_events(true); - self.call_events_cleared(); - self.call_event_handler(Event::RedrawEventsCleared); + self.call_event_handler(Event::MainEventsCleared); + self.call_redraw_events_cleared(); } (_, Uninitialized) => panic!("cannot move state to Uninitialized"), - (NoEvents, MainEvents) => { + (Idle, HandlingMainEvents) => { self.call_new_events(false); } - (NoEvents, RedrawEvents) => { + (Idle, HandlingRedrawEvents) => { self.call_new_events(false); - self.call_events_cleared(); + self.call_event_handler(Event::MainEventsCleared); } - (MainEvents, RedrawEvents) => { - self.call_events_cleared(); + (HandlingMainEvents, HandlingRedrawEvents) => { + self.call_event_handler(Event::MainEventsCleared); } - (MainEvents, NoEvents) => { - warn!("RedrawEventsCleared emitted without explicit MainEventsCleared"); - self.call_events_cleared(); - self.call_event_handler(Event::RedrawEventsCleared); + (HandlingMainEvents, Idle) => { + warn!("HandlingRedrawEventsCleared emitted without explicit HandlingMainEventsCleared"); + self.call_event_handler(Event::MainEventsCleared); + self.call_redraw_events_cleared(); } - (RedrawEvents, NoEvents) => { - self.call_event_handler(Event::RedrawEventsCleared); + (HandlingRedrawEvents, Idle) => { + self.call_redraw_events_cleared(); } - (RedrawEvents, MainEvents) => { - warn!("NewEvents emitted without explicit RedrawEventsCleared"); - self.call_event_handler(Event::RedrawEventsCleared); + (HandlingRedrawEvents, HandlingMainEvents) => { + warn!("NewEvents emitted without explicit HandlingRedrawEventsCleared"); + self.call_redraw_events_cleared(); self.call_new_events(false); } } @@ -335,8 +319,44 @@ impl ELRShared { ); } - unsafe fn call_events_cleared(&self) { + unsafe fn call_redraw_events_cleared(&self) { + self.call_event_handler(Event::RedrawEventsCleared); self.last_events_cleared.set(Instant::now()); - self.call_event_handler(Event::MainEventsCleared); + } +} + +impl BufferedEvent { + pub fn from_event(event: Event<'_, T>) -> BufferedEvent { + match event { + Event::WindowEvent { + event: + WindowEvent::ScaleFactorChanged { + scale_factor, + new_inner_size, + }, + window_id, + } => BufferedEvent::ScaleFactorChanged(window_id, scale_factor, *new_inner_size), + event => BufferedEvent::Event(event.to_static().unwrap()), + } + } + + pub fn dispatch_event(self, dispatch: impl FnOnce(Event<'_, T>)) { + match self { + Self::Event(event) => dispatch(event), + Self::ScaleFactorChanged(window_id, scale_factor, mut new_inner_size) => { + dispatch(Event::WindowEvent { + window_id, + event: WindowEvent::ScaleFactorChanged { + scale_factor, + new_inner_size: &mut new_inner_size, + }, + }); + util::set_inner_size_physical( + (window_id.0).0, + new_inner_size.width as _, + new_inner_size.height as _, + ); + } + } } } From 7ec52de661f8283a0b26950a9f6ffa8c1c72a80a Mon Sep 17 00:00:00 2001 From: Osspial Date: Sat, 7 Mar 2020 15:09:33 -0500 Subject: [PATCH 09/19] Further clean up runner code --- .../windows/event_loop/runner.rs | 39 ++++++++++++++++--- 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/src/platform_impl/windows/event_loop/runner.rs b/src/platform_impl/windows/event_loop/runner.rs index bef5f16022..509df4875a 100644 --- a/src/platform_impl/windows/event_loop/runner.rs +++ b/src/platform_impl/windows/event_loop/runner.rs @@ -40,6 +40,7 @@ pub(crate) struct ELRShared { pub type PanicError = Box; +/// See `move_state_to` function for details on how the state loop works. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] enum RunnerState { /// The event loop has just been created, and an `Init` event must be sent. @@ -174,7 +175,10 @@ impl ELRShared { pub(crate) unsafe fn send_event(&self, event: Event<'_, T>) { if let Event::RedrawRequested(_) = event { - self.move_state_to(RunnerState::HandlingRedrawEvents); + if self.runner_state.get() != RunnerState::HandlingRedrawEvents { + warn!("RedrawRequested dispatched without explicit MainEventsCleared"); + self.move_state_to(RunnerState::HandlingRedrawEvents); + } self.call_event_handler(event); } else { if self.should_buffer() { @@ -235,18 +239,38 @@ impl ELRShared { } } - unsafe fn move_state_to(&self, runner_state: RunnerState) { + /// Dispatch control flow events (`NewEvents`, `MainEventsCleared`, and `RedrawEventsCleared`) as + /// necessary to bring the internal `RunnerState` to the new runner state. + /// + /// The state transitions are defined as follows: + /// + /// ```text + /// Uninitialized + /// | + /// V + /// HandlingMainEvents + /// ^ | + /// | V + /// Idle <--- HandlingRedrawEvents + /// ``` + /// + /// Attempting to transition back to `Uninitialized` will result in a panic. Transitioning to + /// the current state is a no-op. Even if the `new_runner_state` isn't the immediate next state + /// in the runner state machine (e.g. `self.runner_state == HandlingMainEvents` and + /// `new_runner_state == Idle`), the intermediate state transitions will still be executed. + unsafe fn move_state_to(&self, new_runner_state: RunnerState) { use RunnerState::{HandlingMainEvents, Idle, HandlingRedrawEvents, Uninitialized}; match ( - self.runner_state.replace(runner_state), - runner_state, + self.runner_state.replace(new_runner_state), + new_runner_state, ) { (Uninitialized, Uninitialized) | (Idle, Idle) | (HandlingMainEvents, HandlingMainEvents) | (HandlingRedrawEvents, HandlingRedrawEvents) => (), + // State transitions that initialize the event loop. (Uninitialized, HandlingMainEvents) => { self.call_new_events(true); } @@ -261,6 +285,7 @@ impl ELRShared { } (_, Uninitialized) => panic!("cannot move state to Uninitialized"), + // State transitions that start the event handling process. (Idle, HandlingMainEvents) => { self.call_new_events(false); } @@ -268,19 +293,21 @@ impl ELRShared { self.call_new_events(false); self.call_event_handler(Event::MainEventsCleared); } + (HandlingMainEvents, HandlingRedrawEvents) => { self.call_event_handler(Event::MainEventsCleared); } (HandlingMainEvents, Idle) => { - warn!("HandlingRedrawEventsCleared emitted without explicit HandlingMainEventsCleared"); + warn!("RedrawEventsCleared emitted without explicit MainEventsCleared"); self.call_event_handler(Event::MainEventsCleared); self.call_redraw_events_cleared(); } + (HandlingRedrawEvents, Idle) => { self.call_redraw_events_cleared(); } (HandlingRedrawEvents, HandlingMainEvents) => { - warn!("NewEvents emitted without explicit HandlingRedrawEventsCleared"); + warn!("NewEvents emitted without explicit RedrawEventsCleared"); self.call_redraw_events_cleared(); self.call_new_events(false); } From 925ba52e1c58470048aa54f9b22b086297ed3da5 Mon Sep 17 00:00:00 2001 From: Osspial Date: Sat, 7 Mar 2020 16:45:04 -0500 Subject: [PATCH 10/19] Fix win32 eating paint messages when ControlFlow::WaitUntil is set --- src/platform_impl/windows/event_loop.rs | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index da9b75c527..554900eec6 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -1882,6 +1882,7 @@ unsafe extern "system" fn thread_event_target_callback( // Because WM_PAINT comes after all other messages, we use it during modal loops to detect // when the event queue has been emptied. See `process_event` for more details. winuser::WM_PAINT => { + winuser::ValidateRect(window, ptr::null()); // If the WM_PAINT handler in `public_window_callback` has already flushed the redraw // events, `handling_events` will return false and we won't emit a second // `RedrawEventsCleared` event. @@ -1896,7 +1897,7 @@ unsafe extern "system" fn thread_event_target_callback( process_control_flow(&subclass_input.event_loop_runner); } - commctrl::DefSubclassProc(window, msg, wparam, lparam) + 0 } winuser::WM_INPUT_DEVICE_CHANGE => { @@ -2042,6 +2043,25 @@ unsafe extern "system" fn thread_event_target_callback( let mut msg = mem::zeroed(); while Instant::now() < wait_until { if 0 != winuser::PeekMessageW(&mut msg, ptr::null_mut(), 0, 0, 0) { + + // This works around a bug in PeekMessageW. If the message PeekMessageW + // gets is a WM_PAINT message that had RDW_INTERNALPAINT set (i.e. doesn't + // have an update region), PeekMessageW will remove that window from the + // redraw queue even though we told it not to remove messages from the + // queue. We fix it by re-dispatching an internal paint message to that + // window. + if msg.message == winuser::WM_PAINT { + let mut rect = mem::zeroed(); + if 0 == winuser::GetUpdateRect(msg.hwnd, &mut rect, 0) { + winuser::RedrawWindow( + msg.hwnd, + ptr::null(), + ptr::null_mut(), + winuser::RDW_INTERNALPAINT, + ); + } + } + break; } } From b08d4b138ac0696ef89823b5b0bb77ab88bc138c Mon Sep 17 00:00:00 2001 From: Osspial Date: Sat, 7 Mar 2020 17:10:02 -0500 Subject: [PATCH 11/19] Catch all unwinds in window callbacks --- src/platform_impl/windows/event_loop.rs | 24 +++++++-- .../windows/event_loop/runner.rs | 49 +++++++++++++------ 2 files changed, 54 insertions(+), 19 deletions(-) diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index 554900eec6..d0fcfcadba 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -724,7 +724,10 @@ unsafe extern "system" fn public_window_callback( ) -> LRESULT { let subclass_input = &*(subclass_input_ptr as *const SubclassInput); - match msg { + // I decided to bind the closure to `callback` and pass it to catch_unwind rather than passing + // the closure to catch_unwind directly so that the match body indendation wouldn't change and + // the git blame and history would be preserved. + let callback = || match msg { winuser::WM_ENTERSIZEMOVE => { subclass_input .window_state @@ -1861,7 +1864,12 @@ unsafe extern "system" fn public_window_callback( commctrl::DefSubclassProc(window, msg, wparam, lparam) } } - } + }; + + subclass_input + .event_loop_runner + .catch_unwind(callback) + .unwrap_or(-1) } unsafe extern "system" fn thread_event_target_callback( @@ -1873,7 +1881,12 @@ unsafe extern "system" fn thread_event_target_callback( subclass_input_ptr: DWORD_PTR, ) -> LRESULT { let subclass_input = &mut *(subclass_input_ptr as *mut ThreadMsgTargetSubclassInput); - match msg { + let runner = subclass_input.event_loop_runner.clone(); + + // I decided to bind the closure to `callback` and pass it to catch_unwind rather than passing + // the closure to catch_unwind directly so that the match body indendation wouldn't change and + // the git blame and history would be preserved. + let callback = || match msg { winuser::WM_DESTROY => { Box::from_raw(subclass_input); drop(subclass_input); @@ -2043,7 +2056,6 @@ unsafe extern "system" fn thread_event_target_callback( let mut msg = mem::zeroed(); while Instant::now() < wait_until { if 0 != winuser::PeekMessageW(&mut msg, ptr::null_mut(), 0, 0, 0) { - // This works around a bug in PeekMessageW. If the message PeekMessageW // gets is a WM_PAINT message that had RDW_INTERNALPAINT set (i.e. doesn't // have an update region), PeekMessageW will remove that window from the @@ -2070,5 +2082,7 @@ unsafe extern "system" fn thread_event_target_callback( 0 } _ => commctrl::DefSubclassProc(window, msg, wparam, lparam), - } + }; + + runner.catch_unwind(callback).unwrap_or(-1) } diff --git a/src/platform_impl/windows/event_loop/runner.rs b/src/platform_impl/windows/event_loop/runner.rs index 509df4875a..111719d29f 100644 --- a/src/platform_impl/windows/event_loop/runner.rs +++ b/src/platform_impl/windows/event_loop/runner.rs @@ -142,8 +142,34 @@ impl ELRShared { } } -/// Associated window functions. +/// Misc. functions impl ELRShared { + pub fn catch_unwind(&self, f: impl FnOnce() -> R) -> Option { + let panic_error = self.panic_error.take(); + if panic_error.is_none() { + let result = panic::catch_unwind(panic::AssertUnwindSafe(f)); + + // Check to see if the panic error was set in a re-entrant call to catch_unwind inside + // of `f`. If it was, that error takes priority. If it wasn't, check if our call to + // catch_unwind caught any panics and set panic_error appropriately. + match self.panic_error.take() { + None => match result { + Ok(r) => Some(r), + Err(e) => { + self.panic_error.set(Some(e)); + None + } + }, + Some(e) => { + self.panic_error.set(Some(e)); + None + } + } + } else { + self.panic_error.set(panic_error); + None + } + } pub fn register_window(&self, window: HWND) { let mut owned_windows = self.owned_windows.take(); owned_windows.insert(window); @@ -204,25 +230,20 @@ impl ELRShared { } pub(crate) unsafe fn call_event_handler(&self, event: Event<'_, T>) { - let mut panic_error = self.panic_error.take(); - if panic_error.is_none() { + self.catch_unwind(|| { let mut control_flow = self.control_flow.take(); let mut event_handler = self.event_handler.take() .expect("either event handler is re-entrant (likely), or no event handler is registered (very unlikely)"); - panic_error = panic::catch_unwind(panic::AssertUnwindSafe(|| { - if control_flow != ControlFlow::Exit { - event_handler(event, &mut control_flow); - } else { - event_handler(event, &mut ControlFlow::Exit); - } - })) - .err(); + if control_flow != ControlFlow::Exit { + event_handler(event, &mut control_flow); + } else { + event_handler(event, &mut ControlFlow::Exit); + } assert!(self.event_handler.replace(Some(event_handler)).is_none()); self.control_flow.set(control_flow); - } - self.panic_error.set(panic_error); + }); } unsafe fn dispatch_buffered_events(&self) { @@ -259,7 +280,7 @@ impl ELRShared { /// in the runner state machine (e.g. `self.runner_state == HandlingMainEvents` and /// `new_runner_state == Idle`), the intermediate state transitions will still be executed. unsafe fn move_state_to(&self, new_runner_state: RunnerState) { - use RunnerState::{HandlingMainEvents, Idle, HandlingRedrawEvents, Uninitialized}; + use RunnerState::{HandlingMainEvents, HandlingRedrawEvents, Idle, Uninitialized}; match ( self.runner_state.replace(new_runner_state), From 5d86c1954e0976d1b063125eb111b07b2798c87c Mon Sep 17 00:00:00 2001 From: Osspial Date: Mon, 9 Mar 2020 14:53:09 -0400 Subject: [PATCH 12/19] The Windows API has no bugs --- src/platform_impl/windows/event_loop.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index d0fcfcadba..89a1e7efd6 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -2056,7 +2056,7 @@ unsafe extern "system" fn thread_event_target_callback( let mut msg = mem::zeroed(); while Instant::now() < wait_until { if 0 != winuser::PeekMessageW(&mut msg, ptr::null_mut(), 0, 0, 0) { - // This works around a bug in PeekMessageW. If the message PeekMessageW + // This works around a "feature" in PeekMessageW. If the message PeekMessageW // gets is a WM_PAINT message that had RDW_INTERNALPAINT set (i.e. doesn't // have an update region), PeekMessageW will remove that window from the // redraw queue even though we told it not to remove messages from the From 652e016226527e2394db6e79c73179e3229db944 Mon Sep 17 00:00:00 2001 From: Osspial Date: Tue, 10 Mar 2020 16:10:46 -0400 Subject: [PATCH 13/19] Rename ELRShared to EventLoopRunner --- src/platform_impl/windows/event_loop.rs | 8 ++++---- src/platform_impl/windows/event_loop/runner.rs | 18 +++++++++--------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index 89a1e7efd6..72920ed093 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -44,7 +44,7 @@ use crate::{ }, window::{Fullscreen, WindowId as RootWindowId}, }; -use runner::{ELRShared, EventLoopRunnerShared}; +use runner::{EventLoopRunner, EventLoopRunnerShared}; type GetPointerFrameInfoHistory = unsafe extern "system" fn( pointerId: UINT, @@ -155,7 +155,7 @@ impl EventLoop { thread::spawn(move || wait_thread(thread_id, send_thread_msg_target as HWND)); let wait_thread_id = get_wait_thread_id(); - let runner_shared = Rc::new(ELRShared::new(thread_msg_target, wait_thread_id)); + let runner_shared = Rc::new(EventLoopRunner::new(thread_msg_target, wait_thread_id)); let thread_msg_sender = subclass_event_target_window(thread_msg_target, runner_shared.clone()); @@ -640,7 +640,7 @@ fn normalize_pointer_pressure(pressure: u32) -> Option { /// Returns `true` if this invocation flushed all the redraw events. If this function is re-entrant, /// it won't flush the redraw events and will return `false`. #[must_use] -unsafe fn flush_paint_messages(except: Option, runner: &ELRShared) -> bool { +unsafe fn flush_paint_messages(except: Option, runner: &EventLoopRunner) -> bool { if !runner.redrawing() { runner.main_events_cleared(); let mut msg = mem::zeroed(); @@ -668,7 +668,7 @@ unsafe fn flush_paint_messages(except: Option, runner: &ELRSha } } -unsafe fn process_control_flow(runner: &ELRShared) { +unsafe fn process_control_flow(runner: &EventLoopRunner) { match runner.control_flow() { ControlFlow::Poll => { winuser::PostMessageW(runner.thread_msg_target(), *PROCESS_NEW_EVENTS_MSG_ID, 0, 0); diff --git a/src/platform_impl/windows/event_loop/runner.rs b/src/platform_impl/windows/event_loop/runner.rs index 111719d29f..258a40c08c 100644 --- a/src/platform_impl/windows/event_loop/runner.rs +++ b/src/platform_impl/windows/event_loop/runner.rs @@ -20,8 +20,8 @@ use crate::{ window::WindowId, }; -pub(crate) type EventLoopRunnerShared = Rc>; -pub(crate) struct ELRShared { +pub(crate) type EventLoopRunnerShared = Rc>; +pub(crate) struct EventLoopRunner { // The event loop's win32 handles thread_msg_target: HWND, wait_thread_id: DWORD, @@ -60,9 +60,9 @@ enum BufferedEvent { ScaleFactorChanged(WindowId, f64, PhysicalSize), } -impl ELRShared { - pub(crate) fn new(thread_msg_target: HWND, wait_thread_id: DWORD) -> ELRShared { - ELRShared { +impl EventLoopRunner { + pub(crate) fn new(thread_msg_target: HWND, wait_thread_id: DWORD) -> EventLoopRunner { + EventLoopRunner { thread_msg_target, wait_thread_id, runner_state: Cell::new(RunnerState::Uninitialized), @@ -87,7 +87,7 @@ impl ELRShared { } pub(crate) fn reset_runner(&self) { - let ELRShared { + let EventLoopRunner { thread_msg_target: _, wait_thread_id: _, runner_state, @@ -106,7 +106,7 @@ impl ELRShared { } /// State retrieval functions. -impl ELRShared { +impl EventLoopRunner { pub fn thread_msg_target(&self) -> HWND { self.thread_msg_target } @@ -143,7 +143,7 @@ impl ELRShared { } /// Misc. functions -impl ELRShared { +impl EventLoopRunner { pub fn catch_unwind(&self, f: impl FnOnce() -> R) -> Option { let panic_error = self.panic_error.take(); if panic_error.is_none() { @@ -194,7 +194,7 @@ impl ELRShared { } /// Event dispatch functions. -impl ELRShared { +impl EventLoopRunner { pub(crate) unsafe fn poll(&self) { self.move_state_to(RunnerState::HandlingMainEvents); } From f3ceee22919b0e21cc4b497c316b26489b729655 Mon Sep 17 00:00:00 2001 From: Osspial Date: Tue, 10 Mar 2020 16:25:41 -0400 Subject: [PATCH 14/19] Format --- src/platform_impl/windows/event_loop.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index 72920ed093..d3281cbd42 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -640,7 +640,10 @@ fn normalize_pointer_pressure(pressure: u32) -> Option { /// Returns `true` if this invocation flushed all the redraw events. If this function is re-entrant, /// it won't flush the redraw events and will return `false`. #[must_use] -unsafe fn flush_paint_messages(except: Option, runner: &EventLoopRunner) -> bool { +unsafe fn flush_paint_messages( + except: Option, + runner: &EventLoopRunner, +) -> bool { if !runner.redrawing() { runner.main_events_cleared(); let mut msg = mem::zeroed(); From 810910937edf6fe445e5ea9ea04d6db940198bcf Mon Sep 17 00:00:00 2001 From: Osspial Date: Sun, 19 Apr 2020 13:59:04 -0400 Subject: [PATCH 15/19] Prevent this branch from making #1484 worse --- src/platform_impl/windows/event_loop.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index d3281cbd42..32e5a07748 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -727,6 +727,13 @@ unsafe extern "system" fn public_window_callback( ) -> LRESULT { let subclass_input = &*(subclass_input_ptr as *const SubclassInput); + winuser::RedrawWindow( + subclass_input.event_loop_runner.thread_msg_target(), + ptr::null(), + ptr::null_mut(), + winuser::RDW_INTERNALPAINT, + ); + // I decided to bind the closure to `callback` and pass it to catch_unwind rather than passing // the closure to catch_unwind directly so that the match body indendation wouldn't change and // the git blame and history would be preserved. @@ -1886,6 +1893,15 @@ unsafe extern "system" fn thread_event_target_callback( let subclass_input = &mut *(subclass_input_ptr as *mut ThreadMsgTargetSubclassInput); let runner = subclass_input.event_loop_runner.clone(); + if msg != winuser::WM_PAINT { + winuser::RedrawWindow( + window, + ptr::null(), + ptr::null_mut(), + winuser::RDW_INTERNALPAINT, + ); + } + // I decided to bind the closure to `callback` and pass it to catch_unwind rather than passing // the closure to catch_unwind directly so that the match body indendation wouldn't change and // the git blame and history would be preserved. From f81c6767febbf6270c582ee55cbb1896919ab397 Mon Sep 17 00:00:00 2001 From: Osspial Date: Sun, 19 Apr 2020 14:17:21 -0400 Subject: [PATCH 16/19] Fix WaitUntil --- src/platform_impl/windows/event_loop.rs | 26 ++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index 32e5a07748..10dc76159c 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -284,7 +284,7 @@ fn get_wait_thread_id() -> DWORD { fn wait_thread(parent_thread_id: DWORD, msg_window_id: HWND) { unsafe { - let mut msg = mem::zeroed(); + let mut msg: winuser::MSG; let cur_thread_id = processthreadsapi::GetCurrentThreadId(); winuser::PostThreadMessageW( @@ -296,12 +296,25 @@ fn wait_thread(parent_thread_id: DWORD, msg_window_id: HWND) { let mut wait_until_opt = None; 'main: loop { - if 0 == winuser::GetMessageW(&mut msg, ptr::null_mut(), 0, 0) { - break 'main; + // Zeroing out the message ensures that the `WaitUntilInstantBox` doesn't get + // double-freed if `MsgWaitForMultipleObjectsEx` returns early and there aren't + // additional messages to process. + msg = mem::zeroed(); + + if wait_until_opt.is_some() { + if 0 != winuser::PeekMessageW(&mut msg, ptr::null_mut(), 0, 0, winuser::PM_REMOVE) { + winuser::TranslateMessage(&mut msg); + winuser::DispatchMessageW(&mut msg); + } + } else { + if 0 == winuser::GetMessageW(&mut msg, ptr::null_mut(), 0, 0) { + break 'main; + } else { + winuser::TranslateMessage(&mut msg); + winuser::DispatchMessageW(&mut msg); + } } - winuser::TranslateMessage(&mut msg); - winuser::DispatchMessageW(&mut msg); if msg.message == *WAIT_UNTIL_MSG_ID { wait_until_opt = Some(*WaitUntilInstantBox::from_raw(msg.lParam as *mut _)); @@ -326,6 +339,9 @@ fn wait_thread(parent_thread_id: DWORD, msg_window_id: HWND) { winuser::PostMessageW(msg_window_id, *PROCESS_NEW_EVENTS_MSG_ID, 0, 0); wait_until_opt = None; } + } else { + winuser::PostMessageW(msg_window_id, *PROCESS_NEW_EVENTS_MSG_ID, 0, 0); + wait_until_opt = None; } } } From fd9a693cfabe71ebbe32abfb754887dba75c2189 Mon Sep 17 00:00:00 2001 From: Osspial Date: Sun, 19 Apr 2020 14:24:56 -0400 Subject: [PATCH 17/19] Add changelog entry --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ecb4aa4f7c..0c2b33e404 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # Unreleased +- On Windows, fix window intermittently hanging when `ControlFlow` was set to `Poll`. + # 0.22.1 (2020-04-16) - On X11, fix `ResumeTimeReached` being fired too early. From 0b588a381517a27e75d5eba42a45ce68bae1cd62 Mon Sep 17 00:00:00 2001 From: Osspial Date: Sun, 19 Apr 2020 14:26:15 -0400 Subject: [PATCH 18/19] Format --- 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 10dc76159c..defe5f480b 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -315,7 +315,6 @@ fn wait_thread(parent_thread_id: DWORD, msg_window_id: HWND) { } } - if msg.message == *WAIT_UNTIL_MSG_ID { wait_until_opt = Some(*WaitUntilInstantBox::from_raw(msg.lParam as *mut _)); } else if msg.message == *CANCEL_WAIT_UNTIL_MSG_ID { From 534772b1d9546cc169d8e49bfc44b584c1b1c022 Mon Sep 17 00:00:00 2001 From: Osspial Date: Sun, 19 Apr 2020 15:24:23 -0400 Subject: [PATCH 19/19] Remove UpdateWindow panic --- src/platform_impl/windows/event_loop.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index defe5f480b..a7deac50c5 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -805,15 +805,14 @@ unsafe extern "system" fn public_window_callback( winuser::WM_PAINT => { if subclass_input.event_loop_runner.should_buffer() { - panic!("InvalidateRgn should be used instead of UpdateWindow"); - // // this branch can happen in response to `UpdateWindow`, if win32 decides to - // // redraw the window outside the normal flow of the event loop. - // winuser::RedrawWindow( - // window, - // ptr::null(), - // ptr::null_mut(), - // winuser::RDW_INTERNALPAINT, - // ); + // this branch can happen in response to `UpdateWindow`, if win32 decides to + // redraw the window outside the normal flow of the event loop. + winuser::RedrawWindow( + window, + ptr::null(), + ptr::null_mut(), + winuser::RDW_INTERNALPAINT, + ); } else { let managing_redraw = flush_paint_messages(Some(window), &subclass_input.event_loop_runner);