[PATCH 5/5] desktop shell: resize background and panel when an output switches mode
Hardening
rdp.effort at gmail.com
Fri Apr 22 21:08:12 UTC 2016
Le 22/04/2016 16:18, Pekka Paalanen a écrit :
> 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.
>
It doesn't crash, but adding a breakpoint with the debugger it looks
like you're right and all surfaces are notified. I will change this.
>> + }
>> +}
>> +
>> +
>> +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().
>
I kinda agree, I'll try to make it more clean.
>> +
>> + 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?
I like your suggestion for the name. I have mutualized this as a
disappearing output is like an output that switches to a 0 width.
>
>> {
>> 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.
Agrees
>
>> }
>> }
>> @@ -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
>
--
David FORT
website: http://www.hardening-consulting.com/
More information about the wayland-devel
mailing list