[PATCH 1/6] srccompositor.c: set surface->plane from destroyed plane to NULL

[Kristian Høgsberg]

On Thu, Oct 17, 2013 at 10:10:48AM +0800, Xiong Zhang wrote:
> In drm backend, the cursor_surface->plane point to drm_output->cursor_plane.
> when this output is removed, drm_output->cursor_plane is destroyed, but
> cursor_surface->plane still point to destroyed plane. So once mouse move to this
> cursor_surface and system will repaint this cursor_surface, segment fault will occure in
> weston_surface_damage_below() function.
> 
> Eech client APP has its own cursor surface, if mouse focus isn't on this app's cursor surface,
> this app's cursor surface won't be on any cursor_layer and compositor->surface_list.But once
> moving mouse to this APP, this APP's cursor surface will be in compositor->surface_list. In
> order to track these hiden cursor surface,plane should track all the surfaces belonged to it,
> when plane is destroyed, all surface's plane on destroyed plane is set to NULL.
> 
> bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=69777
> 
> v2: set surface->plane to NULL whose plane point to unplugged output, then
>     change weston_surface_damage_below() to do nothing if surface->plane is NULL (Kristian)

I'm still not excited about introducing yet another surface list.
What I'd like to do instead is to just set the surface->plane pointer
to NULL in weston_surface_unmap().  That way, all surfaces that have a
non-NULL plane pointer will be on compositor->surface_list and we can
just loop through that.  That's also more correct, since otherwise we
would damage the plane it use to be on when we later map it again on a
different plane.

Also, please format your commit message to be at most 70 characters
wide so it formats nicely in git log.  Thanks!

Kristian

> 
> Signed-off-by: Xiong Zhang <xiong.y.zhang at intel.com>
> ---
>  src/compositor.c | 22 +++++++++++++++++++---
>  src/compositor.h |  2 ++
>  2 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/src/compositor.c b/src/compositor.c
> index 376ddfd..990a378 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -362,6 +362,7 @@ weston_surface_create(struct weston_compositor *compositor)
>  
>  	wl_list_init(&surface->link);
>  	wl_list_init(&surface->layer_link);
> +	wl_list_init(&surface->plane_link);
>  
>  	surface->compositor = compositor;
>  	surface->alpha = 1.0;
> @@ -377,7 +378,7 @@ weston_surface_create(struct weston_compositor *compositor)
>  	surface->pending.buffer_transform = surface->buffer_transform;
>  	surface->pending.buffer_scale = surface->buffer_scale;
>  	surface->output = NULL;
> -	surface->plane = &compositor->primary_plane;
> +	surface->plane = NULL;
>  	surface->pending.newly_attached = 0;
>  
>  	pixman_region32_init(&surface->damage);
> @@ -566,7 +567,10 @@ weston_surface_move_to_plane(struct weston_surface *surface,
>  		return;
>  
>  	weston_surface_damage_below(surface);
> +	if (surface->plane)
> +		wl_list_remove(&surface->plane_link);
>  	surface->plane = plane;
> +	wl_list_insert(plane->surface_list.prev, &surface->plane_link);
>  	weston_surface_damage(surface);
>  }
>  
> @@ -578,8 +582,9 @@ weston_surface_damage_below(struct weston_surface *surface)
>  	pixman_region32_init(&damage);
>  	pixman_region32_subtract(&damage, &surface->transform.boundingbox,
>  				 &surface->clip);
> -	pixman_region32_union(&surface->plane->damage,
> -			      &surface->plane->damage, &damage);
> +	if (surface->plane)
> +		pixman_region32_union(&surface->plane->damage,
> +				&surface->plane->damage, &damage);
>  	pixman_region32_fini(&damage);
>  }
>  
> @@ -1125,6 +1130,9 @@ weston_surface_destroy(struct weston_surface *surface)
>  
>  	weston_surface_set_transform_parent(surface, NULL);
>  
> +	if (surface->plane)
> +		wl_list_remove(&surface->plane_link);
> +
>  	free(surface);
>  }
>  
> @@ -2614,14 +2622,22 @@ weston_plane_init(struct weston_plane *plane, int32_t x, int32_t y)
>  	/* Init the link so that the call to wl_list_remove() when releasing
>  	 * the plane without ever stacking doesn't lead to a crash */
>  	wl_list_init(&plane->link);
> +	wl_list_init(&plane->surface_list);
>  }
>  
>  WL_EXPORT void
>  weston_plane_release(struct weston_plane *plane)
>  {
> +	struct weston_surface *surface, *next;
> +
>  	pixman_region32_fini(&plane->damage);
>  	pixman_region32_fini(&plane->clip);
>  
> +	wl_list_for_each_safe(surface, next, &plane->surface_list, plane_link) {
> +		wl_list_remove(&surface->plane_link);
> +		surface->plane = NULL;
> +	}
> +
>  	wl_list_remove(&plane->link);
>  }
>  
> diff --git a/src/compositor.h b/src/compositor.h
> index a19d966..bb00c79 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -508,6 +508,7 @@ struct weston_plane {
>  	pixman_region32_t damage;
>  	pixman_region32_t clip;
>  	int32_t x, y;
> +	struct wl_list surface_list;
>  	struct wl_list link;
>  };
>  
> @@ -724,6 +725,7 @@ struct weston_surface {
>  	pixman_region32_t input;
>  	struct wl_list link;
>  	struct wl_list layer_link;
> +	struct wl_list plane_link;
>  	float alpha;                     /* part of geometry, see below */
>  	struct weston_plane *plane;
>  	int32_t ref_count;
> -- 
> 1.8.3.2
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel