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

[WIP] Make a tap on a windows title bar focus it #5265

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ArenM
Copy link

@ArenM ArenM commented Apr 27, 2020

This is my first attempt, which is basically a simplified copy of the
mouse down even handling.
this should fix #4182

ArenM added 2 commits April 27, 2020 14:27
This is my first attempt, which is basically a simplified copy of the
mouse down even handling
@ArenM ArenM force-pushed the touchscreen-window-switching branch from ccec906 to daad8c9 Compare April 27, 2020 18:28
Copy link
Member

@Xyene Xyene left a comment

Choose a reason for hiding this comment

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

I'd double-check that #5229 doesn't accomplish the same thing, in a different way. This PR adds a seatop for touch down, but I suspect applications that don't bind touch won't receive any event. #5229 adds pointer simulation to surfaces that don't bind touch, which by extension includes titlebars.

sway_log(SWAY_ERROR, "Touch Event on Titlebar");
node = seat_get_focus_inactive(seat, &cont->node);
seat_set_focus(seat, node);
return;
Copy link
Member

Choose a reason for hiding this comment

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

Don't need this return, since this is the end of the function anyway.

bool on_titlebar = cont && !on_border && !surface;

if (on_titlebar) {
sway_log(SWAY_ERROR, "Touch Event on Titlebar");
Copy link
Member

Choose a reason for hiding this comment

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

This isn't an error :)

@@ -17,6 +17,7 @@ struct sway_seatop_impl {
enum wlr_button_state state);
void (*motion)(struct sway_seat *seat, uint32_t time_msec,
double dx, double dy);
void (*touch_down)(struct sway_seat *seat, uint32_t time_msec);
Copy link
Member

Choose a reason for hiding this comment

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

Just in terms of placement, this is not a good line to put this on. button/motion/axis all refer to pointers, so having a touch handler in the middle is a bit weird. Same with the decl of the prototype, and .touch_down = handle_touch_down,.

@ArenM
Copy link
Author

ArenM commented Apr 29, 2020

You're right, #5229 does have the the same effect.

Does it make sense to reuse this code to get moving windows, to work with that, using mod + dragging a window, for example?

@Xyene
Copy link
Member

Xyene commented Apr 29, 2020

I suspect that #5229 enables mod + dragging a window that doesn't bind touch (like a Wayland-native terminal, e.g. alacritty or kitty), but not one that does (e.g. I expect gedit). This would be good to confirm.

If that's the case, then adding a seatop handler for touch should allow for that case too. That's roughly the stage tablet support is at currently. The motion handler, for instance, can be shimmed for seatops that don't care about the distinction between pointer and touch motion.

@David96
Copy link
Contributor

David96 commented May 2, 2020

@Xyene yes it does, the position is not updated on motion though, only after lifting the finger, I have to check that out. Edit: fixed

Alacritty btw does seem to bind to touch, but not use it (https://github.com/alacritty/alacritty/blob/33abfe34a86863958e70a6b5109eab5740a6bc81/alacritty/src/event.rs#L619, I think it's simply part of the library they are using) - kitty doesn't though, that's what I'm using to test the cursor simulation stuff.

@emersion
Copy link
Member

emersion commented May 2, 2020

Alacritty btw does seem to bind to touch, but not use it

We can't do anything about these clients. Would you mind opening a winit issue to let them know about this issue?

@Xyene
Copy link
Member

Xyene commented May 2, 2020

Alacritty btw does seem to bind to touch, but not use it

That's good to know, I made a false assumption here that because it didn't bind tablet it would also not be binding touch, but that's not the case.

@David96
Copy link
Contributor

David96 commented May 2, 2020

We can't do anything about these clients. Would you mind opening a winit issue to let them know about this issue?

Done: rust-windowing/winit#1555

@emersion emersion marked this pull request as draft May 25, 2020 09:18
@emersion emersion added the enhancement New feature or incremental improvement label Dec 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or incremental improvement
Development

Successfully merging this pull request may close these issues.

basic touch support for container tabs
4 participants