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

[Carlos Garnacho]

Hi!,

On Thu, Jul 7, 2016 at 11:30 PM, Jason Gerecke <killertofu at gmail.com> wrote:
> On 06/30/2016 05:16 AM, Jonas Ådahl wrote:
>> On Thu, Jun 30, 2016 at 01:10:33PM +0200, Carlos Garnacho wrote:
>>> Hi!,
>>>
>>> On Thu, Jun 30, 2016 at 6:27 AM, Jonas Ådahl <jadahl at gmail.com> 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>
>>>>> ---
>>>>>
>>
>> ... snip ...
>>
>>>>> +
>>>>> +    <event name="ring">
>>>>> +      <description summary="ring announced">
>>>>> +     Sent on wp_tablet_pad_group initialization to announce available rings.
>>>>> +     One event is sent for each ring available on this pad group.
>>>>> +
>>>>> +     This event is sent in the initial burst of events before the
>>>>> +     wp_tablet_pad_group.done event. This event is only sent when at least
>>>>> +     one ring is available.
>>>>
>>>> The last sentence is redundant. It already says that there'll be one
>>>> event per ring, thus no rings, no events. It also makes it slightly
>>>> unclear (by the description alone) whethere multiple rings will result
>>>> in multiple events.
>>>
>>> Agreed, removed. As for the last part... I guess the "One event is
>>> sent for each ring available..." bit makes it clear enough already?
>>
>> Yes, I think that part makes it clear. It was only the part you now
>> removed that made it possible to doubt the first part for a second.
>>
>>>
>>
>> ... snip ...
>>
>>>>> +
>>>>> +    <event name="mode_switch">
>>>>> +      <description summary="mode switch event">
>>>>> +     Notification that the mode was switched.
>>>>> +
>>>>> +     A mode applies to all buttons, rings and strips in a group
>>>>> +     simultaneously, but a client is not required to assign different actions
>>>>> +     for each mode. For example, a client may have mode-specific button
>>>>> +     mappings but map the ring to vertical scrolling in all modes. Mode
>>>>> +     indices start at 0.
>>>>> +
>>>>> +     Switching modes is compositor-dependent. The compositor may provide
>>>>> +     visual cues to the client about the mode, e.g. by toggling LEDs on
>>>>> +     the tablet device. Mode-switching may be software-controlled or
>>>>> +     controlled by one or more physical buttons. For example, on a Wacom
>>>>> +     Intuos Pro, the button inside the ring may be assigned to switch
>>>>> +     between modes.
>>>>> +
>>>>> +     The current mode is compositor-global, the compositor will also send
>>>>
>>>> Isn't it just seat global? And why would this event be on the pad group
>>>> and not on the seat wide interface?
>>>
>>> perhaps I should expand that first phrase :).
>>>
>>> The extent of the current mode is pretty much group local, it only
>>> affects the buttons/rings/strips that have been announced through this
>>> group.
>>>
>>> However, the current mode of every given group is global to all
>>> clients. If you have a client focused with a group in mode=1, focus
>>> another client, switch that group to mode=3 and go back to the first
>>> client, you'll receive a .mode_switch with mode=3 after
>>> wp_tablet_pad.enter.
>>>
>>> This could be considered similar to keyboard modifiers, you don't
>>> reset those (eg. caps lock) when focusing another client, the current
>>> state is rather propagated to the newly focused client.
>>>
>>> I'm replacing the beginning of that paragraph with this:
>>> "Compositors will globally track the current mode that every
>>> wp_tablet_pad_group has. This event will also be sent after
>>> wp_tablet_pad.enter for all groups in the pad in order to..."
>>
>> I see. Any reason on why it needs to be compositor global? Couldn't that
>> be a decision left to the compositor? Either way, the new wording makes
>> me understand whats going on, so it now looks good to me.
>>
>
> ...Before I forget (and before I disappear on vacation next week...)
>
> At least at the current moment there's no particular reason the mode has
> to be globally tracked. Its the policy used by libinput, but a
> compositor could, if it wanted to, track the modes per-application or
> based on some other policy.

Yup, Jonas/me briefly discussed this some days ago on IRC and thought
too it's better not to encode this in the protocol docs. In practical
terms it could be hard to implement, but it won't be me who
discourages compositor implementations to go the extra mile. I removed
all mentions to compositor-global per group modes and will send the
full series again, hopefully in time for your last thumbs up, enjoy
vacation otherwise/anyway :).

Cheers,
  Carlos