[PATCH weston 1/3] Introduce pointer lock interface

[Jonas Ådahl]

On Mon, Sep 22, 2014 at 11:31:27AM -0700, Jason Ekstrand wrote:
> On Sun, Sep 21, 2014 at 11:43 PM, Pekka Paalanen <ppaalanen at gmail.com>
> wrote:
> 
> > On Sat, 20 Sep 2014 11:43:22 -0700
> > Jason Ekstrand <jason at jlekstrand.net> wrote:
> >
> > > On Sat, Sep 20, 2014 at 1:29 AM, Pekka Paalanen <ppaalanen at gmail.com>
> > wrote:
> > >
> > > > On Fri, 19 Sep 2014 20:20:53 +0200
> > > > Jonas Ådahl <jadahl at gmail.com> wrote:
> > > >
> > > > > On Fri, Sep 19, 2014 at 11:00:21AM -0600, Jasper St. Pierre wrote:
> > > > > > On Fri, Sep 19, 2014 at 2:54 AM, Pekka Paalanen <
> > ppaalanen at gmail.com>
> > > > wrote:
> > > > > >
> >
> > > > > > > > non-sensible requests (set_cursor) on the additional
> > wl_pointer,
> > > > and
> > > > > > > > potentially more as wl_pointer is extended. The alternatives
> > that
> > > > has
> > > > > > > > come up in IRC discussions and backlog reading are:
> > > > > > > >
> > > > > > > >  1) Create a wl_relative_pointer that would replace wl_pointer,
> > > > i.e.
> > > > > > > >     have its own button, axis, etc events. This would solve all
> > > > problems
> > > > > > > >     related to versioning and wl_pointer being extended, but
> > > > brings a
> > > > > > > >     bunch of other problems for example we'd have to extend
> > > > > > > >     wl_relative_pointer each time wl_pointer is extended to
> > keep
> > > > up, as
> > > > > > > >     well as the issue where a wl_pointer is used in a
> > interface or
> > > > > > > >     request.
> > > > > > >
> > > > > > > Either you use wl_pointer and get extensions automatically even
> > if
> > > > you
> > > > > > > don't want to, or you use a new interface that you have to extend
> > > > > > > explicitly if you want the same as in wl_pointer. Two sides of a
> > > > coin.
> > > > > > >
> > > > > > > As concluded in IRC, wl_pointer is not used as argument in any
> > > > messages.
> > > > >
> > > > > But it is by wl_pointer_lock.lock_pointer, and who says it'll be the
> > only
> > > > > place its used as an argument?
> > > >
> > > > I think wl_seat might be a better argument. After all, all
> > > > wl_pointer objects created from the same wl_seat in the same way
> > > > will be identical. I think we should keep it that way, too.
> > > >
> > >
> > > That's a decent idea.  We could simply have
> > > wl_pointer_lock.get_relative_pointer(seat) which creates a relative
> > > pointer.  At that point, all the absolute pointers get a leave.  The
> > > client, if it wants, can create multiple relative pointers.  Once all of
> > > them are destroyed, the absolute pointers get an enter again, and off we
> > go.
> >
> > Mind, you are saying that simply a client creating the
> > wl_relative_pointer will never get normal wl_pointer events as long as
> > a wl_relative_pointer object exists. Is this a deliberate design
> > choice? You seem to contradict it right below here with the leave
> > events.
> >
> > > One note though:  I don't think we want to try too hard to make multiple
> > > pointers per seat seamless.  That's going to be an awkward case no matter
> > > what we do.  We do need to make it well-defined.  I think that having all
> > > the absolute pointers get a leave is a reasonable way to do that.  If the
> > > compositor has to break the lock, all the relative pointers should get a
> > > leave and absolute motion resumes.  (It probably won't get an absolute
> > > enter right away.  The use probably went to an expose or alt-tab view so
> > it
> > > probably would have gotten an absolute leave anyway.)
> >
> > Yes, that is a logical consequence of the "all <wl_* input device type>
> > objects for the same wl_seat and created the same way are
> > identical." :-)
> >
> > It's actually a pretty logical implication from the globals: binding
> > several times to a specific global name creates identical objects that
> > all refer to the same thing, and act the same way at the same time.
> >
> > This might be a good design rule to set, don't you think?
> >
> > > > > Hmm. Maybe we need to lock pointer given a seat instead. It'd change
> > the
> > > > > "mode" of the underlying pointer to be locked in its position, i.e.
> > not
> > > > > send any wl_pointer.motion events. Locking could then be ref
> > counted, so
> > > > > that as long as more than one wl_relative_pointer is alive, it's in
> > > > > relative mode. Maybe thats unnecessarily complex though.
> > > >
> > > > We are distinguishing "wl_relative_pointer exists" from
> > > > "pointer-lock is active", right?

I think we should at least.

> > > >
> > > > A question is: how does the client know when pointer-lock is
> > > > active? I think we'd need something like
> > > > wl_relative_pointer.enter/leave.
> > > >
> > >
> > > I'm still a fan of destroying relative pointers when not in use.  It's a
> > > bit more protocol churn, but it makes it clear when they are/are not
> > > active.  Maybe enter/leave will work, but I'm not convinced it's the
> > right
> > > way forward.  I'll have to give that a bit more thought before I could
> > give
> > > a real solid argument though.
> >
> > Yes, of course. It's more a matter of how does the client know to
> > destroy the wl_relative_pointer, when the compositor chooses to
> > deactivate the pointer-lock. If there is a leave event for that, good.
> >
> > I think with this, it could actually make sense what you said above: no
> > normal wl_pointer events as long as any wl_relative_pointer exists.
> > This would strongly push app devs to destroy the wl_relative_pointer at
> > the right time, I believe.
> >
> > And then we need a backup plan for clients that do not destroy it, for
> > protocol definition completeness.
> >
> 
> If the server sends a "leave" it becomes inert and the server assumes the
> client will destroy it.  If the client doesn't then you leak an object but
> there's no real harm.

So if a lock will be made invalid if it was deactivated when should the
client re-request the pointer lock? On activation could cause unwanted
pointer locking when the focus model is focus-follow-mouse (or actually
activation-follow-mouse which would be the equivalent I suppose) or does
the client need to keep a state of how a surface was activated in order
to postpone the actual locking?

Anyhow, given that there are enter/leave events sent to the
wl_relative_pointer object, what is left unclear regarding if a pointer
is or is not locked? Whats the benefit of invalidating deactivated
locks, and is it worth that the compositor will have less say about
relocking and rely on the client to know when its appropriate to do so?

> 
> 
> >
> > > > > I can imagine that the second use case may very well be implemented
> > both
> > > > > with confinement and relative pointer events and as such relative
> > > > > pointer events would need support both non-accelerated and
> > accelerated
> > > > > modes.
> > > >
> > > > IMHO that raises the question: should be have separate interfaces,
> > > > or at least separate requests for asking raw relative motion vs.
> > > > desktop-like relative (i.e. accelerated) motion?
> > > >
> > > > Or should wl_relative_pointer maybe deliver both at the same time?
> > > >
> > > > Maybe have separate requests for creating the wl_relative_pointer...
> > > >
> > >
> > > Both of those seem reasonable to me.
> >
> > I would suggest we go with a separate request with "raw" input data.
> > This way we can easily add it later. I suspect it may also need
> > libinput additions. We don't even have a good definition of "raw" yet,
> > either, AFAIK. Going with just the normal (accelerated) motion for now
> > should be easier to try out.
> >
> 
> fine with me

I think a separate wl_relative_pointer is overkill, but instead of
separate requests, could we not just have an "option" bitmask? Because
there really isn't much difference between a lock_pointer and
lock_pointer_noaccel. Haven't thought of any options more than "noaccel"
though, and maybe its a bad idea add more. Hmm.

Another thing that I thought of is that relative motions may not
necessarily be enough for confined absolute motion events, and that is
because of transformations. If we want to allow a client to *only*
confine the cursor, but still have it behave just like a regular cursor,
then surface transformations must be taken into account. While it could
be relevant to put such a thing in wl_pointer_lock, I guess it's better
to focus on only relative motion events first.


Jonas

> 
> 
> >
> >
> > Thanks,
> > pq
> >