Re: [PATCH weston] compositor: fix usage of dangling output pointer after destroying it

[Giulio Camuffo]

Actually, this seems to cause the cursor to sometimes disappear, not sure why...
A different approach would be to go through the view_list in
weston_output_destroy() and reset the output pointers if needed, but I
don't quite like it.

2015-01-22 16:52 GMT+02:00 Giulio Camuffo <giuliocamu...@gmail.com>:
> When an output is destroyed a view may still hold a pointer to it.
> Calling weston_view_assign_output() on one view of a surface ends up
> updating the output on that view and later iterating over all the views
> of a surface and using their output, which may be bogus.
> Instead, call weston_surface_assign_output(), and update the output
> of all the views.
> ---
>  src/compositor.c | 53 ++++++++++++++++++++++++++---------------------------
>  1 file changed, 26 insertions(+), 27 deletions(-)
>
> diff --git a/src/compositor.c b/src/compositor.c
> index 821970a..651c37a 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -799,10 +799,10 @@ weston_surface_update_output_mask(struct weston_surface 
> *es, uint32_t mask)
>
>
>  static void
> -weston_surface_assign_output(struct weston_surface *es)
> +weston_view_assign_output(struct weston_view *ev)
>  {
> -       struct weston_output *new_output;
> -       struct weston_view *view;
> +       struct weston_compositor *ec = ev->surface->compositor;
> +       struct weston_output *output, *new_output;
>         pixman_region32_t region;
>         uint32_t max, area, mask;
>         pixman_box32_t *e;
> @@ -811,34 +811,32 @@ weston_surface_assign_output(struct weston_surface *es)
>         max = 0;
>         mask = 0;
>         pixman_region32_init(&region);
> -       wl_list_for_each(view, &es->views, surface_link) {
> -               if (!view->output)
> -                       continue;
> -
> -               pixman_region32_intersect(&region, 
> &view->transform.boundingbox,
> -                                         &view->output->region);
> +       wl_list_for_each(output, &ec->output_list, link) {
> +               pixman_region32_intersect(&region, &ev->transform.boundingbox,
> +                                         &output->region);
>
>                 e = pixman_region32_extents(&region);
>                 area = (e->x2 - e->x1) * (e->y2 - e->y1);
>
> -               mask |= view->output_mask;
> +               if (area > 0)
> +                       mask |= 1 << output->id;
>
>                 if (area >= max) {
> -                       new_output = view->output;
> +                       new_output = output;
>                         max = area;
>                 }
>         }
>         pixman_region32_fini(&region);
>
> -       es->output = new_output;
> -       weston_surface_update_output_mask(es, mask);
> +       ev->output = new_output;
> +       ev->output_mask = mask;
>  }
>
>  static void
> -weston_view_assign_output(struct weston_view *ev)
> +weston_surface_assign_output(struct weston_surface *es)
>  {
> -       struct weston_compositor *ec = ev->surface->compositor;
> -       struct weston_output *output, *new_output;
> +       struct weston_output *new_output;
> +       struct weston_view *view;
>         pixman_region32_t region;
>         uint32_t max, area, mask;
>         pixman_box32_t *e;
> @@ -847,27 +845,28 @@ weston_view_assign_output(struct weston_view *ev)
>         max = 0;
>         mask = 0;
>         pixman_region32_init(&region);
> -       wl_list_for_each(output, &ec->output_list, link) {
> -               pixman_region32_intersect(&region, &ev->transform.boundingbox,
> -                                         &output->region);
> +       wl_list_for_each(view, &es->views, surface_link) {
> +               weston_view_assign_output(view);
> +               if (!view->output)
> +                       continue;
> +
> +               pixman_region32_intersect(&region, 
> &view->transform.boundingbox,
> +                                         &view->output->region);
>
>                 e = pixman_region32_extents(&region);
>                 area = (e->x2 - e->x1) * (e->y2 - e->y1);
>
> -               if (area > 0)
> -                       mask |= 1 << output->id;
> +               mask |= view->output_mask;
>
>                 if (area >= max) {
> -                       new_output = output;
> +                       new_output = view->output;
>                         max = area;
>                 }
>         }
>         pixman_region32_fini(&region);
>
> -       ev->output = new_output;
> -       ev->output_mask = mask;
> -
> -       weston_surface_assign_output(ev->surface);
> +       es->output = new_output;
> +       weston_surface_update_output_mask(es, mask);
>  }
>
>  static void
> @@ -1035,7 +1034,7 @@ weston_view_update_transform(struct weston_view *view)
>
>         weston_view_damage_below(view);
>
> -       weston_view_assign_output(view);
> +       weston_surface_assign_output(view->surface);
>
>         wl_signal_emit(&view->surface->compositor->transform_signal,
>                        view->surface);
> --
> 2.2.2
>
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel