Fixed CursorEntered and CursorLeft events not getting sent when holding a mouse button down
#3154
Conversation
| 'o: { | ||
| let mut w = userdata.window_state_lock(); | ||
| let mouse_was_outside_window = | ||
| !w.mouse.cursor_flags().contains(CursorFlags::IN_WINDOW); | ||
|
|
||
| // Calling TrackMouseEvent in order to receive mouse leave events. | ||
| unsafe { | ||
| TrackMouseEvent(&mut TRACKMOUSEEVENT { | ||
| cbSize: mem::size_of::<TRACKMOUSEEVENT>() as u32, | ||
| dwFlags: TME_LEAVE, | ||
| hwndTrack: window, | ||
| dwHoverTime: HOVER_DEFAULT, | ||
| }) | ||
| let rect: RECT = unsafe { | ||
| let mut rect: RECT = mem::zeroed(); | ||
| if GetClientRect(window, &mut rect) == false.into() { | ||
| break 'o; // exit early if GetClientRect failed | ||
| } | ||
| rect | ||
| }; | ||
|
|
||
| let x = (rect.left..rect.right).contains(&x); | ||
| let y = (rect.top..rect.bottom).contains(&y); | ||
|
|
||
| if mouse_was_outside_window && x && y { | ||
| w.mouse | ||
| .set_cursor_flags(window, |f| f.set(CursorFlags::IN_WINDOW, true)) | ||
| .ok(); | ||
|
|
||
| userdata.send_event(Event::WindowEvent { | ||
| window_id: RootWindowId(WindowId(window)), | ||
| event: CursorEntered { | ||
| device_id: DEVICE_ID, | ||
| }, | ||
| }); | ||
|
|
||
| // Calling TrackMouseEvent in order to receive mouse leave events. | ||
| unsafe { | ||
| TrackMouseEvent(&mut TRACKMOUSEEVENT { | ||
| cbSize: mem::size_of::<TRACKMOUSEEVENT>() as u32, | ||
| dwFlags: TME_LEAVE, | ||
| hwndTrack: window, | ||
| dwHoverTime: HOVER_DEFAULT, | ||
| }) | ||
| }; | ||
| } else if !(mouse_was_outside_window || x && y) { | ||
| w.mouse | ||
| .set_cursor_flags(window, |f| f.set(CursorFlags::IN_WINDOW, false)) | ||
| .ok(); | ||
|
|
||
| userdata.send_event(Event::WindowEvent { | ||
| window_id: RootWindowId(WindowId(window)), | ||
| event: CursorLeft { | ||
| device_id: DEVICE_ID, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Could you extract all of this mess into a function accepting: window, x, y, position and returning something like PointerMoveKind { Move, Enter, Leave } so you can then do something like that
match get_pointer_move_kind(window, position, x, y) {
PointerMoveKind::Leave => {
// Cursor left handling
}
PointerMoveKind::Enter => {
// Cursor enter handling
}
PointerMoveKind::Move => (),
}There was a problem hiding this comment.
yeah I can do that, although I believe the Move should always happen, so there shouldn't be a separate case for it, maybe instead of creating a new enum with two variant (Leave, Enter) a Option<bool> would be enough?
There was a problem hiding this comment.
Option<bool> is 3 variants as well, it's just not clear what it means, you can define Move as something else.
There was a problem hiding this comment.
Option<bool>is 3 variants as well, it's just not clear what it means, you can defineMoveas something else.
Ok yeah I understand, I meant in a way that None mean there is no need to do anything and Some(..) we have to do something, but yeah having an enum can make the code more organized so I went by that, I applied your suggestions.
| None, | ||
| } | ||
|
|
||
| unsafe fn get_pointer_move_kind( |
There was a problem hiding this comment.
But the function itself is safe. Also move it to the bottom of the file with the PointerMoveKind thing.
There was a problem hiding this comment.
But the function itself is safe. Also move it to the bottom of the file with the
PointerMoveKindthing.
well we do have some unsafe calls inside it, that's why I marked it as unsafe although Im pretty sure none of these unsafe calls are failable so I removed the unsafe.
There was a problem hiding this comment.
unsafe functions in rust serve a different purpose usually, for example, when you need to hold some lifetime, etc invariant when you call a function, which usually is tracked by rust.
In this case though, we know that it as a whole is safe, since the only call uses HWND is error checked.
There was a problem hiding this comment.
oh ok, thanks for information.
Done |
This PR fix the problem mentioned in #3153
CHANGELOG.mdif knowledge of this change could be valuable to users