[PATCH] compositor: Fix pick an incorrect surface due to dirty surface_list

[Kristian Hoegsberg]

On Fri, Mar 09, 2012 at 11:57:36AM +0800, zhiwen.wu at linux.intel.com wrote:
> From: Alex Wu <zhiwen.wu at linux.intel.com>
> 
> When a new toplevel window is activated and positioned under the
> cursor, the pointer focus still on the previous window. The botton
> event can not deliver to the new window. The root cause is that
> weston_surface_restack() change the surface stack but not update
> the weston_compositor::surface_list, and then calling
> weston_compositor_repick() picks an incorrect surface.

Ah yes, nice catch.  The patch does go a bit in the other direction
than where I'd like to go with the layer code and input handling
though.  I've been trying to move most real work to the repaint loop
and rather than rebuilding the surface list in more places, I prefer
to just move repicking (in case of restacking or new or deleted
surfaces) to the repaint loop also.

What I'd like to do is something like the attached patch, but we're
not quite ready to do that yet, since the X backend still handles
events whenever they comes in.  That means we may hit notify_motion
with an out-of-date surface list.  I think we can just move the X11 fd
into the input_loop and that should be ok, but I haven't had time to
try it.

Kristian


> ---
>  src/compositor.c |   25 ++++++++++++++++++-------
>  1 files changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/src/compositor.c b/src/compositor.c
> index 96e7fe7..2b84e4b 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -165,6 +165,20 @@ weston_client_launch(struct weston_compositor *compositor,
>  }
>  
>  static void
> +weston_compositor_rebuild_surface_list(struct weston_compositor *ec)
> +{
> +	struct weston_layer *layer;
> +	struct weston_surface *es;
> +
> +	wl_list_init(&ec->surface_list);
> +	wl_list_for_each(layer, &ec->layer_list, link) {
> +		wl_list_for_each(es, &layer->surface_list, layer_link) {
> +			wl_list_insert(ec->surface_list.prev, &es->link);
> +		}
> +	}
> +}
> +
> +static void
>  surface_handle_buffer_destroy(struct wl_listener *listener,
>  			      struct wl_resource *resource, uint32_t time)
>  {
> @@ -546,6 +560,7 @@ weston_compositor_pick_surface(struct weston_compositor *compositor,
>  {
>  	struct weston_surface *surface;
>  
> +	weston_compositor_rebuild_surface_list(compositor);
>  	wl_list_for_each(surface, &compositor->surface_list, link) {
>  		weston_surface_from_global(surface, x, y, sx, sy);
>  		if (pixman_region32_contains_point(&surface->input,
> @@ -914,13 +929,9 @@ weston_output_repaint(struct weston_output *output, int msecs)
>  	glViewport(0, 0, width, height);
>  
>  	/* Rebuild the surface list and update surface transforms up front. */
> -	wl_list_init(&ec->surface_list);
> -	wl_list_for_each(layer, &ec->layer_list, link) {
> -		wl_list_for_each(es, &layer->surface_list, layer_link) {
> -			weston_surface_update_transform(es);
> -			wl_list_insert(ec->surface_list.prev, &es->link);
> -		}
> -	}
> +	weston_compositor_rebuild_surface_list(ec);
> +	wl_list_for_each(es, &ec->surface_list, link)
> +		weston_surface_update_transform(es);
>  
>  	if (output->assign_planes)
>  		/*
> -- 
> 1.7.5.4
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
-------------- next part --------------
diff --git a/src/compositor.c b/src/compositor.c
index 96e7fe7..f6b4eb9 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -604,7 +604,6 @@ weston_surface_unmap(struct weston_surface *surface)
 	surface->output = NULL;
 	wl_list_remove(&surface->link);
 	wl_list_remove(&surface->layer_link);
-	weston_compositor_repick(surface->compositor);
 
 	if (device->keyboard_focus == &surface->surface)
 		wl_input_device_set_keyboard_focus(device, NULL,
@@ -824,7 +823,6 @@ weston_surface_restack(struct weston_surface *surface, struct wl_list *below)
 
 	wl_list_remove(&surface->layer_link);
 	wl_list_insert(below, &surface->layer_link);
-	weston_compositor_repick(compositor);
 	weston_surface_damage_below(surface);
 	weston_surface_damage(surface);
 }
@@ -991,6 +989,7 @@ weston_output_finish_frame(struct weston_output *output, int msecs)
 		wl_display_get_event_loop(compositor->wl_display);
 	int fd;
 
+	weston_compositor_repick(compositor);
 	wl_event_loop_dispatch(compositor->input_loop, 0);
 	if (output->repaint_needed) {
 		weston_output_repaint(output, msecs);
@@ -1270,7 +1269,7 @@ surface_set_input_region(struct wl_client *client,
 					  surface->geometry.height);
 	}
 
-	weston_compositor_repick(surface->compositor);
+	weston_compositor_schedule_repaint(surface->compositor);
 }
 
 const static struct wl_surface_interface surface_interface = {
diff --git a/src/shell.c b/src/shell.c
index 92b7540..765b0a4 100644
--- a/src/shell.c
+++ b/src/shell.c
@@ -1055,7 +1055,6 @@ resume_desktop(struct wl_shell *shell)
 	wl_list_insert(&shell->panel_layer.link, &shell->toplevel_layer.link);
 
 	shell->locked = false;
-	weston_compositor_repick(shell->compositor);
 	shell->compositor->idle_time = shell->compositor->option_idle_time;
 	weston_compositor_wake(shell->compositor);
 	weston_compositor_damage_all(shell->compositor);
@@ -1440,7 +1439,7 @@ lock(struct weston_shell *base)
 	}
 
 	/* reset pointer foci */
-	weston_compositor_repick(shell->compositor);
+	weston_compositor_schedule_repaint(shell->compositor);
 
 	/* reset keyboard foci */
 	time = weston_compositor_get_time();
@@ -1584,7 +1583,6 @@ map(struct weston_shell *base, struct weston_surface *surface,
 
 	if (surface_type != SHELL_SURFACE_NONE) {
 		weston_surface_assign_output(surface);
-		weston_compositor_repick(compositor);
 		if (surface_type == SHELL_SURFACE_MAXIMIZED)
 			surface->output = shsurf->output;
 	}
@@ -1651,7 +1649,6 @@ configure(struct weston_shell *base, struct weston_surface *surface,
 	/* XXX: would a fullscreen surface need the same handling? */
 	if (surface->output) {
 		weston_surface_assign_output(surface);
-		weston_compositor_repick(surface->compositor);
 
 		if (surface_type == SHELL_SURFACE_SCREENSAVER)
 			surface->output = shsurf->output;