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

[Jonas Ådahl]

On Thu, May 14, 2015 at 12:26:52PM -0700, Bill Spitzak wrote:
> On 05/13/2015 11:34 PM, Jonas Ådahl wrote:
> 
> Okay, looking at the clickdot and resizor examples, it appears you are
> actually responding to events with pointer_lock. This is pretty much what I
> think makes sense but your notes were very misleading.

What notes?

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

> 
> Also very confusing is the purpose of the "locked" event. I think the idea
> is that if there is compositor interaction with the user before the lock
> happens, then the locked event is delayed. But in normal use the locked
> event happens immediately?

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.

> 
> Both of these questions could be clarified with a sketch showing the
> "compositor interaction before the lock" that you are proposing.

A client can be triggered by any action it wants. The requirements by
the compositor in order to avoid "pointer stealing" can be anything.

> 
> >Why and how would a client send a widget layout?
> 
> Because the window was resized!

Can't remember what your original concern was. 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?

> 
> >>- 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.
> 
> The delay you are describing is a DESIRABLE FEATURE. I need the cursor to be
> locked precisely to the graphic it is dragging. Jiggling around is NOT
> wanted and is the entire reason I am proposing this. Other than games this
> is the entire reason anybody is interested in pointer lock.

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.

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

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

> 
> All I can guess is that you think moving the pointer will change the mouse
> events. That is wrong, I want it to work exactly like your "hint": it has no
> effect on mouse x/y events until after the lock ends. The only difference is
> that the cursor is always drawn at the current "hint" position. If the
> cursor is invisible this is EXACTLY 100% IDENTICAL to your proposal!!!
> 
> PS: do not "warp the pointer to the region on lock". This is ABSOLUTELY not
> wanted under any scheme, either yours or mine.
> 
> >>- 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.
> 
> I can't imagine any scheme where the policy makes any difference until the
> client requests a lock.
> 
> >Then how do you lock without an event? Busy-loop? Try on every pointer
> >enter?
> 
> Try on every pointer enter and on mouse clicks.
> 
> >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.
> 
> 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?

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

> 
> 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? How would you make for example an H?
Change the rectangle as the cursor moves? Would that warp the cursor?

> 
> Even if lock_pointer and confine_pointer are different requests, they really
> should return the same object using the same events. The current design just
> doubles the size of all the wayland protocol api for no useful reason, the
> client knows exactly which request was used to create the object.

I don't think separating two separate concepts protocol wise is bad.
Putting everything in one object would be, as we'd have requests/events
that only does things depending on what type the object actually has.
For example a set_cursor_position_hint is pointless for a confined
confined pointer.

The reason for separation really is to simplify things.

> 
> >>- (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.
> 
> The reason is that your "compositor interaction to enable the lock" may want
> to be considerably different if it knows the lock will be lost if all the
> buttons are released. If requested for a mouse-down event then this is no
> more harmful that the normal automatic grab so it probably should be granted
> without question.

Not sure what you are talking about here. If you have comments about the
implementation and whether to reuse "grab machinery" it'd be more
helpful if you replied to the relevant patch.


Jonas