[PATCH 4/5] compositor: Implement pointer_lock

Kristian Høgsberg hoegsberg at gmail.com
Thu Feb 28 09:29:26 PST 2013


On Tue, Feb 26, 2013 at 01:55:11PM +0200, Pekka Paalanen wrote:
> On Mon, 25 Feb 2013 21:33:24 -0500
> Kristian Høgsberg <krh at bitplanet.net> wrote:
> 
> > The pointer lock extension lets a client lock pointer motion and receive
> > relative pointer motion events.  This patch implements the weston side
> > of the extension.
> > ---
> >  src/compositor.c | 184 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  src/compositor.h |   7 +++
> >  2 files changed, 189 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/compositor.c b/src/compositor.c
> > index ccfc67b..a67e422 100644
> > --- a/src/compositor.c
> > +++ b/src/compositor.c
> > @@ -1740,18 +1740,86 @@ notify_motion(struct weston_seat *seat,
> >  
> >  	weston_compositor_wake(ec);
> >  
> > -	move_pointer(seat, dx, dy);
> > +	if (!pointer->grab->relative) {
> > +		move_pointer(seat, dx, dy);
> > +	} else {
> > +		pointer->grab->x = dx;
> > +		pointer->grab->y = dy;
> > +	}
> >  
> >  	interface = pointer->grab->interface;
> >  	interface->motion(pointer->grab, time,
> >  			  pointer->grab->x, pointer->grab->y);
> >  }
> >  
> > +static void
> > +pointer_unmap_sprite(struct weston_seat *seat);
> > +
> > +static void
> > +pointer_lock_grab_focus(struct wl_pointer_grab *grab,
> > +			struct wl_surface *surface, wl_fixed_t x, wl_fixed_t y)
> > +{
> > +	struct weston_pointer_lock *lock =
> > +		container_of(grab, struct weston_pointer_lock, grab);
> > +	struct weston_compositor *compositor = lock->surface->compositor;
> > +	struct wl_pointer *pointer = lock->grab.pointer;
> > +	struct weston_seat *seat =
> > +		(struct weston_seat *) pointer->seat;
> > +	uint32_t serial;
> > +	wl_fixed_t sx, sy;
> > +
> > +	wl_pointer_set_focus(pointer, NULL, 0, 0);
> > +
> > +	weston_surface_from_global_fixed(lock->surface,
> > +					 pointer->x, pointer->y, &sx, &sy);
> > +
> > +	serial = wl_display_next_serial(compositor->wl_display);
> > +	wl_pointer_send_enter(lock->resource, serial,
> > +			      &lock->surface->surface.resource, sx, sy);
> > +	grab->focus = NULL;
> > +
> > +	if (seat->sprite)
> > +		pointer_unmap_sprite(seat);
> > +}
> > +
> > +static void
> > +pointer_lock_grab_motion(struct wl_pointer_grab *grab,
> > +			 uint32_t time, wl_fixed_t x, wl_fixed_t y)
> > +{
> > +	struct weston_pointer_lock *lock =
> > +		container_of(grab, struct weston_pointer_lock, grab);
> > +
> > +	wl_pointer_send_motion(lock->resource, time, x, y);
> > +}
> > +
> > +static void
> > +pointer_lock_grab_button(struct wl_pointer_grab *grab,
> > +			 uint32_t time, uint32_t button, uint32_t state)
> > +{
> > +	struct weston_pointer_lock *lock =
> > +		container_of(grab, struct weston_pointer_lock, grab);
> > +	struct weston_compositor *compositor = lock->surface->compositor;
> > +	uint32_t serial;
> > +
> > +	serial = wl_display_next_serial(compositor->wl_display);
> > +	wl_pointer_send_button(lock->resource, serial, time, button, state);
> > +}
> > +
> > +static const struct wl_pointer_grab_interface pointer_lock_grab_interface = {
> > +	pointer_lock_grab_focus,
> > +	pointer_lock_grab_motion,
> > +	pointer_lock_grab_button,
> > +};
> > +
> > +static struct weston_pointer_lock *
> > +get_pointer_lock(struct weston_surface *surface);
> > +
> >  WL_EXPORT void
> >  weston_surface_activate(struct weston_surface *surface,
> >  			struct weston_seat *seat)
> >  {
> >  	struct weston_compositor *compositor = seat->compositor;
> > +	struct weston_pointer_lock *lock;
> >  
> >  	if (seat->seat.keyboard) {
> >  		wl_keyboard_set_focus(seat->seat.keyboard, &surface->surface);
> > @@ -1759,6 +1827,17 @@ weston_surface_activate(struct weston_surface *surface,
> >  	}
> >  
> >  	wl_signal_emit(&compositor->activate_signal, surface);
> > +
> > +	if (seat->seat.pointer->grab->interface == &pointer_lock_grab_interface) {
> > +		seat->seat.pointer->grab->pointer = NULL;
> > +		wl_pointer_end_grab(seat->seat.pointer);
> > +	}
> > +
> > +	lock = get_pointer_lock(surface);
> > +	if (lock) {
> > +		wl_pointer_start_grab(seat->seat.pointer, &lock->grab);
> > +		lock->grab.relative = 1;
> > +	}
> >  }
> >  
> >  WL_EXPORT void
> > @@ -2381,10 +2460,109 @@ seat_get_touch(struct wl_client *client, struct wl_resource *resource,
> >  	cr->destroy = unbind_resource;
> >  }
> >  
> > +static void
> > +lock_set_cursor(struct wl_client *client, struct wl_resource *resource,
> > +		   uint32_t serial, struct wl_resource *surface_resource,
> > +		   int32_t x, int32_t y)
> > +{
> > +}
> > +
> > +static void
> > +lock_release(struct wl_client *client, struct wl_resource *resource)
> > +{
> > +	wl_resource_destroy(resource);
> > +}
> > +
> > +static const struct wl_pointer_interface lock_interface = {
> > +	lock_set_cursor,
> > +	lock_release
> > +};
> > +
> > +static void
> > +pointer_lock_handle_surface_destroy(struct wl_listener *listener, void *data)
> > +{
> > +	struct weston_pointer_lock *lock =
> > +		container_of(listener, struct weston_pointer_lock,
> > +			     surface_destroy_listener);
> > +
> > +	lock->surface = NULL;
> > +	if (lock->grab.pointer) {
> > +		wl_pointer_end_grab(lock->grab.pointer);
> > +		lock->grab.pointer = NULL;
> > +	}
> > +}
> > +
> > +static void
> > +pointer_lock_handle_destroy(struct wl_resource *resource)
> > +{
> > +	struct weston_pointer_lock *lock = resource->data;
> > +
> > +	if (lock->grab.pointer)
> > +		wl_pointer_end_grab(lock->grab.pointer);
> > +	if (lock->surface)
> > +		wl_list_remove(&lock->surface_destroy_listener.link);
> > +	free(lock);
> > +}
> > +
> > +static struct weston_pointer_lock *
> > +get_pointer_lock(struct weston_surface *surface)
> > +{
> > +	struct wl_listener *listener;
> > +
> > +	listener = wl_signal_get(&surface->surface.resource.destroy_signal,
> > +				 pointer_lock_handle_surface_destroy);
> 
> Strange, weston crashes inside wl_signal_get() here on the text-test and
> keyboard-test. List corruption? Use after free? I don't see why from
> this patch.

Yes, silly me, didn't run the test suite before sending.  The test
plugin calls weston_surface_activate with a NULL surface, and we
unconditionally try to get the pointer lock object from that.  Fixed.

> > +	if (listener)
> > +		return container_of(listener, struct weston_pointer_lock,
> > +				    surface_destroy_listener);
> > +	else
> > +		return NULL;
> > +}
> > +
> > +static void
> > +seat_lock_pointer(struct wl_client *client, struct wl_resource *resource,
> > +		  uint32_t id, struct wl_resource *surface_resource)
> > +{
> > +	struct weston_seat *seat = resource->data;
> > +	struct weston_pointer_lock *lock;
> > +
> > +	if (!seat->seat.pointer)
> > +		return;
> > +
> > +	lock = malloc(sizeof *lock);
> > +	if (lock == NULL) {
> > +		wl_resource_post_no_memory(resource);
> > +		return;
> > +	}
> > +
> > +	lock->resource = wl_client_add_object(client, &wl_pointer_interface,
> > +					      &lock_interface, id, lock);
> > +	lock->resource->destroy = pointer_lock_handle_destroy;
> > +
> > +	lock->grab.interface = &pointer_lock_grab_interface;
> > +	lock->grab.focus = NULL;
> > +	lock->grab.pointer = NULL;
> > +	lock->grab.relative = 1;
> > +
> > +	lock->surface = surface_resource->data;
> > +
> > +	lock->surface_destroy_listener.notify =
> > +		pointer_lock_handle_surface_destroy;
> > +	wl_signal_add(&lock->surface->surface.resource.destroy_signal,
> > +		      &lock->surface_destroy_listener);
> > +
> > +	if (seat->seat.keyboard->focus == &lock->surface->surface) {
> > +		wl_pointer_start_grab(seat->seat.pointer, &lock->grab);
> > +		lock->grab.relative = 1;
> > +	}
> > +
> > +	weston_log("starting relative grab for surface %p\n", lock->surface);
> > +}
> > +
> >  static const struct wl_seat_interface seat_interface = {
> >  	seat_get_pointer,
> >  	seat_get_keyboard,
> >  	seat_get_touch,
> > +	seat_lock_pointer,
> >  };
> >  
> >  static void
> > @@ -2399,8 +2577,10 @@ bind_seat(struct wl_client *client, void *data, uint32_t version, uint32_t id)
> >  	wl_list_insert(&seat->base_resource_list, &resource->link);
> >  	resource->destroy = unbind_resource;
> >  
> > -	if (seat->pointer)
> > +	if (seat->pointer) {
> >  		caps |= WL_SEAT_CAPABILITY_POINTER;
> > +		caps |= WL_SEAT_CAPABILITY_POINTER_LOCK;
> 
> Should this not be checking the interface version before adding
> CAPABILITY_POINTER_LOCK?

Yes, definitely - done.

> > +	}
> >  	if (seat->keyboard)
> >  		caps |= WL_SEAT_CAPABILITY_KEYBOARD;
> >  	if (seat->touch)
> > diff --git a/src/compositor.h b/src/compositor.h
> > index 676e0d9..c02e048 100644
> > --- a/src/compositor.h
> > +++ b/src/compositor.h
> > @@ -363,6 +363,13 @@ struct weston_region {
> >  	pixman_region32_t region;
> >  };
> >  
> > +struct weston_pointer_lock {
> > +	struct wl_pointer_grab grab;
> > +	struct wl_resource *resource;
> > +	struct weston_surface *surface;
> > +	struct wl_listener surface_destroy_listener;
> > +};
> > +
> >  /* Using weston_surface transformations
> >   *
> >   * To add a transformation to a surface, create a struct weston_transform, and
> 
> 
> After this series, also tests button-test and event-test fail due to
> unexpected pointer coordinates.

I forgot to update the test extension to also feed relative
coordinates into notify_motion.  Fixed now.  event-test still fails,
but that goes back to 184df50, oddly.

> I'm running these on a 'git clean -dxf' build, so shouldn't be build
> artifacts.

No, all legitimate errors.

Kristian


More information about the wayland-devel mailing list