[PATCH weston v2 14/21] Introduce pointer locking and confinement protocol

[Jonas Ådahl]

On Wed, May 13, 2015 at 12:50:33PM -0700, Bill Spitzak wrote:
> Again this seems excessively complicated, requires lots of communication
> long before the lock happens, makes the methods used to trigger the lock
> very limited and dependent on compositor features, and it does not look like
> it is possible to avoid an unwanted blink in the cursor.
> 
> - Client must sent the lock_pointer immediately on startup, but also resend
> it after each lock is lost. This just seems strange and unbalanced.

A client does not need to send the lock_pointer immediately on startup.

> 
> - Client has to destroy the (unused) locked_pointer object and send a new
> lock_pointer request when the widget layout changes. This is a lot of
> overhead, and also somewhat painful for some toolkit designs, and it just
> seems wrong for clients to have to send widget layout to the compositor.

How is this a lot of overhead really? It's an event and a request if the
client wants to re-enable. I don't see how it is painful to implement in
toolkits either. Why and how would a client send a widget layout?

> 
> - Client has no choice about what triggers the lock other than the region
> sent with the lock_request. In particular shift keys or different mouse
> buttons cannot change it.

Yes it has. It doesn't need to lock on startup. The only example using
pointer locking provided decides exactly when the pointer is to be
locked. As I wrote, the click-to-activate-allows-lock is just a method
for avoiding clients stealing a pointer if a pointer passes over a
surface. Nothing in the protocol says that clicking activates a lock.

> 
> - I think there are race conditions when new lock_pointer requests are sent.
> Probably solvable by allowing them while the previous locked_pointer object
> still exists (it just cancels it) but this is worrisome.

Please explain. Right now the client is required to destroy any previous
lock before locking again.

> 
> - The client must hide the cursor and draw a fake one. This will blink as
> there is no synchronization between changing the cursor image and mapping
> another surface. And it means that the client cannot take advantage of any
> hardware acceleration the compositor applies to the cursor. Toolkits have to
> implement two different methods of changing the cursor image, too.

If the client controls the pointer position, it'll instead be a delay
between the widget and the pointer (as there is no synchronization
between them), the pointer cursor will lag when the client is slow, and
the there will be an added latency compared to the normal use case. I
don't think its really better to allow the client to move the pointer
cursor. If a client wants to avoid a "blink", it can improve the
situation by scheduling the set_cursor and commit intelligently. I don't
think adding latency lag to pointer cursors is a good enough reason to
support slow-scroll-bar-with-lagging-cursor use case.

> 
> - I am utterly mystified about the "confine" object as it appears it does
> nothing that cannot be done with your "lock" object and the client moving a
> fake cursor, restricting the position to being inside the confine region.

It's a control object. A client can unconfine a pointer by destroying
the confine object. Making wl_locked_pointer do everything would just
mean use-less requests on either object depending on the mode.

> 
> Here is my variation, this is identical to what I have suggested several
> times before but I will try to update the terms to match what you have:
> 
> - The lock_pointer request is sent in response to a triggering event, such
> as a mouse-down inside the region. It does not have a region argument, but
> does have an event serial so the compositor can identify the triggering
> event. The compositor can then decide to grant or refuse the lock, or even
> popup some user interface to allow the user to choose.

This way a client cannot lock 'by default'. For example you might want
to start a game and the lock should work. Right now, since we have the
click-to-activate-allows-lock policy it won't work, but that is just a
policy that is easily changed.

> 
> - There is no "locked" event, only an "unlocked" event. If the compositor
> refuses the lock it sends the unlocked event immediately.

Then how do you lock without an event? Busy-loop? Try on every pointer
enter?

> 
> - Remove the "_hint" from the name of "set_cursor_position" request, and
> make it actually move the cursor. The mouse x/y events reported to the
> client are the same as you have them, they do not jump to this position
> until the lock is destroyed (and only if the device is relative). Note that
> if the client hides the cursor they have EXACTLY the same api as you are
> proposing, perhaps that will make it clear what I am requesting.

Discussed this above.

> 
> - Remove confine_pointer request and confined_pointer object. Instead the
> client uses lock_pointer and locked_pointer, and uses set_cursor_position to
> place the cursor inside the confine region. This allows the region to be
> arbitrarily complicated (like have holes in it or have a slower-motion area
> near the edge or restrict the mouse to the surface of a 3D tumbling sphere).

The point of confine regions is that a client can confine a pointer to a
region without having to introduce extra latency to the pointer cursor.
If a client wants regions more complicated than wl_region, or be in
total control of speed etc, it will probably have to live with the
latency though.

> 
> - (Maybe) Add an argument to lock_pointer to reuse the normal grab machinery
> such that release of all the mouse buttons will automatically cancel the
> lock. The reason for this is to avoid round trips by reusing code that is in
> the compositor anyway.

A client can just unlock on release. See the resize example how this
already works. I dont see a point adding extra protocol for something so
trivial.


Jonas