Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle set_mouse_cursor event on windows #186

Merged
merged 2 commits into from
Apr 2, 2024

Conversation

Fredemus
Copy link
Contributor

@Fredemus Fredemus commented Apr 1, 2024

This adds the ability to change the cursor on Windows platforms. For some reason there are very few default cursors included in Windows so a lot of cursors available on other platforms aren't available yet, which is why many of the MouseCursor options just maps to IDC_ARROW. I'll look into adding custom cursors next.

LoadCursorW is supposedly superseded by LoadImageW but I couldn't find a way to do it with that one that doesn't crash.

Tested with Vizia's cursor icon example here and everything seems to work as it should.
Also tested by hovering over some buttons on a plugin.

@Fredemus Fredemus force-pushed the windows-set-cursor branch from 57dd7ec to a328b4c Compare April 1, 2024 11:06
@Fredemus Fredemus force-pushed the windows-set-cursor branch from a328b4c to 7b926c9 Compare April 1, 2024 22:53
src/win/window.rs Outdated Show resolved Hide resolved
MouseCursor::Help => IDC_HELP,

// hiding cursor can't be turned into an lpcwstr since it's done by ShowCursor instead of SetCursor
MouseCursor::Hidden => IDC_ARROW,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs for SetCursor state that passing NULL hides the cursor, so I think returning ptr::null_mut() here would be more straightforward than using ShowCursor.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh I missed that. I don't really get the point of ShowCursor then. Fixed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just an alternative API that works slightly differently. I think all of Windows, macOS, and X11 have both a "show/hide cursor" API and a way to set the cursor to an empty image. The latter is just a closer fit for Baseview's API, since it treats "hidden" as one of the possible cursor states rather than having separate methods for showing and hiding the cursor.

src/win/window.rs Outdated Show resolved Hide resolved
src/win/window.rs Outdated Show resolved Hide resolved
src/win/window.rs Outdated Show resolved Hide resolved
Copy link
Contributor Author

@Fredemus Fredemus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick review, I appreciate it! Newest commit should resolve all your comments

MouseCursor::Help => IDC_HELP,

// hiding cursor can't be turned into an lpcwstr since it's done by ShowCursor instead of SetCursor
MouseCursor::Hidden => IDC_ARROW,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh I missed that. I don't really get the point of ShowCursor then. Fixed

src/win/window.rs Outdated Show resolved Hide resolved
src/win/window.rs Outdated Show resolved Hide resolved
src/win/window.rs Outdated Show resolved Hide resolved
src/win/window.rs Outdated Show resolved Hide resolved
Copy link
Member

@micahrj micahrj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for making those changes!

@micahrj micahrj merged commit be3d72d into RustAudio:master Apr 2, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants