[PATCH weston 4/4] input: Detect keyboard capabilities
Derek Foreman
derekf at osg.samsung.com
Wed May 20 10:32:46 PDT 2015
Thanks for taking a look!
On 20/05/15 05:04 AM, Jonas Ådahl wrote:
> On Tue, May 05, 2015 at 03:01:54PM -0500, Derek Foreman wrote:
>> Some devices (power buttons, acpi video bus driver) are considered
>> keyboards but you can't type with them.
>>
>> As of libinput 0.15 we can query a keyboard to see which key events it
>> can generate. We use this to detect if a keyboard can type letters or
>> digits.
>>
>> The wayland protocol doesn't propagate these capabilites, so
>> weston_seat_send_dirty_caps() will differentiate between dirty keyboards
>> (which generate the weston signal) and dirty seats (which generate a
>> wayland event) and only send the appropriate updates.
>>
>> Functions are provided for backends that don't use libinput to force these
>> capabilities to a sensible state.
>>
>> Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
>> ---
>> configure.ac | 2 +-
>> src/compositor-headless.c | 1 +
>> src/compositor-rdp.c | 1 +
>> src/compositor-wayland.c | 1 +
>> src/compositor-x11.c | 1 +
>> src/compositor.h | 8 ++++++++
>> src/input.c | 29 +++++++++++++++++++++++++++--
>> src/libinput-device.c | 32 +++++++++++++++++++++++++++++++-
>> src/libinput-device.h | 4 +++-
>> src/libinput-seat.c | 27 +++++++++++++++++++++++++++
>> src/screen-share.c | 1 +
>> 11 files changed, 102 insertions(+), 5 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index d9d8d8f..4e9ae20 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -155,7 +155,7 @@ if test x$enable_drm_compositor = xyes; then
>> fi
>>
>>
>> -PKG_CHECK_MODULES(LIBINPUT_BACKEND, [libinput >= 0.8.0])
>> +PKG_CHECK_MODULES(LIBINPUT_BACKEND, [libinput >= 0.15.0])
>> PKG_CHECK_MODULES(COMPOSITOR, [$COMPOSITOR_MODULES])
>>
>> AC_ARG_ENABLE(wayland-compositor, [ --enable-wayland-compositor],,
>> diff --git a/src/compositor-headless.c b/src/compositor-headless.c
>> index 0ddb26e..0b461c6 100644
>> --- a/src/compositor-headless.c
>> +++ b/src/compositor-headless.c
>> @@ -184,6 +184,7 @@ headless_input_create(struct headless_compositor *c)
>> if (weston_seat_init_keyboard(&c->fake_seat, NULL) < 0)
>> return -1;
>>
>> + weston_seat_override_keyboard_caps(&c->fake_seat, true, true);
>> weston_seat_send_dirty_caps(&c->fake_seat);
>>
>> return 0;
>> diff --git a/src/compositor-rdp.c b/src/compositor-rdp.c
>> index 5b50382..b6e3876 100644
>> --- a/src/compositor-rdp.c
>> +++ b/src/compositor-rdp.c
>> @@ -842,6 +842,7 @@ xf_peer_post_connect(freerdp_peer* client)
>> }
>> weston_seat_init_keyboard(&peerCtx->item.seat, keymap);
>> weston_seat_init_pointer(&peerCtx->item.seat);
>> + weston_seat_override_keyboard_caps(&peerCtx->item.seat, true, true);
>> weston_seat_send_dirty_caps(&peerCtx->item.seat);
>>
>> peerCtx->item.flags |= RDP_PEER_ACTIVATED;
>> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
>> index 53159c4..1e33697 100644
>> --- a/src/compositor-wayland.c
>> +++ b/src/compositor-wayland.c
>> @@ -1463,6 +1463,7 @@ input_handle_keymap(void *data, struct wl_keyboard *keyboard, uint32_t format,
>> else
>> weston_seat_init_keyboard(&input->base, keymap);
>>
>> + weston_seat_override_keyboard_caps(&input->base, true, true);
>> weston_seat_send_dirty_caps(&input->base);
>>
>> xkb_keymap_unref(keymap);
>> diff --git a/src/compositor-x11.c b/src/compositor-x11.c
>> index 4a3b10e..69e3a14 100644
>> --- a/src/compositor-x11.c
>> +++ b/src/compositor-x11.c
>> @@ -336,6 +336,7 @@ x11_input_create(struct x11_compositor *c, int no_input)
>> return -1;
>> xkb_keymap_unref(keymap);
>>
>> + weston_seat_override_keyboard_caps(&c->core_seat, true, true);
>> weston_seat_send_dirty_caps(&c->core_seat);
>>
>> x11_compositor_setup_xkb(c);
>> diff --git a/src/compositor.h b/src/compositor.h
>> index e05b262..da6d126 100644
>> --- a/src/compositor.h
>> +++ b/src/compositor.h
>> @@ -451,6 +451,10 @@ weston_touch_start_drag(struct weston_touch *touch,
>> struct wl_client *client);
>> void
>> weston_seat_send_dirty_caps(struct weston_seat *seat);
>> +void
>> +weston_seat_override_keyboard_caps(struct weston_seat *seat,
>> + bool has_letters,
>> + bool has_digits);
>>
>> struct weston_xkb_info {
>> struct xkb_keymap *keymap;
>> @@ -505,6 +509,10 @@ struct weston_keyboard {
>> enum weston_led leds;
>> } xkb_state;
>> struct xkb_keymap *pending_keymap;
>> +
>> + bool has_digits;
>> + bool has_letters;
>> + bool caps_dirty;
>> };
>>
>> struct weston_seat {
>> diff --git a/src/input.c b/src/input.c
>> index c37bd20..3a0adc3 100644
>> --- a/src/input.c
>> +++ b/src/input.c
>> @@ -601,14 +601,16 @@ weston_touch_destroy(struct weston_touch *touch)
>>
>> /* Send seat capability updates if necessary
>> *
>> - * Checks if the seat capabilities (WL_SEAT_CAPABILITY_*) have changed
>> - * and propagates updates appropriately.
>> + * Checks if the seat capabilities (WL_SEAT_CAPABILITY_*) or the keyboard
>> + * capabilities (has_digits, has_letters) have changed and propagates
>> + * updates appropriately.
>> *
>> * \param seat These seat to send capabilities changes for
>> */
>> WL_EXPORT void
>> weston_seat_send_dirty_caps(struct weston_seat *seat)
>> {
>> + struct weston_keyboard *keyboard = weston_seat_get_keyboard(seat);
>> enum wl_seat_capability caps = 0;
>> struct wl_resource *resource;
>>
>> @@ -622,8 +624,15 @@ weston_seat_send_dirty_caps(struct weston_seat *seat)
>>
>> wl_resource_for_each(resource, &seat->base_resource_list)
>> wl_seat_send_capabilities(resource, caps);
>> + }
>> +
>> + if (seat->caps_dirty || (keyboard && keyboard->caps_dirty)) {
>> wl_signal_emit(&seat->updated_caps_signal, seat);
>> }
>> +
>> + if (keyboard)
>> + keyboard->caps_dirty = false;
>> +
>> seat->caps_dirty = false;
>> }
>>
>> @@ -2230,6 +2239,22 @@ weston_seat_release_keyboard(struct weston_seat *seat)
>> }
>> }
>>
>> +void
>> +weston_seat_override_keyboard_caps(struct weston_seat *seat,
>> + bool has_letters,
>> + bool has_digits)
>> +{
>> + struct weston_keyboard *keyboard = weston_seat_get_keyboard(seat);
>> +
>> + if (!keyboard)
>> + return;
>> +
>> + keyboard->caps_dirty = has_digits ^ keyboard->has_digits ||
>> + has_letters ^ keyboard->has_letters;
>> + keyboard->has_digits = has_digits;
>> + keyboard->has_letters = has_letters;
>> +}
>> +
>> WL_EXPORT void
>> weston_seat_init_pointer(struct weston_seat *seat)
>> {
>> diff --git a/src/libinput-device.c b/src/libinput-device.c
>> index 3fb507f..983665c 100644
>> --- a/src/libinput-device.c
>> +++ b/src/libinput-device.c
>> @@ -480,6 +480,36 @@ configure_device(struct evdev_device *device)
>> evdev_device_set_calibration(device);
>> }
>>
>> +static enum evdev_device_seat_capability
>> +keyboard_caps(struct libinput_device *dev)
>> +{
>> + bool letters = true, digits = true;
>> + uint32_t alpha_keys[] = {
>> + KEY_A, KEY_B, KEY_C, KEY_D, KEY_E, KEY_F,
>> + KEY_G, KEY_H, KEY_I, KEY_J, KEY_K, KEY_L,
>> + KEY_M, KEY_N, KEY_O, KEY_P, KEY_Q, KEY_R,
>> + KEY_S, KEY_T, KEY_U, KEY_V, KEY_W, KEY_X,
>> + KEY_Y, KEY_Z, KEY_RESERVED
>> + };
>> + uint32_t digit_keys[] = {
>> + KEY_1, KEY_2, KEY_3, KEY_4, KEY_5, KEY_6,
>> + KEY_7, KEY_8, KEY_9, KEY_0, KEY_RESERVED
>> + };
>> + int i;
>> +
>> + for (i = 0; letters && alpha_keys[i] != KEY_RESERVED; i++)
>> + letters &= libinput_device_keyboard_has_key(dev,
>> + alpha_keys[i]);
>> +
>> + for (i = 0; digits && digit_keys[i] != KEY_RESERVED; i++)
>> + digits &= libinput_device_keyboard_has_key(dev,
>> + digit_keys[i]);
>> +
>> + return EVDEV_SEAT_KEYBOARD |
>> + (letters ? EVDEV_SEAT_KEYBOARD_LETTERS : 0) |
>> + (digits ? EVDEV_SEAT_KEYBOARD_DIGITS : 0);
>> +}
>> +
>> struct evdev_device *
>> evdev_device_create(struct libinput_device *libinput_device,
>> struct weston_seat *seat)
>> @@ -497,7 +527,7 @@ evdev_device_create(struct libinput_device *libinput_device,
>> if (libinput_device_has_capability(libinput_device,
>> LIBINPUT_DEVICE_CAP_KEYBOARD)) {
>> weston_seat_init_keyboard(seat, NULL);
>> - device->seat_caps |= EVDEV_SEAT_KEYBOARD;
>> + device->seat_caps |= keyboard_caps(libinput_device);
>
> If we don't want keyboards with neither numbers nor letters be
> advertised as keyboards, can't we just simply not set
> EVDEV_SEAT_KEYBOARD here? That way we'd need nothing of the extra API or
> the extra calls.
I assumed we would want those keyboards to still be advertised.
It might be worthwhile to hide keyboards if the compositor binds all the
keys they have (for example, the power button "keyboard" could end up
like this at some point, if weston ever starts caring about the power
button...), but I haven't looked into doing that at all yet.
> On a related note, doing something like this would mean that key events
> would still be sent by libinput-device.c. As far as I can tell, this
> means that if the only keyboard provided by libinput doesn't have digit
> or letter keys, we'd never set seat->keyboard, but still call
> notify_key, meaning a NULL pointer dereference in notify_key().
Yeah, I don't think hiding/removing keyboards is a good idea, with the
exception of hiding keyboards whose entire inventory has been grabbed by
compositor keybinds. I'd thought about jamming them into a special
administrative seat, but that's pretty disgusting.
Hiding the keyboards exposes another interesting issue. A tablet with
only a touch interface can currently have "keyboard focus" because it
likely has a power button that shows up as a keyboard.
Maybe that's something we'll have to address eventually anyway. It's
already visible in multi seat since the first seat is the only one that
gets the funny keyboards.
> If, in libinput-device.c we'd ignore key events when we never set the
> EVDEV_SEAT_KEYBOARD bit, this would mean that even if a client has a
> wl_keyboard, it wouldn't ever get key events from non-keyboard keys.
Well, wouldn't get events from keyboards that only have non-keyboard
keys (or have an incomplete set of keyboard keys)?
There might be keyboards with incomplete sets of keys like this thing:
http://www.dx.com/cs/p/delux-t9-usb-2-0-wired-gaming-46-key-keyboard-w-3-mode-led-backlight-black-red-239391
We're still going to want to pass any event that generates, even though
it fails both the has_digits and has_letters tests in my patch...
Of course, I wouldn't bet money on it accurately reporting what keycodes
it's capable of passing anyway. :)
> I feels like the scope of wl_keyboard needs to be clarified before doing
> this type of change, so that it is clear what type of events can be
> relied on.
I didn't think there would be interest in changing wl_keyboard at all.
What I'm looking for here is a way to auto-hide a virtual keyboard based
on whether there are real keyboards present that make it unnecessary.
That could be done by putting the show/hide logic into the compositor
and letting the compositor figure out what the attached keyboards can do.
We could still pass that decision on to the client/toolkit/whatever if
we added wl proto for informing the client what keys are available, or
even just simple digits/letters capabilities like this.
>
> Jonas
>
>
>> }
>> if (libinput_device_has_capability(libinput_device,
>> LIBINPUT_DEVICE_CAP_POINTER)) {
>> diff --git a/src/libinput-device.h b/src/libinput-device.h
>> index 0775743..70591c2 100644
>> --- a/src/libinput-device.h
>> +++ b/src/libinput-device.h
>> @@ -34,7 +34,9 @@
>> enum evdev_device_seat_capability {
>> EVDEV_SEAT_POINTER = (1 << 0),
>> EVDEV_SEAT_KEYBOARD = (1 << 1),
>> - EVDEV_SEAT_TOUCH = (1 << 2)
>> + EVDEV_SEAT_TOUCH = (1 << 2),
>> + EVDEV_SEAT_KEYBOARD_DIGITS = (1 << 3),
>> + EVDEV_SEAT_KEYBOARD_LETTERS = (1 << 4)
>> };
>>
>> struct evdev_device {
>> diff --git a/src/libinput-seat.c b/src/libinput-seat.c
>> index f0fcd51..af695a6 100644
>> --- a/src/libinput-seat.c
>> +++ b/src/libinput-seat.c
>> @@ -45,6 +45,30 @@ udev_seat_create(struct udev_input *input, const char *seat_name);
>> static void
>> udev_seat_destroy(struct udev_seat *seat);
>>
>> +static void
>> +seat_rescan_keyboard_caps(struct udev_seat *udev_seat)
>> +{
>> + struct evdev_device *dev;
>> + struct weston_seat *weston_seat = &udev_seat->base;
>> + struct weston_keyboard *keyboard = weston_seat->keyboard_resource;
>> + bool has_letters = false, has_digits = false;
>> +
>> + keyboard = weston_seat_get_keyboard(weston_seat);
>> + if (!keyboard)
>> + return;
>> +
>> + wl_list_for_each(dev, &udev_seat->devices_list, link) {
>> + has_letters = has_letters || (dev->seat_caps & EVDEV_SEAT_KEYBOARD_LETTERS);
>> + has_digits = has_digits || (dev->seat_caps & EVDEV_SEAT_KEYBOARD_DIGITS);
>> + }
>> +
>> + if (has_letters != keyboard->has_letters || has_digits != keyboard->has_digits) {
>> + keyboard->has_letters = has_letters;
>> + keyboard->has_digits = has_digits;
>> + keyboard->caps_dirty = true;
>> + }
>> +}
>> +
>> static struct udev_seat *
>> get_udev_seat(struct udev_input *input, struct libinput_device *device)
>> {
>> @@ -81,6 +105,8 @@ device_added(struct udev_input *input, struct libinput_device *libinput_device)
>> udev_seat = (struct udev_seat *) seat;
>> wl_list_insert(udev_seat->devices_list.prev, &device->link);
>>
>> + seat_rescan_keyboard_caps(udev_seat);
>> +
>> pointer = weston_seat_get_pointer(seat);
>> if (seat->output && pointer)
>> weston_pointer_clamp(pointer,
>> @@ -117,6 +143,7 @@ device_removed(struct udev_input *input, struct libinput_device *libinput_device
>>
>> device = libinput_device_get_user_data(libinput_device);
>> evdev_device_destroy(device);
>> + seat_rescan_keyboard_caps(udev_seat);
>> weston_seat_send_dirty_caps(&udev_seat->base);
>> }
>>
>> diff --git a/src/screen-share.c b/src/screen-share.c
>> index 597ad9b..3e92d61 100644
>> --- a/src/screen-share.c
>> +++ b/src/screen-share.c
>> @@ -214,6 +214,7 @@ ss_seat_handle_keymap(void *data, struct wl_keyboard *keyboard,
>> else
>> weston_seat_init_keyboard(&seat->base, keymap);
>>
>> + weston_seat_override_keyboard_caps(&seat->base, true, true);
>> weston_seat_send_dirty_caps(&seat->base);
>>
>> xkb_keymap_unref(keymap);
>> --
>> 2.1.4
>>
>> _______________________________________________
>> wayland-devel mailing list
>> wayland-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list