[PATCH weston 1/3] Introduce pointer lock interface

Thu Sep 18 23:33:13 PDT 2014

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
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.

 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.

 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.

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
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.

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.

Jonas

> 
> >
> > > > > Also, this would not be the first interface that creates objects that
> > > > > other interface do as well. For example wl_shm_pool.create_buffer and
> > > > > wl_drm.create_buffer both create wl_buffer's.
> > > >
> > > > Yes, indeed wl_buffer and wl_callback version can't be increased,
> > afaik.
> >
> > wl_buffer and wl_callback are indeed the special cases, the version of
> > these interfaces can never be bumped, which means they are frozen for
> > all eternity. That is somewhat of a design mistake. You definitely do
> > not want to be repeating that mistake.
> >
> > > Then, another example, (of a non-final protoc though):
> > > wl_input_method_context.grab_keyboard vs wl_seat.get_keyboard, both
> > > creating wl_keyboard objects.
> >
> > I think that is a protocol design bug. wl_keyboard in core is already at
> > version 4, but you can only create wl_keyboard version 1 via
> > wl_input_method_context. You cannot bump wl_input_method_context
> > version without starting to get wl_keyboards of higher version. The
> > actual problem comes, when you bump input-method interface version
> > beyond what is currently the highest version of wl_seat, and then
> > someone adds a new thing to wl_keyboard. Input-method would start
> > dealing out wl_keyboard automatically with the new wl_keyboard thing
> > without clients or servers ever implementing the support for the new
> > thing.
> >
> > Thanks for pointing it out, bug filed:
> > https://bugs.freedesktop.org/show_bug.cgi?id=84034
> >
> 
> Fortunately, that's stuck in weston for now.
> 
> 
> >
> > Thanks,
> > pq
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> >