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

[Pekka Paalanen]

On Wed, 29 Jun 2016 10:26:12 +0200
Giulio Camuffo <giuliocamuffo at gmail.com> wrote:

> 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.

Hi,

definitely. Perhaps documentation on weston_{surface,view}_is_mapped()
functions explaining where we might want to go, and perhaps even a link
to the mailing list discussion.

Armin, could you do that, please? Alos mind the "Re: [PATCH weston v2
2/3] ivi-shell: introduce ivi_layout_view" I sent you.


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/20160629/d99d397f/attachment.sig>