Skip to content

[Windows] Set the cursor icon when the cursor first enters a window#1807

Merged
msiglreith merged 1 commit intorust-windowing:masterfrom
maroider:windows-initial-cursor
Jan 5, 2021
Merged

[Windows] Set the cursor icon when the cursor first enters a window#1807
msiglreith merged 1 commit intorust-windowing:masterfrom
maroider:windows-initial-cursor

Conversation

@maroider
Copy link
Copy Markdown
Member

@maroider maroider commented Dec 23, 2020 •
edited
Loading

  • Tested on all platforms changed
  • Compilation warnings were addressed
  • cargo fmt has been run on this branch
  • cargo doc builds successfully
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

I'm not sure if this is the best way to address #1682 since I'm not familiar with how Windows handles cursors WRT windows, and I plan on looking further into this before converting from a draft PR to a normal PR.

Fixes #1682

@maroider maroider force-pushed the windows-initial-cursor branch from 4a900da to 0d7584e Compare December 23, 2020 19:39
@maroider
Copy link
Copy Markdown
Member Author

maroider commented Dec 23, 2020

It turns out that the first attempt at a fix made it so the "resize" cursor icons never appeared when hovering the cursor over the window borders.

@maroider maroider mentioned this pull request Dec 23, 2020
Closed
@maroider maroider force-pushed the windows-initial-cursor branch from 0d7584e to 2a0c869 Compare January 3, 2021 09:59
@maroider
Copy link
Copy Markdown
Member Author

maroider commented Jan 3, 2021 •
edited
Loading

It turns out that WM_SETCURSOR arrives with hit-test results in the low-order "word" of lParam, so we can use that instead of relying on other messages.

I'm mildly concerned about ordering guarantees here (if any such thing exists for this case), since users may want to set a cursor and have it applied immediately when the cursor enters the window. It may actually be fine in practice, but I don't know for sure.

@maroider maroider marked this pull request as ready for review January 3, 2021 10:02
@msiglreith msiglreith added DS - win32 Affects the Win32/Windows backend C - waiting on maintainer A maintainer must review this code labels Jan 4, 2021
@msiglreith msiglreith self-requested a review January 4, 2021 16:07
msiglreith
msiglreith suggested changes Jan 4, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@msiglreith msiglreith left a comment

Choose a reason for hiding this comment

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

Thanks!

src/platform_impl/windows/event_loop.rs Outdated
.cursor_flags()
.contains(CursorFlags::IN_WINDOW)
{
if (lparam & 0xFFFF) == winuser::HTCLIENT {
Copy link
Copy Markdown
Member

@msiglreith msiglreith Jan 4, 2021

Choose a reason for hiding this comment

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

(lparam & 0xFFFF) -> LOWORD(lparam)
Also, wrapping the condition into a separate let statement and/or short command regarding hit-testing here would be great

Copy link
Copy Markdown
Member Author

@maroider maroider Jan 5, 2021

Choose a reason for hiding this comment

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

command

I assume you meant to write "comment" here.

@maroider maroider force-pushed the windows-initial-cursor branch from 2a0c869 to 7f593f2 Compare January 5, 2021 11:04
@maroider
Copy link
Copy Markdown
Member Author

maroider commented Jan 5, 2021

Never mind that push. I trusted rust-analyzer a bit too much while I simultaneously did something rust-analyzer didn't support.

@maroider maroider force-pushed the windows-initial-cursor branch 2 times, most recently from e496138 to f88ab68 Compare January 5, 2021 12:14
@maroider maroider force-pushed the windows-initial-cursor branch from f88ab68 to cef0e01 Compare January 5, 2021 13:42
@msiglreith msiglreith merged commit 9d63fc7 into rust-windowing:master Jan 5, 2021
@maroider maroider mentioned this pull request Jan 27, 2021
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@msiglreith msiglreith msiglreith approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

C - waiting on maintainer A maintainer must review this code DS - win32 Affects the Win32/Windows backend

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Windows spinning cursor on startup

2 participants

@maroider @msiglreith