[PATCH 5/5] desktop shell: resize background and panel when an output switches mode

Pekka Paalanen ppaalanen at gmail.com
Fri Apr 22 14:18:03 UTC 2016


On Fri, 22 Apr 2016 15:19:06 +0200
David Fort <rdp.effort at gmail.com> wrote:

> When an output permanently switches mode, the desktop shell must be notified so
> that the misc components can resize to the new size of the output. This patch also
> fixes the coodinates of "other" outputs when the resize occurs.
> 
> Signed-off-by: David Fort <contact at hardening-consulting.com>
> ---
>  desktop-shell/shell.c | 34 ++++++++++++++++++++++++++++++++++
>  desktop-shell/shell.h |  1 +
>  src/compositor.c      | 33 ++++++++++++++++++++++-----------
>  src/compositor.h      |  1 +
>  4 files changed, 58 insertions(+), 11 deletions(-)
> 
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index cd269a8..1c1c993 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -6368,11 +6368,43 @@ handle_output_destroy(struct wl_listener *listener, void *data)
>  	shell_for_each_layer(shell, shell_output_destroy_move_layer, output);
>  
>  	wl_list_remove(&output_listener->destroy_listener.link);
> +	wl_list_remove(&output_listener->resized_listener.link);
>  	wl_list_remove(&output_listener->link);
>  	free(output_listener);
>  }
>  
>  static void
> +shell_output_resize_layer(struct desktop_shell *shell,
> +				struct weston_layer *layer,
> +				void *data)
> +{
> +	struct weston_output *output = data;
> +	struct weston_view *view;
> +
> +	wl_list_for_each(view, &layer->view_list.link, layer_link.link) {
> +		if (view->output != output)
> +			continue;
> +
> +		weston_desktop_shell_send_configure(shell->child.desktop_shell, 0,
> +				view->surface->resource,
> +				output->width,
> +				output->height);

Hi,

where do you check that the views you are handling are strictly only
the wallpaper and panel surfaces created by the weston-desktop-shell
client?

I have suspicion that this send call can use desktop_shell resource of
one client with the wl_surface resource of a different client, which is
illegal. Unfortunately, libwayland-server does not catch that, I
believe. The result will be messages sent to clients referring random
object ids that may not even be wl_surfaces.

I'm surprise this doesn't crash random clients - or does it?

Hmm... oh, I see. All the messages are sent to the weston-desktop-shell
client, but rather than referring only the relevant wl_surfaces, they
are sent for *all* mapped wl_surfaces also from other clients.

Are you sure weston-desktop-shell doesn't crash? The crash recovery may
be fast enough that you can only see it in Weston's logs.

> +	}
> +}
> +
> +
> +static void
> +handle_output_resized(struct wl_listener *listener, void *data)
> +{
> +	struct shell_output *output_listener =
> +		container_of(listener, struct shell_output, resized_listener);
> +	struct weston_output *output = output_listener->output;
> +	struct desktop_shell *shell = output_listener->shell;
> +
> +	shell_for_each_layer(shell, shell_output_resize_layer, output);
> +}
> +
> +static void
>  create_shell_output(struct desktop_shell *shell,
>  					struct weston_output *output)
>  {
> @@ -6387,6 +6419,8 @@ create_shell_output(struct desktop_shell *shell,
>  	shell_output->destroy_listener.notify = handle_output_destroy;
>  	wl_signal_add(&output->destroy_signal,
>  		      &shell_output->destroy_listener);
> +	shell_output->resized_listener.notify = handle_output_resized;
> +	wl_signal_add(&output->compositor->output_resized_signal, &shell_output->resized_listener);
>  	wl_list_insert(shell->output_list.prev, &shell_output->link);
>  }
>  
> diff --git a/desktop-shell/shell.h b/desktop-shell/shell.h
> index b430fa2..5457923 100644
> --- a/desktop-shell/shell.h
> +++ b/desktop-shell/shell.h
> @@ -112,6 +112,7 @@ struct shell_output {
>  	struct desktop_shell  *shell;
>  	struct weston_output  *output;
>  	struct exposay_output eoutput;
> +	struct wl_listener    resized_listener;
>  	struct wl_listener    destroy_listener;
>  	struct wl_list        link;
>  };
> diff --git a/src/compositor.c b/src/compositor.c
> index 5500197..a747945 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -143,6 +143,11 @@ static void weston_mode_switch_finish(struct weston_output *output,
>  	}
>  }
>  
> +
> +static void
> +weston_compositor_resize_output(struct weston_compositor *compositor,
> +				struct weston_output *resized_output, int delta_width);
> +
>  WL_EXPORT int
>  weston_output_mode_set_native(struct weston_output *output,
>  			      struct weston_mode *mode,
> @@ -150,6 +155,7 @@ weston_output_mode_set_native(struct weston_output *output,
>  {
>  	int ret;
>  	int mode_changed = 0, scale_changed = 0;
> +	struct weston_mode *old_mode;
>  
>  	if (!output->switch_mode)
>  		return -1;
> @@ -165,11 +171,16 @@ weston_output_mode_set_native(struct weston_output *output,
>  		}
>  	}
>  
> +	old_mode = output->native_mode;
>  	output->native_mode = mode;
>  	output->native_scale = scale;
>  
>  	weston_mode_switch_finish(output, mode_changed, scale_changed);
>  
> +	if (old_mode && (old_mode->width != mode->width))
> +		weston_compositor_resize_output(output->compositor, output, mode->width - old_mode->width);

Why checking only width? Oh, that's because the delta does not depend
on heights. How about computing the delta first and comparing that to
zero?

It is also slightly awkward to encode the assumption of a simple linear
layout in weston_output_modet_set_native().

> +
> +	wl_signal_emit(&output->compositor->output_resized_signal, output);
>  	return 0;
>  }
>  
> @@ -4047,23 +4058,22 @@ bind_output(struct wl_client *client,
>  		wl_output_send_done(resource);
>  }
>  
> -/* Move other outputs when one is removed so the space remains contiguos. */
> +/* Move other outputs when one is resized so the space remains contiguous. */
>  static void
> -weston_compositor_remove_output(struct weston_compositor *compositor,
> -				struct weston_output *remove_output)
> +weston_compositor_resize_output(struct weston_compositor *compositor,
> +				struct weston_output *resized_output, int delta_width)

Heh, first I was WTF here, but then I realized what it does. Both the
old and new name are misleading IMHO. :-)

How about weston_compositor_reflow_outputs() or such?

>  {
>  	struct weston_output *output;
> -	int offset = 0;
> +	bool start_resizing = false;
>  
>  	wl_list_for_each(output, &compositor->output_list, link) {
> -		if (output == remove_output) {
> -			offset = output->width;
> +		if (output == resized_output) {
> +			start_resizing = true;
>  			continue;
>  		}
>  
> -		if (offset > 0) {
> -			weston_output_move(output,
> -					   output->x - offset, output->y);
> +		if (start_resizing) {
> +			weston_output_move(output, output->x + delta_width, output->y);
>  			output->dirty = 1;
>  		}
>  	}
> @@ -4086,7 +4096,7 @@ weston_output_destroy(struct weston_output *output)
>  
>  	weston_presentation_feedback_discard_list(&output->feedback_list);
>  
> -	weston_compositor_remove_output(output->compositor, output);
> +	weston_compositor_resize_output(output->compositor, output, output->width);
>  	wl_list_remove(&output->link);
>  
>  	wl_signal_emit(&output->compositor->output_destroyed_signal, output);
> @@ -4236,7 +4246,7 @@ weston_output_move(struct weston_output *output, int x, int y)
>  					output->model,
>  					output->transform);
>  
> -		if (wl_resource_get_version(resource) >= 2)
> +		if (wl_resource_get_version(resource) >= WL_OUTPUT_DONE_SINCE_VERSION)
>  			wl_output_send_done(resource);

This is an unrelated trivial change that should be a separate patch.

>  	}
>  }
> @@ -4706,6 +4716,7 @@ weston_compositor_create(struct wl_display *display, void *user_data)
>  	wl_signal_init(&ec->output_created_signal);
>  	wl_signal_init(&ec->output_destroyed_signal);
>  	wl_signal_init(&ec->output_moved_signal);
> +	wl_signal_init(&ec->output_resized_signal);
>  	wl_signal_init(&ec->session_signal);
>  	ec->session_active = 1;
>  
> diff --git a/src/compositor.h b/src/compositor.h
> index cb9df00..b71ade0 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -743,6 +743,7 @@ struct weston_compositor {
>  	struct wl_signal output_created_signal;
>  	struct wl_signal output_destroyed_signal;
>  	struct wl_signal output_moved_signal;
> +	struct wl_signal output_resized_signal;
>  
>  	struct wl_signal session_signal;
>  	int session_active;

I think it would be good to separate Weston core and desktop-shell
changes into separate patches.


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/20160422/7006ea51/attachment-0001.sig>


More information about the wayland-devel mailing list