[PATCH v3] protocol: Extend wl_touch with touchpoint shape and orientation

Tue Apr 19 13:28:10 UTC 2016

On Apr 18, 2016, at 11:49 PM, Peter Hutterer <peter.hutterer at who-t.net> wrote:
> On Mon, Apr 18, 2016 at 12:36:39PM +0800, Jonas Ådahl wrote:
>> On Fri, Apr 15, 2016 at 08:01:35AM -0700, Dennis Kempin wrote:
>>> This CL updates the wl_touch interface with a shape and
>>> orientation event.
>>> The shape/orientation of a touch point is not relevant for most UI
>>> applications, but allows a better experience in some cases
>>> such as drawing apps.
>>> 
>>> The events are used by the compositor to inform the client
>>> about changes in the shape and orientation of a touchpoint, which is
>>> approximated by an ellipse and it's angle to the y-axis.
>>> 
>>> The event is optional and only sent when compositor and the
>>> touch device support this type of information. The client is
>>> responsible for making a reasonable assumption about the
>>> touch shape if no shape is reported.
>>> 
>>> Signed-off-by: Dennis Kempin <denniskempin at google.com>
>>> ---
>>> protocol/wayland.xml | 64 ++++++++++++++++++++++++++++++++++++++++++++++++----
>>> 1 file changed, 59 insertions(+), 5 deletions(-)
>>> 
>>> diff --git a/protocol/wayland.xml b/protocol/wayland.xml
>>> index 12a6efd..b8a9835 100644
>>> --- a/protocol/wayland.xml
>>> +++ b/protocol/wayland.xml
>>> @@ -1656,7 +1656,7 @@
>>>     </request>
>>>    </interface>
>>> 
>>> -  <interface name="wl_seat" version="5">
>>> +  <interface name="wl_seat" version="6">
>>>     <description summary="group of input devices">
>>>       A seat is a group of keyboards, pointer and touch devices. This
>>>       object is published as a global during start up, or when such a
>>> @@ -1765,7 +1765,7 @@
>>> 
>>>   </interface>
>>> 
>>> -  <interface name="wl_pointer" version="5">
>>> +  <interface name="wl_pointer" version="6">
>>>     <description summary="pointer input device">
>>>       The wl_pointer interface represents one or more input devices,
>>>       such as mice, which control the pointer location and pointer_focus
>>> @@ -2078,7 +2078,7 @@
>>>     </event>
>>>   </interface>
>>> 
>>> -  <interface name="wl_keyboard" version="5">
>>> +  <interface name="wl_keyboard" version="6">
>>>     <description summary="keyboard input device">
>>>       The wl_keyboard interface represents one or more keyboards
>>>       associated with a seat.
>>> @@ -2192,7 +2192,7 @@
>>>     </event>
>>>   </interface>
>>> 
>>> -  <interface name="wl_touch" version="5">
>>> +  <interface name="wl_touch" version="6">
>>>     <description summary="touchscreen input device">
>>>       The wl_touch interface represents a touchscreen
>>>       associated with a seat.
>>> @@ -2242,7 +2242,12 @@
>>> 
>>>     <event name="frame">
>>>       <description summary="end of touch frame event">
>>> - Indicates the end of a contact point list.
>>> + Indicates the end of a contact point list. The wayland protocol requires
>>> + touch point updates to be sent sequentially, however all events within a
>>> + frame should be considered one hardware event. A wl_touch.frame terminates
>>> + at least one event but otherwise no guarantee is provided about the set of
>>> + events within a frame. A client must assume that any state not updated in a
>>> + frame is unchanged from the previously known state.
>> 
>> Seems to be some formatting error. This paragraph should be indented
>> with one tab, not one space. Same with the description in "shape" and
>> "orientation".
>> 
>> I think this part should be a separate patch. Also no need to point out
>> that this is the wayland protocol. I'd also suggest avoiding stating
>> that they are considered one hardware event. It'd probably be better to
>> follow what wl_pointer says here and say that they "logically belong
>> together". Check out wl_pointer.frame for some inspiration.
> 
> ftr, that's is the wording we used in the tablet protocol for the frame
> events:
>        Marks the end of a series of axis and/or button updates from the
>        tablet. The wayland protocol requires axis updates to be sent
>        sequentially, however all events within a frame should be considered
>        one hardware event.
> but I agree that the wl_pointer.frame wording is better here.
> 
>>>       </description>
>>>     </event>
>>> 
>>> @@ -2262,6 +2267,55 @@
>>>     <request name="release" type="destructor" since="3">
>>>       <description summary="release the touch object"/>
>>>     </request>
>>> +
>>> +    <!-- Version 6 additions -->
>>> +
>>> +    <event name="shape" since="6">
>>> +      <description summary="update shape of touch point">
>>> + Sent when a touchpoint has changed its shape. If the touch position
>>> + or orientation changed at the same time, the wl_touch.motion,
>>> + wl_touch.orientation and wl_touch.shape are sent within the same
>>> + wl_touch.frame. Otherwise, only a wl_touch.shape is sent within this
>>> + wl_touch.frame. The protocol does not guarantee specific ordering of
>>> + wl_touch.orientation, wl_touch.shape and wl_touch.motion events.
>> 
>> I'd suggest changing this paragraph to be more clear that the
>> non-guarantees of what may be sent together, while avoiding the wording
>> "at the same time". Something like
>> 
>> 	Sent when a touchpoint has changed its shape.
>> 
>> 	This event event does not occur on its own. It is sent before a
>> 	wl_touch.frame event, and carries the new shape information for
>> 	the specific touch points for that frame.
> 
> I don't think you need the ',' here
> 
>> 	
>> 	This event may be sent alone or together with other events
>> 	describing the touch point, such as wl_touch.motion and
>> 	wl_touch.orientation. The order of wl_touch.shape,
>> 	wl_touch.orientation and wl_touch.motion is not guaranteed.
> 
> "alone" and "together" aren't perfectly defined either, how about:
>    Other events describing the touch point such as wl_touch.motion or
>    wl_touch.orientation may be sent within the same wl_touch.frame. A
>    client should treat these events as a single logical touch point update.
>    The order of ...
> 
>> 
>>> +
>>> + A touchpoint shape is approximated by an ellipse through the major and minor
>>> + axis length. The major axis length describes the longest diameter of the
>>> + ellipse, while the minor axis length describes the shortest diameter.
> 
> longest->longer, shortest->shorter and add "Major and minor are orthogonal"
> 
>>> + Both are specified in surface coordinates. The center of the ellipse is
>>> + always at the touchpoint location as reported by wl_touch.down or
>>> + wl_touch.move.
>>> +
>>> + This event is only sent by the compositor if the touch device supports shape
>>> + reports. The client has to make reasonable assumptions about the shape if
>>> + it did not receive this event.
>>> +      </description>
>>> +      <arg name="id" type="int" summary="the unique ID of this touch point"/>
>>> +      <arg name="major" type="fixed" summary="length of the major
>>> axis in surface local coordinates"/>
>>> +      <arg name="minor" type="fixed" summary="length of the minor
>>> axis in surface local coordinates"/>
> 
> don't you need a dash here: surface-local? or is that american vs british
> english?

While I agree, since this is a compound adjective, the convention so far in all
other protocol docs has been just "surface local" without the dash. That said,
there are some description paragraphs that do use "surface-local."

Please tell me to resist the temptation to send patches that would standardize
this to the more correct "surface-local" everywhere. :)

yong

> 
>>> +    </event>
>>> +
>>> +    <event name="orientation" since="6">
>>> +      <description summary="update orientation of touch point">
>>> + Sent when a touchpoint has changed its orientation. If the touch position
>>> + or shape changed at the same time, the wl_touch.motion, wl_touch.orientation
>>> + and wl_touch.shape are sent within the same wl_touch.frame.
>>> + Otherwise, only a wl_touch.orientation is sent within this wl_touch.frame.
>>> + The protocol does not guarantee specific ordering of wl_touch.orientation,
>>> + wl_touch.shape and wl_touch.motion events.
>> 
>> Same suggested rewording as for the "shape" event.
>> 
>> 
>> Jonas
>> 
>>> +
>>> + The orientation describes the clockwise angle of touchpoints major axis to
>>> + the surface y-axis and is normalized to the -180 to +180 degrees range.
>>> + The granuality of orientation depends on the touch device, some devices only
>>> + support binary rotation values between 0 and 90 degrees.
>>> +
>>> + This event is only sent by the compositor if the touch device supports
>>> + orientation reports.
>>> +      </description>
>>> +      <arg name="id" type="int" summary="the unique ID of this touch point"/>
>>> +      <arg name="orientation" type="fixed" summary="angle between
>>> major axis and surface y-axis in degrees"/>
> 
> you need to specify the direction of the y axis too, because the angle could
> be cw or ccw and could be off the negative or positive y axis.
> 
> this is all just making sure we are precise enough, the actual protocol
> looks good, thanks.
> 
> Cheers,
>  Peter
> 
>>> +    </event>
>>> +
>>>   </interface>
>>> 
>>>   <interface name="wl_output" version="2">