Re: [PATCH 6/6] window: Add a simple getenv to force SHM rendering
On Mon, Apr 28, 2014 at 11:19:32AM -0400, Jasper St. Pierre wrote: Yeah, that's useful, applied. Kristian > --- > clients/window.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/clients/window.c b/clients/window.c > index d822af7..3897440 100644 > --- a/clients/window.c > +++ b/clients/window.c > @@ -4338,11 +4338,11 @@ surface_create(struct window *window) > return surface; > } > > -static window_buffer_type > +static enum window_buffer_type > get_preferred_buffer_type(struct display *display) > { > #ifdef HAVE_CAIRO_EGL > - if (display->argb_device) > + if (display->argb_device && !getenv("TOYTOOLKIT_NO_EGL")) > return WINDOW_BUFFER_TYPE_EGL_WINDOW; > #endif > > -- > 1.9.0 > > ___ > 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 4/6] cairo-util: Don't show a resize cursor on edges when we're maximized
On Mon, Apr 28, 2014 at 11:19:30AM -0400, Jasper St. Pierre wrote: > This is substantially confusing to users, namely me. That is indeed confusing. However, I edited the patch to just set grip_size to 0 if we're maximized. Kristian > --- > shared/cairo-util.c | 13 - > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/shared/cairo-util.c b/shared/cairo-util.c > index a1568ff..a77d0b6 100644 > --- a/shared/cairo-util.c > +++ b/shared/cairo-util.c > @@ -487,11 +487,14 @@ enum theme_location > theme_get_location(struct theme *t, int x, int y, > int width, int height, int flags) > { > + int maximized; > int vlocation, hlocation, location; > const int grip_size = 8; > int margin, top_margin; > > - margin = (flags & THEME_FRAME_MAXIMIZED) ? 0 : t->margin; > + maximized = (flags & THEME_FRAME_MAXIMIZED); > + > + margin = maximized ? 0 : t->margin; > > if (flags & THEME_FRAME_NO_TITLE) > top_margin = t->width; > @@ -500,22 +503,22 @@ theme_get_location(struct theme *t, int x, int y, > > if (x < margin) > hlocation = THEME_LOCATION_EXTERIOR; > - else if (x < margin + grip_size) > + else if (!maximized && x < margin + grip_size) > hlocation = THEME_LOCATION_RESIZING_LEFT; > else if (x < width - margin - grip_size) > hlocation = THEME_LOCATION_INTERIOR; > - else if (x < width - margin) > + else if (!maximized && x < width - margin) > hlocation = THEME_LOCATION_RESIZING_RIGHT; > else > hlocation = THEME_LOCATION_EXTERIOR; > > if (y < margin) > vlocation = THEME_LOCATION_EXTERIOR; > - else if (y < margin + grip_size) > + else if (!maximized && y < margin + grip_size) > vlocation = THEME_LOCATION_RESIZING_TOP; > else if (y < height - margin - grip_size) > vlocation = THEME_LOCATION_INTERIOR; > - else if (y < height - margin) > + else if (!maximized && y < height - margin) > vlocation = THEME_LOCATION_RESIZING_BOTTOM; > else > vlocation = THEME_LOCATION_EXTERIOR; > -- > 1.9.0 > > ___ > 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 1/6] compositor: Fix the documentation for surface->configure
On Mon, Apr 28, 2014 at 11:19:27AM -0400, Jasper St. Pierre wrote: > It's called on commit, not on attach. Additionally, correct the > interface name to be wl_surface, not surface. > --- > src/compositor.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/compositor.h b/src/compositor.h > index 03d8992..c913f54 100644 > --- a/src/compositor.h > +++ b/src/compositor.h > @@ -908,9 +908,9 @@ struct weston_surface { > } pending; > > /* > - * If non-NULL, this function will be called on surface::attach after > + * If non-NULL, this function will be called on wl_surface::commit after >* a new buffer has been set up for this surface. The integer params > - * are the sx and sy paramerters supplied to surface::attach . > + * are the sx and sy paramerters supplied to wl_surface::attach . Applied and I squashed in a typo fix for "paramerters". Kristian >*/ > void (*configure)(struct weston_surface *es, int32_t sx, int32_t sy); > void *configure_private; > -- > 1.9.0 > > ___ > 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 v2] screen-share: Don't unset server environment variables
On Wed, Apr 30, 2014 at 10:53:30AM -0500, Jason Ekstrand wrote: > Looks good to me. > > Reviewed-by: Jason Ekstrand Thanks, committed. Kristian > On Apr 30, 2014 3:52 AM, "Andrew Wedgbury" > wrote: > > > There is no need to unset WAYLAND_DISPLAY and WAYLAND_SOCKET when > > screen-share > > launches the fullscreen shell server. This was done originally in case the > > launched server decided to use the wayland backend based on the presence of > > these. However, we pass a command line argument telling it to use the RDP > > backend, which overrides the automatic backend selection based on the > > environment. > > > > Keeping these environment variables allows the launched fullscreen shell > > server > > to know the original server's display name, which it may need in order to > > show > > a configuration UI. > > > > Signed-off-by: Andrew Wedgbury > > --- > > src/screen-share.c | 4 > > 1 file changed, 4 deletions(-) > > > > diff --git a/src/screen-share.c b/src/screen-share.c > > index d3e3f05..6f60b81 100644 > > --- a/src/screen-share.c > > +++ b/src/screen-share.c > > @@ -1005,10 +1005,6 @@ weston_output_share(struct weston_output *output, > > } > > > > if (pid == 0) { > > - /* We don't want anything circular */ > > - unsetenv("WAYLAND_DISPLAY"); > > - unsetenv("WAYLAND_SOCKET"); > > - > > /* do not give our signal mask to the new process */ > > sigfillset(&allsigs); > > sigprocmask(SIG_UNBLOCK, &allsigs, NULL); > > -- > > 1.9.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 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2] screen-share: Don't unset server environment variables
Looks good to me. Reviewed-by: Jason Ekstrand On Apr 30, 2014 3:52 AM, "Andrew Wedgbury" wrote: > There is no need to unset WAYLAND_DISPLAY and WAYLAND_SOCKET when > screen-share > launches the fullscreen shell server. This was done originally in case the > launched server decided to use the wayland backend based on the presence of > these. However, we pass a command line argument telling it to use the RDP > backend, which overrides the automatic backend selection based on the > environment. > > Keeping these environment variables allows the launched fullscreen shell > server > to know the original server's display name, which it may need in order to > show > a configuration UI. > > Signed-off-by: Andrew Wedgbury > --- > src/screen-share.c | 4 > 1 file changed, 4 deletions(-) > > diff --git a/src/screen-share.c b/src/screen-share.c > index d3e3f05..6f60b81 100644 > --- a/src/screen-share.c > +++ b/src/screen-share.c > @@ -1005,10 +1005,6 @@ weston_output_share(struct weston_output *output, > } > > if (pid == 0) { > - /* We don't want anything circular */ > - unsetenv("WAYLAND_DISPLAY"); > - unsetenv("WAYLAND_SOCKET"); > - > /* do not give our signal mask to the new process */ > sigfillset(&allsigs); > sigprocmask(SIG_UNBLOCK, &allsigs, NULL); > -- > 1.9.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
[PATCH weston 3/3] weston-info: Display the name from the new wl_output.name event
If the compositor supports version 3 of the wl_output interface and sends a name event then this will now be displayed in the info. --- clients/weston-info.c | 39 ++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/clients/weston-info.c b/clients/weston-info.c index df869e3..05edd21 100644 --- a/clients/weston-info.c +++ b/clients/weston-info.c @@ -59,6 +59,8 @@ struct output_info { struct wl_output *output; + char *name; + struct { int32_t x, y; int32_t physical_width, physical_height; @@ -160,6 +162,9 @@ print_output_info(void *data) print_global_info(data); + if (output->name) + printf("\tname: %s\n", output->name); + switch (output->geometry.subpixel) { case WL_OUTPUT_SUBPIXEL_UNKNOWN: subpixel_orientation = "unknown"; @@ -423,9 +428,35 @@ output_handle_mode(void *data, struct wl_output *wl_output, wl_list_insert(output->modes.prev, &mode->link); } +static void +output_handle_name(void *data, +struct wl_output *wl_output, +const char *name) +{ + struct output_info *output = data; + + output->name = xstrdup(name); +} + +static void +output_handle_done(void *data, + struct wl_output *wl_output) +{ +} + +static void +output_handle_scale(void *data, + struct wl_output *wl_output, + int32_t factor) +{ +} + static const struct wl_output_listener output_listener = { output_handle_geometry, output_handle_mode, + output_handle_done, + output_handle_scale, + output_handle_name, }; static void @@ -440,6 +471,8 @@ destroy_output_info(void *data) free(output->geometry.make); if (output->geometry.model != NULL) free(output->geometry.model); + if (output->name != NULL) + free(output->name); wl_list_for_each_safe(mode, tmp, &output->modes, link) { wl_list_remove(&mode->link); @@ -458,8 +491,12 @@ add_output_info(struct weston_info *info, uint32_t id, uint32_t version) wl_list_init(&output->modes); + if (version > 3) + version = 3; + output->output = wl_registry_bind(info->registry, id, - &wl_output_interface, 1); + &wl_output_interface, + version); wl_output_add_listener(output->output, &output_listener, output); -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 2/3] Send the new wl_output.name event when binding an output
This version for the wl_output interface has been changed to 3 and it now sends out the name event when a client binds an output. --- src/compositor.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/compositor.c b/src/compositor.c index ee8dc24..6a333df 100644 --- a/src/compositor.c +++ b/src/compositor.c @@ -3081,6 +3081,9 @@ bind_output(struct wl_client *client, mode->refresh); } + if (version >= 3 && output->name) + wl_output_send_name(resource, output->name); + if (version >= 2) wl_output_send_done(resource); } @@ -3321,7 +3324,7 @@ weston_output_init(struct weston_output *output, struct weston_compositor *c, output->compositor->output_id_pool |= 1 << output->id; output->global = - wl_global_create(c->wl_display, &wl_output_interface, 2, + wl_global_create(c->wl_display, &wl_output_interface, 3, output, bind_output); wl_signal_emit(&c->output_created_signal, output); } -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH 1/3] protocol: Add an event to specify the name of an output
This bumps the version of the wl_output interface to 3 and adds a separate event to report the output's name. --- protocol/wayland.xml | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/protocol/wayland.xml b/protocol/wayland.xml index 330f8ab..60fa81e 100644 --- a/protocol/wayland.xml +++ b/protocol/wayland.xml @@ -1645,7 +1645,7 @@ - + An output describes part of the compositor geometry. The compositor works in the 'compositor coordinate system' and an @@ -1783,6 +1783,17 @@ + + + + + +The name event contains a human-readable name for the output. +If the output has a name then this event is sent after binding +to the output object. + + + -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] shell: Don't map surfaces of type SHELL_SURFACE_NONE
When commit 07926d90 factored out the code that chooses in which layer a surface is added to, it changed the behavior for surfaces with no type. Instead of not adding it to any layer, the surface is added to the current workspace. This patch restores the old behavior. https://bugs.freedesktop.org/show_bug.cgi?id=77527 --- desktop-shell/shell.c | 27 ++- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c index 7631f1b..7e4ceed 100644 --- a/desktop-shell/shell.c +++ b/desktop-shell/shell.c @@ -2127,6 +2127,12 @@ shell_surface_calculate_layer_link (struct shell_surface *shsurf) struct weston_view *parent; switch (shsurf->type) { + case SHELL_SURFACE_XWAYLAND: + return &shsurf->shell->fullscreen_layer.view_list; + + case SHELL_SURFACE_NONE: + return NULL; + case SHELL_SURFACE_POPUP: case SHELL_SURFACE_TOPLEVEL: if (shsurf->state.fullscreen && !shsurf->state.lowered) { @@ -2142,22 +2148,15 @@ shell_surface_calculate_layer_link (struct shell_surface *shsurf) if (parent) return parent->layer_link.prev; } - break; - case SHELL_SURFACE_XWAYLAND: - return &shsurf->shell->fullscreen_layer.view_list; - - case SHELL_SURFACE_NONE: - default: - /* Go to the fallback, below. */ - break; + /* Move the surface to a normal workspace layer so that surfaces +* which were previously fullscreen or transient are no longer +* rendered on top. */ + ws = get_current_workspace(shsurf->shell); + return &ws->layer.view_list; } - /* Move the surface to a normal workspace layer so that surfaces -* which were previously fullscreen or transient are no longer -* rendered on top. */ - ws = get_current_workspace(shsurf->shell); - return &ws->layer.view_list; + assert(0 && "Unknown shell surface type"); } static void @@ -2198,6 +2197,8 @@ shell_surface_update_layer(struct shell_surface *shsurf) new_layer_link = shell_surface_calculate_layer_link(shsurf); + if (new_layer_link == NULL) + return; if (new_layer_link == &shsurf->view->layer_link) return; -- 1.8.3.2 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH] xwayland: free xcb reply value when malloc returns null
Signed-off-by: vivek --- xwayland/selection.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/xwayland/selection.c b/xwayland/selection.c index b694477..d1f6361 100644 --- a/xwayland/selection.c +++ b/xwayland/selection.c @@ -186,8 +186,10 @@ weston_wm_get_selection_targets(struct weston_wm *wm) } source = malloc(sizeof *source); - if (source == NULL) + if (source == NULL) { + free(reply); return; + } wl_signal_init(&source->base.destroy_signal); source->base.accept = data_source_accept; -- 1.7.9.5 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v2] screen-share: Don't unset server environment variables
There is no need to unset WAYLAND_DISPLAY and WAYLAND_SOCKET when screen-share launches the fullscreen shell server. This was done originally in case the launched server decided to use the wayland backend based on the presence of these. However, we pass a command line argument telling it to use the RDP backend, which overrides the automatic backend selection based on the environment. Keeping these environment variables allows the launched fullscreen shell server to know the original server's display name, which it may need in order to show a configuration UI. Signed-off-by: Andrew Wedgbury --- src/screen-share.c | 4 1 file changed, 4 deletions(-) diff --git a/src/screen-share.c b/src/screen-share.c index d3e3f05..6f60b81 100644 --- a/src/screen-share.c +++ b/src/screen-share.c @@ -1005,10 +1005,6 @@ weston_output_share(struct weston_output *output, } if (pid == 0) { - /* We don't want anything circular */ - unsetenv("WAYLAND_DISPLAY"); - unsetenv("WAYLAND_SOCKET"); - /* do not give our signal mask to the new process */ sigfillset(&allsigs); sigprocmask(SIG_UNBLOCK, &allsigs, NULL); -- 1.9.2 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] client: extend error handling
On Wed, 30 Apr 2014 09:40:19 +0200 Marek Chalupa wrote: > On 24 April 2014 14:45, Pekka Paalanen wrote: > > > On Wed, 23 Apr 2014 14:39:48 +0200 > > Marek Chalupa wrote: > > > > > +/** > > > + * This function is called for error events > > > + * and idicates that in some object occured an error. > > > + * Difference between this function and display_fatal_error() > > > + * is that this one handles errors that will come in wire, whereas > > > + * display_fatal_error() is called for local errors. > > > + * > > > + * \param display > > > + * \param codeerror code > > > + * \param id id of the object that generated the error > > > + * \param intfprotocol interface > > > + */ > > > static void > > > -wl_display_fatal_error(struct wl_display *display, int error) > > > +display_protocol_error(struct wl_display *display, int code, > > > +unsigned int id, const struct wl_interface *intf) > > > { > > > + struct wl_event_queue *iter; > > > + int err; > > > + > > > + /* set correct errno */ > > > + if (wl_interface_equal(intf, &wl_display_interface)) { > > > + switch (code) { > > > + case WL_DISPLAY_ERROR_INVALID_OBJECT: > > > + case WL_DISPLAY_ERROR_INVALID_METHOD: > > > + err = EINVAL; > > > + break; > > > + case WL_DISPLAY_ERROR_NO_MEMORY: > > > + err = ENOMEM; > > > + break; > > > + default: > > > + err = EFAULT; > > > + } > > > + } else { > > > + err = EPROTO; > > > + } > > > + > > > + if (display->last_error) > > > + return; > > > > This 'if' could be the very first thing. Doesn't matter. > > > > > + > > > pthread_mutex_lock(&display->mutex); > > > - display_fatal_error(display, error); > > > + > > > + display->last_error = err; > > > + > > > + display->protocol_error.code = code; > > > + display->protocol_error.id = id; > > > + display->protocol_error.interface = intf; > > > + > > > + wl_list_for_each(iter, &display->event_queue_list, link) > > > + pthread_cond_broadcast(&iter->cond); > > > > What is the purpose of this pthread_cond_broadcast()? I am not too > > familiar with the logic here, but looks like this is something new you > > added, and I'm not sure it belongs with the addition of > > wl_display_get_protocol_error(). > > > > If this is fixing something, I feel like it should be a separate patch, > > since I can't see how adding wl_display_get_protocol_error() would add > > the need for this. Either the need was there to begin with and this > > bug fix should get into the stable releases, or it's not needed. Am I > > missing something? > > > > > This broadcast is not new, so I didn't touch it. So far as I see into it it I was confused about it because your patch never removes any lines doing pthread_cond_broadcast(), but it does add it. Ok, now I see it. Originally, display_fatal_error() does it, and wl_display_fatal_error() called it. Your patch renames wl_display_fatal_error to display_protocol_error and removes the call to display_fatal_error(), so you copy back the relevant parts of display_fatal_error(). > should > wake up all the sleeping event queues (when there's more of them than the > default one - in multi-threaded environment) so that they can receive the > error > and notify the client. > > relevant commits: > 33b7637b4500a682018b503837b8aca9afae36f2 > 385fe30e8b144a968aa88c6546c2ef247771b3d7 > > However, I don't see any pthread_cond_{wait, timedwait} for this condition > anywhere in the code and that's weird... Marking the 'cond' member deprecated, the compiler tells me it is used in the following places in current master: CC src/libwayland_client_la-wayland-client.lo src/wayland-client.c: In function 'display_fatal_error': src/wayland-client.c:113:3: warning: 'cond' is deprecated (declared at src/wayland-client.c:75) [-Wdeprecated-declarations] ** pthread_cond_broadcast(&iter->cond); src/wayland-client.c: In function 'wl_event_queue_init': src/wayland-client.c:128:2: warning: 'cond' is deprecated (declared at src/wayland-client.c:75) [-Wdeprecated-declarations] ** pthread_cond_init(&queue->cond, NULL); src/wayland-client.c: In function 'wl_event_queue_release': src/wayland-client.c:143:2: warning: 'cond' is deprecated (declared at src/wayland-client.c:75) [-Wdeprecated-declarations] ** pthread_cond_destroy(&queue->cond); src/wayland-client.c: In function 'queue_event': src/wayland-client.c:979:3: warning: 'cond' is deprecated (declared at src/wayland-client.c:75) [-Wdeprecated-declarations] ** pthread_cond_signal(&queue->cond); You're right, and I cannot see a related mutex, either. Maybe it is unneeded or we have a hard to trigger bug here. Nice find. Thanks, pq ___ wayland-devel mailing list wayland-devel@lis
[PATCH v3] client: extend error handling
When an error occurs, wl_display_get_error() does not provide any way of getting know if it was a local error or if it was an error event, respectively what object caused the error and what the error was. This patch introduces a new function wl_display_get_protocol_error() which will return error code, interface and id of the object that generated the error. wl_display_get_error() will work the same way as before. wl_display_get_protocol_error() DOES NOT indicate that a non-protocol error happened. It returns valid information only in that case that (protocol) error occurred, so it should be used after calling wl_display_get_error() with positive result. Thanks to Pekka Paalanen for pointing out issues in the first versions of this patch. --- src/wayland-client.c | 138 --- src/wayland-client.h | 3 ++ 2 files changed, 123 insertions(+), 18 deletions(-) diff --git a/src/wayland-client.c b/src/wayland-client.c index bd40313..8dd8804 100644 --- a/src/wayland-client.c +++ b/src/wayland-client.c @@ -78,7 +78,24 @@ struct wl_event_queue { struct wl_display { struct wl_proxy proxy; struct wl_connection *connection; + + /* errno of the last wl_display error */ int last_error; + + /* When display gets an error event from some object, it stores +* information about it here, so that client can get this +* information afterwards */ + struct { + /* Code of the error. It can be compared to +* the interface's errors enumeration. */ + int code; + /* interface (protocol) in which the error occurred */ + const struct wl_interface *interface; + /* id of the proxy that caused the error. There's no warranty +* that the proxy is still valid. It's up to client how it will +* use it */ + uint32_t id; + } protocol_error; int fd; pthread_t display_thread; struct wl_map objects; @@ -96,6 +113,14 @@ struct wl_display { static int debug_client = 0; +/** + * This function is called for local errors (no memory, server hung up) + * + * \param display + * \param errorerror value (EINVAL, EFAULT, ...) + * + * \note this function is called with display mutex locked + */ static void display_fatal_error(struct wl_display *display, int error) { @@ -105,7 +130,7 @@ display_fatal_error(struct wl_display *display, int error) return; if (!error) - error = 1; + error = EFAULT; display->last_error = error; @@ -113,11 +138,56 @@ display_fatal_error(struct wl_display *display, int error) pthread_cond_broadcast(&iter->cond); } +/** + * This function is called for error events + * and idicates that in some object occured an error. + * Difference between this function and display_fatal_error() + * is that this one handles errors that will come in wire, whereas + * display_fatal_error() is called for local errors. + * + * \param display + * \param codeerror code + * \param id id of the object that generated the error + * \param intfprotocol interface + */ static void -wl_display_fatal_error(struct wl_display *display, int error) +display_protocol_error(struct wl_display *display, int code, + unsigned int id, const struct wl_interface *intf) { + struct wl_event_queue *iter; + int err; + + if (display->last_error) + return; + + /* set correct errno */ + if (wl_interface_equal(intf, &wl_display_interface)) { + switch (code) { + case WL_DISPLAY_ERROR_INVALID_OBJECT: + case WL_DISPLAY_ERROR_INVALID_METHOD: + err = EINVAL; + break; + case WL_DISPLAY_ERROR_NO_MEMORY: + err = ENOMEM; + break; + default: + err = EFAULT; + } + } else { + err = EPROTO; + } + pthread_mutex_lock(&display->mutex); - display_fatal_error(display, error); + + display->last_error = err; + + display->protocol_error.code = code; + display->protocol_error.id = id; + display->protocol_error.interface = intf; + + wl_list_for_each(iter, &display->event_queue_list, link) + pthread_cond_broadcast(&iter->cond); + pthread_mutex_unlock(&display->mutex); } @@ -579,25 +649,12 @@ display_handle_error(void *data, uint32_t code, const char *message) { struct wl_proxy *proxy = object; - int err; wl_log("%s@%u: error %d: %s\n", proxy->object.interface->name, proxy->object.id, code, message); - switch (code) { - case WL_DISPLAY_ERROR_INVALID_OBJECT: - case WL_DISPLAY_ERROR_INVALID_METHOD: -
Re: [PATCH] client: extend error handling
On 24 April 2014 14:45, Pekka Paalanen wrote: > On Wed, 23 Apr 2014 14:39:48 +0200 > Marek Chalupa wrote: > > > When an error occurres, than wl_display_get_error() do not > > ... occurs, [then] ... does not > > > provide any way of getting know if it was a local error or if it was > > an error event, respectively what object caused the error and what > > the error was. > > > > This patch introduces a new function wl_display_get_protocol_error() > > which will return error code, interface and id of the object that > > generated the error. > > wl_display_get_error() will work the same way as before. > > > > wl_display_get_protcol_error() DO NOT indicate that the error happed. > > +o, a non-protocol error happened? > > > It returns valid information only in that case that (protocol) error > > occurred, so it should be used after calling wl_display_get_error() > > with positive result. > > > > Thanks to Pekka Paalanen for pointing out > > issues in the first versions of this patch. > > --- > > src/wayland-client.c | 138 > --- > > src/wayland-client.h | 3 ++ > > 2 files changed, 123 insertions(+), 18 deletions(-) > > > > diff --git a/src/wayland-client.c b/src/wayland-client.c > > index bd40313..f8146e7 100644 > > --- a/src/wayland-client.c > > +++ b/src/wayland-client.c > > @@ -78,7 +78,24 @@ struct wl_event_queue { > > struct wl_display { > > struct wl_proxy proxy; > > struct wl_connection *connection; > > + > > + /* errno of the last wl_display error */ > > int last_error; > > + > > + /* When display gets an error event from some object, it stores > > + * information about it here, so that client can get this > > + * information afterwards */ > > + struct { > > + /* Code of the error. It can be compared to > > + * the interface's errors enumeration. */ > > + int code; > > + /* interface (protocol) in wich the error occured */ > > which, occurred > > > + const struct wl_interface *interface; > > + /* id of the proxy that caused the error. There's no > warranty > > + * that the proxy is still valid. It's up to client how it > will > > + * use it */ > > + unsigned int id; > > I think we use uint32_t for ids specifically. > Yes, uint32_t, I'll fix that. > > > + } protocol_error; > > int fd; > > pthread_t display_thread; > > struct wl_map objects; > > @@ -96,6 +113,14 @@ struct wl_display { > > > > static int debug_client = 0; > > > > +/** > > + * This function is called for local errors (no memory, server hung up) > > + * > > + * \param display > > + * \param errorerror value (EINVAL, EFAULT, ...) > > + * > > + * \note this function is called with display mutex locked > > + */ > > static void > > display_fatal_error(struct wl_display *display, int error) > > { > > @@ -105,7 +130,7 @@ display_fatal_error(struct wl_display *display, int > error) > > return; > > > > if (!error) > > - error = 1; > > + error = EFAULT; > > Yup, makes sense. Better EFAULT than EPERM for the case where errno is > still 0, I guess. :-) > > > > > display->last_error = error; > > > > @@ -113,11 +138,56 @@ display_fatal_error(struct wl_display *display, > int error) > > pthread_cond_broadcast(&iter->cond); > > } > > > > +/** > > + * This function is called for error events > > + * and idicates that in some object occured an error. > > + * Difference between this function and display_fatal_error() > > + * is that this one handles errors that will come in wire, whereas > > + * display_fatal_error() is called for local errors. > > + * > > + * \param display > > + * \param codeerror code > > + * \param id id of the object that generated the error > > + * \param intfprotocol interface > > + */ > > static void > > -wl_display_fatal_error(struct wl_display *display, int error) > > +display_protocol_error(struct wl_display *display, int code, > > +unsigned int id, const struct wl_interface *intf) > > { > > + struct wl_event_queue *iter; > > + int err; > > + > > + /* set correct errno */ > > + if (wl_interface_equal(intf, &wl_display_interface)) { > > + switch (code) { > > + case WL_DISPLAY_ERROR_INVALID_OBJECT: > > + case WL_DISPLAY_ERROR_INVALID_METHOD: > > + err = EINVAL; > > + break; > > + case WL_DISPLAY_ERROR_NO_MEMORY: > > + err = ENOMEM; > > + break; > > + default: > > + err = EFAULT; > > + } > > + } else { > > + err = EPROTO; > > + } > > + > > + if (display->last_error) > > + return; > > This 'if' could be the very first thing. Doesn't matter. > > > + > > pthread_mutex_lock(&disp
Re: [PATCH 6/6] window: Add a simple getenv to force SHM rendering
On Tue, 29 Apr 2014 16:18:33 -0400 "Jasper St. Pierre" wrote: > Can we at least do this work independently of this patch? I was debugging > EGL stack issues, so I figured I might as well give my envvar hack to debug > SHM issues. Of course, I never intended to block this patch. I just wanted to start a conversation about cairo-egl in toytoolkit in general. > On Tue, Apr 29, 2014 at 4:07 PM, Kristian Høgsberg wrote: > > > On Tue, Apr 29, 2014 at 5:35 AM, Pekka Paalanen > > wrote: > > > On Mon, 28 Apr 2014 11:19:32 -0400 > > > "Jasper St. Pierre" wrote: > > > > > >> --- > > >> clients/window.c | 4 ++-- > > >> 1 file changed, 2 insertions(+), 2 deletions(-) > > >> > > >> diff --git a/clients/window.c b/clients/window.c > > >> index d822af7..3897440 100644 > > >> --- a/clients/window.c > > >> +++ b/clients/window.c > > >> @@ -4338,11 +4338,11 @@ surface_create(struct window *window) > > >> return surface; > > >> } > > >> > > >> -static window_buffer_type > > >> +static enum window_buffer_type > > >> get_preferred_buffer_type(struct display *display) > > >> { > > >> #ifdef HAVE_CAIRO_EGL > > >> - if (display->argb_device) > > >> + if (display->argb_device && !getenv("TOYTOOLKIT_NO_EGL")) > > >> return WINDOW_BUFFER_TYPE_EGL_WINDOW; > > >> #endif > > >> > > > > > > Nice. > > > > > > I wonder, would it be time to finally just drop cairo-egl completely? > > > It does raise the question on what to do with weston-gears and > > > weston-screensaver. ISTR that krh was not fond of using sub-surfaces > > > for them, so... just turn them into simple-egl kind of clients? Did we > > > already have the decorations code easily integrable to non-cairo GL > > > apps, maybe stemming from the wayland-backend work? > > > > I'd like to drop cairo-egl, but not in a way that makes window.c shm > > only. I'd like it to use GL by default and use cairo for rendering > > assets, for example, render the frame in cairo, but use gl to stretch > > and scale it and composite the title on top. Any particular reason why you want to do that? For all toytoolkit apps that do not explicitly need GL, I see using GL as an unneeded overhead. That is especially true for e.g. weston-desktop-shell and the VKB apps which run always. You would still keep the shm-only path as an alternative, right? That would mean no simplification inside toytoolkit as a bonus from getting rid of cairo-egl. If toytookit uses GL or GLESv2, we won't get rid of the incompatibility we have right now, where if you compile with cairo-gles, you simply cannot run weston-gears or weston-screensaver, because those explicitly use desktop GL. If you compile with cairo-gl, you cannot get subsurfaces demo's GLESv2 widget, because again you cannot mix GL and GLESv2 in the same process. We'd have to use something like libepoxy for window.c and write both GL and GLESv2 paths in it, which I think would be going a bit too far for the use cases it is intended for. OTOH, I suppose we could mandate that all toytoolkit apps must be desktop GL apps, iff the app itself wants to use any GL. That would then exclude things like running subsurfaces demo on RPi, which might otherwise be useful for testing. If we went GLESv2 only, we need to throw weston-screensaver out, but that can be replaced if someone wants to, and switch to GLESv2 gears which is probably just a small case of copying some code. Instead what I would propose is, make window.c shm-only, but also add helpers similar to what compositor-wayland.c does to render window.c decorations into a EGLSurface. That way window.c would not restrict what GL an app can use, but would still support apps using any flavour of GL. Or would you rather have window.c support everything? As part of window.c init, app tells it which GL flavour has been linked in if any, and then it adapts at runtime? I think I'd rather see window.c going in the simpler direction when possible, after all it is not supposed to be a real versatile toolkit. Is it? Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel