[PATCH weston v2 14/21] Introduce pointer locking and confinement protocol

[Peter Hutterer]

On Wed, May 13, 2015 at 06:26:35PM +0800, Jonas Ådahl wrote:
> This patch introduces a new protocol for locking and confining a
> pointer. It consists of a new global object with two requests; one for
> locking the surface to a position, one for confining the pointer to a
> given region.
> 
> See pointer-lock.xml for details of the protocol.
> 
> In this patch, only the locking part is fully implemented as in
> specified in the protocol, while confinement is only implemented for
> when the union of the passed region and the input region of the confined
> surface is a single rectangle.
> 
> Note that the interfaces are prefixed with an underscore in order to
> avoid future incompatibilities with a future stable interface with an
> equivalent name.
> 
> Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> ---
>  Makefile.am               |   3 +
>  protocol/pointer-lock.xml | 208 +++++++++++++++++
>  src/compositor.c          |   4 +
>  src/compositor.h          |  21 ++
>  src/input.c               | 576 ++++++++++++++++++++++++++++++++++++++++++++--
>  5 files changed, 796 insertions(+), 16 deletions(-)
>  create mode 100644 protocol/pointer-lock.xml
> 
> diff --git a/Makefile.am b/Makefile.am
> index 70c436f..201b780 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -106,6 +106,8 @@ nodist_weston_SOURCES =					\
>  	protocol/presentation_timing-server-protocol.h	\
>  	protocol/scaler-protocol.c			\
>  	protocol/scaler-server-protocol.h		\
> +	protocol/pointer-lock-protocol.c		\
> +	protocol/pointer-lock-server-protocol.h		\
>  	protocol/relative-pointer-protocol.c		\
>  	protocol/relative-pointer-server-protocol.h
>  
> @@ -1186,6 +1188,7 @@ EXTRA_DIST +=					\
>  	protocol/scaler.xml			\
>  	protocol/ivi-application.xml		\
>  	protocol/ivi-hmi-controller.xml		\
> +	protocol/pointer-lock.xml		\
>  	protocol/relative-pointer.xml
>  
>  #
> diff --git a/protocol/pointer-lock.xml b/protocol/pointer-lock.xml
> new file mode 100644
> index 0000000..5e7dd9c
> --- /dev/null
> +++ b/protocol/pointer-lock.xml
> @@ -0,0 +1,208 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<protocol name="pointer_lock">
> +
> +  <copyright>
> +    Copyright © 2014      Jonas Ådahl
> +
> +    Permission to use, copy, modify, distribute, and sell this
> +    software and its documentation for any purpose is hereby granted
> +    without fee, provided that the above copyright notice appear in
> +    all copies and that both that copyright notice and this permission
> +    notice appear in supporting documentation, and that the name of
> +    the copyright holders not be used in advertising or publicity
> +    pertaining to distribution of the software without specific,
> +    written prior permission.  The copyright holders make no
> +    representations about the suitability of this software for any
> +    purpose.  It is provided "as is" without express or implied
> +    warranty.
> +
> +    THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> +    SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
> +    FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
> +    SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> +    WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN
> +    AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
> +    ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
> +    THIS SOFTWARE.
> +  </copyright>

note: all my comments below are just looking at the protocol definition.
some of the answers may be in your implementation, but that's not where they
should be ;)

> +
> +  <interface name="_wl_pointer_lock" version="1">
> +    <description summary="lock pointer to a surface">
> +      The global interface exposing pointer locking functionality. It exposes
> +      two requests; lock_pointer for locking the pointer to its position, and
> +      confine_pointer for locking the pointer to a region.
> +
> +      The lock_pointer and confine_pointer creates the objects wl_locked_pointer
> +      and wl_confined_pointer respectively, and the client can use these objects
> +      to interact with the lock.
> +
> +      There may not be another lock of any kind active when requesting a lock,
> +      and if there is, an error will be raised.

"Only one lock or confinement may be active at any time. If a lock or
confinement is requested when another lock or confinement is active, an
error is raised."

do you need to document what error is sent?

also, I'm not sure this will work anyway: there is a nonspecified time between the
request and the actual lock. that guarantees that you _can_ have multiple
locks if you have two or more clients (c1, c2)

1. c1 request lock on surface s1
1.a compositor holds because lock condition isn't met
2. c2 request lock on surface s2
2.a compositor holds because lock condition isn't met
3. lock condition for s1 is met
3.a compositor sends locked to c1
4. c3 requests lock on surface s3
4.a compositor sends error because lock is active

now, I can find a number of ways to interpret the protocol so that the above
isn't actually the case, but none of them are spelled out :)

The locked event already provides enough information for when you have the
lock, so a simple restriction of one lock request per surface and/or region
should be enough.

with multiple seats you can have multiple locks, so I think seats should be
part of the first protocol, or at least a big FIXME so we don't forget.

> +    </description>
> +
> +    <request name="lock_pointer">
> +      <description summary="lock pointer to a position">
> +        The lock_pointer request lets the client disable absolute pointer
> +        movements, locking the pointer to a position.

iirc, the X protocol uses the terms "In the future..." for pointer grabs
which I think would be useful here too to remove ambiguity. add something
like:
   The lock_pointer request lets the client disable absolute pointer
   movements, locking the pointer to a position. In the future, when the
   compositor deems implementation-specific constraints are satisfied, the
   pointer lock will activate and the compositor sends a locked event.

   The constraints to activate the pointer lock are implementation-specific.
   The protocol provides no guarantee that the constraints are ever
   satisfied, and does not require the compositor to send an error if the
   constraints cannot ever be satisifed. It is thus possible to request a lock
   that will never activate.

and speaking of that, how do we let a compositor say "yeah, nah, that's not
going to happen" when the lock is impossible to satisfy?

> +
> +        There may not be another lock of any kind active when requesting a lock,
> +        and if there is, an error will be raised.

see above

> +
> +        The intersection of the region passed with this request and the input
> +        region of the surface is used to determine where the pointer must be
> +        in order for the lock to activate. It is up to the compositor to warp
> +        the pointer, or require some kind of user interaction for the lock to
> +        activate. If the region is null the surface input region is used.

Add:
"The activation of the lock is implementation defined, the pointer may enter
the region without the lock activating."

> +
> +        The request will create a new object wl_locked_pointer which is used to
> +        interact with the lock as well as receive updates about its state. See
> +        the the description of wl_locked_pointer for further information.
> +
> +        Note that while a locked pointer doesn't move its absolute position, it

I don't think you should specify this in terms of behaviour but rather in
terms of protocol, i.e. a locked pointer does not generate wl_pointer.motion
events. what the visible pointer does is still up to the compositor and it
could be acceptable for a exposé- or dashboard-like functionality to unlock
the pointer without actually unlocking it on the protocol.

> +        may still emit relative motion events via the wl_relative_pointer
> +        object.
> +      </description>
> +
> +      <arg name="id" type="new_id" interface="_wl_locked_pointer"/>
> +      <arg name="surface" type="object" interface="wl_surface"
> +           summary="surface to lock pointer to"/>
> +      <arg name="seat" type="object" interface="wl_seat"
> +           summary="seat where the pointer should be locked"/>
> +      <arg name="region" type="object" interface="wl_region" allow-null="true"
> +           summary="region of surface"/>
> +    </request>
> +
> +    <request name="confine_pointer">
> +      <description summary="confine pointer to a region">
> +        The confine_pointer request lets the client confine the pointer cursor
> +        to a given region.
> +
> +        The intersection of the region passed with this request and the input
> +        region of the surface is used to determine where the pointer must be
> +        in order for the confinement to activate. It is up to the compositor to
> +        warp the pointer, or require some kind of user interaction for the
> +        confinement to activate. If the region is null the surface input region
> +        is used.
> +
> +        The request will create a new object wl_confined_pointer which is used
> +        to interact with the confinement as well as receive updates about its
> +        state. See the the description of wl_confined_pointer for further
> +        information.
> +      </description>
> +
> +      <arg name="id" type="new_id" interface="_wl_confined_pointer"/>
> +      <arg name="surface" type="object" interface="wl_surface"
> +           summary="surface to lock pointer to"/>
> +      <arg name="seat" type="object" interface="wl_seat"
> +           summary="seat where the pointer should be locked"/>
> +      <arg name="region" type="object" interface="wl_region" allow-null="true"
> +           summary="region of surface"/>
> +    </request>
> +
> +  </interface>
> +
> +  <interface name="_wl_locked_pointer" version="1">
> +    <description summary="receive relative pointer motion events">
> +      The wl_locked_pointer interface represents a locked pointer state.
> +
> +      While the lock of this object is active, the pointer of the associated
> +      seat will not move.

see above, this should only be expressed in terms of protocol events.

> +
> +      This object will send the event 'locked' when the lock is activated.
> +      Whenever the lock is activated, it is guaranteed that the locked surface
> +      will already have received pointer focus and that the pointer will be
> +      within the region passed to the request creating this object.
> +
> +      To unlock the pointer, send the destroy request. This will also destroy
> +      the wl_locked_pointer object.
> +
> +      If the compositor decides to unlock the pointer the unlocked event is sent.
> +      The wl_locked_pointer object is at this point defunct and should be
> +      destroyed.
> +
> +      When unlocking, the compositor may or may not take the cursor position

may is a superset of may not :)
but if you want to leave this in that's fine with me.

> +      hint provided using the set_cursor_position_hint request and warp the
> +      pointer. If it does, it will not result in any relative motion events.
> +    </description>
> +
> +    <request name="set_cursor_position_hint">
> +      <description summary="set the pointer cursor position hint">
> +        Set the cursor position hint relative to the top left corner of the
> +        surface.
> +
> +        If the client is drawing its own cursor, it should update the position
> +        hint to the position of its own cursor. A compositor may use this
> +        information to warp the pointer upon unlock in order to avoid pointer
> +        jumps.
> +      </description>
> +
> +      <arg name="surface_x" type="fixed"
> +           summary="x coordinate in surface-relative coordinates"/>
> +      <arg name="surface_y" type="fixed"
> +           summary="y coordinate in surface-relative coordinates"/>
> +    </request>
> +
> +    <request name="destroy" type="destructor">
> +      <description summary="destroy the locked pointer object">
> +        Destroy the locked pointer object. The compositor will unlock the
> +        pointer.
> +      </description>
> +    </request>
> +
> +    <event name="locked">
> +      <description summary="enter event">

summary="lock activation event"

> +        Notification that the pointer lock of this seat's pointer is activated.
> +      </description>
> +    </event>
> +
> +    <event name="unlocked">
> +      <description summary="leave event">

summary="lock deactivation event"

> +        Notification that the pointer lock of seat's pointer is no longer
> +        active. This object is no defunct and should be destroyed.

typo: now

> +      </description>
> +    </event>
> +  </interface>
> +
> +  <interface name="_wl_confined_pointer" version="1">
> +    <description summary="confined pointer object">
> +      The wl_confined_pointer interface represents a confined pointer state.
> +
> +      This object will send the event 'confined' when the confinement is
> +      activated. Whenever the confinement is activated, it is guaranteed that
> +      the surface the pointer is confined to will already have received pointer
> +      focus and that the pointer will be within the region passed to the request
> +      creating this object. It is up to the compositor to decide whether this
> +      requires some user interaction and if the pointer will warp to within the
> +      passed region if outside.
> +
> +      To unconfine the pointer, send the destroy request. This will also destroy
> +      the wl_confined_pointer object.
> +
> +      If the compositor decides to unconfine the pointer the unconfined event is
> +      sent. The wl_confined_pointer object is at this point defunct and should
> +      be destoryed.

typo

> +    </description>
> +
> +    <request name="destroy" type="destructor">
> +      <description summary="destroy the confined pointer object">
> +        Destroy the confined pointer object. The compositor will unconfine the
> +        pointer.
> +      </description>
> +    </request>
> +
> +    <event name="confined">
> +      <description summary="enter event">
> +        Notification that the pointer confinement of this seat's pointer is
> +        activated.
> +      </description>
> +    </event>
> +
> +    <event name="unconfined">
> +      <description summary="leave event">
> +        Notification that the pointer confinement of seat's pointer is no
> +        longer active. This object is no defunct and should be destroyed.
> +      </description>
> +    </event>
> +  </interface>
> +
> +</protocol>

one thing I miss here is the specification of what happens with keyboard
focus. is there any guarantee that a locked pointer has keyboard focus too,
do the keyboard focus events get sent normally, etc. I think the latter is
sensible enough to do and it allows a client to unlock on focus-out. but
either way, this should be stated for less ambiguity.

missing is also the various constraints on when something happens with the
surface (e.g. surface is destroyed). I guess most of that is "pointer will
be unlocked" but it's again good to spell out.

Cheers,
   Peter