[PATCH] compositor: Ensure views on hiden workspace are listening correct output

[Ander Conselvan de Oliveira]

On 12/19/2013 11:26 AM, Xiong Zhang wrote:
> The views on hiden workspace can't restored correctly when their
> output was unplugged. There are two reasons:
>
> First:  the views on hiden workspace is restored through animation
> which will change y coordinate continually, if the y coordinate of
> invisible views are setted to first_output->height/4, these views
> will be changed to some invisible region by animation.
>
> Second: the wrong way to manage view->output_destroy_listener.link.
> Considering the following serials:
> a. the view is visible, so the view listen one output signal
> b. the view is invisible, view->output_mask = 0 and view is still
> listenering the previous output signal
> c. the view is visible again. We will add view to previous output
> signal again without removing the view from the previous output
> signal. So the listener list of previous output's signal is wrong
> at this point.When the output is unplgged, the system will dead
> loop on the listener list of this output signal.
>
> Signed-off-by: Xiong Zhang <xiong.y.zhang at intel.com>
> ---
>   src/compositor.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/src/compositor.c b/src/compositor.c
> index 6ca297a..9929be1 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -383,7 +383,7 @@ weston_view_output_destroy_handler(struct wl_listener *listener,
>
>   	/* the child window's view->geometry is a relative coordinate to
>   	 * parent view, no need to move child_view. */
> -	if (ev->geometry.parent)
> +	if (ev->geometry.parent || wl_list_empty(&ec->output_list))
>   		return;
>
>   	x = ev->geometry.x;
> @@ -406,7 +406,8 @@ weston_view_output_destroy_handler(struct wl_listener *listener,
>   					    struct weston_output, link);
>
>   		x = first_output->x + first_output->width / 4;
> -		y = first_output->y + first_output->height / 4;
> +		if (y > first_output->y)
> +			y = first_output->y + first_output->height / 4;
>   	}

We should move this repositioning logic into the shell. I think the 
proper fix for the workspace case then would be to cause all the hidden 
surfaces to have their saved positions invalidated and then run the 
initial positioning algorithm when the user makes them visible by 
switching workspaces.


>   	weston_view_set_position(ev, x, y);
> @@ -905,7 +906,7 @@ weston_view_assign_output(struct weston_view *ev)
>   	}
>   	pixman_region32_fini(&region);
>
> -	if (ev->output_mask != 0) {
> +	if (!wl_list_empty(&ev->output_destroy_listener.link) && mask != 0) {
>   		wl_list_remove(&ev->output_move_listener.link);
>   		wl_list_remove(&ev->output_destroy_listener.link);
>   	}

I sent a patch yesterday to fix a case where we couldn't just 
wl_list_remove() the listener. Do you still need the wl_list_empty() 
check after that?

Cheers,
Ander