On Tue, Jan 05, 2016 at 09:21:27AM +0800, Jonas Ådahl wrote:
> 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 <jad...@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.
> >
> > > 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.
>
> Then again... how would a single "lock" request deal with multiple
> tablet pointers? Seems we would need a request per device, i.e.
> lock_pointer(wl_pointer) and a lock_tablet_pointer(wp_tablet). So maybe
> its best to make lock/confine take a wl_pointer to begin with.
I was about to make an argument against this and then I realised that the
reasoning here is the same as a few paragraphs above where we both agree
that separate interfaces is better. So now I'm out of arguments, wl_pointer
it is :)
Cheers,
Peter
> > the "pointer" vs "wl_pointer" ambiguity isn't helping here, I admit.
> >
> > > > + <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.
> > >
> > > 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.
> > >
> > > > + <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.
> > >
> > > > + <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?
> > >
> > > Other than that, looks pretty good to me! Should've reviewed this earlier
> > > ... :\
> > >
> > > Cheers,
> > > Daniel
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
