Le 22/04/2016 16:18, Pekka Paalanen a écrit :
> On Fri, 22 Apr 2016 15:19:06 +0200
> David Fort <[email protected]> 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 <[email protected]>
>> ---
>>  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/

_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to