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

2013-12-10 Thread Giulio Camuffo
2013/12/10 Jason Ekstrand ja...@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 giuliocamu...@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)
 +{
 +   

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

2013-12-10 Thread Pekka Paalanen
On Tue, 10 Dec 2013 11:13:42 +0100
Giulio Camuffo giuliocamu...@gmail.com wrote:

 2013/12/10 Jason Ekstrand ja...@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.

Does this work if the renderer paints the whole surface regardless of
damage? Rpi-renderer does that, since every surface is on its own
overlay.

The whole damage tracking is kind of unused on the rpi-renderer, since
the firmware will probably redraw everything anyway. Prohibiting damage
will not prevent parts of a surface from being painted. Damage is just
a hint saying what is not necessary to repaint.


Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


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

2013-12-10 Thread Giulio Camuffo
2013/12/10 Pekka Paalanen ppaala...@gmail.com:
 On Tue, 10 Dec 2013 11:13:42 +0100
 Giulio Camuffo giuliocamu...@gmail.com wrote:

 2013/12/10 Jason Ekstrand ja...@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.

 Does this work if the renderer paints the whole surface regardless of
 damage? Rpi-renderer does that, since every surface is on its own
 overlay.

 The whole damage tracking is kind of unused on the rpi-renderer, since
 the firmware will probably redraw everything anyway. Prohibiting damage
 will not prevent parts of a surface from being painted. Damage is just
 a hint saying what is not necessary to repaint.

No, the masking isn't done by not damaging the parts not in the mask,
but by adding the opposite of the mask in weston_view::clip. But i see
that the rpi renderer doesn't clip away that, so no, currently this
wouldn't work with it. I guess it could work though, right?



 Thanks,
 pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


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

2013-12-10 Thread Pekka Paalanen
On Tue, 10 Dec 2013 14:13:50 +0100
Giulio Camuffo giuliocamu...@gmail.com wrote:

 2013/12/10 Pekka Paalanen ppaala...@gmail.com:
  On Tue, 10 Dec 2013 11:13:42 +0100
  Giulio Camuffo giuliocamu...@gmail.com wrote:
 
  2013/12/10 Jason Ekstrand ja...@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.
 
  Does this work if the renderer paints the whole surface regardless
  of damage? Rpi-renderer does that, since every surface is on its own
  overlay.
 
  The whole damage tracking is kind of unused on the rpi-renderer,
  since the firmware will probably redraw everything anyway.
  Prohibiting damage will not prevent parts of a surface from being
  painted. Damage is just a hint saying what is not necessary to
  repaint.
 
 No, the masking isn't done by not damaging the parts not in the mask,
 but by adding the opposite of the mask in weston_view::clip. But i see
 that the rpi renderer doesn't clip away that, so no, currently this
 wouldn't work with it. I guess it could work though, right?

We do not have arbitrary clip rectangles. We can only pick one
arbitrary but axis-aligned rectangle from within the buffer to map onto
the screen and that's it, just like your average overlay hardware.

We could do more complex clips by using one dispmanx element (overlay)
per rectangle of the final paint region, but that gets complicated, and
even though there can be many elements, they are still a limited
resource. I do not think this would be feasible.

So if the surface can still be just a single rect on screen, overlaid
with other surfaces in the z-order, then it would be possible. But I
guess you'd have to separate occlusion clips from mask clips. Note
though, that a rect minus a rect is not in general a rect but a region.


Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


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

2013-12-10 Thread Giulio Camuffo
But the visible region of a surface wouldn't be a rect minus a rect,
but the intersection of two rects, which is always a rect as long as
they are both axis aligned, and surface rotation isn't supported
anyway by the rpi renderer, right? What's a rect minus a rect is the
weston_view::clip region, which is not the visible part of the surface
but the hidden part. So maybe having clip be the visible part would be
better for the rpi renderer, but anyway layer masking could still be
supported.

Still, this patch makes use of a mechanism already present in weston,
and which the rpi renderer doesn't respect. So it is a problem in the
rpi renderer or in the mechanism itself, not in the user of the
mechanism, imho.

Giulio

2013/12/10 Pekka Paalanen ppaala...@gmail.com:
 On Tue, 10 Dec 2013 14:13:50 +0100
 Giulio Camuffo giuliocamu...@gmail.com wrote:

 2013/12/10 Pekka Paalanen ppaala...@gmail.com:
  On Tue, 10 Dec 2013 11:13:42 +0100
  Giulio Camuffo giuliocamu...@gmail.com wrote:
 
  2013/12/10 Jason Ekstrand ja...@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.
 
  Does this work if the renderer paints the whole surface regardless
  of damage? Rpi-renderer does that, since every surface is on its own
  overlay.
 
  The whole damage tracking is kind of unused on the rpi-renderer,
  since the firmware will probably redraw everything anyway.
  Prohibiting damage will not prevent parts of a surface from being
  painted. Damage is just a hint saying what is not necessary to
  repaint.

 No, the masking isn't done by not damaging the parts not in the mask,
 but by adding the opposite of the mask in weston_view::clip. But i see
 that the rpi renderer doesn't clip away that, so no, currently this
 wouldn't work with it. I guess it could work though, right?

 We do not have arbitrary clip rectangles. We can only pick one
 arbitrary but axis-aligned rectangle from within the buffer to map onto
 the screen and that's it, just like your average overlay hardware.

 We could do more complex clips by using one dispmanx element (overlay)
 per rectangle of the final paint region, but that gets complicated, and
 even though there can be many elements, they are still a limited
 resource. I do not think this would be feasible.

 So if the surface can still be just a single rect on screen, overlaid
 with other surfaces in the z-order, then it would be possible. But I
 guess you'd have to separate occlusion clips from mask clips. Note
 though, that a rect minus a rect is not in general a rect but a region.


 Thanks,
 pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


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

2013-12-10 Thread Pekka Paalanen
On Tue, 10 Dec 2013 14:46:25 +0100
Giulio Camuffo giuliocamu...@gmail.com wrote:

 But the visible region of a surface wouldn't be a rect minus a rect,
 but the intersection of two rects, which is always a rect as long as
 they are both axis aligned, and surface rotation isn't supported
 anyway by the rpi renderer, right? What's a rect minus a rect is the
 weston_view::clip region, which is not the visible part of the surface
 but the hidden part. So maybe having clip be the visible part would be
 better for the rpi renderer, but anyway layer masking could still be
 supported.

Ok, so it is a double-negation of the mask rect in the end. If we can
have the mask rect in the rpi-renderer with the view, then it is
implementable indeed.

 Still, this patch makes use of a mechanism already present in weston,
 and which the rpi renderer doesn't respect. So it is a problem in the
 rpi renderer or in the mechanism itself, not in the user of the
 mechanism, imho.

And IMHO, the clip regions are only informational for the renderer, as
it makes sense to compute and cache them in the common code. Not
authoritative. We cannot support arbitrary clip regions in the
rpi-renderer in any case.

In fact, couldn't the renderer simply ignore the clip and damage regions
completely, as long as they just paint everything from back to front,
and produce the exact same output?

To me it's just a same kind of optimization as the damage regions.


Thanks,
pq

 2013/12/10 Pekka Paalanen ppaala...@gmail.com:
  On Tue, 10 Dec 2013 14:13:50 +0100
  Giulio Camuffo giuliocamu...@gmail.com wrote:
 
  2013/12/10 Pekka Paalanen ppaala...@gmail.com:
   On Tue, 10 Dec 2013 11:13:42 +0100
   Giulio Camuffo giuliocamu...@gmail.com wrote:
  
   2013/12/10 Jason Ekstrand ja...@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.
  
   Does this work if the renderer paints the whole surface
   regardless of damage? Rpi-renderer does that, since every
   surface is on its own overlay.
  
   The whole damage tracking is kind of unused on the rpi-renderer,
   since the firmware will probably redraw everything anyway.
   Prohibiting damage will not prevent parts of a surface from being
   painted. Damage is just a hint saying what is not necessary to
   repaint.
 
  No, the masking isn't done by not damaging the parts not in the
  mask, but by adding the opposite of the mask in weston_view::clip.
  But i see that the rpi renderer doesn't clip away that, so no,
  currently this wouldn't work with it. I guess it could work
  though, right?
 
  We do not have arbitrary clip rectangles. We can only pick one
  arbitrary but axis-aligned rectangle from within the buffer to map
  onto the screen and that's it, just like your average overlay
  hardware.
 
  We could do more complex clips by using one dispmanx element
  (overlay) per rectangle of the final paint region, but that gets
  complicated, and even though there can be many elements, they are
  still a limited resource. I do not think this would be feasible.
 
  So if the surface can still be just a single rect on screen,
  overlaid with other surfaces in the z-order, then it would be
  possible. But I guess you'd have to separate occlusion clips from
  mask clips. Note though, that a rect minus a rect is not in general
  a rect but a region.
 
 
  Thanks,
  pq

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


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

2013-12-10 Thread Giulio Camuffo
2013/12/10 Pekka Paalanen ppaala...@gmail.com:
 On Tue, 10 Dec 2013 14:46:25 +0100
 Giulio Camuffo giuliocamu...@gmail.com wrote:

 But the visible region of a surface wouldn't be a rect minus a rect,
 but the intersection of two rects, which is always a rect as long as
 they are both axis aligned, and surface rotation isn't supported
 anyway by the rpi renderer, right? What's a rect minus a rect is the
 weston_view::clip region, which is not the visible part of the surface
 but the hidden part. So maybe having clip be the visible part would be
 better for the rpi renderer, but anyway layer masking could still be
 supported.

 Ok, so it is a double-negation of the mask rect in the end. If we can
 have the mask rect in the rpi-renderer with the view, then it is
 implementable indeed.

It is available in the renderers, as view-layer-mask.


 Still, this patch makes use of a mechanism already present in weston,
 and which the rpi renderer doesn't respect. So it is a problem in the
 rpi renderer or in the mechanism itself, not in the user of the
 mechanism, imho.

 And IMHO, the clip regions are only informational for the renderer, as
 it makes sense to compute and cache them in the common code. Not
 authoritative. We cannot support arbitrary clip regions in the
 rpi-renderer in any case.

 In fact, couldn't the renderer simply ignore the clip and damage regions
 completely, as long as they just paint everything from back to front,
 and produce the exact same output?

 To me it's just a same kind of optimization as the damage regions.

Mmh, i didn't think of clip as an optimization only, but I guess you
may be right. If that's the case maybe I should modify directly the gl
and pixman renderers to do the masking themselves instead of relying
on clip.

Thanks,
Giulio



 Thanks,
 pq

 2013/12/10 Pekka Paalanen ppaala...@gmail.com:
  On Tue, 10 Dec 2013 14:13:50 +0100
  Giulio Camuffo giuliocamu...@gmail.com wrote:
 
  2013/12/10 Pekka Paalanen ppaala...@gmail.com:
   On Tue, 10 Dec 2013 11:13:42 +0100
   Giulio Camuffo giuliocamu...@gmail.com wrote:
  
   2013/12/10 Jason Ekstrand ja...@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.
  
   Does this work if the renderer paints the whole surface
   regardless of damage? Rpi-renderer does that, since every
   surface is on its own overlay.
  
   The whole damage tracking is kind of unused on the rpi-renderer,
   since the firmware will probably redraw everything anyway.
   Prohibiting damage will not prevent parts of a surface from being
   painted. Damage is just a hint saying what is not necessary to
   repaint.
 
  No, the masking isn't done by not damaging the parts not in the
  mask, but by adding the opposite of the mask in weston_view::clip.
  But i see that the rpi renderer doesn't clip away that, so no,
  currently this wouldn't work with it. I guess it could work
  though, right?
 
  We do not have arbitrary clip rectangles. We can only pick one
  arbitrary but axis-aligned rectangle from within the buffer to map
  onto the screen and that's it, just like your average overlay
  hardware.
 
  We could do more complex clips by using one dispmanx element
  (overlay) per rectangle of the final paint region, but that gets
  complicated, and even though there can be many elements, they are
  still a limited resource. I do not think this would be feasible.
 
  So if the surface can still be just a single rect on screen,
  overlaid with other surfaces in the z-order, then it would be
  possible. But I guess you'd have to separate occlusion clips from
  mask clips. Note though, that a rect minus a rect is not in general
  a rect but a region.
 
 
  Thanks,
  pq

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


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

2013-12-10 Thread Pekka Paalanen
On Tue, 10 Dec 2013 16:09:18 +0100
Giulio Camuffo giuliocamu...@gmail.com wrote:

 2013/12/10 Pekka Paalanen ppaala...@gmail.com:
  On Tue, 10 Dec 2013 14:46:25 +0100
  Giulio Camuffo giuliocamu...@gmail.com wrote:
 
  But the visible region of a surface wouldn't be a rect minus a
  rect, but the intersection of two rects, which is always a rect as
  long as they are both axis aligned, and surface rotation isn't
  supported anyway by the rpi renderer, right? What's a rect minus a
  rect is the weston_view::clip region, which is not the visible
  part of the surface but the hidden part. So maybe having clip be
  the visible part would be better for the rpi renderer, but anyway
  layer masking could still be supported.
 
  Ok, so it is a double-negation of the mask rect in the end. If we
  can have the mask rect in the rpi-renderer with the view, then it is
  implementable indeed.
 
 It is available in the renderers, as view-layer-mask.
 
 
  Still, this patch makes use of a mechanism already present in
  weston, and which the rpi renderer doesn't respect. So it is a
  problem in the rpi renderer or in the mechanism itself, not in the
  user of the mechanism, imho.
 
  And IMHO, the clip regions are only informational for the renderer,
  as it makes sense to compute and cache them in the common code. Not
  authoritative. We cannot support arbitrary clip regions in the
  rpi-renderer in any case.
 
  In fact, couldn't the renderer simply ignore the clip and damage
  regions completely, as long as they just paint everything from back
  to front, and produce the exact same output?
 
  To me it's just a same kind of optimization as the damage regions.
 
 Mmh, i didn't think of clip as an optimization only, but I guess you
 may be right. If that's the case maybe I should modify directly the gl
 and pixman renderers to do the masking themselves instead of relying
 on clip.

I'm not sure you need to go that far. I'd be happy with just some
explicit comments in the code either near the mask field or
weston_view, that say that if a renderer does not honour damage and
clip regions, it must use mask etc. directly to produce correct results.

And say in the commit message, that the rpi-renderer does not implement
this. Maybe even add an XXX-comment in the rpi-renderer while at it.
That would be ok by me.

I don't think there is any way to formulate the mask code so that
rpi-renderer would not need modifications, right?

Or hmm... what about output region? Ah no, rpi-renderer uses
0,0 and weston_output::width,height.


Thanks,
pq

  2013/12/10 Pekka Paalanen ppaala...@gmail.com:
   On Tue, 10 Dec 2013 14:13:50 +0100
   Giulio Camuffo giuliocamu...@gmail.com wrote:
  
   2013/12/10 Pekka Paalanen ppaala...@gmail.com:
On Tue, 10 Dec 2013 11:13:42 +0100
Giulio Camuffo giuliocamu...@gmail.com wrote:
   
2013/12/10 Jason Ekstrand ja...@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.
   
Does this work if the renderer paints the whole surface
regardless of damage? Rpi-renderer does that, since every
surface is on its own overlay.
   
The whole damage tracking is kind of unused on the
rpi-renderer, since the firmware will probably redraw
everything anyway. Prohibiting damage will not prevent parts
of a surface from being painted. Damage is just a hint saying
what is not necessary to repaint.
  
   No, the masking isn't done by not damaging the parts not in the
   mask, but by adding the opposite of the mask in
   weston_view::clip. But i see that the rpi renderer doesn't clip
   away that, so no, currently this wouldn't work with it. I guess
   it could work though, right?
  
   We do not have arbitrary clip rectangles. We can only pick one
   arbitrary but axis-aligned rectangle from within the buffer to
   map onto the screen and that's it, just like your average overlay
   hardware.
  
   We could do more complex clips by using one dispmanx element
   (overlay) per rectangle of the final paint region, but that gets
   complicated, and even though there can be many elements, they are
   still a limited resource. I do not think this would be feasible.
  
   So if the surface can still be just a single rect on screen,
   overlaid with other surfaces in the z-order, then it would be
   possible. But I guess you'd have to separate occlusion clips from
   mask clips. Note though, that a rect minus a rect is not in
   general a rect but a region.
  
  
   Thanks,
   pq
 


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

2013-12-09 Thread Jason Ekstrand
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.

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.

More comments below.

On Dec 9, 2013 3:36 PM, Giulio Camuffo giuliocamu...@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.

 }
 }
 @@ -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