From 13bd11689149df75f4610745b1f68a6269cf72ab Mon Sep 17 00:00:00 2001 From: Osspial Date: Wed, 2 Aug 2017 20:49:50 -0400 Subject: [PATCH 1/4] Fix laggy rendering when resizing win32 window --- src/platform/windows/events_loop.rs | 66 ++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/src/platform/windows/events_loop.rs b/src/platform/windows/events_loop.rs index 11716bac73..0926031dda 100644 --- a/src/platform/windows/events_loop.rs +++ b/src/platform/windows/events_loop.rs @@ -22,6 +22,7 @@ use std::ptr; use std::sync::mpsc; use std::sync::Arc; use std::sync::Mutex; +use std::sync::Barrier; use std::thread; use kernel32; @@ -78,22 +79,28 @@ pub struct EventsLoop { thread_id: winapi::DWORD, // Receiver for the events. The sender is in the background thread. receiver: mpsc::Receiver, + // Barrier used to unblock the event loop thread after a resize event is processed. + resize_barrier: Arc } impl EventsLoop { pub fn new() -> EventsLoop { // The main events transfer channel. let (tx, rx) = mpsc::channel(); + let resize_barrier = Arc::new(Barrier::new(2)); + let resize_barrier_child = resize_barrier.clone(); // Local channel in order to block the `new()` function until the background thread has // an events queue. let (local_block_tx, local_block_rx) = mpsc::channel(); - let thread = thread::spawn(move || { + let thread = thread::spawn(move || { CONTEXT_STASH.with(|context_stash| { *context_stash.borrow_mut() = Some(ThreadLocalData { sender: tx, windows: HashMap::with_capacity(4), + resize_barrier: resize_barrier_child, + deferred_waits: 0 }); }); @@ -123,6 +130,15 @@ impl EventsLoop { x if x == *WAKEUP_MSG_ID => { send_event(Event::Awakened); }, + x if x == *USE_WAIT_ID => CONTEXT_STASH.with(|context_stash| { + let mut context_stash = context_stash.borrow_mut(); + let cstash = context_stash.as_mut().unwrap(); + // Run a deferred wait + if cstash.deferred_waits > 0 { + cstash.deferred_waits -= 1; + cstash.resize_barrier.wait(); + } + }), _ => { // Calls `callback` below. user32::TranslateMessage(&msg); @@ -139,6 +155,7 @@ impl EventsLoop { EventsLoop { thread_id: unsafe { kernel32::GetThreadId(thread.as_raw_handle()) }, receiver: rx, + resize_barrier: resize_barrier } } @@ -150,8 +167,12 @@ impl EventsLoop { Ok(e) => e, Err(_) => return }; + let is_resize = event_is_resize(&event); callback(event); + if is_resize { + self.sync_with_thread(); + } } } @@ -163,8 +184,12 @@ impl EventsLoop { Ok(e) => e, Err(_) => return }; + let is_resize = event_is_resize(&event); let flow = callback(event); + if is_resize { + self.sync_with_thread(); + } match flow { ControlFlow::Continue => continue, ControlFlow::Break => break, @@ -178,6 +203,12 @@ impl EventsLoop { } } + fn sync_with_thread(&self) { + let res = unsafe{ user32::PostThreadMessageA(self.thread_id, *USE_WAIT_ID, 0, 0) }; + self.resize_barrier.wait(); + assert!(res != 0, "PostThreadMessage failed ; is the messages queue full?"); + } + /// Executes a function in the background thread. /// /// Note that we use a FnMut instead of a FnOnce because we're too lazy to create an equivalent @@ -251,6 +282,13 @@ lazy_static! { user32::RegisterWindowMessageA("Winit::ExecMsg".as_ptr() as *const i8) } }; + // Message sent when the parent thread receives a resize event and wants this thread to use any + // deferred waits it may have. + static ref USE_WAIT_ID: u32 = { + unsafe { + user32::RegisterWindowMessageA("Winit::UseWait\0".as_ptr() as *const i8) + } + }; } // There's no parameters passed to the callback function, so it needs to get its context stashed @@ -259,12 +297,22 @@ thread_local!(static CONTEXT_STASH: RefCell> = RefCell:: struct ThreadLocalData { sender: mpsc::Sender, windows: HashMap>>, + resize_barrier: Arc, + deferred_waits: u32 +} + +fn event_is_resize(event: &Event) -> bool { + match *event { + Event::WindowEvent{ event: WindowEvent::Resized(..), .. } => true, + _ => false + } } // Utility function that dispatches an event on the current thread. fn send_event(event: Event) { CONTEXT_STASH.with(|context_stash| { let context_stash = context_stash.borrow(); + let _ = context_stash.as_ref().unwrap().sender.send(event); // Ignoring if closed }); } @@ -306,6 +354,22 @@ pub unsafe extern "system" fn callback(window: winapi::HWND, msg: winapi::UINT, window_id: SuperWindowId(WindowId(window)), event: Resized(w, h), }); + + // Wait for the parent thread to process the resize event before returning from the + // callback. + CONTEXT_STASH.with(|context_stash| { + let mut context_stash = context_stash.borrow_mut(); + let cstash = context_stash.as_mut().unwrap(); + + if cstash.windows.get(&window).is_some() { + cstash.resize_barrier.wait(); + } else { + // If the window isn't in the hashmap, this is the resize event that was sent + // upon window creation. The parent thread isn't going to wait until the event + // loop, so record that a wait must happen when the event loop is reached. + cstash.deferred_waits += 1; + } + }); 0 }, From d2034b1700094c99c4317fa17eb308786fbeb786 Mon Sep 17 00:00:00 2001 From: Osspial Date: Wed, 2 Aug 2017 20:50:55 -0400 Subject: [PATCH 2/4] Add null terminator to custom events --- src/platform/windows/events_loop.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/platform/windows/events_loop.rs b/src/platform/windows/events_loop.rs index 0926031dda..b4bc70ac43 100644 --- a/src/platform/windows/events_loop.rs +++ b/src/platform/windows/events_loop.rs @@ -271,7 +271,7 @@ lazy_static! { // WPARAM and LPARAM are unused. static ref WAKEUP_MSG_ID: u32 = { unsafe { - user32::RegisterWindowMessageA("Winit::WakeupMsg".as_ptr() as *const i8) + user32::RegisterWindowMessageA("Winit::WakeupMsg\0".as_ptr() as *const i8) } }; // Message sent when we want to execute a closure in the thread. @@ -279,7 +279,7 @@ lazy_static! { // and LPARAM is unused. static ref EXEC_MSG_ID: u32 = { unsafe { - user32::RegisterWindowMessageA("Winit::ExecMsg".as_ptr() as *const i8) + user32::RegisterWindowMessageA("Winit::ExecMsg\0".as_ptr() as *const i8) } }; // Message sent when the parent thread receives a resize event and wants this thread to use any From 657860a233250da3ce91654e2ac3d1753126d985 Mon Sep 17 00:00:00 2001 From: Osspial Date: Sat, 5 Aug 2017 02:07:58 -0400 Subject: [PATCH 3/4] Re-implement resize patch using Mutex + Convar --- src/platform/windows/events_loop.rs | 70 ++++++++++++----------------- 1 file changed, 29 insertions(+), 41 deletions(-) diff --git a/src/platform/windows/events_loop.rs b/src/platform/windows/events_loop.rs index b4bc70ac43..9df777e0b5 100644 --- a/src/platform/windows/events_loop.rs +++ b/src/platform/windows/events_loop.rs @@ -1,12 +1,12 @@ //! 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 @@ -22,7 +22,7 @@ use std::ptr; use std::sync::mpsc; use std::sync::Arc; use std::sync::Mutex; -use std::sync::Barrier; +use std::sync::Condvar; use std::thread; use kernel32; @@ -79,16 +79,18 @@ pub struct EventsLoop { thread_id: winapi::DWORD, // Receiver for the events. The sender is in the background thread. receiver: mpsc::Receiver, - // Barrier used to unblock the event loop thread after a resize event is processed. - resize_barrier: Arc + // Variable that contains the block state of the win32 event loop thread during a WM_SIZE event. + // The mutex's value is `true` when it's blocked, and should be set to false when it's done + // blocking. That's done by the parent thread when it receives a Resized event. + win32_block_loop: Arc<(Mutex, Condvar)> } impl EventsLoop { pub fn new() -> EventsLoop { // The main events transfer channel. let (tx, rx) = mpsc::channel(); - let resize_barrier = Arc::new(Barrier::new(2)); - let resize_barrier_child = resize_barrier.clone(); + let win32_block_loop = Arc::new((Mutex::new(false), Condvar::new())); + let win32_block_loop_child = win32_block_loop.clone(); // Local channel in order to block the `new()` function until the background thread has // an events queue. @@ -99,8 +101,7 @@ impl EventsLoop { *context_stash.borrow_mut() = Some(ThreadLocalData { sender: tx, windows: HashMap::with_capacity(4), - resize_barrier: resize_barrier_child, - deferred_waits: 0 + win32_block_loop: win32_block_loop_child }); }); @@ -130,15 +131,6 @@ impl EventsLoop { x if x == *WAKEUP_MSG_ID => { send_event(Event::Awakened); }, - x if x == *USE_WAIT_ID => CONTEXT_STASH.with(|context_stash| { - let mut context_stash = context_stash.borrow_mut(); - let cstash = context_stash.as_mut().unwrap(); - // Run a deferred wait - if cstash.deferred_waits > 0 { - cstash.deferred_waits -= 1; - cstash.resize_barrier.wait(); - } - }), _ => { // Calls `callback` below. user32::TranslateMessage(&msg); @@ -155,7 +147,7 @@ impl EventsLoop { EventsLoop { thread_id: unsafe { kernel32::GetThreadId(thread.as_raw_handle()) }, receiver: rx, - resize_barrier: resize_barrier + win32_block_loop } } @@ -204,9 +196,10 @@ impl EventsLoop { } fn sync_with_thread(&self) { - let res = unsafe{ user32::PostThreadMessageA(self.thread_id, *USE_WAIT_ID, 0, 0) }; - self.resize_barrier.wait(); - assert!(res != 0, "PostThreadMessage failed ; is the messages queue full?"); + let (ref mutex, ref cvar) = *self.win32_block_loop; + let mut block_thread = mutex.lock().unwrap(); + *block_thread = false; + cvar.notify_all(); } /// Executes a function in the background thread. @@ -282,13 +275,6 @@ lazy_static! { user32::RegisterWindowMessageA("Winit::ExecMsg\0".as_ptr() as *const i8) } }; - // Message sent when the parent thread receives a resize event and wants this thread to use any - // deferred waits it may have. - static ref USE_WAIT_ID: u32 = { - unsafe { - user32::RegisterWindowMessageA("Winit::UseWait\0".as_ptr() as *const i8) - } - }; } // There's no parameters passed to the callback function, so it needs to get its context stashed @@ -297,8 +283,7 @@ thread_local!(static CONTEXT_STASH: RefCell> = RefCell:: struct ThreadLocalData { sender: mpsc::Sender, windows: HashMap>>, - resize_barrier: Arc, - deferred_waits: u32 + win32_block_loop: Arc<(Mutex, Condvar)> } fn event_is_resize(event: &Event) -> bool { @@ -361,13 +346,16 @@ pub unsafe extern "system" fn callback(window: winapi::HWND, msg: winapi::UINT, let mut context_stash = context_stash.borrow_mut(); let cstash = context_stash.as_mut().unwrap(); + // If this window has been inserted into the window map, the resize event happened + // during the event loop. If it hasn't, the event happened on window creation and + // should be ignored. if cstash.windows.get(&window).is_some() { - cstash.resize_barrier.wait(); - } else { - // If the window isn't in the hashmap, this is the resize event that was sent - // upon window creation. The parent thread isn't going to wait until the event - // loop, so record that a wait must happen when the event loop is reached. - cstash.deferred_waits += 1; + let (ref mutex, ref cvar) = *cstash.win32_block_loop; + let mut block_thread = mutex.lock().unwrap(); + *block_thread = true; + while *block_thread { + block_thread = cvar.wait(block_thread).unwrap(); + } } }); 0 @@ -425,7 +413,7 @@ pub unsafe extern "system" fn callback(window: winapi::HWND, msg: winapi::UINT, window_id: SuperWindowId(WindowId(window)), event: MouseEntered { device_id: DEVICE_ID }, }); - + // Calling TrackMouseEvent in order to receive mouse leave events. user32::TrackMouseEvent(&mut winapi::TRACKMOUSEEVENT { cbSize: mem::size_of::() as winapi::DWORD, @@ -612,7 +600,7 @@ pub unsafe extern "system" fn callback(window: winapi::HWND, msg: winapi::UINT, use events::WindowEvent::MouseInput; use events::MouseButton::Other; use events::ElementState::Released; - let xbutton = winapi::HIWORD(wparam as winapi::DWORD) as winapi::c_int; + let xbutton = winapi::HIWORD(wparam as winapi::DWORD) as winapi::c_int; send_event(Event::WindowEvent { window_id: SuperWindowId(WindowId(window)), event: MouseInput { device_id: DEVICE_ID, state: Released, button: Other(xbutton as u8) } @@ -702,7 +690,7 @@ pub unsafe extern "system" fn callback(window: winapi::HWND, msg: winapi::UINT, if call_def_window_proc { user32::DefWindowProcW(window, msg, wparam, lparam) - } else { + } else { 0 } }, From 786666aca803181aa6a6f8c936fbd5d914db7fcc Mon Sep 17 00:00:00 2001 From: Osspial Date: Sat, 5 Aug 2017 02:51:30 -0400 Subject: [PATCH 4/4] Revise Mutex+Convar implementation based on PR feedback --- src/platform/windows/events_loop.rs | 50 ++++++++++++++++------------- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/src/platform/windows/events_loop.rs b/src/platform/windows/events_loop.rs index 9df777e0b5..7587c1a780 100644 --- a/src/platform/windows/events_loop.rs +++ b/src/platform/windows/events_loop.rs @@ -159,11 +159,17 @@ impl EventsLoop { Ok(e) => e, Err(_) => return }; - let is_resize = event_is_resize(&event); + let is_resize = match event { + Event::WindowEvent{ event: WindowEvent::Resized(..), .. } => true, + _ => false + }; callback(event); if is_resize { - self.sync_with_thread(); + let (ref mutex, ref cvar) = *self.win32_block_loop; + let mut block_thread = mutex.lock().unwrap(); + *block_thread = false; + cvar.notify_all(); } } } @@ -176,11 +182,17 @@ impl EventsLoop { Ok(e) => e, Err(_) => return }; - let is_resize = event_is_resize(&event); + let is_resize = match event { + Event::WindowEvent{ event: WindowEvent::Resized(..), .. } => true, + _ => false + }; let flow = callback(event); if is_resize { - self.sync_with_thread(); + let (ref mutex, ref cvar) = *self.win32_block_loop; + let mut block_thread = mutex.lock().unwrap(); + *block_thread = false; + cvar.notify_all(); } match flow { ControlFlow::Continue => continue, @@ -195,13 +207,6 @@ impl EventsLoop { } } - fn sync_with_thread(&self) { - let (ref mutex, ref cvar) = *self.win32_block_loop; - let mut block_thread = mutex.lock().unwrap(); - *block_thread = false; - cvar.notify_all(); - } - /// Executes a function in the background thread. /// /// Note that we use a FnMut instead of a FnOnce because we're too lazy to create an equivalent @@ -286,13 +291,6 @@ struct ThreadLocalData { win32_block_loop: Arc<(Mutex, Condvar)> } -fn event_is_resize(event: &Event) -> bool { - match *event { - Event::WindowEvent{ event: WindowEvent::Resized(..), .. } => true, - _ => false - } -} - // Utility function that dispatches an event on the current thread. fn send_event(event: Event) { CONTEXT_STASH.with(|context_stash| { @@ -335,10 +333,6 @@ pub unsafe extern "system" fn callback(window: winapi::HWND, msg: winapi::UINT, use events::WindowEvent::Resized; let w = winapi::LOWORD(lparam as winapi::DWORD) as u32; let h = winapi::HIWORD(lparam as winapi::DWORD) as u32; - send_event(Event::WindowEvent { - window_id: SuperWindowId(WindowId(window)), - event: Resized(w, h), - }); // Wait for the parent thread to process the resize event before returning from the // callback. @@ -346,6 +340,11 @@ pub unsafe extern "system" fn callback(window: winapi::HWND, msg: winapi::UINT, let mut context_stash = context_stash.borrow_mut(); let cstash = context_stash.as_mut().unwrap(); + let event = Event::WindowEvent { + window_id: SuperWindowId(WindowId(window)), + event: Resized(w, h), + }; + // If this window has been inserted into the window map, the resize event happened // during the event loop. If it hasn't, the event happened on window creation and // should be ignored. @@ -353,9 +352,16 @@ pub unsafe extern "system" fn callback(window: winapi::HWND, msg: winapi::UINT, let (ref mutex, ref cvar) = *cstash.win32_block_loop; let mut block_thread = mutex.lock().unwrap(); *block_thread = true; + + // The event needs to be sent after the lock to ensure that `notify_all` is + // called after `wait`. + cstash.sender.send(event).ok(); + while *block_thread { block_thread = cvar.wait(block_thread).unwrap(); } + } else { + cstash.sender.send(event).ok(); } }); 0