[PATCH xf86-input-libinput 3/3] Split mixed pointer/keyboard devices into two separate X devices

Peter Hutterer peter.hutterer at who-t.net
Wed Nov 11 21:26:14 PST 2015


On Thu, Nov 12, 2015 at 10:27:05AM +1000, Peter Hutterer wrote:
> The server struggles with devices that are both, the protocol (especially XI2)
> requires a fairly strict separation of pointer vs keyboard devices. Though the
> server has a couple of hacks to route events correctly, mixed
> devices still experience bugs like [1].
> 
> Instead of advertising the device as a single mixed device, split the device
> into two X devices, one with only a pointer/touch component, one with only a
> keyboard component. This ensures that the device is effectively attached to
> both the VCP and the VCK, something the XI2 protocol doesn't really allow.
> 
> This patch drops the keyboard capability on a mixed device, duplicates the
> input options and attributes and queues a NewInputDeviceRequest call. The new
> device only has the keyboard capability but is otherwise unchanged.  This
> wacom driver has used this approach for years.
> 
> Note that (unlike the wacom driver) we leave the device as-is otherwise.
> libinput allows us to add the same device node twice as two separate devices,
> so we do that and merely filter out the events based on our capabilities. That
> means both devices handle all events, but only pass on the right ones to the
> server.

I'm withdrawing the patch. The above is true for the non-logind case, but if
logind handles the fds we try to use the same fd twice and that fails to add
the second device, and would cause dropped events otherwise.

Cheers,
   Peter

> 
> Since the device is effectively duplicate, expecially the "config_info"
> InputOption it will be removed as the original device is removed.
> 
> The WorkProc is necessary to avoid inconsistent state, the server doesn't
> handle a NewInputDeviceRequest during PreInit well.
> 
> [1] https://bugs.freedesktop.org/show_bug.cgi?id=49950
> 
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
>  src/xf86libinput.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 103 insertions(+), 6 deletions(-)
> 
> diff --git a/src/xf86libinput.c b/src/xf86libinput.c
> index 3c00879..b27721f 100644
> --- a/src/xf86libinput.c
> +++ b/src/xf86libinput.c
> @@ -676,6 +676,9 @@ xf86libinput_handle_motion(InputInfoPtr pInfo, struct libinput_event_pointer *ev
>  	ValuatorMask *mask = driver_data->valuators;
>  	double x, y;
>  
> +	if ((driver_data->capabilities & CAP_POINTER) == 0)
> +		return;
> +
>  	x = libinput_event_pointer_get_dx(event);
>  	y = libinput_event_pointer_get_dy(event);
>  
> @@ -713,6 +716,9 @@ xf86libinput_handle_absmotion(InputInfoPtr pInfo, struct libinput_event_pointer
>  		return;
>  	}
>  
> +	if ((driver_data->capabilities & CAP_POINTER) == 0)
> +		return;
> +
>  	x = libinput_event_pointer_get_absolute_x_transformed(event, TOUCH_AXIS_MAX);
>  	y = libinput_event_pointer_get_absolute_y_transformed(event, TOUCH_AXIS_MAX);
>  
> @@ -731,6 +737,9 @@ xf86libinput_handle_button(InputInfoPtr pInfo, struct libinput_event_pointer *ev
>  	int button;
>  	int is_press;
>  
> +	if ((driver_data->capabilities & CAP_POINTER) == 0)
> +		return;
> +
>  	button = btn_linux2xorg(libinput_event_pointer_get_button(event));
>  	is_press = (libinput_event_pointer_get_button_state(event) == LIBINPUT_BUTTON_STATE_PRESSED);
>  
> @@ -745,9 +754,13 @@ static void
>  xf86libinput_handle_key(InputInfoPtr pInfo, struct libinput_event_keyboard *event)
>  {
>  	DeviceIntPtr dev = pInfo->dev;
> +	struct xf86libinput *driver_data = pInfo->private;
>  	int is_press;
>  	int key = libinput_event_keyboard_get_key(event);
>  
> +	if ((driver_data->capabilities & CAP_KEYBOARD) == 0)
> +		return;
> +
>  	key += XORG_KEYCODE_OFFSET;
>  
>  	is_press = (libinput_event_keyboard_get_key_state(event) == LIBINPUT_KEY_STATE_PRESSED);
> @@ -764,6 +777,9 @@ xf86libinput_handle_axis(InputInfoPtr pInfo, struct libinput_event_pointer *even
>  	enum libinput_pointer_axis axis;
>  	enum libinput_pointer_axis_source source;
>  
> +	if ((driver_data->capabilities & CAP_POINTER) == 0)
> +		return;
> +
>  	valuator_mask_zero(mask);
>  
>  	source = libinput_event_pointer_get_axis_source(event);
> @@ -822,6 +838,9 @@ xf86libinput_handle_touch(InputInfoPtr pInfo,
>  	static unsigned int next_touchid;
>  	static unsigned int touchids[TOUCH_MAX_SLOTS] = {0};
>  
> +	if ((driver_data->capabilities & CAP_TOUCH) == 0)
> +		return;
> +
>  	slot = libinput_event_touch_get_slot(event);
>  
>  	switch (event_type) {
> @@ -1504,6 +1523,72 @@ xf86libinput_get_type_name(struct libinput_device *device,
>  	return type_name;
>  }
>  
> +struct xf86libinput_hotplug_info {
> +	InputAttributes *attrs;
> +	InputOption *input_options;
> +};
> +
> +static Bool
> +xf86libinput_hotplug_device(ClientPtr client, pointer closure)
> +{
> +	struct xf86libinput_hotplug_info *hotplug = closure;
> +	DeviceIntPtr unused;
> +
> +	NewInputDeviceRequest(hotplug->input_options,
> +			      hotplug->attrs,
> +			      &unused);
> +
> +	input_option_free_list(&hotplug->input_options);
> +	FreeInputAttributes(hotplug->attrs);
> +	free(hotplug);
> +
> +	return TRUE;
> +}
> +
> +static void
> +xf86libinput_create_keyboard_subdevice(InputInfoPtr pInfo)
> +{
> +	struct xf86libinput_hotplug_info *hotplug;
> +	InputOption *iopts = NULL;
> +	XF86OptionPtr options, o;
> +
> +	/* need convert from one option list to the other. woohoo. */
> +	options = xf86OptionListDuplicate(pInfo->options);
> +	options = xf86ReplaceStrOption(options, "_source", "_driver/libinput");
> +	options = xf86ReplaceStrOption(options, "_libinput/caps", "keyboard");
> +
> +	o = options;
> +	while (o) {
> +		iopts = input_option_new(iopts,
> +					 xf86OptionName(o),
> +					 xf86OptionValue(o));
> +		o = xf86NextOption(o);
> +	}
> +	xf86OptionListFree(options);
> +
> +	hotplug = calloc(1, sizeof(*hotplug));
> +	if (!hotplug)
> +		return;
> +
> +	hotplug->input_options = iopts;
> +	hotplug->attrs = DuplicateInputAttributes(pInfo->attrs);
> +
> +	QueueWorkProc(xf86libinput_hotplug_device, serverClient, hotplug);
> +}
> +
> +static BOOL
> +xf86libinput_is_subdevice(InputInfoPtr pInfo)
> +{
> +	char *source;
> +	BOOL is_subdevice;
> +
> +	source = xf86SetStrOption(pInfo->options, "_source", NULL);
> +	is_subdevice = source && strcmp(source, "_driver/libinput") == 0;
> +	free(source);
> +
> +	return is_subdevice;
> +}
> +
>  static int
>  xf86libinput_pre_init(InputDriverPtr drv,
>  		      InputInfoPtr pInfo,
> @@ -1574,12 +1659,16 @@ xf86libinput_pre_init(InputDriverPtr drv,
>  	driver_data->path = path;
>  	driver_data->device = device;
>  
> -	if (libinput_device_has_capability(device, LIBINPUT_DEVICE_CAP_POINTER))
> -		driver_data->capabilities |= CAP_POINTER;
> -	if (libinput_device_has_capability(device, LIBINPUT_DEVICE_CAP_KEYBOARD))
> -		driver_data->capabilities |= CAP_KEYBOARD;
> -	if (libinput_device_has_capability(device, LIBINPUT_DEVICE_CAP_TOUCH))
> -		driver_data->capabilities |= CAP_TOUCH;
> +	if (xf86libinput_is_subdevice(pInfo)) {
> +		driver_data->capabilities = CAP_KEYBOARD;
> +	} else {
> +		if (libinput_device_has_capability(device, LIBINPUT_DEVICE_CAP_POINTER))
> +			driver_data->capabilities |= CAP_POINTER;
> +		if (libinput_device_has_capability(device, LIBINPUT_DEVICE_CAP_KEYBOARD))
> +			driver_data->capabilities |= CAP_KEYBOARD;
> +		if (libinput_device_has_capability(device, LIBINPUT_DEVICE_CAP_TOUCH))
> +			driver_data->capabilities |= CAP_TOUCH;
> +	}
>  
>  	/* Disable acceleration in the server, libinput does it for us */
>  	pInfo->options = xf86ReplaceIntOption(pInfo->options, "AccelerationProfile", -1);
> @@ -1587,6 +1676,14 @@ xf86libinput_pre_init(InputDriverPtr drv,
>  
>  	xf86libinput_parse_options(pInfo, driver_data, device);
>  
> +	/* Device is both keyboard and pointer. Drop the keyboard cap from
> +	 * this device, create a separate device instead */
> +	if (driver_data->capabilities & CAP_KEYBOARD &&
> +	    driver_data->capabilities & (CAP_POINTER|CAP_TOUCH)) {
> +		driver_data->capabilities &= ~CAP_KEYBOARD;
> +		xf86libinput_create_keyboard_subdevice(pInfo);
> +	}
> +
>  	pInfo->type_name = xf86libinput_get_type_name(device, driver_data);
>  
>  	return Success;
> -- 
> 2.4.3
> 


More information about the xorg-devel mailing list