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

[Pekka Paalanen]

On Tue, 28 Jun 2016 16:05:00 +0200
Armin Krezović <krezovic.armin at gmail.com> wrote:

> On 28.06.2016 14:30, Pekka Paalanen wrote:
> > 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.
> > 
> > 
> > Thanks,
> > pq
> >   
> 
> Hi,
> 
> Thanks for the review and a note about missed places.
> 
> I'll fix the remaining places you've outlined and send another series soon.
> 
> As for the Giulio's note, I think I could handle that, but only if there's
> enough time left after I finish everything I've planed for GSoC. I still have
> more than month and a half left.

Hi Armin,

I'm sure we can dig up lots of details on the zero-outputs case (all
shell protocol interactions could use testing without outputs, I think
there are quite some possibilities for NULL dereferences), and the
output configuration task might turn out fairly big too if we make it
all nice for libweston users. ;-)

But yeah, it's a thought to keep in mind.

That reminds me, we should probably talk with Quentin about how the
output configuration will look like, since I'll be away much of the
time.


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/20160628/b679847a/attachment.sig>