Re: [PATCH xf86-input-libinput] Support for rel+abs composite devices on the same node

[Peter Hutterer]

On Sat, Mar 11, 2017 at 12:26:38PM +0100, Gergely Nagy wrote:
> Largely based on work by Peter Hutterer, this patch makes composite relative +
> absolute devices work with the driver. Such devices are anything that presents
> itself as both a mouse, and - for example - a touch screen, like the 
> Keyboardio
> Model 01 (or anything that uses Nico Hood's HID library for Arduino).
> 
> If a device has both capabilities present, we split them into two separate
> devices for X (see xf86libinput_pre_init()), like we do with the composite
> keyboard + mouse devices. Then, we route absolute events to the absolute
> subdevice (see xf86libinput_pick_device()). The rest of the patch is mostly
> small updates to support the newly introduced CAP_POINTER_ABSOLUTE flag.
> 
> There's one missing thing, however: event routing for buttons. At the moment,
> button routing is simply not done, but in practice, this does not seem to have
> any ill side effects.

Most likely because the buttons are merged in the virtual core pointer in X.
But to an XI2 application they still look different which may cause some
problems down the road.

I think the best option here is to route BTN_LEFT to exclusing BTN_JOYSTICK
through the relative device and all others through the absolute device. Any
device that needs special routing can be fixed as it comes.

Cheers,
   Peter

> 
> https://bugs.freedesktop.org/show_bug.cgi?id=99914
> 
> Signed-off-by: Gergely Nagy 
> ---
>  src/xf86libinput.c | 81 
> ++
>  1 file changed, 58 insertions(+), 23 deletions(-)
> 
> diff --git a/src/xf86libinput.c b/src/xf86libinput.c
> index 888c8f2..a099c5b 100644
> --- a/src/xf86libinput.c
> +++ b/src/xf86libinput.c
> @@ -87,6 +87,7 @@
>  #define CAP_TABLET   0x8
>  #define CAP_TABLET_TOOL  0x10
>  #define CAP_TABLET_PAD   0x20
> +#define CAP_POINTER_ABSOLUTE 0x40
>  
>  struct xf86libinput_driver {
>   struct libinput *libinput;
> @@ -805,6 +806,20 @@ xf86libinput_init_pointer_absolute(InputInfoPtr pInfo)
>   Atom btnlabels[MAX_BUTTONS];
>   Atom axislabels[TOUCHPAD_NUM_AXES];
>  
> + /* We always initialize rel as parent on absrel devices */
> + if (xf86libinput_is_subdevice(pInfo)) {
> + /*
> +  * FIXME: well, we can't really know which buttons belong to
> +  * which device here, but adding it to both doesn't make
> +  * sense either. Options are: assume LMR is on rel, BTN_0
> +  * and friends on absolute. That's not always correct but
> +  * that's as best as we can do.
> +  *
> +  * FIXME: event routing for buttons isn't set up correctly
> +  * yet.
> +  */
> + }
> +
>   for (i = BTN_BACK; i >= BTN_SIDE; i--) {
>   if (libinput_device_pointer_has_button(device, i)) {
>   nbuttons += i - BTN_SIDE + 1;
> @@ -1179,13 +1194,10 @@ xf86libinput_init(DeviceIntPtr dev)
>  
>   if (driver_data->capabilities & CAP_KEYBOARD)
>   xf86libinput_init_keyboard(pInfo);
> - if (driver_data->capabilities & CAP_POINTER) {
> - if (libinput_device_config_calibration_has_matrix(device) &&
> - !libinput_device_config_accel_is_available(device))
> - xf86libinput_init_pointer_absolute(pInfo);
> - else
> - xf86libinput_init_pointer(pInfo);
> - }
> + if (driver_data->capabilities & CAP_POINTER)
> + xf86libinput_init_pointer(pInfo);
> + if (driver_data->capabilities & CAP_POINTER_ABSOLUTE)
> + xf86libinput_init_pointer_absolute(pInfo);
>   if (driver_data->capabilities & CAP_TOUCH)
>   xf86libinput_init_touch(pInfo);
>   if (driver_data->capabilities & CAP_TABLET_TOOL)
> @@ -1347,7 +1359,7 @@ xf86libinput_handle_absmotion(InputInfoPtr pInfo, 
> struct libinput_event_pointer
>   return;
>   }
>  
> - if ((driver_data->capabilities & CAP_POINTER) == 0)
> + if ((driver_data->capabilities & CAP_POINTER_ABSOLUTE) == 0)
>   return;
>  
>   x = libinput_event_pointer_get_absolute_x_transformed(event, 
> TOUCH_AXIS_MAX);
> @@ -1368,7 +1380,7 @@ xf86libinput_handle_button(InputInfoPtr pInfo, struct 
> libinput_event_pointer *ev
>   int button;
>   int is_press;
>  
> - if ((driver_data->capabilities & CAP_POINTER) == 0)
> + if ((driver_data->capabilities & (CAP_POINTER|CAP_POINTER_ABSOLUTE)) == 
> 0)
>   return;
>  
>   button = btn_linux2xorg(libinput_event_pointer_get_button(event));
> @@ -1493,7 +1505,7 @@ xf86libinput_handle_axis(InputInfoPtr pInfo, struct 
> libinput_event_pointer *even
>   double value;
>   enum libinput_pointer_axis_source source;
>  
> - if ((driver_data->capabilities & CAP_POINTER) == 0)
> + if ((driver_data->capabilities & (CAP_POINTER|CAP_POINTER_ABSOLUTE)) == 
> 

[PATCH xf86-input-libinput] Support for rel+abs composite devices on the same node

[Gergely Nagy]

Largely based on work by Peter Hutterer, this patch makes composite relative +
absolute devices work with the driver. Such devices are anything that presents
itself as both a mouse, and - for example - a touch screen, like the Keyboardio
Model 01 (or anything that uses Nico Hood's HID library for Arduino).

If a device has both capabilities present, we split them into two separate
devices for X (see xf86libinput_pre_init()), like we do with the composite
keyboard + mouse devices. Then, we route absolute events to the absolute
subdevice (see xf86libinput_pick_device()). The rest of the patch is mostly
small updates to support the newly introduced CAP_POINTER_ABSOLUTE flag.

There's one missing thing, however: event routing for buttons. At the moment,
button routing is simply not done, but in practice, this does not seem to have
any ill side effects.

https://bugs.freedesktop.org/show_bug.cgi?id=99914

Signed-off-by: Gergely Nagy 
---
 src/xf86libinput.c | 81 ++
 1 file changed, 58 insertions(+), 23 deletions(-)

diff --git a/src/xf86libinput.c b/src/xf86libinput.c
index 888c8f2..a099c5b 100644
--- a/src/xf86libinput.c
+++ b/src/xf86libinput.c
@@ -87,6 +87,7 @@
 #define CAP_TABLET 0x8
 #define CAP_TABLET_TOOL0x10
 #define CAP_TABLET_PAD 0x20
+#define CAP_POINTER_ABSOLUTE   0x40
 
 struct xf86libinput_driver {
struct libinput *libinput;
@@ -805,6 +806,20 @@ xf86libinput_init_pointer_absolute(InputInfoPtr pInfo)
Atom btnlabels[MAX_BUTTONS];
Atom axislabels[TOUCHPAD_NUM_AXES];
 
+   /* We always initialize rel as parent on absrel devices */
+   if (xf86libinput_is_subdevice(pInfo)) {
+   /*
+* FIXME: well, we can't really know which buttons belong to
+* which device here, but adding it to both doesn't make
+* sense either. Options are: assume LMR is on rel, BTN_0
+* and friends on absolute. That's not always correct but
+* that's as best as we can do.
+*
+* FIXME: event routing for buttons isn't set up correctly
+* yet.
+*/
+   }
+
for (i = BTN_BACK; i >= BTN_SIDE; i--) {
if (libinput_device_pointer_has_button(device, i)) {
nbuttons += i - BTN_SIDE + 1;
@@ -1179,13 +1194,10 @@ xf86libinput_init(DeviceIntPtr dev)
 
if (driver_data->capabilities & CAP_KEYBOARD)
xf86libinput_init_keyboard(pInfo);
-   if (driver_data->capabilities & CAP_POINTER) {
-   if (libinput_device_config_calibration_has_matrix(device) &&
-   !libinput_device_config_accel_is_available(device))
-   xf86libinput_init_pointer_absolute(pInfo);
-   else
-   xf86libinput_init_pointer(pInfo);
-   }
+   if (driver_data->capabilities & CAP_POINTER)
+   xf86libinput_init_pointer(pInfo);
+   if (driver_data->capabilities & CAP_POINTER_ABSOLUTE)
+   xf86libinput_init_pointer_absolute(pInfo);
if (driver_data->capabilities & CAP_TOUCH)
xf86libinput_init_touch(pInfo);
if (driver_data->capabilities & CAP_TABLET_TOOL)
@@ -1347,7 +1359,7 @@ xf86libinput_handle_absmotion(InputInfoPtr pInfo, struct 
libinput_event_pointer
return;
}
 
-   if ((driver_data->capabilities & CAP_POINTER) == 0)
+   if ((driver_data->capabilities & CAP_POINTER_ABSOLUTE) == 0)
return;
 
x = libinput_event_pointer_get_absolute_x_transformed(event, 
TOUCH_AXIS_MAX);
@@ -1368,7 +1380,7 @@ xf86libinput_handle_button(InputInfoPtr pInfo, struct 
libinput_event_pointer *ev
int button;
int is_press;
 
-   if ((driver_data->capabilities & CAP_POINTER) == 0)
+   if ((driver_data->capabilities & (CAP_POINTER|CAP_POINTER_ABSOLUTE)) == 
0)
return;
 
button = btn_linux2xorg(libinput_event_pointer_get_button(event));
@@ -1493,7 +1505,7 @@ xf86libinput_handle_axis(InputInfoPtr pInfo, struct 
libinput_event_pointer *even
double value;
enum libinput_pointer_axis_source source;
 
-   if ((driver_data->capabilities & CAP_POINTER) == 0)
+   if ((driver_data->capabilities & (CAP_POINTER|CAP_POINTER_ABSOLUTE)) == 
0)
return;
 
valuator_mask_zero(mask);
@@ -1600,6 +1612,9 @@ xf86libinput_pick_device(struct xf86libinput_device 
*shared_device,
case LIBINPUT_EVENT_TABLET_TOOL_TIP:
needed_cap = CAP_TABLET_TOOL;
break;
+   case LIBINPUT_EVENT_POINTER_MOTION_ABSOLUTE:
+   needed_cap = CAP_POINTER_ABSOLUTE;
+   break;
default:
needed_cap = ~CAP_KEYBOARD;
break;
@@ -2940,7 +2955,7 @@ xf86libinput_get_type_name(struct libinput_device *device,
/* now