[PATCH weston v2 1/5] protocol: Add _wl_pointer_gestures (swipe/pinch) protocol

Jonas Ådahl jadahl at gmail.com
Sat Aug 1 20:38:35 PDT 2015


On Fri, Jul 31, 2015 at 03:53:43PM +0200, Carlos Garnacho wrote:
> Hey Jonas :),
> 
> On Wed, Jul 29, 2015 at 4:52 AM, Jonas Ådahl <jadahl at gmail.com> wrote:
> > On Thu, Jul 23, 2015 at 07:00:27PM +0200, Carlos Garnacho wrote:
> >> The whole feature is exposed by the wl_pointer_gestures global
> >> resource, which can be used to obtain individual swipe/pinch
> >> gesture interfaces for a given wl_pointer.
> >>
> >> The lifetime and progress of gestures is maintained by the separate
> >> wl_pointer_gesture_pinch and wl_pointer_gesture_swipe interfaces,
> >> each of these has begin/update(optional)/end phases, as transmitted
> >> by their events.
> >>
> >> The protocol is marked as unstable, an is thus '_' prefixed, this
> >> prefix will be removed when the protocol is deemed stable.
> >>
> >> Signed-off-by: Carlos Garnacho <carlosg at gnome.org>
> >> Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
> >
> > In general the protocol mostly looks great to me, but I have some
> > comments and questions below:
> >
> >> ---
> >>  Makefile.am           |   3 +
> >>  protocol/gestures.xml | 170 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 173 insertions(+)
> >>  create mode 100644 protocol/gestures.xml
> >>
> >> diff --git a/Makefile.am b/Makefile.am
> >> index a3590c0..abc90a4 100644
> >> --- a/Makefile.am
> >> +++ b/Makefile.am
> >> @@ -96,6 +96,8 @@ weston_SOURCES =                                    \
> >>       src/weston-egl-ext.h
> >>
> >>  nodist_weston_SOURCES =                                      \
> >> +     protocol/gestures-protocol.c                    \
> >> +     protocol/gestures-server-protocol.h             \
> >>       protocol/screenshooter-protocol.c               \
> >>       protocol/screenshooter-server-protocol.h        \
> >>       protocol/text-cursor-position-protocol.c        \
> >> @@ -1298,6 +1300,7 @@ BUILT_SOURCES +=                                \
> >>       protocol/text-client-protocol.h
> >>
> >>  EXTRA_DIST +=                                        \
> >> +     protocol/gestures.xml                   \
> >>       protocol/desktop-shell.xml              \
> >>       protocol/screenshooter.xml              \
> >>       protocol/text.xml                       \
> >> diff --git a/protocol/gestures.xml b/protocol/gestures.xml
> >> new file mode 100644
> >> index 0000000..91c8194
> >> --- /dev/null
> >> +++ b/protocol/gestures.xml
> >> @@ -0,0 +1,170 @@
> >> +<protocol name="pointer_gestures">
> >> +  <interface name="_wl_pointer_gestures" version="1">
> >> +    <description summary="touchpad gestures">
> >> +      A global interface to provide semantic touchpad gestures for a given
> >> +      pointer.
> >> +
> >> +      Two gestures are currently supported: swipe and zoom/rotate.
> >> +      All gestures follow a three-stage cycle: begin, update, end and
> >> +      are identified by a unique id.
> >> +
> >> +      Warning! The protocol described in this file is experimental. Each
> >> +      version of this protocol should be considered incompatible with any
> >> +      other version, and a client binding to a version different to the one
> >> +      advertised will be terminated. Once the protocol is declared stable,
> >> +      compatibility is guaranteed, the '_' prefix will be removed from the
> >> +      name and the version will be reset to 1.
> >> +    </description>
> >> +
> >> +    <request name="get_swipe_gesture">
> >> +      <description summary="get swipe gesture">
> >> +     Create a swipe gesture object. See the
> >> +     wl_pointer_gesture_swipe interface for details.
> >> +      </description>
> >> +      <arg name="id" type="new_id" interface="_wl_pointer_gesture_swipe"/>
> >> +      <arg name="pointer" type="object" interface="wl_pointer"/>
> >> +    </request>
> >> +
> >> +    <request name="get_pinch_gesture">
> >> +      <description summary="get pinch gesture">
> >> +     Create a pinch gesture object. See the
> >> +     wl_pointer_gesture_pinch interface for details.
> >> +      </description>
> >> +      <arg name="id" type="new_id" interface="_wl_pointer_gesture_pinch"/>
> >> +      <arg name="pointer" type="object" interface="wl_pointer"/>
> >> +    </request>
> >> +  </interface>
> >> +
> >> +  <interface name="_wl_pointer_gesture_swipe" version="1">
> >> +    <description summary="a swipe gesture object">
> >> +      A swipe gesture object notifies a client about a multi-finger swipe
> >> +      gesture detected on an indirect input device such as a touchpad.
> >> +      The gesture is usually initiated by multiple fingers moving in the
> >> +      same direction but once initiated the direction may change.
> >> +      The precise conditions of when such a gesture is detected are
> >> +      implementation-dependent.
> >> +
> >> +      A gesture consists of three stages: begin, update (optional) and end.
> >> +      There cannot be multiple simultaneous pinch or swipe gestures, how
> >> +      compositors prevent these situations is implementation-dependent.
> >
> > There cannot be multiple simultaneous gestures one one seat. There can
> > be multiple gestures but that means they are on different seats.
> 
> Right, that's the idea. On multi-seat, there would be several
> wl_pointers, and each could be able to perform gestures individually.
> I reworded the docs blurb to be more specific there.
> 
> >
> >> +
> >> +      A gesture may be cancelled by the compositor or the hardware.
> >> +      Destructive actions should not be considered until the end of a
> >> +      gesture has been received.
> >
> > Does this mean that a client may not destroy a gesture object after a
> > "begin" before an "end"? Why should that not be allowed?
> 
> As Peter said, that's rather a remark for clients, changed the wording
> ("permanent or irreversible" instead of "destructive"), and made it
> clear it's specific to clients.
> 
> >
> >> +    </description>
> >> +
> >> +    <request name="destroy" type="destructor">
> >> +      <description summary="destroy the pointer swipe gesture object"/>
> >> +    </request>
> >> +
> >> +    <event name="begin">
> >> +      <description summary="multi-finger swipe begin">
> >> +     This event is sent when a multi-finger swipe gesture is detected
> >> +     on the device.
> >> +      </description>
> >> +      <arg name="serial" type="uint"/>
> >> +      <arg name="time" type="uint" summary="timestamp with millisecond granularity"/>
> >> +      <arg name="surface" type="object" interface="wl_surface"/>
> >> +      <arg name="fingers" type="uint" summary="number of fingers"/>
> >> +    </event>
> >> +
> >> +    <event name="update">
> >> +      <description summary="multi-finger swipe motion">
> >> +     This event is sent when a multi-finger swipe gesture changes the
> >> +     position of the logical center.
> >> +
> >> +     The dx and dy coordinates are relative coordinates of the logical
> >> +     center of the gesture compared to the previous event.
> >> +      </description>
> >> +      <arg name="time" type="uint" summary="timestamp with millisecond granularity"/>
> >> +      <arg name="dx" type="fixed" summary="delta x coordinate in surface coordinate space"/>
> >> +      <arg name="dy" type="fixed" summary="delta y coordinate in surface coordinate space"/>
> >> +    </event>
> >> +
> >> +    <event name="end">
> >> +      <description summary="multi-finger swipe end">
> >> +     This event is sent when a multi-finger swipe gesture ceases to
> >> +     be valid. This may happen when one or more finger is lifted or
> >> +     the gesture is cancelled.
> >> +
> >> +     When a gesture is cancelled, the client should undo state changes
> >> +     caused by this gesture. What causes a gesture to be cancelled is
> >> +     implementation-dependent.
> >> +      </description>
> >> +      <arg name="serial" type="uint"/>
> >> +      <arg name="time" type="uint" summary="timestamp with millisecond granularity"/>
> >> +      <arg name="cancelled" type="int" summary="whether the gesture was cancelled"/>
> >
> > Since this is an "int" maybe it should say "1 if the gesture was
> > cancelled, otherwise 0". Or maybe be an enum?
> 
> I've been kept thinking about the enum possibility, but I could just
> come up with awkward names
> - success/cancelled (what did succeed?)
> - succeeded/cancelled (same than above, mixes passive/active forms)
> - end/cancel (the event is already named "end")
> 
> So I've finally stick to the wording you suggested above. I don't
> think we'd ever need to extend the end reasons, but AFAICS it would
> still be compatible if we switch to an enum.

I suppose what I had in mind would be the variable name "flag", and the
enum "gesture end flag" with the values "none", and "cancelled" or
something like that. But the value 0 and 1 would, as you say, be
compatible with an initial version if we switched to something like that
since we'd probably only have "none" and "cancelled" flags, so no reason
to stall on that I think.

> 
> I'm sending again the patchset with these protocol doc changes and
> fixes to your comments in the code patches. Patches 2 and 3 have been
> merged as you suggested, but there's an extra one implementing the
> weston-image zoom support.

I'll have a look!


Jonas

> 
> Cheers,
>    Carlos


More information about the wayland-devel mailing list