From 8f394f117b2b13715a8f0c06e5963336bf138e05 Mon Sep 17 00:00:00 2001 From: aloucks Date: Tue, 19 Jun 2018 10:30:15 -0400 Subject: [PATCH] Change set_cursor_position to return Result<(), String> (#562) * Change set_cursor_position to return Result<(), String> This is now consistent with `grab_cursor`, and enables `window.set_cursor_position(x, y)?` in functions that return `Result<_, Box>`. * Adjust error handling of unimplemented cusor opertions in wayland * The final nitpick * Actually one more --- CHANGELOG.md | 1 + src/platform/android/mod.rs | 5 ++--- src/platform/emscripten/mod.rs | 4 ++-- src/platform/ios/mod.rs | 5 ++--- src/platform/linux/mod.rs | 6 +++--- src/platform/linux/wayland/window.rs | 25 +++++++++++++++++-------- src/platform/linux/x11/window.rs | 6 +++--- src/platform/macos/window.rs | 9 +++++---- src/platform/windows/window.rs | 8 ++++---- src/window.rs | 2 +- 10 files changed, 40 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae7f4993..c6bca654 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - Fixed quirk on macOS where certain keys would generate characters at twice the normal rate when held down. - On X11, all event loops now share the same `XConnection`. - **Breaking:** `Window::set_cursor_state` and `CursorState` enum removed in favor of the more composable `Window::grab_cursor` and `Window::hide_cursor`. As a result, grabbing the cursor no longer automatically hides it; you must call both methods to retain the old behavior on Windows and macOS. `Cursor::NoneCursor` has been removed, as it's no longer useful. +- **Breaking:** `Window::set_cursor_position` now returns `Result<(), String>`, thus allowing for `Box` conversion via `?`. # Version 0.15.1 (2018-06-13) diff --git a/src/platform/android/mod.rs b/src/platform/android/mod.rs index 5b140276..165c5459 100644 --- a/src/platform/android/mod.rs +++ b/src/platform/android/mod.rs @@ -346,9 +346,8 @@ impl Window { } #[inline] - pub fn set_cursor_position(&self, _position: LogicalPosition) -> Result<(), ()> { - // N/A - Ok(()) + pub fn set_cursor_position(&self, _position: LogicalPosition) -> Result<(), String> { + Err("Setting cursor position is not possible on Android.".to_owned()) } #[inline] diff --git a/src/platform/emscripten/mod.rs b/src/platform/emscripten/mod.rs index f68df623..392e22cf 100644 --- a/src/platform/emscripten/mod.rs +++ b/src/platform/emscripten/mod.rs @@ -559,8 +559,8 @@ impl Window { } #[inline] - pub fn set_cursor_position(&self, _position: LogicalPosition) -> Result<(), ()> { - Err(()) + pub fn set_cursor_position(&self, _position: LogicalPosition) -> Result<(), String> { + Err("Setting cursor position is not possible on Emscripten.".to_owned()) } #[inline] diff --git a/src/platform/ios/mod.rs b/src/platform/ios/mod.rs index 10e6ddef..fab6e4f0 100644 --- a/src/platform/ios/mod.rs +++ b/src/platform/ios/mod.rs @@ -408,9 +408,8 @@ impl Window { } #[inline] - pub fn set_cursor_position(&self, _position: LogicalPosition) -> Result<(), ()> { - // N/A - Ok(()) + pub fn set_cursor_position(&self, _position: LogicalPosition) -> Result<(), String> { + Err("Setting cursor position is not possible on iOS.".to_owned()) } #[inline] diff --git a/src/platform/linux/mod.rs b/src/platform/linux/mod.rs index c5e7cac8..9259db92 100644 --- a/src/platform/linux/mod.rs +++ b/src/platform/linux/mod.rs @@ -251,7 +251,7 @@ impl Window { pub fn grab_cursor(&self, grab: bool) -> Result<(), String> { match self { &Window::X(ref window) => window.grab_cursor(grab), - &Window::Wayland(ref _window) => Err("Cursor grabbing is not yet possible on Wayland.".to_owned()), + &Window::Wayland(ref window) => window.grab_cursor(grab), } } @@ -259,7 +259,7 @@ impl Window { pub fn hide_cursor(&self, hide: bool) { match self { &Window::X(ref window) => window.hide_cursor(hide), - &Window::Wayland(ref _window) => unimplemented!(), + &Window::Wayland(ref window) => window.hide_cursor(hide), } } @@ -272,7 +272,7 @@ impl Window { } #[inline] - pub fn set_cursor_position(&self, position: LogicalPosition) -> Result<(), ()> { + pub fn set_cursor_position(&self, position: LogicalPosition) -> Result<(), String> { match self { &Window::X(ref w) => w.set_cursor_position(position), &Window::Wayland(ref w) => w.set_cursor_position(position), diff --git a/src/platform/linux/wayland/window.rs b/src/platform/linux/wayland/window.rs index 085c336d..6a774612 100644 --- a/src/platform/linux/wayland/window.rs +++ b/src/platform/linux/wayland/window.rs @@ -235,11 +235,6 @@ impl Window { self.frame.lock().unwrap().set_resizable(resizable); } - #[inline] - pub fn set_cursor(&self, _cursor: MouseCursor) { - // TODO - } - #[inline] pub fn hidpi_factor(&self) -> i32 { self.monitors.lock().unwrap().compute_hidpi_factor() @@ -273,9 +268,23 @@ impl Window { } #[inline] - pub fn set_cursor_position(&self, _pos: LogicalPosition) -> Result<(), ()> { - // TODO: not yet possible on wayland - Err(()) + pub fn set_cursor(&self, _cursor: MouseCursor) { + // TODO + } + + #[inline] + pub fn hide_cursor(&self, _hide: bool) { + // TODO: This isn't possible on Wayland yet + } + + #[inline] + pub fn grab_cursor(&self, _grab: bool) -> Result<(), String> { + Err("Cursor grabbing is not yet possible on Wayland.".to_owned()) + } + + #[inline] + pub fn set_cursor_position(&self, _pos: LogicalPosition) -> Result<(), String> { + Err("Setting the cursor position is not yet possible on Wayland.".to_owned()) } pub fn get_display(&self) -> &Display { diff --git a/src/platform/linux/x11/window.rs b/src/platform/linux/x11/window.rs index 1f2c9b3d..dd78af21 100644 --- a/src/platform/linux/x11/window.rs +++ b/src/platform/linux/x11/window.rs @@ -1113,7 +1113,7 @@ impl UnownedWindow { self.get_current_monitor().hidpi_factor } - pub(crate) fn set_cursor_position_physical(&self, x: i32, y: i32) -> Result<(), ()> { + pub fn set_cursor_position_physical(&self, x: i32, y: i32) -> Result<(), String> { unsafe { (self.xconn.xlib.XWarpPointer)( self.xconn.display, @@ -1126,12 +1126,12 @@ impl UnownedWindow { x, y, ); - self.xconn.flush_requests().map_err(|_| ()) + self.xconn.flush_requests().map_err(|e| format!("`XWarpPointer` failed: {:?}", e)) } } #[inline] - pub fn set_cursor_position(&self, logical_position: LogicalPosition) -> Result<(), ()> { + pub fn set_cursor_position(&self, logical_position: LogicalPosition) -> Result<(), String> { let (x, y) = logical_position.to_physical(self.get_hidpi_factor()).into(); self.set_cursor_position_physical(x, y) } diff --git a/src/platform/macos/window.rs b/src/platform/macos/window.rs index d41680b7..d025467e 100644 --- a/src/platform/macos/window.rs +++ b/src/platform/macos/window.rs @@ -1025,17 +1025,18 @@ impl Window2 { } #[inline] - pub fn set_cursor_position(&self, cursor_position: LogicalPosition) -> Result<(), ()> { + pub fn set_cursor_position(&self, cursor_position: LogicalPosition) -> Result<(), String> { let window_position = self.get_inner_position() - .expect("`get_inner_position` failed"); + .ok_or("`get_inner_position` failed".to_owned())?; let point = appkit::CGPoint { x: (cursor_position.x + window_position.x) as CGFloat, y: (cursor_position.y + window_position.y) as CGFloat, }; CGDisplay::warp_mouse_cursor_position(point) - .expect("`CGWarpMouseCursorPosition` failed"); + .map_err(|e| format!("`CGWarpMouseCursorPosition` failed: {:?}", e))?; CGDisplay::associate_mouse_and_mouse_cursor_position(true) - .expect("`CGAssociateMouseAndMouseCursorPosition` failed"); + .map_err(|e| format!("`CGAssociateMouseAndMouseCursorPosition` failed: {:?}", e))?; + Ok(()) } diff --git a/src/platform/windows/window.rs b/src/platform/windows/window.rs index 06d0701b..385fbe6d 100644 --- a/src/platform/windows/window.rs +++ b/src/platform/windows/window.rs @@ -438,21 +438,21 @@ impl Window { get_window_scale_factor(self.window.0, self.window.1) } - fn set_cursor_position_physical(&self, x: i32, y: i32) -> Result<(), ()> { + fn set_cursor_position_physical(&self, x: i32, y: i32) -> Result<(), String> { let mut point = POINT { x, y }; unsafe { if winuser::ClientToScreen(self.window.0, &mut point) == 0 { - return Err(()); + return Err("`ClientToScreen` failed".to_owned()); } if winuser::SetCursorPos(point.x, point.y) == 0 { - return Err(()); + return Err("`SetCursorPos` failed".to_owned()); } } Ok(()) } #[inline] - pub fn set_cursor_position(&self, logical_position: LogicalPosition) -> Result<(), ()> { + pub fn set_cursor_position(&self, logical_position: LogicalPosition) -> Result<(), String> { let dpi_factor = self.get_hidpi_factor(); let (x, y) = logical_position.to_physical(dpi_factor).into(); self.set_cursor_position_physical(x, y) diff --git a/src/window.rs b/src/window.rs index 4ad34af6..86f7be17 100644 --- a/src/window.rs +++ b/src/window.rs @@ -322,7 +322,7 @@ impl Window { /// Changes the position of the cursor in window coordinates. #[inline] - pub fn set_cursor_position(&self, position: LogicalPosition) -> Result<(), ()> { + pub fn set_cursor_position(&self, position: LogicalPosition) -> Result<(), String> { self.window.set_cursor_position(position) }