[PATCH 4/4] The external key state api
David Herrmann
dh.herrmann at gmail.com
Tue Sep 10 04:22:54 PDT 2013
Hi
On Tue, Sep 10, 2013 at 11:16 AM, Martin Minarik
<minarik11 at student.fiit.stuba.sk> wrote:
> The problem is compositor should keep the key state. Compositor needs to:
> * deliver the key state to applications on initial focus and frequent alt tab
It can just retrieve the keystate from libevdev. Why does the
compositor has to manage the arrays itself?
> * the same key state may be shared by all seat devices
No. Keystate is bound to a device. It's not shared. You attach one
libevdev context to each device. If you higher layers cannot deal with
that, you need to fix them, not libevdev.
> * when syn dropped occurs, the state is corrected and libevdev will emit events
>
> The state should be kept in a single place only, preferably in the compositor.
The compositor is in control of libevdev. So the compositor actually
_does_ store the keystate. It doesn't get direct access, though. But
it can read the state via libevdev. Why isn't that enough?
> The state should not be in both places, because compositor should not waste time deriving from passing events something the libevdev already knows.
> Libevdev does not need the state for anything except updating it when keys, syn
> drop occurs. Libevdev only polls it rarely.
libevdev needs it to handle SYN values. Furthermore, libevdev allows
users to query it for the current state.
> My proposed solution is to add an api to libevdev. Compositor will activate
> this api and will start keeping the state. Compositor will handle get, update,
> sync calls from libevdev. Before libevdev is freed and before compositor frees
> the state, compositor must call deactivate().
>
> It is fully backwards compatible. In case the compositor does not activate()
> the api in between libevdev_new() and libevdev_set_fd(), libevdev will assume
> the compositor is legacy. Libevdev will thus track the key state in a bitmap.
>
> Advantages
> * passes test suite
> * works ok on my experimental libevdev - weston port including seat shared
> state, alt tab, modifiers and syn drop
Would be nice to see this code to actually understand why you want
this. I don't think this is needed, but maybe you can convince us.
Cheers
David
> Disadvantages:
> * to take advantage, needs to have support on compositor side (not in upstream)
> * I've not written a new test suite tests for this
> * in compositor is responsible for free the buffers it gets from sync()
>
> Signed-off-by: Martin Minarik <minarik11 at student.fiit.stuba.sk>
>
> ---
> libevdev/libevdev-int.h | 10 +-
> libevdev/libevdev.c | 238 ++++++++++++++++++++++++++++++++++++++---------
> libevdev/libevdev.h | 93 ++++++++++++++++++
> 3 files changed, 298 insertions(+), 43 deletions(-)
>
> diff --git a/libevdev/libevdev-int.h b/libevdev/libevdev-int.h
> index ce044e3..5b0d3a3 100644
> --- a/libevdev/libevdev-int.h
> +++ b/libevdev/libevdev-int.h
> @@ -75,6 +75,10 @@ enum SyncState {
> SYNC_IN_PROGRESS,
> };
>
> +struct libevdev_keys_bitmap {
> + unsigned long *key_values;
> +};
> +
> struct libevdev {
> int fd;
> char *name;
> @@ -93,7 +97,6 @@ struct libevdev {
> unsigned long rep_bits[NLONGS(REP_CNT)]; /* convenience, always 1 */
> unsigned long ff_bits[NLONGS(FF_CNT)];
> unsigned long snd_bits[NLONGS(SND_CNT)];
> - unsigned long key_values[NLONGS(KEY_CNT)];
> unsigned long led_values[NLONGS(LED_CNT)];
> unsigned long sw_values[NLONGS(SW_CNT)];
> struct input_absinfo abs_info[ABS_CNT];
> @@ -102,6 +105,11 @@ struct libevdev {
> int current_slot;
> int rep_values[REP_CNT];
>
> + struct libevdev_external_key_values_interface *interface;
> + void * external_key_values;
> + unsigned long key_values_bit_id;
> + unsigned int key_values_id;
> +
> enum SyncState sync_state;
> enum libevdev_grab_mode grabbed;
>
> diff --git a/libevdev/libevdev.c b/libevdev/libevdev.c
> index 70680d6..7676901 100644
> --- a/libevdev/libevdev.c
> +++ b/libevdev/libevdev.c
> @@ -35,6 +35,107 @@
>
> #define MAXEVENTS 64
>
> +static inline void
> +init_event(struct libevdev *dev, struct input_event *ev, int type, int code, int value)
> +{
> + ev->time = dev->last_event_time;
> + ev->type = type;
> + ev->code = code;
> + ev->value = value;
> +}
> +
> +static void custom_callback(void *ptr, int key, int val)
> +{
> + int emit_fake_events = ptr != NULL;
> + if (emit_fake_events) {
> + struct libevdev *dev = ptr;
> + struct input_event *ev = queue_push(dev);
> + init_event(dev, ev, EV_KEY, key, val ? 1 : 0);
> + }
> +}
> +
> +static int
> +sync_external_key_state(struct libevdev *dev, int emit_fake_events)
> +{
> + int rc;
> + const int s = ((KEY_CNT + 7) / 8);
> + unsigned long *keystate = malloc(s);
> + void *ptr = emit_fake_events ? (void *) dev : NULL;
> +
> + if (keystate == NULL)
> + return -1;
> +
> + rc = ioctl(dev->fd, EVIOCGKEY(s), keystate);
> + if (rc < 0)
> + goto out;
> +
> + dev->interface->sync(dev->external_key_values,
> + dev->key_values_bit_id, dev->key_values_id,
> + &keystate[0], &keystate[NLONGS(KEY_MAX)],
> + ptr, &custom_callback);
> +
> + rc = 0;
> +out:
> + return rc ? -errno : 0;
> +}
> +
> +static int bitmap_keyboard_keys_get_update(void *external, unsigned long bit_id, unsigned int id, uint32_t key, int val)
> +{
> + struct libevdev_keys_bitmap *bmp = external;
> + int i = bit_is_set(bmp->key_values, key);
> + set_bit_state(bmp->key_values, key, val);
> + return -(i == val);
> +}
> +
> +static int bitmap_keyboard_keys_get(void *external, unsigned long bit_id, unsigned int id, uint32_t key)
> +{
> + struct libevdev_keys_bitmap *bmp = external;
> + return bit_is_set(bmp->key_values, key);
> +}
> +
> +static void bitmap_keyboard_keys_sync(void *external, unsigned long bit_id, unsigned int id, unsigned long *buf, unsigned long *buf_end, void *ptr, void (*callback)(void *ptr, int key, int val))
> +{
> + unsigned int i;
> + struct libevdev_keys_bitmap *bmp = external;
> +
> + for (i = 0; i < KEY_CNT; i++) {
> + int old, new;
> + old = bit_is_set(bmp->key_values, i);
> + new = bit_is_set(buf, i);
> + if (old ^ new) {
> + callback(ptr, i, new);
> + }
> + }
> +
> + /* zero-copy */
> + free(bmp->key_values);
> + bmp->key_values = buf;
> +}
> +
> +static void bitmap_keyboard_keys_deactivate(void *external, unsigned long int bit_id, unsigned int id)
> +{
> +}
> +static int bitmap_keyboard_keys_activate(void *external, unsigned long int *bit_id, unsigned int *id)
> +{
> + *bit_id = 1;
> + *id = 0;
> + return 0;
> +}
> +
> +
> +struct libevdev_external_key_values_interface bitmap_state_interface = {
> + NULL,
> + NULL,
> + &bitmap_keyboard_keys_get_update,
> + &bitmap_keyboard_keys_get,
> + NULL,
> + NULL,
> + NULL,
> + &bitmap_keyboard_keys_sync,
> + &bitmap_keyboard_keys_deactivate,
> + &bitmap_keyboard_keys_activate
> +};
> +
> static int sync_mt_state(struct libevdev *dev, int create_events);
>
> static int
> @@ -116,9 +217,47 @@ libevdev_new(void)
> dev->grabbed = LIBEVDEV_UNGRAB;
> dev->sync_state = SYNC_NONE;
>
> + dev->external_key_values = NULL;
> + dev->interface = NULL;
> + dev->key_values_bit_id = 0;
> + dev->key_values_id = 0;
> +
> return dev;
> }
>
> +static int
> +libevdev_init_legacy_bitmap_keys(struct libevdev* dev, int fd)
> +{
> + size_t s = (KEY_CNT + 7) / 8;
> + unsigned long *buf;
> +
> + if ((dev->interface == NULL) && (dev->external_key_values == NULL)) {
> + buf = calloc(1, s);
> +
> + if (buf != NULL) {
> + struct libevdev_keys_bitmap *wrapper =
> + malloc(sizeof(struct libevdev_keys_bitmap));
> + if (wrapper != NULL) {
> + wrapper->key_values = buf;
> +
> + if (libevdev_external_key_values_activate(dev,
> + &bitmap_state_interface, wrapper))
> + return -2;
> +
> + if (libevdev_has_event_type(dev, EV_KEY))
> + sync_external_key_state(dev, 0);
> +
> + } else {
> + free(buf);
> + return -1;
> + }
> + }
> + }
> +
> +
> + return 0;
> +}
> +
> LIBEVDEV_EXPORT int
> libevdev_new_from_fd(int fd, struct libevdev **dev)
> {
> @@ -129,6 +268,9 @@ libevdev_new_from_fd(int fd, struct libevdev **dev)
> if (!d)
> return -ENOMEM;
>
> + if (libevdev_init_legacy_bitmap_keys(d, fd))
> + return -ENOMEM;
> +
> rc = libevdev_set_fd(d, fd);
> if (rc < 0)
> libevdev_free(d);
> @@ -137,12 +279,50 @@ libevdev_new_from_fd(int fd, struct libevdev **dev)
> return rc;
> }
>
> +int libevdev_external_key_values_activate(
> + struct libevdev *dev,
> + struct libevdev_external_key_values_interface *interface,
> + void *external_structure
> +)
> +{
> + if (dev->external_key_values != NULL)
> + return -2;
> +
> + dev->interface = interface;
> + dev->external_key_values = external_structure;
> +
> + if (interface->activate(dev->external_key_values,
> + &dev->key_values_bit_id, &dev->key_values_id))
> + return -1;
> +
> + return 0;
> +}
> +
> +void libevdev_external_key_values_deactivate(struct libevdev *dev)
> +{
> + dev->interface->deactivate(dev->external_key_values,
> + dev->key_values_bit_id, dev->key_values_id);
> + dev->interface = NULL;
> + dev->external_key_values = NULL;
> + dev->key_values_bit_id = 0;
> + dev->key_values_id = 0;
> +}
> +
> LIBEVDEV_EXPORT void
> libevdev_free(struct libevdev *dev)
> {
> if (!dev)
> return;
>
> + if (dev->interface == &bitmap_state_interface) {
> +
> + struct libevdev_keys_bitmap *wrapper =
> + (struct libevdev_keys_bitmap *)
> + dev->external_key_values;
> + free(wrapper->key_values);
> + free(wrapper);
> + }
> +
> free(dev->name);
> free(dev->phys);
> free(dev->uniq);
> @@ -197,6 +377,11 @@ libevdev_set_fd(struct libevdev* dev, int fd)
> int i;
> char buf[256];
>
> + if (libevdev_init_legacy_bitmap_keys(dev, fd)) {
> + errno = ENOMEM;
> + goto out;
> + }
> +
> if (dev->fd != -1) {
> log_bug("device already initialized.\n");
> return -EBADF;
> @@ -293,10 +478,6 @@ libevdev_set_fd(struct libevdev* dev, int fd)
> if (rc < 0)
> goto out;
>
> - rc = ioctl(fd, EVIOCGKEY(sizeof(dev->key_values)), dev->key_values);
> - if (rc < 0)
> - goto out;
> -
> rc = ioctl(fd, EVIOCGLED(sizeof(dev->led_values)), dev->led_values);
> if (rc < 0)
> goto out;
> @@ -345,7 +526,7 @@ libevdev_set_fd(struct libevdev* dev, int fd)
> */
>
> out:
> - return rc ? -errno : 0;
> + return rc < 0 ? -errno : 0;
> }
>
> LIBEVDEV_EXPORT int
> @@ -354,41 +535,10 @@ libevdev_get_fd(const struct libevdev* dev)
> return dev->fd;
> }
>
> -static inline void
> -init_event(struct libevdev *dev, struct input_event *ev, int type, int code, int value)
> -{
> - ev->time = dev->last_event_time;
> - ev->type = type;
> - ev->code = code;
> - ev->value = value;
> -}
> -
> static int
> sync_key_state(struct libevdev *dev)
> {
> - int rc;
> - int i;
> - unsigned long keystate[NLONGS(KEY_CNT)] = {0};
> -
> - rc = ioctl(dev->fd, EVIOCGKEY(sizeof(keystate)), keystate);
> - if (rc < 0)
> - goto out;
> -
> - for (i = 0; i < KEY_CNT; i++) {
> - int old, new;
> - old = bit_is_set(dev->key_values, i);
> - new = bit_is_set(keystate, i);
> - if (old ^ new) {
> - struct input_event *ev = queue_push(dev);
> - init_event(dev, ev, EV_KEY, i, new ? 1 : 0);
> - }
> - }
> -
> - memcpy(dev->key_values, keystate, rc);
> -
> - rc = 0;
> -out:
> - return rc ? -errno : 0;
> + sync_external_key_state(dev, 1);
> }
>
> static int
> @@ -595,9 +745,10 @@ update_key_state(struct libevdev *dev, const struct input_event *e)
> if (e->code > KEY_MAX)
> return 1;
>
> - set_bit_state(dev->key_values, e->code, e->value != 0);
> -
> - return 0;
> + return dev->interface->get_update(dev->external_key_values,
> + dev->key_values_bit_id,
> + dev->key_values_id,
> + e->code, e->value != 0);
> }
>
> static int
> @@ -938,7 +1089,10 @@ libevdev_get_event_value(const struct libevdev *dev, unsigned int type, unsigned
>
> switch (type) {
> case EV_ABS: value = dev->abs_info[code].value; break;
> - case EV_KEY: value = bit_is_set(dev->key_values, code); break;
> + case EV_KEY: value = dev->interface->get(dev->external_key_values,
> + dev->key_values_bit_id,
> + dev->key_values_id,
> + code); break;
> case EV_LED: value = bit_is_set(dev->led_values, code); break;
> case EV_SW: value = bit_is_set(dev->sw_values, code); break;
> default:
> @@ -964,7 +1118,7 @@ libevdev_set_event_value(struct libevdev *dev, unsigned int type, unsigned int c
>
> switch(type) {
> case EV_ABS: rc = update_abs_state(dev, &e); break;
> - case EV_KEY: rc = update_key_state(dev, &e); break;
> + case EV_KEY: rc = update_key_state(dev, &e) > 0 ? -1 : 0; break;
> case EV_LED: rc = update_led_state(dev, &e); break;
> case EV_SW: rc = update_sw_state(dev, &e); break;
> default:
> diff --git a/libevdev/libevdev.h b/libevdev/libevdev.h
> index 472e1b9..b2ee424 100644
> --- a/libevdev/libevdev.h
> +++ b/libevdev/libevdev.h
> @@ -29,6 +29,7 @@ extern "C" {
>
> #include <linux/input.h>
> #include <stdarg.h>
> +#include <stdint.h>
>
> /**
> * @mainpage
> @@ -288,6 +289,61 @@ extern "C" {
> */
> struct libevdev;
>
> +/**
> + * @ingroup init
> + *
> + * Opaque external key values interface.
> + */
> +
> +struct libevdev_external_key_values_interface {
> +int (*get_reset)(void *external, unsigned long bit_id, unsigned int id, uint32_t key);
> +int (*get_set)(void *external, unsigned long bit_id, unsigned int id, uint32_t key);
> +int (*get_update)(void *external, unsigned long bit_id, unsigned int id, uint32_t key, int val);
> +
> +int (*get)(void *external, unsigned long bit_id, unsigned int id, uint32_t key);
> +
> +void (*reset)(void *external, unsigned long bit_id, unsigned int id, uint32_t key);
> +void (*set)(void *external, unsigned long bit_id, unsigned int id, uint32_t key);
> +void (*update)(void *external, unsigned long bit_id, unsigned int id, uint32_t key, int val);
> +
> +void (*sync)(void *external, unsigned long bit_id, unsigned int id, unsigned long *buf, unsigned long *buf_end, void *ptr, void (*callback)(void *ptr, int key, int val));
> +
> + /**
> + * deactivate - quit and free stored key values in an external structure
> + * @external: the pointer to the storage for key values
> + * @bit_id: access identifier
> + * @id: access identifier
> + *
> + * This destroys stored key values of the identified device inside the
> + * external structure. After this call, the access identifiers become
> + * invalid.
> + *
> + * An external structure may not be freed until all its activated libevdev
> + * devices become deactivated.
> + *
> + */
> + void (*deactivate)(void *external, unsigned long int bit_id, unsigned int id);
> + /**
> + * activate - request to store key values in an external structure
> + * @external: the pointer to the storage for key values
> + * @bit_id: access identifier
> + * @id: access identifier
> + *
> + * The libevdev will use this to get the identifiers it needs before
> + * it can store the key values in an external structure. Different libevdev
> + * devices may store their data in the same external structure, but each
> + * libevdev device uses it's own pair of identifiers to do so. These ids
> + * are used by the external structure's methods to distinguish the callers.
> + *
> + * Return values
> + * Success: 0
> + * Failure: -1
> + */
> + int (*activate)(void *external, unsigned long int *bit_id, unsigned int *id);
> +};
> +
> +
> +
> enum libevdev_read_flag {
> LIBEVDEV_READ_SYNC = 1, /**< Process data in sync mode */
> LIBEVDEV_READ_NORMAL = 2, /**< Process data in normal mode */
> @@ -340,6 +396,43 @@ int libevdev_new_from_fd(int fd, struct libevdev **dev);
> /**
> * @ingroup init
> *
> + * Start storing key values in an external structure. After this call, the
> + * internal key state is lost, and external state of this device is emptied.
> + *
> + * Return values:
> + * 0 .. success
> + * -1 .. activation failed
> + * -2 .. error: already activated
> + *
> + * @param dev The evdev device
> + * @param interface The methods used to access an external structure
> + * @param external_structure The structure to access
> + *
> + * @note This function may be called before libevdev_set_fd().
> + */
> +
> +int libevdev_external_key_values_activate(
> + struct libevdev *dev,
> + struct libevdev_external_key_values_interface *interface,
> + void *external_structure
> +);
> +
> +/**
> + * @ingroup init
> + *
> + * Clean up and free our stored data in the external structure. When all
> + * libevdev devices are deactivated, the external structure may be freed.
> + * After this call, the device state is stored internally.
> + *
> + * @param dev The evdev device
> + *
> + */
> +
> +void libevdev_external_key_values_deactivate(struct libevdev *dev);
> +
> +/**
> + * @ingroup init
> + *
> * Clean up and free the libevdev struct. After completion, the <code>struct
> * libevdev</code> is invalid and must not be used.
> *
> --
> 1.7.10.4
>
> _______________________________________________
> Input-tools mailing list
> Input-tools at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/input-tools
More information about the Input-tools
mailing list