Re: Texture object locking
Jason wrote:
Kristian and I were looking at this today, and there seems to be a
substantial race in the way that we are doing texture locking.
Yes, it does look like this is a problem. I also noticed another
dodgy-looking pattern when implementing the GL_ARB_clear_texture
extension. Whenever it enters a meta function that operates on a
texture image it unlocks the texture object and then does stuff
directly using the texture image pointer. Presumably that pointer
could also be freed at any point. Take a look at
copytexsubimage_using_blit_framebuffer in meta.c for an example.
I suppose in practice though it's probably not all that likely that an
application will go and delete textures in another thread without the
other threads expecting it so I don't know whether it's a major
concern.
I knocked up the following patch to Weston's simple-egl example to try
and get it to crash when deleting textures from multiple threads and
sure enough it does segfault.
Regards,
- Neil
--- 8 --- (use git am --scissors to automatically chop here)
Subject: Hack to test multi-threaded deleting textures
---
clients/simple-egl.c | 125 +--
1 file changed, 112 insertions(+), 13 deletions(-)
diff --git a/clients/simple-egl.c b/clients/simple-egl.c
index 2097b4c..6e592b4 100644
--- a/clients/simple-egl.c
+++ b/clients/simple-egl.c
@@ -39,6 +39,7 @@
#include GLES2/gl2.h
#include EGL/egl.h
#include EGL/eglext.h
+#include pthread.h
#include xdg-shell-client-protocol.h
@@ -100,6 +101,13 @@ struct window {
int fullscreen, opaque, buffer_size, frame_sync;
};
+struct thread_data {
+ EGLDisplay dpy;
+ EGLSurface surface;
+ EGLContext ctx;
+ int thread_num;
+};
+
static const char *vert_shader_text =
uniform mat4 rotation;\n
attribute vec4 pos;\n
@@ -117,15 +125,25 @@ static const char *frag_shader_text =
gl_FragColor = v_color;\n
}\n;
+static const EGLint context_attribs[] = {
+ EGL_CONTEXT_CLIENT_VERSION, 2,
+ EGL_NONE
+};
+
static int running = 1;
+#define N_THREADS 63
+#define N_TEXTURES 512
+#define TEX_SIZE 128
+
+static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
+static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
+static uint64_t running_threads;
+static GLuint textures[N_TEXTURES];
+
static void
init_egl(struct display *display, struct window *window)
{
- static const EGLint context_attribs[] = {
- EGL_CONTEXT_CLIENT_VERSION, 2,
- EGL_NONE
- };
const char *extensions;
EGLint config_attribs[] = {
@@ -748,13 +766,101 @@ usage(int error_code)
exit(error_code);
}
+static GLuint
+create_texture(void)
+{
+ GLuint tex;
+ GLubyte data[TEX_SIZE * TEX_SIZE * 4];
+
+ glGenTextures(1, tex);
+ glBindTexture(GL_TEXTURE_2D, tex);
+ glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, TEX_SIZE, TEX_SIZE, 0,
+GL_RGBA, GL_UNSIGNED_BYTE, data);
+ glBindTexture(GL_TEXTURE_2D, 0);
+
+ return tex;
+}
+
+static void *
+thread_func(void *data)
+{
+ struct thread_data *thread_data = data;
+ int i;
+
+ eglMakeCurrent(thread_data-dpy,
+ thread_data-surface,
+ thread_data-surface,
+ thread_data-ctx);
+
+ while (true) {
+ /* Wait until this thread is told to run */
+ pthread_mutex_lock(mutex);
+ while (!(running_threads (1ULL thread_data-thread_num)))
+ pthread_cond_wait(cond, mutex);
+ pthread_mutex_unlock(mutex);
+
+ /* Delete all of the textures */
+ for (i = 0; i N_TEXTURES; i++)
+ glDeleteTextures(1, textures + i);
+
+ /* Report that our thread is no longer running */
+ pthread_mutex_lock(mutex);
+ running_threads = ~(1ULL thread_data-thread_num);
+ pthread_cond_broadcast(cond);
+ pthread_mutex_unlock(mutex);
+ }
+
+ return data;
+}
+
+static void
+delete_thread_test(struct display *display)
+{
+ pthread_t thread;
+ struct thread_data thread_data[N_THREADS];
+ int i;
+
+ for (i = 0; i N_THREADS; i++) {
+ thread_data[i].dpy = display-egl.dpy;
+ thread_data[i].surface = display-window-egl_surface;
+ thread_data[i].ctx = eglCreateContext(display-egl.dpy,
+ display-egl.conf,
+ display-egl.ctx,
+ context_attribs);
+ thread_data[i].thread_num = i;
+ pthread_create(thread,
+ NULL, /* attr */
+ thread_func,
+ thread_data + i);
+ }
+
+
[PATCH weston] Apply output transform to the cursor while copying into a plane
Previously if the output had a transform then the cursor plane would
be disabled. However as we have to copy the cursor image into a buffer
with the CPU anyway it probably won't cost much extra to also apply
the transform in the process and then we can avoid disabling the
plane.
If the transform is either normal or flipped-180 then a slightly
faster path is used that still copies the lines one at a time with
memcpy. Otherwise there is a slower path which copies one pixel at a
time.
Previously the check for whether to disable the plane was only looking
at whether the output has a transform. However it should probably have
also been disabled if the surface has a transform. This patch changes
it to just look at the surface transform instead. It could be possible
to work out a relative transform and apply that so that any
combinations of transforms will work, but this patch doesn't attempt
that.
---
src/compositor-drm.c | 120 ---
1 file changed, 113 insertions(+), 7 deletions(-)
diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index 7d514e4..553454d 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -958,7 +958,7 @@ drm_output_prepare_cursor_view(struct weston_output
*output_base,
if (c-gbm == NULL)
return NULL;
- if (output-base.transform != WL_OUTPUT_TRANSFORM_NORMAL)
+ if (viewport-buffer.transform != WL_OUTPUT_TRANSFORM_NORMAL)
return NULL;
if (viewport-buffer.scale != output_base-current_scale)
return NULL;
@@ -979,6 +979,102 @@ drm_output_prepare_cursor_view(struct weston_output
*output_base,
}
static void
+transform_cursor_buffer_slow(enum wl_output_transform transform,
+uint32_t *buf,
+unsigned char *s,
+int32_t width,
+int32_t height,
+int stride)
+{
+ int angle = transform 3;
+ int swap_xy = transform 1;
+ int flip_x, flip_y;
+ int buf_x_increment, buf_y_increment;
+ int s_y_increment;
+ int sx, sy;
+
+ /* Flip here means we flip the direction of the destination
+* iterators */
+ flip_x = (angle 1) ^ (angle 1);
+ flip_y = (angle 1);
+
+ if (swap_xy) {
+ if (transform 4)
+ flip_y = !flip_y;
+
+ if (flip_y) {
+ buf += (width - 1) * 64;
+ buf_x_increment = -64;
+ } else {
+ buf_x_increment = 64;
+ }
+
+ if (flip_x) {
+ buf += height - 1;
+ buf_y_increment = -1 - buf_x_increment * width;
+ } else {
+ buf_y_increment = 1 - buf_x_increment * width;
+ }
+ } else {
+ if (transform 4)
+ flip_x = !flip_x;
+
+ if (flip_x) {
+ buf += width - 1;
+ buf_x_increment = -1;
+ } else {
+ buf_x_increment = 1;
+ }
+
+ if (flip_y) {
+ buf += (height - 1) * 64;
+ buf_y_increment = -64 - buf_x_increment * width;
+ } else {
+ buf_y_increment = 64 - buf_x_increment * width;
+ }
+ }
+
+ s_y_increment = stride - width * 4;
+
+ for (sy = 0; sy height; sy++) {
+ for (sx = 0; sx width; sx++) {
+ memcpy(buf, s, 4);
+ s += 4;
+ buf += buf_x_increment;
+ }
+ s += s_y_increment;
+ buf += buf_y_increment;
+ }
+}
+
+static void
+transform_cursor_buffer(enum wl_output_transform transform,
+ uint32_t *buf,
+ unsigned char *s,
+ int32_t width,
+ int32_t height,
+ int stride)
+{
+ int y;
+
+ switch (transform) {
+ case WL_OUTPUT_TRANSFORM_NORMAL:
+ for (y = 0; y height; y++)
+ memcpy(buf + y * 64, s + y * stride, width * 4);
+ break;
+ case WL_OUTPUT_TRANSFORM_FLIPPED_180:
+ for (y = 0; y height; y++)
+ memcpy(buf + y * 64, s + (height - 1 - y) * stride,
+ width * 4);
+ break;
+ default:
+ transform_cursor_buffer_slow(transform,
+buf, s, width, height, stride);
+ break;
+ }
+}
+
+static void
drm_output_set_cursor(struct drm_output *output)
{
struct weston_view *ev = output-cursor_view;
@@ -989,7 +1085,8 @@ drm_output_set_cursor(struct drm_output *output)
struct gbm_bo
Re: [PATCH 3/4] Add a macro to get the opcode number of a request given the interface
Jason Ekstrand ja...@jlekstrand.net writes: Ooh, I like this. I thought about having wayland-scanner emit more #defines, but this works rather nicely. One question: do we want it in wayland-util or wayland-server? Putting it in wayland-util exposes it client-side as well. --Jason Yeah, I agree, perhaps it would make more sense in wayland-server.h. Thanks for the review. - Neil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/4] Handle requests using outputs that have been unplugged
Jason Ekstrand ja...@jlekstrand.net writes: Yeah, these are the wrong semantics. If they specify an output and it turns out to be a zombie, we should do nothing. A null output in wl_fullscreen_shell.present_surface means put it somewhere. In the case of weston's implementation, it goes on all outputs. We don't want a surface to accidentally end up everywhere when it was intended for a particular output. Ok yes, that makes sense. This is a bit ugly. In my original snippit, I pulled a similar stunt and stashed the opcode in the implementation implementation pointer. Is there some particular reason why you opted for the internal wl_list link instead? Well, the main reason was that I wanted to avoid having to add a wl_resource_get_implementation function, but I agree it would be cleaner to just add that as you say. Thanks for the review. - Neil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/4] Handle requests using outputs that have been unplugged
Jason Ekstrand ja...@jlekstrand.net writes: The destructor passed in here should be 0, why is it ~0? Also, it might be a good idea to throw it in a #define or enum for clarity. The ~0 is meant to signify ‘there is no destructor’, ie, it's a fake opcode that will never match a request. The reason is that I put this patch first before the separate patch to handle the wl_output.release request which changes it to 0. Yes, it would probably be good to have a define for this but as it was only in one temporary patch I thought it wasn't worth adding. Maybe just reordering the patches would make it cleaner. Regards, - Neil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH] Add wl_resource_get_implementation
This gets the implementation for a resource that was set in the
implementation argument of wl_resource_set_implementation or
wl_resource_set_dispatcher. This is mostly useful in conjuction with a
dispatcher which may want to dispatch via the implementation pointers
or just store some extra information there.
---
src/wayland-server.c | 6 ++
src/wayland-server.h | 2 ++
2 files changed, 8 insertions(+)
diff --git a/src/wayland-server.c b/src/wayland-server.c
index 1c9d4d0..4ce219a 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -1166,6 +1166,12 @@ wl_resource_set_implementation(struct wl_resource
*resource,
resource-dispatcher = NULL;
}
+WL_EXPORT const void *
+wl_resource_get_implementation(struct wl_resource *resource)
+{
+ return resource-object.implementation;
+}
+
WL_EXPORT void
wl_resource_set_dispatcher(struct wl_resource *resource,
wl_dispatcher_func_t dispatcher,
diff --git a/src/wayland-server.h b/src/wayland-server.h
index 24cd53f..4f56457 100644
--- a/src/wayland-server.h
+++ b/src/wayland-server.h
@@ -369,6 +369,8 @@ wl_resource_set_implementation(struct wl_resource *resource,
const void *implementation,
void *data,
wl_resource_destroy_func_t destroy);
+const void *
+wl_resource_get_implementation(struct wl_resource *resource);
void
wl_resource_set_dispatcher(struct wl_resource *resource,
wl_dispatcher_func_t dispatcher,
--
1.9.0
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v2] Add a macro to get the opcode number of a request given the interface
Here is a version two of the patch which moves the macro to
wayland-server.h and fixes a typo in the docs.
- Neil
--- 8 --- (use git am --scissors to automatically chop here)
This adds a macro called WL_REQUEST_OPCODE which takes the name of the
struct for the interface and the name of one of its members. It then
calculates the opcode number by dividing the offsetof the member by
the size of a function pointer. This assumes the interface struct only
contains function pointers and that they are in order of the opcodes.
---
src/wayland-server.h | 12
1 file changed, 12 insertions(+)
diff --git a/src/wayland-server.h b/src/wayland-server.h
index 7fc5b47..24cd53f 100644
--- a/src/wayland-server.h
+++ b/src/wayland-server.h
@@ -39,6 +39,18 @@ enum {
WL_EVENT_ERROR= 0x08
};
+/**
+ * Gets the opcode of a request from its interface struct
+ *
+ * Given the name of an interface struct and a request within it this
+ * will return the opcode number. For example it can be used like
+ * this:
+ *
+ * int opcode = WL_REQUEST_OPCODE(wl_region_interface, subtract);
+ */
+#define WL_REQUEST_OPCODE(interface, member) \
+ (offsetof(struct interface, member) / sizeof (void (*) (void)))
+
struct wl_event_loop;
struct wl_event_source;
typedef int (*wl_event_loop_fd_func_t)(int fd, uint32_t mask, void *data);
--
1.9.0
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v2] Handle wl_output.release
Here is a version two of the patch which is just needed so that the
patches can be reordered and this can come before the patch to handle
zombies.
--- 8 --- (use git am --scissors to automatically chop here)
The wl_output.release request is now handled. It just causes the
resource to be destroyed. This is also set as the destructor when
zombifying the resource.
---
src/compositor.c | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/src/compositor.c b/src/compositor.c
index 574db2d..d3a52bd 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -3038,6 +3038,18 @@ weston_compositor_stack_plane(struct weston_compositor
*ec,
wl_list_insert(ec-plane_list, plane-link);
}
+static void
+output_release(struct wl_client *client,
+ struct wl_resource *resource)
+{
+ wl_resource_destroy(resource);
+}
+
+static struct wl_output_interface
+output_interface = {
+ output_release
+};
+
static void unbind_resource(struct wl_resource *resource)
{
wl_list_remove(wl_resource_get_link(resource));
@@ -3052,14 +3064,15 @@ bind_output(struct wl_client *client,
struct wl_resource *resource;
resource = wl_resource_create(client, wl_output_interface,
- MIN(version, 2), id);
+ MIN(version, 3), id);
if (resource == NULL) {
wl_client_post_no_memory(client);
return;
}
wl_list_insert(output-resource_list, wl_resource_get_link(resource));
- wl_resource_set_implementation(resource, NULL, data, unbind_resource);
+ wl_resource_set_implementation(resource, output_interface,
+ data, unbind_resource);
wl_output_send_geometry(resource,
output-x,
@@ -3321,7 +3334,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 weston v2] Handle requests using outputs that have been unplugged
Version 2 of the patch makes the following changes:
• The patch is reordered to be on top of the patch to handle
wl_output.release so that it doesn't need to have a dummy marker for
zombifying without a destructor.
• The destructor opcode is stored in the implementation pointer rather
than using the list node. This requires the Wayland patch to add
wl_resource_get_implementation.
• The wl_fullscreen_shell.present_surface function now bails out if
the output is a zombie instead of treating it as if it were NULL.
I've pushed the patches to the following two branches to make it a bit
easier to untangle the thread:
https://github.com/bpeel/wayland/commits/wip/output-release
https://github.com/bpeel/weston/commits/wip/output-release
- Neil
--- 8 --- (use git am --scissors to automatically chop here)
Previously when an output was unplugged the clients' resources for it
were left around and if they were used in a request it could cause
Weston to access invalid memory. Now when an output is unplugged the
resources for it are marked as zombies. This is done by setting a
dummy dispatcher and setting the user data to NULL. The dispatcher
ignores all requests except for one which is marked as a destructor.
When the destructor is called the zombie resource will be destroyed.
The opcode for the destructor request is stashed into the
implementation pointer so that it can be compared against all of the
opcodes later.
Any requests that take a wl_output as an argument have also been
patched to take into account that the output can now be a zombie.
These are handled on a case-by-case basis but in general if the
argument is allowed to be NULL then zombie resources will be treated
as such. Otherwise the request is generally completely ignored.
The screenshooter interface is a special case because that has a
callback so we can't really just ignore the request. Instead the
buffer for the screenshot is cleared and the callback is immediately
invoked.
The code for zombifying a resource is based on a snippet by Jason
Esktrand.
https://bugs.freedesktop.org/show_bug.cgi?id=78415
---
desktop-shell/input-panel.c | 6 +
desktop-shell/shell.c | 38 ++---
fullscreen-shell/fullscreen-shell.c | 13 ++
src/compositor.c| 33 +
src/compositor.h| 3 +++
src/screenshooter.c | 48 +
6 files changed, 128 insertions(+), 13 deletions(-)
diff --git a/desktop-shell/input-panel.c b/desktop-shell/input-panel.c
index 7623f6c..df365fb 100644
--- a/desktop-shell/input-panel.c
+++ b/desktop-shell/input-panel.c
@@ -252,6 +252,12 @@ input_panel_surface_set_toplevel(struct wl_client *client,
struct input_panel_surface *input_panel_surface =
wl_resource_get_user_data(resource);
struct desktop_shell *shell = input_panel_surface-shell;
+ struct weston_output *output;
+
+ output = wl_resource_get_user_data(output_resource);
+ /* Ignore the request if the output has become a zombie */
+ if (output == NULL)
+ return;
wl_list_insert(shell-input_panel.surfaces,
input_panel_surface-link);
diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index dd0b2f9..97c5d11 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -2431,10 +2431,12 @@ shell_surface_set_fullscreen(struct wl_client *client,
struct shell_surface *shsurf = wl_resource_get_user_data(resource);
struct weston_output *output;
- if (output_resource)
+ if (output_resource) {
output = wl_resource_get_user_data(output_resource);
- else
+ /* Output can be NULL here if the resource is a zombie */
+ } else {
output = NULL;
+ }
shell_surface_set_parent(shsurf, NULL);
@@ -2566,10 +2568,12 @@ shell_surface_set_maximized(struct wl_client *client,
shsurf-type = SHELL_SURFACE_TOPLEVEL;
shell_surface_set_parent(shsurf, NULL);
- if (output_resource)
+ if (output_resource) {
output = wl_resource_get_user_data(output_resource);
- else
+ /* output can be NULL here if the resource is zombified */
+ } else {
output = NULL;
+ }
shell_surface_set_output(shsurf, output);
@@ -3487,10 +3491,13 @@ xdg_surface_set_fullscreen(struct wl_client *client,
shsurf-state_requested = true;
shsurf-requested_state.fullscreen = true;
- if (output_resource)
+ if (output_resource) {
output = wl_resource_get_user_data(output_resource);
- else
+ /* output can still be NULL here if the resource has
+* become a zombie */
+ } else {
output = NULL;
+ }
shell_surface_set_output(shsurf, output);
[PATCH weston 1/4] Handle requests using outputs that have been unplugged
Previously when an output was unplugged the clients' resources for it
were left around and if they were used in a request it could cause
Weston to access invalid memory. Now when an output is unplugged the
resources for it are marked as zombies. This is done by setting a
dummy dispatcher and setting the user data to NULL. The dispatcher
ignores all requests except for one which is marked as a destructor.
When the destructor is called the zombie resource will be destroyed.
The opcode for the destructor request is stashed into one of the
pointers in the resource's linked-list node so that it can be compared
against all of the opcodes later. The wl_output interface doesn't
currently have any requests nor a destructor but the intention is that
this mechanism could also be used later for more complicated
interfaces.
Any requests that take a wl_output as an argument have also been
patched to take into account that the output can now be a zombie.
These are handled on a case-by-case basis but in general if the
argument is allowed to be NULL then zombie resources will be treated
as such. Otherwise the request is generally completely ignored.
The screenshooter interface is a special case because that has a
callback so we can't really just ignore the request. Instead the
buffer for the screenshot is cleared and the callback is immediately
invoked.
The code for zombifying a resource is based on a snippet by Jason
Esktrand.
https://bugs.freedesktop.org/show_bug.cgi?id=78415
---
desktop-shell/input-panel.c | 6 +
desktop-shell/shell.c | 38 ++---
fullscreen-shell/fullscreen-shell.c | 12 ++
src/compositor.c| 36
src/compositor.h| 3 +++
src/screenshooter.c | 48 +
6 files changed, 130 insertions(+), 13 deletions(-)
diff --git a/desktop-shell/input-panel.c b/desktop-shell/input-panel.c
index 7623f6c..df365fb 100644
--- a/desktop-shell/input-panel.c
+++ b/desktop-shell/input-panel.c
@@ -252,6 +252,12 @@ input_panel_surface_set_toplevel(struct wl_client *client,
struct input_panel_surface *input_panel_surface =
wl_resource_get_user_data(resource);
struct desktop_shell *shell = input_panel_surface-shell;
+ struct weston_output *output;
+
+ output = wl_resource_get_user_data(output_resource);
+ /* Ignore the request if the output has become a zombie */
+ if (output == NULL)
+ return;
wl_list_insert(shell-input_panel.surfaces,
input_panel_surface-link);
diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index dd0b2f9..97c5d11 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -2431,10 +2431,12 @@ shell_surface_set_fullscreen(struct wl_client *client,
struct shell_surface *shsurf = wl_resource_get_user_data(resource);
struct weston_output *output;
- if (output_resource)
+ if (output_resource) {
output = wl_resource_get_user_data(output_resource);
- else
+ /* Output can be NULL here if the resource is a zombie */
+ } else {
output = NULL;
+ }
shell_surface_set_parent(shsurf, NULL);
@@ -2566,10 +2568,12 @@ shell_surface_set_maximized(struct wl_client *client,
shsurf-type = SHELL_SURFACE_TOPLEVEL;
shell_surface_set_parent(shsurf, NULL);
- if (output_resource)
+ if (output_resource) {
output = wl_resource_get_user_data(output_resource);
- else
+ /* output can be NULL here if the resource is zombified */
+ } else {
output = NULL;
+ }
shell_surface_set_output(shsurf, output);
@@ -3487,10 +3491,13 @@ xdg_surface_set_fullscreen(struct wl_client *client,
shsurf-state_requested = true;
shsurf-requested_state.fullscreen = true;
- if (output_resource)
+ if (output_resource) {
output = wl_resource_get_user_data(output_resource);
- else
+ /* output can still be NULL here if the resource has
+* become a zombie */
+ } else {
output = NULL;
+ }
shell_surface_set_output(shsurf, output);
shsurf-fullscreen_output = shsurf-output;
@@ -3919,6 +3926,10 @@ desktop_shell_set_background(struct wl_client *client,
return;
}
+ /* Skip the request if the output has become a zombie */
+ if (wl_resource_get_user_data(output_resource) == NULL)
+ return;
+
wl_list_for_each_safe(view, next, surface-views, surface_link)
weston_view_destroy(view);
view = weston_view_create(surface);
@@ -3953,6 +3964,7 @@ desktop_shell_set_panel(struct wl_client *client,
struct desktop_shell *shell = wl_resource_get_user_data(resource);
[PATCH 2/4] Add a release request to wl_output
Outputs can come and go within the compositor. If we don't have a way for the client to destroy the resource then the resources within the compositor will effectively leak until the client disconnects. This is similar to the release event for wl_pointer. --- protocol/wayland.xml | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/protocol/wayland.xml b/protocol/wayland.xml index 22eb6e7..d1680f7 100644 --- a/protocol/wayland.xml +++ b/protocol/wayland.xml @@ -1652,7 +1652,7 @@ /request /interface - interface name=wl_output version=2 + interface name=wl_output version=3 description summary=compositor output region An output describes part of the compositor geometry. The compositor works in the 'compositor coordinate system' and an @@ -1790,6 +1790,12 @@ /description arg name=factor type=int summary=scaling factor of output/ /event + +!-- Version 3 additions -- + +request name=release type=destructor since=3 + description summary=release the output object/ +/request /interface interface name=wl_region version=1 -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 4/4] Handle wl_output.release
The wl_output.release request is now handled. It just causes the
resource to be destroyed. This is also set as the destructor when
zombifying the resource.
---
src/compositor.c | 23 +++
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/src/compositor.c b/src/compositor.c
index ea0c9b4..f9bd25b 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -3038,6 +3038,18 @@ weston_compositor_stack_plane(struct weston_compositor
*ec,
wl_list_insert(ec-plane_list, plane-link);
}
+static void
+output_release(struct wl_client *client,
+ struct wl_resource *resource)
+{
+ wl_resource_destroy(resource);
+}
+
+static struct wl_output_interface
+output_interface = {
+ output_release
+};
+
static void unbind_resource(struct wl_resource *resource)
{
wl_list_remove(wl_resource_get_link(resource));
@@ -3052,14 +3064,15 @@ bind_output(struct wl_client *client,
struct wl_resource *resource;
resource = wl_resource_create(client, wl_output_interface,
- MIN(version, 2), id);
+ MIN(version, 3), id);
if (resource == NULL) {
wl_client_post_no_memory(client);
return;
}
wl_list_insert(output-resource_list, wl_resource_get_link(resource));
- wl_resource_set_implementation(resource, NULL, data, unbind_resource);
+ wl_resource_set_implementation(resource, output_interface,
+ data, unbind_resource);
wl_output_send_geometry(resource,
output-x,
@@ -3159,7 +3172,9 @@ weston_output_destroy(struct weston_output *output)
wl_global_destroy(output-global);
wl_resource_for_each_safe(resource, tmp, output-resource_list)
- weston_resource_zombify(resource, ~0);
+ weston_resource_zombify(resource,
+ WL_REQUEST_OPCODE(wl_output_interface,
+ release));
}
static void
@@ -3357,7 +3372,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 3/4] Add a macro to get the opcode number of a request given the interface
This adds a macro called WL_REQUEST_OPCODE which takes the name of the
struct for the interface and the name of one of its members. It then
calculates the opcode number by dividing the offsetof the member by
the size of a function pointer. This assumes the interface struct only
contains function pointers and that they are in order of the opcodes.
---
src/wayland-util.h | 12
1 file changed, 12 insertions(+)
diff --git a/src/wayland-util.h b/src/wayland-util.h
index fd32826..480a887 100644
--- a/src/wayland-util.h
+++ b/src/wayland-util.h
@@ -73,6 +73,18 @@ struct wl_interface {
const struct wl_message *events;
};
+/**
+ * Gets the opcode of a request from its interface struct
+ *
+ * Given the name of an interface struct an a request within it this
+ * will return the opcode number. For example it can be used like
+ * this:
+ *
+ * int opcode = WL_REQUEST_OPCODE(wl_region_interface, subtract);
+ */
+#define WL_REQUEST_OPCODE(interface, member) \
+ (offsetof(struct interface, member) / sizeof (void (*) (void)))
+
/** \class wl_list
*
* \brief doubly-linked list
--
1.9.0
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v2] configure.ac: Change in build system to use the path from pkg-config for wayland-scanner.
Hi,
Srivardhan Hebbar sri.heb...@samsung.com writes:
+AC_PATH_PROG([wayland_scanner], [wayland-scanner],,
+ [${WAYLAND_SCANNER_PATH}])
I don't think it makes much sense to search the path for wayland-scanner
when pkg-config already gives you the complete filename. Instead of
trying to derive a directory to search we could just use the filename
directly and check whether it is executable. Then instead of AC_PATH_PROG it
could do something like this:
AC_MSG_CHECKING([for wayland-scanner])
AS_IF([AS_EXECUTABLE_P([${wayland_scanner}])],
[AC_MSG_RESULT([${wayland_scanner}])
AC_SUBST(wayland_scanner)],
[AC_MSG_RESULT([not found])
AC_MSG_ERROR([wayland-scanner was not found])])
The --wayland-scanner-path option could also be replaced with something
that would just take the whole filename too.
Regards,
- Neil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] Destroy resources when destroying input and output
After looking at it a bit more I don't think we can really make the automatic zombie idea work. The trouble is that libwayland-server would need to know when the client has destroyed the proxy in order to destroy the zombie resource. However the destroy semantics are interface-dependent so only an implementation of the interface can know when the resource is really destroyed. The wl_output interface doesn't currently have a destructor so in this particular case it's even worse. I guess if a client binds to an output then the resource for it has to leak until the client disconnects. Assuming we later add a release request to wl_output (I think we should) then I think we would still need to make the zombie handling specific to the output implementation in Weston. We can't just set the implementation to NULL because then nothing would destroy the resource when the release request is called. Perhaps the best thing to do would be to make Weston swap out the interface for a simple one that only waits for the release event when an output is unplugged. We would also have to do this for pointers, keyboards and touch devices. This could end up with quite a lot of code because you have to double up all of the implementations. We also have to be aware of these zombie objects if we ever add any more requests that can refer to these objects because they would have to check if the object is a zombie and skip the request. If there's ever a Wayland 2.0 I think it would be good to make all interfaces implicitly have a destructor which is handled outside of the protocol description. I can't think of an interface that shouldn't have a destructor. Even with wl_callback which is defined to be destroyed when it is signalled it might make sense to allow the client to destroy the callback early if it's no longer interested. Regards, - Neil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] Destroy resources when destroying input and output
Perhaps we should consider applying the patch anyway even though it's not ideal. Currently if a client uses a dead output in a request such as xdg_surface.set_output Weston will end up with a weston_output pointer that points to freed memory. This could cause the compositor to crash. That is worse than the alternative provided by this patch which is to make the client abort. The clients know about the output being destroyed via the wl_registry.global_remove event so in practice they would only hit the problem in the unlikely event that they used the output in a request in the short time between the output being unplugged and noticing the removal event. In the longer term I was thinking maybe it would be good to handle the inert resource idea within libwayland-server. We could add a function like wl_resource_zombify() which would mark the resource as a zombie and call the destroy handlers. From the compositor's perspective it can then act as if the resource has been destroyed. We could detect zombie resources being used within the request marshalling code and ignore the request. If the request creates new resource we can internally create new zombie resources too and Weston would never need to know about it. I am planning to experiment with this approach now. Regards, - Neil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] Destroy resources when destroying input and output
Jason Ekstrand ja...@jlekstrand.net writes: Most of the magic there is in allowing resources with no handler in libwayland-server. The patch would be about 4 lines. Right now, client-side wl_proxy objects are allowed to have a NULL implementation and there's no problem; server-side, this is not currently allowed. If we allowed this ten Neil's wl_resource_zombify would only have to set the destructor, and implementation to NULL. For that matter, we could just do that explicitly in weston and not add API for it. Don't forget that we also want to ignore requests that have zombie resources as arguments, not just if the resource is directly used as a target of a request. It looks like the client-side already has code to handle zombie objects by putting a marker called WL_ZOMBIE_OBJECT in the ID map. Perhaps we should use the same mechanism on the server side too. If a zombie object is received in an event on the client side it looks like wl_closure_lookup_objects just sets the proxy object to NULL. Is that safe? Wouldn't the event handlers crash if they get a NULL resource in an event? If we can somehow make wl_closure_lookup_objects report when it finds a zombie object we can make the upper layers just consume the event. We could do this on both the client and side and the server side. - Neil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] Destroy resources when destroying an output
When an output is destroyed it now also destroys any resources that
were pointing to it. Otherwise if the resource is destroyed after the
output then the resource would try to remove itself from the resource
list but the head of the resource list would no longer be valid and it
would write to invalid memory.
This was found using Valgrind. It looks like there is a similar
problem for weston_pointer_destroy and in that case there is even a
comment suggesting we should do something about it.
https://bugs.freedesktop.org/show_bug.cgi?id=78415
---
src/compositor.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/src/compositor.c b/src/compositor.c
index cd1ca9a..4162315 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -3110,6 +3110,8 @@ weston_compositor_remove_output(struct weston_compositor
*compositor,
WL_EXPORT void
weston_output_destroy(struct weston_output *output)
{
+ struct wl_resource *resource, *tmp;
+
output-destroying = 1;
weston_compositor_remove_output(output-compositor, output);
@@ -3124,6 +3126,9 @@ weston_output_destroy(struct weston_output *output)
output-compositor-output_id_pool = ~(1 output-id);
wl_global_destroy(output-global);
+
+ wl_resource_for_each_safe(resource, tmp, output-resource_list)
+ wl_resource_destroy(resource);
}
static void
--
1.9.0
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] simple-touch: Handle multiple seats
Previously simple-touch would only handle one seat. If there were
multiple seats it would lose track of whether there is a touch device
available depending on what order the capability events arrive in.
This makes it keep a linked list of seats and to track a separate
touch device for each seat so that it doesn't matter what order they
arrive in.
https://bugs.freedesktop.org/show_bug.cgi?id=78365
---
clients/simple-touch.c | 77 +++---
1 file changed, 60 insertions(+), 17 deletions(-)
diff --git a/clients/simple-touch.c b/clients/simple-touch.c
index b5a84d7..a45de46 100644
--- a/clients/simple-touch.c
+++ b/clients/simple-touch.c
@@ -42,18 +42,25 @@ struct touch {
struct wl_compositor *compositor;
struct wl_shell *shell;
struct wl_shm *shm;
- struct wl_seat *seat;
- struct wl_touch *wl_touch;
struct wl_pointer *pointer;
struct wl_keyboard *keyboard;
struct wl_surface *surface;
struct wl_shell_surface *shell_surface;
struct wl_buffer *buffer;
+ struct wl_list seats;
int has_argb;
int width, height;
void *data;
};
+struct touch_seat {
+ struct touch *touch;
+ struct wl_touch *wl_touch;
+ struct wl_seat *wl_seat;
+ struct wl_list link;
+ uint32_t name;
+};
+
static void
create_shm_buffer(struct touch *touch)
{
@@ -156,7 +163,8 @@ touch_handle_down(void *data, struct wl_touch *wl_touch,
uint32_t serial, uint32_t time, struct wl_surface *surface,
int32_t id, wl_fixed_t x_w, wl_fixed_t y_w)
{
- struct touch *touch = data;
+ struct touch_seat *seat = data;
+ struct touch *touch = seat-touch;
float x = wl_fixed_to_double(x_w);
float y = wl_fixed_to_double(y_w);
@@ -173,7 +181,8 @@ static void
touch_handle_motion(void *data, struct wl_touch *wl_touch,
uint32_t time, int32_t id, wl_fixed_t x_w, wl_fixed_t y_w)
{
- struct touch *touch = data;
+ struct touch_seat *seat = data;
+ struct touch *touch = seat-touch;
float x = wl_fixed_to_double(x_w);
float y = wl_fixed_to_double(y_w);
@@ -199,18 +208,18 @@ static const struct wl_touch_listener touch_listener = {
};
static void
-seat_handle_capabilities(void *data, struct wl_seat *seat,
+seat_handle_capabilities(void *data, struct wl_seat *wl_seat,
enum wl_seat_capability caps)
{
- struct touch *touch = data;
-
- if ((caps WL_SEAT_CAPABILITY_TOUCH) !touch-wl_touch) {
- touch-wl_touch = wl_seat_get_touch(seat);
- wl_touch_set_user_data(touch-wl_touch, touch);
- wl_touch_add_listener(touch-wl_touch, touch_listener, touch);
- } else if (!(caps WL_SEAT_CAPABILITY_TOUCH) touch-wl_touch) {
- wl_touch_destroy(touch-wl_touch);
- touch-wl_touch = NULL;
+ struct touch_seat *seat = data;
+
+ if ((caps WL_SEAT_CAPABILITY_TOUCH) !seat-wl_touch) {
+ seat-wl_touch = wl_seat_get_touch(wl_seat);
+ wl_touch_set_user_data(seat-wl_touch, seat);
+ wl_touch_add_listener(seat-wl_touch, touch_listener, seat);
+ } else if (!(caps WL_SEAT_CAPABILITY_TOUCH) seat-wl_touch) {
+ wl_touch_destroy(seat-wl_touch);
+ seat-wl_touch = NULL;
}
}
@@ -243,6 +252,31 @@ static const struct wl_shell_surface_listener
shell_surface_listener = {
};
static void
+add_seat(struct touch *touch, uint32_t name)
+{
+ struct touch_seat *seat;
+
+ seat = malloc(sizeof *seat);
+ seat-touch = touch;
+ seat-wl_seat = wl_registry_bind(touch-registry, name,
+wl_seat_interface, 1);
+ seat-name = name;
+ seat-wl_touch = NULL;
+ wl_seat_add_listener(seat-wl_seat, seat_listener, seat);
+ wl_list_insert(touch-seats, seat-link);
+}
+
+static void
+remove_seat(struct touch_seat *seat)
+{
+ if (seat-wl_touch)
+ wl_touch_destroy(seat-wl_touch);
+ wl_seat_destroy(seat-wl_seat);
+ wl_list_remove(seat-link);
+ free(seat);
+}
+
+static void
handle_global(void *data, struct wl_registry *registry,
uint32_t name, const char *interface, uint32_t version)
{
@@ -261,15 +295,22 @@ handle_global(void *data, struct wl_registry *registry,
wl_shm_interface, 1);
wl_shm_add_listener(touch-shm, shm_listener, touch);
} else if (strcmp(interface, wl_seat) == 0) {
- touch-seat = wl_registry_bind(registry, name,
- wl_seat_interface, 1);
- wl_seat_add_listener(touch-seat, seat_listener, touch);
+ add_seat(touch, name);
}
}
static void
handle_global_remove(void *data, struct wl_registry *registry, uint32_t name)
{
Re: [PATCH weston 2/3] simple-touch: Handle multiple seats properly
Oh no, I didn't notice this patch buried in the patch series and
implemented the same thing:
http://lists.freedesktop.org/archives/wayland-devel/2014-May/014690.html
Ander, it would be really helpful if you could leave a little note on
the bug report if you post a patch to reduce that chance that I'll waste
time looking at the same bug.
Regards,
- Neil
Ander Conselvan de Oliveira conselv...@gmail.com writes:
From: Ander Conselvan de Oliveira ander.conselvan.de.olive...@intel.com
If simple-touch ran on a compositor with multiple seats, and the first
one happened to have the touch capability while the second one didn't,
the handler for seat capabilities would destroy the wl_touch device it
created on the first call for the first seat when it was called a again
for the second seat that has not touch capabilities.
Fix this problem by creating a separate struct for each seat.
https://bugs.freedesktop.org/show_bug.cgi?id=78365
---
clients/simple-touch.c | 48 +---
1 file changed, 33 insertions(+), 15 deletions(-)
diff --git a/clients/simple-touch.c b/clients/simple-touch.c
index b5a84d7..d8439ac 100644
--- a/clients/simple-touch.c
+++ b/clients/simple-touch.c
@@ -36,14 +36,18 @@
#define ARRAY_LENGTH(a) (sizeof (a) / sizeof (a)[0])
+struct seat {
+ struct touch *touch;
+ struct wl_seat *seat;
+ struct wl_touch *wl_touch;
+};
+
struct touch {
struct wl_display *display;
struct wl_registry *registry;
struct wl_compositor *compositor;
struct wl_shell *shell;
struct wl_shm *shm;
- struct wl_seat *seat;
- struct wl_touch *wl_touch;
struct wl_pointer *pointer;
struct wl_keyboard *keyboard;
struct wl_surface *surface;
@@ -199,18 +203,19 @@ static const struct wl_touch_listener touch_listener = {
};
static void
-seat_handle_capabilities(void *data, struct wl_seat *seat,
+seat_handle_capabilities(void *data, struct wl_seat *wl_seat,
enum wl_seat_capability caps)
{
- struct touch *touch = data;
-
- if ((caps WL_SEAT_CAPABILITY_TOUCH) !touch-wl_touch) {
- touch-wl_touch = wl_seat_get_touch(seat);
- wl_touch_set_user_data(touch-wl_touch, touch);
- wl_touch_add_listener(touch-wl_touch, touch_listener, touch);
- } else if (!(caps WL_SEAT_CAPABILITY_TOUCH) touch-wl_touch) {
- wl_touch_destroy(touch-wl_touch);
- touch-wl_touch = NULL;
+ struct seat *seat = data;
+ struct touch *touch = seat-touch;
+
+ if ((caps WL_SEAT_CAPABILITY_TOUCH) !seat-wl_touch) {
+ seat-wl_touch = wl_seat_get_touch(wl_seat);
+ wl_touch_set_user_data(seat-wl_touch, touch);
+ wl_touch_add_listener(seat-wl_touch, touch_listener, touch);
+ } else if (!(caps WL_SEAT_CAPABILITY_TOUCH) seat-wl_touch) {
+ wl_touch_destroy(seat-wl_touch);
+ seat-wl_touch = NULL;
}
}
@@ -219,6 +224,21 @@ static const struct wl_seat_listener seat_listener = {
};
static void
+add_seat(struct touch *touch, uint32_t name, uint32_t version)
+{
+ struct seat *seat;
+
+ seat = malloc(sizeof *seat);
+ assert(seat);
+
+ seat-touch = touch;
+ seat-wl_touch = NULL;
+ seat-seat = wl_registry_bind(touch-registry, name,
+ wl_seat_interface, 1);
+ wl_seat_add_listener(seat-seat, seat_listener, seat);
+}
+
+static void
handle_ping(void *data, struct wl_shell_surface *shell_surface,
uint32_t serial)
{
@@ -261,9 +281,7 @@ handle_global(void *data, struct wl_registry *registry,
wl_shm_interface, 1);
wl_shm_add_listener(touch-shm, shm_listener, touch);
} else if (strcmp(interface, wl_seat) == 0) {
- touch-seat = wl_registry_bind(registry, name,
-wl_seat_interface, 1);
- wl_seat_add_listener(touch-seat, seat_listener, touch);
+ add_seat(touch, name, version);
}
}
--
1.8.3.2
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 1/3] protocol: Add an event to specify the name of an output
Pekka Paalanen ppaala...@gmail.com writes: were you aware of […] perhaps? :-) Ah, no, I hadn't noticed those patches. Oops! Your series is almost identical to Quanxian's, except: - wording differences, of course - patch 2 checks if output-name is set before sending (can it ever be NULL?) - patch 2 missed setting the actual protocol object version - you add support to weston-info in patch 3 - Quanxian adds empty handlers to window.c and desktop-shell.c which isn't really necessary Ok, it sounds like it would make the most sense to use Quanxian's patches without the empty handlers and then land my weston-info patch on top. Regards, - Neil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] Apply the zoom transformation before the output transformation
The zoom translation is just a scale and a translate. The translation
is calculated based on the coordinates of the pointer which are in
global space. Previously the calculated translation was transformed by
the output transformation so that when the zoom transform is applied
after the output transform then it will be correct. However if we just
apply the zoom transformation first then we get the same result
without the zoom code having to be aware of the output transformation.
This also fixes weston_output_transform_coordinate which was applying
the output and zoom transforms in the wrong order.
https://bugs.freedesktop.org/show_bug.cgi?id=78211
---
src/compositor.c | 4 ++--
src/zoom.c | 47 ---
2 files changed, 2 insertions(+), 49 deletions(-)
diff --git a/src/compositor.c b/src/compositor.c
index 3d65e4c..cd1ca9a 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -3198,8 +3198,6 @@ weston_output_update_matrix(struct weston_output *output)
2.0 / output-width,
-2.0 / output-height, 1);
- weston_output_compute_transform(output);
-
if (output-zoom.active) {
magnification = 1 / (1 - output-zoom.spring_z.current);
weston_output_update_zoom(output);
@@ -3209,6 +3207,8 @@ weston_output_update_matrix(struct weston_output *output)
magnification, 1.0);
}
+ weston_output_compute_transform(output);
+
output-dirty = 0;
}
diff --git a/src/zoom.c b/src/zoom.c
index 622c0d7..7553849 100644
--- a/src/zoom.c
+++ b/src/zoom.c
@@ -111,50 +111,6 @@ zoom_area_center_from_pointer(struct weston_output *output,
}
static void
-weston_zoom_apply_output_transform(struct weston_output *output,
- float *x, float *y)
-{
- float tx, ty;
-
- switch(output-transform) {
- case WL_OUTPUT_TRANSFORM_NORMAL:
- default:
- return;
- case WL_OUTPUT_TRANSFORM_90:
- tx = -*y;
- ty = *x;
- break;
- case WL_OUTPUT_TRANSFORM_180:
- tx = -*x;
- ty = -*y;
- break;
- case WL_OUTPUT_TRANSFORM_270:
- tx = *y;
- ty = -*x;
- break;
- case WL_OUTPUT_TRANSFORM_FLIPPED:
- tx = -*x;
- ty = *y;
- break;
- case WL_OUTPUT_TRANSFORM_FLIPPED_90:
- tx = -*y;
- ty = -*x;
- break;
- case WL_OUTPUT_TRANSFORM_FLIPPED_180:
- tx = *x;
- ty = -*y;
- break;
- case WL_OUTPUT_TRANSFORM_FLIPPED_270:
- tx = *y;
- ty = *x;
- break;
- }
-
- *x = tx;
- *y = ty;
-}
-
-static void
weston_output_update_zoom_transform(struct weston_output *output)
{
float global_x, global_y;
@@ -183,9 +139,6 @@ weston_output_update_zoom_transform(struct weston_output
*output)
global_y - output-y) / output-height) *
(level * 2)) - level) * ratio;
- weston_zoom_apply_output_transform(output, output-zoom.trans_x,
- output-zoom.trans_y);
-
trans_max = level * 2 - level;
trans_min = -trans_max;
--
1.9.0
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Thoughts on getting surfaces to appear on the right output
Hi, Currently Weston has a problem that it always puts new surfaces on the same output as the one the first pointer is in. I guess the idea is that most of the time surfaces are created as a result of mouse events and there is usually only one pointer so it works most of the time. However of course this doesn't work if a surface is created by a touch event, a keyboard event or an event from a second pointer. The problem is mentioned in the following bug report: https://bugs.freedesktop.org/show_bug.cgi?id=73715 I guess ideally you want the client to explicitly say what output new surfaces should appear on because only the client knows which input event caused the new surface to be created. We already have xdg_shell.set_output which the client could use for this purpose. It doesn't look like Weston takes that chosen output into account if the surface is not fullscreen or maximised, but perhaps we could change it so that it does? This isn't a complete solution however because when the client first connects it doesn't know what event caused its process to be executed so it doesn't know which output to put itself on. This is the case when panel buttons are used to launch an application from desktop-shell. I was thinking that maybe we need a request that desktop-shell can make to specify a default output for new surfaces from the clients that it launches. To do this I was thinking that maybe desktop-shell could make a connection to the compositor on behalf of the client, call a request to set the default output and then pass the socket down to client using the existing WAYLAND_SOCKET mechanism. That would be an unusual use of WAYLAND_SOCKET because in the other cases the socket that is passed down is created with socketpair and is not first used for any communication. This would have the effect that wl_display_connect_to_fd is called twice on a connected socket. However, it doesn't look like any data is sent or any negotiation is done when that is called so it might not be a problem. Doing requests on the connection before passing it down might cause the resource ID numbers to get out of sync. However I was thinking that if the parent process is careful to destroy any resources that it creates before forking the child and if the Wayland protocol aggressively reuses IDs then it might reset back to zero anyway and it would just work. I was going to experiment with adding this request as a Weston-specific extension unless someone points out why it won't work. However, I think this probably shouldn't be Weston-specific because normal clients may also want to launch child clients in response to an input event and they too will want to do something similar. This approach wouldn't help for something like launching clients from a terminal window because obviously bash isn't going to do this trick before forking. As a fallback perhaps the positioning mechanism should be that the surface will appear on the last output that received *any* input event instead of where one of the pointers is. It's probably relatively safe to assume that a new surface is related to last input event. You could argue that doing that alone would be enough to fix the problem and we don't need to bother with the new request. However I don't think it would be very robust in cases where the new application is slow to start and there are multiple seats. If someone launches LibreOffice on their seat you don't want the surface to appear on someone else's seat just because it took a while to start and the other person had clicked a button in the meantime. Any thoughts? Regards, - Neil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Thoughts on getting surfaces to appear on the right output
Bill Spitzak spit...@gmail.com writes: 1. It would seem more useful for the desktop shell to send some info about how the client was launched in environment variables. Yes, maybe it would be cleaner to agree on some protocol for the parent process to send the information directly to the child. I'm not sure about using environment variables though because they tend to be inherited. For example if you used the desktop shell to launch a terminal then all clients that that terminal launched would end up with the same environment variables. 2. If the desktop shell creates the socket, there is no need for it to send commands on it. Instead it can directly manipulate the local end to have the results of those commands. The desktop shell is also a client in a separate process so it can't directly manipulate the compositor's view of the new client without some sort of protocol. Regards, - Neil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] Take into account the zoom when applying the output transform
When converting output-relative coordinates (such as from an input
event) to global coordinates it now takes into account the zoom
transform. Previously this would only work for the primary pointer
because the transform doesn't affect the primary pointer position due
to that way zoom follows the mouse. Touch events and multiple pointers
were not working correctly.
https://bugs.freedesktop.org/show_bug.cgi?id=68620
---
src/compositor.c | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/src/compositor.c b/src/compositor.c
index ee8dc24..3d65e4c 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -,6 +,7 @@ weston_output_transform_coordinate(struct weston_output
*output,
{
wl_fixed_t tx, ty;
wl_fixed_t width, height;
+ float zoom_scale, zx, zy;
width = wl_fixed_from_int(output-width * output-current_scale - 1);
height = wl_fixed_from_int(output-height * output-current_scale - 1);
@@ -3373,8 +3374,23 @@ weston_output_transform_coordinate(struct weston_output
*output,
break;
}
- *x = tx / output-current_scale + wl_fixed_from_int(output-x);
- *y = ty / output-current_scale + wl_fixed_from_int(output-y);
+ tx /= output-current_scale;
+ ty /= output-current_scale;
+
+ if (output-zoom.active) {
+ zoom_scale = output-zoom.spring_z.current;
+ zx = (wl_fixed_to_double(tx) * (1.0f - zoom_scale) +
+ output-width / 2.0f *
+ (zoom_scale + output-zoom.trans_x));
+ zy = (wl_fixed_to_double(ty) * (1.0f - zoom_scale) +
+ output-height / 2.0f *
+ (zoom_scale + output-zoom.trans_y));
+ tx = wl_fixed_from_double(zx);
+ ty = wl_fixed_from_double(zy);
+ }
+
+ *x = tx + wl_fixed_from_int(output-x);
+ *y = ty + wl_fixed_from_int(output-y);
}
static void
--
1.9.0
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] drm: Don't use the cursor overlay if the scale doesn't match
If the scale for the cursor surface doesn't match that of the output
then we shouldn't use the cursor overlay because otherwise it will be
drawn at the wrong size. This problem is particularly noticable with
multiple pointers because it randomly alternates between drawing one
cursor or the other at a larger size depending on which one gets put
in the cursor overlay.
---
src/compositor-drm.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index 9d293bc..4441308 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -953,12 +953,15 @@ drm_output_prepare_cursor_view(struct weston_output
*output_base,
{
struct drm_compositor *c =
(struct drm_compositor *) output_base-compositor;
+ struct weston_buffer_viewport *viewport = ev-surface-buffer_viewport;
struct drm_output *output = (struct drm_output *) output_base;
if (c-gbm == NULL)
return NULL;
if (output-base.transform != WL_OUTPUT_TRANSFORM_NORMAL)
return NULL;
+ if (viewport-buffer.scale != output_base-current_scale)
+ return NULL;
if (output-cursor_view)
return NULL;
if (ev-output_mask != (1u output_base-id))
--
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 @@ /event /interface - interface name=wl_output version=2 + interface name=wl_output version=3 description summary=compositor output region An output describes part of the compositor geometry. The compositor works in the 'compositor coordinate system' and an @@ -1783,6 +1783,17 @@ /description arg name=factor type=int summary=scaling factor of output/ /event + +!-- Version 3 additions -- + +event name=name since=3 + description summary=name of the output +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. + /description + arg name=name type=string summary=name of the output/ +/event /interface interface name=wl_region version=1 -- 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 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] Simply the matrix calculation for zooming
In order to apply the zoom transformation to the output matrix, Weston was
doing the following:
• Create a temporary matrix to hold the translation
• Invert the translation matrix using weston_matrix_invert into
another temporary matrix
• Scale that matrix by the scale factor
• Multiply the current matrix with the temporary matrix
Using weston_matrix_invert to invert a translation matrix is over the top.
Instead we can just negate the values we pass to weston_matrix_translate.
Matrix multiplication is associative so creating a temporary matrix to hold the
scale and translation transform should be equivalent to just applying them
directly to the output matrix.
---
src/compositor.c | 13 -
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/src/compositor.c b/src/compositor.c
index fd2decb..f836cf7 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -3186,8 +3186,6 @@ WL_EXPORT void
weston_output_update_matrix(struct weston_output *output)
{
float magnification;
- struct weston_matrix camera;
- struct weston_matrix modelview;
weston_matrix_init(output-matrix);
weston_matrix_translate(output-matrix,
@@ -3202,14 +3200,11 @@ weston_output_update_matrix(struct weston_output
*output)
if (output-zoom.active) {
magnification = 1 / (1 - output-zoom.spring_z.current);
- weston_matrix_init(camera);
- weston_matrix_init(modelview);
weston_output_update_zoom(output);
- weston_matrix_translate(camera, output-zoom.trans_x,
- -output-zoom.trans_y, 0);
- weston_matrix_invert(modelview, camera);
- weston_matrix_scale(modelview, magnification, magnification,
1.0);
- weston_matrix_multiply(output-matrix, modelview);
+ weston_matrix_translate(output-matrix, -output-zoom.trans_x,
+ output-zoom.trans_y, 0);
+ weston_matrix_scale(output-matrix, magnification,
+ magnification, 1.0);
}
output-dirty = 0;
--
1.9.0
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 3/4] evdev: Fix assertion error for unplugged output with paired touchscreen
I think I wrote the goto handled code. The advantage of that over putting the assert in the default handler is that if a new event type is added to the evdev_event_type enum then GCC will give a nice warning for that switch statement so we'd know we need to add a handler for it. I think I would personally prefer to change the breaks to be ‘goto handled’ too but I suppose that is a trade off for readability so I won't complain either way. The rest of the patches look good to me. I've tested it briefly and I don't get the crashes anymore. Regards, - Neil Ander Conselvan de Oliveira conselv...@gmail.com writes: From: Ander Conselvan de Oliveira ander.conselvan.de.olive...@intel.com If the output a touchscreen is paired to is unplugged, events coming from it should be ignored. Commit 17bccaed introduced logic for that in evdev_flush_pending_damage(). However, the break statements it introduced would cause the assertion after the switch statement to fail. That function has the odd behavior that goto's are used to skip the assertion after the switch statement and jump to the hunk of code that marks the event as processed. Only in the case where the event type has an invalid value the assertion should trigger. So this patch fixes the problem by moving the assertion into the default case of the switch and replacing the goto statements with break ones. https://bugs.freedesktop.org/show_bug.cgi?id=73950 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] Assign any output to a seat if it doesn't have one
Commit 4ade0e4a29 changed the order of initialising the seats and outputs so
that the seats would be done before the outputs. However the device_added
function in libinput-seat and udev-seat which gets called during initialisation
was trying to use the output list to assign an output to the device. If the
device doesn't have an output name associated with it then it would just use
the first output in the list. At this point during the initialisation the
output list is empty so it would actually end up with a garbage pointer and it
would later crash when a touch event tries to access the details of the output.
This patch changes it to only assign an output to the device if the output list
is not empty. That way the device's output will remain NULL during the
initialisation. It now also assigns the first output that is created to the
device during notify_output_create if it does not already have an output. If a
later output is created which matches the device's output name then that will
replace the selected output as before.
---
src/libinput-seat.c | 10 ++
src/udev-seat.c | 10 ++
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/src/libinput-seat.c b/src/libinput-seat.c
index 8bf538c..bea8f6e 100644
--- a/src/libinput-seat.c
+++ b/src/libinput-seat.c
@@ -86,7 +86,7 @@ device_added(struct udev_input *input, struct libinput_device
*libinput_device)
evdev_device_set_output(device, output);
}
- if (device-output == NULL) {
+ if (device-output == NULL !wl_list_empty(c-output_list)) {
output = container_of(c-output_list.next,
struct weston_output, link);
evdev_device_set_output(device, output);
@@ -316,11 +316,13 @@ notify_output_create(struct wl_listener *listener, void
*data)
struct evdev_device *device;
struct weston_output *output = data;
- wl_list_for_each(device, seat-devices_list, link)
- if (device-output_name
- strcmp(output-name, device-output_name) == 0) {
+ wl_list_for_each(device, seat-devices_list, link) {
+ if (device-output == NULL ||
+ (device-output_name
+strcmp(output-name, device-output_name) == 0)) {
evdev_device_set_output(device, output);
}
+ }
}
static struct udev_seat *
diff --git a/src/udev-seat.c b/src/udev-seat.c
index 5e018de..853a15c 100644
--- a/src/udev-seat.c
+++ b/src/udev-seat.c
@@ -127,7 +127,7 @@ device_added(struct udev_device *udev_device, struct
udev_input *input)
evdev_device_set_output(device, output);
}
- if (device-output == NULL) {
+ if (device-output == NULL !wl_list_empty(c-output_list)) {
output = container_of(c-output_list.next,
struct weston_output, link);
evdev_device_set_output(device, output);
@@ -359,11 +359,13 @@ notify_output_create(struct wl_listener *listener, void
*data)
struct evdev_device *device;
struct weston_output *output = data;
- wl_list_for_each(device, seat-devices_list, link)
- if (device-output_name
- strcmp(output-name, device-output_name) == 0) {
+ wl_list_for_each(device, seat-devices_list, link) {
+ if (device-output == NULL ||
+ (device-output_name
+strcmp(output-name, device-output_name) == 0)) {
evdev_device_set_output(device, output);
}
+ }
}
static struct udev_seat *
--
1.9.0
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 4/4] input: Fix errors due to initializing input before creating outputs
I think we accientally wrote nearly identical patches at the same time:
http://lists.freedesktop.org/archives/wayland-devel/2014-April/014392.html
However, I still get the crash with Ander's patch because it looks like
it is missing the check for wl_list_empty in the libinput-seat.c version
of device_added.
I think it would make sense to fix this patch and use it rather than my
version because my one assumes the old behaviour where if we can't find
an output with the right name for a seat then we default to using any
output. This is changed in patch 1 of Ander's series. I think Ander's
approach is more robust. Ie, if we can't find an output for a seat that
has an explicit output name then we just ignore events for it.
However I notice that patch 1 of the series also doesn't make this
revert for the libinput-seat.c version of device_added so that will
still maintain the behaviour of resorting to a default output from this
commit:
http://cgit.freedesktop.org/wayland/weston/commit/?id=161c6c56944cdfbda
- Neil
Ander Conselvan de Oliveira conselv...@gmail.com writes:
From: Ander Conselvan de Oliveira ander.conselvan.de.olive...@intel.com
Make sure that we don't map a device to an invalid output pointer and
intead remap devices when an output is created.
---
src/evdev.c | 2 +-
src/libinput-device.c | 2 +-
src/libinput-seat.c | 6 +-
src/udev-seat.c | 8 ++--
4 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/src/evdev.c b/src/evdev.c
index ff951d3..888dfbd 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -601,7 +601,7 @@ notify_output_destroy(struct wl_listener *listener, void
*data)
struct weston_compositor *c = device-seat-compositor;
struct weston_output *output;
- if (!device-output_name) {
+ if (!device-output_name !wl_list_empty(c-output_list)) {
output = container_of(c-output_list.next,
struct weston_output, link);
evdev_device_set_output(device, output);
diff --git a/src/libinput-device.c b/src/libinput-device.c
index 753583a..4605a76 100644
--- a/src/libinput-device.c
+++ b/src/libinput-device.c
@@ -264,7 +264,7 @@ notify_output_destroy(struct wl_listener *listener, void
*data)
struct weston_compositor *c = device-seat-compositor;
struct weston_output *output;
- if (!device-output_name) {
+ if (!device-output_name !wl_list_empty(c-output_list)) {
output = container_of(c-output_list.next,
struct weston_output, link);
evdev_device_set_output(device, output);
diff --git a/src/libinput-seat.c b/src/libinput-seat.c
index 8bf538c..e900744 100644
--- a/src/libinput-seat.c
+++ b/src/libinput-seat.c
@@ -316,11 +316,15 @@ notify_output_create(struct wl_listener *listener, void
*data)
struct evdev_device *device;
struct weston_output *output = data;
- wl_list_for_each(device, seat-devices_list, link)
+ wl_list_for_each(device, seat-devices_list, link) {
if (device-output_name
strcmp(output-name, device-output_name) == 0) {
evdev_device_set_output(device, output);
}
+
+ if (device-output_name == NULL device-output == NULL)
+ evdev_device_set_output(device, output);
+ }
}
static struct udev_seat *
diff --git a/src/udev-seat.c b/src/udev-seat.c
index dfeb17f..93984e1 100644
--- a/src/udev-seat.c
+++ b/src/udev-seat.c
@@ -125,7 +125,7 @@ device_added(struct udev_device *udev_device, struct
udev_input *input)
wl_list_for_each(output, c-output_list, link)
if (strcmp(output-name, device-output_name) == 0)
evdev_device_set_output(device, output);
- } else if (device-output == NULL) {
+ } else if (device-output == NULL !wl_list_empty(c-output_list)) {
output = container_of(c-output_list.next,
struct weston_output, link);
evdev_device_set_output(device, output);
@@ -357,11 +357,15 @@ notify_output_create(struct wl_listener *listener, void
*data)
struct evdev_device *device;
struct weston_output *output = data;
- wl_list_for_each(device, seat-devices_list, link)
+ wl_list_for_each(device, seat-devices_list, link) {
if (device-output_name
strcmp(output-name, device-output_name) == 0) {
evdev_device_set_output(device, output);
}
+
+ if (device-output_name == NULL device-output == NULL)
+ evdev_device_set_output(device, output);
+ }
}
static struct udev_seat *
--
1.8.3.2
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
[PATCH weston] clients/window: Don't remove the touch listener on a frame event
It looks like the handler for frame events from the wl_touch interface for widgets may have been erroneously copied from the cancel handler so that it removes all handlers as they are processed. I don't think this makes much sense for the frame event. This was stopping the panel icons from being pushable with touch events when using libinput since commit 1679f232e541489e. All that commit does it make it start sending the frame events. --- clients/window.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/clients/window.c b/clients/window.c index e770a04..e2f7010 100644 --- a/clients/window.c +++ b/clients/window.c @@ -3065,9 +3065,6 @@ touch_handle_frame(void *data, struct wl_touch *wl_touch) if (tp-widget-touch_frame_handler) (*tp-widget-touch_frame_handler)(tp-widget, input, tp-widget-user_data); - - wl_list_remove(tp-link); - free(tp); } } -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] compositor-drm: Fix crash when setting up seat constrained by an output
It looks like this patch makes Weston crash on touch events. The device_added functions in udev-seat.c and libinput-seat.c try to use the output list in order to assign the output for the newly created device. These functions get called via udev_input_init so I guess that means this function and create_outputs now depend on each other making a chicken and egg situation. If the device_added function can't find an output for the new device then it defaults to first output in the list. However since this patch the output list is now empty at that point so it ends up with a garbage pointer for the output. When the input code tries to handle an absolute event (eg, a touch event) then it tries to get the width and height from the current mode of the output for the device but this is now garbage. For me with libinput this causes it to segfault whereas without libinput it gets garbage width and height values and causes a floating point exception later on. I haven't looked any further to decide what's the best thing to do. - Neil Ander Conselvan de Oliveira conselv...@gmail.com writes: From: Ander Conselvan de Oliveira ander.conselvan.de.olive...@intel.com Commit 58e15865 changed the parameters for udev_get_seat_by_name() to receive a struct udev_input. However, when this gets called from create_output_from_connector() during initialization, the input struct is not yet initialized, leading to a crash. Previously, that function would take only a pointer to the compositor. This patch fixes the crash by initializing input before creating any outputs. https://bugs.freedesktop.org/show_bug.cgi?id=77503 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: An idea for a newbie project: Wayland protocol dumper
I think this could be really useful. I've struggled a few times trying to filter the output from WAYLAND_DEBUG=server when I only want to know about a particular client. Pekka Paalanen ppaala...@gmail.com writes: - Some messages carry file descriptors. Is it possible to write a proxy without having the protocol definition (XML files) needed to decode the messages? If not, the dumper won't work for unknown protocol extensions. The file descriptors come out-of-band as ‘control data’. Presumably we could make the tracer always read the control data when it reads from the socket and if it sees a file descriptor it can just pass it on without having to understand the protocol. Regards, - Neil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 1/2] Reset focus on unknown seats when restoring focus state
The focus_state list on a workspace only contains entries for seats
which have a keyboard focus on that workspace. For workspaces that
have no surfaces the list will be empty. That means that when a
workspace with no surfaces is switched to it would previously leave
the keyboard focus unaffected and you could still type in the surface
on the old workspace.
This patch makes it instead reset the keyboard focus to NULL for seats
without a focus_state. It does this by temporarily stealing the
compositor's list of seats while it iterates the focus_states. After
all of the focus states have been processed any seats remaining in
this temporary list have their focus reset.
https://bugs.freedesktop.org/show_bug.cgi?id=73905
---
desktop-shell/shell.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 466ea93..fa081f3 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -694,8 +694,20 @@ restore_focus_state(struct desktop_shell *shell, struct
workspace *ws)
{
struct focus_state *state, *next;
struct weston_surface *surface;
+ struct wl_list pending_seat_list;
+ struct weston_seat *seat, *next_seat;
+
+ /* Temporarily steal the list of seats so that we can keep
+* track of the seats we've already processed */
+ wl_list_init(pending_seat_list);
+ wl_list_insert_list(pending_seat_list, shell-compositor-seat_list);
+ wl_list_init(shell-compositor-seat_list);
wl_list_for_each_safe(state, next, ws-focus_list, link) {
+ wl_list_remove(state-seat-link);
+ wl_list_insert(shell-compositor-seat_list,
+ state-seat-link);
+
if (state-seat-keyboard == NULL)
continue;
@@ -703,6 +715,17 @@ restore_focus_state(struct desktop_shell *shell, struct
workspace *ws)
weston_keyboard_set_focus(state-seat-keyboard, surface);
}
+
+ /* For any remaining seats that we don't have a focus state
+* for we'll reset the keyboard focus to NULL */
+ wl_list_for_each_safe(seat, next_seat, pending_seat_list, link) {
+ wl_list_insert(shell-compositor-seat_list, seat-link);
+
+ if (state-seat-keyboard == NULL)
+ continue;
+
+ weston_keyboard_set_focus(seat-keyboard, NULL);
+ }
}
static void
--
1.8.5.3
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 2/2] Reset the keyboard focus on all seats when the compositor is locked
Before commit 2f5faff7f9142 when the compositor is locked it would
reset the keyboard focus on all of the seats as part of pushing the
focus_state. This was removed because it now always keeps track of the
focus_state in the workspace instead of waiting until the state is
pushed. However this had the side effect that the active surface would
retain focus when the compositor is locked. This patch just makes it
explicitly set the keyboard focus to NULL on all of the seats when
locking. This will be restored based on the workspace's state when
unlocking.
https://bugs.freedesktop.org/show_bug.cgi?id=73905
---
desktop-shell/shell.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index fa081f3..b19965f 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -4498,6 +4498,19 @@ touch_to_activate_binding(struct weston_seat *seat,
uint32_t time, void *data)
}
static void
+unfocus_all_seats(struct desktop_shell *shell)
+{
+ struct weston_seat *seat, *next;
+
+ wl_list_for_each_safe(seat, next, shell-compositor-seat_list, link) {
+ if (seat-keyboard == NULL)
+ continue;
+
+ weston_keyboard_set_focus(seat-keyboard, NULL);
+ }
+}
+
+static void
lock(struct desktop_shell *shell)
{
struct workspace *ws = get_current_workspace(shell);
@@ -4523,6 +4536,11 @@ lock(struct desktop_shell *shell)
launch_screensaver(shell);
+ /* Remove the keyboard focus on all seats. This will be
+* restored to the workspace's saved state via
+* restore_focus_state when the compositor is unlocked */
+ unfocus_all_seats(shell);
+
/* TODO: disable bindings that should not work while locked. */
/* All this must be undone in resume_desktop(). */
--
1.8.5.3
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 2/2] Reset the keyboard focus on all seats when the compositor is locked
Jasper St. Pierre jstpie...@mecheye.net writes: Why is this necessary? Won't events never be delivered while locked anyway? Events do seem to get delivered, you can try it if you run weston -i5 and then open a terminal and let it lock. You can still type in the terminal and launch programs. But yeah, I guess an alternative solution could be to block it at a lower level in the input handling code. I'm not sure whether that's a good idea though because eventually we might want a password entry or something on the lock screen and then we would want the keyboard to work. It would a shame to have to put too much special magic in there to get that to work. - Neil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] gl-renderer: Keep track of the GL format used for SHM buffers
Jason Ekstrand wrote:
We may still have a problem if the client changes buffer formats
mid-stream. Say it starts out with RGB565 to save memory but latter
decides it wants alpha so it switches to ARGB. We should
probably detect this and make sure glTexImage2D gets called again to
reset the internal format.
Yeah, you're right. I think it would be good to fix this especially
now that we support multiple outputs with different formats. It's not
entirely unlikely that a client might try to detect when it is moved
from one output to another with a different bit depth and then try to
switch buffer depths. The original patch has already landed in master
so here is an extra patch.
Regards,
- Neil
--- 8 --- (use git am --scissors to automatically chop here)
If the client attaches a new SHM buffer with a different format from a
previous one then we ought to trigger a full upload so that GL can
allocate a new texture. Otherwise Weston would technically be doing
invalid operations because it would call glTexSubImage2D with a
different format from the one specified in glTexImage2D. Presumably it
would also mean GL would have to convert the buffer as it copies the
sub-region in which isn't ideal.
This patch makes it decide the GL format when the buffer is attached
instead of when processing the damage and it will set
needs_full_upload if it is different from what it used before.
---
src/gl-renderer.c | 45 ++---
1 file changed, 22 insertions(+), 23 deletions(-)
diff --git a/src/gl-renderer.c b/src/gl-renderer.c
index 1cebc79..f534f51 100644
--- a/src/gl-renderer.c
+++ b/src/gl-renderer.c
@@ -90,6 +90,12 @@ struct gl_surface_state {
int needs_full_upload;
pixman_region32_t texture_damage;
+ /* These are only used by SHM surfaces to detect when we need
+* to do a full upload to specify a new internal texture
+* format */
+ GLenum gl_format;
+ GLenum gl_pixel_type;
+
EGLImageKHR images[3];
GLenum target;
int num_images;
@@ -998,8 +1004,6 @@ gl_renderer_flush_damage(struct weston_surface *surface)
struct weston_buffer *buffer = gs-buffer_ref.buffer;
struct weston_view *view;
int texture_used;
- GLenum format;
- int pixel_type;
#ifdef GL_EXT_unpack_subimage
pixman_box32_t *rectangles;
@@ -1032,29 +1036,13 @@ gl_renderer_flush_damage(struct weston_surface *surface)
!gs-needs_full_upload)
goto done;
- switch (wl_shm_buffer_get_format(buffer-shm_buffer)) {
- case WL_SHM_FORMAT_XRGB:
- case WL_SHM_FORMAT_ARGB:
- format = GL_BGRA_EXT;
- pixel_type = GL_UNSIGNED_BYTE;
- break;
- case WL_SHM_FORMAT_RGB565:
- format = GL_RGB;
- pixel_type = GL_UNSIGNED_SHORT_5_6_5;
- break;
- default:
- weston_log(warning: unknown shm buffer format\n);
- format = GL_BGRA_EXT;
- pixel_type = GL_UNSIGNED_BYTE;
- }
-
glBindTexture(GL_TEXTURE_2D, gs-textures[0]);
if (!gr-has_unpack_subimage) {
wl_shm_buffer_begin_access(buffer-shm_buffer);
- glTexImage2D(GL_TEXTURE_2D, 0, format,
+ glTexImage2D(GL_TEXTURE_2D, 0, gs-gl_format,
gs-pitch, buffer-height, 0,
-format, pixel_type,
+gs-gl_format, gs-gl_pixel_type,
wl_shm_buffer_get_data(buffer-shm_buffer));
wl_shm_buffer_end_access(buffer-shm_buffer);
@@ -1069,9 +1057,9 @@ gl_renderer_flush_damage(struct weston_surface *surface)
glPixelStorei(GL_UNPACK_SKIP_PIXELS_EXT, 0);
glPixelStorei(GL_UNPACK_SKIP_ROWS_EXT, 0);
wl_shm_buffer_begin_access(buffer-shm_buffer);
- glTexImage2D(GL_TEXTURE_2D, 0, format,
+ glTexImage2D(GL_TEXTURE_2D, 0, gs-gl_format,
gs-pitch, buffer-height, 0,
-format, pixel_type, data);
+gs-gl_format, gs-gl_pixel_type, data);
wl_shm_buffer_end_access(buffer-shm_buffer);
goto done;
}
@@ -1087,7 +1075,7 @@ gl_renderer_flush_damage(struct weston_surface *surface)
glPixelStorei(GL_UNPACK_SKIP_ROWS_EXT, r.y1);
glTexSubImage2D(GL_TEXTURE_2D, 0, r.x1, r.y1,
r.x2 - r.x1, r.y2 - r.y1,
- format, pixel_type, data);
+ gs-gl_format, gs-gl_pixel_type, data);
}
wl_shm_buffer_end_access(buffer-shm_buffer);
#endif
@@ -1127,6 +1115,7 @@ gl_renderer_attach_shm(struct weston_surface *es, struct
weston_buffer *buffer,
struct weston_compositor *ec = es-compositor;
struct gl_renderer
[PATCH weston] Always use glTexImage2D instead of glTexSubImage2D for first upload
Previously when uploading SHM data we would initialise the texture with glTexImage2D and NULL data when the buffer is attached. Then if the GL_EXT_unpack_subimage extension is available we would always use glTexSubImage2D to upload the data. The problem with that is that the first glTexImage2D was always setting the internal format to GL_BGRA_EXT and then if a 16-bit texture is used we would later call glTexSubImage2D with a data format of GL_RGBA. Under GLES2 the internal format must always match the data format so this is technically invalid. This patch makes it so that it always calls glTexImage2D when flushing the damage for the first time. That way it will use the right internal format and we don't need to call glTexImage2D with NULL data. https://bugs.freedesktop.org/show_bug.cgi?id=75251 --- src/gl-renderer.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/gl-renderer.c b/src/gl-renderer.c index 6ef1240..1cebc79 100644 --- a/src/gl-renderer.c +++ b/src/gl-renderer.c @@ -1069,9 +1069,9 @@ gl_renderer_flush_damage(struct weston_surface *surface) glPixelStorei(GL_UNPACK_SKIP_PIXELS_EXT, 0); glPixelStorei(GL_UNPACK_SKIP_ROWS_EXT, 0); wl_shm_buffer_begin_access(buffer-shm_buffer); - glTexSubImage2D(GL_TEXTURE_2D, 0, - 0, 0, gs-pitch, buffer-height, - format, pixel_type, data); + glTexImage2D(GL_TEXTURE_2D, 0, format, +gs-pitch, buffer-height, 0, +format, pixel_type, data); wl_shm_buffer_end_access(buffer-shm_buffer); goto done; } @@ -1168,10 +1168,6 @@ gl_renderer_attach_shm(struct weston_surface *es, struct weston_buffer *buffer, gs-surface = es; ensure_textures(gs, 1); - glBindTexture(GL_TEXTURE_2D, gs-textures[0]); - glTexImage2D(GL_TEXTURE_2D, 0, GL_BGRA_EXT, -gs-pitch, buffer-height, 0, -GL_BGRA_EXT, GL_UNSIGNED_BYTE, NULL); } } -- 1.8.5.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [Mesa-dev] [PATCH 0/7] EGL_MESA_configless_context
Kristian Høgsberg k...@bitplanet.net writes: Thanks Neil, series applied to mesa and weston. Great, thanks! I've reposted the piglit patch to the piglit list in case someone wants to review it there. Regards, - Neil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH mesa 3/7] eglCreateContext: Remove the check for whether config == 0
In eglCreateContext there is a check for whether the config parameter is zero
and in this case it will avoid reporting an error if the
EGL_KHR_surfacless_context extension is supported. However there is nothing in
that extension which says you can create a context without a config and Mesa
breaks if you try this so it is probably better to leave it reporting an
error.
The original check was added in b90a3e7d8b1bc based on the API-specific
extensions EGL_KHR_surfaceless_opengl/gles1/gles2. This was later changed to
refer to EGL_KHR_surfacless_context in b50703aea5. Perhaps the original
extensions specified a configless context but the new one does not.
---
src/egl/main/eglapi.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
index 59e214c..42bcb72 100644
--- a/src/egl/main/eglapi.c
+++ b/src/egl/main/eglapi.c
@@ -431,11 +431,8 @@ eglCreateContext(EGLDisplay dpy, EGLConfig config,
EGLContext share_list,
_EGL_CHECK_DISPLAY(disp, EGL_NO_CONTEXT, drv);
- if (!config) {
- /* config may be NULL if surfaceless */
- if (!disp-Extensions.KHR_surfaceless_context)
- RETURN_EGL_ERROR(disp, EGL_BAD_CONFIG, EGL_NO_CONTEXT);
- }
+ if (!config)
+ RETURN_EGL_ERROR(disp, EGL_BAD_CONFIG, EGL_NO_CONTEXT);
if (!share share_list != EGL_NO_CONTEXT)
RETURN_EGL_ERROR(disp, EGL_BAD_CONTEXT, EGL_NO_CONTEXT);
--
1.8.5.3
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH mesa 4/7] Add the EGL_MESA_configless_context extension
This extension provides a way for an application to render to multiple surfaces with different buffer formats without having to use multiple contexts. An EGLContext can be created without an EGLConfig by passing EGL_NO_CONFIG_MESA. In that case there are no restrictions on the surfaces that can be used with the context apart from that they must be using the same EGLDisplay. _mesa_initialze_context can now take a NULL gl_config which will mark the context as ‘configless’. It will memset the visual to zero in that case. Previously the i965 and i915 drivers were explicitly creating a zeroed visual whenever 0 is passed for the EGLConfig. Mesa needs to be aware that the context is configless because it affects the initial value to use for glDrawBuffer. The first time the context is bound it will set the initial value for configless contexts depending on whether the framebuffer used is double-buffered. --- docs/specs/MESA_configless_context.spec | 120 ++ include/EGL/eglext.h | 5 ++ src/egl/drivers/dri2/egl_dri2.c | 1 + src/egl/main/eglapi.c | 2 +- src/egl/main/eglcontext.c | 20 - src/egl/main/egldisplay.h | 1 + src/egl/main/eglmisc.c| 1 + src/mesa/drivers/dri/i915/intel_context.c | 6 -- src/mesa/drivers/dri/i965/brw_context.c | 6 -- src/mesa/main/context.c | 82 +++- src/mesa/main/mtypes.h| 6 ++ 11 files changed, 213 insertions(+), 37 deletions(-) create mode 100644 docs/specs/MESA_configless_context.spec diff --git a/docs/specs/MESA_configless_context.spec b/docs/specs/MESA_configless_context.spec new file mode 100644 index 000..8bed90d --- /dev/null +++ b/docs/specs/MESA_configless_context.spec @@ -0,0 +1,120 @@ +Name + +MESA_configless_context + +Name Strings + +EGL_MESA_configless_context + +Contact + +Neil Roberts neil.s.robe...@intel.com + +Status + +Proposal + +Version + +Version 1, February 28, 2014 + +Number + +EGL Extension #not assigned + +Dependencies + +Requires EGL 1.4 or later. This extension is written against the +wording of the EGL 1.4 specification. + +Overview + +This extension provides a means to use a single context to render to +multiple surfaces which have different EGLConfigs. Without this extension +the EGLConfig for every surface used by the context must be compatible +with the one used by the context. The only way to render to surfaces with +different formats would be to create multiple contexts but this is +inefficient with modern GPUs where this restriction is unnecessary. + +IP Status + +Open-source; freely implementable. + +New Procedures and Functions + +None. + +New Tokens + +Accepted as config in eglCreateContext + +EGL_NO_CONFIG_MESA ((EGLConfig)0) + +Additions to the EGL Specification section 2.2 Rendering Contexts and Drawing +Surfaces + +Add the following to the 3rd paragraph: + + EGLContexts can also optionally be created with respect to an EGLConfig +depending on the parameters used at creation time. If a config is provided +then additional restrictions apply on what surfaces can be used with the +context. + +Replace the last sentence of the 6th paragraph with: + + In order for a context to be compatible with a surface they both must have +been created with respect to the same EGLDisplay. If the context was +created without respect to an EGLConfig then there are no further +constraints. Otherwise they are only compatible if: + +Remove the last bullet point in the list of constraints. + +Additions to the EGL Specification section 3.7.1 Creating Rendering Contexts + +Replace the paragraph starting If config is not a valid EGLConfig... +with + + The config argument can either be a valid EGLConfig or EGL_NO_CONFIG_MESA. +If it is neither of these then an EGL_BAD_CONFIG error is generated. If a +valid config is passed then the error will also be generated if the config +does not support the requested client API (this includes requesting +creation of an OpenGL ES 1.x context when the EGL_RENDERABLE_TYPE +attribute of config does not contain EGL_OPENGL_ES_BIT, or creation of an +OpenGL ES 2.x context when the attribute does not contain +EGL_OPENGL_ES2_BIT). + +Passing EGL_NO_CONFIG_MESA will create a configless context. When a +configless context is used with the OpenGL API it can be assumed that the +initial values of the context's state will be decided when the context is +first made current. In particular this means that the decision of whether +to use GL_BACK or GL_FRONT for the initial value of the first output in +glDrawBuffers will be decided based on the config of the draw surface when +it is first bound. If the context is later bound to another
[PATCH piglit 5/7] Add a test for EGL_MESA_configless_context
This tests creating an EGLContext without an EGLConfig and then
creates three surfaces with different configs and tries rendering to
them. The test is skipped if the extension is not available.
---
tests/all.py | 4 +
tests/egl/CMakeLists.gl.txt| 1 +
tests/egl/egl-configless-context.c | 380 +
3 files changed, 385 insertions(+)
create mode 100644 tests/egl/egl-configless-context.c
diff --git a/tests/all.py b/tests/all.py
index d6daed2..8e6ac8e 100644
--- a/tests/all.py
+++ b/tests/all.py
@@ -3765,6 +3765,10 @@ egl_khr_create_context['valid debug flag GL'] =
plain_test('egl-create-context-v
for api in ('gles1', 'gles2', 'gles3'):
egl_khr_create_context['valid debug flag ' + api] =
plain_test('egl-create-context-valid-flag-debug-gles ' + api)
+egl_mesa_configless_context = Group()
+spec['EGL_MESA_configless_context']= egl_mesa_configless_context
+egl_mesa_configless_context['basic'] = plain_test('egl-configless-context')
+
egl_ext_client_extensions = Group()
spec['EGL_EXT_client_extensions'] = egl_ext_client_extensions
for i in [1, 2, 3]:
diff --git a/tests/egl/CMakeLists.gl.txt b/tests/egl/CMakeLists.gl.txt
index d7f17f6..bad6d87 100644
--- a/tests/egl/CMakeLists.gl.txt
+++ b/tests/egl/CMakeLists.gl.txt
@@ -20,6 +20,7 @@ IF(${CMAKE_SYSTEM_NAME} MATCHES Linux)
target_link_libraries(egl-create-surface pthread ${X11_X11_LIB})
piglit_add_executable (egl-query-surface egl-util.c egl-query-surface.c)
target_link_libraries(egl-query-surface pthread ${X11_X11_LIB})
+piglit_add_executable (egl-configless-context egl-configless-context.c)
ENDIF(${CMAKE_SYSTEM_NAME} MATCHES Linux)
# vim: ft=cmake:
diff --git a/tests/egl/egl-configless-context.c
b/tests/egl/egl-configless-context.c
new file mode 100644
index 000..f088056
--- /dev/null
+++ b/tests/egl/egl-configless-context.c
@@ -0,0 +1,380 @@
+/*
+ * Copyright © 2010, 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors: Neil Roberts neil.s.robe...@intel.com
+ * Kristian Høgsberg k...@bitplanet.net
+ */
+
+/** @file egl-configless-context.c
+ *
+ * Test the EGL_MESA_configless_context extension
+ */
+
+/* Chunks of code in this file are taken from egl-util.c */
+
+#include X11/Xlib.h
+
+#include piglit-util-gl-common.h
+#include piglit-util-egl.h
+
+#ifndef EGL_MESA_configless_context
+#define EGL_MESA_configless_context 1
+#define EGL_NO_CONFIG_MESA ((EGLConfig)0)
+#endif
+
+struct state {
+ Display *dpy;
+ EGLDisplay egl_dpy;
+ EGLint egl_major, egl_minor;
+ EGLContext ctx;
+};
+
+struct window {
+ EGLConfig config;
+ Window win;
+ EGLSurface surface;
+};
+
+static EGLint
+get_config_attrib(EGLDisplay egl_dpy,
+ EGLConfig config,
+ EGLenum attrib)
+{
+ EGLBoolean status;
+ EGLint value;
+
+ status = eglGetConfigAttrib(egl_dpy,
+ config,
+ attrib,
+ value);
+ if (!status) {
+ fprintf(stderr, eglGetConfigAttrib failed\n);
+ piglit_report_result(PIGLIT_FAIL);
+ }
+
+ return value;
+}
+
+static EGLConfig
+choose_config(EGLDisplay egl_dpy,
+ int depth,
+ EGLBoolean has_depth_buffer)
+{
+ EGLint attribs[32], *a = attribs;
+ EGLConfig configs[128];
+ EGLBoolean status;
+ EGLint config_count, i;
+ EGLint buffer_size;
+ EGLint depth_size;
+ EGLConfig best_config = 0;
+
+ switch (depth) {
+ case 16:
+ *(a++) = EGL_RED_SIZE;
+ *(a++) = 5;
+ *(a++) = EGL_GREEN_SIZE;
+ *(a++) = 6;
+ *(a++) = EGL_BLUE_SIZE;
+ *(a++) = 5
[PATCH 0/7] EGL_MESA_configless_context
Here is a series of patches to add an extension which makes it possible to create an EGL context without specifying a config. A context created in this way can be bound with any surface using the same EGLDisplay rather than being restricted to those using the same config. The main use case is that it would then be possible for a Wayland compositor to drive two displays which may have different bit-depths without having to wastefully create two contexts. Mesa seems to mostly cope with this already so the patches are mostly just to add the extension and remove the restriction. There is also a patch for Piglit to try and test the extension. I tried running the branch though the 'all tests' test suite and apart from this new test now passing there was only one difference which is that the GLX_OML_sync_control/swapbuffersmsc-divisor-zero test failed. However if I run piglit-run.py again and set a regexp so it only runs that one test then it passes so I'm wondering if it might just be an intermittent failure. The main thorny issue with the extension is how to handle the initial value of glDrawBuffer when a configless context is used. If a config is used then we can just say the default is GL_BACK if the config is double-buffered and GL_FRONT otherwise. I have taken the approach that this decision is made the first time the context is bound rather than when it is first created. There should be no way to query the value of glDrawBuffer until the context is first bound so it shouldn't cause any harm that Mesa changes the value at that point. I think this is worth doing for the convenience to the application which would otherwise have to remember to explicitly set it to GL_BACK in the common case that double-buffering is used. I've also included a couple of patches to take advantage of the extension in Weston. Regards, - Neil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 7/7] Add support for having different GBM formats for different outputs
The gbm-format configuration option can now be specified per-output as
well as in the core config section. If it is not specified it will
default to the format specified in the core section. The
EGL_MESA_configless_context extension is required for this to work. If
this extension is available it will create a context without an
EGLConfig and then it will potentially use a different EGLConfig for
each output.
The gl-renderer interface has been changed so that it takes the EGL
attributes and visual ID in the create_output function as well as in
the create function.
---
src/compositor-drm.c | 65 ---
src/compositor-fbdev.c | 4 +-
src/compositor-wayland.c | 4 +-
src/compositor-x11.c | 4 +-
src/gl-renderer.c| 134 +--
src/gl-renderer.h| 4 +-
6 files changed, 141 insertions(+), 74 deletions(-)
diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index e45f47d..3f584a6 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -147,6 +147,7 @@ struct drm_output {
drmModeCrtcPtr original_crtc;
struct drm_edid edid;
drmModePropertyPtr dpms_prop;
+ uint32_t format;
int vblank_pending;
int page_flip_pending;
@@ -420,8 +421,6 @@ static uint32_t
drm_output_check_scanout_format(struct drm_output *output,
struct weston_surface *es, struct gbm_bo *bo)
{
- struct drm_compositor *c =
- (struct drm_compositor *) output-base.compositor;
uint32_t format;
pixman_region32_t r;
@@ -442,7 +441,7 @@ drm_output_check_scanout_format(struct drm_output *output,
pixman_region32_fini(r);
}
- if (c-format == format)
+ if (output-format == format)
return format;
return 0;
@@ -507,7 +506,7 @@ drm_output_render_gl(struct drm_output *output,
pixman_region32_t *damage)
return;
}
- output-next = drm_fb_get_from_bo(bo, c, c-format);
+ output-next = drm_fb_get_from_bo(bo, c, output-format);
if (!output-next) {
weston_log(failed to get drm_fb for bo\n);
gbm_surface_release_buffer(output-surface, bo);
@@ -1528,12 +1527,13 @@ find_crtc_for_connector(struct drm_compositor *ec,
static int
drm_output_init_egl(struct drm_output *output, struct drm_compositor *ec)
{
+ EGLint format = output-format;
int i, flags;
output-surface = gbm_surface_create(ec-gbm,
output-base.current_mode-width,
output-base.current_mode-height,
-ec-format,
+format,
GBM_BO_USE_SCANOUT |
GBM_BO_USE_RENDERING);
if (!output-surface) {
@@ -1541,7 +1541,9 @@ drm_output_init_egl(struct drm_output *output, struct
drm_compositor *ec)
return -1;
}
- if (gl_renderer-output_create(output-base, output-surface) 0) {
+ if (gl_renderer-output_create(output-base, output-surface,
+ gl_renderer-opaque_attribs,
+ format) 0) {
weston_log(failed to create gl renderer output state\n);
gbm_surface_destroy(output-surface);
return -1;
@@ -1853,6 +1855,35 @@ setup_output_seat_constraint(struct drm_compositor *ec,
}
static int
+get_gbm_format_from_section(struct weston_config_section *section,
+ uint32_t default_value,
+ uint32_t *format)
+{
+ char *s;
+ int ret = 0;
+
+ weston_config_section_get_string(section,
+gbm-format, s, NULL);
+
+ if (s == NULL)
+ *format = default_value;
+ else if (strcmp(s, xrgb) == 0)
+ *format = GBM_FORMAT_XRGB;
+ else if (strcmp(s, rgb565) == 0)
+ *format = GBM_FORMAT_RGB565;
+ else if (strcmp(s, xrgb2101010) == 0)
+ *format = GBM_FORMAT_XRGB2101010;
+ else {
+ weston_log(fatal: unrecognized pixel format: %s\n, s);
+ ret = -1;
+ }
+
+ free(s);
+
+ return ret;
+}
+
+static int
create_output_for_connector(struct drm_compositor *ec,
drmModeRes *resources,
drmModeConnector *connector,
@@ -1919,6 +1950,11 @@ create_output_for_connector(struct drm_compositor *ec,
transform = parse_transform(s, output-base.name);
free(s);
+ if (get_gbm_format_from_section(section,
+ ec-format,
+ output-format) == -1)
+ output-format = ec-format;
+
[PATCH mesa 2/7] Fix the initial value of glDrawBuffers for GLES
Under GLES 3 it is not valid to pass GL_FRONT to glDrawBuffers. Instead,
GL_BACK has a magic interpretation which means it will render to the front
buffer on single-buffered contexts and the back buffer on double-buffered. We
were incorrectly setting the initial value to GL_FRONT for single-buffered
contexts. This probably doesn't really matter at the moment except that
presumably it would be exposed in the API via glGetIntegerv.
When we switch to configless contexts this is more important because in that
case we always want to rely on the magic interpretation of GL_BACK in order to
automatically switch between the front and back buffer when a new surface with
a different number of buffers is bound. We also do this for GLES 1 and 2
because the internal value doesn't matter in that case and it is convenient to
use the same code to have the magic interpretation of GL_BACK.
---
src/mesa/main/blend.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/mesa/main/blend.c b/src/mesa/main/blend.c
index eb4f1d6..c37c0fe 100644
--- a/src/mesa/main/blend.c
+++ b/src/mesa/main/blend.c
@@ -911,7 +911,9 @@ void _mesa_init_color( struct gl_context * ctx )
ctx-Color.LogicOp = GL_COPY;
ctx-Color.DitherFlag = GL_TRUE;
- if (ctx-Visual.doubleBufferMode) {
+ /* GL_FRONT is not possible on GLES. Instead GL_BACK will render to either
+* the front or the back buffer depending on the config */
+ if (ctx-Visual.doubleBufferMode || _mesa_is_gles(ctx)) {
ctx-Color.DrawBuffer[0] = GL_BACK;
}
else {
--
1.8.5.3
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH mesa 1/7] Use the magic behaviour of GL_BACK in GLES 1 and 2 as well as 3
In GLES 3 it is not possible to select rendering to the front buffer and
instead selecting GL_BACK has the magic interpretation that it is either the
front buffer on single-buffered configs or the back buffer on double-buffered.
GLES 1 and 2 have no way of selecting the draw buffer at all. In that case we
were initialising the draw buffer to either GL_FRONT or GL_BACK depending on
the context's config and then leaving it at that. When we switch to having
configless contexts we ideally want Mesa to automatically switch between the
front and back buffer whenever a double- or single-buffered surface is bound.
To make this happen we can just allow the magic behaviour from GLES 3 in GLES
1 and 2 as well. It shouldn't matter what the internal value of the draw
buffer is in GLES 1 and 2 because there is no way to query it from the
external API.
---
src/mesa/main/buffers.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/mesa/main/buffers.c b/src/mesa/main/buffers.c
index 6cbce9d..b13a7af 100644
--- a/src/mesa/main/buffers.c
+++ b/src/mesa/main/buffers.c
@@ -101,7 +101,7 @@ draw_buffer_enum_to_bitmask(const struct gl_context *ctx,
GLenum buffer)
case GL_FRONT:
return BUFFER_BIT_FRONT_LEFT | BUFFER_BIT_FRONT_RIGHT;
case GL_BACK:
- if (_mesa_is_gles3(ctx)) {
+ if (_mesa_is_gles(ctx)) {
/* Page 181 (page 192 of the PDF) in section 4.2.1 of the OpenGL
* ES 3.0.1 specification says:
*
@@ -111,6 +111,11 @@ draw_buffer_enum_to_bitmask(const struct gl_context *ctx,
GLenum buffer)
*
* Since there is no stereo rendering in ES 3.0, only return the
* LEFT bits. This also satisfies the n must be 1 requirement.
+ *
+ * We also do this for GLES 1 and 2 because those APIs have no
+ * concept of selecting the front and back buffer anyway and it's
+ * convenient to be able to maintain the magic behaviour of
+ * GL_BACK in that case.
*/
if (ctx-DrawBuffer-Visual.doubleBufferMode)
return BUFFER_BIT_BACK_LEFT;
--
1.8.5.3
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v2] Add a nested YUV client to the weston-nested example
Here is a second version of the patch to match with the changes in v2
of the extension.
Regards,
- Neil
--- 8 --- (use git am --scissors to automatically chop here)
This adds a separate nested client to the weston-nested example which
will create YUV buffers using libva. The code is based on the
putsurface test in libva. The client can be used by passing the -y
option to weston-nested.
In the subcompositor when it detects that the client has passed a
planar buffer it will use eglCreateWaylandBufferFromPlanarImagesWL
instead of the original function. The function pointer prototype in
eglext.h for this is not used so that Weston can still be compiled
with the old version of the extension. In that case weston-nested will
crash if you pass the -y option, but it will otherwise work.
---
.gitignore |1 +
Makefile.am | 11 +
clients/nested-client-yuv.c | 1035 +++
clients/nested.c| 125 --
configure.ac| 21 +-
5 files changed, 1151 insertions(+), 42 deletions(-)
create mode 100644 clients/nested-client-yuv.c
diff --git a/.gitignore b/.gitignore
index e0a73c0..3728f36 100644
--- a/.gitignore
+++ b/.gitignore
@@ -42,6 +42,7 @@ weston-gears
weston-image
weston-nested
weston-nested-client
+weston-nested-client-yuv
weston-resizor
weston-scaler
weston-simple-egl
diff --git a/Makefile.am b/Makefile.am
index 64d0743..788bcdf 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -478,6 +478,17 @@ weston_nested_CFLAGS = $(AM_CFLAGS) $(CLIENT_CFLAGS)
weston_nested_client_SOURCES = clients/nested-client.c
weston_nested_client_LDADD = $(SIMPLE_EGL_CLIENT_LIBS) -lm
weston_nested_client_CFLAGS = $(AM_CFLAGS) $(CLIENT_CFLAGS)
+
+if HAVE_LIBVA_WAYLAND
+demo_clients += weston-nested-client-yuv
+weston_nested_client_yuv_SOURCES = clients/nested-client-yuv.c
+weston_nested_client_yuv_LDADD = $(LIBVA_WAYLAND_LIBS) $(SIMPLE_CLIENT_LIBS)
+weston_nested_client_yuv_CFLAGS = \
+ $(CLIENT_CFLAGS) \
+ $(AM_CFLAGS) \
+ $(LIBVA_WAYLAND_CFLAGS)
+endif
+
endif
weston_eventdemo_SOURCES = clients/eventdemo.c
diff --git a/clients/nested-client-yuv.c b/clients/nested-client-yuv.c
new file mode 100644
index 000..c37b999
--- /dev/null
+++ b/clients/nested-client-yuv.c
@@ -0,0 +1,1035 @@
+/*
+ * Copyright (c) 2008, 2009, 2014 Intel Corporation. All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * Software), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS
+ * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
+ * IN NO EVENT SHALL PRECISION INSIGHT AND/OR ITS SUPPLIERS BE LIABLE FOR
+ * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+ * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+ * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#include stdio.h
+#include string.h
+#include stdlib.h
+#include stdint.h
+
+#include sys/time.h
+
+#include unistd.h
+
+#include sys/types.h
+#include sys/stat.h
+#include fcntl.h
+#include assert.h
+#include pthread.h
+
+#include va/va_wayland.h
+
+#define CHECK_VASTATUS(va_status,func) \
+ if (va_status != VA_STATUS_SUCCESS) { \
+ fprintf(stderr,%s:%s (%d) failed,exit\n, \
+ __func__, \
+ func, \
+ __LINE__); \
+ exit(1);\
+ }
+
+static void *open_display(void);
+static void close_display(void *win_display);
+static int create_window(void *win_display,
+int x, int y, int width, int height);
+static int check_window_event(void *win_display, void *drawable,
+ int *width, int *height, int *quit);
+
+struct display;
+struct drawable;
+
+static VAStatus
+va_put_surface(VADisplaydpy,
+ struct drawable *wl_drawable,
+ VASurfaceID va_surface,
+ const VARectangle*src_rect,
+ const VARectangle*dst_rect,
+ const VARectangle*cliprects,
+ unsigned
[PATCH weston] nested: Disable cairo on the subsurfaces
The subsurface widgets on the nested example aren't using Cairo to render so we should turn it off to prevent the toy toolkit from creating a redundant extra surface for it. This is particularly important since Mesa commit 6c9d6898fdfd7e2 because the surface that the toolkit tries to create is zero-sized and that patch prevents that from working. This was causing weston-nested to crash. --- clients/nested.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/clients/nested.c b/clients/nested.c index d75e953..3bdb961 100644 --- a/clients/nested.c +++ b/clients/nested.c @@ -976,6 +976,8 @@ ss_surface_init(struct nested_surface *surface) nested, SUBSURFACE_SYNCHRONIZED); + widget_set_use_cairo(ss_surface-widget, 0); + ss_surface-surface = widget_get_wl_surface(ss_surface-widget); ss_surface-subsurface = widget_get_wl_subsurface(ss_surface-widget); -- 1.8.5.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v2] Adapt the EGL_WL_create_wayland_buffer_from_image extension for YUV
updates to the image will also be updated in the wl_buffer. Typically the
@@ -82,13 +89,50 @@ Additions to the EGL 1.4 Specification:
format or tiling mode or that the buffer is in memory that is inaccessible
to the GPU that the given EGLDisplay is using.
+Alternatively it is possible to create a buffer from a planar image where
+each plane is contained in a separate EGLImage using the following
+function:
+
+struct wl_buffer *eglCreateWaylandBufferFromPlanarImagesWL(EGLDisplay dpy,
+ EGLenum format,
+ EGLImageKHR
*ims,
+ EGLint
n_images);
+
+The number of images passed should match the number of planes required for
+the format given in the format parameter. For non-planar formats this will
+just be 1. Each image represents one plane within the buffer.
+
+The formats accepted in the format parameter are:
+
+ EGL_TEXTURE_RGBA single image is used which contains RGB
+ components.
+ EGL_TEXTURE_RGBA A single image is used which contains RGBA
+ components.
+ EGL_TEXTURE_Y_U_V_WL Three images are used; one containing the
+ luminance component, one containing the
+ U component of the chrominance data and another
+ containing the V component.
+ EGL_TEXTURE_Y_UV_WLTwo images are used; one containing a single
+ luminance component and the other containing the
+ two chrominance components.
+ EGL_TEXTURE_Y_XUXV_WL Two images are used; one containing a single
+ luminance component and the other containing the
+ two chrominance components along with two unused
+ components.
+
+Dependencies on EGL_WL_bind_wayland_display
+
+If EGL_WL_bind_wayland_display is not implemented then references to
+EGL_TEXTURE_Y_U_V_WL, EGL_TEXTURE_Y_UV_WL and EGL_TEXTURE_Y_XUXV_WL are
+void.
+
Issues
1) Under what circumstances can the EGL_BAD_MATCH error be generated? Does
this include for example unsupported tiling modes?
RESOLVED: Yes, the EGL_BAD_MATCH error can be generated for any reason
- which prevents the implementation from representing the image as a
+ which prevents the implementation from representing the images as a
wl_buffer. For example, these problems can be but are not limited to
unsupported tiling modes, inaccessible memory or an unsupported pixel
format.
@@ -99,3 +143,6 @@ Revision History
Initial draft (Neil Roberts)
Version 2, October 25, 2013
Added a note about more possible reasons for returning EGL_BAD_FORMAT.
+Version 3, February 5, 2014
+Support for passing multiple images in order to create a planar
+buffer.
diff --git a/include/EGL/eglmesaext.h b/include/EGL/eglmesaext.h
index b3229e3..b78aa8b 100644
--- a/include/EGL/eglmesaext.h
+++ b/include/EGL/eglmesaext.h
@@ -139,8 +139,10 @@ typedef EGLBoolean (EGLAPIENTRYP
PFNEGLQUERYWAYLANDBUFFERWL) (EGLDisplay dpy, st
#ifdef EGL_EGLEXT_PROTOTYPES
EGLAPI struct wl_buffer * EGLAPIENTRY
eglCreateWaylandBufferFromImageWL(EGLDisplay dpy, EGLImageKHR image);
+EGLAPI struct wl_buffer * EGLAPIENTRY
eglCreateWaylandBufferFromPlanarImagesWL(EGLDisplay dpy, EGLenum format,
EGLImageKHR *images, EGLint n_images);
#endif
typedef struct wl_buffer * (EGLAPIENTRYP PFNEGLCREATEWAYLANDBUFFERFROMIMAGEWL)
(EGLDisplay dpy, EGLImageKHR image);
+typedef struct wl_buffer * (EGLAPIENTRYP
PFNEGLCREATEWAYLANDBUFFERFROMPLANARIMAGESWL) (EGLDisplay dpy, EGLenum format,
EGLImageKHR *images, EGLint n_images);
#endif
diff --git a/src/egl/drivers/dri2/platform_wayland.c
b/src/egl/drivers/dri2/platform_wayland.c
index 089ed62..556a45a 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -698,64 +698,238 @@ dri2_can_handle_format(struct dri2_egl_display *dri2_dpy,
return EGL_FALSE;
}
+struct plane_attribs {
+ struct dri2_egl_image *dri2_img;
+ __DRIimage *image;
+ int offset;
+ int stride;
+ int width;
+ int height;
+ int format;
+};
+
+struct planar_format {
+ enum wl_drm_format wl_format;
+ EGLenum components;
+ int n_planes;
+ struct {
+ int width_shift;
+ int height_shift;
+ uint32_t dri_format;
+ } planes[3];
+};
+
+static const struct planar_format
+planar_formats[] = {
+ { WL_DRM_FORMAT_ARGB, EGL_TEXTURE_RGBA, 1,
+ { { 0, 0, __DRI_IMAGE_FORMAT_ARGB } } },
+
+ { WL_DRM_FORMAT_XRGB, EGL_TEXTURE_RGB, 1,
+ { { 0, 0, __DRI_IMAGE_FORMAT_XRGB }, } },
+
+ { WL_DRM_FORMAT_RGB565, EGL_TEXTURE_RGB
[PATCH mesa 3/3] Adapt the EGL_WL_create_wayland_buffer_from_image extension for YUV
.
+
+The formats accepted in the format parameter are:
+
+ EGL_TEXTURE_RGBA single image is used which contains RGB
+ components.
+ EGL_TEXTURE_RGBA A single image is used which contains RGBA
+ components.
+ EGL_TEXTURE_Y_U_V_WL Three images are used; one containing the
+ luminance component, one containing the
+ U component of the chrominance data and another
+ containing the V component.
+ EGL_TEXTURE_Y_UV_WLTwo images are used; one containing a single
+ luminance component and the other containing the
+ two chrominance components.
+ EGL_TEXTURE_Y_XUXV_WL Two images are used; one containing a single
+ luminance component and the other containing the
+ two chrominance components along with two unused
+ components.
+
If there was an error then the function will return NULL. In particular it
will generate EGL_BAD_MATCH if the implementation is not able to represent
-the image as a wl_buffer. The possible reasons for this error are
+the images as a wl_buffer. The possible reasons for this error are
implementation-dependant but may include problems such as an unsupported
format or tiling mode or that the buffer is in memory that is inaccessible
to the GPU that the given EGLDisplay is using.
+Dependencies on EGL_WL_bind_wayland_display
+
+If EGL_WL_bind_wayland_display is not implemented then references to
+EGL_TEXTURE_Y_U_V_WL, EGL_TEXTURE_Y_UV_WL and EGL_TEXTURE_Y_XUXV_WL are
+void.
+
Issues
1) Under what circumstances can the EGL_BAD_MATCH error be generated? Does
this include for example unsupported tiling modes?
RESOLVED: Yes, the EGL_BAD_MATCH error can be generated for any reason
- which prevents the implementation from representing the image as a
+ which prevents the implementation from representing the images as a
wl_buffer. For example, these problems can be but are not limited to
unsupported tiling modes, inaccessible memory or an unsupported pixel
format.
@@ -99,3 +133,6 @@ Revision History
Initial draft (Neil Roberts)
Version 2, October 25, 2013
Added a note about more possible reasons for returning EGL_BAD_FORMAT.
+Version 3, February 5, 2014
+Support for passing multiple images in order to create a planar
+buffer.
diff --git a/include/EGL/eglmesaext.h b/include/EGL/eglmesaext.h
index b3229e3..de90369 100644
--- a/include/EGL/eglmesaext.h
+++ b/include/EGL/eglmesaext.h
@@ -138,9 +138,9 @@ typedef EGLBoolean (EGLAPIENTRYP
PFNEGLQUERYWAYLANDBUFFERWL) (EGLDisplay dpy, st
#define EGL_WL_create_wayland_buffer_from_image 1
#ifdef EGL_EGLEXT_PROTOTYPES
-EGLAPI struct wl_buffer * EGLAPIENTRY
eglCreateWaylandBufferFromImageWL(EGLDisplay dpy, EGLImageKHR image);
+EGLAPI struct wl_buffer * EGLAPIENTRY
eglCreateWaylandBufferFromImageWL(EGLDisplay dpy, EGLenum format, EGLImageKHR
*images, EGLint n_images);
#endif
-typedef struct wl_buffer * (EGLAPIENTRYP PFNEGLCREATEWAYLANDBUFFERFROMIMAGEWL)
(EGLDisplay dpy, EGLImageKHR image);
+typedef struct wl_buffer * (EGLAPIENTRYP PFNEGLCREATEWAYLANDBUFFERFROMIMAGEWL)
(EGLDisplay dpy, EGLenum format, EGLImageKHR *images, EGLint n_images);
#endif
diff --git a/src/egl/drivers/dri2/platform_wayland.c
b/src/egl/drivers/dri2/platform_wayland.c
index 089ed62..e9262fb 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -698,64 +698,238 @@ dri2_can_handle_format(struct dri2_egl_display *dri2_dpy,
return EGL_FALSE;
}
+struct plane_attribs {
+ struct dri2_egl_image *dri2_img;
+ __DRIimage *image;
+ int offset;
+ int stride;
+ int width;
+ int height;
+ int format;
+};
+
+struct planar_format {
+ enum wl_drm_format wl_format;
+ EGLenum components;
+ int n_planes;
+ struct {
+ int width_shift;
+ int height_shift;
+ uint32_t dri_format;
+ } planes[3];
+};
+
+static const struct planar_format
+planar_formats[] = {
+ { WL_DRM_FORMAT_ARGB, EGL_TEXTURE_RGBA, 1,
+ { { 0, 0, __DRI_IMAGE_FORMAT_ARGB } } },
+
+ { WL_DRM_FORMAT_XRGB, EGL_TEXTURE_RGB, 1,
+ { { 0, 0, __DRI_IMAGE_FORMAT_XRGB }, } },
+
+ { WL_DRM_FORMAT_RGB565, EGL_TEXTURE_RGB, 1,
+ { { 0, 0, __DRI_IMAGE_FORMAT_RGB565 } } },
+
+ { WL_DRM_FORMAT_YUV410, EGL_TEXTURE_Y_U_V_WL, 3,
+ { { 0, 0, __DRI_IMAGE_FORMAT_R8 },
+ { 2, 2, __DRI_IMAGE_FORMAT_R8 },
+ { 2, 2, __DRI_IMAGE_FORMAT_R8 } } },
+
+ { WL_DRM_FORMAT_YUV411, EGL_TEXTURE_Y_U_V_WL, 3,
+ { { 0, 0, __DRI_IMAGE_FORMAT_R8 },
+ { 2, 0, __DRI_IMAGE_FORMAT_R8 },
+ { 2, 0, __DRI_IMAGE_FORMAT_R8 } } },
+
+ { WL_DRM_FORMAT_YUV420
[PATCH mesa 1/3] wayland: Keep track of the formats in a sorted array instead of flags
In order to support YUV formats in CreateWaylandBufferFromImageWL we need to
be able to check whether the compositor supports a larger number of formats so
storing them in flags is a bit awkard. Instead all of the formats are now
stored in a sorted array using wl_array. A binary search is used to check for
the format.
---
src/egl/drivers/dri2/egl_dri2.h | 2 +-
src/egl/drivers/dri2/platform_wayland.c | 85 -
2 files changed, 64 insertions(+), 23 deletions(-)
diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index dfc5927..3f90582 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -131,7 +131,7 @@ struct dri2_egl_display
struct wl_drm*wl_drm;
struct wl_event_queue*wl_queue;
int authenticated;
- int formats;
+ struct wl_array formats;
uint32_t capabilities;
#endif
diff --git a/src/egl/drivers/dri2/platform_wayland.c
b/src/egl/drivers/dri2/platform_wayland.c
index 50750a9..089ed62 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -41,12 +41,6 @@
#include wayland-client.h
#include wayland-drm-client-protocol.h
-enum wl_drm_format_flags {
- HAS_ARGB = 1,
- HAS_XRGB = 2,
- HAS_RGB565 = 4,
-};
-
static void
sync_callback(void *data, struct wl_callback *callback, uint32_t serial)
{
@@ -679,6 +673,31 @@ dri2_swap_buffers(_EGLDriver *drv, _EGLDisplay *disp,
_EGLSurface *draw)
return dri2_swap_buffers_with_damage (drv, disp, draw, NULL, 0);
}
+static EGLBoolean
+dri2_can_handle_format(struct dri2_egl_display *dri2_dpy,
+ enum wl_drm_format format)
+{
+ const enum wl_drm_format *formats;
+ int min, mid, max;
+
+ /* Binary search for the format */
+ min = 0;
+ max = dri2_dpy-formats.size / sizeof (enum wl_drm_format);
+ formats = dri2_dpy-formats.data;
+
+ while (max min) {
+ mid = (max + min) / 2;
+ if (formats[mid] format)
+ min = mid + 1;
+ else if (formats[mid] format)
+ max = mid;
+ else
+ return EGL_TRUE;
+ }
+
+ return EGL_FALSE;
+}
+
static struct wl_buffer *
dri2_create_wayland_buffer_from_image_wl(_EGLDriver *drv,
_EGLDisplay *disp,
@@ -695,12 +714,12 @@ dri2_create_wayland_buffer_from_image_wl(_EGLDriver *drv,
switch (format) {
case __DRI_IMAGE_FORMAT_ARGB:
- if (!(dri2_dpy-formats HAS_ARGB))
+ if (!dri2_can_handle_format(dri2_dpy, WL_DRM_FORMAT_ARGB))
goto bad_format;
wl_format = WL_DRM_FORMAT_ARGB;
break;
case __DRI_IMAGE_FORMAT_XRGB:
- if (!(dri2_dpy-formats HAS_XRGB))
+ if (!dri2_can_handle_format(dri2_dpy, WL_DRM_FORMAT_XRGB))
goto bad_format;
wl_format = WL_DRM_FORMAT_XRGB;
break;
@@ -794,6 +813,7 @@ dri2_terminate(_EGLDriver *drv, _EGLDisplay *disp)
wl_drm_destroy(dri2_dpy-wl_drm);
if (dri2_dpy-own_device)
wl_display_disconnect(dri2_dpy-wl_dpy);
+ wl_array_release(dri2_dpy-formats);
free(dri2_dpy);
disp-DriverData = NULL;
@@ -834,18 +854,36 @@ static void
drm_handle_format(void *data, struct wl_drm *drm, uint32_t format)
{
struct dri2_egl_display *dri2_dpy = data;
-
- switch (format) {
- case WL_DRM_FORMAT_ARGB:
- dri2_dpy-formats |= HAS_ARGB;
- break;
- case WL_DRM_FORMAT_XRGB:
- dri2_dpy-formats |= HAS_XRGB;
- break;
- case WL_DRM_FORMAT_RGB565:
- dri2_dpy-formats |= HAS_RGB565;
- break;
+ enum wl_drm_format *formats = dri2_dpy-formats.data;
+ int n_formats;
+ int min, mid, max;
+
+ n_formats = dri2_dpy-formats.size / sizeof (enum wl_drm_format);
+
+ /* Search for the insert position to keep the format array sorted */
+ min = 0;
+ max = n_formats;
+
+ while (max min) {
+ mid = (max + min) / 2;
+ if (formats[mid] format)
+ min = mid + 1;
+ else if (formats[mid] format)
+ max = mid;
+ else
+ return;
}
+
+ if (wl_array_add(dri2_dpy-formats, sizeof (enum wl_drm_format)) == NULL)
+ return;
+
+ formats = dri2_dpy-formats.data;
+
+ /* Move the subsequent formats out of the way */
+ memmove(formats + min + 1,
+ formats + min,
+ (n_formats - min) * sizeof *formats);
+ formats[min] = format;
}
static void
@@ -980,6 +1018,8 @@ dri2_initialize_wayland(_EGLDriver *drv, _EGLDisplay *disp)
if (!dri2_dpy)
return _eglError(EGL_BAD_ALLOC, eglInitialize);
+ wl_array_init(dri2_dpy-formats);
+
disp-DriverData = (void *) dri2_dpy;
if (disp-PlatformDisplay == NULL) {
dri2_dpy-wl_dpy = wl_display_connect(NULL);
@@ -1050,11 +1090,11 @@ dri2_initialize_wayland(_EGLDriver *drv, _EGLDisplay
*disp)
types = EGL_WINDOW_BIT;
for (i = 0; dri2_dpy-driver_configs[i]; i++)
[PATCH] Don't deref the sample pointer in the wl_container_of macro
The previous implementation of the wl_container_of macro was
dereferencing the sample pointer in order to get an address of the
member to calculate the offset. Ideally this shouldn't cause any
problems because the dereference doesn't actually cause the address to
be read from so it shouldn't matter if the pointer is uninitialised.
However this is probably technically invalid and could cause undefined
behavior. Clang appears to take advantage of this undefined behavior
and doesn't bother doing the subtraction. It also gives a warning when
it does this.
The documentation for wl_container_of implies that it should only be
given an initialised pointer and if that is done then there is no
problem with clang. However this is quite easy to forget and doesn't
cause any problems or warnings with gcc so it's quite easy to
accidentally break clang.
To fix the problem this changes the macro to use pointer -
offsetof(__typeof__(sample), member) so that it doesn't need to deref
the sample pointer. This does however require that the __typeof__
operator is supported by the compiler. In practice we probably only
care about gcc and clang and both of these happily support the
operator.
The previous implementation was also using __typeof__ but it had a
fallback path avoiding it when the operator isn't available. The
fallback effectively has undefined behaviour and it is targetting
unknown compilers so it is probably not a good idea to leave it in.
Instead, this patch just removes it. If someone finds a compiler that
doesn't have __typeof__ but does work with the old implementation then
maybe they could add it back in as a special case.
This patch removes the initialisation anywhere where the sample
pointer was being unitialised before using wl_container_of. The
documentation for the macro has also been updated to specify that this
is OK.
---
src/wayland-server.c | 2 +-
src/wayland-server.h | 2 +-
src/wayland-util.h | 30 +-
3 files changed, 15 insertions(+), 19 deletions(-)
diff --git a/src/wayland-server.c b/src/wayland-server.c
index 489b99d..294497d 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -571,7 +571,7 @@ wl_resource_get_link(struct wl_resource *resource)
WL_EXPORT struct wl_resource *
wl_resource_from_link(struct wl_list *link)
{
- struct wl_resource *resource = NULL;
+ struct wl_resource *resource;
return wl_container_of(link, resource, link);
}
diff --git a/src/wayland-server.h b/src/wayland-server.h
index 3fcaaf6..7fc5b47 100644
--- a/src/wayland-server.h
+++ b/src/wayland-server.h
@@ -162,7 +162,7 @@ wl_client_post_no_memory(struct wl_client *client);
* \code
* void your_listener(struct wl_listener *listener, void *data)
* {
- * struct your_data *data = NULL;
+ * struct your_data *data;
*
* your_data = wl_container_of(listener, data, your_member_name);
* }
diff --git a/src/wayland-util.h b/src/wayland-util.h
index 68d91e2..57764d9 100644
--- a/src/wayland-util.h
+++ b/src/wayland-util.h
@@ -135,7 +135,7 @@ void wl_list_insert_list(struct wl_list *list, struct
wl_list *other);
*
* void example_container_destroy(struct wl_listener *listener, void *data)
* {
- * struct example_container *ctr = NULL;
+ * struct example_container *ctr;
*
* ctr = wl_container_of(listener, ctr, destroy_listener);
* \comment{destroy ctr...}
@@ -144,44 +144,40 @@ void wl_list_insert_list(struct wl_list *list, struct
wl_list *other);
*
* \param ptr A valid pointer to the contained item.
*
- * \param sample A pointer to the type of content that the list item stores.
- * Sample does not need be a valid pointer; a null pointer will suffice.
+ * \param sample A pointer to the type of content that the list item
+ * stores. Sample does not need be a valid pointer; a null or
+ * an uninitialised pointer will suffice.
*
* \param member The named location of ptr within the sample type.
*
* \return The container for the specified pointer.
*/
-#ifdef __GNUC__
#define wl_container_of(ptr, sample, member) \
- (__typeof__(sample))((char *)(ptr) - \
-((char *)(sample)-member - (char *)(sample)))
-#else
-#define wl_container_of(ptr, sample, member) \
- (void *)((char *)(ptr) - \
-((char *)(sample)-member - (char *)(sample)))
-#endif
+ (__typeof__(sample))((char *)(ptr) -\
+offsetof(__typeof__(*sample), member))
+/* If the above macro causes problems on your compiler you might be
+ * able to find an alternative name for the non-standard __typeof__
+ * operator and add a special case here */
#define wl_list_for_each(pos, head, member)\
- for (pos = 0, pos = wl_container_of((head)-next, pos, member); \
+ for (pos =
Re: Make safe double remove?
Jonathan Howard jonat...@unbiased.name writes: wl_list_remove changed isn't 100% guaranteed to not break anything, on the other hand there is no guarantee the code does not have the fatal double remove already. We definitely do already have a double-remove bug in Weston as described here: http://lists.freedesktop.org/archives/wayland-devel/2014-January/012891.html I think the proposed change to wl_list_remove would fix that too. Regards, - Neil pgpe_dAD0aq9Y.pgp Description: PGP signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: weston crash in destroy_shell_surface
It looks like the rest of the code assumes that shsurf-children_link
is always a consistent list. For example, shell_surface_set_parent
resets children_link to the empty list after it unlinks a child from
its parent. The destroy_shell_surface code just calls wl_list_remove
which leaves the list node with invalid NULL pointers. Maybe the
simplest way to fix it is just to call shell_surface_set_parent in
that code instead of directly manipulating the list. Here is a patch
to do that.
Regards,
- Neil
--- 8 --- (use git am --scissors to automatically chop here)
Subject: shell: Use shell_surface_set_parent to unlink children on destroy
Previously when a shell surface is destroyed it would walk through the
list of child surfaces and directly remove their children_link from
the list. This would leave children_link as an invalid list with just
NULL pointers for prev and next. Other parts of the code such as
shell_surface_set_parent assume that children_link is always in a
consistent state (ie, it is the list of sibling surfaces).
destroy_shell_surface also assumes that it can unconditionally call
wl_list_remove on the children_link of the surface being destroyed to
unlink it from its parent if there is one. However, this crashes if
the child's parent is destroyed first because it would leave the child
with an invalid children_link.
To fix it this patch makes it call shell_surface_set_parent(..., NULL)
when unlinking the surface's children instead of directly manipulating
the list because that function will reset children_link to an empty
list leaving it in a consistent state.
---
desktop-shell/shell.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 2f8e610..e3e2c96 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -2914,10 +2914,8 @@ destroy_shell_surface(struct shell_surface *shsurf)
weston_view_destroy(shsurf-view);
wl_list_remove(shsurf-children_link);
- wl_list_for_each_safe(child, next, shsurf-children_list,
children_link) {
- wl_list_remove(child-children_link);
- child-parent = NULL;
- }
+ wl_list_for_each_safe(child, next, shsurf-children_list,
children_link)
+ shell_surface_set_parent(child, NULL);
wl_list_remove(shsurf-link);
free(shsurf);
--
1.8.4.2
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] westoy: Add an option to explicitly disable cairo on a widget
This problem was found while looking at the following bug:
https://bugs.freedesktop.org/show_bug.cgi?id=72612
It turns out the patch doesn't help to fix the bug but I think it
would be a good thing to do anyway.
--- 8 --- (use git am --scissors to automatically chop here)
The subsurfaces example creates a subsurface widget and uses EGL to
render to it directly rather than using the cairo context from the
widget. In theory this shouldn't cause any problems because the westoy
window code lazily creates the cairo surface when an application
creates a cairo context. However commit fdca95c7 changed the behaviour
to force the lazy creation at the beginning of each surface redraw.
This ends up making the triangle surface get two attaches – one from
Cairo and one from the direct EGL.
It looks like it would be difficult to reinstate the lazy surface
creation behaviour whilst still maintaining the error handling for
surface creation because none of the redraw handlers in the example
clients are designed to cope with that. Instead, this patch adds an
explicit option on a widget to disable creating the Cairo surface and
the subsurface example now uses that.
---
clients/subsurfaces.c | 1 +
clients/window.c | 18 +-
clients/window.h | 2 ++
3 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/clients/subsurfaces.c b/clients/subsurfaces.c
index 0f11009..45cc44b 100644
--- a/clients/subsurfaces.c
+++ b/clients/subsurfaces.c
@@ -498,6 +498,7 @@ triangle_create(struct window *window, struct egl_state
*egl)
tri-egl = egl;
tri-widget = window_add_subsurface(window, tri,
int_to_mode(option_triangle_mode));
+ widget_set_use_cairo(tri-widget, 0);
widget_set_resize_handler(tri-widget, triangle_resize_handler);
widget_set_redraw_handler(tri-widget, triangle_redraw_handler);
diff --git a/clients/window.c b/clients/window.c
index 43761ca..5a8df43 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ -285,6 +285,11 @@ struct widget {
int opaque;
int tooltip_count;
int default_cursor;
+ /* If this is set to false then no cairo surface will be
+* created before redrawing the surface. This is useful if the
+* redraw handler is going to do completely custom rendering
+* such as using EGL directly */
+ int use_cairo;
};
struct touch_point {
@@ -1608,6 +1613,7 @@ widget_create(struct window *window, struct surface
*surface, void *data)
widget-tooltip = NULL;
widget-tooltip_count = 0;
widget-default_cursor = CURSOR_LEFT_PTR;
+ widget-use_cairo = 1;
return widget;
}
@@ -1706,6 +1712,8 @@ widget_get_cairo_surface(struct widget *widget)
struct surface *surface = widget-surface;
struct window *window = widget-window;
+ assert(widget-use_cairo);
+
if (!surface-cairo_surface) {
if (surface == window-main_surface)
window_create_main_surface(window);
@@ -1938,6 +1946,13 @@ widget_schedule_redraw(struct widget *widget)
window_schedule_redraw_task(widget-window);
}
+void
+widget_set_use_cairo(struct widget *widget,
+int use_cairo)
+{
+ widget-use_cairo = use_cairo;
+}
+
cairo_surface_t *
window_get_surface(struct window *window)
{
@@ -3942,7 +3957,8 @@ surface_redraw(struct surface *surface)
wl_callback_destroy(surface-frame_cb);
}
- if (!widget_get_cairo_surface(surface-widget)) {
+ if (surface-widget-use_cairo
+ !widget_get_cairo_surface(surface-widget)) {
DBG_OBJ(surface-surface, cancelled due buffer failure\n);
return -1;
}
diff --git a/clients/window.h b/clients/window.h
index cf8fc6c..1e12374 100644
--- a/clients/window.h
+++ b/clients/window.h
@@ -507,6 +507,8 @@ widget_set_axis_handler(struct widget *widget,
widget_axis_handler_t handler);
void
widget_schedule_redraw(struct widget *widget);
+void
+widget_set_use_cairo(struct widget *widget, int use_cairo);
struct widget *
window_frame_create(struct window *window, void *data);
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] tests: Test whether a simple EGL main loop uses too many buffers
Hi,
Here is a rebased version of the patch. The old one didn't apply
cleanly anymore because of some changes in tests/Makefile.am
I think there is some confusion about why it ends up using 3 buffers
and I also keep getting in a muddle about it. I'll try to describe
what I think is happening here.
The client has a loop like this:
while (1) {
draw_something();
eglSwapBuffers();
}
It begins its first frame. In draw_something it will call get_back_bo
which will allocate a buffer because there are currently no buffers.
It will then call eglSwapBuffers which will install a frame callback
and give the buffer to the compositor. As this is the first buffer,
the compositor won't send a release event yet.
The client will now start rendering its second frame. It will call
get_back_bo which will detect that the first buffer is still locked so
it will allocate a second buffer. It will then call eglSwapBuffers
which will cause it to block for the frame callback. There is still no
release event so this won't cause any buffers to be unlocked. Once the
frame callback is received it will commit the second buffer. The
compositor only needs to keep hold of one buffer so it will
immediately queue a release event. (It could alternatively immediately
post the event here but that doesn't actually fix the problem.) After
the commit the client will install a frame callback. This will
guarantee that the release event will be flushed to the client
eventually.
Now the client will start rendering its third frame. The release event
may or may not have been received in the client's socket buffer
depending on the timing. It doesn't actually matter though and the bug
will be triggered either way. The first thing it does while rendering
is call get_back_bo. The first thing get_back_bo does is dispatch any
pending events in its queue. However this doesn't do anything for the
release event because nothing has read from the socket yet so it is
still only sitting the socket's buffer. At this point the client
hasn't seen any release events yet so it will think both buffers are
still locked and it will allocate a third buffer. Eventually the
client will finish rendering to this third buffer and it will call
eglSwapBuffers. At this point the client will block on the frame
callback which will end up reading from the socket and dispatching the
release event, but it is too late.
When the client calls get_back_bo for its fourth frame it will have
seen one release event so it will reuse the first buffer. It will
continue like this cycling between three buffers.
I hope that makes sense!
Regards,
- Neil
--- 8 --- (use git am --scissors to automatically chop here)
This adds a test that tries to simulate a simple game loop that would
be like this:
while (1) {
draw_something();
eglSwapBuffers();
}
In this case the test is relying on eglSwapBuffers to throttle to a
sensible frame rate.
The test then verifies that only 2 EGL buffers are used. This is done
via a new request and event in the wayland-test protocol.
Currently this causes 3 buffers to be created because the release
event generated by the swap buffers is not processed by Mesa until it
blocks for the frame complete event in the next swap buffers call, but
that is too late.
This can be fixed in Mesa by issuing a sync request after the swap
buffers and blocking on it before deciding whether to allocate a new
buffer.
---
configure.ac | 7 +++
protocol/wayland-test.xml | 7 +++
tests/Makefile.am | 15 -
tests/buffer-count-test.c | 128 ++
tests/weston-test-client-helper.c | 22 ++-
tests/weston-test-client-helper.h | 4 ++
tests/weston-test.c | 51 ++-
7 files changed, 231 insertions(+), 3 deletions(-)
create mode 100644 tests/buffer-count-test.c
diff --git a/configure.ac b/configure.ac
index 86940d5..6e57c25 100644
--- a/configure.ac
+++ b/configure.ac
@@ -61,10 +61,17 @@ COMPOSITOR_MODULES=wayland-server = 1.3.90 pixman-1
AC_ARG_ENABLE(egl, [ --disable-egl],,
enable_egl=yes)
+AC_ARG_ENABLE(egl-tests, [ --enable-egl-tests],,
+ enable_egl_tests=yes)
AM_CONDITIONAL(ENABLE_EGL, test x$enable_egl = xyes)
+AM_CONDITIONAL(ENABLE_EGL_TESTS, test x$enable_egl = xyes -a
x$enable_egl_tests = xyes)
if test x$enable_egl = xyes; then
AC_DEFINE([ENABLE_EGL], [1], [Build Weston with EGL support])
PKG_CHECK_MODULES(EGL, [egl = 7.10 glesv2])
+
+if test x$enable_egl_tests = xyes; then
+ PKG_CHECK_MODULES([EGL_TESTS], [egl = 7.10 glesv2 wayland-client
wayland-egl])
+fi
fi
AC_ARG_ENABLE(xkbcommon,
diff --git a/protocol/wayland-test.xml b/protocol/wayland-test.xml
index 2993f08..18b6625 100644
--- a/protocol/wayland-test.xml
+++ b/protocol/wayland-test.xml
@@ -51,5 +51,12 @@
arg name=x type=fixed/
arg name=y type=fixed/
/event
Re: [PATCH] Flush the wl_display at the end of SwapBuffers
I think this is a good idea. Imagine a simple game main loop like this:
while (TRUE) {
do_some_game_logic();
redraw();
eglSwapBuffers();
}
In that case it wouldn't flush the commit until the right at the
beginning of each call to eglSwapBuffers when it waits for the frame
callback. Then it would have to block waiting for the compositor to
finish rendering before it receives the frame callback. If we dispatch
after the swap buffers then in this case the compositor could start
rendering while the game is in do_some_game_logic().
There was a similar patch proposed on the Mesa-dev mailing list here:
http://lists.freedesktop.org/archives/mesa-dev/2013-August/043697.html
That also points out the additional problem case where the rendering is
done in a separate thread. After the rendering is done potentially
nothing would wake up the main loop thread and cause a dispatch.
That patch also moves the call to dri2_dpy-flush-flush further up. I'm
not sure if that is necessary. It also adds a pointer to the wl_display
in struct dri2_egl_surface which definitely looks unnecessary. So I
think Axel's patch makes more sense.
Reviewed-by: Neil Roberts n...@linux.intel.com
Regards,
- Neil
Axel Davy axel.d...@ens.fr writes:
We would like the compositor to receive the commited buffer
as soon as possible, so it has the time to treat it, and
release old ones. We shouldn't rely on the client
to flush the queue for us.
Signed-off-by: Axel Davy axel.d...@ens.fr
---
We flush the wl_display after we flush the drawable.
src/egl/drivers/dri2/platform_wayland.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/egl/drivers/dri2/platform_wayland.c
b/src/egl/drivers/dri2/platform_wayland.c
index 7e3733b..8c3d1f1 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -627,6 +627,8 @@ dri2_swap_buffers_with_damage(_EGLDriver *drv,
(*dri2_dpy-flush-flush)(dri2_surf-dri_drawable);
(*dri2_dpy-flush-invalidate)(dri2_surf-dri_drawable);
+ wl_display_flush(dri2_dpy-wl_dpy);
+
return EGL_TRUE;
}
--
1.8.1.2
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
pgpfefzQHS56N.pgp
Description: PGP signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] Automatically generate weston.ini with the right paths
Kristian Høgsberg hoegsb...@gmail.com writes:
That's a nice idea. Could we just generate it from configure.ac by
listing it in AC_CONFIG_FILES?
Sadly that doesn't work because the autoconf expansions still include
variables for make. Eg, @bindir@ expands to ${exec_prefix}/bin. I think
that is needed so that you can use DESTDIR to change the prefix without
rerunning configure. This trick of doing it with sed is recommended in
the autoconf manual: http://is.gd/YivZyl (near the bottom).
Also, I was thinking that we should add a comment to the top of the
file saying that this is a sample weston.ini, that there's a
weston.ini man page, and that you can copy the sample weston.ini to
~/.config/weston.ini or /etc/xdg/weston/weston.ini.
Yes, that would be nice. I didn't discover that until yesterday! Perhaps
that would make more sense as a separate patch.
Regards,
- Neil
pgpAG_T_F1M6r.pgp
Description: PGP signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 0/10] Add a mechanism for nested compositors to use subsurfaces without blitting
Hi, I think this thread has gotten a bit tangled so I've done a bit of minor rebasing for the patches and pushed them all to github: https://github.com/bpeel/wayland/commits/wip/wayland-subcompositor https://github.com/bpeel/mesa/commits/wip/wayland-subcompositor https://github.com/bpeel/weston/commits/wip/wayland-subcompositor Regards, - Neil pgpkWCY7gPFId.pgp Description: PGP signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] Automatically generate weston.ini with the right paths
Previously weston.ini had hardcoded paths for the weston-* clients in /usr/bin and /usr/libexec. This was a bit annoying when testing Weston because you wouldn't usually install those in the system prefix. This patch adds a make rule to automatically generate weston.ini from a template file with some replacement markers for the paths so that they can have the right prefix. --- .gitignore| 1 + Makefile.am | 11 +- weston.ini| 67 --- weston.ini.in | 67 +++ 4 files changed, 78 insertions(+), 68 deletions(-) delete mode 100644 weston.ini create mode 100644 weston.ini.in diff --git a/.gitignore b/.gitignore index b3fb2a1..111c56c 100644 --- a/.gitignore +++ b/.gitignore @@ -28,6 +28,7 @@ cscope.out /missing /stamp-h1 /test-driver +/weston.ini Makefile Makefile.in TAGS diff --git a/Makefile.am b/Makefile.am index e9ecc38..3a7d337 100644 --- a/Makefile.am +++ b/Makefile.am @@ -6,4 +6,13 @@ SUBDIRS = shared src clients data protocol tests $(wcap_subdir) man DISTCHECK_CONFIGURE_FLAGS = --disable-setuid-install -EXTRA_DIST = weston.ini wayland-scanner.mk +EXTRA_DIST = weston.ini.in wayland-scanner.mk + +weston.ini : $(srcdir)/weston.ini.in + $(AM_V_GEN)$(SED) \ + -e 's|@bindir[@]|$(bindir)|g' \ + -e 's|@abs_top_builddir[@]|$(abs_top_builddir)|g' \ + -e 's|@libexecdir[@]|$(libexecdir)|g' \ + $ $@ + +all-local : weston.ini diff --git a/weston.ini b/weston.ini deleted file mode 100644 index 4761bed..000 --- a/weston.ini +++ /dev/null @@ -1,67 +0,0 @@ -[core] -#modules=xwayland.so,cms-colord.so -#shell=desktop-shell.so -#gbm-format=xrgb2101010 - -[shell] -background-image=/usr/share/backgrounds/gnome/Aqua.jpg -background-color=0xff002244 -background-type=tile -panel-color=0x90ff -locking=true -animation=zoom -startup-animation=fade -#binding-modifier=ctrl -#num-workspaces=6 -#cursor-theme=whiteglass -#cursor-size=24 - -#lockscreen-icon=/usr/share/icons/gnome/256x256/actions/lock.png -#lockscreen=/usr/share/backgrounds/gnome/Garden.jpg -#homescreen=/usr/share/backgrounds/gnome/Blinds.jpg -#animation=fade - -[launcher] -icon=/usr/share/icons/gnome/24x24/apps/utilities-terminal.png -path=/usr/bin/gnome-terminal - -[launcher] -icon=/usr/share/icons/gnome/24x24/apps/utilities-terminal.png -path=/usr/bin/weston-terminal - -[launcher] -icon=/usr/share/icons/hicolor/24x24/apps/google-chrome.png -path=/usr/bin/google-chrome - -[launcher] -icon=/usr/share/icons/gnome/24x24/apps/arts.png -path=./clients/weston-flower - -[screensaver] -# Uncomment path to disable screensaver -path=/usr/libexec/weston-screensaver -duration=600 - -[input-method] -path=/usr/libexec/weston-keyboard - -#[output] -#name=LVDS1 -#mode=1680x1050 -#transform=90 -#icc_profile=/usr/share/color/icc/colord/Bluish.icc - -#[output] -#name=VGA1 -#mode=173.00 1920 2048 2248 2576 1080 1083 1088 1120 -hsync +vsync -#transform=flipped - -#[output] -#name=X1 -#mode=1024x768 -#transform=flipped-270 - -#[touchpad] -#constant_accel_factor = 50 -#min_accel_factor = 0.16 -#max_accel_factor = 1.0 diff --git a/weston.ini.in b/weston.ini.in new file mode 100644 index 000..5181a9e --- /dev/null +++ b/weston.ini.in @@ -0,0 +1,67 @@ +[core] +#modules=xwayland.so,cms-colord.so +#shell=desktop-shell.so +#gbm-format=xrgb2101010 + +[shell] +background-image=/usr/share/backgrounds/gnome/Aqua.jpg +background-color=0xff002244 +background-type=tile +panel-color=0x90ff +locking=true +animation=zoom +startup-animation=fade +#binding-modifier=ctrl +#num-workspaces=6 +#cursor-theme=whiteglass +#cursor-size=24 + +#lockscreen-icon=/usr/share/icons/gnome/256x256/actions/lock.png +#lockscreen=/usr/share/backgrounds/gnome/Garden.jpg +#homescreen=/usr/share/backgrounds/gnome/Blinds.jpg +#animation=fade + +[launcher] +icon=/usr/share/icons/gnome/24x24/apps/utilities-terminal.png +path=/usr/bin/gnome-terminal + +[launcher] +icon=/usr/share/icons/gnome/24x24/apps/utilities-terminal.png +path=@bindir@/weston-terminal + +[launcher] +icon=/usr/share/icons/hicolor/24x24/apps/google-chrome.png +path=/usr/bin/google-chrome + +[launcher] +icon=/usr/share/icons/gnome/24x24/apps/arts.png +path=@abs_top_builddir@/clients/weston-flower + +[screensaver] +# Uncomment path to disable screensaver +path=@libexecdir@/weston-screensaver +duration=600 + +[input-method] +path=@libexecdir@/weston-keyboard + +#[output] +#name=LVDS1 +#mode=1680x1050 +#transform=90 +#icc_profile=/usr/share/color/icc/colord/Bluish.icc + +#[output] +#name=VGA1 +#mode=173.00 1920 2048 2248 2576 1080 1083 1088 1120 -hsync +vsync +#transform=flipped + +#[output] +#name=X1 +#mode=1024x768 +#transform=flipped-270 + +#[touchpad] +#constant_accel_factor = 50 +#min_accel_factor = 0.16 +#max_accel_factor = 1.0 -- 1.8.3.1 ___ wayland-devel mailing list
Re: Thoughts about decoration information in the xdg_shell
Thiago Macieira thiago.macie...@intel.com writes: Make it simpler: all clients MUST be able to draw decorations. That's what Wayland up until now requires anyway. I think it's a shame to throw out the idea of making the policy be that clients are allowed to expect SSD if they don't want to draw decorations themselves. Requiring CSD support only makes it simpler for compositor developers, but it adds a lot of burden on things like SDL, glut and applications that really just want a space to render GL content into. I guess you could make a toolkit-agnostic decorations library using subsurfaces that these types of applications can use. However I don't think that will solve the consistency issue because most game-type applications will want to bundle all of their dependencies so they will end up wanting to bundle this library. The consistency will then break when the distro updates its version of the library. I think the most important decision to make for xdg-shell is whether to require CSD support or SSD support. How it is actually negotatied is not as important. I think you have to have a policy of requiring support for one or the other because it'd be a mess to have a situation where some apps can't work on certain compositors. If Gnome Shell doesn't add support for SSD then I suppose that effectively mandates CSD support in clients that want to be portable regardless of what is specified in xdg-shell. It looks like the main incentive to not require SSD support is that it creates work for the Gnome Shell developers. However you have to bear in mind that requiring CSD also creates work for all other toolkit developers which may turn out to be more work overall. Regards, - Neil pgpUPMn3FzCC9.pgp Description: PGP signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH mesa 1/2] wayland: Block for the frame callback in get_back_bo not dri2_swap_buffers
As per Kristian's suggestion we can avoid the problem of effectively
disabling the event queuing mechanism by only doing the sync request
when the swap interval is zero. To fix the bug of using three buffers
we can just block for the frame callback in get_back_bo instead of
swap_buffers. I was originally a bit reluctant to do this because it
will end up blocking in an unusual place (ie, usually in glClear) but
I suppose that doesn't really matter.
I've split that change out into a separate patch because it is an
independent problem. The new swap interval patch can now share the
callback variable with the frame callback because they are now blocked
for in the same place.
Just to note, in addition to these two patches I think it would be
good to apply these other two patches which are buried in this thread
(although they are not required):
http://lists.freedesktop.org/archives/wayland-devel/2013-October/011766.html
http://lists.freedesktop.org/archives/wayland-devel/2013-September/010967.html
- Neil
--- 8 --- (use git am --scissors to automatically chop here)
Consider a typical game-style main loop which might be like this:
while (1) {
draw_something();
eglSwapBuffers();
}
In this case the game is relying on eglSwapBuffers to throttle to a sensible
frame rate. Previously this game would end up using three buffers even though
it should only need two. This is because Mesa decides whether to allocate a
new buffer in get_back_bo which would be before it has tried to read any
events from the compositor so it wouldn't have seen any buffer release events
yet.
This patch just moves the block for the frame callback to get_back_bo.
Typically the compositor will send a release event immediately after one of
the attaches so if we block for the frame callback here then we can be sure to
have completed at least one roundtrip and received that release event after
attaching the previous buffer before deciding whether to allocate a new one.
dri2_swap_buffers always calls get_back_bo so even if the client doesn't
render anything we will still be sure to block to the frame callback. The code
to create the new frame callback has been moved to after this call so that we
can be sure to have cleared the previous frame callback before requesting a
new one.
---
src/egl/drivers/dri2/platform_wayland.c | 34 +++--
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/src/egl/drivers/dri2/platform_wayland.c
b/src/egl/drivers/dri2/platform_wayland.c
index f9065bb..5ce8981 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -263,8 +263,19 @@ get_back_bo(struct dri2_egl_surface *dri2_surf)
dri2_egl_display(dri2_surf-base.Resource.Display);
int i;
- /* There might be a buffer release already queued that wasn't processed */
- wl_display_dispatch_queue_pending(dri2_dpy-wl_dpy, dri2_dpy-wl_queue);
+ if (dri2_surf-frame_callback == NULL) {
+ /* There might be a buffer release already queued that wasn't processed
*/
+ wl_display_dispatch_queue_pending(dri2_dpy-wl_dpy, dri2_dpy-wl_queue);
+ } else {
+ /* We throttle to the frame callback here so that we can be sure to have
+ * received any release events before trying to decide whether to
+ * allocate a new buffer */
+ do {
+ if (wl_display_dispatch_queue(dri2_dpy-wl_dpy,
+ dri2_dpy-wl_queue) == -1)
+return EGL_FALSE;
+ } while (dri2_surf-frame_callback != NULL);
+ }
if (dri2_surf-back == NULL) {
for (i = 0; i ARRAY_SIZE(dri2_surf-color_buffers); i++) {
@@ -557,18 +568,7 @@ dri2_swap_buffers_with_damage(_EGLDriver *drv,
{
struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
struct dri2_egl_surface *dri2_surf = dri2_egl_surface(draw);
- int i, ret = 0;
-
- while (dri2_surf-frame_callback ret != -1)
- ret = wl_display_dispatch_queue(dri2_dpy-wl_dpy, dri2_dpy-wl_queue);
- if (ret 0)
- return EGL_FALSE;
-
- dri2_surf-frame_callback = wl_surface_frame(dri2_surf-wl_win-surface);
- wl_callback_add_listener(dri2_surf-frame_callback,
-frame_listener, dri2_surf);
- wl_proxy_set_queue((struct wl_proxy *) dri2_surf-frame_callback,
- dri2_dpy-wl_queue);
+ int i;
for (i = 0; i ARRAY_SIZE(dri2_surf-color_buffers); i++)
if (dri2_surf-color_buffers[i].age 0)
@@ -581,6 +581,12 @@ dri2_swap_buffers_with_damage(_EGLDriver *drv,
return EGL_FALSE;
}
+ dri2_surf-frame_callback = wl_surface_frame(dri2_surf-wl_win-surface);
+ wl_callback_add_listener(dri2_surf-frame_callback,
+frame_listener, dri2_surf);
+ wl_proxy_set_queue((struct wl_proxy *) dri2_surf-frame_callback,
+ dri2_dpy-wl_queue);
+
dri2_surf-back-age = 1;
dri2_surf-current = dri2_surf-back;
dri2_surf-back = NULL;
[PATCH mesa v5 2/2] wayland: Add support for eglSwapInterval
The Wayland EGL platform now respects the eglSwapInterval value. The value is
clamped to either 0 or 1 because it is difficult (and probably not useful) to
sync to more than 1 redraw.
The main change is that if the swap interval is 0 then Mesa won't install a
frame callback so that eglSwapBuffers can be executed as often as necessary.
Instead it will do a sync request after the swap buffers. It will block for
sync complete event in get_back_bo instead of the frame callback. The
compositor is likely to send a release event while processing the new buffer
attach and this makes sure we will receive that before deciding whether to
allocate a new buffer.
If there are no buffers available then instead of returning with an error,
get_back_bo will now poll the compositor by repeatedly sending sync requests
every 10ms. This is a last resort and in theory this shouldn't happen because
there should be no reason for the compositor to hold on to more than three
buffers. That means whenever we attach the fourth buffer we should always get
an immediate release event which should come in with the notification for the
first sync request that we are throttled to.
When the compositor is directly scanning out from the application's buffer it
may end up holding on to three buffers. These are the one that is is currently
scanning out from, one that has been given to DRM as the next buffer to flip
to, and one that has been attached and will be given to DRM as soon as the
previous flip completes. When we attach a fourth buffer to the compositor it
should replace that third buffer so we should get a release event immediately
after that. This patch therefore also changes the number of buffer slots to 4
so that we can accomodate that situation.
If DRM eventually gets a way to cancel a pending page flip then the compositors
can be changed to only need to hold on to two buffers and this value can be
put back to 3.
This also moves the vblank configuration defines from platform_x11.c to the
common egl_dri2.h header so they can be shared by both platforms.
---
src/egl/drivers/dri2/egl_dri2.h | 10 +-
src/egl/drivers/dri2/platform_wayland.c | 199 +++-
src/egl/drivers/dri2/platform_x11.c | 6 -
3 files changed, 179 insertions(+), 36 deletions(-)
diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index 8c303d9..d29dd98 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -174,7 +174,7 @@ struct dri2_egl_surface
struct wl_egl_window *wl_win;
intdx;
intdy;
- struct wl_callback*frame_callback;
+ struct wl_callback*throttle_callback;
int format;
#endif
@@ -194,7 +194,7 @@ struct dri2_egl_surface
#endif
int locked;
int age;
- } color_buffers[3], *back, *current;
+ } color_buffers[4], *back, *current;
#endif
#ifdef HAVE_ANDROID_PLATFORM
@@ -220,6 +220,12 @@ struct dri2_egl_image
__DRIimage *dri_image;
};
+/* From xmlpool/options.h, user exposed so should be stable */
+#define DRI_CONF_VBLANK_NEVER 0
+#define DRI_CONF_VBLANK_DEF_INTERVAL_0 1
+#define DRI_CONF_VBLANK_DEF_INTERVAL_1 2
+#define DRI_CONF_VBLANK_ALWAYS_SYNC 3
+
/* standard typecasts */
_EGL_DRIVER_STANDARD_TYPECASTS(dri2_egl)
_EGL_DRIVER_TYPECAST(dri2_egl_image, _EGLImage, obj)
diff --git a/src/egl/drivers/dri2/platform_wayland.c
b/src/egl/drivers/dri2/platform_wayland.c
index 5ce8981..fca35db 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -34,6 +34,7 @@
#include unistd.h
#include fcntl.h
#include xf86drm.h
+#include poll.h
#include egl_dri2.h
@@ -183,8 +184,16 @@ dri2_create_window_surface(_EGLDriver *drv, _EGLDisplay
*disp,
_EGLConfig *conf, EGLNativeWindowType window,
const EGLint *attrib_list)
{
- return dri2_create_surface(drv, disp, EGL_WINDOW_BIT, conf,
+ struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
+ _EGLSurface *surf;
+
+ surf = dri2_create_surface(drv, disp, EGL_WINDOW_BIT, conf,
window, attrib_list);
+
+ if (surf != NULL)
+ drv-API.SwapInterval(drv, disp, surf, dri2_dpy-default_swap_interval);
+
+ return surf;
}
/**
@@ -217,8 +226,8 @@ dri2_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp,
_EGLSurface *surf)
dri2_dpy-dri2-releaseBuffer(dri2_dpy-dri_screen,
dri2_surf-dri_buffers[i]);
- if (dri2_surf-frame_callback)
- wl_callback_destroy(dri2_surf-frame_callback);
+ if (dri2_surf-throttle_callback)
+ wl_callback_destroy(dri2_surf-throttle_callback);
if (dri2_surf-base.Type == EGL_WINDOW_BIT) {
dri2_surf-wl_win-private = NULL;
@@ -257,41 +266,99 @@ dri2_release_buffers(struct dri2_egl_surface *dri2_surf)
}
static int
+poll_compositor(struct
[PATCH] Add documentation for wl_shm_buffer_begin/end_access
It's not obvious that these functions are needed so it would be good
to have some documentation for them.
---
doc/doxygen/Makefile.am | 3 ++-
src/wayland-shm.c | 64 +
2 files changed, 66 insertions(+), 1 deletion(-)
diff --git a/doc/doxygen/Makefile.am b/doc/doxygen/Makefile.am
index 0351c1e..078506d 100644
--- a/doc/doxygen/Makefile.am
+++ b/doc/doxygen/Makefile.am
@@ -14,7 +14,8 @@ scanned_src_files_client =\
scanned_src_files_server = \
$(scanned_src_files_shared) \
$(top_srcdir)/src/wayland-server.c \
- $(top_srcdir)/src/wayland-server.h
+ $(top_srcdir)/src/wayland-server.h \
+ $(top_srcdir)/src/wayland-shm.c
# find all man/man3/wl_foo.3 pages
# for this to work, we need to create them before the man target (hence
diff --git a/src/wayland-shm.c b/src/wayland-shm.c
index 28f52f4..814a4cf 100644
--- a/src/wayland-shm.c
+++ b/src/wayland-shm.c
@@ -358,6 +358,23 @@ wl_shm_buffer_get_stride(struct wl_shm_buffer *buffer)
return buffer-stride;
}
+
+/** Get a pointer to the memory for the SHM buffer
+ *
+ * \param buffer The buffer object
+ *
+ * Returns a pointer which can be used to read the data contained in
+ * the given SHM buffer.
+ *
+ * As this buffer is memory-mapped, reading it from may generate
+ * SIGBUS signals. This can happen if the client claims that the
+ * buffer is larger than it is or if something truncates the
+ * underlying file. To prevent this signal from causing the compositor
+ * to crash you should call wl_shm_buffer_begin_access and
+ * wl_shm_buffer_end_access around code that reads from the memory.
+ *
+ * \memberof wl_shm_buffer
+ */
WL_EXPORT void *
wl_shm_buffer_get_data(struct wl_shm_buffer *buffer)
{
@@ -454,6 +471,42 @@ init_sigbus_data_key(void)
pthread_key_create(wl_shm_sigbus_data_key, destroy_sigbus_data);
}
+/** Mark that the given SHM buffer is about to be accessed
+ *
+ * \param buffer The SHM buffer
+ *
+ * An SHM buffer is a memory-mapped file given by the client.
+ * According to POSIX, reading from a memory-mapped region that
+ * extends off the end of the file will cause a SIGBUS signal to be
+ * generated. Normally this would cause the compositor to terminate.
+ * In order to make the compositor robust against clients that change
+ * the size of the underlying file or lie about its size, you should
+ * protect access to the buffer by calling this function before
+ * reading from the memory and call wl_shm_buffer_end_access
+ * afterwards. This will install a signal handler for SIGBUS which
+ * will prevent the compositor from crashing.
+ *
+ * After calling this function the signal handler will remain
+ * installed for the lifetime of the compositor process. Note that
+ * this function will not work properly if the compositor is also
+ * installing its own handler for SIGBUS.
+ *
+ * If a SIGBUS signal is received for an address within the range of
+ * the SHM pool of the given buffer then the client will be sent an
+ * error event when wl_shm_buffer_end_access is called. If the signal
+ * is for an address outside that range then the signal handler will
+ * reraise the signal which would will likely cause the compositor to
+ * terminate.
+ *
+ * It is safe to nest calls to these functions as long as the nested
+ * calls are all accessing the same buffer. The number of calls to
+ * wl_shm_buffer_end_access must match the number of calls to
+ * wl_shm_buffer_begin_access. These functions are thread-safe and it
+ * is allowed to simultaneously access different buffers or the same
+ * buffer from multiple threads.
+ *
+ * \memberof wl_shm_buffer
+ */
WL_EXPORT void
wl_shm_buffer_begin_access(struct wl_shm_buffer *buffer)
{
@@ -480,6 +533,17 @@ wl_shm_buffer_begin_access(struct wl_shm_buffer *buffer)
sigbus_data-access_count++;
}
+/** Ends the access to a buffer started by wl_shm_buffer_begin_access
+ *
+ * \param buffer The SHM buffer
+ *
+ * This should be called after wl_shm_buffer_begin_access once the
+ * buffer is no longer being accessed. If a SIGBUS signal was
+ * generated in-between these two calls then the resource for the
+ * given buffer will be sent an error.
+ *
+ * \memberof wl_shm_buffer
+ */
WL_EXPORT void
wl_shm_buffer_end_access(struct wl_shm_buffer *buffer)
{
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v2] server: Add API to protect access to an SHM buffer
Ok, here is a second version of the patch which leaves the signal
handler permanently installed and uses thread-local storage to keep
track of the current pool.
Regards,
- Neil
--- 8 --- (use git am --scissors to automatically chop here)
Linux will let you mmap a region of a file that is larger than the
size of the file. If you then try to read from that region the process
will get a SIGBUS signal. Currently the clients can use this to crash
a compositor because it can create a pool and lie about the size of
the file which will cause the compositor to try and read past the end
of it. The compositor can't simply check the size of the file to
verify that it is big enough because then there is a race condition
where the client may truncate the file after the check is performed.
This patch adds the following two public functions in the server API
which can be used wrap access to an SHM buffer:
void wl_shm_buffer_begin_access(struct wl_shm_buffer *buffer);
void wl_shm_buffer_end_access(struct wl_shm_buffer *buffer);
The first time wl_shm_buffer_begin_access is called a signal handler
for SIGBUS will be installed. If the signal is caught then the buffer
for the current pool is remapped to an anonymous private buffer at the
same address which allows the compositor to continue without crashing.
The end_access function will then post an error to the buffer
resource.
The current pool is stored as part of some thread-local storage so
that multiple threads can safely independently access separate
buffers.
Eventually we may want to add some more API so that compositors can
hook into the signal handler or replace it entirely if they also want
to do some SIGBUS handling.
---
src/wayland-server.h | 6 +++
src/wayland-shm.c| 131 +++
2 files changed, 137 insertions(+)
diff --git a/src/wayland-server.h b/src/wayland-server.h
index 36c9a15..f5427fd 100644
--- a/src/wayland-server.h
+++ b/src/wayland-server.h
@@ -412,6 +412,12 @@ wl_resource_get_destroy_listener(struct wl_resource
*resource,
struct wl_shm_buffer;
+void
+wl_shm_buffer_begin_access(struct wl_shm_buffer *buffer);
+
+void
+wl_shm_buffer_end_access(struct wl_shm_buffer *buffer);
+
struct wl_shm_buffer *
wl_shm_buffer_get(struct wl_resource *resource);
diff --git a/src/wayland-shm.c b/src/wayland-shm.c
index eff29c3..28f52f4 100644
--- a/src/wayland-shm.c
+++ b/src/wayland-shm.c
@@ -32,10 +32,20 @@
#include string.h
#include sys/mman.h
#include unistd.h
+#include assert.h
+#include signal.h
+#include pthread.h
#include wayland-private.h
#include wayland-server.h
+/* This once_t is used to synchronize installing the SIGBUS handler
+ * and creating the TLS key. This will be done in the first call
+ * wl_shm_buffer_begin_access which can happen from any thread */
+static pthread_once_t wl_shm_sigbus_once = PTHREAD_ONCE_INIT;
+static pthread_key_t wl_shm_sigbus_data_key;
+static struct sigaction wl_shm_old_sigbus_action;
+
struct wl_shm_pool {
struct wl_resource *resource;
int refcount;
@@ -52,6 +62,12 @@ struct wl_shm_buffer {
struct wl_shm_pool *pool;
};
+struct wl_shm_sigbus_data {
+ struct wl_shm_pool *current_pool;
+ int access_count;
+ int fallback_mapping_used;
+};
+
static void
shm_pool_unref(struct wl_shm_pool *pool)
{
@@ -368,3 +384,118 @@ wl_shm_buffer_get_height(struct wl_shm_buffer *buffer)
{
return buffer-height;
}
+
+static void
+reraise_sigbus(void)
+{
+ /* If SIGBUS is raised for some other reason than accessing
+* the pool then we'll uninstall the signal handler so we can
+* reraise it. This would presumably kill the process */
+ sigaction(SIGBUS, wl_shm_old_sigbus_action, NULL);
+ raise(SIGBUS);
+}
+
+static void
+sigbus_handler(int signum, siginfo_t *info, void *context)
+{
+ struct wl_shm_sigbus_data *sigbus_data =
+ pthread_getspecific(wl_shm_sigbus_data_key);
+ struct wl_shm_pool *pool;
+
+ if (sigbus_data == NULL) {
+ reraise_sigbus();
+ return;
+ }
+
+ pool = sigbus_data-current_pool;
+
+ /* If the offending address is outside the mapped space for
+* the pool then the error is a real problem so we'll reraise
+* the signal */
+ if (pool == NULL ||
+ (char *) info-si_addr pool-data ||
+ (char *) info-si_addr = pool-data + pool-size) {
+ reraise_sigbus();
+ return;
+ }
+
+ sigbus_data-fallback_mapping_used = 1;
+
+ /* This should replace the previous mapping */
+ if (mmap(pool-data, pool-size,
+PROT_READ | PROT_WRITE,
+MAP_PRIVATE | MAP_FIXED | MAP_ANONYMOUS,
+0, 0) == (void *) -1) {
+ reraise_sigbus();
+ return;
+ }
+}
+
+static void
+destroy_sigbus_data(void *data)
+{
+ struct wl_shm_sigbus_data
[PATCH weston v2] Add calls to wl_shm_buffer_begin/end_access
Thanks for the feedback. Here is a version two of the patch which
fixes some merge conflicts on master and adds support for the pixman
renderer.
Kristian wrote:
As for the pixman renderer it should be a matter of just wrapping
the calls to pixman_image_composite32() in
pixman_renderer_read_pixels() and around the last if/else in
draw_view() where we end up calling repaint_region() in either
branch.
I think the pixman_renderer_read_pixels case doesn't need the wrapper
functions because it is reading from the output's buffer into another
malloc'd buffer. Both of those are allocated by the compositor and
shouldn't cause any problems.
I put the wrapper calls inside repaint_region in order to minimise the
length of time that the wrapper is enabled.
Regards,
- Neil
--- 8 --- (use git am --scissors to automatically chop here)
This wraps all accesses to an SHM buffer between wl_shm_buffer_begin
and end so that wayland-shm can install a handler for SIGBUS and catch
attempts to pass the compositor a buffer that is too small.
---
src/compositor-drm.c | 11 ---
src/gl-renderer.c | 6 ++
src/pixman-renderer.c | 6 ++
src/rpi-renderer.c| 4
src/screenshooter.c | 4
5 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index b929728..d4fc871 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -974,6 +974,7 @@ static void
drm_output_set_cursor(struct drm_output *output)
{
struct weston_view *ev = output-cursor_view;
+ struct weston_buffer *buffer;
struct drm_compositor *c =
(struct drm_compositor *) output-base.compositor;
EGLint handle, stride;
@@ -988,18 +989,22 @@ drm_output_set_cursor(struct drm_output *output)
return;
}
- if (ev-surface-buffer_ref.buffer
+ buffer = ev-surface-buffer_ref.buffer;
+
+ if (buffer
pixman_region32_not_empty(output-cursor_plane.damage)) {
pixman_region32_fini(output-cursor_plane.damage);
pixman_region32_init(output-cursor_plane.damage);
output-current_cursor ^= 1;
bo = output-cursor_bo[output-current_cursor];
memset(buf, 0, sizeof buf);
- stride =
wl_shm_buffer_get_stride(ev-surface-buffer_ref.buffer-shm_buffer);
- s =
wl_shm_buffer_get_data(ev-surface-buffer_ref.buffer-shm_buffer);
+ stride = wl_shm_buffer_get_stride(buffer-shm_buffer);
+ s = wl_shm_buffer_get_data(buffer-shm_buffer);
+ wl_shm_buffer_begin_access(buffer-shm_buffer);
for (i = 0; i ev-geometry.height; i++)
memcpy(buf + i * 64, s + i * stride,
ev-geometry.width * 4);
+ wl_shm_buffer_end_access(buffer-shm_buffer);
if (gbm_bo_write(bo, buf, sizeof buf) 0)
weston_log(failed update cursor: %m\n);
diff --git a/src/gl-renderer.c b/src/gl-renderer.c
index 68a071f..7a535c7 100644
--- a/src/gl-renderer.c
+++ b/src/gl-renderer.c
@@ -899,10 +899,12 @@ gl_renderer_flush_damage(struct weston_surface *surface)
glBindTexture(GL_TEXTURE_2D, gs-textures[0]);
if (!gr-has_unpack_subimage) {
+ wl_shm_buffer_begin_access(buffer-shm_buffer);
glTexImage2D(GL_TEXTURE_2D, 0, format,
gs-pitch, buffer-height, 0,
format, pixel_type,
wl_shm_buffer_get_data(buffer-shm_buffer));
+ wl_shm_buffer_end_access(buffer-shm_buffer);
goto done;
}
@@ -914,13 +916,16 @@ gl_renderer_flush_damage(struct weston_surface *surface)
if (gs-needs_full_upload) {
glPixelStorei(GL_UNPACK_SKIP_PIXELS_EXT, 0);
glPixelStorei(GL_UNPACK_SKIP_ROWS_EXT, 0);
+ wl_shm_buffer_begin_access(buffer-shm_buffer);
glTexSubImage2D(GL_TEXTURE_2D, 0,
0, 0, gs-pitch, buffer-height,
format, pixel_type, data);
+ wl_shm_buffer_end_access(buffer-shm_buffer);
goto done;
}
rectangles = pixman_region32_rectangles(gs-texture_damage, n);
+ wl_shm_buffer_begin_access(buffer-shm_buffer);
for (i = 0; i n; i++) {
pixman_box32_t r;
@@ -932,6 +937,7 @@ gl_renderer_flush_damage(struct weston_surface *surface)
r.x2 - r.x1, r.y2 - r.y1,
format, pixel_type, data);
}
+ wl_shm_buffer_end_access(buffer-shm_buffer);
#endif
done:
diff --git a/src/pixman-renderer.c b/src/pixman-renderer.c
index a80be5f..b719829 100644
--- a/src/pixman-renderer.c
+++ b/src/pixman-renderer.c
@@ -305,6 +305,9 @@ repaint_region(struct weston_view *ev, struct
Re: [PATCH mesa v4] wayland: Add support for eglSwapInterval
Tomeu Vizoso to...@tomeuvizoso.net writes: What I fail to see is why a single sync should be enough, as we don't know when the GPU will signal that it's done with the buffer that we are waiting to be released. You are right that we don't know when the GPU will release the buffer. However we are not waiting for that. We are assuming that the GPU is only going to hold on to at most 2 buffers. In the DRM / Mesa case it needs to hold on to these because one will be used for scanout and one will be queued for the page flip. If we attach a third buffer then it will be held by the compositor as a queue for the next flip. We are assuming it won't have given this buffer to the GPU. Therefore if we attach a fourth buffer it is easy for the compositor to immediately release its lock on the third buffer and replace it with the fourth. So we can assume that the fourth attach will always generate an immediate release event. Sending a sync request will ensure that we always get this release event. So if we have four buffer slots we can assume that one of the attaches will always generate an immediate release event. In the non-fullscreen case where we don't really need to wait for the GPU, we only need 2 slots because the compositor will only lock one buffer. Regards, - Neil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH mesa v4] wayland: Add support for eglSwapInterval
Jason Ekstrand ja...@jlekstrand.net writes: You don't have to continuously sync, just sync after every attach/commit. While it may be somewhat non-obvious, I don't see how calling sync once per frame is any worse than setting some flag somewhere. Hrmm thinking about it, I suppose sending the sync request isn't totally free because it will end up waking up the client an extra time to get the sync complete event. In a normal client this information wouldn't be used until the next time get_back_bo is called and Mesa's event queue is flushed so the wake up is not necessary. Waking up an extra time effectively negates the whole point of having the queuing mechanism in the first place. In this version 4 of the patch I made it so that it always does a sync request even if the application is using eglSwapInterval(1). This is needed because otherwise if the application isn't installing its own frame callback then it won't try to block for Mesa's frame callback until the next eglSwapInterval which means it won't have had a chance to receive any release events when get_back_bo is called. That would cause it to use an extra unnecessary buffer. So the patch effectively just turns off the queuing mechanism for all EGL clients through the backdoor. You could argue that we shouldn't use the sync request for eglSwapInterval(0) but I don't think that would be a good idea. The case where this would cause an extra redundant buffer is not a corner case. I think most game-style applications (eg, something using SDL) would hit this case because they would be relying on eglSwapInterval to do the throttling. I think making those games use an extra buffer is a worse crime than waking up the clients an extra time. The problem is not specific to the my patch and is happening right now with Weston master. I'm starting to wonder if we should just give up on the event queuing altogether. Regards, - Neil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] tests: Test whether a simple EGL main loop uses too many buffers
Here is a test case which demonstrates the 3 buffers problem. It is
fixed by the eglSwapInterval patch because that installs a sync event,
but note the problem isn't really related to eglSwapInterval and is
something we should probably fix anyway.
Regards,
- Neil
--- 8 --- (use git am --scissors to automatically chop here)
This adds a test that tries to simulate a simple game loop that would
be like this:
while (1) {
draw_something();
eglSwapBuffers();
}
In this case the test is relying on eglSwapBuffers to throttle to a
sensible frame rate.
The test then verifies that only 2 EGL buffers are used. This is done
via a new request and event in the wayland-test protocol.
Currently this causes 3 buffers to be created because the release
event generated by the swap buffers is not processed by Mesa until it
blocks for the frame complete event in the next swap buffers call, but
that is too late.
This can be fixed in Mesa by issuing a sync request after the swap
buffers and blocking on it before deciding whether to allocate a new
buffer.
---
configure.ac | 7 +++
protocol/wayland-test.xml | 7 +++
tests/Makefile.am | 12
tests/buffer-count-test.c | 128 ++
tests/weston-test-client-helper.c | 22 ++-
tests/weston-test-client-helper.h | 4 ++
tests/weston-test.c | 51 ++-
7 files changed, 229 insertions(+), 2 deletions(-)
create mode 100644 tests/buffer-count-test.c
diff --git a/configure.ac b/configure.ac
index 5d0d060..ce0fdbe 100644
--- a/configure.ac
+++ b/configure.ac
@@ -59,10 +59,17 @@ COMPOSITOR_MODULES=wayland-server = 1.3.90 pixman-1
AC_ARG_ENABLE(egl, [ --disable-egl],,
enable_egl=yes)
+AC_ARG_ENABLE(egl-tests, [ --enable-egl-tests],,
+ enable_egl_tests=yes)
AM_CONDITIONAL(ENABLE_EGL, test x$enable_egl = xyes)
+AM_CONDITIONAL(ENABLE_EGL_TESTS, test x$enable_egl = xyes -a
x$enable_egl_tests = xyes)
if test x$enable_egl = xyes; then
AC_DEFINE([ENABLE_EGL], [1], [Build Weston with EGL support])
PKG_CHECK_MODULES(EGL, [egl = 7.10 glesv2])
+
+if test x$enable_egl_tests = xyes; then
+ PKG_CHECK_MODULES([EGL_TESTS], [egl = 7.10 glesv2 wayland-client
wayland-egl])
+fi
fi
AC_ARG_ENABLE(xkbcommon,
diff --git a/protocol/wayland-test.xml b/protocol/wayland-test.xml
index 2993f08..18b6625 100644
--- a/protocol/wayland-test.xml
+++ b/protocol/wayland-test.xml
@@ -51,5 +51,12 @@
arg name=x type=fixed/
arg name=y type=fixed/
/event
+request name=get_n_egl_buffers
+ !-- causes a n_egl_buffers event to be sent which reports how many
+ buffer objects are live for the client --
+/request
+event name=n_egl_buffers
+ arg name=n type=uint/
+/event
/interface
/protocol
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 5be52c6..e458eff 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -18,6 +18,11 @@ weston_tests = \
subsurface.weston \
$(xwayland_test)
+if ENABLE_EGL_TESTS
+weston_tests +=\
+ buffer-count.weston
+endif
+
AM_TESTS_ENVIRONMENT = \
abs_builddir='$(abs_builddir)'; export abs_builddir;
@@ -63,6 +68,10 @@ weston_test_la_SOURCES = \
wayland-test-protocol.c \
wayland-test-server-protocol.h
+if ENABLE_EGL_TESTS
+weston_test_la_LDFLAGS += $(EGL_TESTS_LIBS)
+endif
+
weston_test_runner_src = \
weston-test-runner.c\
weston-test-runner.h
@@ -117,6 +126,9 @@ text_weston_LDADD = $(weston_test_client_libs)
subsurface_weston_SOURCES = subsurface-test.c $(weston_test_client_src)
subsurface_weston_LDADD = $(weston_test_client_libs)
+buffer_count_weston_SOURCES = buffer-count-test.c $(weston_test_client_src)
+buffer_count_weston_LDADD = $(weston_test_client_libs) $(EGL_TESTS_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/buffer-count-test.c b/tests/buffer-count-test.c
new file mode 100644
index 000..ad2bcee
--- /dev/null
+++ b/tests/buffer-count-test.c
@@ -0,0 +1,128 @@
+/*
+ * 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
Re: [PATCH mesa v4] wayland: Add support for eglSwapInterval
Just to be clear, I think the discussion about whether to queue release events is no longer related to implementing eglSwapInterval(0) so it shouldn't hold up the patch. As far as I understand if you want to render at the maximum rate you need four buffer slots and being able to disable the queuing mechanism isn't going to affect that. If the device can't handle four buffers then applications simply can't use eglSwapInterval(0) when rendering fullscreen. Increasing the number of buffer slots doesn't affect the number of buffers that will actually be used in the normal case of non-fullscreen applications which should continue to just use two buffers. I agree that we should probably do something about the event queuing anyway. Currently if a fullscreen application goes idle after drawing its last frame it will never get the release event for the buffer because nothing will flush the queue. This would deny the application a chance to free the buffer. However I don't know if having a mechanism to explicitly disable the queuing is the right answer in this case and it might make more sense for the compositor to ensure that it always eventually flushes the queue instead of keeping it indefinitely. My previous patch to disable the queuing when there are no frame callbacks could still be a good way to achieve that. Regards, - Neil Daniel Stone dan...@fooishbar.org writes: Hi, On 28 October 2013 11:19, Tomeu Vizoso to...@tomeuvizoso.net wrote: I'm still concerned about platforms with high resolution displays but relatively little memory. I'm thinking of the RPi, but my understanding is that Android goes to great lengths to reduce the number of buffers that clients have to keep, because of general memory consumption, but also because scanout buffers are precious when you try to get the smoothest of the experiences that is possible on these phones. I think we should still consider adding a flag through which the client can tell the compositor to send the release events immediately instead of queuing them. Otherwise, the compositor is making a very broad assumption on the client's inner workings if it assumes that release events can be queued without a negative impact on performance. Yeah, I agree. Maybe it could be an eglQueryWaylandBufferWL parameter for EGL buffers? Ramping up the number of buffers used just isn't an option on platforms with not massive amounts of memory, but enormous displays. It also kinda cancels out some of the buffer_age benefits too. I take the point that this is solving a different symptom of the same problem, but I'm worried that it'll paper over the problem and we'll end up just shipping patched versions (or fielding bug reports) on ARM indefinitely. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH mesa v3] wayland: Add an extension to create wl_buffers from EGLImages
Here is an updated version of the patch to address Axel Davy's concerns and change the wording of the extension to make it clear that the function can return EGL_BAD_MATCH for any reason which prevents the buffer from being represented as a wl_buffer and not just due to differences in the format. This should make it clear that it is acceptable for the function to fail if the compositor is running on a different graphics card from the compositor and the buffer is using an unsupported tiling format. I haven't made any changes to the implementation because as far as I understand the Wayland backend currently always uses the same device as the compositor which is the one that is passed down in the device event in the wl_drm interface so it can't have problems with tiling layouts yets. There is still some ongoing discussion about whether to add wl_display_get_main_queue(). Kristian had two alternative proposals. The first was to make Mesa internally bind a second wl_drm object which wouldn't have a queue set on it and use that to create the wl_buffers. I couldn't get that to work because that would also need a second wl_registry with the default queue but we wouldn't be able to read the events from that without messing up the application. The second proposal was to make wl_proxy_set_queue take a NULL argument which would reset the queue back to the default. I'm not really a big fan of that because it's not very obvious and I don't see a good reason to hide the main queue. Regards, - Neil --- 8 --- (use git am --scissors to automatically chop here) This adds an extension called EGL_WL_create_wayland_buffer_from_image which adds the following single function: struct wl_buffer * eglCreateWaylandBufferFromImageWL(EGLDisplay dpy, EGLImageKHR image); The function creates a wl_buffer which shares its contents with the given EGLImage. The expected use case for this is in a nested Wayland compositor which is using subsurfaces to present buffers from its clients. Using this extension it can attach the client buffers directly to the subsurface without having to blit the contents into an intermediate buffer. The compositing can then be done in the parent compositor. The extension is only implemented in the Wayland EGL platform because of course it wouldn't make sense anywhere else. --- .../specs/WL_create_wayland_buffer_from_image.spec | 101 + include/EGL/eglmesaext.h | 10 ++ src/egl/drivers/dri2/platform_wayland.c| 79 src/egl/main/eglapi.c | 25 + src/egl/main/eglapi.h | 8 ++ src/egl/main/egldisplay.h | 1 + src/egl/main/eglmisc.c | 1 + 7 files changed, 225 insertions(+) create mode 100644 docs/specs/WL_create_wayland_buffer_from_image.spec diff --git a/docs/specs/WL_create_wayland_buffer_from_image.spec b/docs/specs/WL_create_wayland_buffer_from_image.spec new file mode 100644 index 000..aa5eb4d --- /dev/null +++ b/docs/specs/WL_create_wayland_buffer_from_image.spec @@ -0,0 +1,101 @@ +Name + +WL_create_wayland_buffer_from_image + +Name Strings + +EGL_WL_create_wayland_buffer_from_image + +Contributors + +Neil Roberts +Axel Davy +Daniel Stone + +Contact + +Neil Roberts neil.s.robe...@intel.com + +Status + +Proposal + +Version + +Version 2, October 25, 2013 + +Number + +EGL Extension #not assigned + +Dependencies + +Requires EGL 1.4 or later. This extension is written against the +wording of the EGL 1.4 specification. + +EGL_KHR_base_image is required. + +Overview + +This extension provides an entry point to create a wl_buffer which shares +its contents with a given EGLImage. The expected use case for this is in a +nested Wayland compositor which is using subsurfaces to present buffers +from its clients. Using this extension it can attach the client buffers +directly to the subsurface without having to blit the contents into an +intermediate buffer. The compositing can then be done in the parent +compositor. + +The nested compositor can create an EGLImage from a client buffer resource +using the existing WL_bind_wayland_display extension. It should also be +possible to create buffers using other types of images although there is +no expected use case for that. + +IP Status + +Open-source; freely implementable. + +New Procedures and Functions + +struct wl_buffer *eglCreateWaylandBufferFromImageWL(EGLDisplay dpy, +EGLImageKHR image); + +New Tokens + +None. + +Additions to the EGL 1.4 Specification: + +To create a client-side wl_buffer from an EGLImage call + + struct wl_buffer *eglCreateWaylandBufferFromImageWL(EGLDisplay dpy, + EGLImageKHR image); + +The returned
[PATCH mesa v4] wayland: Add support for eglSwapInterval
Ok, here is version 4 of the patch taking into account the discussion with Jason Ekstrand. The assumption is that if we have enough buffer slots then we should always get a release event immediately after one of the attaches. That means we can just rely on sending a sync request after the commit in order to get a buffer release and we don't need to bother with the request to disable the queuing mechanism. The previous version of the patch would block in a loop calling wl_dispatch_queue if it couldn't find a buffer. This is only a sensible option if we know that the compositor isn't queueing the release events. If not this loop would just block indefinitely. If the theory about getting release events is correct then we should never actually hit this loop so it probably doesn't really matter what it does. However, I didn't like the idea of having a loop there that would just block forever so I changed it to poll the compositor with a sync request every 10ms in order to force it to flush the queue. It prints a warning if this case is hit so that we will know there is a problem. I made the change to make it use 4 buffer slots in this patch and tested that it does use exactly all 4 of them when the application is fullscreen. This does work and it doesn't hit the polling path. I guess we could change to be five in order to cope with the subsurface case but I'm a bit reluctant to do that because it seems like quite a corner case and maybe it's better to just let it hit the warning path in that case. In the previous versions of the patch it would only do a sync request if the swap interval is zero. In this version I've changed it so that it always installs it. This is necessary because if an application is doing swap interval 1 but isn't installing a frame callback it would end up rendering and calling get_back_bo before we've handled any data from the compositor and it would use a redundant third buffer. Regards, - Neil --- 8 --- (use git am --scissors to automatically chop here) The Wayland EGL platform now respects the eglSwapInterval value. The value is clamped to either 0 or 1 because it is difficult (and probably not useful) to sync to more than 1 redraw. The main change is that if the swap interval is 0 then Mesa won't install a frame callback so that eglSwapBuffers can be executed as often as necessary. However it now always does a sync request after the swap buffers and blocks until this is complete in get_back_bo. The compositor is likely to send a release event while processing the new buffer attach and this makes sure we will receive that before deciding whether to allocate a new buffer. This is done even if the application is using swap interval 1 because otherwise if the application is not installing its own frame callback it may end up calling get_back_bo before we've handled any data from the compositor and it would end up using a redundant extra buffer. If there are no buffers available then instead of returning with an error, get_back_bo will now poll the compositor by repeatedly sending sync requests every 10ms. This is a last resort and in theory this shouldn't happen because there should be no reason for the compositor to hold on to more than three buffers. That means whenever we attach the fourth buffer we should always get an immediate release event which should come in with the notification for the first sync request that we are throttled to. When the compositor is directly scanning out from the application's buffer it may end up holding on to three buffers. These are the one that is is currently scanning out from, one that has been given to DRM as the next buffer to flip to, and one that has been attached and will be given to DRM as soon as the previous flip completes. When we attach a fourth buffer to the compositor it should replace that third buffer so we should get a release event immediately after that. This patch therefore also changes the number of buffer slots to 4 so that we can accomodate that situation. If DRM eventually gets a way to cancel a pending page flip then the compositors can be changed to only need to hold on to two buffers and this value can be put back to 3. This also moves the vblank configuration defines from platform_x11.c to the common egl_dri2.h header so they can be shared by both platforms. --- src/egl/drivers/dri2/egl_dri2.h | 9 +- src/egl/drivers/dri2/platform_wayland.c | 204 +--- src/egl/drivers/dri2/platform_x11.c | 6 - 3 files changed, 193 insertions(+), 26 deletions(-) diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h index 7a2e098..7de5916 100644 --- a/src/egl/drivers/dri2/egl_dri2.h +++ b/src/egl/drivers/dri2/egl_dri2.h @@ -174,6 +174,7 @@ struct dri2_egl_surface intdx; intdy; struct wl_callback*frame_callback; + struct wl_callback*sync_callback; int format;
Re: Buffer release events (was: Add support for eglSwapInterval)
Hi Thanks for the interesting insights. It seems to me as if the default should always be to just send the event. I think I would vote for leaving the default as it is, ie, queuing the release events. It's really quite a corner case that delaying events has any effect on an application because most applications don't need to know about the release events until they are about to draw something. Usually they would only draw something in response to some event such as a frame callback or an input event. In that case the event will have caused the queue to flush so they will certainly be up-to-date about what buffers are available at the point when they start drawing. If we default to not queuing the event then I'd imagine most applications wouldn't realise they should enable it and would miss out on the optimisation. 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. Yes, I guess even if the compositor posts the event it's not going to actually send it to the client until the compositor goes idle again anyway and at that point it will probably have posted a frame complete callback too so the client would wake up anyway. 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. I think case number 3 is the main problem. It's useful for most fullscreen apps to have the event queued because most of them will be throttled to the frame callback and don't need the release events immediately. However this is also the use case most likely to want eglSwapInterval(0) which would want them immediately so really for this situation it is an application choice whether they should be queued or not. Number 1 above is the source of the vast majority of out release events. [...] 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. Yes, I think it makes sense to always sync the rendering to at least a wl_display.sync call and the Mesa patch I sent does already do this. You are right that in practice this effectively solves the problem for most use cases. So really the only case where this matters is when the compositor is directly scanning out from the client's buffer. But on the other hand, that is exactly what a fullscreen game is likely to be doing and that is the most likely candidate for doing eglSwapInterval(0). In any case, dummy sync and frame requests (you may need both) will allow you to achieve glSwapInterval(0) without server-side support. I'm not sure I follow you here. The release event may be queued at any point after the frame complete is sent. In that case sending a sync event to flush the queue is only going to help if Mesa sends it repeatedly, but that amounts to busy-waiting which would be terrible. I still feel like the new request is the right way to go. The difficulty with interface versioning feels like a separate wider problem that we should think about. The crux of the problem is that Mesa probably shouldn't be using proxy objects that are created outside of Mesa because in that case it doesn't have control over what interface version or event queue it is using. Working around the need for the new request would just side-step the issue but it doesn't seem unlikely that Mesa would want to use further new surface interfaces requests later too and the same problem would come back. Maybe we should have a separate object that you can create per surface in order to do new requests. This could be created by a new global object much like the wl_shell interface. In order to make it usable to both Mesa and the application, we would have to allow creating multiple resources of the new interface for a single surface. I'm not sure what to call it though because it would just end up being something like ‘wl_surface_extra_stuff’. Considering that other objects may end up also needing a similar kind of interface, maybe it would make more sense to rethink it a bit and make the compositor allow multiple resources for an object in general. Then you could have something like wl_compositor.rebind_resource(object, version) which would make a new resource for any object and it could have its own interface version. I am just thinking aloud here though, I haven't really thought that through much. I will take a look at how much hassle
[PATCH] protocol: Add a request to disable queuing of buffer release events
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 @@ /event /interface - interface name=wl_compositor version=3 + interface name=wl_compositor version=4 description summary=the compositor singleton A compositor. This object is a singleton global. The compositor is in charge of combining the contents of multiple @@ -959,7 +959,7 @@ /event /interface - interface name=wl_surface version=3 + interface name=wl_surface version=4 description summary=an onscreen surface A surface is a rectangular area that is displayed on the screen. It has a location, size and pixel contents. @@ -1233,7 +1233,52 @@ /description arg name=scale type=int/ /request - /interface + +!-- Version 4 additions -- + +enum name=release since=4 + description summary=options for when to send a buffer release event +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. + /description + entry name=delayed value=0/ + entry name=immediate value=1/ +/enum + +request name=set_release since=4 + description summary=sets whether the release event for the buffer being attached will be 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. + /description + arg name=value type=uint summary=The new value/ +/request + /interface interface name=wl_seat version=3 description summary=group of input devices -- 1.8.3.1 ___ 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
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_client_post_no_memory(client);
return;
@@
Re: Buffer release events (was: Add support for eglSwapInterval)
Neil Roberts n...@linux.intel.com 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 2/2] Add a test for wl_surface.set_release
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 string.h
+
+#include weston-test-client-helper.h
+#include stdio.h
+#include poll.h
+#include time.h
+
+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
Re: [PATCH v2] Post buffer release events instead of queue when no frame callback
Neil Roberts n...@linux.intel.com writes: I think that would mean you could cause tearing if you are using eglSwapInterval(0) because you could write into the released buffer while the GPU is actually still rendering the previous frame using the buffer in a texture. I think this doesn't actually happen at the moment because as far as I can tell the gl-renderer immediately queues the buffer release when you attach a new one rather than waiting for the swap to actually complete. Thanks to some explanation from Kristian and Pekka I now understand that this is the expected behaviour. The assumption is that it will be up to the GL driver or hardware to ensure that GL commands submitted by the client will be executed after those submitted by the compositor so the compositor is free to release straight after the eglSwapBuffer call. If the hardware doesn't just do this naturally then it would still be possible for a driver's buffer sharing implementation to also transfer fence objects so that it can ensure these semantics in the driver. Regards, - Neil - Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Buffer release events (was: Add support for eglSwapInterval)
Here is a quick summary of the discussion about the release event handling that has partially happened on this list and partially on IRC. Currently the release events are put into a queue so that they are only sent alongside another event sent by the compositor in order to avoid waking up the client more times than necessary. Usually this will end up being sent alongside the frame complete callback. The assumption is that most clients don't care about the buffer release events until they are about to draw something, which usually only happens when they get the frame callback so they don't need to wake up early. This assumption doesn't work if the client isn't installing a frame callback for example when doing eglSwapInterval(0). In that case the client may be blocked waiting for a buffer release event but the queue will never get flushed because no other events are being sent from the compositor. It could also happen if the compositor is not sending the release events until some time after the frame callback completes, for example if it waits until a pending swap buffer completes before releasing the buffer. Currently this doesn't happen for normal clients because the GL renderer immediately releases the buffer when a new one is attached but it may happen for fullscreen clients. One solution is to always post the release event instead of queuing whenever the client doesn't have a frame callback currently installed. This fixes the clients doing eglSwapInterval(0) without breaking all the normal clients. However this still isn't a perfect solution because it's still possible that a client would want to do eglSwapInterval(0) and render as fast as possible but still install its own frame callback, perhaps just to get information about how fast the frames are being displayed on the screen. Currently the only proposed complete solution is just to add a request to explicitly disable the queuing mechanism. This would have to be a request on the surface rather than being hidden away as part of wl_drm because the compositor needs to know about it. Mesa could implicitly send this request whenever eglSwapInterval is called. As far as I understand the explicit toggle is the current expected plan although it does feel a bit like a kludge so we may want to sit on it a bit in case we can think of something better. Regards, - Neil - Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 1/4] Add a touch binding to activate a surface
Adds a new binding type for touch events via the new function
weston_compositor_add_touch_binding. The binding can only be added for
a touch down with the first finger. The shell now uses this to install
a binding to activate the current surface.
---
src/bindings.c | 36
src/compositor.c | 2 ++
src/compositor.h | 14 ++
src/input.c | 2 ++
src/shell.c | 36
5 files changed, 82 insertions(+), 8 deletions(-)
diff --git a/src/bindings.c b/src/bindings.c
index f6ec9ea..03c9238 100644
--- a/src/bindings.c
+++ b/src/bindings.c
@@ -94,6 +94,24 @@ weston_compositor_add_button_binding(struct
weston_compositor *compositor,
}
WL_EXPORT struct weston_binding *
+weston_compositor_add_touch_binding(struct weston_compositor *compositor,
+ uint32_t modifier,
+ weston_touch_binding_handler_t handler,
+ void *data)
+{
+ struct weston_binding *binding;
+
+ binding = weston_compositor_add_binding(compositor, 0, 0, 0,
+ modifier, handler, data);
+ if (binding == NULL)
+ return NULL;
+
+ wl_list_insert(compositor-touch_binding_list.prev, binding-link);
+
+ return binding;
+}
+
+WL_EXPORT struct weston_binding *
weston_compositor_add_axis_binding(struct weston_compositor *compositor,
uint32_t axis, uint32_t modifier,
weston_axis_binding_handler_t handler,
@@ -253,6 +271,24 @@ weston_compositor_run_button_binding(struct
weston_compositor *compositor,
}
}
+WL_EXPORT void
+weston_compositor_run_touch_binding(struct weston_compositor *compositor,
+ struct weston_seat *seat, uint32_t time,
+ int touch_type)
+{
+ struct weston_binding *b;
+
+ if (seat-num_tp != 1 || touch_type != WL_TOUCH_DOWN)
+ return;
+
+ wl_list_for_each(b, compositor-touch_binding_list, link) {
+ if (b-modifier == seat-modifier_state) {
+ weston_touch_binding_handler_t handler = b-handler;
+ handler(seat, time, b-data);
+ }
+ }
+}
+
WL_EXPORT int
weston_compositor_run_axis_binding(struct weston_compositor *compositor,
struct weston_seat *seat,
diff --git a/src/compositor.c b/src/compositor.c
index b8e442a..935015a 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -3112,6 +3112,7 @@ weston_compositor_init(struct weston_compositor *ec,
wl_list_init(ec-output_list);
wl_list_init(ec-key_binding_list);
wl_list_init(ec-button_binding_list);
+ wl_list_init(ec-touch_binding_list);
wl_list_init(ec-axis_binding_list);
wl_list_init(ec-debug_binding_list);
@@ -3172,6 +3173,7 @@ weston_compositor_shutdown(struct weston_compositor *ec)
weston_binding_list_destroy_all(ec-key_binding_list);
weston_binding_list_destroy_all(ec-button_binding_list);
+ weston_binding_list_destroy_all(ec-touch_binding_list);
weston_binding_list_destroy_all(ec-axis_binding_list);
weston_binding_list_destroy_all(ec-debug_binding_list);
diff --git a/src/compositor.h b/src/compositor.h
index 9b48287..ad5a786 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -575,6 +575,7 @@ struct weston_compositor {
struct wl_list plane_list;
struct wl_list key_binding_list;
struct wl_list button_binding_list;
+ struct wl_list touch_binding_list;
struct wl_list axis_binding_list;
struct wl_list debug_binding_list;
@@ -984,6 +985,15 @@ weston_compositor_add_button_binding(struct
weston_compositor *compositor,
weston_button_binding_handler_t binding,
void *data);
+typedef void (*weston_touch_binding_handler_t)(struct weston_seat *seat,
+ uint32_t time,
+ void *data);
+struct weston_binding *
+weston_compositor_add_touch_binding(struct weston_compositor *compositor,
+ enum weston_keyboard_modifier modifier,
+ weston_touch_binding_handler_t binding,
+ void *data);
+
typedef void (*weston_axis_binding_handler_t)(struct weston_seat *seat,
uint32_t time, uint32_t axis,
wl_fixed_t value, void *data);
@@ -1014,6 +1024,10 @@ weston_compositor_run_button_binding(struct
weston_compositor *compositor,
struct weston_seat *seat, uint32_t time,
uint32_t button,
[PATCH weston 2/4] Add a touch move binding
When holding the compositor super key the touch events can now be used
to move a window.
---
src/shell.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/src/shell.c b/src/shell.c
index 4a0c122..2822a2b 100644
--- a/src/shell.c
+++ b/src/shell.c
@@ -2774,6 +2774,26 @@ move_binding(struct weston_seat *seat, uint32_t time,
uint32_t button, void *dat
}
static void
+touch_move_binding(struct weston_seat *seat, uint32_t time, void *data)
+{
+ struct weston_surface *focus =
+ (struct weston_surface *) seat-touch-focus;
+ struct weston_surface *surface;
+ struct shell_surface *shsurf;
+
+ surface = weston_surface_get_main_surface(focus);
+ if (surface == NULL)
+ return;
+
+ shsurf = get_shell_surface(surface);
+ if (shsurf == NULL || shsurf-type == SHELL_SURFACE_FULLSCREEN ||
+ shsurf-type == SHELL_SURFACE_MAXIMIZED)
+ return;
+
+ surface_touch_move(shsurf, (struct weston_seat *) seat);
+}
+
+static void
resize_binding(struct weston_seat *seat, uint32_t time, uint32_t button, void
*data)
{
struct weston_surface *focus =
@@ -4531,6 +4551,7 @@ shell_add_bindings(struct weston_compositor *ec, struct
desktop_shell *shell)
zoom_key_binding, NULL);
weston_compositor_add_button_binding(ec, BTN_LEFT, mod, move_binding,
shell);
+ weston_compositor_add_touch_binding(ec, mod, touch_move_binding, shell);
weston_compositor_add_button_binding(ec, BTN_MIDDLE, mod,
resize_binding, shell);
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 4/4] Don't remove the touch grab until the last touch point is removed
Previously if you move a window around and temporarily add a second finger then it will cancel the grab even though the original finger is still held on the screen. It seems more robust to avoid cancelling the grab until all fingers have been removed. --- src/shell.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/shell.c b/src/shell.c index 2822a2b..45bbed3 100644 --- a/src/shell.c +++ b/src/shell.c @@ -1085,7 +1085,9 @@ touch_move_grab_up(struct weston_touch_grab *grab, uint32_t time, int touch_id) struct shell_touch_grab *shell_grab = container_of(grab, struct shell_touch_grab, grab); - shell_touch_grab_end(shell_grab); + + if (grab-touch-seat-num_tp == 0) + shell_touch_grab_end(shell_grab); } static void -- 1.8.3.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 3/4] Only update the touch grab position for the first finger
Previously if you add a second finger while moving a window with a
touch grab then the position will keep jumping between the position of
each finger as you move them around. This patch changes it so that it
keeps track of the first touch id that starts the grab and only
updates the grab position when that finger moves.
---
src/compositor.h | 1 +
src/input.c | 7 +--
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/compositor.h b/src/compositor.h
index ad5a786..ffa5124 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -343,6 +343,7 @@ struct weston_touch {
struct weston_touch_grab *grab;
struct weston_touch_grab default_grab;
+ int grab_touch_id;
wl_fixed_t grab_x, grab_y;
uint32_t grab_serial;
uint32_t grab_time;
diff --git a/src/input.c b/src/input.c
index 3e4f4b1..4e6818c 100644
--- a/src/input.c
+++ b/src/input.c
@@ -1093,8 +1093,10 @@ notify_touch(struct weston_seat *seat, uint32_t time,
int touch_id,
wl_fixed_t sx, sy;
/* Update grab's global coordinates. */
- touch-grab_x = x;
- touch-grab_y = y;
+ if (touch_id == touch-grab_touch_id touch_type != WL_TOUCH_UP) {
+ touch-grab_x = x;
+ touch-grab_y = y;
+ }
switch (touch_type) {
case WL_TOUCH_DOWN:
@@ -1124,6 +1126,7 @@ notify_touch(struct weston_seat *seat, uint32_t time, int
touch_id,
if (seat-num_tp == 1) {
touch-grab_serial =
wl_display_get_serial(ec-wl_display);
+ touch-grab_touch_id = touch_id;
touch-grab_time = time;
touch-grab_x = x;
touch-grab_y = y;
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v2] Post buffer release events instead of queue when no frame callback
I don't think this function would help in cases where the buffer is released after the frame is drawn but the compositor is not drawing anything else. Ie, if the events were like this: 1 - client attaches a buffer and waits for the next release 2 - compositor redraws a frame 3 - the redraw completes so the compositor flushes all event queues 4 - the swap completes and the compositor queues a release event If nothing else causes the compositor to queue a redraw the release event will just sit in the event queue forever and the client will still be blocked waiting for it. I think this doesn't actually happen at the moment because as far as I can tell the gl-renderer immediately queues the buffer release when you attach a new one rather than waiting for the swap to actually complete. I think that would mean you could cause tearing if you are using eglSwapInterval(0) because you could write into the released buffer while the GPU is actually still rendering the previous frame using the buffer in a texture. I will try an experiment to check this. If that's right then we should probably add an extra buffer reference in the gl-renderer whenever it uses a texture to draw and then only release it when the frame is complete. Regards, - Neil Jason Ekstrand ja...@jlekstrand.net writes: All, Perhaps a simpler approach would be better. We could add a function to wayland-server called wl_client_post_event_queue which would simply set the wants_flush flag on the client connection. This function would be fast ,require no I/O, and safe to call multiple times so Weston could simply call it on all clients with surfaces visible on the output in question at the time when it would call frame events. Then, when the event loop flushes the clients, the release events will get sent out. --Jason Ekstrand - Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] server: Add API to protect access to an SHM buffer
Hi José Bollo jo...@nonadev.net writes: That is a really interesting point. I have two questions about it: - Is it normal that the client trucates the buffer? Is your patch designed to allow normal operations? or to allow forbiden uses? - If it is not normal, is there good reasons to continue to serve a nasty client? No, it's not normal that the client would truncate the buffer. The patch is effectively designed to disallow this and recover gracefully instead of making the compositor crash. It won't continue to serve the client but instead it will send it an error. The problem with truncating is probably only an issue if there are malicious clients. However the case where the client sends the wrong size to wl_shm.create_pool would be worth guarding against in any case because it would be quite easy for a buggy client to get that wrong and the compositor should really be robust against that. Regards, - Neil - Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH] server: Add API to protect access to an SHM buffer
Linux will let you mmap a region of a file that is larger than the
size of the file. If you then try to read from that region the process
will get a SIGBUS signal. Currently the clients can use this to crash
a compositor because it can create a pool and lie about the size of
the file which will cause the compositor to try and read past the end
of it. The compositor can't simply check the size of the file to
verify that it is big enough because then there is a race condition
where the client may truncate the file after the check is performed.
This patch adds the following two public functions in the server API
which can be used wrap access to an SHM buffer:
void wl_shm_buffer_begin_access(struct wl_shm_buffer *buffer);
void wl_shm_buffer_end_access(struct wl_shm_buffer *buffer);
In-between calls to these functions there will be a signal handler for
SIGBUS. If the signal is caught then the buffer is remapped to an
anonymous private buffer at the same address which allows the
compositor to continue without crashing. The end_access function will
then post an error to the buffer resource.
---
This problem was pointed out earlier by wm4 on #wayland and it seems
like quite a thorny issue. Here is a first attempt at a patch which
does seem to work for Weston but I think there are plenty of issues to
think about. It might not be such a good idea to be messing with
signal handlers if the compositor itself also wants to handle SIGBUS.
There are probably some threading concerns here too.
I think you could do the patch without adding any extra API and just
make it automatically work out which region to remap if you had a list
of wl_shm_pools. The signal handler gets passed the address so it
could check which pool contains it and just remap that one. However
the end_access function seems like a nice place to post an event back
to the client when it fails so I'm not sure which way is better. It's
also probably a good idea to keep the signal handler as simple as
possible.
src/wayland-server.h | 6
src/wayland-shm.c| 87
2 files changed, 93 insertions(+)
diff --git a/src/wayland-server.h b/src/wayland-server.h
index c93987d..b371c9e 100644
--- a/src/wayland-server.h
+++ b/src/wayland-server.h
@@ -412,6 +412,12 @@ wl_resource_get_destroy_listener(struct wl_resource
*resource,
struct wl_shm_buffer;
+void
+wl_shm_buffer_begin_access(struct wl_shm_buffer *buffer);
+
+void
+wl_shm_buffer_end_access(struct wl_shm_buffer *buffer);
+
struct wl_shm_buffer *
wl_shm_buffer_get(struct wl_resource *resource);
diff --git a/src/wayland-shm.c b/src/wayland-shm.c
index eff29c3..95b1f33 100644
--- a/src/wayland-shm.c
+++ b/src/wayland-shm.c
@@ -32,15 +32,22 @@
#include string.h
#include sys/mman.h
#include unistd.h
+#include assert.h
+#include signal.h
#include wayland-private.h
#include wayland-server.h
+static struct wl_shm_pool *wl_shm_current_pool;
+static struct sigaction wl_shm_old_sigbus_action;
+
struct wl_shm_pool {
struct wl_resource *resource;
int refcount;
char *data;
int size;
+ int access_count;
+ int fallback_mapping_used;
};
struct wl_shm_buffer {
@@ -219,6 +226,7 @@ shm_create_pool(struct wl_client *client, struct
wl_resource *resource,
pool-refcount = 1;
pool-size = size;
+ pool-fallback_mapping_used = 0;
pool-data = mmap(NULL, size,
PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
if (pool-data == MAP_FAILED) {
@@ -368,3 +376,82 @@ wl_shm_buffer_get_height(struct wl_shm_buffer *buffer)
{
return buffer-height;
}
+
+static void
+unset_sigbus_handler(void)
+{
+ sigaction(SIGBUS, wl_shm_old_sigbus_action, NULL);
+}
+
+static void
+sigbus_handler(int signum, siginfo_t *info, void *context)
+{
+ struct wl_shm_pool *pool = wl_shm_current_pool;
+
+ unset_sigbus_handler();
+
+ /* If the offending address is outside the mapped space for
+* the pool then the error is a real problem so we'll reraise
+* the signal */
+ if ((char *) info-si_addr pool-data ||
+ (char *) info-si_addr = pool-data + pool-size)
+ raise(signum);
+
+ pool-fallback_mapping_used = 1;
+
+ /* This should replace the previous mapping */
+ if (mmap(pool-data, pool-size,
+PROT_READ | PROT_WRITE,
+MAP_PRIVATE | MAP_FIXED | MAP_ANONYMOUS,
+0, 0) == (void *) -1)
+ raise(signum);
+}
+
+static void
+set_sigbus_handler(void)
+{
+ struct sigaction new_action = {
+ .sa_sigaction = sigbus_handler,
+ .sa_flags = SA_SIGINFO | SA_NODEFER
+ };
+
+ sigemptyset(new_action.sa_mask);
+
+ sigaction(SIGBUS, new_action, wl_shm_old_sigbus_action);
+}
+
+WL_EXPORT void
+wl_shm_buffer_begin_access(struct wl_shm_buffer *buffer)
+{
+ struct wl_shm_pool *pool = buffer-pool;
+
+
[PATCH weston] Add calls to wl_shm_buffer_begin/end_access
This wraps all accesses to an SHM buffer between wl_shm_buffer_begin
and end so that wayland-shm can install a handler for SIGBUS and catch
attempts to pass the compositor a buffer that is too small.
Note, this patch doesn't do anything to fix the pixman renderer.
---
src/compositor-drm.c | 3 +++
src/gl-renderer.c | 6 ++
src/pixman-renderer.c | 2 ++
src/rpi-renderer.c| 4
src/screenshooter.c | 4
5 files changed, 19 insertions(+)
diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index 2770c85..180c3ee 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -964,9 +964,12 @@ drm_output_set_cursor(struct drm_output *output)
memset(buf, 0, sizeof buf);
stride =
wl_shm_buffer_get_stride(es-buffer_ref.buffer-shm_buffer);
s = wl_shm_buffer_get_data(es-buffer_ref.buffer-shm_buffer);
+
+ wl_shm_buffer_begin_access(es-buffer_ref.buffer-shm_buffer);
for (i = 0; i es-geometry.height; i++)
memcpy(buf + i * 64, s + i * stride,
es-geometry.width * 4);
+ wl_shm_buffer_end_access(es-buffer_ref.buffer-shm_buffer);
if (gbm_bo_write(bo, buf, sizeof buf) 0)
weston_log(failed update cursor: %m\n);
diff --git a/src/gl-renderer.c b/src/gl-renderer.c
index ae69f22..90e8148 100644
--- a/src/gl-renderer.c
+++ b/src/gl-renderer.c
@@ -875,10 +875,12 @@ gl_renderer_flush_damage(struct weston_surface *surface)
glBindTexture(GL_TEXTURE_2D, gs-textures[0]);
if (!gr-has_unpack_subimage) {
+ wl_shm_buffer_begin_access(buffer-shm_buffer);
glTexImage2D(GL_TEXTURE_2D, 0, format,
gs-pitch, buffer-height, 0,
format, pixel_type,
wl_shm_buffer_get_data(buffer-shm_buffer));
+ wl_shm_buffer_end_access(buffer-shm_buffer);
goto done;
}
@@ -890,13 +892,16 @@ gl_renderer_flush_damage(struct weston_surface *surface)
if (gs-needs_full_upload) {
glPixelStorei(GL_UNPACK_SKIP_PIXELS_EXT, 0);
glPixelStorei(GL_UNPACK_SKIP_ROWS_EXT, 0);
+ wl_shm_buffer_begin_access(buffer-shm_buffer);
glTexSubImage2D(GL_TEXTURE_2D, 0,
0, 0, gs-pitch, buffer-height,
format, pixel_type, data);
+ wl_shm_buffer_end_access(buffer-shm_buffer);
goto done;
}
rectangles = pixman_region32_rectangles(gs-texture_damage, n);
+ wl_shm_buffer_begin_access(buffer-shm_buffer);
for (i = 0; i n; i++) {
pixman_box32_t r;
@@ -908,6 +913,7 @@ gl_renderer_flush_damage(struct weston_surface *surface)
r.x2 - r.x1, r.y2 - r.y1,
format, pixel_type, data);
}
+ wl_shm_buffer_end_access(buffer-shm_buffer);
#endif
done:
diff --git a/src/pixman-renderer.c b/src/pixman-renderer.c
index 987c539..80d41a8 100644
--- a/src/pixman-renderer.c
+++ b/src/pixman-renderer.c
@@ -574,6 +574,8 @@ pixman_renderer_attach(struct weston_surface *es, struct
weston_buffer *buffer)
buffer-width = wl_shm_buffer_get_width(shm_buffer);
buffer-height = wl_shm_buffer_get_height(shm_buffer);
+ /* XXX what about wl_shm_buffer_begin_access? */
+
ps-image = pixman_image_create_bits(pixman_format,
buffer-width, buffer-height,
wl_shm_buffer_get_data(shm_buffer),
diff --git a/src/rpi-renderer.c b/src/rpi-renderer.c
index 3ba5fc4..a964899 100644
--- a/src/rpi-renderer.c
+++ b/src/rpi-renderer.c
@@ -277,6 +277,8 @@ rpi_resource_update(struct rpi_resource *resource, struct
weston_buffer *buffer,
pixman_region32_intersect(write_region,
write_region, region);
+ wl_shm_buffer_begin_access(buffer-shm_buffer);
+
#ifdef HAVE_RESOURCE_WRITE_DATA_RECT
/* XXX: Can this do a format conversion, so that scanout does not have
to? */
r = pixman_region32_rectangles(write_region, n);
@@ -311,6 +313,8 @@ rpi_resource_update(struct rpi_resource *resource, struct
weston_buffer *buffer,
width, r-y2 - r-y1, 0, r-y1, ret);
#endif
+ wl_shm_buffer_end_access(buffer-shm_buffer);
+
pixman_region32_fini(write_region);
return ret ? -1 : 0;
diff --git a/src/screenshooter.c b/src/screenshooter.c
index 645114d..0c657bc 100644
--- a/src/screenshooter.c
+++ b/src/screenshooter.c
@@ -144,6 +144,8 @@ screenshooter_frame_notify(struct wl_listener *listener,
void *data)
d = wl_shm_buffer_get_data(l-buffer-shm_buffer);
s = pixels + stride * (l-buffer-height - 1);
+ wl_shm_buffer_begin_access(l-buffer-shm_buffer);
+
switch (compositor-read_format) {
case
Re: idle tasks starving in toytoolkit
Pekka Paalanen ppaala...@gmail.com writes:
If not, is there not a possibility to break existing applications by
blocking too early?
Yes, you're right, the patch is nonsense because it won't work when the
application does wl_display_dispatch_pending because it might end up
with some events still in the queue but the poll won't wake up to
process them.
It would be nice if the recommended main loop was more like this:
while (TRUE) {
int need_read;
int timeout;
if (wl_display_prepare_read(display)) {
need_read = 1;
timeout = -1;
} else {
need_read = 0;
timeout = 0;
}
wl_display_flush(dpy);
poll(fds, nfds, timeout);
if (need_read)
wl_display_read_events(dpy);
wl_display_dispatch_pending(dpy);
}
That way it doesn't matter if wl_display_dispatch_pending doesn't clear
all of the events.
That is effectively what I think we need do in GTK anyway because in
that case you really ought to avoid dispatching events in the prepare
function before the poll and instead should only be dispatching after.
I guess it's too late to change the semantics of
wl_display_dispatch_pending though so this doesn't help much.
Regards,
- Neil
-
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
RE: idle tasks starving in toytoolkit
One idea to fix this might be to make dispatch_queue only ever
dispatch the events that were current when the loop is started. That
way if any further events are added while processing the current
events it will give control back to the main loop before processing
them.
Here's a not-heavily-tested patch that would do that.
Regards,
- Neil
--- 8 --- (use git am --scissors to automatically chop here)
Subject: [PATCH] client: Don't dispatch events that are added while dispatching
Previously wl_display_dispatch_queue would keep dispatching events
until the event queue becomes empty. If more events are queued while
dispatching those events the loop can run indefinitely. This patch
changes it so that instead of iterating the list of events directly in
the queue it steals the list of events before dispatching and iterates
the local list instead. That way it will only iterate the events that
were current before the loop started and if further events are added
the application will drop back to the main loop before processing
them.
---
src/wayland-client.c | 31 ++-
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/src/wayland-client.c b/src/wayland-client.c
index a20a71e..ae00ef0 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -918,16 +918,12 @@ decrease_closure_args_refcount(struct wl_closure *closure)
}
static void
-dispatch_event(struct wl_display *display, struct wl_event_queue *queue)
+dispatch_event(struct wl_display *display, struct wl_closure *closure)
{
- struct wl_closure *closure;
struct wl_proxy *proxy;
int opcode;
bool proxy_destroyed;
- closure = container_of(queue-event_list.next,
- struct wl_closure, link);
- wl_list_remove(closure-link);
opcode = closure-opcode;
/* Verify that the receiving object is still valid by checking if has
@@ -1049,19 +1045,36 @@ wl_display_read_events(struct wl_display *display)
static int
dispatch_queue(struct wl_display *display, struct wl_event_queue *queue)
{
- int count;
+ struct wl_closure *closure, *tmp;
+ struct wl_list event_list;
+ int count = 0;
if (display-last_error)
goto err;
- for (count = 0; !wl_list_empty(queue-event_list); count++) {
- dispatch_event(display, queue);
+ /* Steal the list of events from the queue so that if more
+* events are added while dispatching the current ones we
+* won't get stuck in this loop indefinitely */
+ wl_list_init(event_list);
+ wl_list_insert_list(event_list, queue-event_list);
+ wl_list_init(queue-event_list);
+
+ wl_list_for_each_safe(closure, tmp, event_list, link) {
+ wl_list_remove(closure-link);
+ count++;
+
+ dispatch_event(display, closure);
+
if (display-last_error)
- goto err;
+ goto err_queue;
}
return count;
+err_queue:
+ /* Put any remaining events back in the queue */
+ wl_list_insert_list(queue-event_list, event_list);
+
err:
errno = display-last_error;
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v2] client: Fix handling display-reader_count if poll fails
In wl_display_dispatch_queue, if poll fails then it would previously
return immediately and leak a reference in display-reader_count. Then
if the application ignores the error and tries to read again it will
block forever. This can happen for example if the poll fails with
EINTR which the application might consider to be a recoverable error.
This patch makes it cancel the read so the reader_count will be
decremented when poll fails.
---
Ok, here is a second version of the patch which calls
wl_display_cancel_read as you suggested. Thanks for the review.
src/wayland-client.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/wayland-client.c b/src/wayland-client.c
index cad587d..a20a71e 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -1224,8 +1224,10 @@ wl_display_dispatch_queue(struct wl_display *display,
pfd[0].fd = display-fd;
pfd[0].events = POLLIN;
- if (poll(pfd, 1, -1) == -1)
+ if (poll(pfd, 1, -1) == -1) {
+ wl_display_cancel_read(display);
return -1;
+ }
pthread_mutex_lock(display-mutex);
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH 1/3 wayland-fits] core/pointer: Keep track of the focus serial
The focus serial is needed to correctly send a cursor so it is useful
to be able to verify that this is working correctly.
---
src/test/core/pointer.cpp | 2 ++
src/test/core/pointer.h | 2 ++
2 files changed, 4 insertions(+)
diff --git a/src/test/core/pointer.cpp b/src/test/core/pointer.cpp
index be6f9a9..eb94fd3 100644
--- a/src/test/core/pointer.cpp
+++ b/src/test/core/pointer.cpp
@@ -29,6 +29,7 @@ namespace core {
Pointer::Pointer(const Seat seat)
: seat_(seat)
, focus_(NULL)
+ , focusSerial_(0)
, x_(-1)
, y_(-1)
, button_(0)
@@ -73,6 +74,7 @@ bool Pointer::hasFocus(wl_surface* surface)
// wl_fixed_to_int(y) std::endl;
pointer-focus_ = wl_surface;
+ pointer-focusSerial_ = serial;
pointer-x_ = wl_fixed_to_int(x);
pointer-y_ = wl_fixed_to_int(y);
}
diff --git a/src/test/core/pointer.h b/src/test/core/pointer.h
index e891142..3607861 100644
--- a/src/test/core/pointer.h
+++ b/src/test/core/pointer.h
@@ -57,6 +57,7 @@ public:
const uint32_t button() const { return button_; }
const uint32_t buttonState() const { return buttonState_; }
wl_surface* focus() const { return focus_; }
+ const uint32_t focusSerial() const { return focusSerial_; }
bool hasFocus(wl_surface*);
@@ -76,6 +77,7 @@ private:
const Seat seat_;
wl_surface* focus_;
+ uint32_tfocusSerial_;
int32_t x_;
int32_t y_;
uint32_tbutton_;
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH 2/3 wayland-fits] core: Add a wrapper for the keyboard
This adds a wrapper for a wl_keyboard in a similar way to the pointer
wrapper. It keeps track of the keys that are pressed so that they can
be quickly verified. wayland-fits now depends on libxkbcommon so that
the keyboard wrapper can pass the keymap to it and get the modifier
indices.
---
configure.ac | 2 +
src/test/Makefile.am | 2 +
src/test/core/Makefile.am | 3 +
src/test/core/display.cpp | 13 +++
src/test/core/display.h| 3 +
src/test/core/keyboard.cpp | 213 +
src/test/core/keyboard.h | 107 +++
src/test/efl/Makefile.am | 2 +
src/test/gtk+/Makefile.am | 2 +
9 files changed, 347 insertions(+)
create mode 100644 src/test/core/keyboard.cpp
create mode 100644 src/test/core/keyboard.h
diff --git a/configure.ac b/configure.ac
index de84adf..5ad59ca 100644
--- a/configure.ac
+++ b/configure.ac
@@ -61,6 +61,8 @@ PKG_CHECK_MODULES([WAYLAND],
)
PKG_CHECK_MODULES(WAYLAND_SERVER, [wayland-server = wayland_req_version])
+PKG_CHECK_MODULES([XKBCOMMON], xkbcommon)
+
have_weston=no
want_weston_extensions=auto
AC_ARG_ENABLE([weston-extensions],
diff --git a/src/test/Makefile.am b/src/test/Makefile.am
index ba5e5ce..e50c0fc 100644
--- a/src/test/Makefile.am
+++ b/src/test/Makefile.am
@@ -14,6 +14,7 @@ wfits_SOURCES = \
wfits_LDADD = \
core/libwfits-core.la \
$(CHECK_LIBS) \
+ $(XKBCOMMON_LIBS) \
$(WAYLAND_LIBS) \
$(BOOST_LIBS)
@@ -23,6 +24,7 @@ wfits_LDFLAGS = \
wfits_CPPFLAGS = \
-I$(top_srcdir)/src \
$(CHECK_CFLAGS) \
+ $(XKBCOMMON_CFLAGS) \
$(WAYLAND_CFLAGS) \
$(BOOST_CPPFLAGS)
diff --git a/src/test/core/Makefile.am b/src/test/core/Makefile.am
index bff1c5a..3a93a67 100644
--- a/src/test/core/Makefile.am
+++ b/src/test/core/Makefile.am
@@ -3,6 +3,7 @@ noinst_LTLIBRARIES = libwfits-core.la
libwfits_core_la_SOURCES = \
display.cpp \
compositor.cpp \
+ keyboard.cpp\
shell.cpp \
seat.cpp\
pointer.cpp \
@@ -23,11 +24,13 @@ libwfits_core_la_SOURCES = \
libwfits_core_la_LIBADD = \
$(WAYLAND_LIBS) \
+ $(XKBCOMMON_LIBS) \
$(CHECK_LIBS)
libwfits_core_la_CPPFLAGS =\
-I$(top_srcdir)/src \
$(WAYLAND_CFLAGS) \
+ $(XKBCOMMON_CFLAGS) \
$(CHECK_CFLAGS)
AM_CXXFLAGS = \
diff --git a/src/test/core/display.cpp b/src/test/core/display.cpp
index 1c29dbf..5c9a459 100644
--- a/src/test/core/display.cpp
+++ b/src/test/core/display.cpp
@@ -30,6 +30,7 @@ Display::Display()
: wl_display_(wl_display_connect(0))
, wl_registry_(NULL)
, globals_()
+ , xkbContext_(NULL)
{
ASSERT(wl_display_ != NULL);
@@ -49,6 +50,8 @@ Display::Display()
/*virtual*/ Display::~Display()
{
+ if (xkbContext_)
+ xkb_context_unref(xkbContext_);
wl_registry_destroy(wl_registry_);
wl_display_disconnect(*this);
}
@@ -69,6 +72,16 @@ void Display::yield(const unsigned time) const
usleep(time);
}
+struct xkb_context *Display::xkbContext() const
+{
+ if (xkbContext_ == NULL) {
+ xkbContext_ = xkb_context_new((enum xkb_context_flags) 0);
+ ASSERT(xkbContext_ != NULL);
+ }
+
+ return xkbContext_;
+}
+
/*static*/ void Display::global(
void *data, struct wl_registry *wl_registry, uint32_t id,
const char* interface, uint32_t version)
diff --git a/src/test/core/display.h b/src/test/core/display.h
index 0a4c10a..bf09057 100644
--- a/src/test/core/display.h
+++ b/src/test/core/display.h
@@ -25,6 +25,7 @@
#include map
#include wayland-client.h
+#include xkbcommon/xkbcommon.h
#include test/tools.h
namespace wfits {
@@ -63,6 +64,7 @@ public:
void roundtrip() const;
void dispatch() const;
void yield(const unsigned = 0.001 * 1e6) const;
+ struct xkb_context *xkbContext() const;
operator wl_display*() const { return wl_display_; }
operator wl_registry*() const { return wl_registry_; }
@@ -74,6 +76,7 @@ private:
wl_display *wl_display_;
wl_registry *wl_registry_;
Globals globals_;
+ mutable struct xkb_context *xkbContext_;
};
} // namespace core
diff --git a/src/test/core/keyboard.cpp b/src/test/core/keyboard.cpp
new file mode 100644
index 000..aa49413
--- /dev/null
+++ b/src/test/core/keyboard.cpp
@@ -0,0 +1,213 @@
+/*
+ * Copyright © 2013 Intel
[PATCH 3/3 wayland-fits] core: Add a test for multiple pointer and keyboard resources
This adds a test which creates multiple pointer and keyboard resources
for the same client and verifies that they all receive events. It also
tests various combiniations of pointer and keyboard focus and ensures
that for example a keyboard created while the surface already has
focus will correctly get updated about the state.
---
src/test/core/Makefile.am | 1 +
src/test/core/test_multi_resource.cpp | 259 ++
2 files changed, 260 insertions(+)
create mode 100644 src/test/core/test_multi_resource.cpp
diff --git a/src/test/core/Makefile.am b/src/test/core/Makefile.am
index 3a93a67..ec7901b 100644
--- a/src/test/core/Makefile.am
+++ b/src/test/core/Makefile.am
@@ -19,6 +19,7 @@ libwfits_core_la_SOURCES =\
test_bind_interface.cpp \
test_cursor.cpp \
test_data.cpp \
+ test_multi_resource.cpp \
test_surface.cpp\
test_dnd.cpp
diff --git a/src/test/core/test_multi_resource.cpp
b/src/test/core/test_multi_resource.cpp
new file mode 100644
index 000..d3fb003
--- /dev/null
+++ b/src/test/core/test_multi_resource.cpp
@@ -0,0 +1,259 @@
+/*
+ * 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 harness.h
+#include compositor.h
+#include pointer.h
+#include keyboard.h
+#include seat.h
+#include surface.h
+#include shell.h
+#include shell_surface.h
+#include shm.h
+
+namespace wfits {
+namespace test {
+namespace core {
+namespace input {
+
+class DummyClient
+{
+public:
+ DummyClient()
+ : display_()
+ , compositor_(display_)
+ , shell_(display_)
+ , seat_(display_)
+ , surface_(compositor_)
+ , shellSurface_(shell_, surface_)
+ , shm_(display_)
+ , buffer_(shm_, 128, 128)
+ {
+ wl_surface_attach(surface_, buffer_, 0, 0);
+ wl_surface_damage(surface_, 0, 0,
+ buffer_.width(), buffer_.height());
+ surface_.commit();
+ }
+
+private:
+ Display display_;
+ Compositor compositor_;
+ Shell shell_;
+ Seat seat_;
+ Surface surface_;
+ ShellSurface shellSurface_;
+ SharedMemory shm_;
+ SharedMemoryBuffer buffer_;
+};
+
+class MultiResourceTest : public Harness
+{
+public:
+ MultiResourceTest()
+ : Harness::Harness()
+ , compositor_(display())
+ , shell_(display())
+ , seat_(display())
+ , surface_(compositor_)
+ , shellSurface_(shell_, surface_)
+ , shm_(display())
+ , buffer_(shm_, 128, 128)
+ {
+ }
+
+ void setup();
+ void testPointer();
+ void testKeyboard();
+
+ void movePointer(const int32_t x, const int32_t y)
+ {
+ Geometry geometry(getSurfaceGeometry(surface_));
+ setGlobalPointerPosition(geometry.x + x, geometry.y + y);
+ }
+
+ void checkPointer(Pointer *pointer, const int32_t x, const int32_t y)
+ {
+ YIELD_UNTIL(pointer-x() == x and pointer-y() == y);
+ }
+
+private:
+ Compositor compositor_;
+ Shell shell_;
+ Seat seat_;
+ Surface surface_;
+ ShellSurface shellSurface_;
+ SharedMemory shm_;
+ SharedMemoryBuffer buffer_;
+};
+
+void MultiResourceTest::setup()
+{
+ wl_surface_attach(surface_, buffer_, 0, 0);
+ wl_surface_damage(surface_, 0, 0, buffer_.width(), buffer_.height());
+ surface_.commit();
+
+ queueStep(boost::bind(MultiResourceTest::testPointer,
boost::ref(*this)));
+ queueStep(boost::bind(MultiResourceTest::testKeyboard,
boost::ref(*this)));
+}
+
[PATCH 1/2] Fix a typo in a log message
Without the space it would end up printing ‘downbut’. --- src/input.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/input.c b/src/input.c index 1313b52..0c3e480 100644 --- a/src/input.c +++ b/src/input.c @@ -1115,7 +1115,7 @@ notify_touch(struct weston_seat *seat, uint32_t time, int touch_id, /* Unexpected condition: We have non-initial touch but * there is no focused surface. */ - weston_log(touch event received with %d points down + weston_log(touch event received with %d points down but no surface focused\n, seat-num_tp); return; } -- 1.8.3.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/2 v2] input: Emit events on all resources for a client
Kristian Høgsberg hoegsb...@gmail.com writes: Instead of this manual marker, you can add comments like the above below the three dashes that git inserts between the commit message and the actual patch. git am will take the text up until the --- marker as the commit message and discard everything from there on until the actual patch. This isn't quite a manual marker. If you apply the patch using ‘git am --scissors’ it will automatically cut out the message at the top. You can also do ‘git config --global mailinfo.scissors yes’ to set that automatically. I'm not sure why that's not the default. I think the scissors are slightly nicer than putting a comment next to the diffstat because you can put the message at the beginning of the email. However I'm happy to follow whatever convention the list uses. It looks like the ‘From’ tag got messed up when the patch was applied so now it has given me credit for Rob's patch and I have now co-authored it with myself. Git is hard! Regards, - Neil - Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 1/2 v2] input: Emit events on all resources for a client
Here is a second version of Robert's patch with the following changes:
I think the problem where clients would not immediately become active
when starting was caused by it calling weston_*_set_focus instead of
sending the enter event for the first keyboard or pointer resource.
The set_focus functions don't do anything if the surface already has
focus so the client was never getting the enter event. I've changed it
so that it uses the same code path regardless of whether it is the
first or a subsequent resource and now it just always explicitly sends
the enter event.
I've removed the focus listeners. We don't need them anymore because
the resource will automatically remove itself from the list when it is
destroyed and that will have the same effect as the old destroy
callback which would just unset the focus resource pointer. This
should remove the need for Robert's patch which changes the destroy
listeners to listen to the client resource.
When setting the pointer focus it will now send the keyboard modifiers
regardless of whether the focused client has a pointer resource. This
is not how it works in master and Rob's patch also changed it to do
this but I'm not sure if that was intentional. This is important
because otherwise if you get the pointer later than getting the
keyboard then the modifiers might not be up-to-date.
I've changed the move_resource function to just use wl_list_insert
which should have the same effect but will be faster because it
doesn't need to iterate the list.
In a couple of places the patch made it so that it would increment the
serial even if there are no resources for the pointer or keyboard.
I've changed these so that it checks if the resource list is empty to
make it retain the old behaviour. In particular this was happening in
binding_key and default_grab_key.
When getting the keyboard, it now sends the modifiers to the client
when it either has the pointer focus or the keyboard focus, instead of
only if it has the keyboard focus.
I've split up long lines and moved some bits of code into common
functions, for example to emit the modifiers event.
I've brought back the find_resource_for_surface functions so that the
set focus functions can avoid incrementing the serial if there is no
pointer or keyboard resource for the client of that surface.
I removed the duplicated checks for seat-touch_focus != surface in
wl_touch_set_focus because this is ensured right at the start of the
function so it is redundant to check it again.
I fixed the spelling of ‘focussed‘.
Regards,
- Neil
-- 8 --
From: Rob Bradford r...@linux.intel.com
The Wayland protocol permits a client to request the pointer, keyboard
and touch multiple times from the seat global. This is very useful in a
component like Clutter-GTK where we are combining two libraries that use
Wayland together.
This change migrates the weston input handling code to emit the
events for all the resources for the client by using the newly added
wl_resource_for_each macro to iterate over the resources that are
associated with the focused surface's client.
We maintain a list of focused resources on the pointer and keyboard
which is updated when the focus changes. However since we can have
resources created after the focus has already been set we must add the
resources to the right list and also update any state.
Additionally when setting the pointer focus it will now send the
keyboard modifiers regardless of whether the focused client has a
pointer resource. This is important because otherwise if the client
gets the pointer later than you getting the keyboard then the
modifiers might not be up-to-date.
Co-author: Neil Roberts n...@linux.intel.com
---
src/bindings.c| 21 +--
src/compositor.h | 9 +-
src/data-device.c | 16 ++-
src/input.c | 399 +++---
src/shell.c | 34 +++--
5 files changed, 309 insertions(+), 170 deletions(-)
diff --git a/src/bindings.c b/src/bindings.c
index a871c26..f6ec9ea 100644
--- a/src/bindings.c
+++ b/src/bindings.c
@@ -160,7 +160,6 @@ binding_key(struct weston_keyboard_grab *grab,
struct weston_keyboard *keyboard = grab-keyboard;
struct wl_display *display = keyboard-seat-compositor-wl_display;
- resource = grab-keyboard-focus_resource;
if (key == b-key) {
if (state == WL_KEYBOARD_KEY_STATE_RELEASED) {
weston_keyboard_end_grab(grab-keyboard);
@@ -168,9 +167,15 @@ binding_key(struct weston_keyboard_grab *grab,
keyboard-grab = keyboard-input_method_grab;
free(b);
}
- } else if (resource) {
+ } else if (!wl_list_empty(keyboard-focus_resource_list)) {
serial = wl_display_next_serial(display);
- wl_keyboard_send_key(resource, serial, time, key, state);
+ wl_resource_for_each(resource, keyboard-focus_resource_list
[PATCH 2/2] Add a hacky client to test multiple pointer/keyboard resources
** I don't expect this patch to be landed but it might be useful if
anyone wants to test the multi-resource stuff **
This adds a hacked version of simple-shm which can create multiple
pointer and keyboard resources. The resources are created with the
command line options -p and -k. Both take an integer argument which
specifies the time in seconds after the program is started when the
resource should be created. It can also take a second time with a
colon separator to specify when the resource should be released.
For example:
weston-multi-resource -p5 -p7 -k9 -p12:14
That would create a pointer after 5 seconds, a second pointer 2
seconds later, a keyboard 2 seconds after that, a third pointer after
a further 3 seconds and finally after 2 more seconds it would release
that final pointer resource.
This can be used along with WAYLAND_DEBUG to check that it gets the
right events for example if the pointer is created while the client's
surface already has focus and so on.
---
clients/Makefile.am | 9 +-
clients/multi-resource.c | 596 +++
2 files changed, 604 insertions(+), 1 deletion(-)
create mode 100644 clients/multi-resource.c
diff --git a/clients/Makefile.am b/clients/Makefile.am
index 24c6489..d4bee2a 100644
--- a/clients/Makefile.am
+++ b/clients/Makefile.am
@@ -32,7 +32,8 @@ AM_CPPFLAGS = \
if BUILD_SIMPLE_CLIENTS
simple_clients_programs = \
weston-simple-shm \
- weston-simple-touch
+ weston-simple-touch \
+ weston-multi-resource
weston_simple_shm_SOURCES = simple-shm.c \
../shared/os-compatibility.c\
@@ -45,6 +46,12 @@ weston_simple_touch_SOURCES = simple-touch.c \
../shared/os-compatibility.h
weston_simple_touch_CPPFLAGS = $(SIMPLE_CLIENT_CFLAGS)
weston_simple_touch_LDADD = $(SIMPLE_CLIENT_LIBS)
+
+weston_multi_resource_SOURCES = multi-resource.c \
+ ../shared/os-compatibility.c\
+ ../shared/os-compatibility.h
+weston_multi_resource_CPPFLAGS = $(SIMPLE_CLIENT_CFLAGS)
+weston_multi_resource_LDADD = $(SIMPLE_CLIENT_LIBS)
endif
if BUILD_SIMPLE_EGL_CLIENTS
diff --git a/clients/multi-resource.c b/clients/multi-resource.c
new file mode 100644
index 000..1c2e5c9
--- /dev/null
+++ b/clients/multi-resource.c
@@ -0,0 +1,596 @@
+/*
+ * Copyright © 2011 Benjamin Franzke
+ * Copyright © 2010, 2013 Intel Corporation
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * the above copyright notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting documentation, and
+ * that the name of the copyright holders not be used in advertising or
+ * publicity pertaining to distribution of the software without specific,
+ * written prior permission. The copyright holders make no representations
+ * about the suitability of this software for any purpose. It is provided as
+ * is without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
+ * DATA OR PROFITS, 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 config.h
+
+#include stdio.h
+#include stdlib.h
+#include errno.h
+#include string.h
+#include stdbool.h
+#include assert.h
+#include unistd.h
+#include sys/mman.h
+#include signal.h
+#include time.h
+#include sys/poll.h
+#include float.h
+
+#include wayland-client.h
+#include ../shared/os-compatibility.h
+
+struct device {
+ enum { KEYBOARD, POINTER } type;
+
+ int start_time;
+ int end_time;
+ struct wl_list link;
+
+ union {
+ struct wl_keyboard *keyboard;
+ struct wl_pointer *pointer;
+ } p;
+};
+
+struct display {
+ struct wl_display *display;
+ struct wl_registry *registry;
+ struct wl_compositor *compositor;
+ struct wl_shell *shell;
+ struct wl_seat *seat;
+ struct wl_shm *shm;
+ uint32_t formats;
+ struct wl_list devices;
+};
+
+struct window {
+ struct display *display;
+ int width, height;
+ struct wl_surface *surface;
+ struct wl_shell_surface *shell_surface;
+};
+
+static void
+buffer_release(void *data, struct wl_buffer *buffer)
+{
+ wl_buffer_destroy(buffer);
+}
+
+static const struct wl_buffer_listener buffer_listener = {
+ buffer_release
+};
+
+static int
+attach_buffer(struct window *window, int width, int height)
+{
