Windows: CursorState improvements (#526)

* Windows: CursorState improvements

Fixes #523

Prior to changing the cursor state, we now check the current grab
state, since it can be invalidated by alt-tabbing and other things.

`CursorState::Hide` is also implemented now.

The cursor name is now wrapped in a `Cursor` struct to allow
multithreaded access.

`Window::set_cursor_state` has been reworked to use
`execute_in_thread`. Two unneeded `transmute` calls were also
removed.

The `WM_SETCURSOR` handler is much more readable now.

`MonitorId::get_adapter_name` has been removed, since it's dead
code and appears to be a relic from 4 years ago.

* Windows: CursorState::Grab no longer hides cursor

`MouseCursor::NoneCursor` has been implemented to allow for
equivalent behavior to the older implementation.

Windows and X11 now have consistent cursor grabbing behavior.
macOS still needs to be updated.

* Windows: Grabbing auto-hides again (for now)

This API needs more work, so let's stick to a bug fix and some
refactoring. However, it now hides using a different technique
than it did originally, which applies instantly instead of after
mouse movement.

CHANGELOG.md

@@ -17,6 +17,9 @@
 - Added the `Window::set_ime_spot(x: i32, y: i32)` method, which is implemented on X11 and macOS.
 - **Breaking**: `os::unix::WindowExt::send_xim_spot(x: i16, y: i16)` no longer exists. Switch to the new `Window::set_ime_spot(x: i32, y: i32)`, which has equivalent functionality.
 - Fixed detection of `Pause` and `Scroll` keys on Windows.
+- On Windows, alt-tabbing while the cursor is grabbed no longer makes it impossible to re-grab the window.
+- On Windows, using `CursorState::Hide` when the cursor is grabbed now ungrabs the cursor first.
+- Implemented `MouseCursor::NoneCursor` on Windows.
 
 # Version 0.14.0 (2018-05-09)
 

src/platform/windows/events_loop.rs

@@ -926,30 +926,24 @@ pub unsafe extern "system" fn callback(window: HWND, msg: UINT,
 
         winuser::WM_SETCURSOR => {
             let call_def_window_proc = CONTEXT_STASH.with(|context_stash| {
-                let cstash = context_stash.borrow();
-                let mut call_def_window_proc = false;
-                if let Some(cstash) = cstash.as_ref() {
-                    if let Some(w_stash) = cstash.windows.get(&window) {
-                        if let Ok(window_state) = w_stash.lock() {
-                            if window_state.mouse_in_window {
-                                match window_state.cursor_state {
-                                    CursorState::Normal => {
-                                        winuser::SetCursor(winuser::LoadCursorW(
-                                                ptr::null_mut(),
-                                                window_state.cursor));
-                                    },
-                                    CursorState::Grab | CursorState::Hide => {
-                                        winuser::SetCursor(ptr::null_mut());
-                                    }
-                                }
-                            } else {
-                                call_def_window_proc = true;
-                            }
+                context_stash
+                    .borrow()
+                    .as_ref()
+                    .and_then(|cstash| cstash.windows.get(&window))
+                    .map(|window_state_mutex| {
+                        let window_state = window_state_mutex.lock().unwrap();
+                        if window_state.mouse_in_window {
+                            let cursor = winuser::LoadCursorW(
+                                ptr::null_mut(),
+                                window_state.cursor.0,
+                            );
+                            winuser::SetCursor(cursor);
+                            false
+                        } else {
+                            true
                         }
-                    }
-                }
-
-                call_def_window_proc
+                    })
+                    .unwrap_or(true)
             });
 
             if call_def_window_proc {

src/platform/windows/mod.rs

@@ -16,8 +16,11 @@ pub struct PlatformSpecificWindowBuilderAttributes {
 unsafe impl Send for PlatformSpecificWindowBuilderAttributes {}
 unsafe impl Sync for PlatformSpecificWindowBuilderAttributes {}
 
-// TODO: document what this means
-pub type Cursor = *const winapi::ctypes::wchar_t;
+// Cursor name in UTF-16. Used to set cursor in `WM_SETCURSOR`.
+#[derive(Debug, Clone)]
+pub struct Cursor(pub *const winapi::ctypes::wchar_t);
+unsafe impl Send for Cursor {}
+unsafe impl Sync for Cursor {}
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct DeviceId(u32);

src/platform/windows/monitor.rs

@@ -152,13 +152,6 @@ impl MonitorId {
         self.dimensions
     }
 
-    /// This is a Win32-only function for `MonitorId` that returns the system name of the adapter
-    /// device.
-    #[inline]
-    pub fn get_adapter_name(&self) -> &[wchar_t] {
-        &self.adapter_name
-    }
-
     /// A window that is positioned at these coordinates will overlap the monitor.
     #[inline]
     pub fn get_position(&self) -> (i32, i32) {

src/platform/windows/util.rs

@@ -3,6 +3,7 @@ use std::ops::BitAnd;
 
 use winapi::ctypes::wchar_t;
 use winapi::shared::minwindef::DWORD;
+use winapi::shared::windef::RECT;
 use winapi::um::errhandlingapi::GetLastError;
 use winapi::um::winbase::{
     FormatMessageW,
@@ -32,6 +33,15 @@ pub fn wchar_to_string(wchar: &[wchar_t]) -> String {
         .to_string()
 }
 
+// This won't be needed anymore if we just add a derive to winapi.
+pub fn rect_eq(a: &RECT, b: &RECT) -> bool {
+    let left_eq = a.left == b.left;
+    let right_eq = a.right == b.right;
+    let top_eq = a.top == b.top;
+    let bottom_eq = a.bottom == b.bottom;
+    left_eq && right_eq && top_eq && bottom_eq
+}
+
 #[derive(Debug, Default, Clone, PartialEq, Eq)]
 pub struct WinError(Option<String>);
 

src/platform/windows/window.rs

@@ -8,9 +8,9 @@ use std::os::windows::ffi::OsStrExt;
 use std::sync::{Arc, Mutex};
 use std::sync::mpsc::channel;
 
-use winapi::shared::minwindef::{BOOL, DWORD, UINT};
-use winapi::shared::windef::{HDC, HWND, POINT, RECT};
-use winapi::um::{combaseapi, dwmapi, libloaderapi, processthreadsapi, winuser};
+use winapi::shared::minwindef::{BOOL, DWORD, FALSE, TRUE, UINT};
+use winapi::shared::windef::{HDC, HWND, LPPOINT, POINT, RECT};
+use winapi::um::{combaseapi, dwmapi, libloaderapi, winuser};
 use winapi::um::objbase::{COINIT_MULTITHREADED};
 use winapi::um::unknwnbase::{IUnknown, IUnknownVtbl};
 use winapi::um::winnt::{HRESULT, LONG, LPCWSTR};
@@ -22,10 +22,11 @@ use MonitorId as RootMonitorId;
 use MouseCursor;
 use WindowAttributes;
 
-use platform::platform::{EventsLoop, PlatformSpecificWindowBuilderAttributes, WindowId};
+use platform::platform::{Cursor, EventsLoop, PlatformSpecificWindowBuilderAttributes, WindowId};
 use platform::platform::events_loop::{self, DESTROY_MSG_ID};
 use platform::platform::icon::{self, IconType, WinIcon};
 use platform::platform::raw_input::register_all_mice_and_keyboards_for_raw_input;
+use platform::platform::util;
 
 /// The Win32 implementation of the main `Window` object.
 pub struct Window {
@@ -275,70 +276,108 @@ impl Window {
             MouseCursor::Wait => winuser::IDC_WAIT,
             MouseCursor::Progress => winuser::IDC_APPSTARTING,
             MouseCursor::Help => winuser::IDC_HELP,
+            MouseCursor::NoneCursor => ptr::null(),
             _ => winuser::IDC_ARROW, // use arrow for the missing cases.
         };
 
         let mut cur = self.window_state.lock().unwrap();
-        cur.cursor = cursor_id;
+        cur.cursor = Cursor(cursor_id);
     }
 
-    // TODO: it should be possible to rework this function by using the `execute_in_thread` method
-    // of the events loop.
-    pub fn set_cursor_state(&self, state: CursorState) -> Result<(), String> {
-        let mut current_state = self.window_state.lock().unwrap();
+    unsafe fn cursor_is_grabbed(&self) -> Result<bool, String> {
+        let mut client_rect: RECT = mem::uninitialized();
+        let mut clip_rect: RECT = mem::uninitialized();
+        if winuser::GetClientRect(self.window.0, &mut client_rect) == 0 {
+            return Err("`GetClientRect` failed".to_owned());
+        }
+        // A `POINT` is two `LONG`s (x, y), and the `RECT` field after `left` is `top`.
+        if winuser::ClientToScreen(self.window.0, &mut client_rect.left as *mut _ as LPPOINT) == 0 {
+            return Err("`ClientToScreen` (left, top) failed".to_owned());
+        }
+        if winuser::ClientToScreen(self.window.0, &mut client_rect.right as *mut _ as LPPOINT) == 0 {
+            return Err("`ClientToScreen` (right, bottom) failed".to_owned());
+        }
+        if winuser::GetClipCursor(&mut clip_rect) == 0 {
+            return Err("`GetClipCursor` failed".to_owned());
+        }
+        Ok(util::rect_eq(&client_rect, &clip_rect))
+    }
 
-        let foreground_thread_id = unsafe { winuser::GetWindowThreadProcessId(self.window.0, ptr::null_mut()) };
-        let current_thread_id = unsafe { processthreadsapi::GetCurrentThreadId() };
+    fn change_cursor_state(
+        window: &WindowWrapper,
+        current_state: CursorState,
+        state: CursorState,
+    ) -> Result<CursorState, String> {
+        match (current_state, state) {
+            (CursorState::Normal, CursorState::Normal)
+            | (CursorState::Hide, CursorState::Hide)
+            | (CursorState::Grab, CursorState::Grab) => (), // no-op
 
-        unsafe { winuser::AttachThreadInput(foreground_thread_id, current_thread_id, 1) };
-
-        let res = match (state, current_state.cursor_state) {
-            (CursorState::Normal, CursorState::Normal) => Ok(()),
-            (CursorState::Hide, CursorState::Hide) => Ok(()),
-            (CursorState::Grab, CursorState::Grab) => Ok(()),
-
-            (CursorState::Hide, CursorState::Normal) => {
-                current_state.cursor_state = CursorState::Hide;
-                Ok(())
+            (CursorState::Normal, CursorState::Hide) => unsafe {
+                winuser::ShowCursor(FALSE);
             },
 
-            (CursorState::Normal, CursorState::Hide) => {
-                current_state.cursor_state = CursorState::Normal;
-                Ok(())
-            },
-
-            (CursorState::Grab, CursorState::Normal) | (CursorState::Grab, CursorState::Hide) => {
-                unsafe {
-                    let mut rect = mem::uninitialized();
-                    if winuser::GetClientRect(self.window.0, &mut rect) == 0 {
-                        return Err(format!("GetWindowRect failed"));
-                    }
-                    winuser::ClientToScreen(self.window.0, mem::transmute(&mut rect.left));
-                    winuser::ClientToScreen(self.window.0, mem::transmute(&mut rect.right));
-                    if winuser::ClipCursor(&rect) == 0 {
-                        return Err(format!("ClipCursor failed"));
-                    }
-                    current_state.cursor_state = CursorState::Grab;
-                    Ok(())
+            (CursorState::Grab, CursorState::Hide) => unsafe {
+                if winuser::ClipCursor(ptr::null()) == 0 {
+                    return Err("`ClipCursor` failed".to_owned());
                 }
             },
 
-            (CursorState::Normal, CursorState::Grab) => {
-                unsafe {
-                    if winuser::ClipCursor(ptr::null()) == 0 {
-                        return Err(format!("ClipCursor failed"));
-                    }
-                    current_state.cursor_state = CursorState::Normal;
-                    Ok(())
+            (CursorState::Hide, CursorState::Normal) => unsafe {
+                winuser::ShowCursor(TRUE);
+            },
+
+            (CursorState::Normal, CursorState::Grab)
+            | (CursorState::Hide, CursorState::Grab) => unsafe {
+                let mut rect = mem::uninitialized();
+                if winuser::GetClientRect(window.0, &mut rect) == 0 {
+                    return Err("`GetClientRect` failed".to_owned());
+                }
+                if winuser::ClientToScreen(window.0, &mut rect.left as *mut _ as LPPOINT) == 0 {
+                    return Err("`ClientToScreen` (left, top) failed".to_owned());
+                }
+                if winuser::ClientToScreen(window.0, &mut rect.right as *mut _ as LPPOINT) == 0 {
+                    return Err("`ClientToScreen` (right, bottom) failed".to_owned());
+                }
+                if winuser::ClipCursor(&rect) == 0 {
+                    return Err("`ClipCursor` failed".to_owned());
+                }
+                if current_state != CursorState::Hide {
+                    winuser::ShowCursor(FALSE);
                 }
             },
 
-            _ => unimplemented!(),
+            (CursorState::Grab, CursorState::Normal) => unsafe {
+                if winuser::ClipCursor(ptr::null()) == 0 {
+                    return Err("`ClipCursor` failed".to_owned());
+                }
+                winuser::ShowCursor(TRUE);
+            },
         };
+        Ok(state)
+    }
 
-        unsafe { winuser::AttachThreadInput(foreground_thread_id, current_thread_id, 0) };
-
-        res
+    pub fn set_cursor_state(&self, state: CursorState) -> Result<(), String> {
+        let is_grabbed = unsafe { self.cursor_is_grabbed() }?;
+        let (tx, rx) = channel();
+        let window = self.window.clone();
+        let window_state = Arc::clone(&self.window_state);
+        self.events_loop_proxy.execute_in_thread(move |_| {
+            let mut window_state_lock = window_state.lock().unwrap();
+            // We should probably also check if the cursor is hidden,
+            // but `GetCursorInfo` isn't in winapi-rs yet, and it doesn't seem to matter as much.
+            let current_state = match window_state_lock.cursor_state {
+                CursorState::Normal if is_grabbed => CursorState::Grab,
+                CursorState::Grab if !is_grabbed => CursorState::Normal,
+                current_state => current_state,
+            };
+            let result = Self::change_cursor_state(&window, current_state, state)
+                .map(|_| {
+                    window_state_lock.cursor_state = state;
+                });
+            let _ = tx.send(result);
+        });
+        rx.recv().unwrap()
     }
 
     #[inline]
@@ -813,7 +852,7 @@ unsafe fn init(
 
     // 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
+        cursor: Cursor(winuser::IDC_ARROW), // use arrow by default
         cursor_state: CursorState::Normal,
         attributes: window,
         mouse_in_window: false,