[PATCH weston] compositor: add a masking mechanism to weston_layer

Giulio Camuffo giuliocamuffo at gmail.com
Tue Dec 10 02:13:42 PST 2013


2013/12/10 Jason Ekstrand <jason at jlekstrand.net>:
> Giulio,
> Couple thoughts.  First, you don't provide an implementation of the clipping
> in  any of the renderers.  Probably have to wait on the Collabora people for
> the RPi renderer, but we should have pixman and gl implementations of this.

There is no need to add support in the renderers for that. The masking
is done in view_accumulate_damage(): the part of the view's
boundingbox that doesn't fit in the mask is added to the view's clip,
and the renderers then clip that away already.

>
> Second, is there any particular reason why you're using pixman_box32_t
> instead of pixman_rectangle32_t?  One's as good as the other, it just seems
> a little strange that there's a difference between how it's storred and the
> function to set it.

Not really, I just saw that pixman_box32_t is used already in
compositor.h while pixman_rectangle32_t isn't. Though with
pixman_box32_t I don't need to calculate the x2 and y2 in
weston_compositor_pick_view() everytime.

>
> More comments below.
>
> On Dec 9, 2013 3:36 PM, "Giulio Camuffo" <giuliocamuffo at gmail.com> wrote:
>>
>> this adds a mechanism to mask the views belonging to a layer
>> to an arbitrary rect, in the global space. The parts that don't fit
>> in that rect will be clipped away.
>> ---
>>
>> As per the discussion on IRC, masking layers is preferred to masking
>> views,
>> so this replaces
>>
>> http://lists.freedesktop.org/archives/wayland-devel/2013-December/012422.html
>>
>>  src/compositor.c | 37 ++++++++++++++++++++++++++++++++++++-
>>  src/compositor.h |  9 +++++++++
>>  2 files changed, 45 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/compositor.c b/src/compositor.c
>> index 8f4bdef..6beea9c 100644
>> --- a/src/compositor.c
>> +++ b/src/compositor.c
>> @@ -1193,10 +1193,14 @@ weston_compositor_pick_view(struct
>> weston_compositor *compositor,
>>                             wl_fixed_t *vx, wl_fixed_t *vy)
>>  {
>>         struct weston_view *view;
>> +        int ix = wl_fixed_to_int(x);
>> +        int iy = wl_fixed_to_int(y);
>>
>>         wl_list_for_each(view, &compositor->view_list, link) {
>>                 weston_view_from_global_fixed(view, x, y, vx, vy);
>> -               if (pixman_region32_contains_point(&view->surface->input,
>> +               if (ix >= view->layer->mask.x1 && iy >=
>> view->layer->mask.y1 &&
>> +                   ix <= view->layer->mask.x2 && iy <=
>> view->layer->mask.y2 &&
>> +                   pixman_region32_contains_point(&view->surface->input,
>>                                                    wl_fixed_to_int(*vx),
>>                                                    wl_fixed_to_int(*vy),
>>                                                    NULL))
>> @@ -1489,6 +1493,18 @@ view_accumulate_damage(struct weston_view *view,
>>         pixman_region32_union(&view->plane->damage,
>>                               &view->plane->damage, &damage);
>>         pixman_region32_fini(&damage);
>> +
>> +       /* set view->clip with the part of view->transform.boundingbox
>> +        * that doesn't fit into view->layer->mask, then add the opaque
>> region
>> +        * to it. We don't do the opposite, adding view->clip to opaque,
>> +        * because opaque is then passed to the next views, which may be
>> +        * in a different layer with a different mask.
>> +        */
>> +       pixman_region32_t mask;
>> +       pixman_region32_init_with_extents(&mask, &view->layer->mask);
>> +       pixman_region32_subtract(&view->clip,
>> &view->transform.boundingbox, &mask);
>> +       pixman_region32_fini(&mask);
>> +
>>         pixman_region32_copy(&view->clip, opaque);
>>         pixman_region32_union(opaque, opaque, &view->transform.opaque);
>>  }
>> @@ -1652,6 +1668,7 @@ weston_compositor_build_view_list(struct
>> weston_compositor *compositor)
>>         wl_list_init(&compositor->view_list);
>>         wl_list_for_each(layer, &compositor->layer_list, link) {
>>                 wl_list_for_each(view, &layer->view_list, layer_link) {
>> +                       view->layer = layer;
>>                         view_list_add(compositor, view);
>
> I think it's probably better to have a function to add a view to a layer.
> I'm not a big fan of saying view.layer may or may not be null and may not
> correspond in any way to layer_link.  We should keep layer and layer_link in
> sync at all times.

That'd be better, yeah. Something like wl_view_add_to_layer(view,
layer), removing the view from any layer if layer is NULL.

>
>>                 }
>>         }
>> @@ -1776,11 +1793,29 @@ WL_EXPORT void
>>  weston_layer_init(struct weston_layer *layer, struct wl_list *below)
>>  {
>>         wl_list_init(&layer->view_list);
>> +       weston_layer_set_mask_infinite(layer);
>>         if (below != NULL)
>>                 wl_list_insert(below, &layer->link);
>>  }
>>
>>  WL_EXPORT void
>> +weston_layer_set_mask(struct weston_layer *layer,
>> +                     int x, int y, int width, int height)
>> +{
>> +       layer->mask.x1 = x;
>> +       layer->mask.x2 = x + width;
>> +       layer->mask.y1 = y;
>> +       layer->mask.y2 = y + height;
>> +}
>> +
>> +WL_EXPORT void
>> +weston_layer_set_mask_infinite(struct weston_layer *layer)
>> +{
>> +       weston_layer_set_mask(layer, INT32_MIN, INT32_MIN,
>> +                                    UINT32_MAX, UINT32_MAX);
>> +}
>> +
>> +WL_EXPORT void
>>  weston_output_schedule_repaint(struct weston_output *output)
>>  {
>>         struct weston_compositor *compositor = output->compositor;
>> diff --git a/src/compositor.h b/src/compositor.h
>> index a161345..ab3de19 100644
>> --- a/src/compositor.h
>> +++ b/src/compositor.h
>> @@ -523,6 +523,7 @@ enum {
>>  struct weston_layer {
>>         struct wl_list view_list;
>>         struct wl_list link;
>> +       pixman_box32_t mask;
>>  };
>>
>>  struct weston_plane {
>> @@ -753,6 +754,8 @@ struct weston_view {
>>         struct wl_list link;
>>         struct wl_list layer_link;
>>         struct weston_plane *plane;
>> +       /* layer is set when building the views list, it's not always
>> valid! */
>> +       struct weston_layer *layer;
>>
>>         pixman_region32_t clip;
>>         float alpha;                     /* part of geometry, see below */
>> @@ -980,6 +983,12 @@ void
>>  weston_layer_init(struct weston_layer *layer, struct wl_list *below);
>>
>>  void
>> +weston_layer_set_mask(struct weston_layer *layer, int x, int y, int
>> width, int height);
>> +
>> +void
>> +weston_layer_set_mask_infinite(struct weston_layer *layer);
>> +
>> +void
>>  weston_plane_init(struct weston_plane *plane,
>>                         struct weston_compositor *ec,
>>                         int32_t x, int32_t y);
>> --
>> 1.8.5.1
>>
>> _______________________________________________
>> wayland-devel mailing list
>> wayland-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel


More information about the wayland-devel mailing list