[PATCH v3] protocol: Extend wl_touch with touchpoint shape event

Fri Apr 15 10:59:46 UTC 2016

On Apr 14, 2016, at 8:54 PM, Jonas Ådahl <jadahl at gmail.com> wrote:
> 
> On Thu, Apr 14, 2016 at 07:04:25PM -0500, Yong Bakos wrote:
>> On Apr 14, 2016, at 3:23 PM, Dennis Kempin <denniskempin at google.com> 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>
>> 
>> Hi Dennis,
>> I've noted some concerns inline below. I'm a bit inexperienced here, so
>> please do correct/educate me where I'm wrong.
>> 
>> 
>>> ---
>>> protocol/wayland.xml | 75 ++++++++++++++++++++++++++++++++++++++++++++--------
>>> 1 file changed, 64 insertions(+), 11 deletions(-)
>>> 
>>> diff --git a/protocol/wayland.xml b/protocol/wayland.xml
>>> index 12a6efd..d19d898 100644
>>> --- a/protocol/wayland.xml
>>> +++ b/protocol/wayland.xml
>>> @@ -910,11 +910,6 @@
>>>      copy-and-paste and drag-and-drop.  These mechanisms are tied to
>>>      a wl_seat and this interface lets a client get a wl_data_device
>>>      corresponding to a wl_seat.
>>> -
>>> -      Depending on the version bound, the objects created from the bound
>>> -      wl_data_device_manager object will have different requirements for
>>> -      functioning properly. See wl_data_source.set_actions,
>>> -      wl_data_offer.accept and wl_data_offer.finish for details.
>> 
>> What does the deletion of this paragraph have to do with adding the
>> two new touchpoint shape events?
>> 
>> 
>>>    </description>
>>> 
>>>    <request name="create_data_source">
>>> @@ -1656,7 +1651,7 @@
>>>    </request>
>>>   </interface>
>>> 
>>> -  <interface name="wl_seat" version="5">
>>> +  <interface name="wl_seat" version="6">
>> 
>> Why are you bumping this instead of just bumping the version of the 
>> wl_touch interface?
> 
> Hi,
> 
> Just going to reply to your comments regarding the version bumps.
> 
> In Wayland, objects (except wl_display) are created from some other
> object, and most "concepts" (such as input, surfaces, data
> device/clipboard) have a single "global" object where all the other
> objects are created from.
> 
> When you create an object from another object, except for global
> objects, the new object will inherit the version of the object it was
> created from. So for example if you have a global wl_compositor object
> with version N, all wl_surface's created from that object will also have
> the version N. If you need to bump the version of wl_surface, if you
> want to be able to create an object with the bumped version, you also
> need to bump the version of the interface the wl_surface is created
> from, i.e. wl_compositor.
> 
> In this patch, the interface bumped is wl_touch, and the interface of
> the object a wl_touch object is created from is wl_seat. Thus in order
> to be able to get a wl_touch object with the bumped version, you also
> need to bump the version of each parent interface up to the global
> object.
> 
> Note that when you bump the version of the interface of the global
> object, when creating an object from that global, it'll inherit the
> version. This means that when you bump wl_touch and wl_seat, you'll
> effectively get wl_pointer and wl_keyboard objects with the bumped
> version as well.
> 
> While it is required to bump the parents up to the global object, it is
> completely optional to actually bump the interface versions of all the
> other interfaces in an "interface tree". The reason for doing so may be
> to make it clear the actual highest version an object created with this
> interface may be created at, even though no new events nor requests were
> added for some particular version.
> 
> Hope this cleares things up.

Ah - I was suspecting that. /Thank you/ for taking the time to explain!
I swear I've read the documentation, which mentions this in the
Versioning section, but it didn't stick in my mind.

yong

> 
> Jonas
> 
>> 
>> 
>>>    <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 +1760,7 @@
>>> 
>>>  </interface>
>>> 
>>> -  <interface name="wl_pointer" version="5">
>>> +  <interface name="wl_pointer" version="6">
>> 
>> Why are you bumping this instead of just bumping the version of the 
>> wl_touch interface?
>> 
>> 
>>>    <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
>>> @@ -2038,7 +2033,7 @@
>>> 
>>> The timestamp is to be interpreted identical to the timestamp in the
>>> wl_pointer.axis event. The timestamp value may be the same as a
>>> - preceding wl_pointer.axis event.
>>> + preceeding wl_pointer.axis event.
>> 
>> Why are you introducing a misspelling?
>> 
>> 
>>>      </description>
>>>      <arg name="time" type="uint" summary="timestamp with
>>> millisecond granularity"/>
>>>      <arg name="axis" type="uint" enum="axis" summary="the axis
>>> stopped with this event"/>
>>> @@ -2078,7 +2073,7 @@
>>>    </event>
>>>  </interface>
>>> 
>>> -  <interface name="wl_keyboard" version="5">
>>> +  <interface name="wl_keyboard" version="6">
>> 
>> Why are you bumping this instead of just bumping the version of the 
>> wl_touch interface?
>> 
>> 
>>>    <description summary="keyboard input device">
>>>      The wl_keyboard interface represents one or more keyboards
>>>      associated with a seat.
>>> @@ -2192,7 +2187,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 +2237,12 @@
>>> 
>>>    <event name="frame">
>>>      <description summary="end of touch frame event">
>>> - Indicates the end of a contact point list.
>>> +  Indicates the end of a contract point list. The wayland protocol requires
>> 
>> contact point list.
>> 
>> 
>>> +  touch point updates to be sent sequentially, however all events within a
>>> +  frame should be considered one hardware event. A wl_touch.frames terminates
>> 
>> wl_touch.frame
>> 
>> 
>>> +  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.
>> 
>> Am I misreading this or has indentation been obliterated?
>> 
>> 
>>>      </description>
>>>    </event>
>>> 
>>> @@ -2262,6 +2262,59 @@
>>>    <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.
>> 
>> Same paragraph? And indentation?
>> 
>> 
>>> +  The protocol does not guarantee specific ordering of wl_touch.orientation,
>>> +  wl_touch.shape and wl_touch.motion events.
>>> +
>>> +  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.
>>> +  Both are specified in surface coordinates.
>>> +  The center of the ellipse is always at the touchpoint location as reported
>> 
>> Same paragraph? And indentation?
>> 
>> 
>>> +  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.
>> 
>> This last sentence seems unnecessary.
>> 
>> 
>>> +      </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 coordinates"/>
>>> +      <arg name="minor" type="fixed" summary="length of the minor
>>> axis in surface coordinates"/>
>> 
>> I politely encourage "surface local coordinates."
>> https://lists.freedesktop.org/archives/wayland-devel/2016-April/027804.html
>> 
>> 
>>> +    </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.
>>> +
>>> +  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.
>>> +  The client has to make reasonable assumptions about the orientation if
>>> +  it did not receive this event.
>> 
>> As mentioned in the shape event, I feel this last sentence is unnecessary.
>> And, am I misreading or is the indentation missing?
>> 
>>> +      </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"/>
>>> +    </event>
>>> +
>> 
>> Omitting this line break would match the current convention.
>> 
>>>  </interface>
>>> 
>>>  <interface name="wl_output" version="2">
>>> --
>>> 2.8.0.rc3.226.g39d4020
>>> _______________________________________________
>>> wayland-devel mailing list
>>> wayland-devel at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>> 
>> _______________________________________________
>> wayland-devel mailing list
>> wayland-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel