[PATCH wayland-protocols v2 2/2] Introduce pointer locking and confinement protocol
Jonas Ådahl
jadahl at gmail.com
Mon Jan 4 16:53:16 PST 2016
On Mon, Jan 04, 2016 at 02:21:37PM +1000, Peter Hutterer wrote:
> On Fri, Jan 01, 2016 at 04:54:14PM +0000, Daniel Stone wrote:
> > Hi,
> > A couple of (belated) comments ...
> >
> > On 3 December 2015 at 07:28, Jonas Ådahl <jadahl at gmail.com> wrote:
> > > + <request name="lock_pointer">
> > > + <description summary="lock pointer to a position">
> > > + The lock_pointer request lets the client request to disable movements of
> > > + the virtual pointer (i.e. the cursor), effectively locking the pointer
> > > + to a position. This request may not take effect immediately; in the
> > > + future, when the compositor deems implementation-specific constraints
> > > + are satisfied, the pointer lock will be activated and the compositor
> > > + sends a locked event.
> > > +
> > > + 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 satisfied. It is thus possible to request a
> > > + lock that will never activate.
> > > +
> > > + There may not be another lock of any kind requested or active on the
> > > + surface for the seat when requesting a lock. If there is, an error will
> > > + be raised. See general pointer lock documentation for more details.
> > > +
> > > + 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 whether 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.
> > > +
> > > + A surface may receive pointer focus without the lock being activated.
> > > +
> > > + The request will create a new object wp_locked_pointer which is used to
> > > + interact with the lock as well as receive updates about its state. See
> > > + the the description of wp_locked_pointer for further information.
> > > +
> > > + Note that while a pointer is locked, the wl_pointer objects of the
> > > + corresponding seat will not emit any wl_pointer.motion events, but
> > > + relative motion events will still be emitted via wp_relative_pointer
> > > + objects of the same seat. wl_pointer.axis and wl_pointer.button events
> > > + are unaffected.
> > > + </description>
> > > +
> > > + <arg name="id" type="new_id" interface="zwp_locked_pointer_v1"/>
> > > + <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 request to confine the
> > > + pointer cursor to a given region. This request may not take effect
> > > + immediately; in the future, when the compositor deems implementation-
> > > + specific constraints are satisfied, the pointer confinement will be
> > > + activated and the compositor sends a confined event.
> > > +
> > > + 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
> > > + whether 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 wp_confined_pointer which is used
> > > + to interact with the confinement as well as receive updates about its
> > > + state. See the the description of wp_confined_pointer for further
> > > + information.
> > > + </description>
> > > +
> > > + <arg name="id" type="new_id" interface="zwp_confined_pointer_v1"/>
> > > + <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>
> >
> > I almost wonder if we couldn't make peoples' lives easier by merging
> > locking and confinement into a single interface, adding a bool for
> > whether or not to allow pointer movement (confine) or not (lock). Or
> > perhaps, rather than a bool, we could add an allow-null
> > wp_relative_pointer argument instead: gives you lock if non-null, or
> > confine if null.
>
> you get a much less expressive API, less checking, etc. for a fairly minimal
> benefit. Furthermore, any extension to *either/or* the lock/confine
> interface would need to handle the appropriate NULL cases for the other
> interface. Right now we can easily add a locking-specific request without
> messing up the logic of the confinement, and vice versa.
>
> I don't think merging the two is a good long-term strategy.
I agree with Peter here. I don't think it helps anyone to merge them,
even if it may only mean a couple of requests that are no-ops in either
mode. There is no real complexity in creating different types of
objects, and I see no advantage in merging.
>
> > Also, for symmetry with wp_relative_pointer, should this take a
> > wl_pointer instead of wl_seat?
>
> you'd be restricting locking and confinement to a pointer device, with a
> wl_seat you can confine and lock devices that are not wl_pointer devices,
> e.g. a wl_tablet tool in relative mode.
>
> the "pointer" vs "wl_pointer" ambiguity isn't helping here, I admit.
A tablet will still have a "pointer" that is locked, so maybe it's not
that bad?
>
> > > + <interface name="zwp_locked_pointer_v1" version="1">
> > > + <description summary="receive relative pointer motion events">
> > > + The wp_locked_pointer interface represents a locked pointer state.
> > > +
> > > + While the lock of this object is active, the wl_pointer objects of the
> > > + associated seat will not emit any wl_pointer.motion 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 wp_locked_pointer object.
> > > +
> > > + If the compositor decides to unlock the pointer the unlocked event is
> > > + sent. The wp_locked_pointer object is at this point defunct and should be
> > > + destroyed.
> >
> > Egads. :( This is a bit hostile - what's the harm in allowing latent
> > objects to remain? To my mind, once a previously-locked/confined
> > wp_{locked,confined}_pointer object is unlocked, it is for all intents
> > and purposes the same as one which just hasn't yet been activated.
>
> what happens whe the wl_pointer goes away on the wl_seat? does the client
> expect the wp_locked_pointer object to hang around and reattach to the new
> pointer should it come back on the seat?
> see the mess with documenting the capabilities and lifetime of wl_pointer
> objects, forcing this object to be dead avoids all that for very little
> cost.
>
> Cheers,
> Peter
>
> > It
> > also introduces an annoying race where if you recreate the object just
> > after you receive focus (not unlikely for transient loss of focus,
> > e.g. cancelled Alt-Tab or temporary notifications ... ?), then you'll
> > receive a different event stream to if you won the race.
I wonder if the race could be fixed if we introduce methods to force the
server to treat a series of requests "atomically" i.e. wait for all
those requests and invoke them without drawing. It'd fix various other
of these races. A wl_transaction of some kind.
> >
> > There shouldn't be any need the event to prompt destruction anyway, if
> > you define the conditions under which a client must destroy more
> > clearly: i.e. when the wp_relative_pointer (for locks) or
> > wl_pointer/wl_seat (for confinement) is destroyed, or when the
> > wl_surface is destroyed.
> >
> > > + If the surface the lock was requested on is destroyed and the lock is not
> > > + yet activated, the wp_locked_pointer object is now defunct and must be
> > > + destroyed.
> >
> > I think you can delete '... and the lock is not yet activated',
> > assuming that the intention here was just to make sure that people
> > process outstanding events before they destroy the object. If they've
> > deleted the surface, presumably any input events are going to be
> > discarded as being meaningless anyway.
Makes sense.
> >
> > > + <request name="set_region">
> > > + <description summary="set a new lock region">
> > > + Set a new region used to lock the pointer.
> > > +
> > > + The new lock region is double-buffered. The new lock region will
> > > + only take effect when the associated surface gets its pending state
> > > + applied. See wl_surface.commit for details.
> > > +
> > > + The new region has no effect on a lock that has already been activated.
> > > +
> > > + For details about the lock region, see wp_locked_pointer.
> > > + </description>
> > > +
> > > + <arg name="region" type="object" interface="wl_region" allow-null="true"
> > > + summary="region of surface"/>
> > > + </request>
> >
> > If unlocked/unconfined didn't require destruction and recreation of
> > the object (as above), the wording of 'new region has no effect' would
> > become ambiguous here: it could mean 'this state is double-buffered on
> > lock/confine activation state and the update will be applied the next
> > time the region is unlocked', or 'any changes requested whilst a lock
> > is active will be discarded'. The latter reading makes it unreliable,
> > and the former makes implementing it more of a pain.
The double-bufferedness is only to make it possible to match the region
with the surface content. The "locked, now discarding" is just for
locked pointers meaning there is no point in changing the region after
it was locked. If we would need to reword things indeed. something like
that its always double buffered.
> >
> > > + <event name="locked">
> > > + <description summary="lock activation event">
> > > + Notification that the pointer lock of this seat's pointer is activated.
> > > + </description>
> > > + </event>
> > > +
> > > + <event name="unlocked">
> > > + <description summary="lock deactivation event">
> > > + Notification that the pointer lock of seat's pointer is no longer
> > > + active. This object is now defunct and should be destroyed.
> > > + </description>
> > > + </event>
> >
> > I think these (and ditto for confinement) need clarification of their
> > relationship to wl_pointer::{enter,leave}. Are they still sent? What
> > about in corner cases such as destroying a currently-active
> > lock/confinement region, i.e. where focus is retained and ownership
> > thus passes to wl_pointer?
I guess they could be clarified, at least say that
"unlocked"/"unconfined" will always come before any "leave" even though
there may be a "unlocked"/"unconfined" withoun any "leave" if the
pointer didn't leave the surface.
Jonas
> >
> > Other than that, looks pretty good to me! Should've reviewed this earlier ... :\
> >
> > Cheers,
> > Daniel
More information about the wayland-devel
mailing list