[PATCH weston] compositor: add a layer clipping mechanism

[Giulio Camuffo]

2013/4/23 Pekka Paalanen <ppaalanen at gmail.com>

> On Mon,  8 Apr 2013 14:31:53 +0200
> Giulio Camuffo <giuliocamuffo at gmail.com> wrote:
>
> > this adds a mechanism to clip the surfaces belonging to a layer
> > to an arbitrary rect
>
> Hi Giulio,
>
> I don't have anything against the idea of clipping layers, but the
> implementation below is puzzling.
>
> > ---
> >
> > I'm using this functionality in my shell plugin
> > (https://github.com/giucam/orbital/tree/workspaces) to implement
> workspaces.
> > See http://www.youtube.com/watch?v=_o-sKdyUPO8 for a screencast.
> > As you see there i need to be able to show all the windows of all the
> > workspaces, but clipped to their workspace, so they don't stick out over
> > neighbour ones. So simply showing/hiding layers does not work.
> >
> >  src/compositor.c | 15 +++++++++++++++
> >  src/compositor.h |  8 ++++++++
> >  2 files changed, 23 insertions(+)
> >
> > diff --git a/src/compositor.c b/src/compositor.c
> > index 3645b04..a748894 100644
> > --- a/src/compositor.c
> > +++ b/src/compositor.c
> > @@ -1159,6 +1159,7 @@ surface_accumulate_damage(struct weston_surface
> *surface,
> >       pixman_region32_union(&surface->plane->damage,
> >                             &surface->plane->damage, &surface->damage);
> >       empty_region(&surface->damage);
> > +     pixman_region32_subtract(opaque, &surface->transform.boundingbox,
> &surface->layer->clip);
>
> So, you subtract layer.clip from the surface bounding box to produce
> 'opaque'. Hmm... I don't really understand the set operations here. It
> seems somehow inverted. What does layer.clip mean, is it the region
> that is inside or outside of the paintable region?
>
> If layer.clip is the outside, i.e. the region to be clipped away, isn't
> weston_layer_clip() then incapable of clipping from more than one side?
>
> Doesn't this subtracting into 'opaque' clobber the state that is being
> accumulated into it over several calls to surface_accumulate_damage()?
> 'opaque' is supposed to accumulate the opaque regions when walking the
> surface stack from top to bottom, so we can skip painting surface
> regions that are already hidden by opaque regions above. Simple
> assignment to it is strange.
>
> Could you explain these operations, since to me they look just wrong?
>

I don't fully understand how the repaint is done, and what e.g. the opaque
region there is, so i may well have done some stupid mistake.
layer->clip is a region outside of which any layer surface should be
clipped, so it renders only what is inside that region. Maybe 'clip' isn't
the right name, and e.g. 'visible' would be better.


>
> >       pixman_region32_copy(&surface->clip, opaque);
> >       pixman_region32_union(opaque, opaque, &surface->transform.opaque);
> >  }
> > @@ -1223,6 +1224,7 @@ weston_output_repaint(struct weston_output
> *output, uint32_t msecs)
> >       wl_list_for_each(layer, &ec->layer_list, link) {
> >               wl_list_for_each(es, &layer->surface_list, layer_link) {
> >                       weston_surface_update_transform(es);
> > +                     es->layer = layer;
> >                       wl_list_insert(ec->surface_list.prev, &es->link);
> >                       if (es->output == output) {
> >                               wl_list_insert_list(&frame_callback_list,
> > @@ -1319,11 +1321,24 @@ WL_EXPORT void
> >  weston_layer_init(struct weston_layer *layer, struct wl_list *below)
> >  {
> >       wl_list_init(&layer->surface_list);
> > +     region_init_infinite(&layer->clip);
> >       if (below != NULL)
> >               wl_list_insert(below, &layer->link);
> >  }
> >
> >  WL_EXPORT void
> > +weston_layer_clip(struct weston_layer *layer, int x, int y, int width,
> int height)
> > +{
> > +     pixman_region32_init_rect(&layer->clip, x, y, width, height);
> > +}
> > +
> > +WL_EXPORT void
> > +weston_layer_clip_infinite(struct weston_layer *layer)
> > +{
> > +    region_init_infinite(&layer->clip);
> > +}
> > +
> > +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 1e999a6..b3038bd 100644
> > --- a/src/compositor.h
> > +++ b/src/compositor.h
> > @@ -271,6 +271,7 @@ enum {
> >  struct weston_layer {
> >       struct wl_list surface_list;
> >       struct wl_list link;
> > +     pixman_region32_t clip;
> >  };
> >
> >  struct weston_plane {
> > @@ -413,6 +414,7 @@ struct weston_surface {
> >       struct wl_list layer_link;
> >       float alpha;                     /* part of geometry, see below */
> >       struct weston_plane *plane;
> > +     struct weston_layer *layer;
>
> I think we should have some comment or grouping here to indicate, that
> 'layer' is only valid during repaint.
>

Right.


>
> >
> >       void *renderer_state;
> >
> > @@ -598,6 +600,12 @@ void
> >  weston_layer_init(struct weston_layer *layer, struct wl_list *below);
> >
> >  void
> > +weston_layer_clip(struct weston_layer *layer, int x, int y, int width,
> int height);
> > +
> > +void
> > +weston_layer_clip_infinite(struct weston_layer *layer);
> > +
> > +void
> >  weston_plane_init(struct weston_plane *plane, int32_t x, int32_t y);
> >  void
> >  weston_plane_release(struct weston_plane *plane);
>
> Thanks,
> pq
>

Thanks for the review,
Giulio
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130423/445ba12c/attachment-0001.html>