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

[Giulio Camuffo]

2013/12/4 Pekka Paalanen <ppaalanen at gmail.com>:
> On Tue,  3 Dec 2013 14:29:21 +0100
> Giulio Camuffo <giuliocamuffo at gmail.com> wrote:
>
>> This adds a 'mask' pixman_region32 to weston_view and to
>> weston_view::geometry. A user, a shell probably, can set a view's
>> mask to some region (e.g. the workspace area) and the view, and all
>> its transform children, will be clipped to that region.
>> weston_view::geometry.mask is the intersection of the view's mask and
>> of the parent's geometry.mask.
>> ---
>>  src/compositor.c | 25 +++++++++++++++++++++++--
>>  src/compositor.h |  2 ++
>>  2 files changed, 25 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/compositor.c b/src/compositor.c
>> index 32e72b1..8316653 100644
>> --- a/src/compositor.c
>> +++ b/src/compositor.c
>> @@ -366,6 +366,8 @@ weston_view_create(struct weston_surface *surface)
>>       view->plane = NULL;
>>
>>       pixman_region32_init(&view->clip);
>> +     region_init_infinite(&view->mask);
>> +     pixman_region32_init(&view->geometry.mask);
>>
>>       view->alpha = 1.0;
>>       pixman_region32_init(&view->transform.opaque);
>> @@ -926,6 +928,11 @@ weston_view_update_transform(struct weston_view
>> *view)
>>       weston_view_damage_below(view);
>>
>> +     if (parent)
>> +             pixman_region32_intersect(&view->geometry.mask,
>> &view->mask, &parent->geometry.mask);
>> +     else
>> +             pixman_region32_copy(&view->geometry.mask,
>> &view->mask); +
>>       pixman_region32_fini(&view->transform.boundingbox);
>>       pixman_region32_fini(&view->transform.opaque);
>>       pixman_region32_init(&view->transform.opaque);
>> @@ -1185,10 +1192,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
>> (pixman_region32_contains_point(&view->geometry.mask,
>> +                                                ix, iy, NULL) &&
>> +
>> pixman_region32_contains_point(&view->surface->input,
>> wl_fixed_to_int(*vx), wl_fixed_to_int(*vy),
>>                                                  NULL))
>> @@ -1274,8 +1285,10 @@ weston_view_destroy(struct weston_view *view)
>>       wl_list_remove(&view->link);
>>       wl_list_remove(&view->layer_link);
>>
>> +     pixman_region32_fini(&view->mask);
>>       pixman_region32_fini(&view->clip);
>>       pixman_region32_fini(&view->transform.boundingbox);
>> +     pixman_region32_fini(&view->geometry.mask);
>>
>>       weston_view_set_transform_parent(view, NULL);
>>
>> @@ -1481,7 +1494,15 @@ view_accumulate_damage(struct weston_view
>> *view, pixman_region32_union(&view->plane->damage,
>>                             &view->plane->damage, &damage);
>>       pixman_region32_fini(&damage);
>> -     pixman_region32_copy(&view->clip, opaque);
>> +
>> +     /* set view->clip with the part of
>> view->transform.boundingbox
>> +      * that doesn't fit into view->geometry.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 have
>> +      * a different mask.
>> +      */
>> +     pixman_region32_subtract(&view->clip,
>> &view->transform.boundingbox, &view->geometry.mask);
>> +     pixman_region32_union(&view->clip, &view->clip, opaque);
>>       pixman_region32_union(opaque, opaque,
>> &view->transform.opaque); }
>>
>> diff --git a/src/compositor.h b/src/compositor.h
>> index 3e878c8..e0b3895 100644
>> --- a/src/compositor.h
>> +++ b/src/compositor.h
>> @@ -750,6 +750,7 @@ struct weston_view {
>>       struct wl_list layer_link;
>>       struct weston_plane *plane;
>>
>> +     pixman_region32_t mask;
>>       pixman_region32_t clip;
>>       float alpha;                     /* part of geometry, see
>> below */
>> @@ -762,6 +763,7 @@ struct weston_view {
>>       struct {
>>               float x, y; /* surface translation on display */
>>               int32_t width, height;
>> +             pixman_region32_t mask;
>>
>>               /* struct weston_transform */
>>               struct wl_list transformation_list;
>
> Hi Giulio,
>
> these new fields are structured a little unexpectedly IMO. Originally,
> everything under 'geometry' is modifiable, and modifying them requires
> one to also call weston_view_geometry_dirty(), so that all
> transform-children will be updated too. The state under 'transform' are
> read-only, derived from 'geometry' state as needed (marked by the dirty
> flag).

Ah, I wasn't aware of this separation, thanks.

>
> Unfortunately, partly due to hysterical raisins(*), there are bits of
> 'geometry' also elsewhere, like the 'alpha' seen above, and
> weston_surface::width,height.
>
> Would it not be more consistent to have geometry.mask as the settable
> field, and transform.mask as the used field that also integrates the
> parent state?

I guess, but the reason I didn't put it into transform is that it
isn't really transformed with the weston_transforms set for the view.
I don't need that for my use case, and I can't think of any that would
atm.
So having a filed inside transform that isn't transformed sounds a
buit weird to me.


Thanks for the review,
Giulio

>
>
> Thanks,
> pq
>
> * Oh how I've been waiting to get to use that phrase. ;-)