[PATCH xf86-input-libinput] Handle capability events after adding a device
Hans de Goede
hdegoede at redhat.com
Tue Feb 3 00:29:08 PST 2015
Hi,
On 30-01-15 06:31, Peter Hutterer wrote:
> Needs a temporary libinput context to get all capability events without
> events from other devices interfering.
>
> This doesn't yet handle true capability changes, only the initial burst of
> events after the DEVICE_ADDED event.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
Hmm, this one does not give me a warm/fuzzy feeling. Are we sure that having
the capabilities of libinput events on the fly is the best way to go ?
It is the logical thing to do from a libinput pov, but it seems to put an
unnecessary burden on libinput users, we're likely going to see similar
problems in compositors. I would prefer to just solve this in libinput,
e.g. :
1) Add a heuristic to see if a device may have multiple evdev nodes representing
a single device.
2) If 1) is true, then wait for evdev nodes to show up for 0.5 seconds (or maybe
we can even know which evdev nodes to wait for) and do not give the device to
the libinput user until it is complete.
This is more work in libinput, but it seems much easier on libinput users, and
thus the right thing to do.
Regards,
Hans
> ---
> That's based on a patchset scheduled for libinput 0.10
>
> src/libinput.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 61 insertions(+), 7 deletions(-)
>
> diff --git a/src/libinput.c b/src/libinput.c
> index a24cbff..29a624c 100644
> --- a/src/libinput.c
> +++ b/src/libinput.c
> @@ -51,6 +51,8 @@
> #define TOUCH_MAX_SLOTS 15
> #define XORG_KEYCODE_OFFSET 8
>
> +#define AS_MASK(v) (1 << v)
> +
> /*
> libinput does not provide axis information for absolute devices, instead
> it scales into the screen dimensions provided. So we set up the axes with
> @@ -70,6 +72,7 @@ static struct xf86libinput_driver driver_context;
> struct xf86libinput {
> char *path;
> struct libinput_device *device;
> + unsigned int capabilities;
>
> int scroll_vdist;
> int scroll_hdist;
> @@ -600,15 +603,15 @@ xf86libinput_init(DeviceIntPtr dev)
>
> dev->public.on = FALSE;
>
> - if (libinput_device_has_capability(device, LIBINPUT_DEVICE_CAP_KEYBOARD))
> + if (driver_data->capabilities & AS_MASK(LIBINPUT_DEVICE_CAP_KEYBOARD))
> xf86libinput_init_keyboard(pInfo);
> - if (libinput_device_has_capability(device, LIBINPUT_DEVICE_CAP_POINTER)) {
> + if (driver_data->capabilities & AS_MASK(LIBINPUT_DEVICE_CAP_POINTER)) {
> if (libinput_device_config_calibration_has_matrix(device))
> xf86libinput_init_pointer_absolute(pInfo);
> else
> xf86libinput_init_pointer(pInfo);
> }
> - if (libinput_device_has_capability(device, LIBINPUT_DEVICE_CAP_TOUCH))
> + if (driver_data->capabilities & AS_MASK(LIBINPUT_DEVICE_CAP_TOUCH))
> xf86libinput_init_touch(pInfo);
>
> /* unref the device now, because we'll get a new ref during
> @@ -815,6 +818,8 @@ xf86libinput_handle_event(struct libinput_event *event)
> case LIBINPUT_EVENT_NONE:
> case LIBINPUT_EVENT_DEVICE_ADDED:
> case LIBINPUT_EVENT_DEVICE_REMOVED:
> + case LIBINPUT_EVENT_DEVICE_CAPABILITY_ADDED:
> + case LIBINPUT_EVENT_DEVICE_CAPABILITY_REMOVED:
> break;
> case LIBINPUT_EVENT_POINTER_MOTION_ABSOLUTE:
> xf86libinput_handle_absmotion(pInfo,
> @@ -1117,6 +1122,49 @@ xf86libinput_parse_options(InputInfoPtr pInfo,
>
> }
>
> +/* libinput sends capabilities as events after opening the device. In X we
> + need the capabilities to be done when we get to DEVICE_INIT.
> + The capability events may be intermixed with real events from other
> + devices, making handling device init a lot more complicated than it needs
> + to be. To avoid this, we create a temporary libinput context, pull the
> + capability events off the context, then ignore them in the main context.
> + */
> +static int
> +get_capabilities(const char *path)
> +{
> + int rc = -1;
> + struct libinput *libinput = NULL;
> + struct libinput_device *device;
> + struct libinput_event *event;
> + struct libinput_event_device_capability *cap;
> +
> + libinput = libinput_path_create_context(&interface,
> + &driver_context);
> + if (!libinput)
> + goto out;
> +
> +
> + device = libinput_path_add_device(libinput, path);
> + if (!device)
> + goto out;
> +
> + libinput_dispatch(libinput);
> +
> + rc = 0;
> + while ((event = libinput_get_event(libinput))) {
> + if (libinput_event_get_type(event) == LIBINPUT_EVENT_DEVICE_CAPABILITY_ADDED) {
> + cap = libinput_event_get_device_capability_event(event);
> + rc |= AS_MASK(libinput_event_device_capability_get_capability(cap));
> + }
> + libinput_event_destroy(event);
> + }
> +
> +out:
> + libinput_unref(libinput);
> +
> + return rc;
> +}
> +
> static int
> xf86libinput_pre_init(InputDriverPtr drv,
> InputInfoPtr pInfo,
> @@ -1125,6 +1173,7 @@ xf86libinput_pre_init(InputDriverPtr drv,
> struct xf86libinput *driver_data = NULL;
> struct libinput *libinput = NULL;
> struct libinput_device *device;
> + int capabilities;
> char *path;
>
> pInfo->type_name = 0;
> @@ -1170,6 +1219,12 @@ xf86libinput_pre_init(InputDriverPtr drv,
> if (use_server_fd(pInfo))
> fd_push(&driver_context, pInfo->fd, path);
>
> + capabilities = get_capabilities(path);
> + if (capabilities == -1) {
> + xf86IDrvMsg(pInfo, X_ERROR, "Failed to create a device for %s\n", path);
> + goto fail;
> + }
> +
> device = libinput_path_add_device(libinput, path);
> if (!device) {
> xf86IDrvMsg(pInfo, X_ERROR, "Failed to create a device for %s\n", path);
> @@ -1187,6 +1242,7 @@ xf86libinput_pre_init(InputDriverPtr drv,
> pInfo->private = driver_data;
> driver_data->path = path;
> driver_data->device = device;
> + driver_data->capabilities = (unsigned int)capabilities;
>
> /* Disable acceleration in the server, libinput does it for us */
> pInfo->options = xf86ReplaceIntOption(pInfo->options, "AccelerationProfile", -1);
> @@ -1197,11 +1253,9 @@ xf86libinput_pre_init(InputDriverPtr drv,
> /* now pick an actual type */
> if (libinput_device_config_tap_get_finger_count(device) > 0)
> pInfo->type_name = XI_TOUCHPAD;
> - else if (libinput_device_has_capability(device,
> - LIBINPUT_DEVICE_CAP_TOUCH))
> + else if (capabilities & AS_MASK(LIBINPUT_DEVICE_CAP_TOUCH))
> pInfo->type_name = XI_TOUCHSCREEN;
> - else if (libinput_device_has_capability(device,
> - LIBINPUT_DEVICE_CAP_POINTER))
> + else if (capabilities & AS_MASK(LIBINPUT_DEVICE_CAP_POINTER))
> pInfo->type_name = XI_MOUSE;
> else
> pInfo->type_name = XI_KEYBOARD;
>
More information about the xorg-devel
mailing list