Re: [PATCH v2 2/6] shell: register output->destroy_signal handler

2013-10-23 Thread Kristian Høgsberg
On Wed, Oct 23, 2013 at 01:58:32PM +0800, Xiong Zhang wrote:
> setup_output_destroy_handler() deal with output created at
> drm backend initialize time.
> handle_output_create() deal with output created by hot plug handler
> output_destroy_handler is removed when output was unplugged or
> shell is destroyed.
> 
> Signed-off-by: Xiong Zhang 
> ---
>  src/shell.c | 75 
> +
>  1 file changed, 75 insertions(+)

I applied this one too, with a couple of renames.  I try to avoid
editing patches as I apply to make sure we discuss all changes here on
the list.  I this case the changes are mostly renames and I'd like to
get these output unplug fixes in soon so we can do a 1.3.1 release
with them.  See below for what I changed.

I'll try to review the remaining patches tomorrow.

Kristian

> diff --git a/src/shell.c b/src/shell.c
> index f033e8c..2a9c024 100644
> --- a/src/shell.c
> +++ b/src/shell.c
> @@ -85,6 +85,13 @@ struct input_panel_surface {
>   uint32_t panel;
>  };
>  
> +struct shell_output {
> + struct desktop_shell  *shell;
> + struct weston_output  *output;
> + struct wl_listenerdestroy_listener;
> + struct wl_listlink;
> +};
> +
>  struct desktop_shell {
>   struct weston_compositor *compositor;
>  
> @@ -163,6 +170,9 @@ struct desktop_shell {
>  
>   uint32_t binding_modifier;
>   enum animation_type win_animation_type;
> +
> + struct wl_listener output_create_listener;
> + struct wl_list output_destroy_listener_list;

Since we renamed output_destroy_listener to just shell_output, I
renamed the output_destroy_listener_list to just output_list.

>  };
>  
>  enum shell_surface_type {
> @@ -4461,11 +4471,65 @@ workspace_move_surface_down_binding(struct 
> weston_seat *seat, uint32_t time,
>  }
>  
>  static void
> +handle_output_destroy(struct wl_listener *listener, void *data)
> +{
> + struct shell_output *output_listener =
> + container_of(listener, struct shell_output, destroy_listener);

And here, I renamed output_listener to shell_output.

> + wl_list_remove(&output_listener->destroy_listener.link);
> + wl_list_remove(&output_listener->link);
> + free(output_listener);
> +}
> +
> +static void
> +create_shell_output(struct desktop_shell *shell,
> + struct weston_output *output)
> +{
> + struct shell_output *output_listener;
> +
> + output_listener = malloc(sizeof(*output_listener));
> + memset(output_listener, 0, sizeof(*output_listener));

Use zalloc instead of malloc+memset.  Also we need to handle OOM here.
I changed this to just return if zalloc fails, which means that we
fail to set up the shell_output and can't detect unplug of that
output.

> + output_listener->output = output;
> + output_listener->shell = shell;
> + output_listener->destroy_listener.notify = handle_output_destroy;
> + wl_signal_add(&output->destroy_signal,
> + &output_listener->destroy_listener);
> + wl_list_insert(shell->output_destroy_listener_list.prev,
> + &output_listener->link);
> +}
> +
> +static void
> +handle_output_create(struct wl_listener *listener, void *data)
> +{
> + struct desktop_shell *shell =
> + container_of(listener, struct desktop_shell, 
> output_create_listener);
> + struct weston_output *output = (struct weston_output *)data;
> +
> + create_shell_output(shell, output);
> +}
> +
> +static void
> +setup_output_destroy_handler(struct weston_compositor *ec,
> + struct desktop_shell 
> *shell)
> +{
> + struct weston_output *output;
> +
> + wl_list_init(&shell->output_destroy_listener_list);
> + wl_list_for_each(output, &ec->output_list, link)
> + create_shell_output(shell, output);
> +
> + shell->output_create_listener.notify = handle_output_create;
> + wl_signal_add(&ec->output_created_signal,
> + &shell->output_create_listener);
> +}
> +
> +static void
>  shell_destroy(struct wl_listener *listener, void *data)
>  {
>   struct desktop_shell *shell =
>   container_of(listener, struct desktop_shell, destroy_listener);
>   struct workspace **ws;
> + struct shell_output *output_listener, *tmp;
>  
>   if (shell->child.client)
>   wl_client_destroy(shell->child.client);
> @@ -4475,6 +4539,15 @@ shell_destroy(struct wl_listener *listener, void *data)
>   wl_list_remove(&shell->show_input_panel_listener.link);
>   wl_list_remove(&shell->hide_input_panel_listener.link);
>  
> + wl_list_for_each_safe(output_listener, tmp, 
> &shell->output_destroy_listener_list,
> + link) {
> + wl_list_remove(&output_listener->destroy_listener.link);
> + wl_list_remove(&output_listener->link);
> + free(output_listener);
> + }
>

Re: [PATCH v2 5/6] window, desktop-shell: deal with output unplug on client side

2013-10-23 Thread Kristian Høgsberg
On Wed, Oct 23, 2013 at 01:58:35PM +0800, Xiong Zhang wrote:
> when output is removed, weston-desktop-shell should destroy panel
> and background surface on destroyed output.
> 
> Signed-off-by: Xiong Zhang 
> ---
>  clients/desktop-shell.c | 20 
>  clients/window.c| 32 
>  clients/window.h|  3 +++
>  3 files changed, 55 insertions(+)

This one looks good, applied.

Kristian

> diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
> index 599c0a5..1a034da 100644
> --- a/clients/desktop-shell.c
> +++ b/clients/desktop-shell.c
> @@ -95,6 +95,7 @@ struct background {
>  
>  struct output {
>   struct wl_output *output;
> + uint32_t server_output_id;
>   struct wl_list link;
>  
>   struct panel *panel;
> @@ -1213,6 +1214,7 @@ create_output(struct desktop *desktop, uint32_t id)
>  
>   output->output =
>   display_bind(desktop->display, id, &wl_output_interface, 2);
> + output->server_output_id = id;
>  
>   wl_output_add_listener(output->output, &output_listener, output);
>  
> @@ -1242,6 +1244,23 @@ global_handler(struct display *display, uint32_t id,
>  }
>  
>  static void
> +global_handler_remove(struct display *display, uint32_t id,
> +const char *interface, uint32_t version, void *data)
> +{
> + struct desktop *desktop = data;
> + struct output *output;
> +
> + if (!strcmp(interface, "wl_output")) {
> + wl_list_for_each(output, &desktop->outputs, link) {
> + if (output->server_output_id == id) {
> + output_destroy(output);
> + break;
> + }
> + }
> + }
> +}
> +
> +static void
>  panel_add_launchers(struct panel *panel, struct desktop *desktop)
>  {
>   struct weston_config_section *s;
> @@ -1298,6 +1317,7 @@ int main(int argc, char *argv[])
>  
>   display_set_user_data(desktop.display, &desktop);
>   display_set_global_handler(desktop.display, global_handler);
> + display_set_global_handler_remove(desktop.display, 
> global_handler_remove);
>  
>   /* Create panel and background for outputs processed before the shell
>* global interface was processed */
> diff --git a/clients/window.c b/clients/window.c
> index 5b20da5..b35be3c 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -117,6 +117,7 @@ struct display {
>  
>   display_output_handler_t output_configure_handler;
>   display_global_handler_t global_handler;
> + display_global_handler_t global_handler_remove;
>  
>   void *user_data;
>  
> @@ -339,6 +340,7 @@ struct input {
>  struct output {
>   struct display *display;
>   struct wl_output *output;
> + uint32_t server_output_id;
>   struct rectangle allocation;
>   struct wl_list link;
>   int transform;
> @@ -4836,6 +4838,7 @@ display_add_output(struct display *d, uint32_t id)
>   output->scale = 1;
>   output->output =
>   wl_registry_bind(d->registry, id, &wl_output_interface, 2);
> + output->server_output_id = id;
>   wl_list_insert(d->output_list.prev, &output->link);
>  
>   wl_output_add_listener(output->output, &output_listener, output);
> @@ -4852,6 +4855,19 @@ output_destroy(struct output *output)
>   free(output);
>  }
>  
> +static void
> +display_destroy_output(struct display *d, uint32_t id)
> +{
> + struct output *output;
> +
> + wl_list_for_each(output, &d->output_list, link) {
> + if (output->server_output_id == id) {
> + output_destroy(output);
> + break;
> + }
> + }
> +}
> +
>  void
>  display_set_global_handler(struct display *display,
>  display_global_handler_t handler)
> @@ -4869,6 +4885,15 @@ display_set_global_handler(struct display *display,
>  }
>  
>  void
> +display_set_global_handler_remove(struct display *display,
> +display_global_handler_t remove_handler)
> +{
> + display->global_handler_remove = remove_handler;
> + if (!remove_handler)
> + return;
> +}
> +
> +void
>  display_set_output_configure_handler(struct display *display,
>display_output_handler_t handler)
>  {
> @@ -5102,9 +5127,16 @@ registry_handle_global_remove(void *data, struct 
> wl_registry *registry,
>   if (global->name != name)
>   continue;
>  
> + if (strcmp(global->interface, "wl_output") == 0)
> + display_destroy_output(d, name);
> +
>   /* XXX: Should destroy bound globals, and call
>* the counterpart of display::global_handler
>*/

I updated this comment to reflect that we now have global remove
handler, it's now just:

/* XXX: Should destroy remaining bound globals */

> + if (d->global_handler_remove)
> + 

Re: [PATCH v2 1/6] compositor: set surface->plane from destroyed plane to NULL

2013-10-23 Thread Kristian Høgsberg
On Wed, Oct 23, 2013 at 01:58:31PM +0800, Xiong Zhang wrote:
> In drm backend, the cursor_surface->plane point to
> drm_output->cursor_plane.when this output is removed,
> drm_output->cursor_plane is destroyed, butcursor_surface->plane
> still point to destroyed plane. So once mouse move to this
> cursor_surface and system will repaint this cursor_surface,
> segment fault will generate in weston_surface_damage_below() function.
> 
> V2:
> -set surface->plane to NULL whose plane point to unplugged output,
>  then change weston_surface_damage_below() to do nothing if
>  surface->plane is NULL (Kristian)
> -set surface->plane to NULL in weston_surface_unmap(),
>  so that all surfaces that have a non-NULL plane pointer wil be
>  on compositor->surface_list (Kristian).
> 
> bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=69777
> 
> Signed-off-by: Xiong Zhang 
> ---
>  src/compositor-drm.c |  6 +++---
>  src/compositor.c | 22 +-
>  src/compositor.h |  5 -
>  3 files changed, 24 insertions(+), 9 deletions(-)

This one looks good now, thanks.  Applied to master.

Kristian

> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index ffdec89..b5c3051 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -1974,8 +1974,8 @@ create_output_for_connector(struct drm_compositor *ec,
>   output->base.gamma_size = output->original_crtc->gamma_size;
>   output->base.set_gamma = drm_output_set_gamma;
>  
> - weston_plane_init(&output->cursor_plane, 0, 0);
> - weston_plane_init(&output->fb_plane, 0, 0);
> + weston_plane_init(&output->cursor_plane, &ec->base, 0, 0);
> + weston_plane_init(&output->fb_plane, &ec->base, 0, 0);
>  
>   weston_compositor_stack_plane(&ec->base, &output->cursor_plane, NULL);
>   weston_compositor_stack_plane(&ec->base, &output->fb_plane,
> @@ -2050,7 +2050,7 @@ create_sprites(struct drm_compositor *ec)
>   memcpy(sprite->formats, plane->formats,
>  plane->count_formats * sizeof(plane->formats[0]));
>   drmModeFreePlane(plane);
> - weston_plane_init(&sprite->plane, 0, 0);
> + weston_plane_init(&sprite->plane, &ec->base, 0, 0);
>   weston_compositor_stack_plane(&ec->base, &sprite->plane,
> &ec->base.primary_plane);
>  
> diff --git a/src/compositor.c b/src/compositor.c
> index 376ddfd..9eb3d5d 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -377,7 +377,7 @@ weston_surface_create(struct weston_compositor 
> *compositor)
>   surface->pending.buffer_transform = surface->buffer_transform;
>   surface->pending.buffer_scale = surface->buffer_scale;
>   surface->output = NULL;
> - surface->plane = &compositor->primary_plane;
> + surface->plane = NULL;
>   surface->pending.newly_attached = 0;
>  
>   pixman_region32_init(&surface->damage);
> @@ -578,8 +578,9 @@ weston_surface_damage_below(struct weston_surface 
> *surface)
>   pixman_region32_init(&damage);
>   pixman_region32_subtract(&damage, &surface->transform.boundingbox,
>&surface->clip);
> - pixman_region32_union(&surface->plane->damage,
> -   &surface->plane->damage, &damage);
> + if (surface->plane)
> + pixman_region32_union(&surface->plane->damage,
> + &surface->plane->damage, &damage);
>   pixman_region32_fini(&damage);
>  }
>  
> @@ -1056,6 +1057,7 @@ weston_surface_unmap(struct weston_surface *surface)
>  
>   weston_surface_damage_below(surface);
>   surface->output = NULL;
> + surface->plane = NULL;
>   wl_list_remove(&surface->layer_link);
>   wl_list_remove(&surface->link);
>   wl_list_init(&surface->link);
> @@ -2604,12 +2606,15 @@ idle_handler(void *data)
>  }
>  
>  WL_EXPORT void
> -weston_plane_init(struct weston_plane *plane, int32_t x, int32_t y)
> +weston_plane_init(struct weston_plane *plane,
> + struct weston_compositor *ec,
> + int32_t x, int32_t y)
>  {
>   pixman_region32_init(&plane->damage);
>   pixman_region32_init(&plane->clip);
>   plane->x = x;
>   plane->y = y;
> + plane->compositor = ec;
>  
>   /* Init the link so that the call to wl_list_remove() when releasing
>* the plane without ever stacking doesn't lead to a crash */
> @@ -2619,9 +2624,16 @@ weston_plane_init(struct weston_plane *plane, int32_t 
> x, int32_t y)
>  WL_EXPORT void
>  weston_plane_release(struct weston_plane *plane)
>  {
> + struct weston_surface *surface;
> +
>   pixman_region32_fini(&plane->damage);
>   pixman_region32_fini(&plane->clip);
>  
> + wl_list_for_each(surface, &plane->compositor->surface_list, link) {
> + if (surface->plane == plane)
> + surface->plane = NULL;
> + }
> +
>   wl_list_remove(&plane->link);
>  }
>  
> @@ -3015

Re: Buffer release events (was: Add support for eglSwapInterval)

2013-10-23 Thread Jason Ekstrand
Neil,
Thanks for cracking away at this.  I've been kicking it around and I'm
wondering if we're going in the wrong direction.  I apologize if what I'm
about to say has already been thought about and discarded for some reason.
I don't remember everything that was considered.

It seems to me as if the default should always be to just send the event.
It's a "release" event after all and not sending it seems like a break of
protocol.  Instead, it seems as if we should look at delaying it as an
optimization that we only do if it makes sense.  I've kicked around a
number of different ideas for how to detect this but haven't found anything
bulletproof yet.  I'll keep kicking it around and mail again if I find
something.

One non-automatic to do this would be to have a lazy_release flag that you
can set on a surface if you don't want to be woken up.  (Note that this is
opt-in not opt-out).  Then Mesa or whoever can assume that, unless the
client has set lazy_release, it will get all its buffer release events.
The obvious downside here is that this is an extra step required for
otherwise simple clients to get an obvious optimization.

There may also be a way that we can sidestep the whole issue.  (I suggested
this to Axel Davy [mannerov] and it worked for him in his wlglamor DDX.)
We can identify buffer release events in weston as coming from one of three
sources:

1) wl_surface.commit
2) surface_flush_damage (the gl renderer releases SHM buffers here)
3) random releases from the backdend/renderer

Number 2 above happens during the redraw loop so we can just post the event
and won't get a double-wakeup.  Number 3 is something we can't really
control; I'd personally lean towards posting the event here, but it's
probably at most one reference per surface so we can probably get away with
queueing.  (Also, if the backend knows when it would release in the render
cycle, it may be able to optimize this better than some compositor-general
solution.)  For these two, we can add an argument to
weston_buffer_reference to set the release event type.

Number 1 above is the source of the vast majority of out release events.
This includes all buffers in the pixman renderer, EGL buffers for the gl
renderer, and any time you commit a buffer before the previous one gets a
chance to hit the screen.  The good news is that we can, from a client
perspective, deal with this one easily.  The solution is to send a
wl_display.sync request immediately after the commit.  This will force any
queued wl_buffer.release events to get sent out immediately.  The client
can even destroy the returned proxy immediately and completely ignore the
resulting event.  This seems a bit hackish, but this is exactly the kind of
thing the sync request is intended for: flushing the stream.

In any case, dummy sync and frame requests (you may need both) will allow
you to achieve glSwapInterval(0) without server-side support.  If you
decide to go ahead with the proposed interface, you can use this stunt in
the case where the wl_surface version < 4.

Hope this is useful.
--Jason Ekstrand


On Wed, Oct 23, 2013 at 2:01 PM, Neil Roberts  wrote:

> Neil Roberts  writes:
>
> > Currently the only proposed complete solution is just to add a request
> > to explicitly disable the queuing mechanism.
>
> I started writing this patch but I've hit a stumbling block while trying
> to make use of it in Mesa. Adding the new request requires altering the
> version of the wl_compositor and wl_surface interfaces. The problem is
> that Mesa can't really use the new request unless the wl_surface object
> is using the new version. However, the version of the interface that is
> actually in use is determined by the application when it binds the
> compositor object. As far as I can tell, there is no way for Mesa to
> determine what version the surface proxy object is using. Potentially we
> could add some API to query this, but I think even that wouldn't be
> ideal because really Mesa wants to know the interface version number
> right up front when the display is initialised so that it can determine
> the value to report for EGL_MIN_SWAP_INTERVAL.
>
> I'm not sure how to proceed with this. It seems like ideally Mesa should
> be able to make requests on the surface using its own binding of the
> compositor object so that it's not tied to the version of the interface
> that the application is using. I guess that would imply that it would
> have some way to get its own resource for the surface. That seems like
> quite a packed can of worms though.
>
> Anyone have any ideas?
>
> I'll attach the work in progress patches anyway just for good measure.
>
> Regards,
> - Neil
> ___
> 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

Re: [PATCH 2/2] protocol: add state set functions for maximized and fullscreen.

2013-10-23 Thread Bill Spitzak

Jason Ekstrand wrote:

At this point, I think I can propose a solution which should allow for 
client control while still keeping the compositor in control:
 1) The xdg_surface has a pare of activate/deactivate events.  I think 
we need more than keyboard focus here because the user may not have a 
physical keyboard.


How does a client request an activation? And can it refuse an activation 
from the compositor? Here is the sequence I see:


 - Compositor sends some kind of "I want you to activate" to client
 - Client (if it wants to activate) sends an "activate" request. Clients
can also do this in response to mouse/key events without an activate 
from the composiitor.

 - Compositor sends a "lost activation" to previous active client, and a
"gained activation" to the new client.

As I see it the only reason the compositor will send the first event is 
due to global shortcuts like Alt+Tab or some pager application, and 
destruction of the current active client. If clicks or hovering is going 
to activate a client, it is the client's responsibility to implement it.


I still feel it is going to be identical to the keyboard focus for some 
seat. A seat can have a focus even if it has no keyboard.


 2) A request_raise event to which the client *can* respond with any 
number of raise requests.


Again exactly the same sequence. The compositor can send a "I want you 
to raise" to the client, which it *can* respond to with raises. The 
client can also raise itself in response to events. There probably does 
not need to be feedback to the client whether the raise worked.


 3) Both the request_raise and the activate events have a serial.  If 
the event corresponded to some user input (such as a mouse press) then 
the serial will match that of the input event.  Otherwise, it will have 
its own serial.


Yes. The compositor must be able to tell what event caused the client to 
make the request. It can then ignore them, and solve race conditions 
with multiple clients.


Ok. I think I may be understanding transient windows better now (or 
maybe have come up with a good definition).  I think it's best seen in 
comparison to toplevel windows.  A toplevel window is "primary" in the 
sense that you expect it to have a button in the task bar, want it to 
show in expose, etc.  A transient window, on the other hand, is one that 
is peripheral such as a dialogue box or the floating toolboxes in GIMP. 
 You don't want every transient window show up in expose mode or in the 
task bar.  In fact, you don't want it to show up except in relation to 
another toplevel window.


That said, it is a very different concept to subsurfaces.  In 
particular, the client should not be able to always know where it is nor 
should it set it's location relative to another window directly. (This 
is different from popups).


Having written some applications, I absolutely want complete control 
over where transient windows appear They must appear in correct 
positions relative to the mouse cursor and to not obscure important 
information in the main window. Also it is very useful to make fancy 
transient tooltips that are hovering windows with pointers and that is 
not going to work if I can't make the pointers point at things!


If the client tells the compostor to drag a window (due perhaps to a 
mouse drag started in the window border) it is probably ok that the 
client does not know where the window ends up (though it is really 
annoying because it prevents clients from saving their window layouts).


Also I see no reason that the client can't tell the compositor to drag a 
subwindow as well. You are not convincing me that there is any 
difference, the more I look at it the more I am convinced that these 
concepts MUST be merged so that they each don't lack useful abilities 
the other ones have.


Given this understanding of transient windows, I'm less convinced that 
we need a window raise tree.


I *think* you are saying "there is no reason for the 'raise tree' to be 
a different tree than the 'parent tree'". This is prehaps why I did not 
understand what you were getting at. I never wanted more than a single 
tree, and I thought Kristian was asking whether there should be *one* or 
*zero* trees. Your email was about whether there should be *two* or 
*one* tree. I want one, the same as you do, I think.


So what I propose I think is the same as you are proposing: there is a 
single tree (actually a forest), of parent/child surface relationships. 
The "children" are sometimes called "transient windows", or "popup 
windows", etc. This both communicates what window is the "parent" and 
makes map and raise of the parents atomic with the children.


If a client really has a reason to make the parenting different than the 
raising it can temporarily rearrange the tree before doing a raise and 
then put it back.

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

Re: [RFC 1/5] Add a fullscreen shell protocol

2013-10-23 Thread Jason Ekstrand
Jonas,
Thanks for the review!

On Wed, Oct 23, 2013 at 3:56 PM, Jonas Ådahl  wrote:

> Hi,
>
> Using this protocol, how would the fullscreen shell client turn on and
> off outputs? I can see how clone, next-to and modeset by controlling
> what surface is presented and what fullscreen method is used, but that
> is not enough for turning an output off, isn't it?
>

I think that would be done by simply calling present_surface with a null
surface to remove the surface from the output.  I'm not 100% sure how this
will interact with KMS surface detection, but hopefully we should be able
to make it that simple.


> Another question is synchronising. Would a client ever want to change
> anything atomically? Do we need something like wl_surface.commit?
>

I'm not sure what would get changed atomically.  There's only one operation
and different outputs will probably have different refresh rates so changes
won't be atomic anyway.  Or are you thinking about trying to future-proof
stuff?


>
> Also, one wording comment below.
>
> Thanks
>
> Jonas
>
>
> On Tue, Oct 22, 2013 at 08:48:25PM -0500, Jason Ekstrand wrote:
> > ---
> >  protocol/fullscreen-shell.xml | 44
> +++
> >  1 file changed, 44 insertions(+)
> >  create mode 100644 protocol/fullscreen-shell.xml
> >
> > diff --git a/protocol/fullscreen-shell.xml
> b/protocol/fullscreen-shell.xml
> > new file mode 100644
> > index 000..b696828
> > --- /dev/null
> > +++ b/protocol/fullscreen-shell.xml
> > @@ -0,0 +1,44 @@
> > +
> > +  
> > +
> > +  Displays a single surface per output.
> > +
> > +  This interface can only be bound to by one client at a time.
> > +
> > +
> > +
> > +  
> > + Hints to indicate to the compositor how to deal with a conflict
> > + between the dimensions of the surface and the dimensions of the
> > + output. The compositor is free to ignore this parameter.
> > +  
> > +  
> > +  
> > +  
> > +  
> > +
> > +
> > +
> > +  
> > + This requests the system compositor to display surface on output.
> > + Each client of the system compositor can have at most one surface
>
> You still use the term 'system compositor' here. Also, it should be
> "display a surface" instead of "display surface".
>

Thanks.  Good catch.  I've fixed it locally.


>
> > + per output at any one time. Subsequent requests with the same
> > + output replace the surface bound to that output.  The same surface
> > + may be presented on multiple outputs.
> > +
> > + If the output is null, the compositor will present the surface on
> > + whatever display (or displays) it thinks best.  In particular, this
> > + may replace any or all surfaces currently presented so it should
> > + not be used in combination with placing surfaces on specific
> > + outputs.
> > +
> > + The method specifies how the surface is to be persented.  These
> > + methods are identical to those in wl_shell_surface.set_fullscreen.
> > +  
> > +  
> > +  
> > +  
> > +   allow-null="true"/>
> > +
> > +  
> > +
> > --
> > 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 2/2] protocol: add state set functions for maximized and fullscreen.

2013-10-23 Thread Jonas Ådahl
On Wed, Oct 23, 2013 at 03:20:52PM -0500, Jason Ekstrand wrote:
> On Tue, Oct 22, 2013 at 10:46 PM, Bill Spitzak  wrote:
> 
> > On 10/22/2013 05:59 PM, Jason Ekstrand wrote:
> >
> >  I see what you mean here.  However, I think apps doing this will cause
> >> more trouble than it's worth.  One thing about current sloppy focus
> >> implementations is that you can click anywhere in the window to raise
> >> it.  You want to change this.  However, what happens if that window is
> >> partially obscured.  In your scheme, you would have to move the window
> >> so that the magic text box is uncovered in order to raise it.  As is, if
> >> I want to see said text box, I click the window to raise it so I can get
> >> to the text box.
> >>
> >
> >  Yes, I agree it would be nice if you could select text without raising
> >> the window.  However, I'd still like a way to raise it without having to
> >> click a particular region.  That's an interesting UI problem
> >>
> >
> > I think the clients can raise if the user clicks on a "dead" area where
> > the click serves no purpose. The most obvious is the window borders, but
> > also gaps between widgets and widgets that don't use click for anything
> > could raise the window.
> >
> > It would still be possible for all the visible area of a surface to be
> > covered with buttons so there is nowhere to click to raise it. If this is
> > really a problem then perhaps the user holds down some shift key and clicks?
> >
> >
> >  Perhaps we could allow for some more flexibility if there was a way for
> >> the client to reject an activation.  However, I still have the above
> >> fears.
> >>
> >
> > Absolutely a client must be able to "reject an activation". This would be
> > just like raise and focus: the compositor can only send a request event to
> > the client. The client must respond with an activate request if it really
> > wants activation.
> >
> > However I think this may mean there is no difference between activation
> > and having a particular seat's keyboard focus.
> >
> >
> At this point, I think I can propose a solution which should allow for
> client control while still keeping the compositor in control:
>  1) The xdg_surface has a pare of activate/deactivate events.  I think we
> need more than keyboard focus here because the user may not have a physical
> keyboard.
>  2) A request_raise event to which the client *can* respond with any number
> of raise requests.
>  3) Both the request_raise and the activate events have a serial.  If the
> event corresponded to some user input (such as a mouse press) then the
> serial will match that of the input event.  Otherwise, it will have its own
> serial.

 4) Inter-surface-commit for making sure the client can raise a
 group of windows atomically (?)

 Jonas

> 
> 
> >
> >  Yes, I like this.  I don't think it's feasible to try and keep some
> >> crazy DAG.  As long as a client can atomically raise a bunch of windows
> >> we should be ok.  A tree, whether or not it persists after the raise,
> >> will accomplish this.
> >>
> >
> > I think if the tree does not persist then it is identical to the "client
> > does a lot of raises atomically" version. This is however a possible
> > solution.
> >
> > The only reason for the tree is so that other clients can examine it and
> > get some ideas about the relationship between surfaces. And also for
> > familiarity with other window systems. I think it may also be useful for
> > RDP to other window systems which may not have atomic sets of raises.
> >
> >   I believe these child surfaces and "subsurfaces" are
> >> EXACTLY the
> >>  same thing.
> >> I still don't think these *should* be the same.  I understand that the
> >> semantics are similar, particularly for popup windows.  That said,
> >> Kristian has talked about removing the coordinates from "transient".
> >>
> >
> > Not clear what he is getting at there. If the client can't position a
> > popup menu in the correct location it is pretty useless. I suspect he is
> > using a different definition of "transient" than I am.
> >
> >
> Ok. I think I may be understanding transient windows better now (or maybe
> have come up with a good definition).  I think it's best seen in comparison
> to toplevel windows.  A toplevel window is "primary" in the sense that you
> expect it to have a button in the task bar, want it to show in expose, etc.
>  A transient window, on the other hand, is one that is peripheral such as a
> dialogue box or the floating toolboxes in GIMP.  You don't want every
> transient window show up in expose mode or in the task bar.  In fact, you
> don't want it to show up except in relation to another toplevel window.
> 
> That said, it is a very different concept to subsurfaces.  In particular,
> the client should not be able to always know where it is nor should it set
> it's location relative to another window directly. (This is different from
> popups).
> 
> Given this understanding of transie

Re: [RFC 1/5] Add a fullscreen shell protocol

2013-10-23 Thread Jonas Ådahl
Hi,

Using this protocol, how would the fullscreen shell client turn on and
off outputs? I can see how clone, next-to and modeset by controlling
what surface is presented and what fullscreen method is used, but that
is not enough for turning an output off, isn't it?

Another question is synchronising. Would a client ever want to change
anything atomically? Do we need something like wl_surface.commit?

Also, one wording comment below.

Thanks

Jonas


On Tue, Oct 22, 2013 at 08:48:25PM -0500, Jason Ekstrand wrote:
> ---
>  protocol/fullscreen-shell.xml | 44 
> +++
>  1 file changed, 44 insertions(+)
>  create mode 100644 protocol/fullscreen-shell.xml
> 
> diff --git a/protocol/fullscreen-shell.xml b/protocol/fullscreen-shell.xml
> new file mode 100644
> index 000..b696828
> --- /dev/null
> +++ b/protocol/fullscreen-shell.xml
> @@ -0,0 +1,44 @@
> +
> +  
> +
> +  Displays a single surface per output.
> +
> +  This interface can only be bound to by one client at a time.
> +
> +
> +
> +  
> + Hints to indicate to the compositor how to deal with a conflict
> + between the dimensions of the surface and the dimensions of the
> + output. The compositor is free to ignore this parameter.
> +  
> +  
> +  
> +  
> +  
> +
> +
> +
> +  
> + This requests the system compositor to display surface on output.
> + Each client of the system compositor can have at most one surface

You still use the term 'system compositor' here. Also, it should be
"display a surface" instead of "display surface".

> + per output at any one time. Subsequent requests with the same
> + output replace the surface bound to that output.  The same surface
> + may be presented on multiple outputs.
> +
> + If the output is null, the compositor will present the surface on
> + whatever display (or displays) it thinks best.  In particular, this
> + may replace any or all surfaces currently presented so it should
> + not be used in combination with placing surfaces on specific
> + outputs.
> +
> + The method specifies how the surface is to be persented.  These
> + methods are identical to those in wl_shell_surface.set_fullscreen.
> +  
> +  
> +  
> +  
> +   allow-null="true"/>
> +
> +  
> +
> -- 
> 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 2/2] protocol: add state set functions for maximized and fullscreen.

2013-10-23 Thread Jason Ekstrand
On Tue, Oct 22, 2013 at 10:46 PM, Bill Spitzak  wrote:

> On 10/22/2013 05:59 PM, Jason Ekstrand wrote:
>
>  I see what you mean here.  However, I think apps doing this will cause
>> more trouble than it's worth.  One thing about current sloppy focus
>> implementations is that you can click anywhere in the window to raise
>> it.  You want to change this.  However, what happens if that window is
>> partially obscured.  In your scheme, you would have to move the window
>> so that the magic text box is uncovered in order to raise it.  As is, if
>> I want to see said text box, I click the window to raise it so I can get
>> to the text box.
>>
>
>  Yes, I agree it would be nice if you could select text without raising
>> the window.  However, I'd still like a way to raise it without having to
>> click a particular region.  That's an interesting UI problem
>>
>
> I think the clients can raise if the user clicks on a "dead" area where
> the click serves no purpose. The most obvious is the window borders, but
> also gaps between widgets and widgets that don't use click for anything
> could raise the window.
>
> It would still be possible for all the visible area of a surface to be
> covered with buttons so there is nowhere to click to raise it. If this is
> really a problem then perhaps the user holds down some shift key and clicks?
>
>
>  Perhaps we could allow for some more flexibility if there was a way for
>> the client to reject an activation.  However, I still have the above
>> fears.
>>
>
> Absolutely a client must be able to "reject an activation". This would be
> just like raise and focus: the compositor can only send a request event to
> the client. The client must respond with an activate request if it really
> wants activation.
>
> However I think this may mean there is no difference between activation
> and having a particular seat's keyboard focus.
>
>
At this point, I think I can propose a solution which should allow for
client control while still keeping the compositor in control:
 1) The xdg_surface has a pare of activate/deactivate events.  I think we
need more than keyboard focus here because the user may not have a physical
keyboard.
 2) A request_raise event to which the client *can* respond with any number
of raise requests.
 3) Both the request_raise and the activate events have a serial.  If the
event corresponded to some user input (such as a mouse press) then the
serial will match that of the input event.  Otherwise, it will have its own
serial.


>
>  Yes, I like this.  I don't think it's feasible to try and keep some
>> crazy DAG.  As long as a client can atomically raise a bunch of windows
>> we should be ok.  A tree, whether or not it persists after the raise,
>> will accomplish this.
>>
>
> I think if the tree does not persist then it is identical to the "client
> does a lot of raises atomically" version. This is however a possible
> solution.
>
> The only reason for the tree is so that other clients can examine it and
> get some ideas about the relationship between surfaces. And also for
> familiarity with other window systems. I think it may also be useful for
> RDP to other window systems which may not have atomic sets of raises.
>
>   I believe these child surfaces and "subsurfaces" are
>> EXACTLY the
>>  same thing.
>> I still don't think these *should* be the same.  I understand that the
>> semantics are similar, particularly for popup windows.  That said,
>> Kristian has talked about removing the coordinates from "transient".
>>
>
> Not clear what he is getting at there. If the client can't position a
> popup menu in the correct location it is pretty useless. I suspect he is
> using a different definition of "transient" than I am.
>
>
Ok. I think I may be understanding transient windows better now (or maybe
have come up with a good definition).  I think it's best seen in comparison
to toplevel windows.  A toplevel window is "primary" in the sense that you
expect it to have a button in the task bar, want it to show in expose, etc.
 A transient window, on the other hand, is one that is peripheral such as a
dialogue box or the floating toolboxes in GIMP.  You don't want every
transient window show up in expose mode or in the task bar.  In fact, you
don't want it to show up except in relation to another toplevel window.

That said, it is a very different concept to subsurfaces.  In particular,
the client should not be able to always know where it is nor should it set
it's location relative to another window directly. (This is different from
popups).

Given this understanding of transient windows, I'm less convinced that we
need a window raise tree.  I don't see a reason why the client would want
to raise more than one toplevel window at any given time.  It should be
able to just re-parent the needed transient windows and raise the toplevel
one.  (or, for that matter, just raise a transient window).  The only issue
here is that it needs a way to "com

[PATCH weston 3/4] autotools: Move the path magic to configure.ac

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

We provide them all in our pkg-config file for usage by external
modules.
Also use pkg-prefixed directories

Signed-off-by: Quentin Glidic 
---
 clients/Makefile.am  |  2 +-
 clients/desktop-shell.c  |  4 ++--
 configure.ac | 32 
 data/Makefile.am |  4 +---
 man/Makefile.am  |  2 +-
 shared/Makefile.am   |  2 +-
 shared/frame.c   |  8 
 src/Makefile.am  |  9 -
 src/compositor-wayland.c |  2 +-
 src/compositor-x11.c |  2 +-
 src/weston.pc.in |  8 ++--
 src/xwayland/Makefile.am |  4 ++--
 12 files changed, 56 insertions(+), 23 deletions(-)

diff --git a/clients/Makefile.am b/clients/Makefile.am
index 4f9dc48..0b6b5ea 100644
--- a/clients/Makefile.am
+++ b/clients/Makefile.am
@@ -25,7 +25,7 @@ libexec_PROGRAMS =\
 
 AM_CFLAGS = $(GCC_CFLAGS)
 AM_CPPFLAGS =  \
-   -DDATADIR='"$(datadir)"'\
+   -DPKGDATADIR='"$(pkgdatadir)"'  \
-DBINDIR='"$(bindir)"'  \
$(CLIENT_CFLAGS) $(CAIRO_EGL_CFLAGS)
 
diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
index 8c97104..3081b98 100644
--- a/clients/desktop-shell.c
+++ b/clients/desktop-shell.c
@@ -1054,7 +1054,7 @@ background_create(struct desktop *desktop)
s = weston_config_get_section(desktop->config, "shell", NULL, NULL);
weston_config_section_get_string(s, "background-image",
 &background->image,
-DATADIR "/weston/pattern.png");
+PKGDATADIR "/pattern.png");
weston_config_section_get_uint(s, "background-color",
   &background->color, 0xff002244);
 
@@ -1272,7 +1272,7 @@ panel_add_launchers(struct panel *panel, struct desktop 
*desktop)
if (count == 0) {
/* add default launcher */
panel_add_launcher(panel,
-  DATADIR "/weston/terminal.png",
+  PKGDATADIR "/terminal.png",
   BINDIR "/weston-terminal");
}
 }
diff --git a/configure.ac b/configure.ac
index 2bb7c9c..0fa1766 100644
--- a/configure.ac
+++ b/configure.ac
@@ -36,6 +36,38 @@ LT_INIT([disable-static])
 AC_ARG_VAR([WESTON_NATIVE_BACKEND],
[Set the native backend to use, if Weston is not running under 
Wayland nor X11. @<:@default=drm-backend.so@:>@])
 
+
+#
+# Directories
+#
+
+# package-specific dirs
+# Autotools define them for us already,
+# but we do that here for configure substitution
+pkglibdir='${libdir}/'${PACKAGE}
+pkglibexecdir='${libexecdir}/'${PACKAGE}
+pkgincludedir='${includedir}/'${PACKAGE}
+pkgdatadir='${datadir}/'${PACKAGE}
+AC_SUBST([pkglibdir])
+AC_SUBST([pkglibexecdir])
+AC_SUBST([pkgincludedir])
+AC_SUBST([pkgdatadir])
+
+# pkg-config files
+# We prefer the system-provided value if available,
+# and fallback to a well-known value
+m4_ifdef([PKG_INSTALLDIR], [
+   PKG_INSTALLDIR()
+   ], [
+   pkgconfigdir='${libdir}/pkgconfig'
+   AC_SUBST([pkgconfigdir])
+   ])
+
+# weston specific dirs
+moduledir='${pkglibdir}/plugins'
+AC_SUBST([moduledir])
+
+
 PKG_PROG_PKG_CONFIG()
 
 AC_CHECK_FUNC([dlopen], [],
diff --git a/data/Makefile.am b/data/Makefile.am
index a7cc944..f6f05f9 100644
--- a/data/Makefile.am
+++ b/data/Makefile.am
@@ -1,6 +1,4 @@
-westondatadir = $(datadir)/weston
-
-dist_westondata_DATA = \
+dist_pkgdata_DATA =\
wayland.svg \
$(wayland_icon_png) \
pattern.png \
diff --git a/man/Makefile.am b/man/Makefile.am
index e4abd8c..37174e2 100644
--- a/man/Makefile.am
+++ b/man/Makefile.am
@@ -6,7 +6,7 @@ endif
 
 MAN_SUBSTS = \
-e 's|__weston_native_backend__|$(WESTON_NATIVE_BACKEND)|g' \
-   -e 's|__weston_modules_dir__|$(pkglibdir)|g' \
+   -e 's|__weston_modules_dir__|$(moduledir)|g' \
-e 's|__version__|$(PACKAGE_VERSION)|g'
 
 SUFFIXES = .1 .5 .7 .man
diff --git a/shared/Makefile.am b/shared/Makefile.am
index 31fab5f..432d6d9 100644
--- a/shared/Makefile.am
+++ b/shared/Makefile.am
@@ -10,7 +10,7 @@ libshared_la_SOURCES =\
os-compatibility.h
 
 libshared_cairo_la_CFLAGS =\
-   -DDATADIR='"$(datadir)"'\
+   -DPKGDATADIR='"$(pkgdatadir)"'  \
$(GCC_CFLAGS)   \
$(COMPOSITOR_CFLAGS)\
$(PIXMAN_CFLAGS)\
diff --git a/shared/frame.c b/shared/frame.c
index fc85950..1aad0be 100644
--- a/shared/frame.c
+++ b/shared/frame.c
@@ -291,7 +291,7 @@ frame_create(struct theme *t, int32_t width, int32_t 
height, uint

[PATCH weston 4/4] clients: Install to pkglibexecdir

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

Signed-off-by: Quentin Glidic 
---
 clients/Makefile.am  | 2 +-
 src/Makefile.am  | 2 +-
 src/screenshooter.c  | 2 +-
 src/shell.c  | 2 +-
 src/tablet-shell.c   | 2 +-
 src/text-backend.c   | 2 +-
 src/xwayland/Makefile.am | 2 --
 7 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/clients/Makefile.am b/clients/Makefile.am
index 0b6b5ea..1fd5c2a 100644
--- a/clients/Makefile.am
+++ b/clients/Makefile.am
@@ -15,7 +15,7 @@ else
 noinst_PROGRAMS = $(demo_clients)
 endif
 
-libexec_PROGRAMS = \
+pkglibexec_PROGRAMS =  \
$(desktop_shell)\
$(tablet_shell) \
$(screenshooter)\
diff --git a/src/Makefile.am b/src/Makefile.am
index 7bbc7ba..74dc152 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -5,7 +5,7 @@ AM_CPPFLAGS =   \
-I$(top_srcdir)/shared  \
-DPKGDATADIR='"$(pkgdatadir)"'  \
-DMODULEDIR='"$(moduledir)"'\
-   -DLIBEXECDIR='"$(libexecdir)"'  \
+   -DPKGLIBEXECDIR='"$(pkglibexecdir)"'\
-DIN_WESTON
 
 weston_LDFLAGS = -export-dynamic
diff --git a/src/screenshooter.c b/src/screenshooter.c
index 645114d..b91c6ac 100644
--- a/src/screenshooter.c
+++ b/src/screenshooter.c
@@ -248,7 +248,7 @@ screenshooter_binding(struct weston_seat *seat, uint32_t 
time, uint32_t key,
  void *data)
 {
struct screenshooter *shooter = data;
-   const char *screenshooter_exe = LIBEXECDIR "/weston-screenshooter";
+   const char *screenshooter_exe = PKGLIBEXECDIR "/weston-screenshooter";
 
if (!shooter->client)
shooter->client = weston_client_launch(shooter->ec,
diff --git a/src/shell.c b/src/shell.c
index 9663870..4577dfb 100644
--- a/src/shell.c
+++ b/src/shell.c
@@ -3858,7 +3858,7 @@ static void
 launch_desktop_shell_process(void *data)
 {
struct desktop_shell *shell = data;
-   const char *shell_exe = LIBEXECDIR "/weston-desktop-shell";
+   const char *shell_exe = PKGLIBEXECDIR "/weston-desktop-shell";
 
shell->child.client = weston_client_launch(shell->compositor,
 &shell->child.process,
diff --git a/src/tablet-shell.c b/src/tablet-shell.c
index 5e92678..9ea9e18 100644
--- a/src/tablet-shell.c
+++ b/src/tablet-shell.c
@@ -403,7 +403,7 @@ static const struct tablet_shell_interface 
tablet_shell_implementation = {
 static void
 launch_ux_daemon(struct tablet_shell *shell)
 {
-   const char *shell_exe = LIBEXECDIR "/weston-tablet-shell";
+   const char *shell_exe = PKGLIBEXECDIR "/weston-tablet-shell";
 
shell->client = weston_client_launch(shell->compositor,
   &shell->process,
diff --git a/src/text-backend.c b/src/text-backend.c
index 37dfb75..0b7887e 100644
--- a/src/text-backend.c
+++ b/src/text-backend.c
@@ -918,7 +918,7 @@ text_backend_configuration(struct text_backend 
*text_backend)
"input-method", NULL, NULL);
weston_config_section_get_string(section, "path",
 &text_backend->input_method.path,
-LIBEXECDIR "/weston-keyboard");
+PKGLIBEXECDIR "/weston-keyboard");
 }
 
 static void
diff --git a/src/xwayland/Makefile.am b/src/xwayland/Makefile.am
index 2cf5615..9f47f3d 100644
--- a/src/xwayland/Makefile.am
+++ b/src/xwayland/Makefile.am
@@ -2,8 +2,6 @@ AM_CPPFLAGS =   \
-I$(top_srcdir)/shared  \
-I$(top_builddir)/src   \
-DPKGDATADIR='"$(pkgdatadir)"'  \
-   -DMODULEDIR='"$(moduledir)"'\
-   -DLIBEXECDIR='"$(libexecdir)"'  \
-DXSERVER_PATH='"@XSERVER_PATH@"'
 
 # moduledir is defined in configure.ac
-- 
1.8.4.1

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


[PATCH weston 1/4] tests: Use check_* variables but build by default

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

Signed-off-by: Quentin Glidic 
---
 tests/Makefile.am | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 5be52c6..d810d8a 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -31,16 +31,19 @@ clean-local:
 # To remove when automake 1.11 support is dropped
 export abs_builddir
 
-noinst_LTLIBRARIES =   \
+check_LTLIBRARIES =\
$(weston_test)  \
$(module_tests)
 
-noinst_PROGRAMS =  \
+check_PROGRAMS =   \
$(setbacklight) \
$(shared_tests) \
$(weston_tests) \
matrix-test
 
+# We build tests with the default "all" target too
+all-local: $(check_LTLIBRARIES) $(check_PROGRAMS)
+
 AM_CFLAGS = $(GCC_CFLAGS)
 AM_CPPFLAGS =  \
-I$(top_srcdir)/src \
-- 
1.8.4.1

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


[PATCH weston 2/4] weston.pc: Add dependencies for pixman-1 and xkbcommon

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

Weston headers include pixman and libxkbcommon headers
Using Requires.private means that CFLAGS from pixman-1 and xkbcommon are
added to weston CFLAGS, while LIBS are added in case of static linking
only. This way, plugins does not have to use them, but will need to do
so explicitely if needed, to properly resolve symbols

Signed-off-by: Quentin Glidic 
---
 configure.ac | 4 
 src/weston.pc.in | 1 +
 2 files changed, 5 insertions(+)

diff --git a/configure.ac b/configure.ac
index 80a5d69..2bb7c9c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -56,6 +56,7 @@ AC_CHECK_HEADERS([execinfo.h])
 AC_CHECK_FUNCS([mkostemp strchrnul initgroups])
 
 COMPOSITOR_MODULES="wayland-server >= 1.2.91 pixman-1"
+WESTON_PC_REQUIRES_PRIVATE="pixman-1"
 
 AC_ARG_ENABLE(egl, [  --disable-egl],,
   enable_egl=yes)
@@ -76,6 +77,7 @@ AC_ARG_ENABLE(xkbcommon,
 if test x$enable_xkbcommon = xyes; then
AC_DEFINE(ENABLE_XKBCOMMON, [1], [Build Weston with libxkbcommon 
support])
COMPOSITOR_MODULES="$COMPOSITOR_MODULES xkbcommon"
+   WESTON_PC_REQUIRES_PRIVATE="${WESTON_PC_REQUIRES_PRIVATE} xkbcommon"
 fi
 
 AC_ARG_ENABLE(setuid-install, [  --enable-setuid-install],,
@@ -474,6 +476,8 @@ if test x$wayland_scanner = x; then
AC_MSG_ERROR([wayland-scanner is needed to compile weston])
 fi
 
+AC_SUBST([WESTON_PC_REQUIRES_PRIVATE])
+
 AC_CONFIG_FILES([Makefile
 shared/Makefile
 src/Makefile
diff --git a/src/weston.pc.in b/src/weston.pc.in
index 828cb1f..5e61d3d 100644
--- a/src/weston.pc.in
+++ b/src/weston.pc.in
@@ -8,4 +8,5 @@ pkglibexecdir=${libexecdir}/@PACKAGE@
 Name: Weston Plugin API
 Description: Header files for Weston plugin development
 Version: @WESTON_VERSION@
+Requires.private: @WESTON_PC_REQUIRES_PRIVATE@
 Cflags: -I${includedir}
-- 
1.8.4.1

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


[PATCH weston 2/2] Add a test for wl_surface.set_release

2013-10-23 Thread Neil Roberts
The test attaches a buffer and then verifies that it doesn't get the
release event until a roundtrip is issued causing the event queue to
flush. It then sets the release mode to immediate and then verifies
that it doesn't need to do a roundtrip to get the release event. The
default mode is then used a second time to verify that setting it to
immediate only lasts for a single commit.
---
 tests/Makefile.am |   7 ++
 tests/delayed-release-test.c  | 150 ++
 tests/weston-test-client-helper.c |   2 +-
 3 files changed, 158 insertions(+), 1 deletion(-)
 create mode 100644 tests/delayed-release-test.c

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 5be52c6..c1de1cb 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -16,6 +16,7 @@ weston_tests =\
button.weston   \
text.weston \
subsurface.weston   \
+   delayed-release.weston  \
$(xwayland_test)
 
 AM_TESTS_ENVIRONMENT = \
@@ -117,6 +118,12 @@ text_weston_LDADD = $(weston_test_client_libs)
 subsurface_weston_SOURCES = subsurface-test.c $(weston_test_client_src)
 subsurface_weston_LDADD = $(weston_test_client_libs)
 
+delayed_release_weston_SOURCES = \
+   delayed-release-test.c \
+   $(weston_test_client_src)
+delayed_release_weston_LDADD = \
+   $(weston_test_client_libs)
+
 xwayland_weston_SOURCES = xwayland-test.c  $(weston_test_client_src)
 
 xwayland_weston_LDADD = $(weston_test_client_libs) $(XWAYLAND_TEST_LIBS)
diff --git a/tests/delayed-release-test.c b/tests/delayed-release-test.c
new file mode 100644
index 000..9d61bbc
--- /dev/null
+++ b/tests/delayed-release-test.c
@@ -0,0 +1,150 @@
+/*
+ * Copyright © 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, 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 "weston-test-client-helper.h"
+#include 
+#include 
+#include 
+
+struct test_data {
+   struct client *client;
+   int release_received;
+};
+
+static void
+buffer_release(void *data, struct wl_buffer *buffer)
+{
+   struct test_data *test_data = data;
+
+   test_data->release_received = 1;
+}
+
+static const struct wl_buffer_listener buffer_listener = {
+   buffer_release
+};
+
+static long int
+timespec_diff(const struct timespec *a, const struct timespec *b)
+{
+   return ((a->tv_sec - b->tv_sec) * 1000 +
+   (a->tv_nsec - b->tv_nsec) / 100);
+}
+
+static void wait_release_event(struct test_data *test_data)
+{
+   struct timespec start_time, now;
+   struct pollfd pollfd;
+
+   test_data->release_received = 0;
+
+   clock_gettime(CLOCK_MONOTONIC, &start_time);
+
+   /* Wait for up to ¼ seconds for a release event from the
+* compositor. We don't want to call wl_display_roundtrip
+* because the callback that it installs would cause the event
+* queue to be flushed */
+
+   pollfd.fd = wl_display_get_fd(test_data->client->wl_display);
+   pollfd.events = POLLIN;
+
+   while (!test_data->release_received) {
+   long int diff;
+
+   wl_display_dispatch_pending(test_data->client->wl_display);
+   wl_display_flush(test_data->client->wl_display);
+
+   clock_gettime(CLOCK_MONOTONIC, &now);
+
+   diff = timespec_diff(&now, &start_time);
+
+   if (diff >= 250)
+   break;
+
+   pollfd.revents = 0;
+
+   poll(&pollfd, 1, 250 - diff);
+
+   if (pollfd.revents)
+   wl_display_dispatch(test_data->client->wl_display);
+   }
+}
+
+static void assert_delayed_release(struct test_data *test_data)
+{
+   wait_release_event(test_data);
+   assert(!test_data->release_received);
+
+   /* Do a roundtrip to forc

Re: Buffer release events (was: Add support for eglSwapInterval)

2013-10-23 Thread Neil Roberts
Neil Roberts  writes:

> Currently the only proposed complete solution is just to add a request
> to explicitly disable the queuing mechanism.

I started writing this patch but I've hit a stumbling block while trying
to make use of it in Mesa. Adding the new request requires altering the
version of the wl_compositor and wl_surface interfaces. The problem is
that Mesa can't really use the new request unless the wl_surface object
is using the new version. However, the version of the interface that is
actually in use is determined by the application when it binds the
compositor object. As far as I can tell, there is no way for Mesa to
determine what version the surface proxy object is using. Potentially we
could add some API to query this, but I think even that wouldn't be
ideal because really Mesa wants to know the interface version number
right up front when the display is initialised so that it can determine
the value to report for EGL_MIN_SWAP_INTERVAL.

I'm not sure how to proceed with this. It seems like ideally Mesa should
be able to make requests on the surface using its own binding of the
compositor object so that it's not tied to the version of the interface
that the application is using. I guess that would imply that it would
have some way to get its own resource for the surface. That seems like
quite a packed can of worms though.

Anyone have any ideas?

I'll attach the work in progress patches anyway just for good measure.

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


[PATCH weston 1/2] Implement wl_surface.set_release

2013-10-23 Thread Neil Roberts
Implements the wl_surface.set_release request which just causes the
buffer release events to be sent with wl_resource_post_event instead
of wl_resource_queue_event. The release mode is part of the
double-buffered surface state and gets reset to the default as soon as
a commit is performed on the surface.
---
 src/compositor.c | 53 +
 src/compositor.h |  4 
 2 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/src/compositor.c b/src/compositor.c
index 7e2a394..0a48f39 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -376,6 +376,7 @@ weston_surface_create(struct weston_compositor *compositor)
surface->buffer_scale = 1;
surface->pending.buffer_transform = surface->buffer_transform;
surface->pending.buffer_scale = surface->buffer_scale;
+   surface->pending.release_mode = WL_SURFACE_RELEASE_DELAYED;
surface->output = NULL;
surface->plane = &compositor->primary_plane;
surface->pending.newly_attached = 0;
@@ -1167,6 +1168,7 @@ weston_buffer_from_resource(struct wl_resource *resource)
wl_signal_init(&buffer->destroy_signal);
buffer->destroy_listener.notify = weston_buffer_destroy_handler;
buffer->y_inverted = 1;
+   buffer->release_mode = WL_SURFACE_RELEASE_DELAYED;
wl_resource_add_destroy_listener(resource, &buffer->destroy_listener);

return buffer;
@@ -1184,17 +1186,30 @@ weston_buffer_reference_handle_destroy(struct 
wl_listener *listener,
ref->buffer = NULL;
 }
 
+static void
+weston_buffer_send_release(struct weston_buffer *buffer)
+{
+   assert(wl_resource_get_client(buffer->resource));
+
+   if (buffer->release_mode == WL_SURFACE_RELEASE_DELAYED) {
+   wl_resource_queue_event(buffer->resource, WL_BUFFER_RELEASE);
+   } else {
+   /* The release mode state should only effect a single
+* attach so we'll reset it back to the default after
+* posting the event */
+   buffer->release_mode = WL_SURFACE_RELEASE_DELAYED;
+   wl_resource_post_event(buffer->resource, WL_BUFFER_RELEASE);
+   }
+}
+
 WL_EXPORT void
 weston_buffer_reference(struct weston_buffer_reference *ref,
struct weston_buffer *buffer)
 {
if (ref->buffer && buffer != ref->buffer) {
ref->buffer->busy_count--;
-   if (ref->buffer->busy_count == 0) {
-   assert(wl_resource_get_client(ref->buffer->resource));
-   wl_resource_queue_event(ref->buffer->resource,
-   WL_BUFFER_RELEASE);
-   }
+   if (ref->buffer->busy_count == 0)
+   weston_buffer_send_release(ref->buffer);
wl_list_remove(&ref->destroy_listener.link);
}
 
@@ -1655,6 +1670,17 @@ weston_surface_commit(struct weston_surface *surface)
/* wl_surface.set_buffer_scale */
surface->buffer_scale = surface->pending.buffer_scale;
 
+   /* wl_surface.set_release */
+   if (surface->pending.release_mode == WL_SURFACE_RELEASE_IMMEDIATE) {
+   if (surface->pending.buffer)
+   surface->pending.buffer->release_mode =
+   WL_SURFACE_RELEASE_IMMEDIATE;
+   /* The release mode state should only effect a single
+* attach so we'll reset it back to the default after
+* setting it on the buffer */
+   surface->pending.release_mode = WL_SURFACE_RELEASE_DELAYED;
+   }
+
/* wl_surface.attach */
if (surface->pending.buffer || surface->pending.newly_attached)
weston_surface_attach(surface, surface->pending.buffer);
@@ -1762,6 +1788,16 @@ surface_set_buffer_scale(struct wl_client *client,
surface->pending.buffer_scale = scale;
 }
 
+static void
+surface_set_release(struct wl_client *client,
+   struct wl_resource *resource,
+   uint32_t value)
+{
+   struct weston_surface *surface = wl_resource_get_user_data(resource);
+
+   surface->pending.release_mode = value;
+}
+
 static const struct wl_surface_interface surface_interface = {
surface_destroy,
surface_attach,
@@ -1771,7 +1807,8 @@ static const struct wl_surface_interface 
surface_interface = {
surface_set_input_region,
surface_commit,
surface_set_buffer_transform,
-   surface_set_buffer_scale
+   surface_set_buffer_scale,
+   surface_set_release
 };
 
 static void
@@ -2928,7 +2965,7 @@ compositor_bind(struct wl_client *client,
struct wl_resource *resource;
 
resource = wl_resource_create(client, &wl_compositor_interface,
- MIN(version, 3), id);
+ MIN(version, 4), id);
if (resource == NULL) {
wl_clien

[PATCH] protocol: Add a request to disable queuing of buffer release events

2013-10-23 Thread Neil Roberts
Adds a request called wl_surface.set_release which provides a way for
a client to explicitly disable the queuing mechanism for buffer
release events so that it can get them as soon as the buffer is no
longer being used. This is useful for example when doing
eglSwapInterval(0) because in that case the client is not likely to be
installing a frame complete callback so nothing will cause the event
queue to be flushed. However the EGL driver is likely to be using a
finite number of buffers and it may be blocking until a release event
is received. Without this request the driver may end up blocking
forever.

There was some discussion for this patch on whether the request should
be on the wl_buffer interface rather than wl_surface. However it would
be difficult for the compositor to use if it was on the wl_buffer
interface because the implementation for that is in Mesa and
wayland-shm. That means that we would have to have some extra EGL API
to query the release mode in order for the compositor to see it.
---
 protocol/wayland.xml | 51 ---
 1 file changed, 48 insertions(+), 3 deletions(-)

diff --git a/protocol/wayland.xml b/protocol/wayland.xml
index a1df007..77d098a 100644
--- a/protocol/wayland.xml
+++ b/protocol/wayland.xml
@@ -173,7 +173,7 @@
 
   
 
-  
+  
 
   A compositor.  This object is a singleton global.  The
   compositor is in charge of combining the contents of multiple
@@ -959,7 +959,7 @@
 
   
 
-  
+  
 
   A surface is a rectangular area that is displayed on the screen.
   It has a location, size and pixel contents.
@@ -1233,7 +1233,52 @@
   
   
 
-   
+
+
+
+
+  
+These values are used for the wl_surface.set_release request to
+specify when to send buffer release events.
+WL_SURFACE_RELEASE_IMMEDIATE means the release event will be
+sent as soon as the buffer is no longer used by the
+compositor. WL_SURFACE_RELEASE_DELAYED means that the
+compositor may choose to put the release event into a queue
+and only flush it once some other events are ready to send as
+well. Typically a client will want the latter because it
+doesn't usually need to know about the release events until it
+is about to render something so it can avoid being woken up
+unnecessarily. The default value is WL_SURFACE_RELEASE_DELAYED.
+  
+  
+  
+
+
+
+  
+By default the compositor will typically delay sending release
+events for a buffer until some other event is also being sent
+in order to avoid waking up the clients more often than
+necessary. Usually the buffer release will end up being sent
+along with the frame complete callback. However some clients
+will want to be able to reuse the buffers earlier than that
+for example if they are not using the frame complete callback.
+This can be used to give a hint to the compositor that the
+client wants the release event for the given buffer as soon as
+possible.
+
+The release mode is double-buffered state, see
+wl_surface.commit.
+
+The default value is WL_SURFACE_RELEASE_DELAYED meaning that
+the release events will be delayed. Setting the value to
+WL_SURFACE_RELEASE_IMMEDIATE only effects the buffer currently
+being attached and subsequent attaches will continue to default
+to WL_SURFACE_RELEASE_DELAYED.
+  
+  
+
+  
 
   
 
-- 
1.8.3.1

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


Re: [PATCH 2/2] compositor: finish frame if redraw fails

2013-10-23 Thread David Herrmann
Hi

On Tue, Oct 22, 2013 at 9:45 PM, Kristian Høgsberg  wrote:
> On Tue, Oct 22, 2013 at 05:11:26PM +0200, David Herrmann wrote:
>> If we are about to finish a frame, but a redraw is pending and we let the
>> compositor redraw, we need to check for errors. If the redraw fails and
>> the backend cannot schedule a page-flip, we need to finish the frame,
>> anyway.
>>
>> All backends except DRM use a timer to schedule frames. Hence, they cannot
>> fail. But for DRM, we need to be able to handle drmModePageFlip() failures
>> in case access got revoked.
>>
>> This fixes a bug where logind+drm caused keyboard input to be missed as we
>> didn't reenable it after a failed page-flip during deactivation.
>
> Yes, that's better, applied both of these patches.  Aside from being
> unable to turn off sprites and hw cursor, the async deactivate also
> means that we can lose drm master at any time, in particular between
> starting repaint and pageflip.  So we need to handle this better and I
> think these two patches covers it.
>
> There's one last thing that doesn't quite work yet - when I switch to
> weston, it doesn't immediately (re)set the current framebuffers.  I
> have to trigger a repaint by moving the mouse or such.  I'm not sure
> why that doesn't work, we still have the call to
> drm_compositor_set_modes() and that should run with drm master set and
> restore the last mode and buffer.

It works for me. Does the log show anything?
Thanks for testing!

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


Re: [RFC wayland v2] libwayland-client: Add GLib main loop integration

2013-10-23 Thread sardemff7+wayland

Forgot to amend the commit message:
libwayland-client: Add GLib main loop integration

--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


[RFC wayland v2] Add libwayland-glib for GLib main loop integration

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

Signed-off-by: Quentin Glidic 
---

Here is a new version of my GLib main loop integration for Wayland.
Now everything is in libwayland-client (I didn’t work on the server
side) and package detection can be done using the pkg-config file
or the header if needed.


 configure.ac  |  22 -
 src/Makefile.am   |  17 
 src/wayland-client-glib.c | 182 ++
 src/wayland-client-glib.h |  46 +++
 src/wayland-client-glib.pc.in |   9 +++
 5 files changed, 275 insertions(+), 1 deletion(-)
 create mode 100644 src/wayland-client-glib.c
 create mode 100644 src/wayland-client-glib.h
 create mode 100644 src/wayland-client-glib.pc.in

diff --git a/configure.ac b/configure.ac
index fa924ae..ca50c65 100644
--- a/configure.ac
+++ b/configure.ac
@@ -50,7 +50,7 @@ AC_CHECK_DECL(TFD_CLOEXEC,[],
 AC_CHECK_DECL(CLOCK_MONOTONIC,[],
  [AC_MSG_ERROR("CLOCK_MONOTONIC is needed to compile wayland")],
  [[#include ]])
-AC_CHECK_HEADERS([execinfo.h])
+AC_CHECK_HEADERS([execinfo.h errno.h])
 
 AC_ARG_ENABLE([scanner],
   [AC_HELP_STRING([--disable-scanner],
@@ -126,6 +126,25 @@ if test "x$enable_documentation" = "xyes"; then
 fi
 AM_CONDITIONAL([HAVE_PUBLICAN], [test "x$PUBLICAN" != "x"])
 
+
+# GLib main loop integration library
+
+glib_min_major="2"
+glib_min_minor="36"
+glib_min_version="${glib_min_major}.${glib_min_minor}"
+
+AC_ARG_ENABLE([glib],
+  [AC_HELP_STRING([--enable-glib],
+  [Enable GLib main loop integration library])],
+  [],
+  [enable_glib=no])
+if test "x$enable_glib" = "xyes"; then
+   PKG_CHECK_MODULES(GLIB, [glib-2.0 >= $glib_min_version])
+   AC_DEFINE_UNQUOTED([GLIB_VERSION_MIN_REQUIRED], 
[(G_ENCODE_VERSION(${glib_min_major},${glib_min_minor}))], [The lower GLib 
version supported])
+fi
+AM_CONDITIONAL([ENABLE_GLIB], [test "x$enable_glib" = "xyes"])
+
+
 AC_CONFIG_FILES([Makefile
 cursor/Makefile
 cursor/wayland-cursor.pc
@@ -140,6 +159,7 @@ AC_CONFIG_FILES([Makefile
 src/wayland-scanner-uninstalled.pc
 src/wayland-server.pc
 src/wayland-client.pc
+src/wayland-client-glib.pc
 src/wayland-scanner.pc
 src/wayland-version.h
 protocol/Makefile
diff --git a/src/Makefile.am b/src/Makefile.am
index 4226f63..9026e5a 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -28,6 +28,8 @@ libwayland_server_la_SOURCES =\
 
 libwayland_client_la_LIBADD = $(FFI_LIBS) libwayland-util.la -lrt -lm
 libwayland_client_la_LDFLAGS = -version-info 1:0:1
+libwayland_client_la_CFLAGS = $(AM_CFLAGS)
+libwayland_client_la_CPPFLAGS = $(AM_CPPFLAGS)
 libwayland_client_la_SOURCES = \
wayland-protocol.c  \
wayland-client.c
@@ -35,6 +37,21 @@ libwayland_client_la_SOURCES =   \
 pkgconfigdir = $(libdir)/pkgconfig
 pkgconfig_DATA = wayland-client.pc wayland-server.pc
 
+if ENABLE_GLIB
+pkgconfig_DATA += wayland-client-glib.pc
+
+include_HEADERS += \
+   wayland-client-glib.h
+
+libwayland_client_la_SOURCES += \
+   wayland-client-glib.h   \
+   wayland-client-glib.c
+
+libwayland_client_la_CPPFLAGS += -DG_LOG_DOMAIN=\"GWayland\"
+libwayland_client_la_CFLAGS += $(GLIB_CFLAGS)
+libwayland_client_la_LIBADD += $(GLIB_LIBS)
+endif
+
 AM_CPPFLAGS = $(FFI_CFLAGS)
 AM_CFLAGS = $(GCC_CFLAGS)
 
diff --git a/src/wayland-client-glib.c b/src/wayland-client-glib.c
new file mode 100644
index 000..1577abb
--- /dev/null
+++ b/src/wayland-client-glib.c
@@ -0,0 +1,182 @@
+/*
+ * wayland-client-glib - Wayland integration to GLib main loop
+ *
+ * Copyright © 2012-2013 Quentin "Sardem FF7" Glidic
+ *
+ * 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

Re: [PATCH] wayland: Don't race when releasing named buffers

2013-10-23 Thread Jonas Ådahl
Still needs review.

On Wed, Oct 2, 2013 at 5:06 PM, Jonas Ådahl  wrote:
> This patch fixes a race when a client releases a named buffer before the
> server had the time to handle 'wl_drm_create_buffer'. When triggered,
> the server would fail to create the buffer since the client already
> having destroyed it, throwing out the client in the process with a
> protocol error.
>
> To solve this, use a lazy non-blocking roundtrip when creating the
> buffer that will only potentially block when releasing if the roundtrip
> callback was not already invoked.
>
> Signed-off-by: Jonas Ådahl 
> ---
>  src/egl/drivers/dri2/egl_dri2.h |  1 +
>  src/egl/drivers/dri2/platform_wayland.c | 74 
> -
>  2 files changed, 73 insertions(+), 2 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
> index fba5f81..6508612 100644
> --- a/src/egl/drivers/dri2/egl_dri2.h
> +++ b/src/egl/drivers/dri2/egl_dri2.h
> @@ -189,6 +189,7 @@ struct dri2_egl_surface
>struct wl_buffer   *wl_buffer;
>__DRIimage *dri_image;
>int pitch, name;
> +  struct wl_callback *created_callback;
>  #endif
>  #ifdef HAVE_DRM_PLATFORM
>struct gbm_bo   *bo;
> diff --git a/src/egl/drivers/dri2/platform_wayland.c 
> b/src/egl/drivers/dri2/platform_wayland.c
> index 1d417bb..3a873f7 100644
> --- a/src/egl/drivers/dri2/platform_wayland.c
> +++ b/src/egl/drivers/dri2/platform_wayland.c
> @@ -184,6 +184,55 @@ dri2_create_window_surface(_EGLDriver *drv, _EGLDisplay 
> *disp,
>   window, attrib_list);
>  }
>
> +static void
> +buffer_create_sync_callback(void *data,
> +struct wl_callback *callback,
> +uint32_t serial)
> +{
> +   struct wl_callback **created_callback = data;
> +
> +   *created_callback = NULL;
> +   wl_callback_destroy(callback);
> +}
> +
> +static const struct wl_callback_listener buffer_create_sync_listener = {
> +   buffer_create_sync_callback
> +};
> +
> +static void
> +lazy_create_buffer_roundtrip(struct dri2_egl_display *dri2_dpy,
> + struct dri2_egl_surface *dri2_surf)
> +{
> +   struct wl_callback *callback;
> +
> +   callback = wl_display_sync(dri2_dpy->wl_dpy);
> +   dri2_surf->current->created_callback = callback;
> +
> +   wl_callback_add_listener(callback,
> +&buffer_create_sync_listener,
> +&dri2_surf->current->created_callback);
> +   wl_proxy_set_queue((struct wl_proxy *) callback, dri2_dpy->wl_queue);
> +}
> +
> +static int
> +synchronize_create_buffer(struct dri2_egl_display *dri2_dpy,
> +  struct dri2_egl_surface *dri2_surf,
> +  int i)
> +{
> +   int ret = 0;
> +
> +   while (ret != -1 && dri2_surf->color_buffers[i].created_callback)
> +  ret = wl_display_dispatch_queue(dri2_dpy->wl_dpy, dri2_dpy->wl_queue);
> +
> +   if (dri2_surf->color_buffers[i].created_callback) {
> +  wl_callback_destroy(dri2_surf->color_buffers[i].created_callback);
> +  dri2_surf->color_buffers[i].created_callback = NULL;
> +  return -1;
> +   }
> +
> +   return 0;
> +}
> +
>  /**
>   * Called via eglDestroySurface(), drv->API.DestroySurface().
>   */
> @@ -206,6 +255,8 @@ dri2_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp, 
> _EGLSurface *surf)
>   wl_buffer_destroy(dri2_surf->color_buffers[i].wl_buffer);
>if (dri2_surf->color_buffers[i].dri_image)
>   
> dri2_dpy->image->destroyImage(dri2_surf->color_buffers[i].dri_image);
> +  if (dri2_surf->color_buffers[i].created_callback)
> + wl_callback_destroy(dri2_surf->color_buffers[i].created_callback);
> }
>
> for (i = 0; i < __DRI_BUFFER_COUNT; i++)
> @@ -227,7 +278,7 @@ dri2_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp, 
> _EGLSurface *surf)
> return EGL_TRUE;
>  }
>
> -static void
> +static int
>  dri2_release_buffers(struct dri2_egl_surface *dri2_surf)
>  {
> struct dri2_egl_display *dri2_dpy =
> @@ -235,6 +286,11 @@ dri2_release_buffers(struct dri2_egl_surface *dri2_surf)
> int i;
>
> for (i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
> +  if (dri2_surf->color_buffers[i].created_callback) {
> + if (synchronize_create_buffer(dri2_dpy, dri2_surf, i) != 0)
> +return -1;
> +  }
> +
>if (dri2_surf->color_buffers[i].wl_buffer &&
>!dri2_surf->color_buffers[i].locked)
>   wl_buffer_destroy(dri2_surf->color_buffers[i].wl_buffer);
> @@ -251,6 +307,8 @@ dri2_release_buffers(struct dri2_egl_surface *dri2_surf)
>dri2_surf->dri_buffers[i]->attachment != __DRI_BUFFER_BACK_LEFT)
>   dri2_dpy->dri2->releaseBuffer(dri2_dpy->dri_screen,
> dri2_surf->dri_buffers[i]);
> +
> +   return 0;
>  }
>
>  static int
> @@ -349,7 +407,10 @@ dri2_get_buffers_with_format(__DRId