[PATCH libinput 0/24] Tablet support

[Peter Hutterer]

Hey,

On Mon, Apr 21, 2014 at 07:11:09PM +0200, Carlos Garnacho wrote:
> Hey there!,
> 
> Here's a few patches to have libinput handle events from tablets,
> these devices are basically pointer devices, with a varying range
> of extra buttons (either stylus or "pad" buttons), and extra ABS_*
> axes. These devices also often offer information about the stylus
> in use, and its BTN_TOOL_* codes.
> 
> So I've gone for reusing and extending libinput_event_pointer, adding
> extra libinput_pointer_axis values, and adding an "axis_frame" event
> to mark the end of a set of simultaneous axis changes, and a "tool_update"
> event to mark tool changes (and delimit proximity). These features are
> only triggered if a new LIBINPUT_DEVICE_CAP_STYLUS capability is set.

thanks for all that, this is much appreciated. At this point I should
probably also point out that the GSoC project by  Stephen Chandler Paul is
to add tablet support to Wayland. 
http://www.google-melange.com/gsoc/project/details/google/gsoc2014/lyude/5685265389584384

adding support to libinput is one of the first steps here, so it's great
that some work is already being done.

> Caveats:
> 
> * Many of these devices have also tactile strips or wheels, but these are
>   unhandled so far... On the devices I've got available for testing, current 
>   kernel support seems varyingly inconsistent:
> 
>   - One device has 2 strips, which report on ABS_RX/RY (radial??). Min/max
>     are 0..4096, but the reported values are 1,2,4,8,16... So effectively
>     a log2 scale, or more graphically a bit shifting over a bunch of 0s,
>     which is somewhat more resembling to the physical action on the strip.
> 
>   - Another device has one wheel, reported through ABS_WHEEL. Even though
>     min/max are reported as [0..1023], on interaction it goes [0..71] (funky
>     range too)
> 
>   We could just forward this as-is, but seems hindering enough to do anything
>   useful with those unless that behavior is corrected.
> 
>   When supported, IMO it'd make sense to have those axes behave similar to
>   scroll axes, so the axis value increments or decrements depending on the
>   direction. I'm not sure if there would be cases where the absolute value
>   matters here?
> 
> * One thing worth noting is that axes are currently normalized, to [-1..1] 
>   for stylus tilt, and [0..1] for everything else. I remember Peter's
>   tablet wayland protocol proposal basically forwarded input_absinfo, this
>   might not be fully compatible with that, although TBH I'm unsure
>   clamping/normalization should take place so high in the stack...
> 
> * No filtering/hysteresis of coords is done yet.

I have a couple of general comments on the approach. You added AXIS_FRAME
to the pointer event sequence, and then a CAP_STYLUS. Those two are
on a different hierarchy level. In 07/24 you add a couple of axes that we
send. From a high-level this now bears the question: is this the best split?
The only real benefit you get from this is that you can re-use x/y
positions. So why not split it out completely? Have a CAP_STYLUS and
LIBINPUT_EVENT_STYLUS_... events.  This becomes interesting when you look at
those few tablets that are used as touchpads  _and_ stylus (though they
advertise on two different kernel devices, so this is rather academic).

what it does give us is more power with the events too. e.g.
libinput_event_stylus_get_tool() being always valid. It also gives us a
chance to explicitly define what e.g. BTN_TOUCH, means in a stylus context
(though the kernel is relatively consistent here anyway).

next thing but I'm not sure myself yet whether that's a good idea: having
tablet-specific events means we can pack everything into one event and skip
the AXIS_FRAME bits. which makes the tool update event superfluous and
merely a getter on the event itself.

I was about to comment on adding axis definition instead of just re-using
linux/input.h defines but I think we do get some power by defining the ones
we support. Most notably, the ABS_RX/RY mess, ABS_WHEEL etc. can be avoided
- we can forward well-defined events this way. So +1 for that from me.

Normalization: I'm not convinced. by normalising in libinput, you're taking
away the ability to use the full range. A device having a binary distance
detection capability has the same range as one with real detection. most
clients won't use it, but limiting this here removes the ability to do this
properly. and the range of 0..1 is limiting, if anything you'd want to
normalize to the maximum range possible.

This is a general problem with axes other than x/y anyway: without giving
the client a chance to detect the resolution, you're limiting what a client
can do with the data. So we need some extra device capability detection
calls.

I've got some more comments on the various patches, but I'll re-review all
patches once we've discussed the things above. Some bits I just skimmed,
especially the tests ;).

Cheers,
   Peter