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

[Bill Spitzak]

On 12/02/2014 05:49 AM, Jonas Ådahl wrote:

> +    <request name="lock_pointer">
> +      <description summary="lock pointer to a position">
> +        The lock_pointer request lets the client disable absolute pointer
> +        movements, locking the pointer to a position.
> +
> +        There may not be another lock of any kind active when requesting a lock,
> +        and if there is, an error will be raised.
> +
> +        The intersection of the region passed with this request and the input
> +        region of the surface is used to determine where the pointer must be
> +        in order for the lock to activate. It is up to the compositor to warp
> +        the pointer, or require some kind of user interaction for the lock to
> +        activate. If the region is null, then an infinit region is used.
> +
> +        The request will create a new object wl_locked_pointer which is used to
> +        interact with the lock as well as receive updates about its state. See
> +        the the description of wl_locked_pointer for further information.
> +
> +        Note that while a locked pointer doesn't move its absolute position, it
> +        may still emit relative motion events via the wl_relative_pointer
> +        object.
> +      </description>
> +
> +      <arg name="id" type="new_id" interface="_wl_locked_pointer"/>
> +      <arg name="surface" type="object" interface="wl_surface"
> +           summary="surface to lock pointer to"/>
> +      <arg name="seat" type="object" interface="wl_seat"
> +           summary="seat where the pointer should be locked"/>
> +      <arg name="region" type="object" interface="wl_region" allow-null="true"
> +           summary="region of surface"/>
> +    </request>

- Does this need some id of the triggering event? Mostly to determine if 
the surface had the pointer focus at the time the request was made.

A lock that is lost when the mouse button is released might be nice. It 
would be used for the "slow scrollbar" and similar widgets, avoid a 
round-trip on the release event, and would probably share code with 
normal mouse-up handling. I think this could be determined based on what 
type of event triggered the lock.

- I am very unclear on what "require some kind of user interaction for 
the lock to activate" means. What I expect is that it will work if and 
only if the surface has the pointer focus. Any "user interaction" has 
already happened (ie they may have moved to another surface). Can 
somebody explain? Or perhaps this is just badly worded.

- Do not warp the pointer. The pointer should freeze exactly where it is 
(even if outside the region), and only client requests to set the cursor 
position should move it. Any compositor-chosen position may be incorrect 
and will result in a flicker as the cursor is placed in this wrong 
position temporarily.

- Is the region really necessary? I think it would be easy for a client 
to see that the cursor has moved out of any desired region and just not 
do this request. Also this removes any questions about complex regions.

 > +    <request name="set_cursor_position_hint">
 > +      <description summary="set the pointer cursor position hint">

Why is this called a "hint"? This better be a lot stronger than a 
"hint", it is pretty near useless if the cursor does not move to where 
the client wants. It's true that it sometimes won't work (for instance 
if the lock has been lost) but if that was the rule *every* request in 
Wayland would have to be called a "hint"!

It would be really nice if you removed this excess verbage and called 
this "set_cursor_position" or even "set_position".

What happens is the compositor cannot set the position, for instance if 
it is off-screen? Does it send a motion event saying where it was really 
placed?

> +    <request name="confine_pointer">
> +      <description summary="confine pointer to a region">
> +        The confine_pointer request lets the client confine the pointer cursor
> +        to a given region.

I don't think this should be necessary. The plain pointer-lock can be 
used, and the client uses set_cursor_position to do the confinement. And 
that removes any questions about complicated regions.

> +    <event name="locked">
> +      <description summary="enter event">
> +        Notification that the pointer lock of this seat's pointer is activated.
> +      </description>

Can't this just be assumed by the fact that you created the pointer lock 
object and have not gotten the unlocked event yet?

> +    <event name="unlocked">
> +      <description summary="leave event">
> +        Notification that the pointer lock of seat's pointer is no longer
> +        active. This object is no defunct and should be destroyed.
> +      </description>

May want to point out that this is also sent immediately if there is a 
failure to get the pointer lock.