[PATCH v2 xserver 9/9] xwayland: add tablet pad support

[Carlos Garnacho]

Hey :),

On Fri, Feb 10, 2017 at 12:06 AM, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> On Thu, Feb 09, 2017 at 12:14:47PM -0800, Jason Gerecke wrote:
>> On Tue, Feb 7, 2017 at 6:42 PM, Peter Hutterer <peter.hutterer at who-t.net> wrote:
>> > Hooked up a bit differently to the other tools. Those tools can be static for
>> > all and be re-used. The wacom driver initializes the pad with the correct
>> > number of buttons though and we can't do this until we have the pad done event.
>> >
>> > If the tablet is removed and we plug a different one in, we should initialize
>> > that correctly, so unlike the other tools the pad is properly removed and
>> > re-initialized on plug.
>> >
>> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
>> > ---
>> > New in this series
>> >
>> >  hw/xwayland/xwayland-input.c | 417 +++++++++++++++++++++++++++++++++++++++++++
>> >  hw/xwayland/xwayland.h       |  28 +++
>> >  2 files changed, 445 insertions(+)
>> >
>> > diff --git a/hw/xwayland/xwayland-input.c b/hw/xwayland/xwayland-input.c
>> > index 31e2473..1079ff4 100644
>> > --- a/hw/xwayland/xwayland-input.c
>> > +++ b/hw/xwayland/xwayland-input.c
>> > @@ -1334,6 +1334,7 @@ tablet_handle_removed(void *data, struct zwp_tablet_v2 *tablet)
>> >              DisableDevice(xwl_seat->eraser, TRUE);
>> >          if (xwl_seat->puck)
>> >              DisableDevice(xwl_seat->puck, TRUE);
>> > +        /* pads are removed separately */
>> >      }
>> >
>> >      zwp_tablet_v2_destroy(tablet);
>> > @@ -1694,6 +1695,418 @@ static const struct zwp_tablet_tool_v2_listener tablet_tool_listener = {
>> >  };
>> >
>> >  static void
>> > +tablet_pad_ring_destroy(struct xwl_tablet_pad_ring *ring)
>> > +{
>> > +    zwp_tablet_pad_ring_v2_destroy(ring->ring);
>> > +    xorg_list_del(&ring->link);
>> > +    free(ring);
>> > +}
>> > +
>> > +static void
>> > +tablet_pad_ring_source(void *data,
>> > +                       struct zwp_tablet_pad_ring_v2 *zwp_tablet_pad_ring_v2,
>> > +                       uint32_t source)
>> > +{
>> > +}
>> > +
>> > +static void
>> > +tablet_pad_ring_angle(void *data,
>> > +                      struct zwp_tablet_pad_ring_v2 *zwp_tablet_pad_ring_v2,
>> > +                      wl_fixed_t degrees)
>> > +{
>> > +    struct xwl_tablet_pad_ring *ring = data;
>> > +    struct xwl_tablet_pad *pad = ring->group->pad;
>> > +    double deg = wl_fixed_to_double(degrees);
>> > +    ValuatorMask mask;
>> > +
>> > +    valuator_mask_zero(&mask);
>> > +    valuator_mask_set(&mask, 5 + ring->index, deg/360.0  * 71);
>> > +    QueuePointerEvents(pad->xdevice, MotionNotify, 0, 0, &mask);
>> > +}
>> > +
>> > +static void
>> > +tablet_pad_ring_stop(void *data,
>> > +                     struct zwp_tablet_pad_ring_v2 *zwp_tablet_pad_ring_v2)
>> > +{
>> > +}
>> > +
>> > +static void
>> > +tablet_pad_ring_frame(void *data,
>> > +                      struct zwp_tablet_pad_ring_v2 *zwp_tablet_pad_ring_v2,
>> > +                      uint32_t time)
>> > +{
>> > +}
>> > +
>> > +static const struct zwp_tablet_pad_ring_v2_listener tablet_pad_ring_listener = {
>> > +    tablet_pad_ring_source,
>> > +    tablet_pad_ring_angle,
>> > +    tablet_pad_ring_stop,
>> > +    tablet_pad_ring_frame,
>> > +};
>> > +
>> > +
>> > +static void
>> > +tablet_pad_strip_destroy(struct xwl_tablet_pad_strip *strip)
>> > +{
>> > +    zwp_tablet_pad_strip_v2_destroy(strip->strip);
>> > +    xorg_list_del(&strip->link);
>> > +    free(strip);
>> > +}
>> > +
>> > +static void
>> > +tablet_pad_strip_source(void *data,
>> > +                        struct zwp_tablet_pad_strip_v2 *zwp_tablet_pad_strip_v2,
>> > +                        uint32_t source)
>> > +{
>> > +}
>> > +
>> > +static void
>> > +tablet_pad_strip_position(void *data,
>> > +                          struct zwp_tablet_pad_strip_v2 *zwp_tablet_pad_strip_v2,
>> > +                          uint32_t position)
>> > +{
>> > +    struct xwl_tablet_pad_strip *strip = data;
>> > +    struct xwl_tablet_pad *pad = strip->group->pad;
>> > +    ValuatorMask mask;
>> > +
>> > +    valuator_mask_zero(&mask);
>> > +    valuator_mask_set(&mask, 3 + strip->index, position/65535.0 * 2048);
>>
>> Note: the values reported by xf86-input-wacom are the raw hardware
>> values which use a walking bit to indicate where the finger is along
>> the strip. When your finger is at the top of the strip, we report '1';
>> moving down slightly '2', then '4', then '8', and so on until you
>> reach the bottom and we report '4096'.
>>
>> The linear relationship you use here won't be compatible with software
>> that expects the exponential relationship used by xf86-input-wacom.
>> You could potentially use something like round(pow(2.0,
>> 12*position/65535.0)) to get the "expected" values, along with
>> changing the axis maximums to 4096.
>
> hmm. for the libinput xorg driver I also just forward the strip events
> as-is, in the hope that eventually we can get rid of the pow2 which is
> effectively just a long-standing bug that we never removed for fear of
> breaking applications. Do we *know*  about applications that actually rely
> on this exact behaviour and that cannot change?
>

FWIW I know of none. gnome-settings-daemon used to listen to button
presses rather than checking valuator state (and so does mutter).
Actually, given the passive grab on XIAnyButton, I think no
application saw much of these events on a gnome session for a long
time now.

> I'm really uncomfortable continuing this in xwayland (and the libinput xorg
> driver).

I agree, an exponential axis seems entirely out of place.

Cheers,
  Carlos