diff --git a/CHANGELOG.md b/CHANGELOG.md index 75d927c6..e86e6d44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Unreleased - On Windows, fix minor timing issue in wait_until_time_or_msg +- On Windows, rework handling of request_redraw() to address panics. - On macOS, fix `set_simple_screen` to remember frame excluding title bar. - On Wayland, fix coordinates in touch events when scale factor isn't 1. - On Wayland, fix color from `close_button_icon_color` not applying. diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index a443a6ce..26a2d321 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -218,69 +218,68 @@ impl EventLoop { } runner.new_events(); - loop { - if !unread_message_exists { - if 0 == winuser::PeekMessageW(&mut msg, ptr::null_mut(), 0, 0, 1) { - break; - } - } - if msg.message == winuser::WM_PAINT { - unread_message_exists = true; - break; - } - winuser::TranslateMessage(&mut msg); - winuser::DispatchMessageW(&mut msg); - - unread_message_exists = false; - } - runner.main_events_cleared(); loop { if !unread_message_exists { if 0 == winuser::PeekMessageW( &mut msg, ptr::null_mut(), - winuser::WM_PAINT, - winuser::WM_PAINT, - 1, + 0, + 0, + winuser::PM_REMOVE, ) { break; } } - winuser::TranslateMessage(&mut msg); winuser::DispatchMessageW(&mut msg); unread_message_exists = false; - } - if runner.redraw_events_cleared().events_buffered() { - if runner.control_flow() == ControlFlow::Exit { - break 'main; - } - continue; - } - if !unread_message_exists { - let control_flow = runner.control_flow(); - match 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 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; } } + // 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 => (), + } } } - unsafe { - runner.call_event_handler(Event::LoopDestroyed); - } + runner.destroy_loop(); runner.destroy_runner(); } @@ -652,13 +651,6 @@ unsafe extern "system" fn public_window_callback( } winuser::WM_NCLBUTTONDOWN => { - // jumpstart the modal loop - winuser::RedrawWindow( - window, - ptr::null(), - ptr::null_mut(), - winuser::RDW_INTERNALPAINT, - ); if wparam == winuser::HTCAPTION as _ { winuser::PostMessageW(window, winuser::WM_MOUSEMOVE, 0, 0); } @@ -688,7 +680,6 @@ unsafe extern "system" fn public_window_callback( } winuser::WM_PAINT => { - subclass_input.event_loop_runner.main_events_cleared(); subclass_input.send_event(Event::RedrawRequested(RootWindowId(WindowId(window)))); commctrl::DefSubclassProc(window, msg, wparam, lparam) } @@ -1780,50 +1771,39 @@ unsafe extern "system" fn thread_event_target_callback( }; 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(); - if 0 == winuser::PeekMessageW(&mut msg, ptr::null_mut(), 0, 0, 0) { - if msg.message != 0 && msg.message != winuser::WM_PAINT { - queue_call_again(); - return 0; + loop { + if 0 == winuser::PeekMessageW( + &mut msg, + ptr::null_mut(), + winuser::WM_PAINT, + winuser::WM_PAINT, + winuser::PM_QS_PAINT | winuser::PM_REMOVE, + ) { + break; } - subclass_input.event_loop_runner.main_events_cleared(); - loop { - if 0 == winuser::PeekMessageW( - &mut msg, - ptr::null_mut(), - winuser::WM_PAINT, - winuser::WM_PAINT, - 1, - ) { - break; - } - - if msg.hwnd != window { - winuser::TranslateMessage(&mut msg); - winuser::DispatchMessageW(&mut msg); - } + if msg.hwnd != window { + winuser::TranslateMessage(&mut msg); + winuser::DispatchMessageW(&mut msg); } } - - // we don't borrow until here because TODO SAFETY - let runner = &subclass_input.event_loop_runner; - if runner.redraw_events_cleared().events_buffered() { - queue_call_again(); - runner.new_events(); - } else { - 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(); - } + 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(); } } } diff --git a/src/platform_impl/windows/event_loop/runner.rs b/src/platform_impl/windows/event_loop/runner.rs index 44a044d4..e5c062b0 100644 --- a/src/platform_impl/windows/event_loop/runner.rs +++ b/src/platform_impl/windows/event_loop/runner.rs @@ -6,7 +6,7 @@ use crate::{ dpi::PhysicalSize, event::{Event, StartCause, WindowEvent}, event_loop::ControlFlow, - platform_impl::platform::{event_loop::EventLoop, util}, + platform_impl::platform::event_loop::{util, EventLoop}, window::WindowId, }; @@ -14,8 +14,8 @@ pub(crate) type EventLoopRunnerShared = Rc>; pub(crate) struct ELRShared { runner: RefCell>>, buffer: RefCell>>, - redraw_buffer: Rc>>, } + struct EventLoopRunner { control_flow: ControlFlow, runner_state: RunnerState, @@ -23,8 +23,8 @@ struct EventLoopRunner { in_modal_loop: bool, event_handler: Box, &mut ControlFlow)>, panic_error: Option, - redraw_buffer: Rc>>, } + pub type PanicError = Box; pub enum BufferedEvent { @@ -32,22 +32,6 @@ pub enum BufferedEvent { ScaleFactorChanged(WindowId, f64, PhysicalSize), } -#[must_use] -#[derive(Debug, Clone, Copy)] -pub enum AreEventsBuffered { - EventsBuffered, - ReadyToSleep, -} - -impl AreEventsBuffered { - pub fn events_buffered(&self) -> bool { - match self { - Self::EventsBuffered => true, - Self::ReadyToSleep => false, - } - } -} - impl BufferedEvent { pub fn from_event(event: Event<'_, T>) -> BufferedEvent { match event { @@ -89,7 +73,6 @@ impl ELRShared { ELRShared { runner: RefCell::new(None), buffer: RefCell::new(VecDeque::new()), - redraw_buffer: Default::default(), } } @@ -97,16 +80,11 @@ impl ELRShared { where F: FnMut(Event<'_, T>, &mut ControlFlow), { - let mut runner = EventLoopRunner::new(event_loop, self.redraw_buffer.clone(), f); + let mut runner = EventLoopRunner::new(event_loop, f); { let mut runner_ref = self.runner.borrow_mut(); - loop { - let event = self.buffer.borrow_mut().pop_front(); - match event { - Some(e) => e.dispatch_event(|e| runner.process_event(e)), - None => break, - } - } + // Dispatch any events that were buffered during the creation of the window + self.dispatch_buffered_events(&mut runner); *runner_ref = Some(runner); } } @@ -119,80 +97,46 @@ impl ELRShared { let mut runner_ref = self.runner.borrow_mut(); if let Some(ref mut runner) = *runner_ref { runner.new_events(); - 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, - } - } + // Dispatch any events that were buffered during the call `new_events` + self.dispatch_buffered_events(runner); } } - pub(crate) unsafe fn send_event(&self, event: Event<'_, T>) { - let handling_redraw = self - .runner - .borrow() - .as_ref() - .map(|r| RunnerState::HandlingRedraw == r.runner_state) - .unwrap_or(false); - let mut send = None; - if handling_redraw { + 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 { - send = Some(event); - } else { - self.buffer_event(event); - } - } else { - send = Some(event); - } - if let Some(event) = send { - 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. - self.buffer_event(event); + panic!("buffering RedrawRequested event"); } + self.buffer + .borrow_mut() + .push_back(BufferedEvent::from_event(event)); } } - unsafe fn send_event_unbuffered<'e>(&self, event: Event<'e, T>) -> Result<(), Event<'e, T>> { + 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); - - let handling_redraw = if let RunnerState::HandlingRedraw = runner.runner_state { - true - } else { - false - }; - - if !handling_redraw { - // Dispatch any events that were buffered during the call to `process_event`. - 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.buffer.borrow_mut().pop_front(); - match buffered_event_opt { - Some(e) => e.dispatch_event(|e| runner.process_event(e)), - None => break, - } - } - } - + // 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 call_event_handler(&self, event: Event<'static, T>) { - if let Ok(mut runner_ref) = self.runner.try_borrow_mut() { - if let Some(ref mut runner) = *runner_ref { - runner.call_event_handler(event); - return; + 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, } } } @@ -201,17 +145,27 @@ impl ELRShared { 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(crate) fn redraw_events_cleared(&self) -> AreEventsBuffered { + 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"); + } } - match self.buffer.borrow().len() { - 0 => AreEventsBuffered::ReadyToSleep, - _ => AreEventsBuffered::EventsBuffered, + } + + 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); + } } } @@ -228,6 +182,17 @@ impl ELRShared { 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, + ); + } + } } } @@ -248,18 +213,6 @@ impl ELRShared { ControlFlow::Exit } } - - fn buffer_event(&self, event: Event<'_, T>) { - match event { - Event::RedrawRequested(window_id) => { - self.redraw_buffer.borrow_mut().push_back(window_id) - } - _ => self - .buffer - .borrow_mut() - .push_back(BufferedEvent::from_event(event)), - } - } } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -275,15 +228,13 @@ enum RunnerState { /// 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, - redraw_buffer: Rc>>, - f: F, - ) -> EventLoopRunner + unsafe fn new(event_loop: &EventLoop, f: F) -> EventLoopRunner where F: FnMut(Event<'_, T>, &mut ControlFlow), { @@ -297,7 +248,6 @@ impl EventLoopRunner { Box, &mut ControlFlow)>, >(Box::new(f)), panic_error: None, - redraw_buffer, } } @@ -323,6 +273,8 @@ impl EventLoopRunner { } // 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. @@ -411,24 +363,21 @@ impl EventLoopRunner { // 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::HandlingRedraw, Event::RedrawRequested(_)) => { - self.call_event_handler(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::New, Event::RedrawRequested(_)) - | (RunnerState::Idle(..), Event::RedrawRequested(_)) => { - self.new_events(); - self.main_events_cleared(); - self.call_event_handler(event); - } - (_, Event::RedrawRequested(_)) => { - panic!("redraw event in non-redraw phase"); + (RunnerState::HandlingRedraw, Event::RedrawRequested(window_id)) => { + self.call_event_handler(Event::RedrawRequested(*window_id)); } (RunnerState::HandlingRedraw, _) => { - panic!( - "Non-redraw event dispatched durning redraw phase: {:?}", + warn!( + "non-redraw event in redraw phase: {:?}", event.map_nonuser_event::<()>().ok() ); } @@ -439,16 +388,6 @@ impl EventLoopRunner { } } - fn flush_redraws(&mut self) { - loop { - let redraw_window_opt = self.redraw_buffer.borrow_mut().pop_front(); - match redraw_window_opt { - Some(window_id) => self.process_event(Event::RedrawRequested(window_id)), - None => break, - } - } - } - fn main_events_cleared(&mut self) { match self.runner_state { // If we were handling events, send the MainEventsCleared message. @@ -457,7 +396,9 @@ impl EventLoopRunner { self.runner_state = RunnerState::HandlingRedraw; } - RunnerState::HandlingRedraw => (), + // 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(..) => (), @@ -469,6 +410,7 @@ impl EventLoopRunner { // 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; } @@ -482,6 +424,7 @@ impl EventLoopRunner { requested_resume: resume_time, }, )); + self.runner_state = RunnerState::HandlingEvents; self.call_event_handler(Event::MainEventsCleared); self.runner_state = RunnerState::HandlingRedraw; } @@ -496,57 +439,18 @@ impl EventLoopRunner { fn redraw_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; - self.flush_redraws(); - self.call_event_handler(Event::RedrawEventsCleared); - self.runner_state = RunnerState::Idle(Instant::now()); - } - + // If we were handling redraws, send the RedrawEventsCleared message. RunnerState::HandlingRedraw => { - self.flush_redraws(); self.call_event_handler(Event::RedrawEventsCleared); self.runner_state = RunnerState::Idle(Instant::now()); } - - // 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.call_event_handler(Event::MainEventsCleared); - self.flush_redraws(); - self.call_event_handler(Event::RedrawEventsCleared); - } - // 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.call_event_handler(Event::MainEventsCleared); - self.flush_redraws(); - self.call_event_handler(Event::RedrawEventsCleared); - } - } - // If we deferred a wait and no events were received, the user doesn't have to - // get an event. - ControlFlow::Wait | ControlFlow::Exit => (), - } - // Mark that we've entered an idle state. - self.runner_state = RunnerState::Idle(wait_start) - } + // 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 + ), } } diff --git a/src/platform_impl/windows/util.rs b/src/platform_impl/windows/util.rs index a5c60f9b..5faaae3e 100644 --- a/src/platform_impl/windows/util.rs +++ b/src/platform_impl/windows/util.rs @@ -116,7 +116,7 @@ pub(crate) fn set_inner_size_physical(window: HWND, x: u32, y: u32) { | winuser::SWP_NOMOVE | winuser::SWP_NOACTIVATE, ); - winuser::UpdateWindow(window); + winuser::InvalidateRgn(window, ptr::null_mut(), 0); } } diff --git a/src/platform_impl/windows/window.rs b/src/platform_impl/windows/window.rs index 7feb631c..0f9a09c7 100644 --- a/src/platform_impl/windows/window.rs +++ b/src/platform_impl/windows/window.rs @@ -187,7 +187,7 @@ impl Window { | winuser::SWP_NOSIZE | winuser::SWP_NOACTIVATE, ); - winuser::UpdateWindow(self.window.0); + winuser::InvalidateRgn(self.window.0, ptr::null_mut(), 0); } } @@ -506,7 +506,7 @@ impl Window { size.1 as i32, winuser::SWP_ASYNCWINDOWPOS | winuser::SWP_NOZORDER, ); - winuser::UpdateWindow(window.0); + winuser::InvalidateRgn(window.0, ptr::null_mut(), 0); } } None => { @@ -532,7 +532,7 @@ impl Window { | winuser::SWP_NOZORDER | winuser::SWP_NOACTIVATE, ); - winuser::UpdateWindow(window.0); + winuser::InvalidateRgn(window.0, ptr::null_mut(), 0); } } } diff --git a/src/platform_impl/windows/window_state.rs b/src/platform_impl/windows/window_state.rs index 4be0c796..9a5414a6 100644 --- a/src/platform_impl/windows/window_state.rs +++ b/src/platform_impl/windows/window_state.rs @@ -31,9 +31,6 @@ pub struct WindowState { pub modifiers_state: ModifiersState, pub fullscreen: Option, - /// Used to supress duplicate redraw attempts when calling `request_redraw` multiple - /// times in `MainEventsCleared`. - pub queued_out_of_band_redraw: bool, pub is_dark_mode: bool, pub high_surrogate: Option, window_flags: WindowFlags, @@ -123,7 +120,6 @@ impl WindowState { modifiers_state: ModifiersState::default(), fullscreen: None, - queued_out_of_band_redraw: false, is_dark_mode, high_surrogate: None, window_flags: WindowFlags::empty(), @@ -273,7 +269,7 @@ impl WindowFlags { | winuser::SWP_NOSIZE | winuser::SWP_NOACTIVATE, ); - winuser::UpdateWindow(window); + winuser::InvalidateRgn(window, ptr::null_mut(), 0); } }