Re: [PATCH weston] compositor: update the transform when attaching a new buffer
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
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
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
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
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
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
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
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 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
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
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
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
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 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
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
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
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 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
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
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
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
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
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