From dd866a74a690d37fdd11385741dee06e9f7cd02f Mon Sep 17 00:00:00 2001 From: Osspial Date: Thu, 2 Jul 2020 16:53:47 -0400 Subject: [PATCH] On Windows, fix bug where we'd try to emit `MainEventsCleared` events during nested win32 event loops (#1615) --- CHANGELOG.md | 2 + src/platform_impl/windows/event_loop.rs | 29 +++++++---- .../windows/event_loop/runner.rs | 49 ++++++++++++++++--- 3 files changed, 63 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c042e13..d28b32c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,5 @@ # Unreleased + - On Unix, X11 and Wayland are now optional features (enabled by default) - On X11, fix deadlock when calling `set_fullscreen_inner`. - On Web, prevent the webpage from scrolling when the user is focused on a winit canvas @@ -7,6 +8,7 @@ - On macOS, add `hide__other_applications` to `EventLoopWindowTarget` via existing `EventLoopWindowTargetExtMacOS` trait. `hide_other_applications` will hide other applications by calling `-[NSApplication hideOtherApplications: nil]`. - On android added support for `run_return`. - On MacOS, Fixed fullscreen and dialog support for `run_return`. +- On Windows, fix bug where we'd try to emit `MainEventsCleared` events during nested win32 event loops. # 0.22.2 (2020-05-16) diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index 739990a6..61ec167f 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -226,7 +226,7 @@ impl EventLoop { } unsafe { - runner.call_event_handler(Event::LoopDestroyed); + runner.loop_destroyed(); } runner.reset_runner(); } @@ -1933,14 +1933,25 @@ unsafe extern "system" fn thread_event_target_callback( // 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); + if subclass_input.event_loop_runner.should_buffer() { + // This branch can be triggered when a nested win32 event loop is triggered + // inside of the `event_handler` callback. + winuser::RedrawWindow( + window, + ptr::null(), + ptr::null_mut(), + winuser::RDW_INTERNALPAINT, + ); + } else { + // 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 diff --git a/src/platform_impl/windows/event_loop/runner.rs b/src/platform_impl/windows/event_loop/runner.rs index 258a40c0..4f90966c 100644 --- a/src/platform_impl/windows/event_loop/runner.rs +++ b/src/platform_impl/windows/event_loop/runner.rs @@ -53,6 +53,8 @@ enum RunnerState { /// 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, + /// The event loop has been destroyed. No other events will be emitted. + Destroyed, } enum BufferedEvent { @@ -229,7 +231,11 @@ impl EventLoopRunner { self.move_state_to(RunnerState::Idle); } - pub(crate) unsafe fn call_event_handler(&self, event: Event<'_, T>) { + pub(crate) unsafe fn loop_destroyed(&self) { + self.move_state_to(RunnerState::Destroyed); + } + + unsafe fn call_event_handler(&self, event: Event<'_, T>) { self.catch_unwind(|| { let mut control_flow = self.control_flow.take(); let mut event_handler = self.event_handler.take() @@ -260,8 +266,8 @@ impl EventLoopRunner { } } - /// Dispatch control flow events (`NewEvents`, `MainEventsCleared`, and `RedrawEventsCleared`) as - /// necessary to bring the internal `RunnerState` to the new runner state. + /// Dispatch control flow events (`NewEvents`, `MainEventsCleared`, `RedrawEventsCleared`, and + /// `LoopDestroyed`) as necessary to bring the internal `RunnerState` to the new runner state. /// /// The state transitions are defined as follows: /// @@ -273,14 +279,20 @@ impl EventLoopRunner { /// ^ | /// | V /// Idle <--- HandlingRedrawEvents + /// | + /// V + /// Destroyed /// ``` /// - /// 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 + /// Attempting to transition back to `Uninitialized` will result in a panic. Attempting to + /// transition *from* `Destroyed` will also reuslt 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, HandlingRedrawEvents, Idle, Uninitialized}; + use RunnerState::{ + Destroyed, HandlingMainEvents, HandlingRedrawEvents, Idle, Uninitialized, + }; match ( self.runner_state.replace(new_runner_state), @@ -289,7 +301,8 @@ impl EventLoopRunner { (Uninitialized, Uninitialized) | (Idle, Idle) | (HandlingMainEvents, HandlingMainEvents) - | (HandlingRedrawEvents, HandlingRedrawEvents) => (), + | (HandlingRedrawEvents, HandlingRedrawEvents) + | (Destroyed, Destroyed) => (), // State transitions that initialize the event loop. (Uninitialized, HandlingMainEvents) => { @@ -304,6 +317,12 @@ impl EventLoopRunner { self.call_event_handler(Event::MainEventsCleared); self.call_redraw_events_cleared(); } + (Uninitialized, Destroyed) => { + self.call_new_events(true); + self.call_event_handler(Event::MainEventsCleared); + self.call_redraw_events_cleared(); + self.call_event_handler(Event::LoopDestroyed); + } (_, Uninitialized) => panic!("cannot move state to Uninitialized"), // State transitions that start the event handling process. @@ -314,6 +333,9 @@ impl EventLoopRunner { self.call_new_events(false); self.call_event_handler(Event::MainEventsCleared); } + (Idle, Destroyed) => { + self.call_event_handler(Event::LoopDestroyed); + } (HandlingMainEvents, HandlingRedrawEvents) => { self.call_event_handler(Event::MainEventsCleared); @@ -323,6 +345,11 @@ impl EventLoopRunner { self.call_event_handler(Event::MainEventsCleared); self.call_redraw_events_cleared(); } + (HandlingMainEvents, Destroyed) => { + self.call_event_handler(Event::MainEventsCleared); + self.call_redraw_events_cleared(); + self.call_event_handler(Event::LoopDestroyed); + } (HandlingRedrawEvents, Idle) => { self.call_redraw_events_cleared(); @@ -332,6 +359,12 @@ impl EventLoopRunner { self.call_redraw_events_cleared(); self.call_new_events(false); } + (HandlingRedrawEvents, Destroyed) => { + self.call_redraw_events_cleared(); + self.call_event_handler(Event::LoopDestroyed); + } + + (Destroyed, _) => panic!("cannot move state from Destroyed"), } }