[PATCH] protocol: Extend wl_touch with touch point shape

[Peter Hutterer]

On Thu, Mar 31, 2016 at 10:49:42AM +0800, Jonas Ådahl wrote:
> On Fri, Mar 25, 2016 at 09:29:56AM -0700, Dennis Kempin wrote:
> > This CL updates the wl_touch interface with a shape event.
> > The shape of a touch point is not relevant for most UI
> > applications, but allows a better experience in some cases
> > such as drawing app.
> > 
> > The shape event is used by the compositor to inform the client
> > about changes in the shape of a touchpoint. That shape is
> > described by an ellipsis, which directly relates to the Linux
> > Multitouch Protocol ABS_TOUCH_MAJOR / MINOR /
> > ORIENTATION axes.
> > 
> > The event is optional and only sent when the 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.
> 
> We tend to suggest to Signed-off-by patches. It is not a hard
> requirement, but just thought I'd mention.
> 
> > ---
> >  protocol/wayland.xml | 28 ++++++++++++++++++++++++----
> >  1 file changed, 24 insertions(+), 4 deletions(-)
> > 
> > diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> > index 8739cd3..85256ea 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.
> > @@ -2262,6 +2262,26 @@
> >      <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, this event can be included
> > +  in the same frame as the contact point list to update position and shape
> > +  simultaneously.
> 
> Does this mean that it may be emitted without a following frame event?
> If a client wants to process all touch point changes upon receiving the
> frame event, how will it know whether a shape event is coming by itself,
> or part of the series?
> 
> I think it'd be better to clarify this, and probably always require it
> to always be followed by a frame event.

tbh, with the commit message I assumed it'll always come in a frame but
you're right, the "can be included" opens some ambiguities. The wording
should be more assertive, and we should extend the wl_touch.frame
documentation to include the new event specifically. Something like this:

"Sent when a touchpoint has changed its shape. If the touch position changed
at the same time, the wl_touch.motion 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.shape and wl_touch.motion events"

And for wl_touch.frame something like:
"Indicates the end of a contract 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.frames 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."

Feel free to merge and reword this as you see fit.

One comment, I'm not 100% happy with the term "shape" but I can't come up
with anything better ("extents" maybe?)

> > +  A touchpoint shape is described by an ellipsis through a major and minor axis
> > +  length and orientation.
> > +  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 message.
> > +      </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 space"/>
> > +      <arg name="minor" type="fixed" summary="length of the minor
> > axis in surface space"/>
> > +      <arg name="orientation" type="fixed" summary="orientation of
> > ellipsis in radians."/>
> 
> In the tablet protocol[0], we are using degrees instead of radians, mostly
> because it is easier to spot strange values. So I think we should
> consider using degrees here as well.

Agreed. And please specify the exact details of what is considered the
major/minor axis (direction, orientation) and where the logical 0 is for the
orientation. Referencing the kernel isn't good enough here, we need to make
sure this is precise. Adding a few paragraphs to the documentation doesn't
hurt and saves headaches later. Other than that, it looks good imo, thanks.

Cheers,
   Peter