[PATCH 4/7] compositor: Mark all views as dirty when all outputs are gone

[Pekka Paalanen]

On Fri, 29 Jul 2016 13:26:25 +0200
Armin Krezović <krezovic.armin at gmail.com> wrote:

> When all outputs are gone and views were created before they
> were gone, all views would have an output that points to a
> destroyed object.
> 
> Instead, mark the view as dirty and set the view output to
> NULL so a view gets an output assigned as soon as it gets
> plugged in.
> 
> Signed-off-by: Armin Krezović <krezovic.armin at gmail.com>
> ---
>  libweston/compositor.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)

Hi Armin,

this seems to a bit problematic... I may have lead you astray.

> 
> diff --git a/libweston/compositor.c b/libweston/compositor.c
> index 96eeb17..4467555 100644
> --- a/libweston/compositor.c
> +++ b/libweston/compositor.c
> @@ -1212,10 +1212,14 @@ get_view_layer(struct weston_view *view)
>  WL_EXPORT void
>  weston_view_update_transform(struct weston_view *view)
>  {
> +	struct weston_compositor *ec = view->surface->compositor;;
>  	struct weston_view *parent = view->geometry.parent;
>  	struct weston_layer *layer;
>  	pixman_region32_t mask;
>  
> +	if (wl_list_empty(&ec->output_list))
> +		return;

Why is this needed?

> +
>  	if (!view->transform.dirty)
>  		return;
>  
> @@ -4085,11 +4089,6 @@ weston_output_destroy(struct weston_output *output)
>  
>  	output->destroying = 1;
>  
> -	wl_list_for_each(view, &output->compositor->view_list, link) {
> -		if (view->output_mask & (1u << output->id))
> -			weston_view_assign_output(view);

Hmm, this forced call to weston_view_assign_output() relies on the
destructing output to still be on the output list.

weston_surface_update_output_mask() does not work otherwise. It needs
the struct weston_output so that it gets to the wl_resources for
clients to be able to send leave events. If there is no weston_output,
it cannot send leave events.

> -	}
> -
>  	wl_event_source_remove(output->repaint_timer);
>  
>  	weston_presentation_feedback_discard_list(&output->feedback_list);
> @@ -4097,6 +4096,16 @@ weston_output_destroy(struct weston_output *output)
>  	weston_compositor_reflow_outputs(output->compositor, output, output->width);
>  	wl_list_remove(&output->link);
>  
> +	wl_list_for_each(view, &output->compositor->view_list, link) {
> +		if (wl_list_empty(&output->compositor->output_list)) {
> +			weston_view_geometry_dirty(view);
> +			view->output = NULL;
> +		}
> +		else if (view->output_mask & (1u << output->id)) {
> +			weston_view_assign_output(view);

Here you do the call after the wl_list_remove(), so it cannot send the
leave events.

> +		}
> +	}
> +
>  	wl_signal_emit(&output->compositor->output_destroyed_signal, output);
>  	wl_signal_emit(&output->destroy_signal, output);
>  

Maybe there was no problem to begin with, and I was just paranoid
saying there might be?

A forced call to weston_view_assing_output() should already clear any
output pointers that might become stale.

However, there is a catch. Not all views may be on the compositor's
view_list. The view_list contains only the views currently
drawn/dispatched. There could be more views lurking on unused layers or
without layers. We'd need to go through all views instead. Maybe we
need a new list in weston_compositor?

I think weston_view_assign_output() should be able to work fine even
when the output list is empty. In that case it should just
automatically end up assigning NULL outputs to everything.

I have to admit I have forgot most details of why and when we wanted to
mark views dirty. Maybe this patch is not necessary, but we'd need a
different patch to ensure all views get processed when an output is
removed?

For the case when an output is added, it will be enough to just mark
the views in the view_list dirty, as no other view could enter the new
output only because the output is being added.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160805/d9b89646/attachment-0001.sig>