On Windows, fix request_redraw() related panics

These panics were introduced by 6a330a2 Fixes #1391 Fixes #1400 Fixes #1466 Probably fixes other related issues See #1429
rust-windowing · Feb 25, 2020 · 37141ef · 37141ef
1 parent 9999f53
commit 37141ef
Show file tree

Hide file tree

Showing 4 changed files with 123 additions and 261 deletions.
diff --git 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
@@ -223,67 +223,61 @@ impl<T: 'static> EventLoop<T> {
                 runner.new_events();
                 loop {
                     if !unread_message_exists {
-                        if 0 == winuser::PeekMessageW(&mut msg, ptr::null_mut(), 0, 0, 1) {
+                        if 0 == winuser::PeekMessageW(
+                            &mut msg,
+                            ptr::null_mut(),
+                            0,
+                            0,
+                            winuser::PM_REMOVE,
+                        ) {
                             break;
                         }
                     }
-                    if msg.message == winuser::WM_PAINT {
-                        unread_message_exists = true;
-                        break;
-                    }
                     winuser::TranslateMessage(&mut msg);
                     winuser::DispatchMessageW(&mut msg);
 
                     unread_message_exists = false;
+
+                    if msg.message == winuser::WM_PAINT {
+                        break;
+                    }
                 }
+                assert!(!unread_message_exists);
                 runner.main_events_cleared();
+                // Drain eventual WM_PAINT messages sent if user called request_redraw()
+                // during handling of MainEventsCleared.
                 loop {
-                    if !unread_message_exists {
-                        if 0 == winuser::PeekMessageW(
-                            &mut msg,
-                            ptr::null_mut(),
-                            winuser::WM_PAINT,
-                            winuser::WM_PAINT,
-                            1,
-                        ) {
-                            break;
-                        }
+                    if 0 == winuser::PeekMessageW(
+                        &mut msg,
+                        ptr::null_mut(),
+                        winuser::WM_PAINT,
+                        winuser::WM_PAINT,
+                        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;
+                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;
                         }
-                        ControlFlow::WaitUntil(resume_time) => {
-                            wait_until_time_or_msg(resume_time);
-                        }
-                        ControlFlow::Poll => (),
+                        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();
     }
 
@@ -674,7 +668,6 @@ unsafe extern "system" fn public_window_callback<T: 'static>(
         }
 
         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)
         }
@@ -1718,50 +1711,39 @@ unsafe extern "system" fn thread_event_target_callback<T: 'static>(
             };
             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_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();
                     }
                 }
             }