[PATCH 2/2] compositor.c: restore surface->plane from destroyed plane to primary plane

[Kristian Høgsberg]

On Fri, Oct 11, 2013 at 02:43:08PM +0800, Xiong Zhang wrote:
> In drm backend, the plane pointer of cursor surface point to drm_output->cursor_plane.
> when this output is removed, drm_output->cursor_plane is destroyed, but
> cursor_surface->plane doesn't restored to primary 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.
> 
> plane should track all the surfaces belonged to it, when plane is destroyed,
> restroe surface on destroyed plane to primary plane.

There is more work involved in fixing this - it's a somewhat bigger
issue.  We do need to move surfaces back to the primary plane if they
were on a plane owned by the output as your patch does.  But we also
need to reset surface->output if it points to the destroyed output.
Probably by moving the surface back to one of the remaining outputs
and calling weston_surface_update_transform(), which will recompute
the current output.

I'm not quite sure how we'll do that... the cursor surface should be
handled in input.c, but all shell surfaces should be moved by shell.c.
Maybe they can just listen on the output->destroy_signal and move
their surfaces when that signal fires.

compositor-drm.c is still responsible for moving all surfaces out of
planes being destroyed, and it can do that just by looping through
compositor->surface_list.  But I don't want to move them into the
primary plane.  When we move a surface to a different output, it may
end out in a different overlay plane when we repaint that output.
Moving it to the primary and then back into an overlay plane means
that we generate unecessary damage.  I think we can change
weston_surface_damage_below() to just do nothing if surface->plane is
NULL.  We can then set surface->plane to NULL for those surface that
points to a plane that is owned by an unplugged output.

Kristian


> bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=69777
> 
> Signed-off-by: Xiong Zhang <xiong.y.zhang at intel.com>
> ---
>  src/compositor-drm.c |  6 +++---
>  src/compositor.c     | 22 ++++++++++++++++++++--
>  src/compositor.h     |  5 ++++-
>  3 files changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index ffdec89..fc78360 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -1105,8 +1105,8 @@ drm_output_destroy(struct weston_output *output_base)
>  		gbm_surface_destroy(output->surface);
>  	}
>  
> -	weston_plane_release(&output->fb_plane);
> -	weston_plane_release(&output->cursor_plane);
> +	weston_plane_release(&c->base, &output->fb_plane);
> +	weston_plane_release(&c->base, &output->cursor_plane);
>  
>  	weston_output_destroy(&output->base);
>  	wl_list_remove(&output->base.link);
> @@ -2076,7 +2076,7 @@ destroy_sprites(struct drm_compositor *compositor)
>  				0, 0, 0, 0, 0, 0, 0, 0);
>  		drm_output_release_fb(output, sprite->current);
>  		drm_output_release_fb(output, sprite->next);
> -		weston_plane_release(&sprite->plane);
> +		weston_plane_release(&compositor->base, &sprite->plane);
>  		free(sprite);
>  	}
>  }
> diff --git a/src/compositor.c b/src/compositor.c
> index 376ddfd..f9f1957 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;
> @@ -378,6 +379,7 @@ weston_surface_create(struct weston_compositor *compositor)
>  	surface->pending.buffer_scale = surface->buffer_scale;
>  	surface->output = NULL;
>  	surface->plane = &compositor->primary_plane;
> +	wl_list_insert(compositor->primary_plane.surface_list.prev, &surface->plane_link);
>  	surface->pending.newly_attached = 0;
>  
>  	pixman_region32_init(&surface->damage);
> @@ -566,7 +568,9 @@ weston_surface_move_to_plane(struct weston_surface *surface,
>  		return;
>  
>  	weston_surface_damage_below(surface);
> +	wl_list_remove(&surface->plane_link);
>  	surface->plane = plane;
> +	wl_list_insert(plane->surface_list.prev, &surface->plane_link);
>  	weston_surface_damage(surface);
>  }
>  
> @@ -1125,6 +1129,8 @@ weston_surface_destroy(struct weston_surface *surface)
>  
>  	weston_surface_set_transform_parent(surface, NULL);
>  
> +	wl_list_remove(&surface->plane_link);
> +
>  	free(surface);
>  }
>  
> @@ -2614,14 +2620,26 @@ 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)
> +weston_plane_release(struct weston_compositor *ec,
> +				struct weston_plane *plane)
>  {
> +	struct weston_surface *surface, *next;
> +
>  	pixman_region32_fini(&plane->damage);
>  	pixman_region32_fini(&plane->clip);
>  
> +	if (plane != &ec->primary_plane) {
> +		wl_list_for_each_safe(surface, next, &plane->surface_list, plane_link) {
> +			wl_list_remove(&surface->plane_link);
> +			surface->plane = &ec->primary_plane;
> +			wl_list_insert(ec->primary_plane.surface_list.prev, &surface->plane_link);
> +		}
> +	}
> +
>  	wl_list_remove(&plane->link);
>  }
>  
> @@ -3075,7 +3093,7 @@ weston_compositor_shutdown(struct weston_compositor *ec)
>  	weston_binding_list_destroy_all(&ec->axis_binding_list);
>  	weston_binding_list_destroy_all(&ec->debug_binding_list);
>  
> -	weston_plane_release(&ec->primary_plane);
> +	weston_plane_release(ec, &ec->primary_plane);
>  
>  	wl_event_loop_destroy(ec->input_loop);
>  
> diff --git a/src/compositor.h b/src/compositor.h
> index a19d966..a4cd4ca 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;
> @@ -929,7 +931,8 @@ weston_layer_init(struct weston_layer *layer, struct wl_list *below);
>  void
>  weston_plane_init(struct weston_plane *plane, int32_t x, int32_t y);
>  void
> -weston_plane_release(struct weston_plane *plane);
> +weston_plane_release(struct weston_compositor *ec,
> +			struct weston_plane *plane);
>  
>  void
>  weston_compositor_stack_plane(struct weston_compositor *ec,
> -- 
> 1.8.3.2
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel