Re: [PATCH weston] compositor: update the transform when attaching a new buffer

2013-12-10 Thread Pekka Paalanen
On Tue, 10 Dec 2013 15:55:29 +0100
Giulio Camuffo  wrote:

> if a surface has not a buffer yet and a weston_view gets created for
> it, the surface's width and height will be 0 and the view's
> output_mask will be 0, because the surface's area is 0. Later commits
> on the surface with valid buffers will then trigger a surface
> repaint, which will do nothing because of the output_mask set to 0.
> So by calling weston_update_transform() on the views of the surface
> at the end of weston_surface_commit() we update the output_mask of
> the surface and of the views, so they will be repainted.
> ---
>  src/compositor.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/compositor.c b/src/compositor.c
> index 6beea9c..20d9efb 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -2005,7 +2005,6 @@ weston_surface_commit(struct weston_surface
> *surface) surface->pending.buffer = NULL;
>   surface->pending.sx = 0;
>   surface->pending.sy = 0;
> - surface->pending.newly_attached = 0;
>  
>   /* wl_surface.damage */
>   pixman_region32_union(&surface->damage, &surface->damage,
> @@ -2046,7 +2045,13 @@ weston_surface_commit(struct weston_surface
> *surface) 
>   weston_surface_commit_subsurface_order(surface);
>  
> + if (surface->pending.newly_attached) {
> + wl_list_for_each(view, &surface->views, surface_link)
> + weston_view_update_transform(view);
> + }
>   weston_surface_schedule_repaint(surface);
> +
> + surface->pending.newly_attached = 0;
>  }
>  
>  static void

Hi,

Why is this fix needed in the first place, in what use case does this
bug manifest? Is this a regression? What introduced it?

Weren't we relying on the weston_surface->configure call that maps the
surface the first time to enforce a non-zero output_mask?

If you look at all (most?) of the functions in shell.c that actually
map surfaces, starting from an unmapped state, you see that they a) put
the view into a layer, and b) forcefully call
weston_view_update_transform(), which will then set up the output_mask.
Or at least it used to be like that, now the only case I can clearly
see in the code is lock_surface_configure(), but looks like map() with
at least shell_map_popup() works roughly the same.

There are likely also other details that have to be right in the right
order when a surface is mapped, but I forget. Every surface type gets
mapped slightly differently which is why we have not had a generic
weston_surface_map() function.

I don't think calling weston_view_update_transform() from the commit
handler *always* when there is a new attach is right. It should happen
only when mapping a surface. I believe there used to be the idea, that
e.g. input events are generated with the old surface state, until
output_repaint actually puts the new state on screen. Though, is that
idea relevant anymore?

(That actually shows a bug I think, a newly mapped surface may get input
events before it is painted on screen, does it not? No wait, we only do
input /from/ output_repaint? or do we? umm...)


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  wrote:

> 2013/12/10 Pekka Paalanen :
> > On Tue, 10 Dec 2013 14:46:25 +0100
> > Giulio Camuffo  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 :
> >> > On Tue, 10 Dec 2013 14:13:50 +0100
> >> > Giulio Camuffo  wrote:
> >> >
> >> >> 2013/12/10 Pekka Paalanen :
> >> >> > On Tue, 10 Dec 2013 11:13:42 +0100
> >> >> > Giulio Camuffo  wrote:
> >> >> >
> >> >> >> 2013/12/10 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.
> >> >> >>
> >> >> >> 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.

Re: [PATCH weston] introduces a setting to give permission to any client to do screenshots

2013-12-10 Thread Sebastian Wick

Am 2013-12-10 00:20, schrieb Bryce W. Harrington:

On Wed, Dec 04, 2013 at 05:38:23PM +0100, Sebastian Wick wrote:

This patch adds a screenshooter section with the "restrict-access"
setting which is on by default and is the current behavior of weston.
When turning it off, all clients can use the screenshooter protocol.
This makes screen capturing for clients easier because they don't
have to be started by weston.
---
 man/weston.ini.man  | 6 ++
 src/screenshooter.c | 8 +++-
 weston.ini.in   | 3 +++
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/man/weston.ini.man b/man/weston.ini.man
index 6be90bf..b94ac3d 100644
--- a/man/weston.ini.man
+++ b/man/weston.ini.man
@@ -408,6 +408,12 @@ The terminal shell (string). Sets the $TERM 
variable.

 sets the path to the xserver to run (string).
 .RE
 .RE
+.SH "SCREENSHOOTER SECTION"
+.TP 7
+.BI "restrict-access=" true
+only allow authenticated clients to take screenshots (boolean).
+.RE
+.RE
 .SH "SEE ALSO"
 .BR weston (1),
 .BR weston-launch (1),
diff --git a/src/screenshooter.c b/src/screenshooter.c
index 0c657bc..65b6c09 100644
--- a/src/screenshooter.c
+++ b/src/screenshooter.c
@@ -224,11 +224,17 @@ bind_shooter(struct wl_client *client,
 {
struct screenshooter *shooter = data;
struct wl_resource *resource;
+   struct weston_config_section *section;
+   int restrict_access;

resource = wl_resource_create(client,
  &screenshooter_interface, 1, id);

-   if (client != shooter->client) {
+	section = weston_config_get_section(shooter->ec->config, 
"screenshooter", NULL, NULL);

+   weston_config_section_get_bool(section,
+   "restrict-access", &restrict_access, 1);


Could also check the return value of weston_config_section_get_bool;
it'll set errno and return -1 if the config value was typo'd or
omitted.


It will save the default value, true in this case, if it's missing so it
should be fine.


But does this have security implications?  I assume it is restricted by
default in order to prevent clients from snooping.  Could you add a bit
more detail about the specific problem(s) being solved with this?  
Maybe

there's a way to solve the problem without fully dropping the
restriction?


I wrote a GStreamer wayland source element which needs to receive the 
data

somehow and it uses the screenshooter protocol to do so.

The screenshooter protocol however is restricted to clients which got
started by weston itself (only weston-screenshooter so far) to make sure
the client has not been manipulated. You would have to start every
application which might use the GStreamer wayland source element by 
weston.

It would make it drastically harder to use it and on a average linux PC
the current mechanism doesn't give you more security so it's pretty save
to turn the restriction off.

If you have a system where every client is sandboxed and you don't want 
any

client to see what the others have rendered, you should not turn the
restriction off.


+
+   if (restrict_access && client != shooter->client) {
wl_resource_post_error(resource, 
WL_DISPLAY_ERROR_INVALID_OBJECT,
   "screenshooter failed: permission 
denied");
wl_resource_destroy(resource);
diff --git a/weston.ini.in b/weston.ini.in
index 5181a9e..bc32567 100644
--- a/weston.ini.in
+++ b/weston.ini.in
@@ -65,3 +65,6 @@ path=@libexecdir@/weston-keyboard
 #constant_accel_factor = 50
 #min_accel_factor = 0.16
 #max_accel_factor = 1.0
+
+#[screenshooter]
+#restrict-access=false



Bryce


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


Re: [PATCH weston] compositor: update the transform when attaching a new buffer

2013-12-10 Thread Axel Davy

On 10/12/2013, Giulio Camuffo wrote :

if a surface has not a buffer yet and a weston_view gets created for
it, the surface's width and height will be 0 and the view's output_mask
will be 0, because the surface's area is 0. Later commits on the surface
with valid buffers will then trigger a surface repaint, which will do
nothing because of the output_mask set to 0. So by calling
weston_update_transform() on the views of the surface at the end of
weston_surface_commit() we update the output_mask of the surface and
of the views, so they will be repainted.
---
  src/compositor.c | 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/compositor.c b/src/compositor.c
index 6beea9c..20d9efb 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -2005,7 +2005,6 @@ weston_surface_commit(struct weston_surface *surface)
surface->pending.buffer = NULL;
surface->pending.sx = 0;
surface->pending.sy = 0;
-   surface->pending.newly_attached = 0;
  
  	/* wl_surface.damage */

pixman_region32_union(&surface->damage, &surface->damage,
@@ -2046,7 +2045,13 @@ weston_surface_commit(struct weston_surface *surface)
  
  	weston_surface_commit_subsurface_order(surface);
  
+	if (surface->pending.newly_attached) {

+   wl_list_for_each(view, &surface->views, surface_link)
+   weston_view_update_transform(view);
+   }
weston_surface_schedule_repaint(surface);
+
+   surface->pending.newly_attached = 0;
  }
  
  static void


Reviewed-by: Axel Davy 

It would probably need similarly to call weston_view_update_transform and 
weston_surface_schedule_repaint in weston_view_create in case a buffer is 
already attached, and the surface had no views previously.

Axel Davy

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


[Accessibility] Need to be able to register for key events globally

2013-12-10 Thread Piñeiro

GNOME Assistive Technologies need to be able to listen to key events
globally and have the possibility of consuming them. Example use
cases:

* Orca's presentation of navigation (Events not consumed)
  - Right Arrow: Speak character moved to (right of cursor)
  - Shift Right Arrow: Speak character selected (left of cursor)
  - Down Arrow: Speak next line
  - Etc.
* Orca's navigational commands (Events consumed)
  - H/Shift + H: Move amongst headings
  - NumPad 8: Speak the current line
  - NumPad 5: Speak the current word
  - NumPad 2: Speak the current character
  - Etc.

Current solution: The Orca screen reader calls AT-SPI2's
atspi_register_keystroke_listener(). AT-SPI2 then notifies Orca of key
events it receives from the toolkit implementation of ATK method
atk_add_key_event_listener(). Applications then have to wait for Orca
to consume the event or not. This requires two DBUS messages. Toolkit
authors want to abolish this. That's fine, *if* we have an
alternative. Do we?


--
Alejandro Piñeiro Iglesias (apinhe...@igalia.com)
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


[Accessibility] Need to be able to sythesize mouse events

2013-12-10 Thread Piñeiro
GNOME Assistive Technologies need to be able to synthesize mouse events. 
Use cases:

* Perform a mouse button click for users who cannot use a physical mouse
* Route the mouse pointer to an object or element to trigger its hover 
action


The Orca screen reader currently provides these commands via calls to 
AT-SPI2's atspi_generate_mouse_event() which in turn calls 
spi_dec_x11_generate_mouse_event() [1]. The equivalent functionality is 
needed for Wayland environments.


How are we going to accomplish this?

[1] 
https://git.gnome.org/browse/at-spi2-core/tree/registryd/deviceeventcontroller-x11.c#n1404


--
Alejandro Piñeiro Iglesias (apinhe...@igalia.com)
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston 1/2] compositor: keep track of the weston_layer a weston_view is in

2013-12-10 Thread Giulio Camuffo
This introduces a new struct, weston_layer_entry, which is now used
in place of wl_list to keep the link for the layer list in weston_view
and the head of the list in weston_layer.
weston_layer_entry also has a weston_layer*, which points to the layer
the view is in or, in the case the entry it's the head of the list, to
the layer itself.
---
 desktop-shell/exposay.c |   4 +-
 desktop-shell/input-panel.c |  11 ++--
 desktop-shell/shell.c   | 119 +++-
 src/compositor.c|  33 +---
 src/compositor.h|  14 +-
 src/data-device.c   |   6 +--
 src/input.c |   4 +-
 tests/weston-test.c |   6 +--
 8 files changed, 116 insertions(+), 81 deletions(-)

diff --git a/desktop-shell/exposay.c b/desktop-shell/exposay.c
index 81da00a..0d8eeb3 100644
--- a/desktop-shell/exposay.c
+++ b/desktop-shell/exposay.c
@@ -195,7 +195,7 @@ exposay_layout(struct desktop_shell *shell)
wl_list_init(&shell->exposay.surface_list);
 
shell->exposay.num_surfaces = 0;
-   wl_list_for_each(view, &workspace->layer.view_list, layer_link) {
+   wl_list_for_each(view, &workspace->layer.view_list.link, 
layer_link.link) {
if (!get_shell_surface(view->surface))
continue;
shell->exposay.num_surfaces++;
@@ -246,7 +246,7 @@ exposay_layout(struct desktop_shell *shell)
shell->exposay.surface_size = output->height / 2;
 
i = 0;
-   wl_list_for_each(view, &workspace->layer.view_list, layer_link) {
+   wl_list_for_each(view, &workspace->layer.view_list.link, 
layer_link.link) {
int pad;
 
pad = shell->exposay.surface_size + 
shell->exposay.padding_inner;
diff --git a/desktop-shell/input-panel.c b/desktop-shell/input-panel.c
index d97d073..b848640 100644
--- a/desktop-shell/input-panel.c
+++ b/desktop-shell/input-panel.c
@@ -70,8 +70,8 @@ show_input_panels(struct wl_listener *listener, void *data)
  &shell->input_panel.surfaces, link) {
if (!ipsurf->surface->buffer_ref.buffer)
continue;
-   wl_list_insert(&shell->input_panel_layer.view_list,
-  &ipsurf->view->layer_link);
+   weston_layer_entry_insert(&shell->input_panel_layer.view_list,
+ &ipsurf->view->layer_link);
weston_view_geometry_dirty(ipsurf->view);
weston_view_update_transform(ipsurf->view);
weston_surface_damage(ipsurf->surface);
@@ -97,7 +97,8 @@ hide_input_panels(struct wl_listener *listener, void *data)
wl_list_remove(&shell->input_panel_layer.link);
 
wl_list_for_each_safe(view, next,
- &shell->input_panel_layer.view_list, layer_link)
+ &shell->input_panel_layer.view_list.link,
+ layer_link.link)
weston_view_unmap(view);
 }
 
@@ -142,8 +143,8 @@ input_panel_configure(struct weston_surface *surface, 
int32_t sx, int32_t sy)
weston_view_set_position(ip_surface->view, x, y);
 
if (show_surface) {
-   wl_list_insert(&shell->input_panel_layer.view_list,
-  &ip_surface->view->layer_link);
+   weston_layer_entry_insert(&shell->input_panel_layer.view_list,
+ &ip_surface->view->layer_link);
weston_view_update_transform(ip_surface->view);
weston_surface_damage(surface);
weston_slide_run(ip_surface->view, 
ip_surface->view->surface->height, 0, NULL, NULL);
diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 9fbac00..6910cd0 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -258,12 +258,12 @@ shell_surface_is_top_fullscreen(struct shell_surface 
*shsurf)
 
shell = shell_surface_get_shell(shsurf);
 
-   if (wl_list_empty(&shell->fullscreen_layer.view_list))
+   if (wl_list_empty(&shell->fullscreen_layer.view_list.link))
return false;
 
-   top_fs_ev = container_of(shell->fullscreen_layer.view_list.next,
+   top_fs_ev = container_of(shell->fullscreen_layer.view_list.link.next,
 struct weston_view,
-layer_link);
+layer_link.link);
return (shsurf == get_shell_surface(top_fs_ev->surface));
 }
 
@@ -570,7 +570,8 @@ focus_state_surface_destroy(struct wl_listener *listener, 
void *data)
main_surface = weston_surface_get_main_surface(state->keyboard_focus);
 
next = NULL;
-   wl_list_for_each(view, &state->ws->layer.view_list, layer_link) {
+   wl_list_for_each(view,
+&state->ws->layer.view_list.link, layer_link.link) {
if (view->surface == main_surfa

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

2013-12-10 Thread Giulio Camuffo
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.
Implemented in the gl and pixman renderers only for now.
---

v2: do the masking in the renderers

 src/compositor.c  | 26 +-
 src/compositor.h  |  7 +++
 src/gl-renderer.c |  4 
 src/pixman-renderer.c |  4 
 4 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/src/compositor.c b/src/compositor.c
index 705326a..305f995 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -1194,10 +1194,16 @@ 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_link.layer->mask.x1 &&
+   iy >= view->layer_link.layer->mask.y1 &&
+   ix <= view->layer_link.layer->mask.x2 &&
+   iy <= view->layer_link.layer->mask.y2 &&
+   pixman_region32_contains_point(&view->surface->input,
   wl_fixed_to_int(*vx),
   wl_fixed_to_int(*vy),
   NULL))
@@ -1793,11 +1799,29 @@ weston_layer_init(struct weston_layer *layer, struct 
wl_list *below)
 {
wl_list_init(&layer->view_list.link);
layer->view_list.layer = layer;
+   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 94376f3..d618830 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -524,6 +524,7 @@ struct weston_layer_entry {
 struct weston_layer {
struct weston_layer_entry view_list;
struct wl_list link;
+   pixman_box32_t mask;
 };
 
 struct weston_plane {
@@ -986,6 +987,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);
diff --git a/src/gl-renderer.c b/src/gl-renderer.c
index 0e5afbe..899c280 100644
--- a/src/gl-renderer.c
+++ b/src/gl-renderer.c
@@ -521,6 +521,10 @@ draw_view(struct weston_view *ev, struct weston_output 
*output,
pixman_region32_intersect(&repaint,
  &ev->transform.boundingbox, damage);
pixman_region32_subtract(&repaint, &repaint, &ev->clip);
+   pixman_region32_t mask;
+   pixman_region32_init_with_extents(&mask, &ev->layer_link.layer->mask);
+   pixman_region32_intersect(&repaint, &repaint, &mask);
+   pixman_region32_fini(&mask);
 
if (!pixman_region32_not_empty(&repaint))
goto out;
diff --git a/src/pixman-renderer.c b/src/pixman-renderer.c
index e854e2a..65159c7 100644
--- a/src/pixman-renderer.c
+++ b/src/pixman-renderer.c
@@ -355,6 +355,10 @@ draw_view(struct weston_view *ev, struct weston_output 
*output,
pixman_region32_intersect(&repaint,
  &ev->transform.boundingbox, damage);
pixman_region32_subtract(&repaint, &repaint, &ev->clip);
+   pixman_region32_t mask;
+   pixman_region32_init_with_extents(&mask, &ev->layer_link.layer->mask);
+   pixman_region32_intersect(&repaint, &repaint, &mask);
+   pixman_region32_fini(&mask);
 
if (!pixman_region32_not_empty(&repaint))
goto out;
-- 
1.8.5.1

___
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 :
> On Tue, 10 Dec 2013 14:46:25 +0100
> Giulio Camuffo  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 :
>> > On Tue, 10 Dec 2013 14:13:50 +0100
>> > Giulio Camuffo  wrote:
>> >
>> >> 2013/12/10 Pekka Paalanen :
>> >> > On Tue, 10 Dec 2013 11:13:42 +0100
>> >> > Giulio Camuffo  wrote:
>> >> >
>> >> >> 2013/12/10 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.
>> >> >>
>> >> >> 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


[PATCH weston] compositor: update the transform when attaching a new buffer

2013-12-10 Thread Giulio Camuffo
if a surface has not a buffer yet and a weston_view gets created for
it, the surface's width and height will be 0 and the view's output_mask
will be 0, because the surface's area is 0. Later commits on the surface
with valid buffers will then trigger a surface repaint, which will do
nothing because of the output_mask set to 0. So by calling
weston_update_transform() on the views of the surface at the end of
weston_surface_commit() we update the output_mask of the surface and
of the views, so they will be repainted.
---
 src/compositor.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/compositor.c b/src/compositor.c
index 6beea9c..20d9efb 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -2005,7 +2005,6 @@ weston_surface_commit(struct weston_surface *surface)
surface->pending.buffer = NULL;
surface->pending.sx = 0;
surface->pending.sy = 0;
-   surface->pending.newly_attached = 0;
 
/* wl_surface.damage */
pixman_region32_union(&surface->damage, &surface->damage,
@@ -2046,7 +2045,13 @@ weston_surface_commit(struct weston_surface *surface)
 
weston_surface_commit_subsurface_order(surface);
 
+   if (surface->pending.newly_attached) {
+   wl_list_for_each(view, &surface->views, surface_link)
+   weston_view_update_transform(view);
+   }
weston_surface_schedule_repaint(surface);
+
+   surface->pending.newly_attached = 0;
 }
 
 static void
-- 
1.8.5.1

___
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  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 :
> > On Tue, 10 Dec 2013 14:13:50 +0100
> > Giulio Camuffo  wrote:
> >
> >> 2013/12/10 Pekka Paalanen :
> >> > On Tue, 10 Dec 2013 11:13:42 +0100
> >> > Giulio Camuffo  wrote:
> >> >
> >> >> 2013/12/10 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.
> >> >>
> >> >> 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 :
> On Tue, 10 Dec 2013 14:13:50 +0100
> Giulio Camuffo  wrote:
>
>> 2013/12/10 Pekka Paalanen :
>> > On Tue, 10 Dec 2013 11:13:42 +0100
>> > Giulio Camuffo  wrote:
>> >
>> >> 2013/12/10 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.
>> >>
>> >> 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:13:50 +0100
Giulio Camuffo  wrote:

> 2013/12/10 Pekka Paalanen :
> > On Tue, 10 Dec 2013 11:13:42 +0100
> > Giulio Camuffo  wrote:
> >
> >> 2013/12/10 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.
> >>
> >> 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 :
> On Tue, 10 Dec 2013 11:13:42 +0100
> Giulio Camuffo  wrote:
>
>> 2013/12/10 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.
>>
>> 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 11:13:42 +0100
Giulio Camuffo  wrote:

> 2013/12/10 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.
> 
> 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


[PATCH v2.5 weston 03/11] configure.ac: Correctly check for functions and libraries

2013-12-10 Thread Quentin Glidic
From: Quentin Glidic 

AC_SEARCH_LIBS is preferred over AC_CHECK_FUNC and AC_CHECK_LIB
See
http://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf.html#Libraries
AC_SEARCH_LIBS is doing precisely the double checking we used to have
for dlopen

Introduce WS_SEARCH_LIBS to make it easier for contributors

Signed-off-by: Quentin Glidic 
---

WS_SEARCH_LIBS one-liner version

 configure.ac | 20 
 m4/weston.m4 | 14 ++
 2 files changed, 18 insertions(+), 16 deletions(-)
 create mode 100644 m4/weston.m4

diff --git a/configure.ac b/configure.ac
index 40ae145..849cad7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -41,9 +41,7 @@ AC_ARG_VAR([WESTON_SHELL_CLIENT],
 
 PKG_PROG_PKG_CONFIG()
 
-AC_CHECK_FUNC([dlopen], [],
-  AC_CHECK_LIB([dl], [dlopen], DLOPEN_LIBS="-ldl"))
-AC_SUBST(DLOPEN_LIBS)
+WS_SEARCH_LIBS(DLOPEN, [dlopen], [dl])
 
 AC_CHECK_DECL(SFD_CLOEXEC,[],
  [AC_MSG_ERROR("SFD_CLOEXEC is needed to compile weston")],
@@ -258,13 +256,7 @@ fi
 AM_CONDITIONAL(ENABLE_VAAPI_RECORDER, test "x$have_libva" = xyes)
 
 
-AC_CHECK_LIB([jpeg], [jpeg_CreateDecompress], have_jpeglib=yes)
-if test x$have_jpeglib = xyes; then
-  JPEG_LIBS="-ljpeg"
-else
-  AC_ERROR([libjpeg not found])
-fi
-AC_SUBST(JPEG_LIBS)
+WS_SEARCH_LIBS(JPEG, [jpeg_CreateDecompress], [jpeg])
 
 PKG_CHECK_MODULES(CAIRO, [cairo])
 
@@ -334,12 +326,8 @@ AS_IF([test "x$have_systemd_login_209" = "xyes"],
 AC_ARG_ENABLE(weston-launch, [  --enable-weston-launch],, 
enable_weston_launch=yes)
 AM_CONDITIONAL(BUILD_WESTON_LAUNCH, test x$enable_weston_launch == xyes)
 if test x$enable_weston_launch == xyes; then
-  AC_CHECK_LIB([pam], [pam_open_session], [have_pam=yes], [have_pam=no])
-  if test x$have_pam == xno; then
-AC_ERROR([weston-launch requires pam])
-  fi
-  PAM_LIBS=-lpam
-  AC_SUBST(PAM_LIBS)
+   WS_SEARCH_LIBS(PAM, [pam_open_session], [pam],
+  [pam support required for weston-launch])
 fi
 
 if test x$enable_egl = xyes; then
diff --git a/m4/weston.m4 b/m4/weston.m4
new file mode 100644
index 000..ca38823
--- /dev/null
+++ b/m4/weston.m4
@@ -0,0 +1,14 @@
+# WS_SEARCH_LIBS(VARIABLE-PREFIX, FUNCTION, SEARCH-LIBS,
+#[ERROR-MSG-IF-NOT-FOUND])
+# 
+# Search for a library defining FUNC, if it's not already available
+AC_DEFUN([WS_SEARCH_LIBS], [dnl
+AC_ARG_VAR([$1][_LIBS], [linker flags for $1])dnl
+AC_SEARCH_LIBS([$2], [$3])
+case "$ac_cv_search_$2" in
+   no) AC_MSG_ERROR([m4_default([$4], [$2 required but not found])]) ;;
+   "none required") ;;
+   *) $1[]_LIBS="$ac_cv_search_$2" ;;
+esac
+AC_SUBST([$1][_LIBS])
+])
-- 
1.8.5.1

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


Re: [PATCH weston 03/11] configure.ac: Correctly check for functions and libraries

2013-12-10 Thread Quentin Glidic

On 10/12/2013 10:57, Ander Conselvan de Oliveira wrote:

On 12/09/2013 12:44 PM, Quentin Glidic wrote:

On 09/12/2013 11:28, Ander Conselvan de Oliveira wrote:
I am trusting the Autoconf manual[1] on this one. Basically, the
double-check we are doing for dlopen is exactly what AC_SEARCH_LIBS is
done for: checking in both a library and in libC.


That is the kind of information I would like to see in the commit
message if I stumble upon this 6 months from now. No need to dig through
the Autoconf manual. :)


You should have already read the whole manual, twice. The only reason 
people hate Autotools so much is that they do not read the manual and 
expect them to automagically get what they mean. :-)


New patch sent as a reply to this one. It breaks the series but I will 
send back the full series once all comments are addressed.




Anyway, this seems like a change for the worse IMO.


Following Autotools best practices/manual is *never* worse.


Now you're typing jpeg_CreateDecompress three times instead of just
one.


8 lines for the new version vs 7 lines for the old one…


My point was that having to type the same thing three times is error
prone, and the copy & paste error above kind of proves my point. Anyway,
if we can turn this into a one line macro that would be much better.


Ideally this could be just a one-liner, since this construct has to be
repeated a few times.


It is easy enough to create a macro to do the check. If so, do we want
the error message to be accurate (e.g. Weston vs clients vs
weston-launch)?


I think it would be good, if it's just a matter of adding one more
parameter.


Introduced WS_SEARCH_LIBS in the new version of this patch.



[1]
http://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf.html#Libraries




--

Quentin “Sardem FF7” Glidic
___
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 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.

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"  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->ma

Re: [PATCH] Fix compilation with FreeRdp 1.1 and master

2013-12-10 Thread Hardening

On 10/12/2013 09:25, Hardening wrote:

On 10/12/2013 00:42, Kristian Høgsberg wrote:

On Mon, Dec 09, 2013 at 10:16:39PM +0100, Hardening wrote:
The API to use remoteFx encoding has changed between master and 
stable 1.1

branch. This patch fixes compilation for both.
Please note that the freerdp/version.h file is generated in the very 
last versions

of FreeRdp so be sure to update/install the last versions.

Yeah, this doesn't compile for me with an older FreeRdp because of the
missing freerdp/version.h.  If we're going to require a newer FreeRdp
for this to compile, can we just drop the #if?  Or is version.h
included by freedrp.h or something so we can avoid the version.h
#include?


Until recent changes there were no safe way to know which version you 
were compiling against.
The FREERDP_VERSION_[MAJOR|MINOR|REVISION] macros have never been 
accurate to detect this (not updated when they should). These macros 
were included in the freerdp/freerdp.h and are now auto-generated in 
the freerdp/version.h file.


You're right i think i can have a better patch that will take care of 
the installation that are missing the freerdp/version.h file.
Sorry to disappoint but my previous patch is the only way to deal 
between stable-1.1 (which is fact is a stabilization in progress branch) 
and master. The patch works only with last version of stable-1.1 or 
master (the config.h is here from a longer time in master). To mitigate 
things, nobody ships FreeRdp 1.1 for now.
Sorry about that, things have not been done cleanly in FreeRdp, but 
we'll take care of that in the future.


Regards

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


Re: [PATCH weston 03/11] configure.ac: Correctly check for functions and libraries

2013-12-10 Thread Ander Conselvan de Oliveira

On 12/09/2013 12:44 PM, Quentin Glidic wrote:

On 09/12/2013 11:28, Ander Conselvan de Oliveira wrote:

On 12/08/2013 08:45 PM, Quentin Glidic wrote:

From: Quentin Glidic 

AC_SEARCH_LIBS is preferred over AC_CHECK_FUNC and AC_CHECK_LIB


Why is it preferred?


I am trusting the Autoconf manual[1] on this one. Basically, the
double-check we are doing for dlopen is exactly what AC_SEARCH_LIBS is
done for: checking in both a library and in libC.


That is the kind of information I would like to see in the commit 
message if I stumble upon this 6 months from now. No need to dig through 
the Autoconf manual. :)





Signed-off-by: Quentin Glidic 
---
  configure.ac | 37 +
  1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/configure.ac b/configure.ac
index 40ae145..c169311 100644
--- a/configure.ac
+++ b/configure.ac
@@ -41,9 +41,13 @@ AC_ARG_VAR([WESTON_SHELL_CLIENT],

  PKG_PROG_PKG_CONFIG()

-AC_CHECK_FUNC([dlopen], [],
-  AC_CHECK_LIB([dl], [dlopen], DLOPEN_LIBS="-ldl"))
-AC_SUBST(DLOPEN_LIBS)
+AC_SEARCH_LIBS([dlopen], [dl])
+case "$ac_cv_search_dlopen" in
+no) AC_MSG_ERROR([dlopen support required for Weston]) ;;
+"none required") ;;
+*) DLOPEN_LIBS="$ac_cv_search_dlopen" ;;
+esac
+AC_SUBST([DLOPEN_LIBS])

  AC_CHECK_DECL(SFD_CLOEXEC,[],
[AC_MSG_ERROR("SFD_CLOEXEC is needed to compile weston")],
@@ -258,13 +262,13 @@ fi
  AM_CONDITIONAL(ENABLE_VAAPI_RECORDER, test "x$have_libva" = xyes)


-AC_CHECK_LIB([jpeg], [jpeg_CreateDecompress], have_jpeglib=yes)
-if test x$have_jpeglib = xyes; then
-  JPEG_LIBS="-ljpeg"
-else
-  AC_ERROR([libjpeg not found])
-fi
-AC_SUBST(JPEG_LIBS)
+AC_SEARCH_LIBS([jpeg_CreateDecompress], [jpeg])
+case "$ac_cv_search_pam_open_session" in


Copy & paste error, you got pam_open_session instead of
jpeg_CreateDecompress.


Oops, fixed locally.



+no) AC_MSG_ERROR([libjpeg required but not found]) ;;
+"none required") ;;
+*) JPEG_LIBS="$ac_cv_search_jpeg_CreateDecompress" ;;
+esac
+AC_SUBST([JPEG_LIBS])


Anyway, this seems like a change for the worse IMO.


Following Autotools best practices/manual is *never* worse.

 > Now you're typing jpeg_CreateDecompress three times instead of just one.

8 lines for the new version vs 7 lines for the old one…


My point was that having to type the same thing three times is error 
prone, and the copy & paste error above kind of proves my point. Anyway, 
if we can turn this into a one line macro that would be much better.



 > Ideally this could be just a one-liner, since this construct has to be

repeated a few times.


It is easy enough to create a macro to do the check. If so, do we want
the error message to be accurate (e.g. Weston vs clients vs weston-launch)?


I think it would be good, if it's just a matter of adding one more 
parameter.



Cheers,
Ander





Cheers,
Ander



  PKG_CHECK_MODULES(CAIRO, [cairo])

@@ -334,12 +338,13 @@ AS_IF([test "x$have_systemd_login_209" = "xyes"],
  AC_ARG_ENABLE(weston-launch, [  --enable-weston-launch],,
enable_weston_launch=yes)
  AM_CONDITIONAL(BUILD_WESTON_LAUNCH, test x$enable_weston_launch ==
xyes)
  if test x$enable_weston_launch == xyes; then
-  AC_CHECK_LIB([pam], [pam_open_session], [have_pam=yes],
[have_pam=no])
-  if test x$have_pam == xno; then
-AC_ERROR([weston-launch requires pam])
-  fi
-  PAM_LIBS=-lpam
-  AC_SUBST(PAM_LIBS)
+AC_SEARCH_LIBS([pam_open_session], [pam])
+case "$ac_cv_search_pam_open_session" in
+no) AC_MSG_ERROR([pam support required for weston-launch]) ;;
+"none required") ;;
+*) PAM_LIBS="$ac_cv_search_pam_open_session" ;;
+esac
+AC_SUBST([PAM_LIBS])
  fi

  if test x$enable_egl = xyes; then





[1]
http://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf.html#Libraries




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


RE: [RFC] Common layout library for surfaces in a shell

2013-12-10 Thread Timo Lotterbach
Hi Nobuhiko-san,

I want to provide some feedback from GENIVI and IVI LayerManagement perspective.
Please find my comments inline.

BR, Timo
-- 
BMW Car IT GmbH
Timo Lotterbach
Spezialist Entwicklung
Petuelring 116
80809 München

Tel.:  +49 89 189311-77
Fax:  +49 89 189311-20
Mail: timo.lotterb...@bmw-carit.de
Web: http://www.bmw-carit.de
--
BMW Car IT GmbH
Geschäftsführer: Michael Würtenberger und Reinhard Stolle
Sitz und Registergericht: München HRB 134810
--



> -Original Message-
> From: Tanibata, Nobuhiko (ADITJ/SWG) [mailto:ntanib...@jp.adit-jv.com]
> Sent: Tuesday, December 03, 2013 1:42 AM
> To: wayland-devel@lists.freedesktop.org
> Cc: michael.schu...@bmw.de; Timo Lotterbach; Ishikawa, Tetsuri
> (ADITJ/SWG)
> Subject: [RFC] Common layout library for surfaces in a shell
> 
> Hi Wayland developers,
> 
> In reply to,
> http://lists.freedesktop.org/archives/wayland-devel/2013-
> September/011026.html
> I realign comments. This is more or less a RFC and any input would be very
> much appreciated.
> 
> When I think developing a shell referring Weston for ivi system, I face two
> difficulties. If a solution to solve these two difficulties, it may be useful 
> for ivi
> shall developers. I am not sure that it is only for ivi so it may apply other 
> types
> of system as well.
> 
> The first difficulty is that laying out surfaces by using Weston internal
> method. For the time being, I have to implement e.g. positioning surfaces by
> using Weston internal APIs. Additionally, ivi system often uses layer concept
> to group surfaces. If APIs standardizing management of structure: screen-
> >layer->surface and properties; position, visibility, and son are available, 
> >it
> might be helpful to reduce effort to devloper ivi type shell.

[TL] From my perspective, implementing ilmControl.so it's exactly the way to go.
We need to map IVI-style scene setups (screens/layers/surfaces) to Weston
internal scene setup. This mapping is independent of shell behavior and should
only be implemented once.

> One example of such APIs is ivi layer manager, http://projects.genivi.org/ivi-
> layer-management/
> It works outside of compositor but the set of definition can be used to define
> standard APIs for ivi-shell as well.

[TL] I agree. The IVI LayerManagement API is proven to work and supports all
Features currently required by IVI systems. Therefore I consider it a good 
starting point.

> The second difficulty is that Identifying specific surfaces in a shell. For
> example, such surfaces will be used by specific application like car 
> navigation
> and traffic information. These surfaces need to be laid out, e.g. top level by
> shell when some request happens in the system, for example, switching
> “destination” in the panel or on the steering. It is not done by touch of the
> surface. So these surfaces are identified by shell internally.

[TL] I fully agree to this.
I want to provide some more background on IVI Use Cases.
In IVI systems there are mainly two ways to setup compositing, and both rely on
identifying semantics of application content
(e.g. Navigation, HMI, Browser, Camera Assistance Systems, ...):

1. Compositor knows content/buffer sematics and composes screen content
 according to built-in ruleset (e.g. HMI always in front of Navigation, 
Browser
 runs in fullscreen mode without HMI, ...)
2. External process controls compositing (e.g. HMI-software). Then the external
 controller must know content/buffer semantics. Additionally a way of
 referencing surfaces from other processes is required.

To allow referencing or identifying surfaces of other processes, we rely on
globally known predefined IDs (e.g. surface 0x1000 = Browser) 

> If Weston has standard way to resolve these two difficulties, it might be
> helpful. Detail parts of proposals are,
> 1.I am enclosing a png file to show overview of related component. I
> plan to implement “ilmControl.so” to standardize APIs, now is raughly
> implemeted. The name of library and APIs need to be brushed up more.

[TL] Yes, we should find a better name. I would prefer Weston-style naming.

> 2.Protocol to identify a surface in shell. Genivi ivi layer manger project
> define a reference of the protocol as ivi-application.xml. It can tie Global 
> ID to
> wl_surface.
> - http://git.projects.genivi.org/?p=wayland-ivi-
> extension.git;a=blob;f=protocol/ivi-
> application.xml;h=aeeeba2267ecb549a43b6a2b4517b257cd66885e;hb=0d8f0
> d36d1696a657426a5bda1342c3743d08a7c (*1)

[TL] Yes, this is mandatory for nearly all IVI Compositing solutions out there.
One of the basic building blocks for Weston IVI-Shell.

> - To identify wl_surface, I thought wl_shell_surface::set_title can be
> used it. However wl_shell is desktop style, it shall not be used for 
> ivi-shell.
> - You also find ivi_surface.visi

Re: [PATCH] Add wl_dmabuf protocol

2013-12-10 Thread Benjamin Gaignard
Hi,

For me mmap capability means that both server and client can call
mmap() on the provided file descriptor.
I have no simple way to know if the driver support mmap().
The better option I see today is to create a dumb buffer and to try to
mmap it at server initialization.
Anyway client and server should always test the result of mmap() to be
sure that the pointer is valid.

About multiple gfx device, I will re-introduce device name capability
(at it was in wl_drm), I think that could solve this issue.

Regards,
Benjamin

2013/12/9 Pekka Paalanen :
> On Thu,  5 Dec 2013 18:36:38 +0100
> benjamin.gaign...@linaro.org wrote:
>
>> From: Benjamin Gaignard 
>>
>> It allow to use a dmabuf file descriptor in a wayland protocol.
>> To make as generic as possible it is up to the server to call
>> wl_dmabuf_send_format() and/or wl_dmabuf_send_capabilities() to signal
>> it capabilities.
>>
>> Signed-off-by: Benjamin Gaignard 
>> ---
>>  protocol/Makefile.am|6 +-
>>  protocol/wayland-dmabuf.xml |  134 +
>>  src/Makefile.am |   12 +-
>>  src/wayland-dmabuf.c|  275
>> +++
>> src/wayland-dmabuf.h|  134 + 5 files
>> changed, 557 insertions(+), 4 deletions(-) create mode 100644
>> protocol/wayland-dmabuf.xml create mode 100644 src/wayland-dmabuf.c
>>  create mode 100644 src/wayland-dmabuf.h
>>
>> diff --git a/protocol/Makefile.am b/protocol/Makefile.am
>> index e8b6290..8c9499f 100644
>> --- a/protocol/Makefile.am
>> +++ b/protocol/Makefile.am
>> @@ -1,4 +1,4 @@
>> -dist_pkgdata_DATA = wayland.xml wayland.dtd
>> +dist_pkgdata_DATA = wayland.xml wayland-dmabuf.xml wayland.dtd
>>
>>  if HAVE_XMLLINT
>>  .PHONY: validate
>> @@ -6,9 +6,9 @@ if HAVE_XMLLINT
>>  .%.xml.valid: %.xml
>>   $(AM_V_GEN)$(XMLLINT) --noout --dtdvalid
>> $(srcdir)/wayland.dtd $^ > $@
>> -validate: .wayland.xml.valid
>> +validate: .wayland.xml.valid .wayland-dmabuf.xml.valid
>>
>>  all-local: validate
>>
>> -CLEANFILES = .wayland.xml.valid
>> +CLEANFILES = .wayland.xml.valid .wayland-dmabuf.xml.valid
>>  endif
>> diff --git a/protocol/wayland-dmabuf.xml b/protocol/wayland-dmabuf.xml
>> new file mode 100644
>> index 000..b3b7ded
>> --- /dev/null
>> +++ b/protocol/wayland-dmabuf.xml
>> @@ -0,0 +1,134 @@
>> +
>> +
>> +
>> +  
>> +Copyright © 2008-2011 Kristian Høgsberg
>> +Copyright © 2010-2011 Intel Corporation
>> +
>> +Permission to use, copy, modify, distribute, and sell this
>> +software and its documentation for any purpose is hereby granted
>> +without fee, provided that\n the above copyright notice appear in
>> +all copies and that both that copyright notice and this
>> permission
>> +notice appear in supporting documentation, and that the name of
>> +the copyright holders not be used in advertising or publicity
>> +pertaining to distribution of the software without specific,
>> +written prior permission.  The copyright holders make no
>> +representations about the suitability of this software for any
>> +purpose.  It is provided "as is" without express or implied
>> +warranty.
>> +
>> +THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
>> +SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
>> +FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR
>> ANY
>> +SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
>> +WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER
>> IN
>> +AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
>> +ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
>> +THIS SOFTWARE.
>> +  
>> +
>> +  
>> +  
>> +
>> + 
>> + 
>> +
>> +
>> +
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +
>> +
>> +
>> +
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +
>> +
>> + 
>> +  
>> +
>> +
>> +
>> +  
>> +Bitmask of capabilities.
>> +  
>> +  
>> +
>> +
>> +
>> +  
>> +
>> +  
>> +
>> +
>
> Hi,
>
> what does the "mmap" capability bit actually mean? How is it used?
>
> I have heard that not all drivers supporting dma-buf do mmap(), but how
> does that reflect here? Is the mmap-capability depen

Re: [PATCH] Fix compilation with FreeRdp 1.1 and master

2013-12-10 Thread Hardening

On 10/12/2013 00:42, Kristian Høgsberg wrote:

On Mon, Dec 09, 2013 at 10:16:39PM +0100, Hardening wrote:

The API to use remoteFx encoding has changed between master and stable 1.1
branch. This patch fixes compilation for both.
Please note that the freerdp/version.h file is generated in the very last 
versions
of FreeRdp so be sure to update/install the last versions.

Yeah, this doesn't compile for me with an older FreeRdp because of the
missing freerdp/version.h.  If we're going to require a newer FreeRdp
for this to compile, can we just drop the #if?  Or is version.h
included by freedrp.h or something so we can avoid the version.h
#include?


Until recent changes there were no safe way to know which version you 
were compiling against.
The FREERDP_VERSION_[MAJOR|MINOR|REVISION] macros have never been 
accurate to detect this (not updated when they should). These macros 
were included in the freerdp/freerdp.h and are now auto-generated in the 
freerdp/version.h file.


You're right i think i can have a better patch that will take care of 
the installation that are missing the freerdp/version.h file.


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