[PATCH libinput] Introduce device groups to group logical devices together
Hans de Goede
hdegoede at redhat.com
Thu Feb 5 03:04:57 PST 2015
Hi,
On 05-02-15 05:30, Peter Hutterer wrote:
> Devices like Wacom tablets have multiple event nodes (touch, pad and stylus).
> This requires some logical grouping, e.g. setting an Intuos 5 tablet
> left-handed effectively turns it upside down. That then applies to both the
> stylus and the touch device.
>
> Merging the devices into one struct libinput_device is not feasable, it
> complicates the API for little benefit. A caller would still need access to
> all subdevices to get udev handles, etc. Some configuration options apply to
> the whole device (left-handed) but some (may) only apply to a single subdevice
> (calibration, natural scrolling).
>
> Addressing this would make the libinput API unwieldly and hard to use.
>
> Instead, add a device group concept. Each device is a member of a device
> group - a singleton for most devices. Wacom tablets will have a single group
> across multiple devices, allowing the caller to associate the devices together
> if needed.
>
> The API is intentionally very simple and requires the caller to keep track of
> groups and which/how many devices are in it. The caller has more powerful
> libraries available to do that than we have.
>
> This patch does not address the actual merging of devices into the same
> device group, it simply creates a new group for each new device.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> Many thanks to Jonas, Hans, Benjamin for making me realise that merged
> device won't work.
Thanks for your work on this, much better then we had before. And the proposed
code looks good to:
Reviewed-by: Hans de Goede <hdegoede at redhat.com>
Regards,
Hans
>
> Also, this obsoletes the patchset for capability events as well as the
> "get_udev_devices()" patch
>
> src/evdev.c | 10 ++++++
> src/libinput-private.h | 13 +++++++
> src/libinput.c | 64 +++++++++++++++++++++++++++++++++
> src/libinput.h | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++
> src/libinput.sym | 5 +++
> test/device.c | 51 +++++++++++++++++++++++++++
> 6 files changed, 239 insertions(+)
>
> diff --git a/src/evdev.c b/src/evdev.c
> index 6e318dc..6024d38 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -1572,6 +1572,7 @@ evdev_device_create(struct libinput_seat *seat,
> int fd;
> int unhandled_device = 0;
> const char *devnode = udev_device_get_devnode(udev_device);
> + struct libinput_device_group *group;
>
> /* Use non-blocking mode so that we can loop on read on
> * evdev_device_data() until all events on the fd are
> @@ -1642,6 +1643,12 @@ evdev_device_create(struct libinput_seat *seat,
> if (!device->source)
> goto err;
>
> + group = libinput_device_group_create();
> + if (!group)
> + goto err;
> + libinput_device_set_device_group(&device->base, group);
> + libinput_device_group_unref(group);
> +
> list_insert(seat->devices_list.prev, &device->base.link);
>
> evdev_tag_device(device);
> @@ -2123,6 +2130,9 @@ evdev_device_destroy(struct evdev_device *device)
> if (dispatch)
> dispatch->interface->destroy(dispatch);
>
> + if (device->base.group)
> + libinput_device_group_unref(device->base.group);
> +
> filter_destroy(device->pointer.filter);
> libinput_seat_unref(device->base.seat);
> libevdev_free(device->evdev);
> diff --git a/src/libinput-private.h b/src/libinput-private.h
> index b938ed0..23f66e4 100644
> --- a/src/libinput-private.h
> +++ b/src/libinput-private.h
> @@ -165,8 +165,14 @@ struct libinput_device_config {
> struct libinput_device_config_click_method *click_method;
> };
>
> +struct libinput_device_group {
> + int refcount;
> + void *user_data;
> +};
> +
> struct libinput_device {
> struct libinput_seat *seat;
> + struct libinput_device_group *group;
> struct list link;
> struct list event_listeners;
> void *user_data;
> @@ -240,6 +246,13 @@ void
> libinput_device_init(struct libinput_device *device,
> struct libinput_seat *seat);
>
> +struct libinput_device_group *
> +libinput_device_group_create(void);
> +
> +void
> +libinput_device_set_device_group(struct libinput_device *device,
> + struct libinput_device_group *group);
> +
> void
> libinput_device_add_event_listener(struct libinput_device *device,
> struct libinput_event_listener *listener,
> diff --git a/src/libinput.c b/src/libinput.c
> index 7456b90..76ecab1 100644
> --- a/src/libinput.c
> +++ b/src/libinput.c
> @@ -1277,6 +1277,12 @@ libinput_device_get_context(struct libinput_device *device)
> return libinput_seat_get_context(device->seat);
> }
>
> +LIBINPUT_EXPORT struct libinput_device_group *
> +libinput_device_get_device_group(struct libinput_device *device)
> +{
> + return device->group;
> +}
> +
> LIBINPUT_EXPORT const char *
> libinput_device_get_sysname(struct libinput_device *device)
> {
> @@ -1387,6 +1393,64 @@ libinput_event_touch_get_base_event(struct libinput_event_touch *event)
> return &event->base;
> }
>
> +LIBINPUT_EXPORT struct libinput_device_group *
> +libinput_device_group_ref(struct libinput_device_group *group)
> +{
> + group->refcount++;
> + return group;
> +}
> +
> +struct libinput_device_group *
> +libinput_device_group_create(void)
> +{
> + struct libinput_device_group *group;
> +
> + group = zalloc(sizeof *group);
> + if (group)
> + group->refcount = 1;
> + return group;
> +}
> +
> +void
> +libinput_device_set_device_group(struct libinput_device *device,
> + struct libinput_device_group *group)
> +{
> + device->group = group;
> + libinput_device_group_ref(group);
> +}
> +
> +static void
> +libinput_device_group_destroy(struct libinput_device_group *group)
> +{
> + free(group);
> +}
> +
> +LIBINPUT_EXPORT struct libinput_device_group *
> +libinput_device_group_unref(struct libinput_device_group *group)
> +{
> + assert(group->refcount > 0);
> + group->refcount--;
> + if (group->refcount == 0) {
> + libinput_device_group_destroy(group);
> + return NULL;
> + } else {
> + return group;
> + }
> +}
> +
> +LIBINPUT_EXPORT void
> +libinput_device_group_set_user_data(struct libinput_device_group *group,
> + void *user_data)
> +{
> + group->user_data = user_data;
> +}
> +
> +LIBINPUT_EXPORT void *
> +libinput_device_group_get_user_data(struct libinput_device_group *group)
> +{
> + return group->user_data;
> +}
> +
> LIBINPUT_EXPORT const char *
> libinput_config_status_to_str(enum libinput_config_status status)
> {
> diff --git a/src/libinput.h b/src/libinput.h
> index c986bfe..d5d12f0 100644
> --- a/src/libinput.h
> +++ b/src/libinput.h
> @@ -195,6 +195,16 @@ struct libinput;
> struct libinput_device;
>
> /**
> + * @ingroup device
> + * @struct libinput_device_group
> + *
> + * A base handle for accessing libinput device groups. This struct is
> + * refcounted, use libinput_device_group_ref() and
> + * libinput_device_group_unref().
> + */
> +struct libinput_device_group;
> +
> +/**
> * @ingroup seat
> * @struct libinput_seat
> *
> @@ -1409,6 +1419,35 @@ libinput_device_get_context(struct libinput_device *device);
> /**
> * @ingroup device
> *
> + * Get the device group this device is assigned to. Some physical
> + * devices like graphics tablets are represented by multiple kernel
> + * devices and thus by multiple struct libinput_device.
> + *
> + * libinput assigns these devices to the same libinput_device_group
> + * allowing the caller to identify such devices and adjust configuration
> + * settings accordingly. For example, setting a tablet to left-handed often
> + * means turning it upside down. A touch device on the same tablet would
> + * need to be turned upside down too to work correctly.
> + *
> + * All devices are part of a device group though for most devices the group
> + * will be a singleton. A device is assigned to a device group on @ref
> + * LIBINPUT_EVENT_DEVICE_ADDED and removed from that group on @ref
> + * LIBINPUT_EVENT_DEVICE_REMOVED. It is up to the caller to track how many
> + * devices are in each device group.
> + *
> + * The returned device group is not refcounted and may become invalid after
> + * the next call to libinput. Use libinput_device_group_ref() and
> + * libinput_device_group_unref() to continue using the handle outside of the
> + * immediate scope.
> + *
> + * @return The device group this device belongs to
> + */
> +struct libinput_device_group *
> +libinput_device_get_device_group(struct libinput_device *device);
> +
> +/**
> + * @ingroup device
> + *
> * Get the system name of the device.
> *
> * To get the descriptive device name, use libinput_device_get_name().
> @@ -1594,6 +1633,63 @@ int
> libinput_device_has_button(struct libinput_device *device, uint32_t code);
>
> /**
> + * @ingroup device
> + *
> + * Increase the refcount of the device group. A device group will be freed
> + * whenever the refcount reaches 0. This may happen during dispatch if all
> + * devices of this group were removed from the system. A caller must ensure
> + * to reference the device group correctly to avoid dangling pointers.
> + *
> + * @param group A previously obtained device group
> + * @return The passed device group
> + */
> +struct libinput_device_group *
> +libinput_device_group_ref(struct libinput_device_group *group);
> +
> +/**
> + * @ingroup device
> + *
> + * Decrease the refcount of the device group. A device group will be freed
> + * whenever the refcount reaches 0. This may happen during dispatch if all
> + * devices of this group were removed from the system. A caller must ensure
> + * to reference the device group correctly to avoid dangling pointers.
> + *
> + * @param group A previously obtained device group
> + * @return NULL if the device group was destroyed, otherwise the passed
> + * device group
> + */
> +struct libinput_device_group *
> +libinput_device_group_unref(struct libinput_device_group *group);
> +
> +/**
> + * @ingroup device
> + *
> + * Set caller-specific data associated with this device group. libinput does
> + * not manage, look at, or modify this data. The caller must ensure the
> + * data is valid.
> + *
> + * @param group A previously obtained device group
> + * @param user_data Caller-specific data pointer
> + * @see libinput_device_group_get_user_data
> + */
> +void
> +libinput_device_group_set_user_data(struct libinput_device_group *group,
> + void *user_data);
> +
> +/**
> + * @ingroup device
> + *
> + * Get the caller-specific data associated with this input device group, if
> + * any.
> + *
> + * @param group A previously obtained group
> + * @return Caller-specific data pointer or NULL if none was set
> + * @see libinput_device_group_set_user_data
> + */
> +void *
> +libinput_device_group_get_user_data(struct libinput_device_group *group);
> +
> +/**
> * @defgroup config Device configuration
> *
> * Enable, disable, change and/or check for device-specific features. For
> diff --git a/src/libinput.sym b/src/libinput.sym
> index 77fccb8..535ff9b 100644
> --- a/src/libinput.sym
> +++ b/src/libinput.sym
> @@ -125,4 +125,9 @@ global:
> libinput_device_config_click_get_method;
> libinput_device_config_click_get_methods;
> libinput_device_config_click_set_method;
> + libinput_device_get_device_group;
> + libinput_device_group_get_user_data;
> + libinput_device_group_ref;
> + libinput_device_group_set_user_data;
> + libinput_device_group_unref;
> } LIBINPUT_0.8.0;
> diff --git a/test/device.c b/test/device.c
> index 76486f0..e72ef19 100644
> --- a/test/device.c
> +++ b/test/device.c
> @@ -661,6 +661,54 @@ START_TEST(device_context)
> }
> END_TEST
>
> +START_TEST(device_group_get)
> +{
> + struct litest_device *dev = litest_current_device();
> + struct libinput_device_group *group;
> +
> + int userdata = 10;
> +
> + group = libinput_device_get_device_group(dev->libinput_device);
> + ck_assert_notnull(group);
> +
> + libinput_device_group_ref(group);
> +
> + libinput_device_group_set_user_data(group, &userdata);
> + ck_assert_ptr_eq(&userdata,
> + libinput_device_group_get_user_data(group));
> +
> + libinput_device_group_unref(group);
> +}
> +END_TEST
> +
> +START_TEST(device_group_ref)
> +{
> + struct libinput *li = litest_create_context();
> + struct litest_device *dev = litest_add_device(li,
> + LITEST_MOUSE);
> + struct libinput_device *device = dev->libinput_device;
> + struct libinput_device_group *group;
> +
> + group = libinput_device_get_device_group(device);
> + ck_assert_notnull(group);
> + libinput_device_group_ref(group);
> +
> + libinput_device_ref(device);
> + litest_drain_events(li);
> + litest_delete_device(dev);
> + litest_drain_events(li);
> +
> + /* make sure the device is dead but the group is still around */
> + ck_assert(libinput_device_unref(device) == NULL);
> +
> + libinput_device_group_ref(group);
> + ck_assert_notnull(libinput_device_group_unref(group));
> + ck_assert(libinput_device_group_unref(group) == NULL);
> +
> + libinput_unref(li);
> +}
> +END_TEST
> +
> int main (int argc, char **argv)
> {
> litest_add("device:sendevents", device_sendevents_config, LITEST_ANY, LITEST_TOUCHPAD);
> @@ -686,5 +734,8 @@ int main (int argc, char **argv)
>
> litest_add("device:udev", device_get_udev_handle, LITEST_ANY, LITEST_ANY);
>
> + litest_add("device:group", device_group_get, LITEST_ANY, LITEST_ANY);
> + litest_add_no_device("device:group", device_group_ref);
> +
> return litest_run(argc, argv);
> }
>
More information about the wayland-devel
mailing list