Windows: Position fixes (rust-windowing#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

@@ -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.

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.

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;

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::<winuser::WINDOWPLACEMENT>() 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