#29: adding touch support #50
Conversation
include/slurp.h
Outdated
| // touch: | ||
| struct wl_touch *wl_touch; | ||
| bool already_touched; | ||
| struct slurp_output *current_touch_output; |
There was a problem hiding this comment.
Do we really need this field? Can't we just use current_output?
There was a problem hiding this comment.
Yes but this one is a bit trickier.
Initially I wanted to use current_output but it was null when entering touch_handle_motion. I've investigated further and found out that
- pointer_leave event is triggered whenever touch down happens (even if mouse is not moved).
- handle_pointer_leave clears the current_output.
I think the solution is either
- checking if has_selection is true and then leaving current_output be
- leaving pointer_handle_leave empty since clearing current_output is the only thing the method does (until multi-monitor funcitonality is added).
Which one is better?
There was a problem hiding this comment.
pointer_leave event is triggered whenever touch down happens (even if mouse is not moved).
That sounds like a Sway bug.
There was a problem hiding this comment.
That sounds like a Sway bug.
Confirmed as this doesn't happen in wayfire. Opened an issue swaywm/sway#5145 for this one.
There was a problem hiding this comment.
The "real" solution would be to handle pointer input and touch input completely separately. Basically put the currently shared state in a struct and have two separate fields with this struct (one for pointer, one for touch). It would be a bigger change though.
There was a problem hiding this comment.
Just to be sure I got it right:
//selection (pointer/touch):
struct {
struct slurp_output *current_output;
struct slurp_output *current_touch_output;
int32_t x, y;
int32_t anchor_x, anchor_y;
struct slurp_box selection;
bool has_selection;
} selection;
Something like this?
There was a problem hiding this comment.
The idea is to track the position of the pointer and touch points independently. So we'd have:
struct slurp_selection {
struct slurp_output *current_output;
int32_t x, y;
int32_t anchor_x, anchor_y;
struct slurp_box box;
bool valid;
};
struct slurp_seat {
…
struct slurp_selection pointer;
struct slurp_selection touch;
};However, I'm fine with keeping this for a future pull request.
Now that the Sway patch has been merged, we can get rid of current_touch_output.
There was a problem hiding this comment.
And done 4e127e3.
main.c
Outdated
| @@ -218,6 +231,54 @@ static const struct wl_keyboard_listener keyboard_listener = { | |||
| .modifiers = noop, | |||
| }; | |||
|
|
|||
| static void touch_handle_down(void *data, struct wl_touch *touch, uint32_t serial, | |||
| uint32_t time, struct wl_surface *surface, int32_t id, | |||
There was a problem hiding this comment.
Style: we don't align, we use two tabs instead (also applies to other functions).
main.c
Outdated
| } | ||
|
|
||
| static void touch_handle_cancel(void *data, struct wl_touch *touch) | ||
| { |
There was a problem hiding this comment.
Style: no newline between ) and {
main.c
Outdated
| uint32_t time, struct wl_surface *surface, int32_t id, | ||
| wl_fixed_t x, wl_fixed_t y) { | ||
| struct slurp_seat *seat = data; | ||
| if (!seat->already_touched) { |
There was a problem hiding this comment.
Instead of having a bool already_touched field, can we store the touch id and check it's the correct one in touch_handle_motion?
There was a problem hiding this comment.
Fixed 585646d. Turns out this also fixes segfaults when attempting to select with different fingers, thanks for this one.
include/slurp.h
Outdated
| @@ -82,8 +84,8 @@ struct slurp_seat { | |||
| int32_t anchor_x, anchor_y; | |||
| struct slurp_box selection; | |||
| bool has_selection; | |||
| bool already_touched; | |||
|
|
|||
| int32_t touch_id; | |||
There was a problem hiding this comment.
One last nit:" can we move this to the "touch" section below?
|
If you could rebase and squash everything into a single commit, that'd make it ready for being merged :) |
gg-rewrite
force-pushed
the
touch-selection
branch
from
ee4e2e7 to
67c65e5
Compare
|
Can we pick a better commit summary + message? Something like: |
This closes emersion#29
gg-rewrite
force-pushed
the
touch-selection
branch
from
67c65e5 to
3ba0645
Compare
|
Thanks! |
Selection by a single touch point, similar to the mouse.
Boilerplate code for touch handling is added.
Three pieces of code are moved to separate functions from handle_pointer* methods so as to be reused by the corresponding handle_touch* one.