[PATCH weston 4/8] gl: take, hold, and release a buffer reference

[Pekka Paalanen]

Instead of relying on the compositor core to keep the wl_buffer around
and unreleased, take a new reference to it in gl-renderer. This makes
sure in the future, that the gl-renderer always has the buffer at hand,
client misbehaviour excluded.

The reference is taken in the attach callback, and released in the
flush_damage callback after copy to texture, or when the next attach
callback with a different buffer occurs.

If the surface is not on the primary plane, the buffer is not released
in flush_damage. This ensures, that the buffer stays valid in case the
surface migrates to the primary plane later.

The flush_damage call in surface_handle_buffer_destroy() is removed,
becaused it would lead to list corruption: gl_renderer_flush_damage()
will drop its buffer reference, and modify the same list at another item
than we are currently traversing.

Therefore flush_damage on wl_buffer destruction does not work
anymore. This is a non-issue, however, since the only case where it
would matter, is a client destroying the buffer before release. The
protocol says the surface contents become undefined in that case.

Signed-off-by: Pekka Paalanen <ppaalanen at gmail.com>
---
 src/compositor.c  |    3 ---
 src/gl-renderer.c |   41 +++++++++++++++++++++++++++++++++++------
 2 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/src/compositor.c b/src/compositor.c
index c1fc287..49f86ea 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -187,9 +187,6 @@ surface_handle_buffer_destroy(struct wl_listener *listener, void *data)
 		container_of(listener, struct weston_surface, 
 			     buffer_destroy_listener);
 
-	if (es->buffer && wl_buffer_is_shm(es->buffer))
-		es->compositor->renderer->flush_damage(es);
-
 	es->buffer = NULL;
 }
 
diff --git a/src/gl-renderer.c b/src/gl-renderer.c
index 249efbc..e56fd28 100644
--- a/src/gl-renderer.c
+++ b/src/gl-renderer.c
@@ -62,6 +62,9 @@ struct gl_surface_state {
 	EGLImageKHR images[3];
 	GLenum target;
 	int num_images;
+
+	struct wl_buffer *buffer;
+	struct wl_listener buffer_destroy_listener;
 };
 
 struct gl_renderer {
@@ -1036,22 +1039,27 @@ gl_renderer_flush_damage(struct weston_surface *surface)
 	pixman_region32_union(&surface->texture_damage,
 			      &surface->texture_damage, &surface->damage);
 
+	if (!gs->buffer)
+		return;
+
 	/* Avoid upload, if the texture won't be used this time.
-	 * We still accumulate the damage in texture_damage.
+	 * We still accumulate the damage in texture_damage, and
+	 * hold the reference to the buffer, in case the surface
+	 * migrates back to the primary plane.
 	 */
 	if (surface->plane != &surface->compositor->primary_plane)
 		return;
 
 	if (!pixman_region32_not_empty(&surface->texture_damage))
-		return;
+		goto done;
 
 	glBindTexture(GL_TEXTURE_2D, gs->textures[0]);
 
 	if (!gr->has_unpack_subimage) {
 		glTexImage2D(GL_TEXTURE_2D, 0, GL_BGRA_EXT,
-			     surface->pitch, surface->buffer->height, 0,
+			     surface->pitch, gs->buffer->height, 0,
 			     GL_BGRA_EXT, GL_UNSIGNED_BYTE,
-			     wl_shm_buffer_get_data(surface->buffer));
+			     wl_shm_buffer_get_data(gs->buffer));
 
 		goto done;
 	}
@@ -1059,7 +1067,7 @@ gl_renderer_flush_damage(struct weston_surface *surface)
 #ifdef GL_UNPACK_ROW_LENGTH
 	/* Mesa does not define GL_EXT_unpack_subimage */
 	glPixelStorei(GL_UNPACK_ROW_LENGTH, surface->pitch);
-	data = wl_shm_buffer_get_data(surface->buffer);
+	data = wl_shm_buffer_get_data(gs->buffer);
 	rectangles = pixman_region32_rectangles(&surface->texture_damage, &n);
 	for (i = 0; i < n; i++) {
 		glPixelStorei(GL_UNPACK_SKIP_PIXELS, rectangles[i].x1);
@@ -1075,6 +1083,9 @@ gl_renderer_flush_damage(struct weston_surface *surface)
 done:
 	pixman_region32_fini(&surface->texture_damage);
 	pixman_region32_init(&surface->texture_damage);
+
+	weston_buffer_reference(&gs->buffer, NULL,
+				&gs->buffer_destroy_listener);
 }
 
 static void
@@ -1106,6 +1117,9 @@ gl_renderer_attach(struct weston_surface *es, struct wl_buffer *buffer)
 	EGLint attribs[3], format;
 	int i, num_planes;
 
+	weston_buffer_reference(&gs->buffer, buffer,
+				&gs->buffer_destroy_listener);
+
 	if (!buffer) {
 		for (i = 0; i < gs->num_images; i++) {
 			gr->destroy_image(gr->egl_display, gs->images[i]);
@@ -1186,6 +1200,8 @@ gl_renderer_attach(struct weston_surface *es, struct wl_buffer *buffer)
 		es->pitch = buffer->width;
 	} else {
 		weston_log("unhandled buffer type!\n");
+		weston_buffer_reference(&gs->buffer, NULL,
+					&gs->buffer_destroy_listener);
 	}
 }
 
@@ -1204,16 +1220,27 @@ gl_renderer_surface_set_color(struct weston_surface *surface,
 	gs->shader = &gr->solid_shader;
 }
 
+static void
+gl_renderer_handle_buffer_destroy(struct wl_listener *listener, void *data)
+{
+	struct gl_surface_state *gs =
+		container_of(listener, struct gl_surface_state, 
+			     buffer_destroy_listener);
+
+	gs->buffer = NULL;
+}
+
 static int
 gl_renderer_create_surface(struct weston_surface *surface)
 {
 	struct gl_surface_state *gs;
 
 	gs = calloc(1, sizeof *gs);
-
 	if (!gs)
 		return -1;
 
+	gs->buffer_destroy_listener.notify = gl_renderer_handle_buffer_destroy;
+
 	surface->renderer_state = gs;
 
 	return 0;
@@ -1231,6 +1258,8 @@ gl_renderer_destroy_surface(struct weston_surface *surface)
 	for (i = 0; i < gs->num_images; i++)
 		gr->destroy_image(gr->egl_display, gs->images[i]);
 
+	weston_buffer_reference(&gs->buffer, NULL,
+				&gs->buffer_destroy_listener);
 	free(gs);
 }
 
-- 
1.7.8.6