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

[Jonas Ådahl]

On Tue, Apr 19, 2016 at 02:49:50PM +1000, Peter Hutterer 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 ...

Agreed, this is even better.


Jonas