Re: wayland tablet support redux

[Peter Hutterer <peter hutterer who-t net>]

On Fri, Apr 01, 2016 at 12:04:22PM +0200, Carlos Garnacho wrote:
> Hey Peter :),
>
> On Fri, Apr 1, 2016 at 6:00 AM, Peter Hutterer <peter hutterer who-t net> wrote:
> > On Wed, Mar 30, 2016 at 04:46:43PM +0200, Carlos Garnacho wrote:
> > > Hey all,
> > >
> > > Now that 3.22 is open, and that the v1 of the wayland tablet protocol
> > > is formalized, I'll be attempting to merge wip/wayland-tablet again.
> > > Some random concepts:
> > >
> > > =What's new in the branch=
> > > The main new API feature is GdkDeviceTool, it (almost, more on this
> > > ahead) univocally represents physical tablet tool (usually a pen),
> > > these structs contain a GdkDeviceToolType saying the kind of tool it
> > > is (pen, eraser, airbrush,...) and their serial number if available.
> > >
> > > The idea behind this struct is that interested apps can do per-tool
> > > tracking/configuration/etc, using a same tool will always result in
> > > the same GdkDeviceTool pointer reported in events, etc. And the serial
> > > number can be used to compose GSettings paths and whatnot.
> > >
> > > The branch also resuscitates proximity events, they are a thing again,
> > > and I added some kind of support for these for X11/XI2.
> > >
> > > There's though low-range tablets that don't report serials (and most
> > > usually, you can't use further/different tools than the one coming
> > > with the device), in wayland it's the compositor which takes care of
> > > creating the separate tool interfaces for those, in x11 we create
> > > extra GdkDeviceTools with serial=0, which are shared for all devices
> > > for all devices with these characteristics.
> > >
> > > =...but we had already an eraser device?=
> > > Yes, and in the branch we still do, all events from the eraser tool
> > > will be sent with a GDK_SOURCE_ERASER device, and all other tools will
> > > be always sent with a GDK_SOURCE_PEN device. That's kind of
> > > unfortunate and something that'd be great to switch on 4.0.
> >
> > can you explain what exactly you mean here? IMO the various device types are
> > a high-level approximation at best anyway and while we have it in libinput
> > and the protocol, I'm not sure things like PENCIL/etc. need to be exported.
> > if you can't use something like libwacom, you're pretty much lost anyway for
> > the device type (short of pen, mouse, eraser).
>
> Hmm, so you're saying that wp_tablet_tool.type is not completely
> reliable per-se? This is mostly a note that the GdkDevice hierarchy
> doesn't change wrt what we did earlier/on X11, where there are already
> "pen" and "eraser" devices. Basically I'd expect this assert to pass
> in an event handler:
>
> GdkDeviceTool *tool = gdk_event_get_device_tool (event);
> GdkDevice *device = gdk_event_get_source_device (event);
>
> if (tool)
>   g_assert ((gdk_device_tool_get_tool_type (tool) == GDK_DEVICE_TOOL_ERASER &&
>              gdk_device_get_source (device) == GDK_SOURCE_ERASER) ||
>             (gdk_device_tool_get_tool_type (tool) != GDK_DEVICE_TOOL_ERASER &&
>              gdk_device_get_source (device) == GDK_SOURCE_PEN));
>
> The typical app now roughly does:
>
> if (gdk_device_get_source (gdk_event_get_source_device (event)) ==
> GDK_SOURCE_ERASER)
>   color = bg_color;
> else
>   color = fg_color;
>
> /*paint*/
>
> I think a more future proof check would be looking up the tool, even
> though that code will still work in the mean time.

the tool type is reliable per pen, but not specific (see the hw-id we added
in the protocol, which is different to the serial ID). It translates into
the BTN_TOOL_* bits in the kernel, but several pens that are quite different
share the same type (and iirc the Pencil type is unused by modern hw). Since
you only have eraser and pen anyway, this shouldn't matter, it matters more
if you want to distinguish the tools by true type.

Cheers,
   Peter

> > > Another option could have been going the other way around, and adding
> > > extra GdkInputSources and devices. I don't think that option scales as
> > > well.
> > >
> > > =Relationship between tablets and tools=
> > > The typical noncontroversial setups are those with one tablet+one
> > > tool, or multiple tablets each with their own tool. Things complicate
> > > soon though when you add multiple tools and multiple compatible
> > > tablets, tools can then be used interchangeably across tablets.
> > >
> > > This is one thing that IMO the wayland protocol got right, both
> > > tablets and tools are announced by the seat, it is tools which do
> > > "emit" the events, have axes, etc. And you can only relate a tool to a
> > > tablet on proximity_in.
> > >
> > > And this is something we kind of do too in gdk. In the end you
> > > shouldn't expect that a given GdkDeviceTool will always be accompanied
> > > with the same GdkDevice in events. The difference here is that in gdk
> > > it is GdkDevices which have axes, "emit" events, etc. In order to keep
> > > doing this, slave pen/eraser devices do change their axes in order to
> > > resemble the tool in proximity_in, just like master devices currently
> > > do in x11 on slave device switch. The end message is the same,
> > > event->xxx.axes is only guaranteed to map correctly to the current
> > > device axes at the time of dispatching the event.
> > >
> > > This also affects the lifetime of objects, unplugging a tablet will
> > > tear down the associated GdkDevices, but GdkDeviceTools may stay
> > > indefinitely longer.
> >
> > I think it's also important to point out that in libinput (and thus by
> > inflection the X11 driver and the compositor) create the tools on the first
> > proximity in. This is a departure from the previous approach where the base
> > set of tools, sufficient for 99% of the use-cases, were available as soon as
> > the tablet was plugged in. This is not the case anymore, clients cannot
> > rely on this anymore. I haven't looked yet how you solved this, but I
> > suspect it's done in the straightforward manner (tool shows up, tool is no
> > available in GTK :)
>
> Right :). As a minor side note, this also means there's bumps ahead
> with gnome-settings-daemon.
>
> Cheers,
>   Carlos