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

input/cursor: correctly transfer focus when using tablet pen #5108

Merged
merged 1 commit into from
Apr 24, 2020

Conversation

Xyene
Copy link
Member

@Xyene Xyene commented Mar 14, 2020

Tentatively fixes #4819.

This commit ensures that seatop_motion is called to transfer focus when a window is selected via a pen. Previously, it would race with node_at_coords, and only properly transfer focus if its returned surface was NULL.

I've tested this locally, and it fixes the issues I was experiencing.

Calling seatop_motion directly here works as well, but my impression is that the confine handling inside cursor_motion should be done for tablet devices too. Applications see the following sequence of events:

[912231.664] wl_pointer@3.motion(7987491, 281.539062, 991.007812)
[912231.673] zwp_tablet_tool_v2@4278190083.motion(281.539062, 991.007812)
[912231.676] zwp_tablet_tool_v2@4278190083.frame(7987491)

In a GNOME session on the same application, the wl_pointer event is not present. I don't know enough to say which is the correct behaviour, or if it even matters. Sway infrequently omits the wl_pointer event, too.

@emersion
Copy link
Member

Calling seatop_motion directly here works as well, but my impression is that the confine handling inside cursor_motion should be done for tablet devices too.

Yes. The wl_pointer.motion event is incorrect.

@Xyene
Copy link
Member Author

Xyene commented Mar 18, 2020

Thanks! So, to confirm:

  • We don't want wlr_relative_pointer_manager_v1_send_relative_motion to be called for tablet events.
  • We do want wlr_region_confine to be called.
  • We don't want wlr_cursor_move to be called. (Is this what triggers wl_pointer.motion?)

In that case, a follow-up question is: is

sway/sway/input/cursor.c

Lines 481 to 484 in fcd524b

if (!surface || !wlr_surface_accepts_tablet_v2(tablet->tablet_v2, surface)) {
wlr_tablet_v2_tablet_tool_notify_proximity_out(sway_tool->tablet_v2_tool);
cursor_motion(cursor, time_msec, input_device->wlr_device, dx, dy, dx, dy);
return;
correct? The branch is taken even if the surface supports tablet v2, if the surface returned by node_at_coords is NULL, and ends up sending wl_pointer.motion. That branch is how tablet focus seems to work right now (even if erratically).

@emersion
Copy link
Member

and ends up sending wl_pointer.motion

Just like touch, we should use seat_set_focus to transfer focus.

@Xyene Xyene force-pushed the fix-4819 branch 2 times, most recently from ba8d6a3 to 3588b1a Compare April 16, 2020 00:24
@Xyene
Copy link
Member Author

Xyene commented Apr 16, 2020

I've updated this PR to call seat_set_focus. I also added a check for focus_follows_mouse, which while not semantically correct (perhaps a new option should be better), matches i3 behavior.

Fixes swaywm#4819.

This commit ensures that `seat_set_focus` is called to transfer focus
when a window is selected via a pen. Previously, it would race with
`node_at_coords`, and only properly transfer focus if its returned
`surface` was NULL.
Copy link
Member

@emersion emersion left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@emersion emersion merged commit d772471 into swaywm:master Apr 24, 2020
@Xyene Xyene deleted the fix-4819 branch October 18, 2020 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Focus does not follow mouse when using tablet (among other misbehaviors)
2 participants