[PATCH weston v2] Add calls to wl_shm_buffer_begin/end_access

Neil Roberts neil at linux.intel.com
Wed Nov 13 07:44:06 PST 2013


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 weston_output *output,
 	else
 		pixman_image_set_filter(ps->image, PIXMAN_FILTER_NEAREST, NULL, 0);
 
+	if (ps->buffer_ref.buffer)
+		wl_shm_buffer_begin_access(ps->buffer_ref.buffer->shm_buffer);
+
 	pixman_image_composite32(pixman_op,
 				 ps->image, /* src */
 				 NULL /* mask */,
@@ -315,6 +318,9 @@ repaint_region(struct weston_view *ev, struct weston_output *output,
 				 pixman_image_get_width (po->shadow_image), /* width */
 				 pixman_image_get_height (po->shadow_image) /* height */);
 
+	if (ps->buffer_ref.buffer)
+		wl_shm_buffer_end_access(ps->buffer_ref.buffer->shm_buffer);
+
 	if (pr->repaint_debug)
 		pixman_image_composite32(PIXMAN_OP_OVER,
 					 pr->debug_color, /* src */
diff --git a/src/rpi-renderer.c b/src/rpi-renderer.c
index 8fb562d..1940db7 100644
--- a/src/rpi-renderer.c
+++ b/src/rpi-renderer.c
@@ -342,6 +342,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);
@@ -376,6 +378,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 PIXMAN_a8r8g8b8:
 	case PIXMAN_x8r8g8b8:
@@ -163,6 +165,8 @@ screenshooter_frame_notify(struct wl_listener *listener, void *data)
 		break;
 	}
 
+	wl_shm_buffer_end_access(l->buffer->shm_buffer);
+
 	screenshooter_send_done(l->resource);
 	free(pixels);
 	free(l);
-- 
1.8.3.1



More information about the wayland-devel mailing list