From 8f47fdbe677b0b5fc4229e968938de05dd984fde Mon Sep 17 00:00:00 2001 From: Francesca Frangipane Date: Thu, 26 Apr 2018 20:09:33 -0400 Subject: [PATCH] Windows: Position fixes (#479) * Remove executable flag from os/macos.rs This was causing me some grief while working on Windows, and it doesn't belong here to begin with. * Windows: get_position returns screen coordinates instead of workspace coordinates Previously, get_position used GetWindowPlacement. As per the documentation of WINDOWSTRUCT, the returned coordinates are in workspace space, meaning they're relative to the taskbar. It's also explicitly remarked that these coordinates should only be used in conjunction with SetWindowPlacement, as mixing them with functions expecting screen coordinates can cause unpleasantness. Since our set_position (correctly) uses SetWindowPos, this meant that passing the return of get_position to set_position would cause the window to move. We now use GetWindowRect, which returns screen coordinates. This gives us both better consistency within the Windows backend and across platforms. Note that this only makes a difference if the taskbar is visible. With the taskbar hidden, the values are exactly the same as before. * Windows: Moved event position values are consistent with get_position The old Moved values had two problems: * They were obtained by casting a WORD (u16) straight to an i32. This meant wrap-around would never be interpreted as negative, thus negative positions (which are ubiquitous when using multiple monitors) would result in positions around u16::MAX. * WM_MOVE supplies client area positions, not window positions. Switching to handling WM_WINDOWPOSCHANGED solves both of these problems. * Better documentation for Moved and Resized --- CHANGELOG.md | 3 +++ src/events.rs | 4 ++-- src/os/macos.rs | 0 src/platform/windows/events_loop.rs | 27 ++++++++++++++++----------- src/platform/windows/window.rs | 20 ++++++++------------ 5 files changed, 29 insertions(+), 25 deletions(-) mode change 100755 => 100644 src/os/macos.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index b0b43fef86..279746893c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Unreleased +- Corrected `get_position` on Windows to be relative to the screen rather than to the taskbar. +- Corrected `Moved` event on Windows to use position values equivalent to those returned by `get_position`. It previously supplied client area positions instead of window positions, and would additionally interpret negative values as being very large (around `u16::MAX`). + # Version 0.13.1 (2018-04-26) - Ensure necessary `x11-dl` version is used. diff --git a/src/events.rs b/src/events.rs index 1faaadf681..2dcc3eada1 100644 --- a/src/events.rs +++ b/src/events.rs @@ -24,10 +24,10 @@ pub enum Event { #[derive(Clone, Debug)] pub enum WindowEvent { - /// The size of the window has changed. + /// The size of the window has changed. Contains the client area's new dimensions. Resized(u32, u32), - /// The position of the window has changed. + /// The position of the window has changed. Contains the window's new position. Moved(i32, i32), /// The window has been requested to close. diff --git a/src/os/macos.rs b/src/os/macos.rs old mode 100755 new mode 100644 diff --git a/src/platform/windows/events_loop.rs b/src/platform/windows/events_loop.rs index 72b014d791..3ca51802ec 100644 --- a/src/platform/windows/events_loop.rs +++ b/src/platform/windows/events_loop.rs @@ -423,6 +423,22 @@ pub unsafe extern "system" fn callback(window: HWND, msg: UINT, winuser::DefWindowProcW(window, msg, wparam, lparam) }, + // WM_MOVE supplies client area positions, so we send Moved here instead. + winuser::WM_WINDOWPOSCHANGED => { + use events::WindowEvent::Moved; + + let windowpos = lparam as *const winuser::WINDOWPOS; + if (*windowpos).flags & winuser::SWP_NOMOVE != winuser::SWP_NOMOVE { + send_event(Event::WindowEvent { + window_id: SuperWindowId(WindowId(window)), + event: Moved((*windowpos).x, (*windowpos).y), + }); + } + + // This is necessary for us to still get sent WM_SIZE. + winuser::DefWindowProcW(window, msg, wparam, lparam) + }, + winuser::WM_SIZE => { use events::WindowEvent::Resized; let w = LOWORD(lparam as DWORD) as u32; @@ -461,17 +477,6 @@ pub unsafe extern "system" fn callback(window: HWND, msg: UINT, 0 }, - winuser::WM_MOVE => { - use events::WindowEvent::Moved; - let x = LOWORD(lparam as DWORD) as i32; - let y = HIWORD(lparam as DWORD) as i32; - send_event(Event::WindowEvent { - window_id: SuperWindowId(WindowId(window)), - event: Moved(x, y), - }); - 0 - }, - winuser::WM_CHAR => { use std::mem; use events::WindowEvent::ReceivedCharacter; diff --git a/src/platform/windows/window.rs b/src/platform/windows/window.rs index 2301149765..e0a37a0ce1 100644 --- a/src/platform/windows/window.rs +++ b/src/platform/windows/window.rs @@ -116,24 +116,20 @@ impl Window { /// See the docs in the crate root file. pub fn get_position(&self) -> Option<(i32, i32)> { - use std::mem; - - let mut placement: winuser::WINDOWPLACEMENT = unsafe { mem::zeroed() }; - placement.length = mem::size_of::() as UINT; + let mut rect: RECT = unsafe { mem::uninitialized() }; - if unsafe { winuser::GetWindowPlacement(self.window.0, &mut placement) } == 0 { - return None + if unsafe { winuser::GetWindowRect(self.window.0, &mut rect) } != 0 { + Some((rect.left as i32, rect.top as i32)) + } else { + None } - - let ref rect = placement.rcNormalPosition; - Some((rect.left as i32, rect.top as i32)) } pub fn get_inner_position(&self) -> Option<(i32, i32)> { use std::mem; - let mut position: POINT = unsafe{ mem::zeroed() }; - if unsafe{ winuser::ClientToScreen(self.window.0, &mut position) } == 0 { + let mut position: POINT = unsafe { mem::zeroed() }; + if unsafe { winuser::ClientToScreen(self.window.0, &mut position) } == 0 { return None; } @@ -749,7 +745,7 @@ unsafe fn init(window: WindowAttributes, pl_attribs: PlatformSpecificWindowBuild winuser::RegisterTouchWindow( real_window.0, winuser::TWF_WANTPALM ); } } - + // Creating a mutex to track the current window state let window_state = Arc::new(Mutex::new(events_loop::WindowState { cursor: winuser::IDC_ARROW, // use arrow by default