[PATCH 3/3 v2] Send surface enter/leave events

[Kristian Hoegsberg]

On Thu, Apr 19, 2012 at 10:50:09PM -0400, cdahlin at redhat.com wrote:
> From: Casey Dahlin <cdahlin at redhat.com>
> 
> These new protocol events allow us to tell which outputs a surface is on, and
> potentially update where we allocate our buffers from.

This patch has a few problems, but I've debugged it and committed with
the problems fixed.

First of all, if we're adding new protocol in a patch, we need to
verify that the events actually get sent out.  I usually run the
resizor client with WAYLAND_DEBUG=1 and look for the events I just
added and check that they get sent as expected.  In the case of this
patch, no events were ever sent.  When a window is initially mapped we
should see an enter event for the output it show up on, but nothing
happens.

The different, entered and left mask logic is good, but when you check
against the mask:

> +	for (i = 0; (1 << i) <= different; i++) {
> +		if (entered & i)

you need to test for (entered & (1 << i)).  I ended up just rewriting
the loop to loop over the outputs and send out leave events for
outputs in left mask and enter events for outputs in entered mask.

There are a few other cases I've commented on in the code.

Kristian

> Signed-off-by: Casey Dahlin <cdahlin at redhat.com>
> ---
>  src/compositor.c |   69 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  src/compositor.h |    7 +++++
>  2 files changed, 75 insertions(+), 1 deletions(-)
> 
> diff --git a/src/compositor.c b/src/compositor.c
> index 798b1e1..5d09042 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -1126,13 +1126,70 @@ find_resource_for_client(struct wl_list *list, struct wl_client *client)
>          return NULL;
>  }
>  
> +static struct weston_output *
> +get_output_by_id(struct weston_compositor *compositor, uint32_t id)
> +{
> +	struct weston_output *ret;
> +
> +        wl_list_for_each(ret, &compositor->output_list, link) {
> +		if (ret->id == id)
> +			return ret;
> +	}
> +
> +	return NULL;
> +}
> +
> +static inline void
> +weston_surface_send_leave(struct weston_surface *es, uint32_t output_id)
> +{
> +	struct wl_resource *output_resource;
> +	struct weston_output *output = get_output_by_id(es->compositor, output_id);
> +
> +	output_resource =
> +		find_resource_for_client(&output->resource_list,
> +					 es->surface.resource.client);
> +
> +	wl_surface_send_leave(&es->surface.resource, output_resource);
> +}
> +
> +static inline void
> +weston_surface_send_enter(struct weston_surface *es, uint32_t output_id)
> +{
> +	struct wl_resource *output_resource;
> +	struct weston_output *output = get_output_by_id(es->compositor, output_id);
> +
> +	output_resource =
> +		find_resource_for_client(&output->resource_list,
> +					 es->surface.resource.client);
> +
> +	wl_surface_send_enter(&es->surface.resource, output_resource);
> +}
> +
> +static void
> +weston_surface_update_output_mask(struct weston_surface *es, uint32_t mask)
> +{
> +	uint32_t different = es->output_mask ^ mask;
> +	uint32_t entered = mask & different;
> +	uint32_t left = es->output_mask & different;
> +	uint32_t i;
> +
> +	es->output_mask = mask;
> +
> +	for (i = 0; (1 << i) <= different; i++) {
> +		if (entered & i)
> +			weston_surface_send_enter(es, i);
> +		if (left & i)
> +			weston_surface_send_leave(es, i);
> +	}
> +}
> +
>  WL_EXPORT void
>  weston_surface_assign_output(struct weston_surface *es)
>  {
>  	struct weston_compositor *ec = es->compositor;
>  	struct weston_output *output, *new_output;
>  	pixman_region32_t region;
> -	uint32_t max, area;
> +	uint32_t max, area, mask;
>  	pixman_box32_t *e;

mask needs to be initialized to 0.

>  	weston_surface_update_transform(es);
> @@ -1147,6 +1204,11 @@ weston_surface_assign_output(struct weston_surface *es)
>  		e = pixman_region32_extents(&region);
>  		area = (e->x2 - e->x1) * (e->y2 - e->y1);
>  
> +		if (! area)
> +			continue;

We can't continue here, the loop has to always assign an output, even
if area is NULL for all outputs.  The area >= max condition below will
make the first output the new_output whether or not the surface
overlaps that output, but then update it if other outputs have bigger
overlap.

> +		mask |= 1 << output->id;
> +
>  		if (area >= max) {
>  			new_output = output;
>  			max = area;
> @@ -1155,6 +1217,8 @@ weston_surface_assign_output(struct weston_surface *es)
>  	pixman_region32_fini(&region);
>  
>  	es->output = new_output;
> +	weston_surface_update_output_mask(es, mask);
> +
>  	if (!wl_list_empty(&es->frame_callback_list)) {
>  		wl_list_insert_list(new_output->frame_callback_list.prev,
>  				    &es->frame_callback_list);
> @@ -2126,6 +2190,8 @@ bind_output(struct wl_client *client,
>  	resource = wl_client_add_object(client,
>  					&wl_output_interface, NULL, id, data);
>  
> +	wl_list_insert(&output->resource_list, &resource->link);

Here we need to add a destructor that takes it out of the list when
the resource goes away.  We do that in bind_input_device and I reused
the destructor there.

> +
>  	wl_output_send_geometry(resource,
>  				output->x,
>  				output->y,
> @@ -2341,6 +2407,7 @@ weston_output_init(struct weston_output *output, struct weston_compositor *c,
>  	weston_output_damage(output);
>  
>  	wl_list_init(&output->frame_callback_list);
> +	wl_list_init(&output->resource_list);
>  
>  	output_allocate_id(output);
>  
> diff --git a/src/compositor.h b/src/compositor.h
> index 183c4e7..ec581d8 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -76,6 +76,7 @@ struct weston_output {
>  	uint32_t id;
>  
>  	struct wl_list link;
> +	struct wl_list resource_list;
>  	struct wl_global *global;
>  	struct weston_compositor *compositor;
>  	struct weston_matrix matrix;
> @@ -331,6 +332,12 @@ struct weston_surface {
>  	 */
>  	struct weston_output *output;
>  
> +	/*
> +	 * A more complete representation of all outputs this surface is
> +	 * displayed on.
> +	 */
> +	uint32_t output_mask;
> +
>  	struct wl_list frame_callback_list;
>  
>  	EGLImageKHR image;
> -- 
> 1.7.7.6
>