[PATCH libinput] Introduce device groups to group logical devices together
Jonas Ådahl
jadahl at gmail.com
Thu Feb 5 03:14:54 PST 2015
On Thu, Feb 05, 2015 at 02:30:58PM +1000, 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>
This is nicer indeed.
Can be considered Reviewed-by: Jonas Ådahl <jadahl at gmail.com> as well,
well, with a comment inline addressed.
Jonas
> ---
> Many thanks to Jonas, Hans, Benjamin for making me realise that merged
> device won't work.
>
> 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.
I think it would be nice to say whether these groups are "recycled" in
any way, like our seats are. I guess it makes no sense if groups are
intended to be just dump containers, but since we have this behavior
elsewhere it is at least good to me precise about it in the API.
> + *
> + * @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);
> }
> --
> 2.1.0
>
More information about the wayland-devel
mailing list