[PATCH libinput 0/24] Tablet support

[Peter Hutterer]

On Mon, May 26, 2014 at 02:27:35AM +0200, Carlos Garnacho wrote:
> Hey Peter,
> 
> On vie, 2014-05-23 at 16:19 +1000, Peter Hutterer wrote:
> > 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?
> 
> Hmm, ditch that. I was mainly thinking on event compression and other
> places where you may want to diff against previous events, but that
> shouldn't be happening often on apps making full use of tablets... And
> this is all moot with getters and a bitmask.
> 
> And I was perhaps thinking too much under the assumption that this is
> largely going to shape the wayland tablet API, hence my use of the term
> "client".
> 
> > 
> > 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.
> 
> It doesn't sound bad to have specific getters instead of
> get_axis()+enum. 

hmm, didn't actually intend that. it sounds nice but depending on the axis
numbers that explodes quite a bit and is probably harder to maintain.
re-read the above as:

 ".... guarantee that all axes are always set to the current value, so most
  callers could then just call get_axis(PRESSURE) to get the pressure,
  regardless of whether that changed.."

the main point is to have every libinput event valid for all supported axes
instead of a diff of just the axes that changed.

> One thing I wonder is, what would be the return value
> of axes not supported by the device? a meaningful constant value? an
> special "axis unsupported" value? exposing a bitmask of features?

how about something like libevdev? it has a triplet of:
    int libevev_has_event_code(type, code)
    int libevdev_get_event_value(type, code)
    int libevdev_fetch_event_value(type, code, *value)

has() does what you'd expect, get() returns the value but is undefined if
the code isn't supported. fetch() gives you the value but returns 0 if the
value is not supported, so it's the combination of has/get in a single line
for the caller. Taking this to libinput, the caller could then use something
like this:

int pressure;
if (libinput_event_stylus_fetch(event, LIBINPUT_AXIS_PRESSURE, &pressure)) {
    // now handle pressure
} else {
    // pressure not supported on this device
}

bit more mental gymnastics and you could either make that return value a
triplet of "not available/available but unchanged/available and changed" or
just provide a separate function:

if (libinput_event_stylus_fetch_new(event, LIBINPUT_AXIS_PRESSURE, &pressure)) {
    // value has changed in this event
} else {
    // unchanged or not supported by this device
}

Cheers,
   Peter