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

Track mouse events outside the window #624

Closed
wants to merge 1 commit into from

Conversation

dcolascione
Copy link
Contributor

On mouse down, there's an "implicit grab" made by the window system
which ensures that mouse events are delivered to the window that
received the mouse-down even if the cursor strays outside the original
window before mouse-up. Kitty has been filtering out these
outside-window events and ignoring them, creating behavior that
differs from most other applications.

This change remembers the window bound to the mouse-down event and
uses it until mouse up, implementing more conventional moues behavior.

On mouse down, there's an "implicit grab" made by the window system
which ensures that mouse events are delivered to the window that
received the mouse-down even if the cursor strays outside the original
window before mouse-up. Kitty has been filtering out these
outside-window events and ignoring them, creating behavior that
differs from most other applications.

This change remembers the window bound to the mouse-down event and
uses it until mouse up, implementing more conventional moues behavior.
@maximbaz
Copy link
Contributor

maximbaz commented Jul 4, 2018

Just curious, what is the status on this PR?

@kovidgoyal
Copy link
Owner

It's waiting for the changes I indicated in my review. Apparently @dcolascione is busy at the moment.

@maximbaz
Copy link
Contributor

maximbaz commented Jul 4, 2018

Did you leave the comments somewhere else outside of this PR, or Github simply doesn't show the review to me? I don't see the review here, and I want to follow the discussion thread 🙂

Screenshot

image

@maximbaz
Copy link
Contributor

maximbaz commented Jul 4, 2018

That's the link to this PR, and I don't see any comments. I think you might have forgotten to click "Submit review", and until you do that, nobody except you sees the comments.

image

@@ -229,7 +233,9 @@ HANDLER(handle_move_event) {
call_boss(switch_focus_to, "I", window_idx);
}
}
if (!cell_for_pos(w, &x, &y, global_state.callback_os_window)) return;
if (!cell_for_pos(w, &x, &y, global_state.callback_os_window)) {
Copy link
Owner

Choose a reason for hiding this comment

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

This looks like an unnecessary change.

@@ -134,6 +134,9 @@ typedef struct {
OSWindow *os_windows;
size_t num_os_windows, capacity;
OSWindow *callback_os_window;
Window *drag_start_window;
bool drag_start_in_tab_bar;
int drag_start_window_idx;
Copy link
Owner

Choose a reason for hiding this comment

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

Tracking window pointers and window_idx is unsafe. Both can change at any time. Instead use the window id, which is guaranteed unique for the lifetime of kitty. As for window_idx, you should calculate it maually by iterating for the windows in the current tab, as window_for_event does anyway.

@kovidgoyal
Copy link
Owner

Wow, that's confusing :) I clicked submit review now should be fine.

@maximbaz
Copy link
Contributor

maximbaz commented Jul 4, 2018

Yup, thanks, now @dcolascione has some work to do 😛

@kovidgoyal kovidgoyal closed this in 77c4f5f Jul 6, 2018
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.

None yet

3 participants