[PATCH weston 12/12] compositor: Switch to new surface/view mapped checks

[Giulio Camuffo]

2016-06-28 14:30 GMT+02:00 Pekka Paalanen <ppaalanen at gmail.com>:
> On Thu, 23 Jun 2016 11:59:40 +0200
> Armin Krezović <krezovic.armin at gmail.com> wrote:
>
>> This patch makes use of new flags which were introduced
>> by previous patches to check if a surface/view is mapped
>>
>> Signed-off-by: Armin Krezović <krezovic.armin at gmail.com>
>> ---
>>  src/compositor.c | 10 ++--------
>>  1 file changed, 2 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/compositor.c b/src/compositor.c
>> index 93371b1..673a4ea 100644
>> --- a/src/compositor.c
>> +++ b/src/compositor.c
>> @@ -1546,19 +1546,13 @@ weston_view_set_mask_infinite(struct weston_view *view)
>>  WL_EXPORT bool
>>  weston_view_is_mapped(struct weston_view *view)
>>  {
>> -     if (view->output)
>> -             return true;
>> -     else
>> -             return false;
>> +     return view->is_mapped;
>>  }
>>
>>  WL_EXPORT bool
>>  weston_surface_is_mapped(struct weston_surface *surface)
>>  {
>> -     if (surface->output)
>> -             return true;
>> -     else
>> -             return false;
>> +     return surface->is_mapped;
>>  }
>>
>>  static void
>
> Hi Armin,
>
> patches 8 - 12 are looking pretty good.
>
> Places for setting is_mapped that you missed:
>
> - tests/weston-test.c: test_surface_configure()
> - fullscreen-shell/fullscreen-shell.c: fs_output_apply_pending()
> - desktop-shell/shell.c: shell_ensure_fullscreen_black_view()
> - desktop-shell/shell.c: shell_fade_create_surface()
>
> If you add setting of surface and view is_mapped to all those, I think
> I will land all these remaining patches with my R-b, and we should be
> able to forget about the mappedness nightmare.
>
> I found them by looking for weston_layer_entry_insert(). It is likely
> that none of these misses caused failures, because the surfaces and
> views are special in a way that nothing explicitly checks for their
> mappedness.
>
> I'm starting to think that Giulio is right, and weston_view_is_mapped()
> should really just return whether the view is on a layer, and
> weston_surface_is_mapped() should be replaced by role-specifics.
> However, I don't want to go there now, we might fall into another
> rabbit hole. There would be a lot to clean up there.

Yeah, i'm ok with leaving it for a later time. I think it would
probably be nice to put some comments about it in the code, however.

Cheers,
Giulio

>
>
> Thanks,
> pq