[PATCH wayland-protocols v2 2/2] Introduce pointer locking and confinement protocol

[Jonas Ådahl]

On Tue, Jan 12, 2016 at 02:48:38PM +0800, Jonas Ådahl wrote:
> On Sat, Jan 09, 2016 at 04:38:51PM +0000, Daniel Stone wrote:
> > Hi,
> > 
> > On 4 January 2016 at 04:21, Peter Hutterer <peter.hutterer at who-t.net> wrote:
> > > On Fri, Jan 01, 2016 at 04:54:14PM +0000, Daniel Stone wrote:
> > >> 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'm not deeply wedded to this (hence 'almost wonder'), but just as
> > long as we're aware of the downside that it does make compositor
> > implementations significantly more complex - see the infrastructure
> > required to support both interfaces whilst mostly treating them as the
> > same interface inside the Weston patch. So, if you're both against it,
> > then shrug, fair enough.
> > 
> > >> > +      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.
> > 
> > Eh? Destroying the object after a single use doesn't make the problem
> > go away, because you still have to specify what happens to a
> > wp_locked_pointer on which the lock has never triggered, when the
> > pointer goes away. Arguably it just makes clients _more_ likely to get
> > that case wrong.
> > 
> > I don't see what making the object single-use buys us. Surely the
> > common usecase is to react to destruction by immediately recreating a
> > new object with the exact same properties. And it introduces a really
> > annoying corner case where input arrives before you have the time to
> > recreate (say you have a very temporal exit), in which you receive
> > normal rather than locked pointer events. Given that it makes clients
> > more complex, introduces an unfortunate race condition, doesn't avoid
> > the case that is its stated reason to exist, and also makes protocol
> > tracing a bit more difficult, why do it?
> 
> FWIW, I'd be fine making the objects outlive an unlock. It'd need
> changes to set_region etc, but thats trivial. The pointer seat
> capability going away I would make it do the same as wl_pointer i.e.
> it'll become useless, won't ever re-lock and should be destroyed.

Come to think of it, we would still introduce a race condition, just for
the other use case. If the intention is that the lock should be
one-shot, the client may not be able to respond with a .destroy() in
time, resulting in the compositor potentially re-locking when that is
not what the client intended. To support both use cases race free we'd
need to bake that into the request creating the lock. Is it worth
complicating the request for fixing both races?


Jonas