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 <[email protected]> > --- > 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 > [email protected] > http://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
