[PATCH weston 4/4] compositor: Move view repositioning logic into shell

[Zhang, Xiong Y]

On Wed, 2014-01-29 at 18:47 +0200, Ander Conselvan de Oliveira wrote:
> Remove the listener for output destroy from weston_view and instead
> iterate views owned by the shell in its own output destroy listener.
> 
> This simplifies the code a bit since keeping the view listening for the
> destroy on the right output was a bit complicated. This also removes the
> function pointer output_destroyed from weston_view. The only user for it
> was desktop shell, but now this is all handled in shell.c.
> ---
>  desktop-shell/shell.c | 86 +++++++++++++++++++++++++++++++++++++++++----------
>  src/compositor.c      | 68 ----------------------------------------
>  src/compositor.h      |  8 -----
>  3 files changed, 70 insertions(+), 92 deletions(-)
> 
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index 888e33b..fb69794 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -2956,8 +2956,6 @@ shell_handle_surface_destroy(struct wl_listener *listener, void *data)
>  
>  static void
>  shell_surface_configure(struct weston_surface *, int32_t, int32_t);
> -static void
> -shell_surface_output_destroyed(struct weston_surface *);
>  
>  struct shell_surface *
>  get_shell_surface(struct weston_surface *surface)
> @@ -2994,7 +2992,6 @@ create_common_surface(void *shell, struct weston_surface *surface,
>  
>  	surface->configure = shell_surface_configure;
>  	surface->configure_private = shsurf;
> -	surface->output_destroyed = shell_surface_output_destroyed;
>  
>  	shsurf->shell = (struct desktop_shell *) shell;
>  	shsurf->unresponsive = 0;
> @@ -4793,19 +4790,6 @@ shell_surface_configure(struct weston_surface *es, int32_t sx, int32_t sy)
>  	}
>  }
>  
> -static void
> -shell_surface_output_destroyed(struct weston_surface *es)
> -{
> -	struct shell_surface *shsurf = get_shell_surface(es);
> -
> -	assert(shsurf);
> -
> -	shsurf->saved_position_valid = false;
> -	shsurf->next_state.maximized = false;
> -	shsurf->next_state.fullscreen = false;
> -	shsurf->state_changed = true;
> -}
> -
>  static void launch_desktop_shell_process(void *data);
>  
>  static void
> @@ -5425,11 +5409,81 @@ workspace_move_surface_down_binding(struct weston_seat *seat, uint32_t time,
>  }
>  
>  static void
> +shell_reposition_view_on_output_destroy(struct weston_view *view)
> +{
> +	struct weston_output *output, *first_output;
> +	struct weston_compositor *ec = view->surface->compositor;
> +	struct shell_surface *shsurf;
> +	float x, y;
> +	int visible;
> +
> +	x = view->geometry.x;
> +	y = view->geometry.y;
> +
> +	/* At this point the destroyed output is not in the list anymore.
> +	 * If the view is still visible somewhere, we leave where it is,
> +	 * otherwise, move it to the first output. */
> +	visible = 0;
> +	wl_list_for_each(output, &ec->output_list, link) {
> +		if (pixman_region32_contains_point(&output->region,
> +						   x, y, NULL)) {
> +			visible = 1;
> +			break;
> +		}
> +	}
> +
> +	if (!visible) {
> +		first_output = container_of(ec->output_list.next,
> +					    struct weston_output, link);
> +
> +		x = first_output->x + first_output->width / 4;
> +		y = first_output->y + first_output->height / 4;
> +	}
> +
> +	weston_view_set_position(view, x, y);
If view is visible, this view's geometry won't be changed.
weston_view_set_position() won't mark this view as dirty, so this view
doesn't have chance to reassign a output and this view will disappear.

> +
> +	shsurf = get_shell_surface(view->surface);
> +
> +	if (shsurf) {
> +		shsurf->saved_position_valid = false;
> +		shsurf->next_state.maximized = false;
> +		shsurf->next_state.fullscreen = false;
> +		shsurf->state_changed = true;
> +	}
> +}
> +
> +static void
> +shell_reposition_views_on_output_destroy(struct shell_output *shell_output)
> +{
> +	struct desktop_shell *shell = shell_output->shell;
> +	struct weston_output *output = shell_output->output;
> +	struct weston_layer *layer;
> +	struct weston_view *view;
> +
> +	/* Move all views in the layers owned by the shell */
> +	wl_list_for_each(layer, shell->fullscreen_layer.link.prev, link) {
> +		wl_list_for_each(view, &layer->view_list, layer_link) {
> +			if (view->output != output)
> +				continue;
> +
> +			shell_reposition_view_on_output_destroy(view);
> +		}
> +
> +		/* We don't start from the beggining of the layer list, so
> +		 * make sure we don't wrap around it. */
> +		if (layer == &shell->background_layer)
> +			break;
> +	}
> +}
> +
> +static void
>  handle_output_destroy(struct wl_listener *listener, void *data)
>  {
>  	struct shell_output *output_listener =
>  		container_of(listener, struct shell_output, destroy_listener);
>  
> +	shell_reposition_views_on_output_destroy(output_listener);
> +
>  	wl_list_remove(&output_listener->destroy_listener.link);
>  	wl_list_remove(&output_listener->link);
>  	free(output_listener);
> diff --git a/src/compositor.c b/src/compositor.c
> index 6ac17e6..15291bd 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -346,59 +346,6 @@ region_init_infinite(pixman_region32_t *region)
>  static struct weston_subsurface *
>  weston_surface_to_subsurface(struct weston_surface *surface);
>  
> -static void
> -weston_view_output_destroy_handler(struct wl_listener *listener,
> -				   void *data)
> -{
> -	struct weston_view *ev;
> -	struct weston_compositor *ec;
> -	struct weston_output *output, *first_output;
> -	float x, y;
> -	int visible;
> -
> -	ev = container_of(listener, struct weston_view,
> -			  output_destroy_listener);
> -
> -	ec = ev->surface->compositor;
> -
> -	/* the child window's view->geometry is a relative coordinate to
> -	 * parent view, no need to move child_view. */
> -	if (ev->geometry.parent)
> -		return;
> -
> -	x = ev->geometry.x;
> -	y = ev->geometry.y;
> -
> -	/* At this point the destroyed output is not in the list anymore.
> -	 * If the view is still visible somewhere, we leave where it is,
> -	 * otherwise, move it to the first output. */
> -	visible = 0;
> -	wl_list_for_each(output, &ec->output_list, link) {
> -		if (pixman_region32_contains_point(&output->region,
> -						   x, y, NULL)) {
> -			visible = 1;
> -			break;
> -		}
> -	}
> -
> -	if (!visible) {
> -		first_output = container_of(ec->output_list.next,
> -					    struct weston_output, link);
> -
> -		x = first_output->x + first_output->width / 4;
> -		y = first_output->y + first_output->height / 4;
> -	}
> -
> -	weston_view_set_position(ev, x, y);
> -
> -	if (ev->surface->output_destroyed)
> -		ev->surface->output_destroyed(ev->surface);
> -
> -	wl_list_remove(&ev->output_destroy_listener.link);
> -
> -	wl_list_init(&ev->output_destroy_listener.link);
> -}
> -
>  WL_EXPORT struct weston_view *
>  weston_view_create(struct weston_surface *surface)
>  {
> @@ -434,10 +381,6 @@ weston_view_create(struct weston_surface *surface)
>  
>  	view->output = NULL;
>  
> -	view->output_destroy_listener.notify =
> -		weston_view_output_destroy_handler;
> -	wl_list_init(&view->output_destroy_listener.link);
> -
>  	return view;
>  }
>  
> @@ -887,15 +830,6 @@ weston_view_assign_output(struct weston_view *ev)
>  	}
>  	pixman_region32_fini(&region);
>  
> -	if (ev->output_mask != 0)
> -		wl_list_remove(&ev->output_destroy_listener.link);
> -
> -	if (mask != 0)
> -		wl_signal_add(&new_output->destroy_signal,
> -			      &ev->output_destroy_listener);
> -	else
> -		wl_list_init(&ev->output_destroy_listener.link);
> -
>  	ev->output = new_output;
>  	ev->output_mask = mask;
>  
> @@ -1349,8 +1283,6 @@ weston_view_unmap(struct weston_view *view)
>  	wl_list_init(&view->layer_link);
>  	wl_list_remove(&view->link);
>  	wl_list_init(&view->link);
> -	wl_list_remove(&view->output_destroy_listener.link);
> -	wl_list_init(&view->output_destroy_listener.link);
>  	view->output_mask = 0;
>  	weston_surface_assign_output(view->surface);
>  
> diff --git a/src/compositor.h b/src/compositor.h
> index e185f94..95d0f61 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -821,8 +821,6 @@ struct weston_view {
>  	 * displayed on.
>  	 */
>  	uint32_t output_mask;
> -
> -	struct wl_listener output_destroy_listener;
>  };
>  
>  struct weston_surface {
> @@ -901,12 +899,6 @@ struct weston_surface {
>  	void (*configure)(struct weston_surface *es, int32_t sx, int32_t sy);
>  	void *configure_private;
>  
> -	/* If non-NULL, this function will be called on surface->output::
> -	 * destroy, after the output is removed from the compositor's
> -	 * output list and the remaining outputs moved.
> -	 */
> -	void (*output_destroyed)(struct weston_surface *surface);
> -
>  	/* Parent's list of its sub-surfaces, weston_subsurface:parent_link.
>  	 * Contains also the parent itself as a dummy weston_subsurface,
>  	 * if the list is not empty.