[PATCH weston] libweston-desktop/xdg-shell: Consolidate configure event sending

[Quentin Glidic]

On 6/23/17 1:11 PM, Daniel Stone wrote:
> Hi Quentin,
> 
> On 13 April 2017 at 20:15, Quentin Glidic
> <sardemff7+wayland at sardemff7.net> wrote:
>> When switching a state twice in a row, we were overwriting the old value
>> without setting it back, sending a wrong state to the client.
>>
>> Now we update our requested state, then check if we need to schedule a
>> configure event, if we have one scheduled already or even if we can
>> cancel it.
> 
> Most of this looks fine, but:
> 
>>   static void
>> -weston_desktop_xdg_surface_schedule_configure(struct weston_desktop_xdg_surface *surface)
>> +weston_desktop_xdg_surface_schedule_configure(struct weston_desktop_xdg_surface *surface,
>> +                                             bool force)
>>   {
>>          struct wl_display *display = weston_desktop_get_display(surface->desktop);
>>          struct wl_event_loop *loop = wl_display_get_event_loop(display);
>> +       bool requested_same = !force;
>>
>> -       if (surface->configure_idle != NULL)
>> -               return;
>> -       surface->configure_idle =
>> -               wl_event_loop_add_idle(loop,
>> -                                      weston_desktop_xdg_surface_send_configure,
>> -                                      surface);
>> +       switch (surface->role) {
>> +       case WESTON_DESKTOP_XDG_SURFACE_ROLE_NONE:
>> +               assert(0 && "not reached");
>> +               break;
>> +       case WESTON_DESKTOP_XDG_SURFACE_ROLE_TOPLEVEL:
>> +               requested_same = requested_same &&
>> +                       weston_desktop_xdg_toplevel_state_compare((struct weston_desktop_xdg_toplevel *) surface);
>> +               break;
>> +       case WESTON_DESKTOP_XDG_SURFACE_ROLE_POPUP:
>> +               break;
>> +       }
>> +
>> +       if (surface->configure_idle != NULL) {
>> +               if (!requested_same)
>> +                       return;
>> +
>> +               wl_event_source_remove(surface->configure_idle);
>> +               surface->configure_idle = NULL;
>> +       } else {
>> +               if (requested_same)
>> +                       return;
> 
> So, we'll never send a configure event for popups, unless force is
> true. Was this intended / is this OK? If so, then:
> Reviewed-by: Daniel Stone <daniels at collabora.com>

It is intended, as we never call it with false for popups.

Pushed:
c84423ba..d51f826c  master -> master

Thanks,

-- 

Quentin “Sardem FF7” Glidic