Re: [PATCH] src/shell.c: set black_surface->width and height
On Fri, Nov 29, 2013 at 11:18:14AM +0800, Xiong Zhang wrote: > From: XiongZhang Btw, it looks like you have your email name set up in .gitconfig as "XiongZhang" not "Xiong Zhang". Is that intentional? I've edited it to "Xiong Zhang" as I commit seeing how your sign-off below has the space in it. Kristian > full screen black_surface doesn't have associated wl_buffer, so > black_surface->width and height can't get value through > weston_surface_commit(). then weston_surface_damage(black_surface) > will be wrong in shell_stack_fullscreen(), the result is that we > can't see a full screen view whe APP window's size isn't equal to > output's size like running weston-gears > > Signed-off-by: Xiong Zhang > --- > src/shell.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/src/shell.c b/src/shell.c > index 605f090..ebcf3e2 100644 > --- a/src/shell.c > +++ b/src/shell.c > @@ -2254,6 +2254,8 @@ create_black_surface(struct weston_compositor *ec, > pixman_region32_init_rect(&surface->opaque, 0, 0, w, h); > pixman_region32_fini(&surface->input); > pixman_region32_init_rect(&surface->input, 0, 0, w, h); > + surface->width = w; > + surface->height = h; > > weston_view_configure(view, x, y, w, h); > > -- > 1.8.3.2 > > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] src/shell.c: set black_surface->width and height
On Fri, Nov 29, 2013 at 10:10:00AM +0200, Pekka Paalanen wrote: > On Fri, 29 Nov 2013 11:18:14 +0800 > Xiong Zhang wrote: > > > From: XiongZhang > > > > full screen black_surface doesn't have associated wl_buffer, so > > black_surface->width and height can't get value through > > weston_surface_commit(). then weston_surface_damage(black_surface) > > will be wrong in shell_stack_fullscreen(), the result is that we > > can't see a full screen view whe APP window's size isn't equal to > > output's size like running weston-gears > > > > Signed-off-by: Xiong Zhang > > --- > > src/shell.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/src/shell.c b/src/shell.c > > index 605f090..ebcf3e2 100644 > > --- a/src/shell.c > > +++ b/src/shell.c > > @@ -2254,6 +2254,8 @@ create_black_surface(struct weston_compositor > > *ec, pixman_region32_init_rect(&surface->opaque, 0, 0, w, h); > > pixman_region32_fini(&surface->input); > > pixman_region32_init_rect(&surface->input, 0, 0, w, h); > > + surface->width = w; > > + surface->height = h; > > > > weston_view_configure(view, x, y, w, h); > > > > I wonder if the surface size should be given as arguments to > weston_surface_set_color() with a possibly more descriptive function > name. A function, that would imitate committing a solid color > buffer to a weston_surface. > > OTOH, solid color surfaces do not go through the configure hook either, > so I'm not sure that makes sense. > > Actually I once tried to clean up these things, among the attempts > there is > http://cgit.collabora.com/git/user/pq/weston.git/commit/?h=clipscale-wip&id=a1f3c20a0dfcbcf30e96a99928c923a7a2ab396c > I never proposed that patch on the list, because the whole series was > quickly going down the rabbit hole, and particularly I wasn't sure > whose responsibility it is to call weston_view_geometry_dirty(). > > But maybe it would give some ideas. I think simply setting > surface->width,height like you propose is prone to forgetting updating > some dependent state. > > Also, IIRC jekstrand was working on removing weston_view::width,height, > which means that this weston_surface::width,height issue would be fixed > in the process. > > As a stop-gap measure, I guess your patch is fine, though. It certainly > looks needed. Indeed, patch committed. Kristian ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 01/16] xdg_shell: Adding a new shell protocol.
On Fri, Nov 29, 2013 at 06:41:59PM -0500, Jasper St. Pierre wrote: > On Fri, Nov 29, 2013 at 5:43 PM, Kristian Høgsberg wrote: > > > On Wed, Nov 27, 2013 at 03:50:17PM -0200, Rafael Antognolli wrote: > > > xdg_shell is a protocol aimed to substitute wl_shell in the long term, > > > but will not be part of the wayland core protocol. It starts as a > > > non-stable API, aimed to be used as a development place at first, and > > > once features are defined as required by several desktop shells, we can > > > finally make it stable. > > > > > > It provides mainly two new interfaces: xdg_surface and xdg_popup. > > > > > > The xdg_surface interface implements a desktop-style window, that can be > > > moved, resized, maximized, etc. It provides a request for creating > > > child/parent relationship, called xdg_surface.set_transient_for. > > > > > > The xdg_popup interface implements a desktop-style popup/menu. A > > > xdg_popup is always transient for another surface, and also has implicit > > > grab. > > > > I think we can land this as is, though there are changes that I'd like > > to see. We'll get back to them after we land the series but what I'm > > thinking is: > > > > - I don't like ping/pong on popup surfaces, and in fact, I think we > >can move it to xdg_shell, since it's really a per-client thing, not > >a per-surface thing. > > > > - set_transient_for is not a great name. I know it comes from ICCCM, > >but I wonder if we can find a better name. If it's just stacking, > >then maybe just set_aboe? > > > > It specifies a parent-child relationship. In mutter, we'll use this to > attach modal dialogs to their parent windows, for instance. Saying it's a parent-child relationship doesn't really help - it's a generic conecept and it's not clear what it might mean in this context. Which is why I'd prefer something more specific. And it sounds like there's something we're not capturing here: dialogs that are transient_for will be attached by mutter, but floating toolboxes are also transient_for but doesn't get attached. I'm guessing mutter also looks at wether the window in question is a dialog type or a toolbox type. > > - protocol for server-initiated move/resize (eg mod+drag or kb > >resize?), probably similar to what we do for server side requestsed > >maximize. > > > > What protocol is there here? When the server initiates a move or resize, the client just sees its pointer focus disappear and then potentially later come back when the move/resize is done. For move, I don't know if we need anything, but for resize, would could have a "request_resize(edges)" event from the server that asks the client to trigger a resize. The client can ignore if it doesn't feel like resizing or start a resize by sending the resize request. This gives the client a more structured way to decline a resize and wm doesn't try to resize a non-resizable window. > > - not sure configure needs the edges flag anymore, in the cases where > >the server requests a change to the surface size, the server > >typically also knows where the window should go. I think the flag > >dates back to before the configure callback and the shell didn't > >have a way to intercept the attach. We relied on the client > >passing the right dx/dy, but now the shell gets the configure > >callback and can keep the right and top edges in place in case > >we're resizing from bottom+left. This should also fix xwayland > >resizing from top or left side. > > > > - we now have set/unset/request_set/request_unset for fullscreen and > >maximize, and we'll add minimized, probably tiled_left/right. This > >is going to explode into a large number of requests and events all > >with the same structure and flow as the maximized flow we've > >discussied this week. We should consider a more generic approach: > > > > - set(MAXIMIZED), unset(MAXIMIZED) requests, where maximized is an > > enum, that is, we don't allow set(MAXIMIZED | FULLSCREEN), but > > since it's all atomic under wl_surface.commit, > > set(MAXIMIZED)+set(FULLSCREEN) will have the intended effect > > (ignore that MAXIMIZED and FULLSCREEN doesn't make sense). > > > > - requet_set(MAXIMIZED) and request_unset(MAXIMIZED) to let the > > server intiate the transition. > > > > My hesitation with the enum would be people adding their own extensions to > the enum without standardizing anything... > > ... if that's something we want (for e.g. tiled-top, tiled-bottom), then we > should look at maybe having strings as states? Yeah, strings could work. Kristian > Kristian > > > > ... snip ... > > > > > ___ > > > wayland-devel mailing list > > > wayland-devel@lists.freedesktop.org > > > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > > ___ > > wayland-devel mailing list > > wayland-devel
Re: [PATCH 00/16] xdg-shell protocol, implementation and port of toytoolkit.
On Fri, Nov 29, 2013 at 02:23:11PM -0800, Kristian Høgsberg wrote: > On Wed, Nov 27, 2013 at 03:50:16PM -0200, Rafael Antognolli wrote: > > As discussed previously, this patch series adds the xdg-shell protocol, its > > implementation on weston, and the client implementation on toytoolkit. > > Hi Raphael, > > I think we want to get this in early and then iterate on it in-tree > instead of kicking this big patch set around on the list for too long. > Overall it looks good, there are just a few things that I'd like to > fix up before we land the patches. I'll reply and review to the > individual patches. > > Also, I'll try to land Philip Withnalls series from Nov 25, since it > looks like a nice clean up and ends up fixing a bug (always lovely). > It does conflict quite a bit with your shell.c patches, but it > shouldn't be too hard to rebase or just redo them on top. Btw, once we get the xdg-shell part of shell.c landed, I'd like to rename the structs to the xdg names, ie struct shell_surface becomes struct xdg_surface etc. > Kristian > > > Jasper St. Pierre (5): > > westoy: Schedule a full resize when we add a subsurface > > westoy: Use subsurfaces for tooltips instead of transient windows > > westoy: Remove fullscreen methods > > westoy: Remove MAXIMIZED and FULLSCREEN as separate window types > > westoy: Port the toy toolkit over to xdg-shell > > > > Rafael Antognolli (11): > > xdg_shell: Adding a new shell protocol. > > shell: Remove SHELL_SURFACE_FULLSCREEN and SHELL_SURFACE_MAXIMIZED. > > shell: Change set_maximized to receive internal structures. > > xdg-shell: Implement part of the interface. > > xdg-shell: Implementing maximized and fullscreen states. > > xdg-shell: Implement set_transient_for request. > > xdg-shell: Implement xdg_popup. > > xdg-shell: Add surface size to configure. > > xdg-shell: xdg_surface_set_output should only affect fullscreen. > > xdg-shell: Use xdg-shell in simple-shm. > > xdg-shell: Add key bindings for setting maximized and fullscreen. > > > > clients/.gitignore | 2 + > > clients/Makefile.am| 12 +- > > clients/fullscreen.c | 19 +- > > clients/simple-shm.c | 50 +-- > > clients/transformed.c | 21 +- > > clients/window.c | 418 - > > clients/window.h | 3 - > > protocol/Makefile.am | 3 +- > > protocol/xdg-shell.xml | 438 ++ > > src/.gitignore | 2 + > > src/Makefile.am| 6 +- > > src/shell.c| 829 > > + > > 12 files changed, 1387 insertions(+), 416 deletions(-) > > create mode 100644 protocol/xdg-shell.xml > > > > -- > > 1.8.3.1 > > > > ___ > > wayland-devel mailing list > > wayland-devel@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 15/16] xdg-shell: Use xdg-shell in simple-shm.
On Wed, Nov 27, 2013 at 03:50:31PM -0200, Rafael Antognolli wrote:
> ---
> clients/Makefile.am | 4 +++-
> clients/simple-shm.c | 50 ++
> 2 files changed, 33 insertions(+), 21 deletions(-)
>
> diff --git a/clients/Makefile.am b/clients/Makefile.am
> index 91174bf..10f0d2a 100644
> --- a/clients/Makefile.am
> +++ b/clients/Makefile.am
> @@ -37,7 +37,9 @@ simple_clients_programs = \
>
> weston_simple_shm_SOURCES = simple-shm.c \
> ../shared/os-compatibility.c\
> - ../shared/os-compatibility.h
> + ../shared/os-compatibility.h\
> + xdg-shell-client-protocol.h \
> + xdg-shell-protocol.c
> weston_simple_shm_CPPFLAGS = $(SIMPLE_CLIENT_CFLAGS)
> weston_simple_shm_LDADD = $(SIMPLE_CLIENT_LIBS)
>
> diff --git a/clients/simple-shm.c b/clients/simple-shm.c
> index 81bb54e..d95b272 100644
> --- a/clients/simple-shm.c
> +++ b/clients/simple-shm.c
> @@ -34,12 +34,13 @@
>
> #include
> #include "../shared/os-compatibility.h"
> +#include "xdg-shell-client-protocol.h"
>
> struct display {
> struct wl_display *display;
> struct wl_registry *registry;
> struct wl_compositor *compositor;
> - struct wl_shell *shell;
> + struct xdg_shell *xdg_shell;
> struct wl_shm *shm;
> uint32_t formats;
> };
> @@ -54,7 +55,8 @@ struct window {
> struct display *display;
> int width, height;
> struct wl_surface *surface;
> - struct wl_shell_surface *shell_surface;
> + struct xdg_surface *shell_surface;
> + // struct wl_shell_surface *shell_surface;
> struct buffer buffers[2];
> struct buffer *prev_buffer;
> struct wl_callback *callback;
> @@ -111,24 +113,24 @@ create_shm_buffer(struct display *display, struct
> buffer *buffer,
> }
>
> static void
> -handle_ping(void *data, struct wl_shell_surface *shell_surface,
> - uint32_t serial)
> +handle_ping(void *data, struct xdg_surface *shell_surface,
> + uint32_t serial)
> {
> - wl_shell_surface_pong(shell_surface, serial);
> + xdg_surface_pong(shell_surface, serial);
> }
>
> static void
> -handle_configure(void *data, struct wl_shell_surface *shell_surface,
> +handle_configure(void *data, struct xdg_surface *shell_surface,
>uint32_t edges, int32_t width, int32_t height)
> {
> }
>
> static void
> -handle_popup_done(void *data, struct wl_shell_surface *shell_surface)
> +handle_popup_done(void *data, struct xdg_surface *shell_surface)
> {
> }
>
> -static const struct wl_shell_surface_listener shell_surface_listener = {
> +static const struct xdg_surface_listener shell_surface_listener = {
> handle_ping,
> handle_configure,
> handle_popup_done
> @@ -148,16 +150,16 @@ create_window(struct display *display, int width, int
> height)
> window->width = width;
> window->height = height;
> window->surface = wl_compositor_create_surface(display->compositor);
> - window->shell_surface = wl_shell_get_shell_surface(display->shell,
> -window->surface);
> + window->shell_surface = xdg_shell_get_xdg_surface(display->xdg_shell,
> + window->surface);
>
> if (window->shell_surface)
> - wl_shell_surface_add_listener(window->shell_surface,
> - &shell_surface_listener, window);
> + xdg_surface_add_listener(window->shell_surface,
> + &shell_surface_listener, window);
>
> - wl_shell_surface_set_title(window->shell_surface, "simple-shm");
> + // wl_shell_surface_set_title(window->shell_surface, "simple-shm");
>
> - wl_shell_surface_set_toplevel(window->shell_surface);
> + // wl_shell_surface_set_toplevel(window->shell_surface);
This looks a bit WIP. Lets use the xdg requests here.
> return window;
> }
> @@ -173,7 +175,7 @@ destroy_window(struct window *window)
> if (window->buffers[1].buffer)
> wl_buffer_destroy(window->buffers[1].buffer);
>
> - wl_shell_surface_destroy(window->shell_surface);
> + // wl_shell_surface_destroy(window->shell_surface);
> wl_surface_destroy(window->surface);
> free(window);
> }
> @@ -310,9 +312,12 @@ registry_handle_global(void *data, struct wl_registry
> *registry,
> d->compositor =
> wl_registry_bind(registry,
>id, &wl_compositor_interface, 1);
> - } else if (strcmp(interface, "wl_shell") == 0) {
> - d->shell = wl_registry_bind(registry,
> - id, &wl_shell_interface, 1);
> + // } else if (strcmp(interface, "wl_shell") == 0) {
> + // d->shell = wl_registry_bind(registry,
> + //
Re: [PATCH 07/16] xdg-shell: Implement xdg_popup.
On Wed, Nov 27, 2013 at 03:50:23PM -0200, Rafael Antognolli wrote:
> ---
> src/shell.c | 159
> ++--
> 1 file changed, 155 insertions(+), 4 deletions(-)
>
> diff --git a/src/shell.c b/src/shell.c
> index 98fb0fe..0daa136 100644
> --- a/src/shell.c
> +++ b/src/shell.c
> @@ -247,6 +247,12 @@ struct ping_timer {
> uint32_t serial;
> };
>
> +enum shell_surface_class_interface {
> + SHELL_CLASS_WL_SHELL_SURFACE,
> + SHELL_CLASS_XDG_SURFACE,
> + SHELL_CLASS_XDG_POPUP
> +};
> +
> struct shell_surface {
> struct wl_resource *resource;
> struct wl_signal destroy_signal;
> @@ -257,6 +263,7 @@ struct shell_surface {
> struct weston_surface *parent;
> struct desktop_shell *shell;
>
> + enum shell_surface_class_interface shell_class;
> enum shell_surface_type type, next_type;
> char *title, *class;
> int32_t saved_x, saved_y;
> @@ -1918,7 +1925,17 @@ ping_handler(struct weston_surface *surface, uint32_t
> serial)
> wl_event_loop_add_timer(loop, ping_timeout_handler,
> shsurf);
> wl_event_source_timer_update(shsurf->ping_timer->source,
> ping_timeout);
>
> - wl_shell_surface_send_ping(shsurf->resource, serial);
> + switch (shsurf->shell_class) {
We can just use wl_resource_instance_of() on shsurf->resource here.
> + case SHELL_CLASS_WL_SHELL_SURFACE:
> + wl_shell_surface_send_ping(shsurf->resource,
> serial);
> + break;
> + case SHELL_CLASS_XDG_SURFACE:
> + xdg_surface_send_ping(shsurf->resource, serial);
> + break;
> + case SHELL_CLASS_XDG_POPUP:
> + xdg_popup_send_ping(shsurf->resource, serial);
> + break;
> + }
> }
> }
>
> @@ -2664,7 +2681,17 @@ popup_grab_end(struct weston_pointer *pointer)
> assert(!wl_list_empty(&shseat->popup_grab.surfaces_list));
> /* Send the popup_done event to all the popups open */
> wl_list_for_each(shsurf, &shseat->popup_grab.surfaces_list,
> popup.grab_link) {
> - wl_shell_surface_send_popup_done(shsurf->resource);
> + switch (shsurf->shell_class) {
Use wl_resource_instance_of() as well, and let's add a helper we can
use in shell_map_popup() as well.
> + case SHELL_CLASS_WL_SHELL_SURFACE:
> +
> wl_shell_surface_send_popup_done(shsurf->resource);
> + break;
> + case SHELL_CLASS_XDG_POPUP:
> +
> xdg_popup_send_popup_done(shsurf->resource,
> +
> shsurf->popup.serial);
> + break;
> + default:
> + break;
> + }
> shsurf->popup.shseat = NULL;
> if (prev) {
> wl_list_init(&prev->popup.grab_link);
> @@ -2726,7 +2753,17 @@ shell_map_popup(struct shell_surface *shsurf)
> if (shseat->seat->pointer->grab_serial == shsurf->popup.serial) {
> add_popup_grab(shsurf, shseat);
> } else {
> - wl_shell_surface_send_popup_done(shsurf->resource);
> + switch (shsurf->shell_class) {
> + case SHELL_CLASS_WL_SHELL_SURFACE:
> +
> wl_shell_surface_send_popup_done(shsurf->resource);
> + break;
> + case SHELL_CLASS_XDG_POPUP:
> + xdg_popup_send_popup_done(shsurf->resource,
> + shsurf->popup.serial);
> + break;
> + default:
> + break;
> + }
> shseat->popup_grab.client = NULL;
> }
> }
> @@ -2932,6 +2969,8 @@ shell_get_shell_surface(struct wl_client *client,
> return;
> }
>
> + shsurf->shell_class = SHELL_CLASS_WL_SHELL_SURFACE;
> +
> shsurf->resource =
> wl_resource_create(client,
> &wl_shell_surface_interface, 1, id);
> @@ -3169,6 +3208,8 @@ xdg_get_xdg_surface(struct wl_client *client,
> return;
> }
>
> + shsurf->shell_class = SHELL_CLASS_XDG_SURFACE;
> +
> shsurf->resource =
> wl_resource_create(client,
> &xdg_surface_interface, 1, id);
> @@ -3177,12 +3218,122 @@ xdg_get_xdg_surface(struct wl_client *client,
> shsurf, shell_destroy_shell_surface);
> }
>
> +
Re: [PATCH 01/16] xdg_shell: Adding a new shell protocol.
On Fri, Nov 29, 2013 at 5:43 PM, Kristian Høgsberg wrote: > On Wed, Nov 27, 2013 at 03:50:17PM -0200, Rafael Antognolli wrote: > > xdg_shell is a protocol aimed to substitute wl_shell in the long term, > > but will not be part of the wayland core protocol. It starts as a > > non-stable API, aimed to be used as a development place at first, and > > once features are defined as required by several desktop shells, we can > > finally make it stable. > > > > It provides mainly two new interfaces: xdg_surface and xdg_popup. > > > > The xdg_surface interface implements a desktop-style window, that can be > > moved, resized, maximized, etc. It provides a request for creating > > child/parent relationship, called xdg_surface.set_transient_for. > > > > The xdg_popup interface implements a desktop-style popup/menu. A > > xdg_popup is always transient for another surface, and also has implicit > > grab. > > I think we can land this as is, though there are changes that I'd like > to see. We'll get back to them after we land the series but what I'm > thinking is: > > - I don't like ping/pong on popup surfaces, and in fact, I think we >can move it to xdg_shell, since it's really a per-client thing, not >a per-surface thing. > > - set_transient_for is not a great name. I know it comes from ICCCM, >but I wonder if we can find a better name. If it's just stacking, >then maybe just set_aboe? > It specifies a parent-child relationship. In mutter, we'll use this to attach modal dialogs to their parent windows, for instance. > - protocol for server-initiated move/resize (eg mod+drag or kb >resize?), probably similar to what we do for server side requestsed >maximize. > What protocol is there here? > - not sure configure needs the edges flag anymore, in the cases where >the server requests a change to the surface size, the server >typically also knows where the window should go. I think the flag >dates back to before the configure callback and the shell didn't >have a way to intercept the attach. We relied on the client >passing the right dx/dy, but now the shell gets the configure >callback and can keep the right and top edges in place in case >we're resizing from bottom+left. This should also fix xwayland >resizing from top or left side. > > - we now have set/unset/request_set/request_unset for fullscreen and >maximize, and we'll add minimized, probably tiled_left/right. This >is going to explode into a large number of requests and events all >with the same structure and flow as the maximized flow we've >discussied this week. We should consider a more generic approach: > > - set(MAXIMIZED), unset(MAXIMIZED) requests, where maximized is an > enum, that is, we don't allow set(MAXIMIZED | FULLSCREEN), but > since it's all atomic under wl_surface.commit, > set(MAXIMIZED)+set(FULLSCREEN) will have the intended effect > (ignore that MAXIMIZED and FULLSCREEN doesn't make sense). > > - requet_set(MAXIMIZED) and request_unset(MAXIMIZED) to let the > server intiate the transition. > My hesitation with the enum would be people adding their own extensions to the enum without standardizing anything... ... if that's something we want (for e.g. tiled-top, tiled-bottom), then we should look at maybe having strings as states? Kristian > ... snip ... > > ___ > > wayland-devel mailing list > > wayland-devel@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > -- Jasper ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 06/16] xdg-shell: Implement set_transient_for request.
On Wed, Nov 27, 2013 at 03:50:22PM -0200, Rafael Antognolli wrote:
> The implementation just sets a parent/child relationship between the
> xdg_surface and its parent, passed as argument of this request. Stacking
> might be affected (that's up to the compositor).
>
> This implementation does not affect the code that handles the previous
> transient surface type. It should still work as expected, but it's not
> possible to set that surface type to a xdg_surface.
> ---
> src/shell.c | 50 +-
> 1 file changed, 29 insertions(+), 21 deletions(-)
>
> diff --git a/src/shell.c b/src/shell.c
> index 6cbb4bd..98fb0fe 100644
> --- a/src/shell.c
> +++ b/src/shell.c
> @@ -2955,6 +2955,18 @@ xdg_surface_destroy(struct wl_client *client,
> }
>
> static void
> +xdg_surface_set_transient_for(struct wl_client *client,
> + struct wl_resource *resource,
> + struct wl_resource *parent_resource)
> +{
> + struct shell_surface *shsurf = wl_resource_get_user_data(resource);
> + struct weston_surface *parent =
> + wl_resource_get_user_data(parent_resource);
> +
> + shsurf->parent = parent;
> +}
> +
> +static void
> xdg_surface_pong(struct wl_client *client,
>struct wl_resource *resource,
>uint32_t serial)
> @@ -3078,7 +3090,7 @@ xdg_surface_unset_maximized(struct wl_client *client,
>
> static const struct xdg_surface_interface xdg_surface_implementation = {
> xdg_surface_destroy,
> - NULL, // set_transient_for
> + xdg_surface_set_transient_for,
> xdg_surface_set_title,
> xdg_surface_set_app_id,
> xdg_surface_pong,
> @@ -4567,27 +4579,23 @@ map(struct desktop_shell *shell, struct shell_surface
> *shsurf,
> }
>
> /* surface stacking order, see also activate() */
> - switch (shsurf->type) {
> - case SHELL_SURFACE_POPUP:
> - case SHELL_SURFACE_TRANSIENT:
> - /* TODO: Handle a parent with multiple views */
> - parent = get_default_view(shsurf->parent);
> - if (parent) {
> - wl_list_remove(&shsurf->view->layer_link);
> - wl_list_insert(parent->layer_link.prev,
> -&shsurf->view->layer_link);
> + if (shsurf->type != SHELL_SURFACE_NONE) {
> + if (shsurf->parent) {
> + /* TODO: Handle a parent with multiple views */
> + parent = get_default_view(shsurf->parent);
> + if (parent) {
> + wl_list_remove(&shsurf->view->layer_link);
> + wl_list_insert(parent->layer_link.prev,
> +&shsurf->view->layer_link);
> + }
> + } else {
> + if (!shsurf->cur.fullscreen) {
> + ws = get_current_workspace(shell);
> + wl_list_remove(&shsurf->view->layer_link);
> + wl_list_insert(&ws->layer.view_list,
> +&shsurf->view->layer_link);
> + }
> }
> - break;
> - case SHELL_SURFACE_NONE:
> - break;
> - case SHELL_SURFACE_XWAYLAND:
> - default:
> - if (shsurf->cur.fullscreen)
> - break;
> - ws = get_current_workspace(shell);
> - wl_list_remove(&shsurf->view->layer_link);
> - wl_list_insert(&ws->layer.view_list, &shsurf->view->layer_link);
> - break;
Can we split this part that changes map() out in a separate patch that
drops SHELL_SURFACE_TRANSIENT first like you did for
SHELL_SURFACE_MAXIMIZE/FULLSCREEN? Then shsurf->parent == NULL means
"not transient_for" and shsurf->parent != NULL means "transient for
that surface".
Kristian
> }
>
> if (shsurf->type != SHELL_SURFACE_NONE) {
> --
> 1.8.3.1
>
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 04/16] xdg-shell: Implement part of the interface.
On Wed, Nov 27, 2013 at 03:50:20PM -0200, Rafael Antognolli wrote:
> Basic requests are implemented, enough to get a surface displayed.
> ---
> src/.gitignore | 2 +
> src/Makefile.am | 6 +-
> src/shell.c | 228
> +---
> 3 files changed, 223 insertions(+), 13 deletions(-)
>
> diff --git a/src/.gitignore b/src/.gitignore
> index 723967d..8e59be6 100644
> --- a/src/.gitignore
> +++ b/src/.gitignore
> @@ -19,3 +19,5 @@ workspaces-protocol.c
> workspaces-server-protocol.h
> input-method-protocol.c
> input-method-server-protocol.h
> +xdg-shell-protocol.c
> +xdg-shell-server-protocol.h
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 551dff9..f6f277d 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -294,7 +294,9 @@ desktop_shell_la_CFLAGS = $(GCC_CFLAGS)
> $(COMPOSITOR_CFLAGS)
> desktop_shell_la_SOURCES = \
> shell.c \
> desktop-shell-protocol.c\
> - desktop-shell-server-protocol.h
> + desktop-shell-server-protocol.h \
> + xdg-shell-protocol.c\
> + xdg-shell-server-protocol.h
> endif
>
> if ENABLE_TABLET_SHELL
> @@ -355,6 +357,8 @@ BUILT_SOURCES = \
> input-method-server-protocol.h \
> workspaces-server-protocol.h\
> workspaces-protocol.c \
> + xdg-shell-protocol.c\
> + xdg-shell-server-protocol.h \
> git-version.h
>
> CLEANFILES = $(BUILT_SOURCES)
> diff --git a/src/shell.c b/src/shell.c
> index 507d46f..f775899 100644
> --- a/src/shell.c
> +++ b/src/shell.c
> @@ -40,6 +40,7 @@
> #include "input-method-server-protocol.h"
> #include "workspaces-server-protocol.h"
> #include "../shared/config-parser.h"
> +#include "xdg-shell-server-protocol.h"
>
> #define DEFAULT_NUM_WORKSPACES 1
> #define DEFAULT_WORKSPACE_CHANGE_ANIMATION_LENGTH 200
> @@ -1564,8 +1565,8 @@ surface_move(struct shell_surface *shsurf, struct
> weston_seat *seat)
> }
>
> static void
> -shell_surface_move(struct wl_client *client, struct wl_resource *resource,
> -struct wl_resource *seat_resource, uint32_t serial)
> +common_surface_move(struct wl_resource *resource,
> + struct wl_resource *seat_resource, uint32_t serial)
> {
> struct weston_seat *seat = wl_resource_get_user_data(seat_resource);
> struct shell_surface *shsurf = wl_resource_get_user_data(resource);
> @@ -1587,6 +1588,13 @@ shell_surface_move(struct wl_client *client, struct
> wl_resource *resource,
> }
> }
>
> +static void
> +shell_surface_move(struct wl_client *client, struct wl_resource *resource,
> +struct wl_resource *seat_resource, uint32_t serial)
> +{
> + common_surface_move(resource, seat_resource, serial);
> +}
> +
> struct weston_resize_grab {
> struct shell_grab base;
> uint32_t edges;
> @@ -1741,9 +1749,9 @@ surface_resize(struct shell_surface *shsurf,
> }
>
> static void
> -shell_surface_resize(struct wl_client *client, struct wl_resource *resource,
> - struct wl_resource *seat_resource, uint32_t serial,
> - uint32_t edges)
> +common_surface_resize(struct wl_resource *resource,
> + struct wl_resource *seat_resource, uint32_t serial,
> + uint32_t edges)
> {
> struct weston_seat *seat = wl_resource_get_user_data(seat_resource);
> struct shell_surface *shsurf = wl_resource_get_user_data(resource);
> @@ -1763,6 +1771,14 @@ shell_surface_resize(struct wl_client *client, struct
> wl_resource *resource,
> }
>
> static void
> +shell_surface_resize(struct wl_client *client, struct wl_resource *resource,
> + struct wl_resource *seat_resource, uint32_t serial,
> + uint32_t edges)
> +{
> + common_surface_resize(resource, seat_resource, serial, edges);
> +}
> +
> +static void
> end_busy_cursor(struct shell_surface *shsurf, struct weston_pointer
> *pointer);
>
> static void
> @@ -1943,12 +1959,10 @@ create_pointer_focus_listener(struct weston_seat
> *seat)
> }
>
> static void
> -shell_surface_pong(struct wl_client *client, struct wl_resource *resource,
> - uint32_t serial)
> +surface_pong(struct shell_surface *shsurf, uint32_t serial)
> {
> - struct shell_surface *shsurf = wl_resource_get_user_data(resource);
> - struct weston_seat *seat;
> struct weston_compositor *ec = shsurf->surface->compositor;
> + struct weston_seat *seat;
>
> if (shsurf->ping_timer == NULL)
> /* Just ignore unsolicited pong. */
> @@ -1965,6 +1979,16 @@ shell_surface_pong(struct wl_client *client, struct
> wl_resource *resource,
> }
>
> static void
> +shell_surface_pong(struct wl_client *client, struct wl_resource *resource,
> +
Re: [PATCH 03/16] shell: Change set_maximized to receive internal structures.
On Wed, Nov 27, 2013 at 03:50:19PM -0200, Rafael Antognolli wrote:
> Change the parameters of set_maximized from wl_resource and wl_client to
> shell_surface and weston_output. This will allow it to be used with
> xdg-shell too.
> ---
> src/shell.c | 19 ---
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/src/shell.c b/src/shell.c
> index cf89a84..507d46f 100644
> --- a/src/shell.c
> +++ b/src/shell.c
> @@ -2204,19 +2204,17 @@ get_output_panel_height(struct desktop_shell *shell,
> }
>
> static void
> -set_maximized(struct wl_client *client,
> - struct wl_resource *resource,
> - struct wl_resource *output_resource)
> +set_maximized(struct shell_surface *shsurf,
> + struct weston_output *output)
> {
> - struct shell_surface *shsurf = wl_resource_get_user_data(resource);
> struct weston_surface *es = shsurf->surface;
> struct desktop_shell *shell = NULL;
> uint32_t edges = 0, panel_height = 0;
>
> /* get the default output, if the client set it as NULL
> check whether the ouput is available */
> - if (output_resource)
> - shsurf->output = wl_resource_get_user_data(output_resource);
> + if (output)
> + shsurf->output = output;
> else if (es->output)
> shsurf->output = es->output;
> else
> @@ -2239,10 +2237,17 @@ shell_surface_set_maximized(struct wl_client *client,
> struct wl_resource *output_resource)
> {
> struct shell_surface *shsurf = wl_resource_get_user_data(resource);
> + struct weston_output *output;
> +
> surface_clear_next_states(shsurf);
> - set_maximized(client, resource, output_resource);
> shsurf->next.maximized = true;
> shsurf->state_changed = true;
I was going to say that these details should be in set_maximized(),
but resetting the state is part of wl_shell semantics and should stay
here. This patch is good to go, but depends on previous patch.
> +
> + if (output_resource)
> + output = wl_resource_get_user_data(output_resource);
> + else
> + output = NULL;
> + set_maximized(shsurf, output);
> }
>
> static void
> --
> 1.8.3.1
>
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 02/16] shell: Remove SHELL_SURFACE_FULLSCREEN and SHELL_SURFACE_MAXIMIZED.
On Wed, Nov 27, 2013 at 03:50:18PM -0200, Rafael Antognolli wrote:
> These surface types don't exist anymore inside weston desktop shell
> implementation. They are just exposed as wl_shell surface types, but
> internally the implementation is done with surface states.
>
> The previous
> behavior (setting a surface type unsets another one) still happens when
> using wl_shell. This change is mainly done as a refactory to allow
> xdg-shell to use the same code.
> ---
> src/shell.c | 224
> +---
> 1 file changed, 123 insertions(+), 101 deletions(-)
This patch is good, it's a nice, self-contained step towards
xdg-shell. I just have a couple of nit-picks below.
> diff --git a/src/shell.c b/src/shell.c
> index f102e9a..cf89a84 100644
> --- a/src/shell.c
> +++ b/src/shell.c
> @@ -237,8 +237,6 @@ enum shell_surface_type {
> SHELL_SURFACE_NONE,
> SHELL_SURFACE_TOPLEVEL,
> SHELL_SURFACE_TRANSIENT,
> - SHELL_SURFACE_FULLSCREEN,
> - SHELL_SURFACE_MAXIMIZED,
> SHELL_SURFACE_POPUP,
> SHELL_SURFACE_XWAYLAND
> };
> @@ -298,6 +296,12 @@ struct shell_surface {
> struct wl_list link;
>
> const struct weston_shell_client *client;
> +
> + struct {
> + bool maximized;
> + bool fullscreen;
> + } cur, next; /* surface states */
We try to avoid abbreviations like cur - can we just call it state and
next_state?
> + bool state_changed;
> };
>
> struct shell_grab {
> @@ -1450,7 +1454,7 @@ surface_touch_move(struct shell_surface *shsurf, struct
> weston_seat *seat)
> if (!shsurf)
> return -1;
>
> - if (shsurf->type == SHELL_SURFACE_FULLSCREEN)
> + if (shsurf->cur.fullscreen)
> return 0;
> if (shsurf->grabbed)
> return 0;
> @@ -1541,7 +1545,7 @@ surface_move(struct shell_surface *shsurf, struct
> weston_seat *seat)
>
> if (shsurf->grabbed)
> return 0;
> - if (shsurf->type == SHELL_SURFACE_FULLSCREEN)
> + if (shsurf->cur.fullscreen)
> return 0;
>
> move = malloc(sizeof *move);
> @@ -1715,8 +1719,7 @@ surface_resize(struct shell_surface *shsurf,
> {
> struct weston_resize_grab *resize;
>
> - if (shsurf->type == SHELL_SURFACE_FULLSCREEN ||
> - shsurf->type == SHELL_SURFACE_MAXIMIZED)
> + if (shsurf->cur.fullscreen || shsurf->cur.maximized)
> return 0;
>
> if (edges == 0 || edges > 15 ||
> @@ -1746,7 +1749,7 @@ shell_surface_resize(struct wl_client *client, struct
> wl_resource *resource,
> struct shell_surface *shsurf = wl_resource_get_user_data(resource);
> struct weston_surface *surface;
>
> - if (shsurf->type == SHELL_SURFACE_FULLSCREEN)
> + if (shsurf->cur.fullscreen)
> return;
>
> surface =
> weston_surface_get_main_surface(seat->pointer->focus->surface);
> @@ -2061,26 +2064,31 @@ shell_unset_maximized(struct shell_surface *shsurf)
> static int
> reset_shell_surface_type(struct shell_surface *surface)
> {
> - switch (surface->type) {
> - case SHELL_SURFACE_FULLSCREEN:
> + if (surface->cur.fullscreen)
> shell_unset_fullscreen(surface);
> - break;
> - case SHELL_SURFACE_MAXIMIZED:
> + if (surface->cur.maximized)
> shell_unset_maximized(surface);
> - break;
> - case SHELL_SURFACE_NONE:
> - case SHELL_SURFACE_TOPLEVEL:
> - case SHELL_SURFACE_TRANSIENT:
> - case SHELL_SURFACE_POPUP:
> - case SHELL_SURFACE_XWAYLAND:
> - break;
> - }
>
> surface->type = SHELL_SURFACE_NONE;
> return 0;
> }
>
> static void
> +set_full_output(struct shell_surface *shsurf)
> +{
> + shsurf->saved_x = shsurf->view->geometry.x;
> + shsurf->saved_y = shsurf->view->geometry.y;
> + shsurf->saved_position_valid = true;
> +
> + if (!wl_list_empty(&shsurf->rotation.transform.link)) {
> + wl_list_remove(&shsurf->rotation.transform.link);
> + wl_list_init(&shsurf->rotation.transform.link);
> + weston_view_geometry_dirty(shsurf->view);
> + shsurf->saved_rotation_valid = true;
> + }
> +}
> +
> +static void
> set_surface_type(struct shell_surface *shsurf)
> {
> struct weston_surface *pes = shsurf->parent;
> @@ -2089,10 +2097,14 @@ set_surface_type(struct shell_surface *shsurf)
> reset_shell_surface_type(shsurf);
>
> shsurf->type = shsurf->next_type;
> + shsurf->cur = shsurf->next;
> shsurf->next_type = SHELL_SURFACE_NONE;
> + shsurf->state_changed = false;
>
> switch (shsurf->type) {
> case SHELL_SURFACE_TOPLEVEL:
> + if (shsurf->cur.maximized || shsurf->cur.fullscreen)
> + set_full_output(shsurf);
> break;
> case SHELL_SURFACE_TRANSIENT:
> if (pev)
> @@ -2101,20 +2113,6 @@ set_surface_type(struct shell_surf
Re: [PATCH 01/16] xdg_shell: Adding a new shell protocol.
On Wed, Nov 27, 2013 at 03:50:17PM -0200, Rafael Antognolli wrote: > xdg_shell is a protocol aimed to substitute wl_shell in the long term, > but will not be part of the wayland core protocol. It starts as a > non-stable API, aimed to be used as a development place at first, and > once features are defined as required by several desktop shells, we can > finally make it stable. > > It provides mainly two new interfaces: xdg_surface and xdg_popup. > > The xdg_surface interface implements a desktop-style window, that can be > moved, resized, maximized, etc. It provides a request for creating > child/parent relationship, called xdg_surface.set_transient_for. > > The xdg_popup interface implements a desktop-style popup/menu. A > xdg_popup is always transient for another surface, and also has implicit > grab. I think we can land this as is, though there are changes that I'd like to see. We'll get back to them after we land the series but what I'm thinking is: - I don't like ping/pong on popup surfaces, and in fact, I think we can move it to xdg_shell, since it's really a per-client thing, not a per-surface thing. - set_transient_for is not a great name. I know it comes from ICCCM, but I wonder if we can find a better name. If it's just stacking, then maybe just set_aboe? - protocol for server-initiated move/resize (eg mod+drag or kb resize?), probably similar to what we do for server side requestsed maximize. - not sure configure needs the edges flag anymore, in the cases where the server requests a change to the surface size, the server typically also knows where the window should go. I think the flag dates back to before the configure callback and the shell didn't have a way to intercept the attach. We relied on the client passing the right dx/dy, but now the shell gets the configure callback and can keep the right and top edges in place in case we're resizing from bottom+left. This should also fix xwayland resizing from top or left side. - we now have set/unset/request_set/request_unset for fullscreen and maximize, and we'll add minimized, probably tiled_left/right. This is going to explode into a large number of requests and events all with the same structure and flow as the maximized flow we've discussied this week. We should consider a more generic approach: - set(MAXIMIZED), unset(MAXIMIZED) requests, where maximized is an enum, that is, we don't allow set(MAXIMIZED | FULLSCREEN), but since it's all atomic under wl_surface.commit, set(MAXIMIZED)+set(FULLSCREEN) will have the intended effect (ignore that MAXIMIZED and FULLSCREEN doesn't make sense). - requet_set(MAXIMIZED) and request_unset(MAXIMIZED) to let the server intiate the transition. Kristian > --- > protocol/Makefile.am | 3 +- > protocol/xdg-shell.xml | 438 > + > 2 files changed, 440 insertions(+), 1 deletion(-) > create mode 100644 protocol/xdg-shell.xml > > diff --git a/protocol/Makefile.am b/protocol/Makefile.am > index 14a4b5a..33e561c 100644 > --- a/protocol/Makefile.am > +++ b/protocol/Makefile.am > @@ -7,7 +7,8 @@ protocol_sources =\ > input-method.xml\ > workspaces.xml \ > text-cursor-position.xml\ > - wayland-test.xml > + wayland-test.xml\ > + xdg-shell.xml > > if HAVE_XMLLINT > .PHONY: validate > diff --git a/protocol/xdg-shell.xml b/protocol/xdg-shell.xml > new file mode 100644 > index 000..4e5cff8 > --- /dev/null > +++ b/protocol/xdg-shell.xml > @@ -0,0 +1,438 @@ > + > + > + > + > +Copyright © 2008-2013 Kristian Høgsberg > +Copyright © 2013 Rafael Antognolli > +Copyright © 2013 Jasper St. Pierre > +Copyright © 2010-2013 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 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,
Re: [PATCH 00/16] xdg-shell protocol, implementation and port of toytoolkit.
On Wed, Nov 27, 2013 at 03:50:16PM -0200, Rafael Antognolli wrote: > As discussed previously, this patch series adds the xdg-shell protocol, its > implementation on weston, and the client implementation on toytoolkit. Hi Raphael, I think we want to get this in early and then iterate on it in-tree instead of kicking this big patch set around on the list for too long. Overall it looks good, there are just a few things that I'd like to fix up before we land the patches. I'll reply and review to the individual patches. Also, I'll try to land Philip Withnalls series from Nov 25, since it looks like a nice clean up and ends up fixing a bug (always lovely). It does conflict quite a bit with your shell.c patches, but it shouldn't be too hard to rebase or just redo them on top. Kristian > Jasper St. Pierre (5): > westoy: Schedule a full resize when we add a subsurface > westoy: Use subsurfaces for tooltips instead of transient windows > westoy: Remove fullscreen methods > westoy: Remove MAXIMIZED and FULLSCREEN as separate window types > westoy: Port the toy toolkit over to xdg-shell > > Rafael Antognolli (11): > xdg_shell: Adding a new shell protocol. > shell: Remove SHELL_SURFACE_FULLSCREEN and SHELL_SURFACE_MAXIMIZED. > shell: Change set_maximized to receive internal structures. > xdg-shell: Implement part of the interface. > xdg-shell: Implementing maximized and fullscreen states. > xdg-shell: Implement set_transient_for request. > xdg-shell: Implement xdg_popup. > xdg-shell: Add surface size to configure. > xdg-shell: xdg_surface_set_output should only affect fullscreen. > xdg-shell: Use xdg-shell in simple-shm. > xdg-shell: Add key bindings for setting maximized and fullscreen. > > clients/.gitignore | 2 + > clients/Makefile.am| 12 +- > clients/fullscreen.c | 19 +- > clients/simple-shm.c | 50 +-- > clients/transformed.c | 21 +- > clients/window.c | 418 - > clients/window.h | 3 - > protocol/Makefile.am | 3 +- > protocol/xdg-shell.xml | 438 ++ > src/.gitignore | 2 + > src/Makefile.am| 6 +- > src/shell.c| 829 > + > 12 files changed, 1387 insertions(+), 416 deletions(-) > create mode 100644 protocol/xdg-shell.xml > > -- > 1.8.3.1 > > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Simple EGL Clients on latest Weston w/ Raspberry Pi backend - 'wayland-egl' package not found?
On 11/30/13, 3:03 AM, Tomeu Vizoso wrote: > On 29 November 2013 15:49, Nicholas Levin wrote: > >> configure: error: Package requirements (wayland-client wayland-egl >> wayland-cursor) were not met: >> >> No package 'wayland-egl' found >> >> Consider adjusting the PKG_CONFIG_PATH environment variable if you >> installed software in a non-standard prefix. > > Hi! > > IIRC, userland.git will install by default to /opt/vc, so that .pc > file will be in /opt/vc/lib/pkgconfig/. So you would need to add that > path to PKG_CONFIG_PATH. > > Once it gets merged and distros start packaging it, I hope it to be > found in a more normal location, so no messing around with paths is > needed. > > Good luck, > > Tomeu > Thank you, that is all I needed. All the demos look great, and the --enable-wayland-compositor flag seems to work without a hitch, too. Exciting times ahead. Really appreciate it, Nicholas Levin ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Simple EGL Clients on latest Weston w/ Raspberry Pi backend - 'wayland-egl' package not found?
On 29 November 2013 15:49, Nicholas Levin wrote: > configure: error: Package requirements (wayland-client wayland-egl > wayland-cursor) were not met: > > No package 'wayland-egl' found > > Consider adjusting the PKG_CONFIG_PATH environment variable if you > installed software in a non-standard prefix. Hi! IIRC, userland.git will install by default to /opt/vc, so that .pc file will be in /opt/vc/lib/pkgconfig/. So you would need to add that path to PKG_CONFIG_PATH. Once it gets merged and distros start packaging it, I hope it to be found in a more normal location, so no messing around with paths is needed. Good luck, Tomeu ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 1/2] os: use posix_fallocate in creating sharable buffers
From: Pekka Paalanen
If posix_fallocate is available, use it instead of ftruncate. Unlike
ftruncate, when posix_fallocate succeeds, it guarantees that you cannot
run out of disk space, when later writing to the mmap()'ed file.
With posix_fallocate, if os_create_anonymous_file() succeeds, the
program cannot get a SIGBUS later from accessing this file via mmap. If
there is insufficient disk space, the function fails and errno is set to
ENOSPC.
This is useful on systems, that limit the available buffer space by
having XDG_RUNTIME_DIR on a small tmpfs.
Signed-off-by: Pekka Paalanen
---
configure.ac | 2 +-
shared/os-compatibility.c | 19 ++-
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/configure.ac b/configure.ac
index a460d3b..362bce0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -53,7 +53,7 @@ AC_CHECK_DECL(CLOCK_MONOTONIC,[],
[[#include ]])
AC_CHECK_HEADERS([execinfo.h])
-AC_CHECK_FUNCS([mkostemp strchrnul initgroups])
+AC_CHECK_FUNCS([mkostemp strchrnul initgroups posix_fallocate])
COMPOSITOR_MODULES="wayland-server >= 1.3.90 pixman-1"
diff --git a/shared/os-compatibility.c b/shared/os-compatibility.c
index 4f96dd4..611e7c8 100644
--- a/shared/os-compatibility.c
+++ b/shared/os-compatibility.c
@@ -132,6 +132,12 @@ create_tmpfile_cloexec(char *tmpname)
* The file is suitable for buffer sharing between processes by
* transmitting the file descriptor over Unix sockets using the
* SCM_RIGHTS methods.
+ *
+ * If the C library implements posix_fallocate(), it is used to
+ * guarantee that disk space is available for the file at the
+ * given size. If disk space is insufficent, errno is set to ENOSPC.
+ * If posix_fallocate() is not supported, program may receive
+ * SIGBUS on accessing mmap()'ed file contents instead.
*/
int
os_create_anonymous_file(off_t size)
@@ -140,6 +146,7 @@ os_create_anonymous_file(off_t size)
const char *path;
char *name;
int fd;
+ int ret;
path = getenv("XDG_RUNTIME_DIR");
if (!path) {
@@ -161,10 +168,20 @@ os_create_anonymous_file(off_t size)
if (fd < 0)
return -1;
- if (ftruncate(fd, size) < 0) {
+#ifdef HAVE_POSIX_FALLOCATE
+ ret = posix_fallocate(fd, 0, size);
+ if (ret != 0) {
close(fd);
+ errno = ret;
return -1;
}
+#else
+ ret = ftruncate(fd, size);
+ if (ret < 0) {
+ close(fd);
+ return -1;
+ }
+#endif
return fd;
}
--
1.8.1.5
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 2/2] window: handle insufficient buffer space
From: Pekka Paalanen
It is quite possible for os_create_anonymous_file() to fail when trying
to allocate a new wl_shm buffer. Propagate this failure out from
shm_surface_prepare. Most parts of toytoolkit are already avoiding NULL
cairo surfaces.
If cairo surface allocation fails, do not try to call the widget redraw
functions, those are not prepared to deal with NULL. Also do not
schedule a frame callback, this allows us to retry drawing the next
time.
If redraw fails for the main_surface of a window, restore the widget
geometry to what the compositor currently is showing. This keeps the
window visual appearance in sync with application state, so interacting
with the application does not break too badly.
If the very first draw of any window fails, then forcefully exit the
program. E.g. if weston-desktop-shell fails to allocate buffers for the
unlock dialog, w-d-s exits, and weston unlocks the screen automatically.
This patch allows e.g. weston-terminal to stop from enlarging while
resizing, if new sized buffers can no longer the allocated. Even then,
the application stays usable, as it can often repaint in the last
successful size. It does not crash, and the user is able to resize it
smaller, too.
Signed-off-by: Pekka Paalanen
---
clients/window.c | 93 +---
1 file changed, 76 insertions(+), 17 deletions(-)
diff --git a/clients/window.c b/clients/window.c
index a201ebb..0e40ab4 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ -1083,6 +1083,9 @@ shm_surface_prepare(struct toysurface *base, int dx, int
dy,
surface->flags,
leaf->resize_pool,
&leaf->data);
+ if (!leaf->cairo_surface)
+ return NULL;
+
wl_buffer_add_listener(leaf->data->buffer,
&shm_surface_buffer_listener, surface);
@@ -3724,21 +3727,10 @@ hack_prevent_EGL_sub_surface_deadlock(struct window
*window)
}
static void
-idle_resize(struct window *window)
+window_do_resize(struct window *window)
{
struct surface *surface;
- window->resize_needed = 0;
- window->redraw_needed = 1;
-
- DBG("from %dx%d to %dx%d\n",
- window->main_surface->server_allocation.width,
- window->main_surface->server_allocation.height,
- window->pending_allocation.width,
- window->pending_allocation.height);
-
- hack_prevent_EGL_sub_surface_deadlock(window);
-
widget_set_allocation(window->main_surface->widget,
window->pending_allocation.x,
window->pending_allocation.y,
@@ -3761,6 +3753,46 @@ idle_resize(struct window *window)
}
}
+static void
+idle_resize(struct window *window)
+{
+ window->resize_needed = 0;
+ window->redraw_needed = 1;
+
+ DBG("from %dx%d to %dx%d\n",
+ window->main_surface->server_allocation.width,
+ window->main_surface->server_allocation.height,
+ window->pending_allocation.width,
+ window->pending_allocation.height);
+
+ hack_prevent_EGL_sub_surface_deadlock(window);
+
+ window_do_resize(window);
+}
+
+static void
+undo_resize(struct window *window)
+{
+ window->pending_allocation.width =
+ window->main_surface->server_allocation.width;
+ window->pending_allocation.height =
+ window->main_surface->server_allocation.height;
+
+ DBG("back to %dx%d\n",
+ window->main_surface->server_allocation.width,
+ window->main_surface->server_allocation.height);
+
+ window_do_resize(window);
+
+ if (window->pending_allocation.width == 0 &&
+ window->pending_allocation.height == 0) {
+ fprintf(stderr, "Error: Could not draw a surface, "
+ "most likely due to insufficient disk space in "
+ "%s (XDG_RUNTIME_DIR).\n", getenv("XDG_RUNTIME_DIR"));
+ exit(EXIT_FAILURE);
+ }
+}
+
void
window_schedule_resize(struct window *window, int width, int height)
{
@@ -3885,25 +3917,30 @@ static const struct wl_callback_listener listener = {
frame_callback
};
-static void
+static int
surface_redraw(struct surface *surface)
{
DBG_OBJ(surface->surface, "begin\n");
if (!surface->window->redraw_needed && !surface->redraw_needed)
- return;
+ return 0;
/* Whole-window redraw forces a redraw even if the previous has
* not yet hit the screen.
*/
if (surface->frame_cb) {
if (!surface->window->redraw_needed)
- return;
+ return 0;
DBG_OBJ(surface->frame_cb, "cancelled\n");
wl_callback_destroy(surface->frame_cb);
}
+ if (!widget_get_cairo_surface(surface
[PATCH wayland 2/2] cursor: handle running out of buffer space
From: Pekka Paalanen
If posix_fallocate is available, use it to detect when we are running
out of buffer space.
Propagate the failure properly through the various functions, stopping
loading cursors but keeping the cursors that were already successfully
loaded.
This may result in an animated cursor not having all of its images, or a
cursor theme not having all of its cursors. When that happens, the
failure is NOT communicated to the application. Instead, the application
will get NULL from wl_cursor_theme_get_cursor() for a cursor that was
not loaded successfully. If an animated cursor is missing only some
images, the animation is truncated but the cursor is still available.
This patch relies on the commit "os: use posix_fallocate in creating
sharable buffers" for defining HAVE_POSIX_FALLOCATE.
Signed-off-by: Pekka Paalanen
---
cursor/wayland-cursor.c | 67 +
1 file changed, 51 insertions(+), 16 deletions(-)
diff --git a/cursor/wayland-cursor.c b/cursor/wayland-cursor.c
index 25e51c2..3dadbdd 100644
--- a/cursor/wayland-cursor.c
+++ b/cursor/wayland-cursor.c
@@ -20,6 +20,7 @@
* OF THIS SOFTWARE.
*/
+#include "config.h"
#include "xcursor.h"
#include "wayland-cursor.h"
#include "wayland-client.h"
@@ -28,6 +29,8 @@
#include
#include
#include
+#include
+#include
#include "os-compatibility.h"
@@ -79,6 +82,12 @@ shm_pool_resize(struct shm_pool *pool, int size)
if (ftruncate(pool->fd, size) < 0)
return 0;
+#ifdef HAVE_POSIX_FALLOCATE
+ errno = posix_fallocate(pool->fd, 0, size);
+ if (errno != 0)
+ return 0;
+#endif
+
wl_shm_pool_resize(pool->pool, size);
munmap(pool->data, pool->size);
@@ -199,21 +208,15 @@ wl_cursor_create_from_data(struct cursor_metadata
*metadata,
cursor->cursor.image_count = 1;
cursor->cursor.images = malloc(sizeof *cursor->cursor.images);
- if (!cursor->cursor.images) {
- free(cursor);
- return NULL;
- }
+ if (!cursor->cursor.images)
+ goto err_free_cursor;
cursor->cursor.name = strdup(metadata->name);
cursor->total_delay = 0;
image = malloc(sizeof *image);
- if (!image) {
- free(cursor->cursor.name);
- free(cursor->cursor.images);
- free(cursor);
- return NULL;
- }
+ if (!image)
+ goto err_free_images;
cursor->cursor.images[0] = (struct wl_cursor_image *) image;
image->theme = theme;
@@ -226,10 +229,25 @@ wl_cursor_create_from_data(struct cursor_metadata
*metadata,
size = metadata->width * metadata->height * sizeof(uint32_t);
image->offset = shm_pool_allocate(theme->pool, size);
+
+ if (image->offset < 0)
+ goto err_free_image;
+
memcpy(theme->pool->data + image->offset,
cursor_data + metadata->offset, size);
return &cursor->cursor;
+
+err_free_image:
+ free(image);
+
+err_free_images:
+ free(cursor->cursor.name);
+ free(cursor->cursor.images);
+
+err_free_cursor:
+ free(cursor);
+ return NULL;
}
static void
@@ -240,12 +258,17 @@ load_default_theme(struct wl_cursor_theme *theme)
free(theme->name);
theme->name = strdup("default");
- theme->cursor_count = ARRAY_LENGTH(cursor_metadata);;
+ theme->cursor_count = ARRAY_LENGTH(cursor_metadata);
theme->cursors = malloc(theme->cursor_count * sizeof(*theme->cursors));
- for (i = 0; i < theme->cursor_count; ++i)
+ for (i = 0; i < theme->cursor_count; ++i) {
theme->cursors[i] =
wl_cursor_create_from_data(&cursor_metadata[i], theme);
+
+ if (theme->cursors[i] == NULL)
+ break;
+ }
+ theme->cursor_count = i;
}
static struct wl_cursor *
@@ -260,7 +283,6 @@ wl_cursor_create_from_xcursor_images(XcursorImages *images,
if (!cursor)
return NULL;
- cursor->cursor.image_count = images->nimage;
cursor->cursor.images =
malloc(images->nimage * sizeof cursor->cursor.images[0]);
if (!cursor->cursor.images) {
@@ -273,7 +295,6 @@ wl_cursor_create_from_xcursor_images(XcursorImages *images,
for (i = 0; i < images->nimage; i++) {
image = malloc(sizeof *image);
- cursor->cursor.images[i] = (struct wl_cursor_image *) image;
image->theme = theme;
image->buffer = NULL;
@@ -283,13 +304,27 @@ wl_cursor_create_from_xcursor_images(XcursorImages
*images,
image->image.hotspot_x = images->images[i]->xhot;
image->image.hotspot_y = images->images[i]->yhot;
image->image.delay = images->images[i]->delay;
- cursor->total_delay += image->image.delay;
- /* copy pixels to shm
[PATCH wayland 1/2] os: use posix_fallocate in creating sharable buffers
From: Pekka Paalanen
If posix_fallocate is available, use it instead of ftruncate. Unlike
ftruncate, when posix_fallocate succeeds, it guarantees that you cannot
run out of disk space, when later writing to the mmap()'ed file.
With posix_fallocate, if os_create_anonymous_file() succeeds, the
program cannot get a SIGBUS later from accessing this file via mmap. If
there is insufficient disk space, the function fails and errno is set to
ENOSPC.
This is useful on systems, that limit the available buffer space by
having XDG_RUNTIME_DIR on a small tmpfs.
Signed-off-by: Pekka Paalanen
---
configure.ac | 2 +-
cursor/os-compatibility.c | 19 ++-
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/configure.ac b/configure.ac
index f8be456..b289567 100644
--- a/configure.ac
+++ b/configure.ac
@@ -39,7 +39,7 @@ if test "x$GCC" = "xyes"; then
fi
AC_SUBST(GCC_CFLAGS)
-AC_CHECK_FUNCS([accept4 mkostemp])
+AC_CHECK_FUNCS([accept4 mkostemp posix_fallocate])
AC_CHECK_DECL(SFD_CLOEXEC,[],
[AC_MSG_ERROR("SFD_CLOEXEC is needed to compile wayland")],
diff --git a/cursor/os-compatibility.c b/cursor/os-compatibility.c
index 418b0d3..0c41242 100644
--- a/cursor/os-compatibility.c
+++ b/cursor/os-compatibility.c
@@ -90,6 +90,12 @@ create_tmpfile_cloexec(char *tmpname)
* The file is suitable for buffer sharing between processes by
* transmitting the file descriptor over Unix sockets using the
* SCM_RIGHTS methods.
+ *
+ * If the C library implements posix_fallocate(), it is used to
+ * guarantee that disk space is available for the file at the
+ * given size. If disk space is insufficent, errno is set to ENOSPC.
+ * If posix_fallocate() is not supported, program may receive
+ * SIGBUS on accessing mmap()'ed file contents instead.
*/
int
os_create_anonymous_file(off_t size)
@@ -98,6 +104,7 @@ os_create_anonymous_file(off_t size)
const char *path;
char *name;
int fd;
+ int ret;
path = getenv("XDG_RUNTIME_DIR");
if (!path) {
@@ -119,10 +126,20 @@ os_create_anonymous_file(off_t size)
if (fd < 0)
return -1;
- if (ftruncate(fd, size) < 0) {
+#ifdef HAVE_POSIX_FALLOCATE
+ ret = posix_fallocate(fd, 0, size);
+ if (ret != 0) {
+ close(fd);
+ errno = ret;
+ return -1;
+ }
+#else
+ ret = ftruncate(fd, size);
+ if (ret < 0) {
close(fd);
return -1;
}
+#endif
return fd;
}
--
1.8.1.5
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Fortifying demo apps against running out of disk space
Hi, I have been looking at RaspberryPi and especially how small the filesystem for XDG_RUNTIME_DIR is. Well, Raspbian does not provide XDG_RUNTIME_DIR, so you define that yourself. In any case, it is very small, I am using /run/shm which is somewhere around 25 MB. Weston demos and also libwayland-cursor allocate all wl_shm buffers from XDG_RUNTIME_DIR, therefore the free space on that file system is important. If space on that file system runs out, ftruncate will not catch it. Instead, apps will get a SIGBUS while trying to draw into the buffer. I am proposing to use posix_fallocate() to ensure at wl_shm_pool allocation time that the space is actually reserved for the pool, and SIGBUS cannot happen. Furthermore, I have fortified toytoolkit to deal gracefully with wl_shm buffer allocation failures. Now you can e.g. resize weston-terminal bigger and bigger until you run out of space. When you run out of space, resizing stops, but the app stays usable. More details are in the patch. No more randomly disappearing demo apps on the RPi. ;-) (At least for this reason.) I will reply to this email with a couple of patches to wayland and weston each. Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Simple EGL Clients on latest Weston w/ Raspberry Pi backend - 'wayland-egl' package not found?
Hello everyone, I am attempting to get the simple EGL clients up and running on Weston for Raspberry Pi, based on the latest code in the Git repo for the Wayland libraries and Weston. With the generous guidance of Pekka Paalanen, I was able to get Weston to build with --enable-egl and a special fork of the Broadcom userland drivers ( via https://github.com/tomeuv/userland/tree/wayland ). I largely followed the directions to install Weston on the Raspberry Pi ( http://wayland.freedesktop.org/raspberrypi.html ), with the major exceptions of replacing --disable-egl with --enable-egl, and setting $WLD to "/usr/local". Other bits included adding flags for linking libraries and installing a few extra Raspbian packages, which I will not cover for the time being. With this build, I've been able to launch Weston as the root user on a stock Raspbian image. Everything seems to work fine, as long as I'm the root user. Unfortunately, my luck runs out when I attempt to run this build with --enable-simple-egl-clients. Here, the configuration stage for Weston fails with the following output; [...] checking for COMPOSITOR... yes checking for WAYLAND_COMPOSITOR... no configure: error: Package requirements (wayland-client wayland-egl wayland-cursor) were not met: No package 'wayland-egl' found Consider adjusting the PKG_CONFIG_PATH environment variable if you installed software in a non-standard prefix. Alternatively, you may set the environment variables WAYLAND_COMPOSITOR_CFLAGS and WAYLAND_COMPOSITOR_LIBS to avoid the need to call pkg-config. See the pkg-config man page for more details. To clarify, I have PKG_CONFIG_PATH set to "/usr/local/lib/pkgconfig/:/usr/local/share/pkgconfig/". /usr/local/share/pkgconfig has bcm_host.pc, egl.pc, glesv2.pc and wayland-scanner.pc . /usr/local/lib/pkgconfig has wayland-client.pc, wayland-cursor.pc, wayland-server.pc and weston.pc, among other packages that aren't related to wayland/weston. weston.pc came from from the previous build with --enable-egl and --disable-simple-egl-clients set. The packages that didn't relate to wayland/weston are glfw3.pc and libevdev.pc, both libraries that I have installed in the past. How might I go about building wayland-egl.pc on the Raspberry Pi? Thank you for your time. I really do appreciate it. Kind regards, Nicholas Levin ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH/RFC] Scanner for tests
Thanks! I'll look at that :) Marek Ch On 29 November 2013 15:36, Pekka Paalanen wrote: > On Fri, 29 Nov 2013 14:52:30 +0100 > Marek Ch wrote: > > > > > Hi! > > > > I was looking into wayland's code and I saw static wl_buffer_* > > functions and I thought: These would use a test.. But how to do it > > when they are static? I can take the code and copy it into test > > manually, but that is not good. The code can change.. > > > > I solved it by writting simple scanner that is invoked, just like > > the wayland-scanner, when compiling the code. It searches for > > definitions marked by WL_EXPORT_TEST and if it finds one, it > > copies it into tests-private.c (.h, respectively). > > Test can then be linked together with this file, so it is > > guaranteed, that the test will use the code from current HEAD. > > > > The adventage is that in order to test static function there's > > no need to delete the static keyword and thus make it public. > > The disadvantage is that if the to-be-tested function uses > > any other static function, it must be exported too. > > > > To sum it up: this is more like experiment but since it worked > > for me, I'd like to ask you for your comments. > > > > P.S. > > The first patch included is not important for the scanner but > > I wrote the code having it applied so I attached it too > > (it's a patch a sent to devel-list some time ago). > > The third patch is simple example test of wl_buffer_put. > > Hi, > > instead of writing a C language parser to copy stuff around, how about > doing what was done with Weston matrix code? See: > > http://cgit.freedesktop.org/wayland/weston/tree/tests/Makefile.am > http://cgit.freedesktop.org/wayland/weston/tree/shared/matrix.c > http://cgit.freedesktop.org/wayland/weston/tree/tests/matrix-test.c > > and note UNIT_TEST and MATRIX_TEST_EXPORT that give the test code > access to otherwise static functions. > > > Thanks, > pq > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH/RFC] Scanner for tests
On Fri, 29 Nov 2013 14:52:30 +0100 Marek Ch wrote: > > Hi! > > I was looking into wayland's code and I saw static wl_buffer_* > functions and I thought: These would use a test.. But how to do it > when they are static? I can take the code and copy it into test > manually, but that is not good. The code can change.. > > I solved it by writting simple scanner that is invoked, just like > the wayland-scanner, when compiling the code. It searches for > definitions marked by WL_EXPORT_TEST and if it finds one, it > copies it into tests-private.c (.h, respectively). > Test can then be linked together with this file, so it is > guaranteed, that the test will use the code from current HEAD. > > The adventage is that in order to test static function there's > no need to delete the static keyword and thus make it public. > The disadvantage is that if the to-be-tested function uses > any other static function, it must be exported too. > > To sum it up: this is more like experiment but since it worked > for me, I'd like to ask you for your comments. > > P.S. > The first patch included is not important for the scanner but > I wrote the code having it applied so I attached it too > (it's a patch a sent to devel-list some time ago). > The third patch is simple example test of wl_buffer_put. Hi, instead of writing a C language parser to copy stuff around, how about doing what was done with Weston matrix code? See: http://cgit.freedesktop.org/wayland/weston/tree/tests/Makefile.am http://cgit.freedesktop.org/wayland/weston/tree/shared/matrix.c http://cgit.freedesktop.org/wayland/weston/tree/tests/matrix-test.c and note UNIT_TEST and MATRIX_TEST_EXPORT that give the test code access to otherwise static functions. Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH/RFC] Scanner for tests
Hi! I was looking into wayland's code and I saw static wl_buffer_* functions and I thought: These would use a test.. But how to do it when they are static? I can take the code and copy it into test manually, but that is not good. The code can change.. I solved it by writting simple scanner that is invoked, just like the wayland-scanner, when compiling the code. It searches for definitions marked by WL_EXPORT_TEST and if it finds one, it copies it into tests-private.c (.h, respectively). Test can then be linked together with this file, so it is guaranteed, that the test will use the code from current HEAD. The adventage is that in order to test static function there's no need to delete the static keyword and thus make it public. The disadvantage is that if the to-be-tested function uses any other static function, it must be exported too. To sum it up: this is more like experiment but since it worked for me, I'd like to ask you for your comments. P.S. The first patch included is not important for the scanner but I wrote the code having it applied so I attached it too (it's a patch a sent to devel-list some time ago). The third patch is simple example test of wl_buffer_put. Cheers Marek Ch ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH 2/3] tests: scanner
Scanner is program that crawles through given files and
copies out every definition/declaration marked by WL_EXPORT_TEST.
It saves these definitions into test-runner/tests-private.[ch]
so that these definition can be tested. Using the scanner guarantee testing of
current code of functions (contrary to manual copying from source file)
---
src/wayland-util.h| 3 +
tests/test-runner/Makefile.am | 15 ++
tests/test-runner/scanner-tests.c | 349 ++
3 files changed, 367 insertions(+)
create mode 100644 tests/test-runner/scanner-tests.c
diff --git a/src/wayland-util.h b/src/wayland-util.h
index 68d91e2..5df0a54 100644
--- a/src/wayland-util.h
+++ b/src/wayland-util.h
@@ -44,6 +44,9 @@ extern "C" {
#define WL_EXPORT
#endif
+/* export function for tests */
+#define WL_EXPORT_TEST
+
/* Deprecated attribute */
#if defined(__GNUC__) && __GNUC__ >= 4
#define WL_DEPRECATED __attribute__ ((deprecated))
diff --git a/tests/test-runner/Makefile.am b/tests/test-runner/Makefile.am
index b8f1e76..bd7c292 100644
--- a/tests/test-runner/Makefile.am
+++ b/tests/test-runner/Makefile.am
@@ -9,5 +9,20 @@ libtest_runner_la_SOURCES =\
libtest_helpers_la_SOURCES = \
test-helpers.c
+scanner_input_files = \
+ scanner-tests.c
+
+noinst_PROGRAMS = scanner-tests
+scanner_tests_SOURCES = scanner-tests.c
+
+$(BUILT_SOURCES) : scanner-tests $(scanner_input_files)
+ $(AM_V_GEN)./scanner-tests $(scanner_input_files)
+
+BUILT_SOURCES = \
+ tests-private.c \
+ tests-private.h
+
AM_CPPFLAGS = -I$(top_builddir)/src -I$(top_srcdir)/src
AM_CFLAGS = $(GCC_CFLAGS)
+
+CLEANFILES = $(BUILT_SOURCES)
diff --git a/tests/test-runner/scanner-tests.c
b/tests/test-runner/scanner-tests.c
new file mode 100644
index 000..427e47e
--- /dev/null
+++ b/tests/test-runner/scanner-tests.c
@@ -0,0 +1,349 @@
+/*
+ * Copyright © 2013 Red Hat, Inc.
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * 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.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/* In order to make static functions testable too */
+#define WL_EXPORT_TEST
+
+WL_EXPORT_TEST
+const char *output = "tests-private.c";
+WL_EXPORT_TEST
+const char *output_header = "tests-private.h";
+
+WL_EXPORT_TEST
+static void
+extract_preprocessor(FILE *in, FILE *outc, FILE *outh)
+{
+ char name[8];
+ int chr, pchr = 0;
+
+ /* # was read, copy only directive */
+ fscanf(in, "%7s", name);
+
+ if (strcmp(name, "include") == 0) {
+ fprintf(outc, "#include ");
+ fprintf(outh, "#include ");
+ while ((chr = getc(in)) != EOF && chr !='\n') {
+ putc(chr, outc);
+ putc(chr, outh);
+ }
+ } else if (strcmp(name, "define") == 0) {
+ fprintf(outc, "\n#define ");
+ fprintf(outh, "\n#define ");
+ while ((chr = getc(in)) != EOF) {
+ if (chr == '\n' && pchr != '\\')
+ break;
+
+ putc(chr, outc);
+ putc(chr, outh);
+ pchr = chr;
+ }
+ }
+}
+
+WL_EXPORT_TEST
+static bool
+is_test_export(FILE *f)
+{
+ int chr;
+ unsigned i;
+ const char pattern[] = "WL_EXPORT_TEST";
+
+ /* W is already captured */
+ i = 1;
+
+ while ((chr = getc(f)) != EOF && i < strlen(pattern)) {
+ if (chr != pattern[i++])
+ return false;
+ }
+
+ /* did we go through the entire pattern? */
+ if (i != strlen(pattern))
+ return false;
+
+ return true;
+}
+
+WL_EXPORT_TEST
+static void *
+reallocate(void *mem, size_t *alloced)
+{
+ *alloced *= 2;
+ mem = realloc(mem, *alloced);
+ if (!mem)
+
[PATCH 1/3] tests: move test-runner into subdirectory
It's nice to keep helper sources separately from tests sources. It's better for searching files as well as for extending the test-suite. Also, there were no need to compile each test with test-*.c files so I used them to create noinst library and link the tests against it. Now we don't need to attach test-runner files to each test sources. --- configure.ac | 3 +- tests/Makefile.am| 46 - tests/test-helpers.c | 64 tests/test-runner.c | 207 --- tests/test-runner.h | 40 tests/test-runner/Makefile.am| 13 +++ tests/test-runner/test-helpers.c | 64 tests/test-runner/test-runner.c | 207 +++ tests/test-runner/test-runner.h | 40 9 files changed, 350 insertions(+), 334 deletions(-) delete mode 100644 tests/test-helpers.c delete mode 100644 tests/test-runner.c delete mode 100644 tests/test-runner.h create mode 100644 tests/test-runner/Makefile.am create mode 100644 tests/test-runner/test-helpers.c create mode 100644 tests/test-runner/test-runner.c create mode 100644 tests/test-runner/test-runner.h diff --git a/configure.ac b/configure.ac index f8be456..09db2e9 100644 --- a/configure.ac +++ b/configure.ac @@ -146,5 +146,6 @@ AC_CONFIG_FILES([Makefile src/wayland-scanner.pc src/wayland-version.h protocol/Makefile -tests/Makefile]) +tests/Makefile +tests/test-runner/Makefile]) AC_OUTPUT diff --git a/tests/Makefile.am b/tests/Makefile.am index 9c673ae..4a999b9 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1,3 +1,6 @@ +SUBDIRS = test-runner +test_runner_dir = $(top_builddir)/tests/test-runner + TESTS =\ array-test \ client-test \ @@ -21,38 +24,37 @@ check_PROGRAMS =\ noinst_PROGRAMS = \ fixed-benchmark -test_runner_src = test-runner.c test-runner.h test-helpers.c - -array_test_SOURCES = array-test.c $(test_runner_src) -client_test_SOURCES = client-test.c $(test_runner_src) -display_test_SOURCES = display-test.c $(test_runner_src) -connection_test_SOURCES = connection-test.c $(test_runner_src) -event_loop_test_SOURCES = event-loop-test.c $(test_runner_src) -fixed_test_SOURCES = fixed-test.c $(test_runner_src) -list_test_SOURCES = list-test.c $(test_runner_src) -map_test_SOURCES = map-test.c $(test_runner_src) -sanity_test_SOURCES = sanity-test.c $(test_runner_src) -socket_test_SOURCES = socket-test.c $(test_runner_src) -queue_test_SOURCES = queue-test.c $(test_runner_src) -signal_test_SOURCES = signal-test.c $(test_runner_src) -resources_test_SOURCES = resources-test.c $(test_runner_src) +array_test_SOURCES = array-test.c +client_test_SOURCES = client-test.c +display_test_SOURCES = display-test.c +connection_test_SOURCES = connection-test.c +event_loop_test_SOURCES = event-loop-test.c +fixed_test_SOURCES = fixed-test.c +list_test_SOURCES = list-test.c +map_test_SOURCES = map-test.c +sanity_test_SOURCES = sanity-test.c +socket_test_SOURCES = socket-test.c +queue_test_SOURCES = queue-test.c +signal_test_SOURCES = signal-test.c +resources_test_SOURCES = resources-test.c fixed_benchmark_SOURCES = fixed-benchmark.c os_wrappers_test_SOURCES = \ os-wrappers-test.c \ - ../src/wayland-os.c \ - $(test_runner_src) + ../src/wayland-os.c -AM_CPPFLAGS = -I$(top_builddir)/src -I$(top_srcdir)/src +AM_CPPFLAGS = \ + -I$(top_builddir)/src \ + -I$(top_srcdir)/src \ + -I$(test_runner_dir) AM_CFLAGS = $(GCC_CFLAGS) $(FFI_CFLAGS) LDADD = $(top_builddir)/src/libwayland-util.la \ $(top_builddir)/src/libwayland-client.la \ $(top_builddir)/src/libwayland-server.la \ + $(test_runner_dir)/libtest-runner.la -lrt -ldl $(FFI_LIBS) exec_fd_leak_checker_SOURCES = \ - exec-fd-leak-checker.c \ - test-runner.h \ - test-helpers.c -exec_fd_leak_checker_LDADD = + exec-fd-leak-checker.c +exec_fd_leak_checker_LDADD = $(test_runner_dir)/libtest-helpers.la diff --git a/tests/test-helpers.c b/tests/test-helpers.c deleted file mode 100644 index 4761b09..000 --- a/tests/test-helpers.c +++ /dev/null @@ -1,64 +0,0 @@ -/* - * Copyright © 2012 Collabora, Ltd. - * - * Permission to use, copy, modify, distribute, and sell this software and its - * documentation for any purpose is hereby granted without fee, provided that - * the above copyright notice appear in all copies and that both that copyright - * notice and this permission notice appear in supp
[PATCH 3/3] tests: add wl_buffer tests
Using scanner to have current code of wl_buffer_* static function,
test their behaviour
---
src/connection.c | 3 ++
tests/Makefile.am | 5 +++-
tests/test-runner/Makefile.am | 1 +
tests/wl_buffer-test.c| 64 +++
4 files changed, 72 insertions(+), 1 deletion(-)
create mode 100644 tests/wl_buffer-test.c
diff --git a/src/connection.c b/src/connection.c
index 1d8b61b..644fb1f 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -44,6 +44,7 @@
#define DIV_ROUNDUP(n, a) ( ((n) + ((a) - 1)) / (a) )
+WL_EXPORT_TEST
struct wl_buffer {
char data[4096];
uint32_t head, tail;
@@ -61,6 +62,7 @@ struct wl_connection {
int want_flush;
};
+WL_EXPORT_TEST
static void
wl_buffer_put(struct wl_buffer *b, const void *data, size_t count)
{
@@ -141,6 +143,7 @@ wl_buffer_copy(struct wl_buffer *b, void *data, size_t
count)
}
}
+WL_EXPORT_TEST
static uint32_t
wl_buffer_size(struct wl_buffer *b)
{
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 4a999b9..6e2001c 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -1,5 +1,6 @@
SUBDIRS = test-runner
test_runner_dir = $(top_builddir)/tests/test-runner
+tests_private = $(test_runner_dir)/tests-private.c
TESTS =\
array-test \
@@ -15,7 +16,8 @@ TESTS = \
socket-test \
queue-test \
signal-test \
- resources-test
+ resources-test \
+ wl_buffer-test
check_PROGRAMS = \
$(TESTS)\
@@ -37,6 +39,7 @@ socket_test_SOURCES = socket-test.c
queue_test_SOURCES = queue-test.c
signal_test_SOURCES = signal-test.c
resources_test_SOURCES = resources-test.c
+wl_buffer_test_SOURCES = wl_buffer-test.c $(tests_private)
fixed_benchmark_SOURCES = fixed-benchmark.c
diff --git a/tests/test-runner/Makefile.am b/tests/test-runner/Makefile.am
index bd7c292..8333bb2 100644
--- a/tests/test-runner/Makefile.am
+++ b/tests/test-runner/Makefile.am
@@ -10,6 +10,7 @@ libtest_helpers_la_SOURCES = \
test-helpers.c
scanner_input_files = \
+ $(top_srcdir)/src/connection.c \
scanner-tests.c
noinst_PROGRAMS = scanner-tests
diff --git a/tests/wl_buffer-test.c b/tests/wl_buffer-test.c
new file mode 100644
index 000..39c42dd
--- /dev/null
+++ b/tests/wl_buffer-test.c
@@ -0,0 +1,64 @@
+#include "test-runner.h"
+#include "tests-private.h"
+
+/* Assert with formated output */
+#define assertf(cond, ...)
\
+ do {
\
+ if (!(cond)) {
\
+ fprintf(stderr, "%s (%s: %d): Assertion %s failed!",
\
+ __FUNCTION__, __FILE__, __LINE__,
#cond);\
+ fprintf(stderr, " " __VA_ARGS__);
\
+ putc('\n', stderr);
\
+ abort();
\
+ }
\
+ } while (0)
+
+static const char data[] = "abcdefghijklmnopqrstuvwxyz";
+
+TEST(wl_buffer_put_tst)
+{
+ struct wl_buffer b = {.head = 0, .tail = 0};
+ size_t index;
+
+ assert(wl_buffer_size(&b) == 0);
+
+ int i;
+ for (i = 1; i * (sizeof data) <= sizeof b.data; i++) {
+ wl_buffer_put(&b, data, sizeof data);
+
+ assert(b.tail == 0);
+ assertf(b.head == i * (sizeof data),
+ "head is %u instead of %lu", b.head, i * (sizeof data));
+ assert(wl_buffer_size(&b) == i * sizeof data);
+ }
+
+ /* do overflow */
+ wl_buffer_put(&b, data, sizeof data);
+
+ /* ensure */
+ assert(i * sizeof data > sizeof b.data);
+
+ assertf(MASK(b.head) < sizeof b.data);
+ assert(b.tail == 0);
+ index = sizeof b.data % sizeof data;
+ assertf(strncmp((char *) b.data + sizeof b.data - index, data, index -
1) == 0,
+ "Should have '%*s', but have '%*s'\n", (int) (index - 1), data,
+ (int) (index - 1), (char *) b.data + sizeof b.data - index);
+ assertf(strncmp(b.data, (char *) data + index, sizeof data - index) ==
0,
+ "Should have '%*s', but have '%*s'\n",
+ (int) (sizeof data - index), data + index,
+ (int) (sizeof data - index), (char *) b.data);
+
+ struct wl_buffer bb = {.head = 0, .tail = 0};
+ wl_buffer_put(&bb, data, sizeof data);
+ assert(strcmp(data, b
Re: [PATCH weston 4/8] protocol: crop & scale RFC v3
On Fri, 29 Nov 2013 01:20:21 -0800 Bill Spitzak wrote: > Okay I think perhaps I am completely failing to comprehend what is > going on. > > The client I am thinking of is not trying to do "partial pixels". > What I am thinking of is the most simple client you can imagine that > knows what the output_scale is and decides it wants to render images > at full resolution. > > If for instance the output_scale is 3, then I think the client will > set the buffer_scale to 3, and then the pixels in the client's buffer > will map 1:1 to the pixels on the output device. There are no > "partial pixels". > > If this client wants a rectangle that is covered by an image produced > by this scaling extension, it appears to me that it cannot be any > integer number of buffer pixels. It can only be multiples of 3 pixels > in both size and position. > > Please correct me if I am misunderstanding this, but I have read it > over and over and over and it sure looks like this is what the design > does! You have read it right. However, the standard units of measure are the surface coordinates. We assume that toolkits and applications set up their scenegraph in surface coordinates, and that integers are sufficient there. The application drawing engine just happens to be applying a scaling factor equal to the desired buffer_scale. After all, toolkits should be prepared to change the buffer_scale at will (window completely moves from HiDPI monitor to a low DPI one), so storing the scenegraph in buffer coordinates would be a lot of work for each change. In other words, all GUI elements are defined in integer surface coordinates. Defining GUI elements in smaller units is not useful, because people will not be able to take advantage of smaller things anyway: they cannot see it clearly, or they cannot poke it (input) accurately enough. You are not supposed to be able to position and size GUI elements in the output resolution. That is exactly the purpose of the whole HiDPI extension. Instead, you design your GUI in "pixels" as always, and the window system turns it into a usable size on an output. If you want to draw the GUI elements in more _detail_, you can, by using buffer_scale. That is to say for your example, you design your GUI in multiples of 3 output pixels in any case. - pq > On 11/27/2013 01:51 PM, Daniel Stone wrote: > > Hi, > > > > On 27 November 2013 20:08, Bill Spitzak wrote: > >> On 11/27/2013 12:34 AM, Pekka Paalanen wrote: > >>> I have explained all this before. Nothing here has changed. > >> > >> I realize this but I still have to express my complete > >> dumbfoundment that you think this is ok. > > > > You're attempting to design for the problem space where clients > > create configurations which cannot be displayed except by > > attempting to invent the concept of 'partial pixels', where a > > buffer size of 79.... is not only meaningful but a design > > goal. The opposing position is 'don't do that': clients should > > avoid getting themselves into these situations in the first place. > > > > Your proposals really come across as attempting to design for > > situations which should never occur (and can't meaningfully be dealt > > with by extant hardware), optimising for hugely misguided clients > > in a fit of completism. That's your view, which you've made very > > clear, but I don't think it's shared by anyone else in these > > threads. > > > > Cheers, > > Daniel > > > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 4/8] protocol: crop & scale RFC v3
Fractions are required so that a client can get the same scale in both directions for a destination that is not square, or to get the same scale for different crops of the destination rectangle. Also all hardware must do this if it is capable of cropping the output, since a cropped subrectangle of a scaled rectangle is equivalent to scaling from a fractional source rectangle to that crop rectangle. On 11/28/2013 05:51 PM, Daniel Stone wrote: Hi, On 26 November 2013 17:19, Jonny Lamb wrote: + + + + In the same vein as my reply to Bill, I'd really like to see these changed to int. I have little sympathy for clients which perform layout such that their buffer_scale doesn't allow them to represent their scene in integer space. I also have a very difficult time imagining that toolkits would actually perform layout like this. The more practical / less ideological objection is that this is an invitation for clients to get things very, very wrong. Using fixed suggests that partial-pixel co-ordinates are okay (e.g. src_x of 8.5 with a buffer_scale of 3); just how this is supposed to be represented accurately in hardware, I'm not quite sure. I don't see any benefit to this additional complication for both protocol and implementations; only downsides. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 4/8] protocol: crop & scale RFC v3
Okay I think perhaps I am completely failing to comprehend what is going on. The client I am thinking of is not trying to do "partial pixels". What I am thinking of is the most simple client you can imagine that knows what the output_scale is and decides it wants to render images at full resolution. If for instance the output_scale is 3, then I think the client will set the buffer_scale to 3, and then the pixels in the client's buffer will map 1:1 to the pixels on the output device. There are no "partial pixels". If this client wants a rectangle that is covered by an image produced by this scaling extension, it appears to me that it cannot be any integer number of buffer pixels. It can only be multiples of 3 pixels in both size and position. Please correct me if I am misunderstanding this, but I have read it over and over and over and it sure looks like this is what the design does! On 11/27/2013 01:51 PM, Daniel Stone wrote: Hi, On 27 November 2013 20:08, Bill Spitzak wrote: On 11/27/2013 12:34 AM, Pekka Paalanen wrote: I have explained all this before. Nothing here has changed. I realize this but I still have to express my complete dumbfoundment that you think this is ok. You're attempting to design for the problem space where clients create configurations which cannot be displayed except by attempting to invent the concept of 'partial pixels', where a buffer size of 79.... is not only meaningful but a design goal. The opposing position is 'don't do that': clients should avoid getting themselves into these situations in the first place. Your proposals really come across as attempting to design for situations which should never occur (and can't meaningfully be dealt with by extant hardware), optimising for hugely misguided clients in a fit of completism. That's your view, which you've made very clear, but I don't think it's shared by anyone else in these threads. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 4/8] protocol: crop & scale RFC v3
On Fri, 29 Nov 2013 01:51:03 + Daniel Stone wrote: > Hi, > > On 26 November 2013 17:19, Jonny Lamb > wrote: > > + > > + > > + > > + > > In the same vein as my reply to Bill, I'd really like to see these > changed to int. I have little sympathy for clients which perform > layout such that their buffer_scale doesn't allow them to represent > their scene in integer space. I also have a very difficult time > imagining that toolkits would actually perform layout like this. > > The more practical / less ideological objection is that this is an > invitation for clients to get things very, very wrong. Using fixed > suggests that partial-pixel co-ordinates are okay (e.g. src_x of 8.5 > with a buffer_scale of 3); just how this is supposed to be represented > accurately in hardware, I'm not quite sure. I don't see any benefit > to this additional complication for both protocol and implementations; > only downsides. Err... but arbitrarily fractional values are technically ok. As I explained before: On Wed, 27 Nov 2013 10:34:10 +0200 Pekka Paalanen wrote: > The values are allowed to be arbitrary, so that you can zoom in to > partial buffer pixels if you want to. They are not meant to counter > buffer_scale, which you will likely choose to set to 1 anyway when > using crop & scale and regardless of output scale. GL-renderer has no trouble realizing fractional src rect values; if I read compositor-drm right the drmModeSetPlane() API uses 16 bits for the fraction, and even rpi Dispmanx uses 16.16 fixed point. The way pixman renderer is written it should handle fractions just fine, too. If I understood anything from v4l2, it seems to leave the matter up to each driver or device. The test program included in the series deliberately has 1px wide green lines in the buffer, which are cut in half by using fractional values to test it. With a 1x1 linear texture sampling filter (pixman and gl renderers) this results in the surface edge being solid green and shifting to blue when moving towards an inner pixel, just like it should. Checking visually with xmag, the results look identical. You're right that I don't know of any real world use case where fractional src rect is needed, but since all the hardware APIs I know about have it, I guess it is useful for something. Therefore I included it. My imaginary use case is a pointer-controlled magnifier, where if the magnification is high enough, pointer motion can move the src rect in less than one buffer pixel steps. Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] src/shell.c: set black_surface->width and height
On Fri, 29 Nov 2013 11:18:14 +0800 Xiong Zhang wrote: > From: XiongZhang > > full screen black_surface doesn't have associated wl_buffer, so > black_surface->width and height can't get value through > weston_surface_commit(). then weston_surface_damage(black_surface) > will be wrong in shell_stack_fullscreen(), the result is that we > can't see a full screen view whe APP window's size isn't equal to > output's size like running weston-gears > > Signed-off-by: Xiong Zhang > --- > src/shell.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/src/shell.c b/src/shell.c > index 605f090..ebcf3e2 100644 > --- a/src/shell.c > +++ b/src/shell.c > @@ -2254,6 +2254,8 @@ create_black_surface(struct weston_compositor > *ec, pixman_region32_init_rect(&surface->opaque, 0, 0, w, h); > pixman_region32_fini(&surface->input); > pixman_region32_init_rect(&surface->input, 0, 0, w, h); > + surface->width = w; > + surface->height = h; > > weston_view_configure(view, x, y, w, h); > I wonder if the surface size should be given as arguments to weston_surface_set_color() with a possibly more descriptive function name. A function, that would imitate committing a solid color buffer to a weston_surface. OTOH, solid color surfaces do not go through the configure hook either, so I'm not sure that makes sense. Actually I once tried to clean up these things, among the attempts there is http://cgit.collabora.com/git/user/pq/weston.git/commit/?h=clipscale-wip&id=a1f3c20a0dfcbcf30e96a99928c923a7a2ab396c I never proposed that patch on the list, because the whole series was quickly going down the rabbit hole, and particularly I wasn't sure whose responsibility it is to call weston_view_geometry_dirty(). But maybe it would give some ideas. I think simply setting surface->width,height like you propose is prone to forgetting updating some dependent state. Also, IIRC jekstrand was working on removing weston_view::width,height, which means that this weston_surface::width,height issue would be fixed in the process. As a stop-gap measure, I guess your patch is fine, though. It certainly looks needed. Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
