[PATCH weston v2 14/21] Introduce pointer locking and confinement protocol
Peter Hutterer
peter.hutterer at who-t.net
Fri Jun 5 00:03:40 PDT 2015
On Thu, Jun 04, 2015 at 05:05:55PM +0800, Jonas Ådahl wrote:
> On Fri, May 29, 2015 at 09:23:29AM +1000, Peter Hutterer wrote:
> > 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.
>
> That is true. In the protocol they kind of already are (since you lock a
> pointer on a seat); that was the intention anyway. I'll make it more
> clear in the next version.
>
> Implementation wise, it is not supported at all, so I'll have to take a
> look on how to make the implementation lockable per seat. The
> restrictions on when the lock may be activated (now click-to-activate)
> might get a bit more funky, since we now need to track that also per
> seat.
>
> In short, I will spell out that there may only be one active lock *per
> surface and per seat*, so that a client can lock multiple seats on
> the same surface.
>
> >
> > > + </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?
>
> I wonder if the only scenario that can happen is when a client provides
> broken input, such as the calculated lock/confine region is empty, we
> can just kill it with an error. Or are there scenarios where the client
> may not be able to predict the lockability and needs to know about it?
> Right now I can't think of any.
I can't think of any either at this point, maybe in some nested conditions?
not a big issue anyway.
> >
> > > +
> > > + 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.
>
> Wanted to make it extra obvious, but might be a bit superfluous :P
>
> >
> > > + 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.
>
> I see a pointer lock/confinemet similar to an explicit grab. There was
> previous discussion about that (that might have been bumped to the top
> of your inbox today) that brings this up. If you have any opinions on
> that it'd be helpful. And just to understand your meaning, do you see
> it more sensible to *not* grab the keyboard focus on lock? Naturally,
> how the events (enter/leave etc) won't change, what matters is whether
> locking takes grabs or not.
I guess my main point here is that it should be specified, even if the
actual wording is "compositor dependent".
I think in most cases a lock should effectively take the keyboard, but I
don't think that should be specified in the protocol. Rather specify that
the pointer lock has no effect on keyboard events and the client should
treat keyboard focus events independently.
> > 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.
>
> Probably makes sense to force the client to unlock before destroying and
> send an error otherwise. Seems a bit undefined what the intetion of a
> lock on a destroyed surface means.
it's fine to have undefined behaviour but that should simply be specified.
the unlocked event doesn't contain the surface itself so it' be fine to send
an unlocked event to the client. that keeps the protocol symmetric and it's
up to the client to deal with the events.
I dare say that the safest behaviour for most use-cases is simply to unlock
and send an event.
> Another state change that needs to be clarified is what happens during
> surface resize. My gut feeling is that for a pointer lock, nothing
> happens (the pointer stays locked), but that for a confined pointer, the
> confinement is cancelled. This means we'd not support confinement with
> resizing surfaces, but I think that is fine. Do you have any opinions on
> that?
based on the IRC discussion it looks like:
1) the client will have to deal with unconfinement during resize anyway since
the compositor may break at any time
2) after a resize the confine region is likely incorrect until the client
updates it.
so yeah, I agree that resizing a surface should break confinement.
not 100% on the pointer lock yet but I think that can remain.
but it brings up a new question: should there be an "update" request to
update the confinement region within the surface? otherwise the client would
have to destroy and re-request a confinement which could require
re-triggering the compositor conditions to activate the confinement.
Cheers,
Peter
More information about the wayland-devel
mailing list