[PATCH v4 wayland-protocols 4/4] tablet: Add pad support to the tablet protocol

Thu Jun 30 09:31:28 UTC 2016

Hi!,

On Thu, Jun 30, 2016 at 5:27 AM, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> On Tue, Jun 28, 2016 at 11:21:50PM +0200, Carlos Garnacho wrote:
>> The pad's interface is similar to the tool interface, a client is notified of
>> the pad after the tablet_added event.
>>
>> The pad has three functionalities: buttons, rings and strips.
>> Buttons are fairly straightforward, rings and strips are separate interfaces
>> with a pointer-axis-like source/value/frame events.
>> The two interfaces are effectively identical but for the actual value they
>> send (degrees vs normalized position).
>>
>> Buttons are sequentially indexed starting with zero, unlike other protocols
>> where a linux/input.h-style semantic event code is used. Since we expect all
>> buttons to have client-specific functionality, an additional event tells the
>> client when a given button index is not available, usually because the
>> compositor assignes some function to it (e.g. mode switching, see below).
>>
>> Specific to the pad device is the set_feedback request which enables a client
>> to set a user-defined string to display for an OSD on the current mappings.
>> This request is available for buttons, rings and strips.
>>
>> Finally, the pad supports groups, effectively sets of button/ring/strip
>> configurations. Those groups may have multiple modes each, so that
>> users/clients may map several actions to a single element.
>>
>> Signed-off-by: Carlos Garnacho <carlosg at gnome.org>
>> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
>> Reviewed-by: Jason Gerecke <jason.gerecke at wacom.com>
>> ---
>>
>> Changes since v3:
>>
>> - Added wp_tablet_pad_group, this divides buttons/strips/rings into subsets,
>>   groups may have separate modes which are announced through a mode_switch
>>   event.
>> - Added serial argument to the set_feedback requests, to be obtained from
>>   the wp_tablet_pad_group.mode_switch events.
>> - Now that groups announce the buttons belonging to those, the
>>   wp_tablet_pad.buttons_reserved event has been removed in favor of implicit
>>   information (i.e. buttons not belonging to any group are reserved by the
>>   compositor).
>> - Added wp_tablet_pad.path event, similar to wp_tablet.path
>> - Button indices are now explicitly mentioned in docs where relevant
>> - Fixed several typos and improved docs here and there
>>
>>  unstable/tablet/tablet-unstable-v2.xml | 546 ++++++++++++++++++++++++++++++++-
>>  1 file changed, 543 insertions(+), 3 deletions(-)
>>
>> diff --git a/unstable/tablet/tablet-unstable-v2.xml b/unstable/tablet/tablet-unstable-v2.xml
>> index 5248c64..63172e2 100644
>> --- a/unstable/tablet/tablet-unstable-v2.xml
>> +++ b/unstable/tablet/tablet-unstable-v2.xml
>> @@ -172,6 +172,22 @@
>>        </description>
>>        <arg name="id" type="new_id" interface="zwp_tablet_tool_v2" summary="the newly added tablet tool"/>
>>      </event>
>> +
>> +    <event name="pad_added">
>> +      <description summary="new pad notification">
>> +     This event is sent whenever a new pad is known to the system. Typically,
>> +     pads are physically attached to tablets and a pad_added event is
>> +     sent immediately after the wp_tablet_seat.tablet_added.
>> +     However, some standalone pad devices logically attach to tablets at
>> +     runtime, and the client must wait for wp_tablet_pad.enter to know
>> +     the tablet a pad is attached to.
>> +
>> +     This event only provides the object id of the pad. All further
>> +     features (buttons, strips, rings) are sent through the wp_tablet_pad
>> +     interface.
>> +      </description>
>> +      <arg name="id" type="new_id" interface="zwp_tablet_pad_v2" summary="the newly added pad"/>
>> +    </event>
>>    </interface>
>>
>>    <interface name="zwp_tablet_tool_v2" version="1">
>> @@ -508,9 +524,9 @@
>>        <description summary="Wheel delta event">
>>       Sent whenever the wheel on the tool emits an event. This event
>>       contains two values for the same axis change. The degrees value is
>> -     in degrees in the same orientation as the wl_pointer.vertical_scroll
>> -     axis. The clicks value is in discrete logical clicks of the mouse
>> -     wheel. This value may be zero if the movement of the wheel was less
>> +     in the same orientation as the wl_pointer.vertical_scroll axis. The
>> +     clicks value is in discrete logical clicks of the mouse wheel. This
>> +     value may be zero if the movement of the wheel was less
>>       than one logical click.
>
> this hunk looks like rebase detritus

Oops, it indeed is. Now I wonder which of your earlier patches this
belonged to...

>>
>>       Clients should choose either value and avoid mixing degrees and
>> @@ -636,4 +652,528 @@
>>      </event>
>>    </interface>
>>
>> +  <interface name="zwp_tablet_pad_ring_v2" version="1">
>> +    <description summary="pad ring">
>> +      A circular interaction area.
>
> I think it'd be best to add " such as the touch ring on the Wacom Intuos Pro
> series tablets", just to save us from having to explain what a ring is ;)
> Same goes for the strip.

Agreed :), added that blurb as-is, and another for strips/cintiqs.

>
> [...]
>> +    <enum name="source">
>> +      <description summary="ring axis source">
>> +     Describes the source types for ring events. This indicates to the
>> +     client how a ring event was physically generated; a client may
>> +     adjust the user interface accordingly. For example, events
>> +     from a "finger" source may trigger kinetic scrolling.
>> +      </description>
>> +      <entry name="finger" value="1" summary="finger"/>
>> +    </enum>
>> +
>> +    <event name="source">
>> +      <description summary="ring event source">
>> +     Source information for ring events.
>> +
>> +     This event does not occur on its own. It is sent before a
>> +     wp_tablet_pad_ring.frame event and carries the source information
>> +     for all events within that frame.
>> +
>> +     The source specifies how this event was generated. If the source is
>> +     wp_tablet_pad_ring.source.finger, a wp_tablet_pad_ring.stop event
>> +     will be sent when the user lifts the finger off the device.
>> +
>> +     This event is optional. If the source is unknown for an interaction,
>> +     no event is sent.
>> +      </description>
>> +      <arg name="source" type="uint" summary="the event source"/>
>
> this should ref the enum, same with the strip and the button event.

Right wp_tablet_pad_ring/strip.source were missing enum="source", the
pad button event seemed fine though. Or do you mean in the summary?

>
> [...]
>
>> +
>> +  <interface name="zwp_tablet_pad_group_v2" version="2">
>> +    <description summary="a set of buttons, rings and strips">
>> +      A pad group describes a distinct (sub)set of buttons, rings and strips
>> +      present in the tablet. The criteria of this grouping is usually positional,
>> +      eg. if a tablet has buttons on left and right sides, 2 groups will be
>
> "on the left and right side"

Fixed locally.

>
>> +      presented. Clients should perform no further assumptions on the actual
>
> s/further//
> I'm not sure you can perform assumptions but some native english speaker may
> have more idea here. :)

Knowing myself, it's most likely broken english :). I've changed that to:

"The physical arrangement of groups is undisclosed and may change on the fly"

>
>> +      position of each group.
>> +
>> +      Pad groups will announce their features during pad initialization. Between
>> +      the corresponding wp_tablet_pad.group event and wp_tablet_pad_group.done, the
>> +      pad group will announce the buttons, rings and strips comprised in it,
>> +      plus the number of supported modes.
>> +
>> +      Modes are a mechanism to allow multiple groups of actions for every element
>> +      in the pad group. The number of groups and available modes in each is
>> +      persistent across device plugs. The current mode is user-switchable and
>> +      compositor-global, it will be announced through the
>> +      wp_tablet_pad_group.mode_switch event both after wp_tablet_pad.enter, and
>> +      whenever it is switched.
>> +
>> +      The current mode logically applies to all elements in the pad group,
>> +      although it is at clients' discretion whether to actually perform different
>> +      actions, and/or issue the respective .set_feedback requests to notify the
>> +      compositor.
>
> for extra clarification, add "see wl_tablet_pad_group.mode_switch for more
> details".

Added locally.

>
> Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>

Thanks!

Cheers,
  Carlos