[PATCH weston 1/2] desktop-shell: fix output removal for background/panel

[Marius-cristian Vlad]

On Thu, 2018-06-21 at 15:53 +0300, Pekka Paalanen wrote:
> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> 
> When the compositor has multiple outputs (not clones) and one of them
> is
> removed, the ones remaining to the right will be moved to close the
> gap.
> Because reflowing the remaining outputs happens before removing the
> wl_output global, we get the new output x,y before the removal. This
> causes us to consider the remaining output immediately to the right
> of
> the removed output to be a clone of the removed output whose x,y
> don't
> get updated. That will then hit the two assertions this patch
> removes.
> 
> The reason the assertions were not actually hit is because of a
> compositor bug which moved the remaining outputs in the wrong
> direction.
> The next patch will fix the reflow, so we need this patch first to
> avoid
> the asserts.
> 
> Remove the assertions and hand over the background and panel if the
> "clone" does not already have them. If the clone already has them, we
> destroy the unnecessary background and panel.
> 
> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> ---
>  clients/desktop-shell.c | 37 ++++++++++++++++++++++++-------------
>  1 file changed, 24 insertions(+), 13 deletions(-)
> 
> diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
> index 6d19d029..fcc0b657 100644
> --- a/clients/desktop-shell.c
> +++ b/clients/desktop-shell.c
> @@ -1337,19 +1337,30 @@ output_remove(struct desktop *desktop, struct
> output *output)
>  	}
>  
>  	if (rep) {
> -		/* If found, hand over the background and panel so
> they don't
> -		 * get destroyed. */
> -		assert(!rep->background);
> -		assert(!rep->panel);
> -
> -		rep->background = output->background;
> -		output->background = NULL;
> -		rep->background->owner = rep;
> -
> -		rep->panel = output->panel;
> -		output->panel = NULL;
> -		if (rep->panel)
> -			rep->panel->owner = rep;
> +		/* If found and it does not already have a
> background or panel,
> +		 * hand over the background and panel so they don't
> get
> +		 * destroyed.
> +		 *
> +		 * We never create multiple backgrounds or panels
> for clones,
> +		 * but if the compositor moves outputs, a pair of
> wl_outputs
> +		 * might become "clones". This may happen
> temporarily when
> +		 * an output is about to be removed and the rest are
> reflowed.
> +		 * In this case it is correct to let the
> background/panel be
> +		 * destroyed.
> +		 */
> +
> +		if (!rep->background) {
> +			rep->background = output->background;
> +			output->background = NULL;
> +			rep->background->owner = rep;
> +		}
> +
> +		if (!rep->panel) {
> +			rep->panel = output->panel;
> +			output->panel = NULL;
> +			if (rep->panel)
Guess copy-pasta from above, but is the test necessary? 

Reviewed-by: Marius Vlad <marius-cristian.vlad at nxp.com>

> +				rep->panel->owner = rep;
> +		}
>  	}
>  
>  	output_destroy(output);