-
Notifications
You must be signed in to change notification settings - Fork 943
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
macOS: Support click-dragging out of a window #1607
Conversation
I can confirm the fix is working on native macOS Catalina |
Not sure why the "CI / Tests (stable, x86_64-apple-ios, macos-latest) (pull_request)" job was auto-canceled. I don't have the access to restart it. upd: restarting the job helped, thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be worth investigating, unless you've done so already.
src/platform_impl/macos/view.rs
Outdated
@@ -77,6 +78,7 @@ pub fn new_view(ns_window: id) -> (IdRef, Weak<Mutex<CursorState>>) { | |||
is_key_down: false, | |||
modifiers: Default::default(), | |||
tracking_rect: None, | |||
mouse_buttons_down: 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mouse events provide the pressedMouseButtons
field. Have you considered using that instead of implementing tracking yourself?
@chrisduerr yeah, pressedMouseButtons seems to work fine, thanks. Updated the PR. |
src/platform_impl/macos/view.rs
Outdated
@@ -910,13 +910,17 @@ fn mouse_motion(this: &Object, event: id) { | |||
let view_point = view.convertPoint_fromView_(window_point, nil); | |||
let view_rect = NSView::frame(view); | |||
|
|||
let mouse_buttons_down: NSInteger = msg_send![class!(NSEvent), pressedMouseButtons]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why put this outside of the conditional? Seems like a waste to do this when you don't need to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can of course keep this outside and just add && mouse_button_down == 0
, which would reduce the level of nesting. Perhaps that was the original intention of putting it here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved the variable inside the if body.
It's possible to merge both conditions, but the result is (subjectively) harder to read because of all the brackets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since there's also no clean way to inline the msg_send! directly, meaning that this variation also has a slight performance advantage in the general case of sending events, I agree that it's the best choice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me and works without any issues.
@vbogaevsky seems to be the macOS backend maintainer, do you have any other feedback?
Otherwise, @kchibisov it's probably good to go.
* macos: Support click-dragging out of a window * macos: Use NSEvent::pressedMouseButtons for click-dragging * macos: Click-dragging: Move pressedMouseButtons inside
This is my naive attempt to fix #1599. It seems to work, though I've only checked on a remote macOS machine.
Fixes #1599 ("[macos] Click-dragging out of a window doesn't work on macOS")
Related to #292 ("Add mouse event capturing when click-dragging out of a win32 window")