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

[Bill Spitzak]

On 05/14/2015 07:53 PM, Jonas Ådahl wrote:

>> but your notes were very misleading.
>
> What notes?

The statement "Serials are dropped" that was in your header email is the 
one that really threw me.

The primary problem I am having is all your description seems to be for 
setting up something like an X11 pointer grab that is permanent until 
the client cancels it.

However both your examples, and virtually every single use of pointer 
lock I can think of other than point & shoot games, involve grabbing the 
pointer on mouse press and releasing it on mouse release. Ideas like the 
"slow scrollbar" or attaching the pointer to the surface of a 3D sphere, 
or a drawing program where a stroke off the edge of the window scrolls 
the canvas so the drawing remains visible.

I believe your api, with minor changes, can support both of these styles 
of interaction. In particular there needs to be a type of pointer lock 
that is cancelled when all the mouse buttons are released (this would be 
identical to current grab behavior except the mouse cursor stops 
moving). I am assuming you get this type of lock if the triggering event 
is a mouse-down. Keystrokes, mouse-up, and mouse enter events get you 
the long-term type of lock you are talking about.

>> The primary problem is that you removed the serial of the triggering event,
>> which is VITAL for this to work. You have to put that back. The compositor
>> has to know what event caused the lock!
>
> No I didn't. They have never been there.

Then it is broken. This needs to be fixed so the compositor can know 
which event triggered the lock, since it makes a huge difference in 
whether to grant the lock or not. It also fixes race conditions.

> I removed other serials that
> was only used with set_cursor_position_hint, but since the lock/confine
> objects are one-shot objects they were useless.

I *think* that makes sense. The only sync issue is that 
set_cursor_position requests may be sent after the compositor cancels 
the lock but before the client sees this. I think correct behavior is 
that the compositor should obey these, though it is probably not 
important if these are dropped.

>> I am still fully mystified as to the purpose of the "region" in the request.
>> I think the purpose of this could be handled by the client sending the
>> request only when the mouse enters the region. However it is possible you
>> intend to use the "region" as part of the compositor interaction, maybe it
>> wants to highlight it, or place a dialog box centered in it.
>
> A pointer entering any region is should not be enough to trigger a lock.

Can you please explain WHAT IS THE PURPOSE OF THE REGION??? I am taking 
wild guesses here because I just don't see it. How about a specific 
question:

   - The client gets a mouse press at x,y

   - Version 1: the client sends lock_pointer with the region set to x,y 
and a width and height of zero

   - Version 2: the client sends lock_pointer with the region set to the 
input area of the clicked surface.

   Question: what happens differently in these two cases???

I really am guessing wildly here that the "region" is supposed to be 
used by the compositor to control what events cause the lock to be 
triggered. But because you limit it to the input region of the surface 
and thus only to events the client would get anyway, I am unable to come 
up with any idea that could not be done by the client.

For instance if a click inside the region is to trigger the lock, it is 
trivial for a client to instead wait for a mouse press, check if it is 
in the region, and then do the lock_pointer request only if it is inside.

Can you please describe an example of user interaction you expect to 
trigger one of your deferred locks? And how the region is used in this 
interaction?

> A lock happens asynchronously. In order for the client to know when the
> lock is activated, an event is needed. Sometimes it will be immediate,
> sometimes not. If a client receives pointer events but no "locked"
> event, then it can deal with those events knowing the pointer is not
> locked.

No, the client has to assume and treat any pointer motion events it gets 
as part of the lock. They may have been sent before the compositor 
received the lock_pointer request.

It also seems to me that if the compositor pops up some kind of user 
interface to allow the user to choose whether the lock should happen, it 
will be eating all the pointer motion events anyway.

> Anyway, while we probably
> need to specify what happens if a surface's input region changes size.
> Right now a lock will be kept, no matter how the region changes, but
> that won't make sense for confine regions. What is the use case for
> resizing and changing the lock/confine region you are talking about?

Because user interaction with the game changes the window size.

> If you need the cursor to be locked precisely to the graphic it is
> drawing the only possible way to achieve this is by drawing that cursor
> client side. As I tried to tell you, a hypothetical
> "set_cursor_position" wouldn't give you this.

No, the only need is for the client to MOVE the cursor client-side, 
which is exactly what I am requesting. This syncs up any latency which 
gets rid of 99% of the problem.

You are correct that there needs to be some kind of sync with a surface 
commit similar to subsurfaces if we want perfect output. But it is silly 
to say because that does not work we won't do *any* of the necessary work.

Drawing the cursor image as part of the surface will not work because it 
will be clipped by the edge. It also can't be a (as currently defined) 
subsurface because it will be clipped by overlapping surfaces.

In fact I think it is impossible under current shell api for a client to 
create a real top-level window that acts exactly like the cursor. So no, 
without the ability to move the cursor, the client cannot get the 
desired api.

>> The blink is not avoidable because cursors are not 100% opaque. This means
>> that the compositor can only draw one image of the two cursors, it cannot
>> composite two atop each other, or there will be a visible defect. I suppose
>> a lot more commit sync rules could be added but it seems an awful lot easier
>> to just reuse the existing ability to change the cursor! I also suspect that
>> the real cursor will have better hardware acceleration than the fake one.
>
> I think the added latency and laggyness induced by roundtripping via the
> client is worse than not using the hw cursor. If a client uses a
> subsurface the compositor can even possibly use the hw cursor plane
> anyway if it has such an optimization.

The laggyness is there and unavoidable, in that the graphic of the thing 
being moved will have this lag. All I'm trying to do is sync them up! 
That is an improvement. Too bad we can't get rid of the latency but that 
is no reason not to try to make it less objectionable...

I also suspect the chances of a compositor using the cursor hardware for 
anything other than the cursor is really slim. It may be impossible to 
switch a surface between using the cursor hardware and not without a 
blink, which will force the compositor to restrict the usage to the cursor.

>> I don't understand why you don't want to allow the client to move the
>> cursor, yet you seem fine with them hiding it and drawing a fake one, which
>> requires far more api and overhead and latency.
>
> I described that in the previous e-mail. It does not require any more
> API, overhead, and adds no extra latency, compared to
> 'set_cursor_position'.

Huh? Your version requires the client to:

1. Create some kind of surface for the fake cursor
2. Draw an image in that fake cursor surface that looks like real one
3. Map that fake cursor surface and simultaneously make the real one 
invisible.
4. Move the fake cursor surface in response to mouse events
5. Remove the fake surface and restore the cursor image simultaneously 
when the lock is dropped.

My version requires the client to:

1. Move the real cursor in response to mouse events

Sorry I think mine is an awful lot simpler!

>> Okay that sort of explains your reason for confine.
>>
>> However I have a much simpler idea: have the position-the-cursor request
>> take a rectangle, not just an xy. The cursor is then confined to this
>> rectangle. A zero-sized rectangle reproduces the original idea.
>
> I don't see why this would be any simpler. You just make locking more
> complex instead. I don't even understand how it'd work exactly. Should
> the pointer jump to the "closest" area in the rectangle? Or the center?
> And then move freely?

The actual pointer position is always clamped to the closest point in 
the rectangle to figure out the location to draw the cursor.

What may be confusing things is you are thinking that this has some 
effect on the mouse x/y position reported in events? It is not supposed 
to, they are unchanged no matter where the client places the cursor.

>> Your design lacks the ability to change the confine region, which is needed
>> if the game is resized. The above request solves that problem.
>
> That is true. A client needs to re-confine (which it would do with
> wl_confine_pointer.destroy(), confine_pointer(new_region)), but that is
> a bit racy since the confinement is potentially disabled for a short
> while.
>
> Before the idea was that a new lock/confine replaces a previous one, but
> that instead would be racy regarding input regions. Not really sure
> resizing is important enough to complicate things. If we really want to
> support resizing, we'd need to make some double-buffered-applied-on
> "commit" region.

I suspect this being out of sync is not a problem, as the whole purpose 
of the confine region is to reduce latency in mouse movements at the 
expense of synchronization. However you did point out that perfect 
output of a cursor locked to a graphic will require attaching the 
set_cursor_position to a surface commit, so this rule could apply to 
changing the confine region too.

>> It also allows complex confine regions with less (but not zero) latency. The
>> client just sends the largest rectangle that is inside the confine region
>> and contains the current pointer position, and only updates this when it
>> changes.
>
> Less latency? Compared to what?

Compared to my original proposal where every motion of the pointer would 
require the client to position the cursor.

But more latency compared to what I think you are proposing which is 
that the region can be very complex.

> How would you make for example an H?

   +---+   +---+
   | A |   | B |
   +---+---+---+
   | C | D | E |
   +---+---+---+
   | F |   | G |
   +---+   +---+

   Find the square nearest (or containing) the mouse
   If A or F then use the rectangle ACF
   If B or G then use the rectangle BEG
   If D then use the rectangle CDE
   If C then use CDE if it is closer to D than A or F, else use ACF
   If E then use CDE if it is closer to D than B or G, else use BEG.

> Change the rectangle as the cursor moves? Would that warp the cursor?

I originally intended that the client could not change the event xy.

However I can see it being useful for confine, so that when the cursor 
hits the edge both the client and compositor agree on the location. 
Possibly there needs to be another argument as to whether the events are 
warped. This will only happen for a relative input device (ie a mouse, 
verses a touch screen or full-screen tablet).