[PATCH weston 1/3] Introduce pointer lock interface

[Jason Ekstrand]

On Fri, Sep 19, 2014 at 11:20 AM, 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:
> >
> > > On Fri, 19 Sep 2014 08:33:13 +0200
> > > Jonas Ådahl <jadahl at gmail.com> wrote:
> > >
> > > > On Thu, Sep 18, 2014 at 10:43:47AM -0700, Jason Ekstrand wrote:
> > > > > On Thu, Sep 18, 2014 at 2:26 AM, Pekka Paalanen <
> ppaalanen at gmail.com>
> > > wrote:
> > > > >
> > > > > > On Wed, 17 Sep 2014 22:35:40 +0200
> > > > > > Jonas Ådahl <jadahl at gmail.com> wrote:
> > > > > >
> > > > > > > On Wed, Sep 17, 2014 at 11:16:06PM +0300, Giulio Camuffo wrote:
> > > > > > > > 2014-09-17 23:11 GMT+03:00 Jonas Ådahl <jadahl at gmail.com>:
> > > > > > > > > On Wed, Sep 17, 2014 at 10:56:13PM +0300, Giulio Camuffo
> wrote:
> > > > > > > > >> I haven't looked at the implementation yet, just at the
> > > protocol,
> > > > > > but
> > > > > > > > >> isn't _wl_pointer_lock.lock_pointer() returning a new
> > > wl_pointer a
> > > > > > > > >> problem? Objects should have a unique factory interface,
> or
> > > else the
> > > > > > > > >> version of the interface can't be determined.
> > > > > > > > >
> > > > > > > > > Is it really? In the implementation below, the wl_pointer
> > > object gets
> > > > > > > > > the same version as the wl_pointer object that is locked.
> It
> > > is also
> > > > > > > > > specified in the last paragraph of
> > > _wl_pointer_lock.lock_pointer.
> > > > > > > >
> > > > > > > > Mmh, then maybe it is fine. I'm not convinced actually, but
> I'm
> > > too
> > > > > > > > tired now. :)
> > > > > >
> > > > > > No, it's not fine. You cannot define exceptions to the versioning
> > > rules
> > > > > > in a protocol spec. We have common code in libwayland handling
> all
> > > > > > runtime versioning, and you just cannot implement any exceptions.
> > > > > >
> > > > >
> > > > > Technically, those haven't been merged yet... So, technically, we
> could
> > > > > have it inherit the version from the wl_pointer.  That said, I
> think
> > > we'll
> > > > > regret if we do.  Even if we did do that, it would cause problems,
> not
> > > > > because we couldn't update wl_pointer, but because we really
> couldn't
> > > > > update wl_pointer_lock.  All in all, it's a bad plan.
> > > > >
> > > >
> > > > So, from what I've heard here now is it's clear we don't want to
> create
> > > > a new wl_pointer object. Other than the versioning issue, we'd have
> > >
> > > You could create a wl_pointer, if you do it from interface whose
> > > ancestor is wl_seat, or directly a request in wl_seat. But it does have
> > > downsides like not being able to let the extension mature in Weston
> > > before moving it to Wayland core.
>
> I think its a good practice to have a isolateable kind of way of
> extending protocols (ala wl_relative_pointer, wl_scaler, etc) so that
> core protocol objects don't grow too large with non-removable request
> and events.
>
> Maybe we need to define "modes of operation" that would be similar to
> "roles" (e.g. xdg_surface), so that wl_relative_pointer changes the
> "mode" of wl_pointer to wl_relative_pointer. This would allow us to add
> a wl_pointer_lock.confine_pointer that doesn't make sense to enable at
> the same time as the pointer is in relative motion mode.
>

Maybe, but that sounds like a problem to solve with the second extension of
wl_pointer, not the first.  Right now, we don't have any other extensions
flying around, so we don't know how it will interact with these other
hypothetical extensions.


>
> > >
> > > > 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?
>
> > >
> > > >  2) Add "begin" and "end" events to wl_pointer_lock that changes the
> > > >     state of an existing wl_pointer. After a wl_pointer_lock.begin is
> > > >     sent, wl_pointer.motion sends relative motion events. After
> > > >     wl_pointer_lock.end it'd revert back to sending absolute motion
> > > >     events.
> > >
> > > This might be confusing if a client creates several wl_pointer objects
> > > for the same seat underlying a wl_seat.
> > >
> > > Apart from being confusing just by changing the meaning of an event
> > > via a mode.
> > >
> > > >  3) Extend wl_pointer similar to how wl_viewport extends wl_surface
> with
> > > >     a wl_relative_pointer (different from in (1)). Such a
> > > >     wl_relative_pointer would, to begin with, only have one request
> > > >     (release) and one event (motion). It wouldn't change anything to
> the
> > > >     wl_pointer object, it'd still receive button, axis etc events,
> but
> > > >     motion events are turned into relative motion events via
> > > >     wl_relative_pointer, not affecting the position of the cursor,
> i.e.
> > > >     not resulting in wl_pointer.motion events.
> > >
> > > This is an interesting idea. I wonder how it works if the client has
> > > two parts (say, libraries or toolkits with their own input processing),
> > > therefore has multiple wl_pointer objects for the same thing, and only
> > > few of them have the pointer_lock interface associated.
>
> 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.
>
> > >
> > > Or what if an app manually starts pointer-lock because the toolkit does
> > > not support it? This is not a use case to design for at all, but if it
> > > happens to work, it might be a nice bonus.
>
> If we make it so that locking simply locks the cursor at its position,
> it'd will just work, as the pointer cursor wouldn't move. No unexpected
> wierd wl_pointer.motion events.
>

I don't see any problem with locking only one of the available pointers.
The absolute ones simply get no motion until it's unlocked.  If we add a
set_position request to wl_relative_pointer, then I guess that would
trigger motion on the absolute pointers.  I think it's probably fine.


>
> > >
> > > > To me, alternative (3) seems like the best choice. It'd give us
> > > > possibility to continue to control the pointer cursor via the
> original
> > > > wl_pointer object, as it will still have focus. It also allows us to
> make
> > >
> > > I don't think we ever want the compositor drawing the cursor in
> > > pointer-locked mode... do we? Would there be any use cases benefitting
> > > from it?
> > >
> >
> > There's an additional problem here: where should the cursor be drawn?
> Will
> > wl_relative_pointer have a set_cursor_position argument while in relative
> > mode?
> >
> > I could see arguments in favor of it (ability to reuse the cursor plane),
> > but we can leave this out for now. If we want to add it back, we say that
> > the cursor is always drawn in the center of the surface, unchanging, and
> if
> > apps want to remove it, they blank it.
>
> I think that the best would be to simply not change the position or
> state of the cursor at all. Locking would mean that while locked, the
> wl_pointer has focus, meaning the client can change the cursor, or hide
> it. For example if pointer-locking would be used to click-drag to select
> items from a list (where dragging changes highlighted element), it would
> be bad if the cursor suddenly jumps to the middle of the surface when
> done.
>
> Staying where it was at the point of locking also works the same way as
> pointer locking does in GLFW. Haven't checked SDL yet.
>
> What would a set_cursor_position argument do? It sounds like that could
> be more or less used to implement actual warping by just locking, set
> position then unlocking. It'd probably be more useful to have a
> set_cursor_position request in wl_relative_pointer that can be used when
> implementing pointer warping in Xwayland and then possibly even SDL and
> GLFW. Hmm.
>

I'm not 100% sure what you're getting at here.  However, I think we do want
to have cases where the client is drawing a warped pointer and then the the
pointer-lock breaks.  One example would be a VM that uses pointer_lock to
give relative events to the client machine.  In that case, when the you
move the pointer to the edge of the window, it sets the pointer position
and breaks the lock.  This way it looks like the pointer is simply moving
through the window.  RTS games will probably want to do similar things.  I
don't think "just lock it where it is" is always going to work.


>
> >
> >
> > > Though, I don't see anything wrong with requiring the client to
> > > explicitly unset the cursor when pointer-lock activates. Or are there
> > > any room for races where the cursor might flicker?
>
> Well, except for the buffer queue extension, is there any way for two
> surfaces to synchronize their rendering (assuming they are not not in a
> subsurface tree)?
>
> > >
> > > > use of any future wl_pointer additions (problem being that they could
> > > > potentially conflict, but that wouldn't be unique for
> > > > wl_relative_pointer AFAIK). Alternative (2) would give us the same
> > > > benefits, but it feels rather awkward to temporarily change the
> meaning
> > > > of wl_pointer.motion.
> > >
> > > Agreed about 2).
> > >
> > > > Given that the three above choices are more or less semantically
> > > > equivalent it makes sense to try it out in a already used API. From
> what
> > > > I can see, the two common places it'd be used are SDL and GLFW, which
> > > > have different API's doing more or less the same thing, so it'd
> probably
> > > > be good to try it out in both of these.
> > >
> > > A very good idea. That is how we get some perspective to the design and
> > > find issues.
> > >
> > > There are at least to different basic use cases:
> > > - Quake, i.e. FPS-games with mouse-look
> > > - a strategy game, which still uses a cursor, but implements special
> > >   behaviour when running against window edges like scrolling the view,
> > >   likely with the pointer confined to the window
> > >
>
> The second use case could potentially be implemented in the toolkit by
> moving around a subsurface over the parent surface given the relative
> motion events. It'd have the problem of the cursor jumping when
> unlocking, as the cursor have most likely ended up in a different place
> from where it started.
>
> >
> > A third use case. A less exciting one, but still an important one:
> > implement the W3C pointer-lock extension on top of the Wayland API.
>
> Indeed, a pretty important use case.
>
> >
> >
> > > A good question is, do we want to support the latter with
> > > pointer-lock, or should it be another extension?
>
> I imagine we could have a wl_pointer_lock.confine_pointer, if we want to
> support such a use case directly in Wayland.
>
> >
> >
> > Also, the latter probably wants the same motion acceleration algorithm
> > > as the desktop has, but the former probably not.
> > >
>
> 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.
>
> >
> > This is effectively an X server grab with "confine_to". I'd take a look
> at
> > some existing RTS games (FreeCiv? OpenTTD?) and see how they implement
> that
> > mode.
>
> OpenRA and 0AD are two more examples.
>
>
> Jonas
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20140919/43f4e38f/attachment-0001.html>