[WIP] Implement touch gestures using libtouch #3428

grahnen · 2019-01-15T03:24:44Z

Implementing touch gesture recognition through libtouch, for now at https://github.com/grahnen/libtouch

I've begun doing what I think is correct, but I am not very well versed in the inner workings of sway, so feel free to correct me if I'm doing something wrong.

sway/input/cursor.c

grahnen · 2019-01-16T18:22:44Z

I am starting to come to the conclusion that I have no idea what I'm doing right now. Not sure I understand how the configuration parsing works, and I don't know how to extend it to support the syntax of #1904. I've done what i think looks correct-ish, but sway still says touch is an invalid command.

RedSoxFan · 2019-01-17T04:55:32Z

So implementing the syntax in #1904 is going to have some challenges with the current config parser. It predates the generic code blocks PR

I'd propose the following instead:

# Declare variables outside of touch to avoid having to deal with scoping
touch {
    gesture <identifier> {
        # Actions
        # I would also suggest changing it so a command comes first so for example
        #     50ms < delay < 1s
        # would become
        #     delay > 50ms < 1s
        # or even `delay <longer-than> [<shorter-than>]` (this is the easiest)
        #     delay 50ms 1s
    }
    binding <gesture-identifier> <command>

    # gesture ...
    # binding ...
}

# The following would also work since this is what the parser will convert it to
touch gesture <identifier> <first-action>
# ...
touch gesture <identifier> <last-action>
touch binding <gesture-identifier> <command>

Some thoughts for the above syntax:

cmd_touch should be in it's own file (sway/commands/touch.c) and have two subcommands touch_cmd_gesture and touch_cmd_binding (which are both in their own files inside of the directory sway/commands/touch)
touch_cmd_gesture has subcommands for each action touch_gesture_cmd_<action>
- They should probably also be split into separate files -- sway/command/touch/gesture/<action>.c
current_gesture should be added to config->handler_context and set/unset in touch_cmd_gesture
cmd_touch should be added to the handler list in config_command so quotes do not get stripped
- touch_cmd_gesture should strip quotes
- touch_cmd_binding should only strip quotes for <gesture-identifier>
cmd_touch should be added as a subcommand of cmd_mode
- touch bindings should be stored per mode
- I'm indecisive on whether gestures should be per mode or global

(Also, note some commands in the example in #1904 like workspace --create next_on_output are not currently implemented and if they do get implements, should be done in a separate PR)

Hopefully, that helps iron out structure. As for cmd_touch not being recognized in your current code, it appears to be recognized for me. However, you are returning NULL instead of a result from the handler so sway segfaults (this should probably be handled better -- but that's for a different PR).

(Side note: the meson.build currently in the libtouch repo generates a library called touch not libtouch so currently the dependency line in this PR's meson.build needs to be changed for it to build)

grahnen · 2019-01-23T04:08:08Z

Sorry about the chaos above. Tried to rebase my local branch to upstream master, but I messed it up big time. It's fixed now, anyway. It even compiles correctly when libtouch is installed.

Somewhere hidden in these commits are my commits. Oops.

Thanks @RedSoxFan for the help with the structure!

Should the gestures be mode-specific? Should they be able to be both mode-specific and mode-agnostic?

RedSoxFan · 2019-01-23T04:15:43Z

Somewhere hidden in these commits are my commits.

Depending on what your remotes are, you may need to alter the commands, but the following should fix the commit history

git reset --hard 12590e9 
git rebase upstream/master
git rebase --skip
git log --oneline upstream/master..
# Only your commits should be listed now
git push -f origin touch-gestures

Should the gestures be mode-specific? Should they be able to be both mode-specific and mode-agnostic?

I think the gestures can be mode-agnostic. You can bind the same gesture to different commands in different modes though.

include/sway/input/cursor.h

RedSoxFan · 2019-01-24T03:09:10Z

I'm just going to leave this here as something to keep in mind when working on the PR:
https://github.com/swaywm/sway/blob/master/CONTRIBUTING.md#style-reference

grahnen · 2019-01-24T03:24:03Z

Now it's crashing with some xcb errors that I have no understanding of. Yet.

The following config is parseable
`touch {

gesture tap {

    touch down

    threshold 3

 }

 binding tap exec "notify-send progress!"

}`

Am I checking for complete gestures in the right place? In cursor.c, after updating the gestures?
I'll give the style guide a proper read before I add anything else! (and commit with a million undescriptive messages, sorry about those)

RedSoxFan · 2019-01-24T04:58:02Z

Now it's crashing with some xcb errors that I have no understanding of. Yet.

If you are getting that from the debug log, it's probably unrelated and harmless. You probably want coredumpctl gdb sway followed by bt full.

Am I checking for complete gestures in the right place? In cursor.c, after updating the gestures?

The location is fine. However, this current data structures do not take into account seats or modes. Each action of a gesture could occur on a different seat and the command will execute on the seat that performed the last action. With the current structure of libtouch, it's probably going to be really hard to account for either since there will have to be multiple engines and a gesture is tied to a single engine.

Also, do not pass the focused container to execute_command, just leave the last argument NULL.

I'll give the style guide a proper read before I add anything else! (and commit with a million undescriptive messages, sorry about those)

Although the final product should have well divided commits with good commit messages, WIP commits are fine for now. This will need some squashing/fix-ups later on anyway and you can reword the final commits then.

grahnen · 2019-01-24T13:53:33Z

Is it reasonable to attach the engine to the sway_seat struct, and simply deep-copy it from the config for each seat, or is that too lazy a solution?

ddevault · 2019-01-24T14:23:47Z

Take a look at how we track xkb state for inspiration.

ddevault · 2019-01-25T18:50:50Z

Re-emphasizing the importance of the style guide - also applies to the code in libtouch, if you want to upstream that.

grahnen · 2019-04-20T15:52:33Z

Rotation is still broken but regions, pinch, swipe and n-touches should work. Unless I've messed up git again. It works on my machine :)

I've had a lot of more pressing matters to attend to, and that will probably continue to be the case at least until summer. Completing courses turns out to require participation; who would've thought?

RedSoxFan

Please review the style guide. There are several style issues including a lot of unneeded blank lines, lines that only have tabs, mixed tabs/spaces, removed/added lines that did not need to be touched, lines that are too long, missing whitespace around if/arguments/etc, and several others. I pointed out some, but there are too many to comment on all of them.

Unfortunately, I cannot comment on the functionality as I don't have the hardware to test with. Hopefully, someone else can help with that.

Also, please rebase against master instead of merging master into the branch for a cleaner commit history.

Oh and the commands will need to be documented

RedSoxFan · 2019-04-20T19:31:32Z

include/sway/config.h

@@ -53,7 +53,6 @@ struct sway_binding {
 	uint32_t modifiers;
 	char *command;
 };
-


Can you readd this blank line?

RedSoxFan · 2019-04-20T19:33:30Z

include/sway/config.h

+	struct libtouch_target *target;
+};
+
+


Extra blank line

RedSoxFan · 2019-04-20T19:36:38Z

include/sway/config.h

@@ -507,7 +521,8 @@ struct sway_config {
 	list_t *command_policies;
 	list_t *feature_policies;
 	list_t *ipc_policies;
-
+


Extra tab on blank line

RedSoxFan · 2019-04-20T19:37:46Z

include/sway/input/cursor.h

@@ -36,6 +36,8 @@ struct sway_cursor {
 	struct wl_listener axis;
 	struct wl_listener frame;

+


Extra blank line

RedSoxFan · 2019-04-20T19:38:03Z

meson.build

@@ -58,6 +59,7 @@ xcb            = dependency('xcb', required: get_option('xwayland'))
 math           = cc.find_library('m')
 rt             = cc.find_library('rt')
 git            = find_program('git', native: true, required: false)
+libtouch       = dependency('libtouch')


The dependency is duplicated

RedSoxFan · 2019-04-20T20:13:12Z

sway/input/cursor.c

+	struct libtouch_gesture *complete;
+	struct gesture_config *cfg;
+	int idx;
+	cfg = (struct gesture_config*) config->gesture_configs->items[0];


This seems unused

RedSoxFan · 2019-04-20T20:14:48Z

sway/input/cursor.c

+
+static void handle_gestures(struct libtouch_progress_tracker *tracker, struct sway_seat *seat) {
+	struct libtouch_gesture *complete;
+	struct gesture_config *cfg;


cfg and idx can be moved inside the while to where they are used

RedSoxFan · 2019-04-20T20:16:45Z

sway/input/cursor.c

+				execute_command(cfg->command, seat, NULL);
+			}
+		} else {
+			sway_log(SWAY_DEBUG, "Gesture unbound!");


This should probably be a SWAY_ERROR as it should not occur

RedSoxFan · 2019-04-20T20:17:29Z

sway/input/cursor.c

@@ -350,16 +381,22 @@ static void handle_touch_down(struct wl_listener *listener, void *data) {
 	seat->touch_x = lx;
 	seat->touch_y = ly;

+	libtouch_progress_register_touch(cursor->gesture_tracker, event->time_msec,
+					 event->touch_id, LIBTOUCH_TOUCH_DOWN,


style: do not line up the arguments, just ident

RedSoxFan · 2019-04-20T20:21:23Z

include/sway/config.h

@@ -507,7 +521,8 @@ struct sway_config {
 	list_t *command_policies;
 	list_t *feature_policies;
 	list_t *ipc_policies;
-
+
+	struct libtouch_engine *gesture_engine;


I'm not seeing where this is free'd/deinitialized. Also, is the engine reusable between config reloads? If so, it should be moved to sway_server. If not, it is fine here.

freswa · 2020-08-19T18:34:15Z

@grahnen Are you still working on this? I'm willing to help to get this ready for merging...

grahnen · 2020-08-26T14:59:35Z

@freswa Unfortunately not. I had to finish my studies, and got rid of my touchscreen PC. Feel free to take over!

emersion reviewed Jan 15, 2019

View reviewed changes

sway/input/cursor.c Outdated Show resolved Hide resolved

ddevault reviewed Jan 23, 2019

View reviewed changes

include/sway/input/cursor.h Outdated Show resolved Hide resolved

emersion added the enhancement New feature or incremental improvement label Mar 11, 2019

RedSoxFan mentioned this pull request Apr 4, 2019

[bug] scroll_button <event-code-or-name> gives error #4016

Closed

RedSoxFan requested changes Apr 20, 2019

View reviewed changes

grahnen and others added 14 commits April 22, 2019 13:06

Link against libtouch

642d7fd

Integrate libtouch into sway and cursor.(c|h)

6a099a7

Register touch events in libtouch engine

bccc14e

Declare cmd_gesture and create an empty body.

79f3cf8

Try to incorporate touch in config

cb94176

Restructure and add a hiearchy of commands

ba2020a

Fix the failed rebase.

6ccc01e

Implement binding

dda3ac1

Add calling actions on gesture

40811a0

Use the right seat pointer

f89f45e

Implement remaining actions, split touch for different seats

7873a0f

Integrate libtouch into sway and cursor.(c|h)

a3e3242

Switch to progress tracker

16e60f2

Follow style guide

068c777

grahnen added 3 commits April 22, 2019 14:12

Fix errors

76ec7d9

Fix duplicated dependency

d70e55e

Remove empty lines

90cbe9c

emersion marked this pull request as draft May 1, 2020 09:46

Aleksanaa mentioned this pull request Apr 14, 2023

1:1 trackpad gestures for switching desktops WillPower3309/swayfx#142

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Implement touch gestures using libtouch #3428

[WIP] Implement touch gestures using libtouch #3428

		@@ -36,6 +36,8 @@ struct sway_cursor {
		struct wl_listener axis;
		struct wl_listener frame;

[WIP] Implement touch gestures using libtouch #3428

Are you sure you want to change the base?

[WIP] Implement touch gestures using libtouch #3428

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment