Adding the ability to change the mouse cursor for MacOS and Windows #128
Conversation
There was a problem hiding this comment.
Rebased here, tested on Windows/Wine and Linux (can also boot up a Mac if noone's tested that). Mostly LGTM functionality wise, though I have some comments on how this changes the current behavior on WIndows.
I would suggest using the same list of cursors and the same cursor assignments as winit though. And thus also dropping MouseCursor::Pointer again. The existing cursor enum was already based on this, so bringing everything back in line with winit's current definitions shouldn't be too difficult. The enum can be found here, and I linked the platform-specific cursor assignments in the comments below. This way it becomes easier to switch between winit and baseview backends for GUI frameworks that support both, as the cursors will look the same on both platforms.
| @@ -62,6 +62,7 @@ pub(super) fn get_xcursor(display: *mut x11::xlib::Display, cursor: MouseCursor) | |||
|
|
|||
| MouseCursor::Hand => loadn(&[b"hand2\0", b"hand1\0"]), | |||
| MouseCursor::HandGrabbing => loadn(&[b"closedhand\0", b"grabbing\0"]), | |||
| MouseCursor::Pointer => loadn(&[b"hand2\0"]), | |||
There was a problem hiding this comment.
This can be load() since there's only a single cursor name here. Won't make any difference in practice, but then it's consistent with the other calls here. And as mentioned above, there probably shouldn't be another MouseCursor::Pointer so the list can stay identical to winit's.
These are winit's x11 cursor assignments:
| match cursor { | ||
| MouseCursor::Default => Cursor::Native("arrowCursor"), | ||
| MouseCursor::Pointer => Cursor::Native("pointingHandCursor"), | ||
| MouseCursor::Hand => Cursor::Native("openHandCursor"), |
There was a problem hiding this comment.
Is MouseCursor::Pointer needed? It might be a good idea to use the exact same cursor assignments as winit, since this list is based on winit's: https://github.com/rust-windowing/winit/blob/bdcbd7d1f99464fe945596b43a922a9632456e44/src/platform_impl/macos/appkit/cursor.rs#L196
That way GUI frameworks that support both winit and baseview will look the same. Now they'd use different cursors for baseview and winit.
| use crate::MouseCursor; | ||
|
|
||
| impl MouseCursor { | ||
| pub(crate) fn to_windows_cursor(self) -> PCWSTR { |
There was a problem hiding this comment.
These should probably also match winit's cursor assignments: https://github.com/rust-windowing/winit/blob/bdcbd7d1f99464fe945596b43a922a9632456e44/src/platform_impl/windows/util.rs#L154
| @@ -350,7 +341,7 @@ unsafe fn register_wnd_class() -> ATOM { | |||
| cbClsExtra: 0, | |||
| cbWndExtra: 0, | |||
| hIcon: null_mut(), | |||
| hCursor: LoadCursorW(null_mut(), IDC_ARROW), | |||
| hCursor: null_mut(), // If the class cursor is not NULL, the system restores the class cursor each time the mouse is moved. | |||
There was a problem hiding this comment.
The downside to defaulting this to null_mut() is that this now breaks every application that doesn't set a custom mouse cursor on startup by hiding the cursor by default. This behavior is then also different between Windows, Linux, and macOS.
I'm not quite sure what the best way to tackle this is. One option is to store the last cursor set through Window::set_mouse_cursor() in WindowState, default this value to IDC_ARROW, and set the cursor to that value whenever the user enters the window. Another less hacky approach is to also change the window class's cursor using SetClassLong(hwnd, GCL_HCURSOR, cursor as i32);. Neither is particularly pretty, but the latter approach is at least pretty foolproof. If you want, you can use this patch for this: (for some reason GitHub doesn't let you upload .patch files, so I gzipped it)
0001-Revert-to-IDC_ARROW-as-default-window-class-cursor.patch.gz
discordance
force-pushed
the
feature/set-mouse-cursor
branch
from
6f9ad7f
to
5744a2b
Compare
The feature was already there for X11, but wasn't available in MacOS and Windows.
The code is based on
winitcode mostly.I've tested locally with a modified version of
iced_baseviewthat I am working on for a project.Not sure if adding a
set_mouse_cursorto the window is the way to go but it works for me.This is related to the #123 issue.