Merge multiple devices into one keyboard

[occivink]

Context: my keyboard has an n-key rollover feature which makes it register itself as multiple HID devices. Each of these is interpreted as an individual keyboard by wlroots compositors.

Consequently, modifiers are only sent if they belong to the same input device as the key being pressed. Grouping multiple devices into one keyboard would allow sharing these modifiers (as well as keymap and repeat rate),

The libinput list-devices command indicates that these devices all belong to the same group, which might be a good way to merge keyboards by default.

Device:           Keyed Up Labs ES-87
Kernel:           /dev/input/event5
Group:            5
...
Device:           Keyed Up Labs ES-87 Keyboard
Kernel:           /dev/input/event6
Group:            5
...
Device:           Keyed Up Labs ES-87 Consumer Control
Kernel:           /dev/input/event7
Group:            5

[Ongy]

Well, found a victim that has to test for us I guess :)

This should be feasable with the same/a similar approach to what I did for graphics tablets (https://github.com/swaywm/wlroots/blob/master/rootston/seat.c#L836).
Your log shows that all devices are in the same group and I hope those are the only ones in said group.

The more interesting question is, how and where this should be handled.
Following the current code examples we have, it's a better fit for compositors. OTOH this is a major issue, should need support in every compositor and should be non-opinionated here.

To put it into the compositor, I'd put the lookup here https://github.com/swaywm/wlroots/blob/master/rootston/seat.c#L620 and merge devices from the same group.
In the library it should probably be around https://github.com/swaywm/wlroots/blob/master/backend/libinput/events.c#L88 and not create a keyboard in the first place.

Compositor version is probably easier to do, since iirc there's no clean way to iterate all libinput devices at that point.

---

Notes:
I'm not 100% sure if I mentioned it before, though I have been aware:
The way we handle keyboards currently is different from other compositors/libraries.
Because wlroots uses a dedicated xkb state for each keyboard, while others have one per seat.
IMO the xkb-common doc hints towards "the other" way being the intended way.
IMO our way is better :) I use the feature of having different layouts/xkb config stuff for different keyboards.

So I'm against just going for the "state per seat" approach, even though it'd fix this.

[occivink]

Your log shows that all devices are in the same group and I hope those are the only ones in said group.

Yes, that's the case.

The way we handle keyboards currently is different from other compositors/libraries.

I noticed, the discrepancy with weston made me dig a bit into this.
I also didn't mean to imply that all keyboard should be merged together, just that it should be possible for a compositor to do so. I was able to hack something on tinywl, but I figured it should be something supported by the library.

I will look into what you did for the tablets and see what I can do.

[emersion]

Following the current code examples we have, it's a better fit for compositors

Not possible as there is one wlr_keyboard (thus one XKB state) per input device.

[Ongy]

I thought we feed that from the compositor and could just ignore the leftovers.
But I might easily be wrong, haven't touched any of this in quite a while.

then yea, it'll have to be done in the backend.

[emersion]

The XKB state is bound to the wlr_keyboard, thus to the wlr_input_device. This makes it impossible to fix this issue. On the other hand, we don't want to bind it to the seat either (to allow for multiple separate devices bound to the same seat). We might need to add a new type for this.

[Ongy]

Why wouldn't it work to change https://github.com/swaywm/wlroots/blob/master/backend/libinput/keyboard.c#L10 to have a list, instead of a single pointer and handle all of this in the libinput backend?

From what I see, the events all have to end up onthe same wlr_keyboard, and this (should be)is libinput specific.

[emersion]

Yeah, making this libinput-specific would work.

[JonathanILevi]

Shouldn't modifiers be shared throughout the whole seat, not just the same keyboard? Xorg has modifiers shared between keyboards, so does the raw tty terminal. The only place I know of that has modifiers specific to the keyboard is OSX.

Maybe I am wrong, but what is the logic behind modifiers not shared anyway?

None the less, having the ability to share modifiers would be appreciated.

[Duncaen]

I think a solution similar to how a wlr_cursor is the shared state between one or more pointers would be good for keyboards too.

This would allow to let the compositor/seat decide if modifiers should be shared by attaching the input device to the "wlr_keyboard".

The only problem stopping me from working on a PR is that I can't come up with a good name for those two different wlr_keyboard types.

Edit:

So I looked more into it, the state is bound to the keymap which means you can't share the state between different keyboards with different keymaps.

My proposed change would be to allow to pass a state to wlr_keyboard_set_keymap as in
#1420 which allows the seat/compositor to decide which keyboards can and should share the state.

But I think the libinput backend still needs to be improved to just create one wlr_keyboard per device group because the seats keyboard would be constantly switched and the keymap would be send to the client each time.

[vesim987]

MayeulC asked for a libinput trace so there it is: https://gist.github.com/vesim987/943235a8b06756dcab73da04bae89faa

I've tested this trace using libinput debug-events and it is showing the events fine, but sway is still not reacting for them so it should be reproducible

To test this I've used

bindsym XF86AudioMicMute exec pactl set-source-mute @DEFAULT_SOURCE@ toggle

in my config.

And also i think it is worth mentioning that the input devices are in different groups:

-event8   DEVICE_ADDED     Huawei WMI hotkeys                seat0 default group10 cap:k
-event5   DEVICE_ADDED     AT Translated Set 2 keyboard      seat0 default group11 cap:k 

EDIT:
After patching libinput to force the same group for all devices it is still not working.