On Windows, fix bug where RedrawRequested would only get emitted ever…

…y other iteration of the event loop (#1366) * Fix bug causing RedrawRequested events to only get emitted every other iteration of the event loop. * Initialize simple_logger in examples. This PR's primary bug was discovered because a friend of mine reported that winit was emitting concerning log messages, which I'd never seen since none of the examples print out the log messages. This addresses that, to hopefully reduce the chance of bugs going unnoticed in the future. * Add changelog entry * Format
rust-windowing · Jan 6, 2020 · 6a330a2 · 6a330a2
1 parent 627a127
commit 6a330a2
Show file tree

Hide file tree

Showing 25 changed files with 323 additions and 122 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,7 @@
 # Unreleased
 
+- On Windows, fix bug where `RedrawRequested` would only get emitted every other iteration of the event loop.
+
 # 0.20.0 (2020-01-05)
 
 - On X11, fix `ModifiersChanged` emitting incorrect modifier change events

diff --git a/Cargo.toml b/Cargo.toml
@@ -29,7 +29,7 @@ bitflags = "1"
 
 [dev-dependencies]
 image = "0.21"
-env_logger = "0.5"
+simple_logger = "1"
 
 [target.'cfg(target_os = "android")'.dependencies.android_glue]
 version = "0.2"

diff --git a/examples/cursor.rs b/examples/cursor.rs
@@ -5,6 +5,7 @@ use winit::{
 };
 
 fn main() {
+    simple_logger::init().unwrap();
     let event_loop = EventLoop::new();
 
     let window = WindowBuilder::new().build(&event_loop).unwrap();

diff --git a/examples/cursor_grab.rs b/examples/cursor_grab.rs
@@ -5,6 +5,7 @@ use winit::{
 };
 
 fn main() {
+    simple_logger::init().unwrap();
     let event_loop = EventLoop::new();
 
     let window = WindowBuilder::new()

diff --git a/examples/custom_events.rs b/examples/custom_events.rs
@@ -11,6 +11,7 @@ fn main() {
         Timer,
     }
 
+    simple_logger::init().unwrap();
     let event_loop = EventLoop::<CustomEvent>::with_user_event();
 
     let _window = WindowBuilder::new()

diff --git a/examples/fullscreen.rs b/examples/fullscreen.rs
@@ -5,6 +5,7 @@ use winit::monitor::{MonitorHandle, VideoMode};
 use winit::window::{Fullscreen, WindowBuilder};
 
 fn main() {
+    simple_logger::init().unwrap();
     let event_loop = EventLoop::new();
 
     print!("Please choose the fullscreen mode: (1) exclusive, (2) borderless: ");

diff --git a/examples/handling_close.rs b/examples/handling_close.rs
@@ -5,6 +5,7 @@ use winit::{
 };
 
 fn main() {
+    simple_logger::init().unwrap();
     let event_loop = EventLoop::new();
 
     let _window = WindowBuilder::new()

diff --git a/examples/min_max_size.rs b/examples/min_max_size.rs
@@ -6,6 +6,7 @@ use winit::{
 };
 
 fn main() {
+    simple_logger::init().unwrap();
     let event_loop = EventLoop::new();
 
     let window = WindowBuilder::new().build(&event_loop).unwrap();

diff --git a/examples/minimize.rs b/examples/minimize.rs
@@ -5,6 +5,7 @@ use winit::event_loop::{ControlFlow, EventLoop};
 use winit::window::WindowBuilder;
 
 fn main() {
+    simple_logger::init().unwrap();
     let event_loop = EventLoop::new();
 
     let window = WindowBuilder::new()

diff --git a/examples/monitor_list.rs b/examples/monitor_list.rs
@@ -1,6 +1,7 @@
 use winit::{event_loop::EventLoop, window::WindowBuilder};
 
 fn main() {
+    simple_logger::init().unwrap();
     let event_loop = EventLoop::new();
     let window = WindowBuilder::new().build(&event_loop).unwrap();
 

diff --git a/examples/multithreaded.rs b/examples/multithreaded.rs
@@ -1,7 +1,5 @@
 #[cfg(not(target_arch = "wasm32"))]
 fn main() {
-    extern crate env_logger;
-
     use std::{collections::HashMap, sync::mpsc, thread, time::Duration};
 
     use winit::{
@@ -14,7 +12,7 @@ fn main() {
     const WINDOW_COUNT: usize = 3;
     const WINDOW_SIZE: PhysicalSize<u32> = PhysicalSize::new(600, 400);
 
-    env_logger::init();
+    simple_logger::init().unwrap();
     let event_loop = EventLoop::new();
     let mut window_senders = HashMap::with_capacity(WINDOW_COUNT);
     for _ in 0..WINDOW_COUNT {

diff --git a/examples/multiwindow.rs b/examples/multiwindow.rs
@@ -6,6 +6,7 @@ use winit::{
 };
 
 fn main() {
+    simple_logger::init().unwrap();
     let event_loop = EventLoop::new();
 
     let mut windows = HashMap::new();

diff --git a/examples/request_redraw.rs b/examples/request_redraw.rs
@@ -5,6 +5,7 @@ use winit::{
 };
 
 fn main() {
+    simple_logger::init().unwrap();
     let event_loop = EventLoop::new();
 
     let window = WindowBuilder::new()

diff --git a/examples/resizable.rs b/examples/resizable.rs
@@ -6,6 +6,7 @@ use winit::{
 };
 
 fn main() {
+    simple_logger::init().unwrap();
     let event_loop = EventLoop::new();
 
     let mut resizable = false;

diff --git a/examples/timer.rs b/examples/timer.rs
@@ -7,6 +7,7 @@ use winit::{
 };
 
 fn main() {
+    simple_logger::init().unwrap();
     let event_loop = EventLoop::new();
 
     let _window = WindowBuilder::new()

diff --git a/examples/transparent.rs b/examples/transparent.rs
@@ -5,6 +5,7 @@ use winit::{
 };
 
 fn main() {
+    simple_logger::init().unwrap();
     let event_loop = EventLoop::new();
 
     let window = WindowBuilder::new()

diff --git a/examples/video_modes.rs b/examples/video_modes.rs
@@ -1,6 +1,7 @@
 use winit::event_loop::EventLoop;
 
 fn main() {
+    simple_logger::init().unwrap();
     let event_loop = EventLoop::new();
     let monitor = event_loop.primary_monitor();
 

diff --git a/examples/window.rs b/examples/window.rs
@@ -5,6 +5,7 @@ use winit::{
 };
 
 fn main() {
+    simple_logger::init().unwrap();
     let event_loop = EventLoop::new();
 
     let window = WindowBuilder::new()

diff --git a/examples/window_debug.rs b/examples/window_debug.rs
@@ -8,6 +8,7 @@ use winit::{
 };
 
 fn main() {
+    simple_logger::init().unwrap();
     let event_loop = EventLoop::new();
 
     let window = WindowBuilder::new()

diff --git a/examples/window_icon.rs b/examples/window_icon.rs
@@ -7,6 +7,8 @@ use winit::{
 };
 
 fn main() {
+    simple_logger::init().unwrap();
+
     // You'll have to choose an icon size at your own discretion. On X11, the desired size varies
     // by WM, and on Windows, you still have to account for screen scaling. Here we use 32px,
     // since it seems to work well enough in most cases. Be careful about going too high, or

diff --git a/examples/window_run_return.rs b/examples/window_run_return.rs
@@ -18,6 +18,7 @@ fn main() {
     };
     let mut event_loop = EventLoop::new();
 
+    simple_logger::init().unwrap();
     let _window = WindowBuilder::new()
         .with_title("A fantastic window!")
         .build(&event_loop)

diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs
@@ -100,13 +100,9 @@ pub(crate) struct SubclassInput<T: 'static> {
 }
 
 impl<T> SubclassInput<T> {
-    unsafe fn send_event(&self, event: Event<'static, T>) {
+    unsafe fn send_event(&self, event: Event<'_, T>) {
         self.event_loop_runner.send_event(event);
     }
-
-    unsafe fn send_event_unbuffered<'e>(&self, event: Event<'e, T>) -> Result<(), Event<'e, T>> {
-        self.event_loop_runner.send_event_unbuffered(event)
-    }
 }
 
 struct ThreadMsgTargetSubclassInput<T: 'static> {
@@ -116,7 +112,7 @@ struct ThreadMsgTargetSubclassInput<T: 'static> {
 }
 
 impl<T> ThreadMsgTargetSubclassInput<T> {
-    unsafe fn send_event(&self, event: Event<'static, T>) {
+    unsafe fn send_event(&self, event: Event<'_, T>) {
         self.event_loop_runner.send_event(event);
     }
 }
@@ -216,36 +212,65 @@ impl<T: 'static> EventLoop<T> {
 
         unsafe {
             let mut msg = mem::zeroed();
-            let mut msg_unprocessed = false;
+            let mut unread_message_exists = false;
 
             'main: loop {
+                if let Err(payload) = runner.take_panic_error() {
+                    runner.destroy_runner();
+                    panic::resume_unwind(payload);
+                }
+
                 runner.new_events();
                 loop {
-                    if !msg_unprocessed {
+                    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);
 
-                    msg_unprocessed = false;
+                    unread_message_exists = false;
                 }
-                runner.events_cleared();
-                if let Err(payload) = runner.take_panic_error() {
-                    runner.destroy_runner();
-                    panic::resume_unwind(payload);
+                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,
+                        ) {
+                            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 !msg_unprocessed {
+                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;
                             }
-                            msg_unprocessed = true;
+                            unread_message_exists = true;
                         }
                         ControlFlow::WaitUntil(resume_time) => {
                             wait_until_time_or_msg(resume_time);
@@ -651,6 +676,7 @@ 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)
         }
@@ -1497,7 +1523,7 @@ unsafe extern "system" fn public_window_callback<T: 'static>(
                 false => old_physical_inner_size,
             };
 
-            let _ = subclass_input.send_event_unbuffered(Event::WindowEvent {
+            let _ = subclass_input.send_event(Event::WindowEvent {
                 window_id: RootWindowId(WindowId(window)),
                 event: ScaleFactorChanged {
                     scale_factor: new_dpi_factor,
@@ -1697,44 +1723,49 @@ 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 mut msg = mem::zeroed();
-                loop {
-                    if 0 == winuser::PeekMessageW(&mut msg, ptr::null_mut(), 0, 0, 0) {
-                        break;
+                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;
                     }
-                    // Clear all paint/timer messages from the queue before sending the events cleared message.
-                    match msg.message {
-                        // Flush the event queue of WM_PAINT messages.
-                        winuser::WM_PAINT | winuser::WM_TIMER => {
-                            // Remove the message from the message queue.
-                            winuser::PeekMessageW(&mut msg, ptr::null_mut(), 0, 0, 1);
-
-                            if msg.hwnd != window {
-                                winuser::TranslateMessage(&mut msg);
-                                winuser::DispatchMessageW(&mut msg);
-                            }
+
+                    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 the message isn't one of those three, it may be handled by the modal
-                        // loop so we should return control flow to it.
-                        _ => {
-                            queue_call_again();
-                            return 0;
+
+                        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;
-                runner.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();
+                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();
+                        }
                     }
                 }
             }
-Original file line number
+Diff line change
@@ Expand Up / @@ -11,6 +11,7 @@ fn main() { @@
             Timer,
         }
+        simple_logger::init().unwrap();
         let event_loop = EventLoop::<CustomEvent>::with_user_event();
         let _window = WindowBuilder::new()
@@ Expand Down @@