[PATCH weston 3/3] compositor: turn weston_view boundingbox into masked

[Pekka Paalanen]

On Mon, 23 Feb 2015 20:43:16 +0200
Giulio Camuffo <giuliocamuffo at gmail.com> wrote:

> The other two in the series look good to me, i have a nitpick below
> for this one.
> With that fixed, and for the other ones:
> Reviewed-By: Giulio Camuffo <giuliocamuffo at gmail.com>
> 
> 2015-02-19 11:27 GMT+02:00 Pekka Paalanen <ppaalanen at gmail.com>:
> > From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> >
> > weston_view::transform.boundingbox is made to include the layer mask,
> > which removes the need for masked_boundingbox.
> >
> > The following were using boundingbox when they should have used
> > masked_boundingbox:
> > - drm_output_prepare_overlay_view() uses boundingbox to compute overlay
> >   position, source and destination coordinates.
> > - drm_assign_planes() uses boundingbox for view overlap checks.
> > - is_view_not_visible() uses boundingbox, but nothing will show outside
> >   the layer mask.
> > - weston_surface_assign_output() intersects boundingbox with output
> >   region to choose the primary output for a surface.
> > - weston_view_assign_output() intersects boundingbox with output region
> >   to pick the outputs the view is on.
> >
> > This patch essentially changes all those cases to use the masked
> > boundingbox.
> >
> > Therefore there are no cases which would need the boundingbox without
> > the layer mask, and we can convert boundingbox into masked and remove
> > the left-over member.
> >
> > Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > Cc: Giulio Camuffo <giuliocamuffo at gmail.com>
> > ---
> >  src/compositor.c      | 14 ++++++--------
> >  src/compositor.h      |  1 -
> >  src/gl-renderer.c     |  2 +-
> >  src/pixman-renderer.c |  2 +-
> >  4 files changed, 8 insertions(+), 11 deletions(-)
> >
> > diff --git a/src/compositor.c b/src/compositor.c
> > index 70a7768..fdafcaf 100644
> > --- a/src/compositor.c
> > +++ b/src/compositor.c
> > @@ -443,7 +443,6 @@ weston_view_create(struct weston_surface *surface)
> >         wl_list_init(&view->layer_link.link);
> >
> >         pixman_region32_init(&view->clip);
> > -       pixman_region32_init(&view->transform.masked_boundingbox);
> >
> >         view->alpha = 1.0;
> >         pixman_region32_init(&view->transform.opaque);
> > @@ -964,7 +963,7 @@ weston_view_damage_below(struct weston_view *view)
> >         pixman_region32_t damage;
> >
> >         pixman_region32_init(&damage);
> > -       pixman_region32_subtract(&damage, &view->transform.masked_boundingbox,
> > +       pixman_region32_subtract(&damage, &view->transform.boundingbox,
> >                                  &view->clip);
> >         if (view->plane)
> >                 pixman_region32_union(&view->plane->damage,
> > @@ -1203,7 +1202,6 @@ weston_view_update_transform(struct weston_view *view)
> >  {
> >         struct weston_view *parent = view->geometry.parent;
> >         struct weston_layer *layer;
> > -       pixman_region32_t mask;
> >
> >         if (!view->transform.dirty)
> >                 return;
> > @@ -1233,9 +1231,11 @@ weston_view_update_transform(struct weston_view *view)
> >
> >         layer = get_view_layer(view);
> >         if (layer) {
> > +               pixman_region32_t mask;
> 
> Why did you move the mask here? That isn't compliant with the wayland style.

Here it is still in the beginning of a block, and it is closer to the
place where it is used. It is not needed outside this block, so I think
it makes sense to limit its scope to this block.

I've been using this style for quite some time, but it seems the
concensus is the beginning of a function, not the beginning of a block,
with the recommendation that if you'd want decls in a block, it should
be a function instead.

So, I dropped this part of the change. All three pushed:
   32318a5..25c0ca5  master -> master


Thanks,
pq


> 
> > +
> >                 pixman_region32_init_with_extents(&mask, &layer->mask);
> > -               pixman_region32_intersect(&view->transform.masked_boundingbox,
> > -                                       &view->transform.boundingbox, &mask);
> > +               pixman_region32_intersect(&view->transform.boundingbox,
> > +                                         &view->transform.boundingbox, &mask);
> >                 pixman_region32_intersect(&view->transform.opaque,
> >                                           &view->transform.opaque, &mask);
> >                 pixman_region32_fini(&mask);