[PATCH libinput 0/24] Tablet support

[Peter Hutterer]

On Fri, May 23, 2014 at 01:40:49AM +0200, Carlos Garnacho wrote:
> Hi,
> 
> First of all, I'm sorry I dropped the ball this long. It's great to see
> you've been doing some progress.
> 
> On jue, 2014-05-22 at 01:17 -0400, Chandler Paul wrote:
> > Hi! Sorry this took so long to write, I've been spending a lot of my
> > time recently trying to understand the libinput code and all of that
> > good stuff, and I wanted to make sure I had a decent understanding of it
> > before I actually wrote up a response.
> > 
> > On Mon, 2014-04-21 at 19:11 +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.
> > I'm with Peter on splitting these up. It seems kind of inconsistent with
> > the rest of libinput (with what I've gathered, anyway). A
> > tablet-specific event sounds interesting too, but I feel like
> > compressing all of the axis changes into. For now I'm going to work on
> > having all the axis changes reported as separate POINTER_AXIS events.
> 
> I'm not against making this a separate set of event types. I would
> advise though against attempting to compress axis changes into a single
> event, it puts certain processing burden on clients, and most of those
> not always want as much data.

can you elaborate on what the processing burdens are?

there are two options for handling compressed events in the library: one is
the approach XI2 uses, or always guaranteeing full data.
in the XI2 approach, we only forward data that is set in the event which
puts some burden on the client to calculate and keep track of the data. I
don't think that's the best aproach for libinput. Rather, we should
guarantee that all axes are always set to the current value, so most clients
could then just call get_pressure() to get the pressure, regardles of
whether that changed with that event. if needed, we can export some extra
mask to tell if a particular value was updated, but just having the full
event should take some burden away.

> Independently of this being a separate set of events, IMO the axis frame
> event is still useful for pointer events, this might help compressing
> the processing of dx/dy for scrolling in clients for example, instead of
> processing 2 events that queue the redraw of different areas, some
> unneeded in the end.

I agree, this is something that has value.

Cheers,
   Peter



> > > 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.
> > Since I'm on a deadline for this and making changes to this in the
> > kernel would take too long, I don't think I'm going to advocate for
> > changing this behavior right now. Although I do agree that eventually it
> > should be changed. Graphically, a bit moving across a field like that
> > makes sense, but I think that would be a difficult value to make
> > practical use of in an application without changing it to a simple 1-13
> > value. If I get far enough that I can start implementing support for
> > tactile strips and all those other fancy features some of these tablets
> > have I might have it convert the values for tactile strips like that to
> > something more usable by default and leave the other axes as-is. I'm
> > curious to hear Ping and Jason's opinion on this though, and what kind
> > of advantages 
> > 
> > >   - 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...
> > I'm with Peter on this actually. If the axes were used for something
> > else I might approve of normalization in libinput but I think having
> > absolute values fits more of the use cases for the extra axes found on
> > many tablets, especially since Ping said that some of Wacom's in-house
> > applications actually need these. I do think however, that maybe we
> > should consider clamping axis values with libinput even if we don't
> > normalize the axes by default.
> > 
> > I've forked libinput and I have a branch where I'm fixing up the patches
> > Carlos sent in based on the feedback from Peter. You can find it here:
> > 
> > 	https://github.com/Lyude1337/libinput/tree/carlos_cleanup
> 
> Thanks for the cleanup and appliying the recommended fixes, the changes
> look good so far.
> 
> > 
> > The history is messy on this, but once this is ready to get sent in as
> > actual patches I'll be rebasing the history.
> > 
> > Right now I've removed normalization from my branch, but if someone
> > brings up a good reason to actually have libinput handle that then I can
> > revert the change.
> 
> I agree, this should just probably be some helper code.
> 
> Cheers,
>   Carlos
>