input management and seat #1505

acrisci · 2017-12-06T13:32:16Z

It's probably going to look a lot like rootston. An input manager that manages device add/remove and passes them to seats.

Tests:

weston-clickdot
focus windows with left click
type into focused windows (with modifiers)
change keyboard layout at runtime: swaymsg input 0:0:X11_keyboard xkb_layout de.
hotplug devices
attach devices to seat at runtime: swaymsg seat seat0 attach 0:0:X11_keyboard

Future work:

input for popups and subsurfaces
detach devices
keycodes to keysym conversion
other device config (libinput, mappings)
touch, tablet tool, tablet pad

ddevault · 2017-12-06T16:56:58Z

input device_id {
    # no change from existing input blocks
    layout us
    options caps:escape
}

cursor "cursor-0" {
    attach device_id...
    attach *
    attach --output=HDMI-A-1 device_id
    attach --region=100x100@10,10 device_id
}

seat "seat-0" {
    attach keyboard_id...
    attach cursor_id...
    attach *
    attach --layout=us --options=caps:escape keyboard_id
    attach --as-pointer touch_id
    attach --as-touch pointer_id...
}

Attaching a touch device as a pointer would present a wl_pointer to the client and simulate that. Attaching n pointers as a touch device simulates a wl_touch with n touchpoints. Attaching a keyboard with layout/options/etc overrides that keyboard's default layout/options for any interactions that go through this seat.

Attaching a device twice to anything should just update its options to match any extra parameters given.

After config parsing, if no cursors were configured, add a default:

cursor "cursor-0" {
    attach *
}

If no seats were configured, add a default:

seat "seat-0" {
    attach *
}

ddevault · 2017-12-06T17:05:58Z

Oh, I should mention that detach should work at runtime for all of these (attach should also work at runtime).

ddevault · 2017-12-06T17:09:31Z

Also, another concern we should address is that previously sway input devices were identified with a consistent identifier based on libinput. We need to carry this identifier over, but there are additional constraints - we should be able to specify sway_id/keyboard for example to refer just to the keyboard capability of a multi-capable device. For other cases the old sway ID needs to remain the same for backwards compatability.

ddevault · 2017-12-06T17:16:17Z

bindsym --to=keyboard_id|seat_id ...

acrisci · 2017-12-07T11:39:04Z

I don't think it makes sense to have cursor config and seat config in separate blocks since they are conceptually bound to each other. There is a 1:1 relationship between seat and cursor (necessary because there can only be a single pointer location within a surface per seat).

It won't work well if seats share cursors because then they would be sharing the devices as well. If they share devices, surfaces then get double input events (one per seat). For instance, two button clicks on the surface when the user clicks once, two scrolls, two motion events, two dnd targets which is going to lead to a lot of nasty bugs that can't be fixed. Clipboard for cursor events is broken because only the first seat that wins the race to click the "copy" button gets the contents. Just picking a seat and going with that will break the clipboard for the same reason and is no different from just putting the device on that seat but with less predictability. I think this is a protocol limitation.

Devices can live on multiple seats, but this is a really dumb thing to do and I doubt anyone would ever us this configuration except by mistake. In this case, the single device will control both cursors at the same time.

acrisci · 2017-12-07T11:50:09Z

Also, another concern we should address is that previously sway input devices were identified with a consistent identifier based on libinput.

And what if libinput is not in use for that backend? Should we just ignore all input configuration in that case?

ddevault · 2017-12-07T15:06:26Z

It won't work well if seats share cursors because then they would be sharing the devices as well. If they share devices, surfaces then get double input events (one per seat). For instance, two button clicks on the surface when the user clicks once, two scrolls, two motion events, two dnd targets which is going to lead to a lot of nasty bugs that can't be fixed. Clipboard for cursor events is broken because only the first seat that wins the race to click the "copy" button gets the contents. Just picking a seat and going with that will break the clipboard for the same reason and is no different from just putting the device on that seat but with less predictability. I think this is a protocol limitation.

Cursors can be closely associated with seats I guess. However, I should note that I want you to be able to tie a keyboard into multiple seats, and we should work it out so that whichever seat got input last (from devices other than that keyboard) would propegate the keyboard events.

And what if libinput is not in use for that backend? Should we just ignore all input configuration in that case?

We should make up some other predictable device naming schema for that device.

acrisci · 2017-12-10T13:58:51Z

I'm messing with the rendering a little bit.

It looks like we were trying to cram the buffer into the space of the desired window geometry. Window geometry is always smaller than the size of the buffer so this leads to scaling issues which 1) makes the windows look ugly when scaled and 2) completely screws up surface coordinate calculations.

To avoid this, the approach is to always render the surface buffer as it is intended to be rendered. When the shell specifies window geometry, we place the top left corner of the window geometry at the top left corner of the sway container.

Shell configure requests for size work on window geometry size and not buffer size so this gives us a good result with xdg-shell. wl-shell does not seem to have a way to get window geometry coordinates so this shell will never be completely supported by sway and we need to do crappy workarounds. Hopefully window maximization will clear this up a bit because it won't render shadows so window geometry offsets should be close to zero (untested).

A consequence is we have to trust the shell surface to actually listen to our configure requests. If it doesn't, then I think we should call that a client bug and just clip the contents to the container extents (which should be done anyway to avoid shadows spilling over to adjacent containers).

…input

acrisci · 2017-12-18T15:47:55Z

The issue is that libinput is giving me two devices for the same keyboard and I was only configuring the first one which wasn't giving events because of an assumption that there would only be one device for the identifier.

It works now.

ddevault · 2017-12-18T17:38:41Z

Can you take a look at this @emersion?

martinetd

Nice, wish I had more time to help more! This is starting to look good!

I have a segfault on shutdown with that, but had another one before the PR so not sure how much we care about these in that state, here it is:

==5562==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000008 (pc 0x7f8e3ec5a327 bp 0x7ffdb9ce8ea0 sp 0x7ffdb9ce8e78 T0)
==5562==The signal is caused by a WRITE memory access.
==5562==Hint: address points to the zero page.
#0 0x7f8e3ec5a326 in wl_list_remove (/usr/lib/x86_64-linux-gnu/libwayland-server.so.0+0xc326)
#1 0x7f8e3e818920 in wlr_output_destroy ../types/wlr_output.c:300
#2 0x7f8e3e7fd606 in wlr_drm_backend_destroy ../backend/drm/backend.c:34
#3 0x7f8e3e7fb354 in wlr_backend_destroy ../backend/backend.c:36
#4 0x7f8e3e806d25 in multi_backend_destroy ../backend/multi/backend.c:35
#5 0x7f8e3e7fb354 in wlr_backend_destroy ../backend/backend.c:36
#6 0x55917aab44ae in server_fini ../sway/server.c:70
#7 0x55917aab3c6a in main ../sway/main.c:417
#8 0x7f8e3dd03560 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x20560)
#9 0x55917aab2179 in _start (/opt/wayland/bin/sway+0x12179)

Could very well be in wlroots, I'm on today's master there (d654a12)

emersion · 2017-12-19T08:56:43Z

sway/commands/input.c

+		return error;
+	}
+
+	if (config->reading && strcmp("{", argv[1]) == 0) {


The if (argc > 2) check should be done before this line and return an error.

Oh, there's already checkarg(argc, "input", EXPECTED_AT_LEAST, 2)). So just remove the check, I guess? (And remove this useless return cmd_results_new(CMD_BLOCK_INPUT, NULL, NULL);)

I think the checkarg needs to expect 3 arguments for seat <name> <command> or seat <name> {, then the argc check will be useless. Maybe this was copied over from the back block, which doesn't require an identifier.

I don't think the command name is included in argc, is it?

argc is 1 for a command with no argument with just argv[0] set, so yes it is. You'll have 1 + number of arguments.

I'm not sure about it. Right below, argv[0] is used as the device identifier and argv[1] is {.

Nevermind, it gets the original args minus the first command. Going to fix it by doing another checkarg so a command like seat <name> attach is not valid.

Yeah this is confusing. In sway's command handlers argv[0] is the first argument and argv[-1] is the command name.

emersion · 2017-12-19T09:06:58Z

sway/commands/seat.c

+		return cmd_results_new(CMD_BLOCK_SEAT, NULL, NULL);
+	}
+
+	if (argc > 2) {


Same here: useless because of checkarg.

emersion · 2017-12-19T09:24:06Z

sway/input/cursor.c

+	double y = cursor->cursor->y;
+
+	wlr_xcursor_manager_set_cursor_image(cursor->xcursor_manager,
+		"left_ptr", cursor->cursor);


There's a bug in wlroots about setting a hardware cursor's image twice (it fallbacks to software cursors, I'll fix that).

Setting the cursor image when it isn't needed is inefficient because it involves uploading the image to the GPU each time. This needs to be fixed here (maybe keep the name of the currently selected cursor in the struct?).

I dunno I'm not going to do the cursor until last and all of this will probably change. I'm happy if it just shows up.

Okay just add a TODO then so we don't forget about it.

emersion · 2017-12-19T09:34:40Z

sway/input/keyboard.c

+
+	xkb_keymap_unref(keyboard->keymap);
+	keyboard->keymap =
+		xkb_keymap_new_from_names(context, &rules, XKB_KEYMAP_COMPILE_NO_FLAGS);


Need to check if keyboard->keymap == NULL here in case the keymap doesn't exist.

acrisci · 2017-12-19T09:48:47Z

@martinetd yeah that's a wlroots issue related to a recent change. Make an issue there and we'll investigate it.

acrisci · 2017-12-19T10:39:22Z

Ok I think I fixed everything.

emersion · 2017-12-19T11:06:32Z

What about that checkarg stuff in seat and input commands?

emersion · 2017-12-19T22:27:49Z

My bad, for some reason I was still looking at the previous diff.

martinetd

I can confirm this addresses all previous issues I could see.
Got a couple more nitpicks that can be fixed now or later, nothing fancy.

martinetd · 2017-12-20T06:34:44Z

sway/config/seat.c

+
+	sway_log(L_DEBUG, "new_seat_config(%s)", name);
+	seat->name = strdup(name);
+	if (!sway_assert(seat->name, "could not allocate name for seat")) {


should free seat here

martinetd · 2017-12-20T06:35:01Z

sway/config/seat.c

+	seat->fallback = -1;
+	seat->attachments = create_list();
+	if (!sway_assert(seat->attachments,
+				"could not allocate seat attachments list")) {


should free seat->name and seat here (free_seat_config isn't safe to use here because it doesn't check seat->attachments before using it)

martinetd · 2017-12-20T06:46:50Z

sway/input/seat.c

+
+	seat->wlr_seat = wlr_seat_create(input->server->wl_display, seat_name);
+	if (!sway_assert(seat->wlr_seat, "could not allocate seat")) {
+		return NULL;


ditto should free seat

martinetd

Looks good 👍

ddevault · 2017-12-20T16:56:42Z

🎉

input skeleton

338a039

acrisci changed the title ~~[wip] input~~ [wip] input management and seat Dec 6, 2017

Tony Crisci added 2 commits December 7, 2017 07:35

rename input to input-manager

21ce208

basic input manager and seat

f6f63f6

Tony Crisci added 5 commits December 8, 2017 08:04

sway cursor

ec7fc42

input include directory

d76e745

sway xcursor manager

7c67bea

working xcursor

9333a7e

working pointer motion

e69b052

Tony Crisci added 6 commits December 10, 2017 10:08

button and axis events

5f644d7

Merge branch 'wlroots' into feature/input

0fdecb4

seat focus on button press

21626e8

remove pointer device

6339c4f

basic keyboard

609f639

set focus on new window

5e9ee32

acrisci force-pushed the feature/input branch from 60fe301 to 5e9ee32 Compare December 10, 2017 20:22

Tony Crisci added 2 commits December 10, 2017 15:37

keyboard remove

4d44974

sway pointer

9ae906c

acrisci force-pushed the feature/input branch from 57613dd to 9ae906c Compare December 10, 2017 21:57

Tony Crisci added 3 commits December 11, 2017 07:55

input config

462a451

sway input device

163edc5

config cleanup

538903b

Merge branch 'feature/input' of github.com:acrisci/sway into feature/…

a879607

…input

Merge branch 'wlroots' into feature/input

a949d7d

Tony Crisci added 2 commits December 18, 2017 14:27

ipc get_inputs

f298500

fix memory leak in swaymsg

8b4d811

martinetd approved these changes Dec 19, 2017

View reviewed changes

emersion reviewed Dec 19, 2017

View reviewed changes

Tony Crisci added 6 commits December 19, 2017 04:52

document fallback seat

9df4a2c

use snprintf to get identifier len

730af5e

error on not enough input/seat args for cmd

5c036a3

dont set cursor image on motion

39e7871

handle keymap not found

f35575f

fix memory leak in swaymsg

a4c1270

martinetd reviewed Dec 20, 2017

View reviewed changes

cleanup

63f7fb9

martinetd approved these changes Dec 20, 2017

View reviewed changes

ddevault approved these changes Dec 20, 2017

View reviewed changes

emersion approved these changes Dec 20, 2017

View reviewed changes

ddevault merged commit 373def4 into swaywm:wlroots Dec 20, 2017

acrisci deleted the feature/input branch December 20, 2017 17:04

cafehaine mentioned this pull request Jun 17, 2018

Per keyboard bindsym #2141

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

input management and seat #1505

input management and seat #1505

acrisci commented Dec 6, 2017 •

edited

Loading

ddevault commented Dec 6, 2017 •

edited

Loading

ddevault commented Dec 6, 2017 •

edited

Loading

ddevault commented Dec 6, 2017 •

edited

Loading

ddevault commented Dec 6, 2017

acrisci commented Dec 7, 2017 •

edited

Loading

acrisci commented Dec 7, 2017

ddevault commented Dec 7, 2017

acrisci commented Dec 10, 2017

acrisci commented Dec 18, 2017 •

edited

Loading

ddevault commented Dec 18, 2017

martinetd left a comment •

edited

Loading

emersion Dec 19, 2017

emersion Dec 19, 2017 •

edited

Loading

acrisci Dec 19, 2017

emersion Dec 19, 2017

martinetd Dec 19, 2017

emersion Dec 19, 2017

acrisci Dec 19, 2017

ddevault Dec 19, 2017

emersion Dec 19, 2017

emersion Dec 19, 2017

acrisci Dec 19, 2017

emersion Dec 19, 2017 •

edited

Loading

emersion Dec 19, 2017

acrisci commented Dec 19, 2017

acrisci commented Dec 19, 2017

emersion commented Dec 19, 2017

emersion commented Dec 19, 2017

martinetd left a comment

martinetd Dec 20, 2017

martinetd Dec 20, 2017

martinetd Dec 20, 2017

martinetd left a comment

ddevault commented Dec 20, 2017

input management and seat #1505

input management and seat #1505

Conversation

acrisci commented Dec 6, 2017 • edited Loading

ddevault commented Dec 6, 2017 • edited Loading

ddevault commented Dec 6, 2017 • edited Loading

ddevault commented Dec 6, 2017 • edited Loading

ddevault commented Dec 6, 2017

acrisci commented Dec 7, 2017 • edited Loading

acrisci commented Dec 7, 2017

ddevault commented Dec 7, 2017

acrisci commented Dec 10, 2017

acrisci commented Dec 18, 2017 • edited Loading

ddevault commented Dec 18, 2017

martinetd left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emersion Dec 19, 2017 • edited Loading

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

emersion Dec 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acrisci commented Dec 19, 2017

acrisci commented Dec 19, 2017

emersion commented Dec 19, 2017

emersion commented Dec 19, 2017

martinetd left a 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

martinetd left a comment

Choose a reason for hiding this comment

ddevault commented Dec 20, 2017

acrisci commented Dec 6, 2017 •

edited

Loading

ddevault commented Dec 6, 2017 •

edited

Loading

ddevault commented Dec 6, 2017 •

edited

Loading

ddevault commented Dec 6, 2017 •

edited

Loading

acrisci commented Dec 7, 2017 •

edited

Loading

acrisci commented Dec 18, 2017 •

edited

Loading

martinetd left a comment •

edited

Loading

emersion Dec 19, 2017 •

edited

Loading

emersion Dec 19, 2017 •

edited

Loading