From de685b8843f9070507ccca631d08cb5fff93e622 Mon Sep 17 00:00:00 2001 From: Pekka Paalanen Date: Tue, 4 Dec 2012 15:58:12 +0200 Subject: [PATCH] compositor: introduce weston_buffer_reference The wl_buffer reference counting API has been inconsistent. You would manually increment the refcount and register a destroy listener, as opposed to calling weston_buffer_post_release(), which internally decremented the refcount, and then removing a list item. Replace both cases with a single function: weston_buffer_reference(weston_buffer_reference *ref, wl_buffer *buffer) Buffer is assigned to ref->buffer, while taking care of all the refcounting and release posting. You take a reference by passing a non-NULL buffer, and release a reference by passing NULL as buffer. The function uses an internal wl_buffer destroy listener, so the pointer gets reset on destruction automatically. This is inspired by the pipe_resource_reference() of Mesa, and modified by krh's suggestion to add struct weston_buffer_reference. Additionally, when a surface gets destroyed, the associated wl_buffer will send a release event. Often the buffer is already destroyed on client side, so the event will be discarded by libwayland-client. Compositor-drm.c is converted to use weston_buffer_reference. Signed-off-by: Pekka Paalanen --- src/compositor-drm.c | 56 ++++++++++++-------------------- src/compositor-rpi.c | 8 +++-- src/compositor.c | 77 +++++++++++++++++++++++++------------------- src/compositor.h | 12 +++++-- src/gl-renderer.c | 7 ++-- src/shell.c | 2 +- src/tablet-shell.c | 4 +-- 7 files changed, 87 insertions(+), 79 deletions(-) diff --git a/src/compositor-drm.c b/src/compositor-drm.c index 27f2c7d8..da36cd3f 100644 --- a/src/compositor-drm.c +++ b/src/compositor-drm.c @@ -118,8 +118,7 @@ struct drm_fb { struct drm_output *output; uint32_t fb_id; int is_client_buffer; - struct wl_buffer *buffer; - struct wl_listener buffer_destroy_listener; + struct weston_buffer_reference buffer_ref; }; struct drm_output { @@ -208,10 +207,7 @@ drm_fb_destroy_callback(struct gbm_bo *bo, void *data) if (fb->fb_id) drmModeRmFB(gbm_device_get_fd(gbm), fb->fb_id); - if (fb->buffer) { - weston_buffer_post_release(fb->buffer); - wl_list_remove(&fb->buffer_destroy_listener.link); - } + weston_buffer_reference(&fb->buffer_ref, NULL); free(data); } @@ -231,7 +227,7 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_compositor *compositor) fb->bo = bo; fb->is_client_buffer = 0; - fb->buffer = NULL; + fb->buffer_ref.buffer = NULL; width = gbm_bo_get_width(bo); height = gbm_bo_get_height(bo); @@ -282,27 +278,14 @@ err_free: return NULL; } -static void -fb_handle_buffer_destroy(struct wl_listener *listener, void *data) -{ - struct drm_fb *fb = container_of(listener, struct drm_fb, - buffer_destroy_listener); - - fb->buffer = NULL; -} - static void drm_fb_set_buffer(struct drm_fb *fb, struct wl_buffer *buffer) { - assert(fb->buffer == NULL); + assert(fb->buffer_ref.buffer == NULL); fb->is_client_buffer = 1; - fb->buffer = buffer; - fb->buffer->busy_count++; - fb->buffer_destroy_listener.notify = fb_handle_buffer_destroy; - wl_signal_add(&fb->buffer->resource.destroy_signal, - &fb->buffer_destroy_listener); + weston_buffer_reference(&fb->buffer_ref, buffer); } static int @@ -340,19 +323,20 @@ drm_output_prepare_scanout_surface(struct weston_output *_output, struct drm_output *output = (struct drm_output *) _output; struct drm_compositor *c = (struct drm_compositor *) output->base.compositor; + struct wl_buffer *buffer = es->buffer_ref.buffer; struct gbm_bo *bo; if (es->geometry.x != output->base.x || es->geometry.y != output->base.y || - es->buffer == NULL || - es->buffer->width != output->base.current->width || - es->buffer->height != output->base.current->height || + buffer == NULL || + buffer->width != output->base.current->width || + buffer->height != output->base.current->height || output->base.transform != es->buffer_transform || es->transform.enabled) return NULL; bo = gbm_bo_import(c->gbm, GBM_BO_IMPORT_WL_BUFFER, - es->buffer, GBM_BO_USE_SCANOUT); + buffer, GBM_BO_USE_SCANOUT); /* Unable to use the buffer for scanout */ if (!bo) @@ -369,7 +353,7 @@ drm_output_prepare_scanout_surface(struct weston_output *_output, return NULL; } - drm_fb_set_buffer(output->next, es->buffer); + drm_fb_set_buffer(output->next, buffer); return &output->fb_plane; } @@ -601,13 +585,13 @@ drm_output_prepare_overlay_surface(struct weston_output *output_base, if (es->output_mask != (1u << output_base->id)) return NULL; - if (es->buffer == NULL) + if (es->buffer_ref.buffer == NULL) return NULL; if (es->alpha != 1.0f) return NULL; - if (wl_buffer_is_shm(es->buffer)) + if (wl_buffer_is_shm(es->buffer_ref.buffer)) return NULL; if (!drm_surface_transform_supported(es)) @@ -628,7 +612,7 @@ drm_output_prepare_overlay_surface(struct weston_output *output_base, return NULL; bo = gbm_bo_import(c->gbm, GBM_BO_IMPORT_WL_BUFFER, - es->buffer, GBM_BO_USE_SCANOUT); + es->buffer_ref.buffer, GBM_BO_USE_SCANOUT); if (!bo) return NULL; @@ -645,7 +629,7 @@ drm_output_prepare_overlay_surface(struct weston_output *output_base, return NULL; } - drm_fb_set_buffer(s->next, es->buffer); + drm_fb_set_buffer(s->next, es->buffer_ref.buffer); box = pixman_region32_extents(&es->transform.boundingbox); s->plane.x = box->x1; @@ -715,7 +699,8 @@ drm_output_prepare_cursor_surface(struct weston_output *output_base, return NULL; if (c->cursors_are_broken) return NULL; - if (es->buffer == NULL || !wl_buffer_is_shm(es->buffer) || + if (es->buffer_ref.buffer == NULL || + !wl_buffer_is_shm(es->buffer_ref.buffer) || es->geometry.width > 64 || es->geometry.height > 64) return NULL; @@ -742,14 +727,15 @@ drm_output_set_cursor(struct drm_output *output) return; } - if (es->buffer && pixman_region32_not_empty(&output->cursor_plane.damage)) { + if (es->buffer_ref.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(es->buffer); - s = wl_shm_buffer_get_data(es->buffer); + stride = wl_shm_buffer_get_stride(es->buffer_ref.buffer); + s = wl_shm_buffer_get_data(es->buffer_ref.buffer); for (i = 0; i < es->geometry.height; i++) memcpy(buf + i * 64, s + i * stride, es->geometry.width * 4); diff --git a/src/compositor-rpi.c b/src/compositor-rpi.c index 4621b72d..e77c5c6f 100644 --- a/src/compositor-rpi.c +++ b/src/compositor-rpi.c @@ -746,7 +746,8 @@ rpi_assign_plane(struct weston_surface *surface, struct rpi_output *output) } /* only shm surfaces supported */ - if (surface->buffer && !wl_buffer_is_shm(surface->buffer)) { + if (surface->buffer_ref.buffer && + !wl_buffer_is_shm(surface->buffer_ref.buffer)) { DBG("surface %p rejected: not shm\n", surface); return NULL; } @@ -760,7 +761,7 @@ rpi_assign_plane(struct weston_surface *surface, struct rpi_output *output) element->plane.y = floor(surface->geometry.y); DBG("surface %p reuse element %p\n", surface, element); } else { - if (!surface->buffer) { + if (!surface->buffer_ref.buffer) { DBG("surface %p rejected: no buffer\n", surface); return NULL; } @@ -817,7 +818,8 @@ rpi_output_assign_planes(struct weston_output *base) * damage, if the plane is new, so no need to force * initial resource update. */ - if (rpi_element_damage(element, surface->buffer, + if (rpi_element_damage(element, + surface->buffer_ref.buffer, &surface->damage) < 0) { rpi_element_schedule_destroy(element); DBG("surface %p rejected: resource update failed\n", diff --git a/src/compositor.c b/src/compositor.c index d80d929a..869fffa2 100644 --- a/src/compositor.c +++ b/src/compositor.c @@ -180,16 +180,6 @@ weston_client_launch(struct weston_compositor *compositor, return client; } -static void -surface_handle_buffer_destroy(struct wl_listener *listener, void *data) -{ - struct weston_surface *es = - container_of(listener, struct weston_surface, - buffer_destroy_listener); - - es->buffer = NULL; -} - static void surface_handle_pending_buffer_destroy(struct wl_listener *listener, void *data) { @@ -241,7 +231,6 @@ weston_surface_create(struct weston_compositor *compositor) pixman_region32_init(&surface->texture_damage); - surface->buffer = NULL; surface->buffer_transform = WL_OUTPUT_TRANSFORM_NORMAL; surface->pending.buffer_transform = surface->buffer_transform; surface->output = NULL; @@ -254,9 +243,6 @@ weston_surface_create(struct weston_compositor *compositor) pixman_region32_init(&surface->transform.opaque); wl_list_init(&surface->frame_callback_list); - surface->buffer_destroy_listener.notify = - surface_handle_buffer_destroy; - wl_list_init(&surface->geometry.transformation_list); wl_list_insert(&surface->geometry.transformation_list, &surface->transform.position.link); @@ -731,9 +717,9 @@ weston_surface_buffer_width(struct weston_surface *surface) case WL_OUTPUT_TRANSFORM_270: case WL_OUTPUT_TRANSFORM_FLIPPED_90: case WL_OUTPUT_TRANSFORM_FLIPPED_270: - return surface->buffer->height; + return surface->buffer_ref.buffer->height; default: - return surface->buffer->width; + return surface->buffer_ref.buffer->width; } } @@ -745,9 +731,9 @@ weston_surface_buffer_height(struct weston_surface *surface) case WL_OUTPUT_TRANSFORM_270: case WL_OUTPUT_TRANSFORM_FLIPPED_90: case WL_OUTPUT_TRANSFORM_FLIPPED_270: - return surface->buffer->width; + return surface->buffer_ref.buffer->width; default: - return surface->buffer->height; + return surface->buffer_ref.buffer->height; } } @@ -877,8 +863,7 @@ destroy_surface(struct wl_resource *resource) if (surface->pending.buffer) wl_list_remove(&surface->pending.buffer_destroy_listener.link); - if (surface->buffer) - wl_list_remove(&surface->buffer_destroy_listener.link); + weston_buffer_reference(&surface->buffer_ref, NULL); pixman_region32_fini(&surface->texture_damage); compositor->renderer->destroy_surface(surface); @@ -907,26 +892,47 @@ weston_surface_destroy(struct weston_surface *surface) } static void -weston_surface_attach(struct weston_surface *surface, struct wl_buffer *buffer) +weston_buffer_reference_handle_destroy(struct wl_listener *listener, + void *data) +{ + struct weston_buffer_reference *ref = + container_of(listener, struct weston_buffer_reference, + destroy_listener); + + assert((struct wl_buffer *)data == ref->buffer); + ref->buffer = NULL; +} + +WL_EXPORT void +weston_buffer_reference(struct weston_buffer_reference *ref, + struct wl_buffer *buffer) { - if (surface->buffer && buffer != surface->buffer) { - weston_buffer_post_release(surface->buffer); - wl_list_remove(&surface->buffer_destroy_listener.link); + if (ref->buffer && buffer != ref->buffer) { + weston_buffer_post_release(ref->buffer); + wl_list_remove(&ref->destroy_listener.link); } - if (buffer && buffer != surface->buffer) { + if (buffer && buffer != ref->buffer) { buffer->busy_count++; wl_signal_add(&buffer->resource.destroy_signal, - &surface->buffer_destroy_listener); + &ref->destroy_listener); } + ref->buffer = buffer; + ref->destroy_listener.notify = weston_buffer_reference_handle_destroy; +} + +static void +weston_surface_attach(struct weston_surface *surface, struct wl_buffer *buffer) +{ + weston_buffer_reference(&surface->buffer_ref, buffer); + if (!buffer) { if (weston_surface_is_mapped(surface)) weston_surface_unmap(surface); } surface->compositor->renderer->attach(surface, buffer); - surface->buffer = buffer; } WL_EXPORT void @@ -1006,7 +1012,8 @@ static void surface_accumulate_damage(struct weston_surface *surface, pixman_region32_t *opaque) { - if (surface->buffer && wl_buffer_is_shm(surface->buffer)) + if (surface->buffer_ref.buffer && + wl_buffer_is_shm(surface->buffer_ref.buffer)) surface->compositor->renderer->flush_damage(surface); if (surface->transform.enabled) { @@ -1243,6 +1250,8 @@ surface_attach(struct wl_client *client, if (buffer_resource) buffer = buffer_resource->data; + /* Attach, attach, without commit in between does not send + * wl_buffer.release. */ if (surface->pending.buffer) wl_list_remove(&surface->pending.buffer_destroy_listener.link); @@ -1380,7 +1389,7 @@ surface_commit(struct wl_client *client, struct wl_resource *resource) if (surface->pending.buffer || surface->pending.remove_contents) weston_surface_attach(surface, surface->pending.buffer); - if (surface->buffer && surface->configure) + if (surface->buffer_ref.buffer && surface->configure) surface->configure(surface, surface->pending.sx, surface->pending.sy); surface->pending.sx = 0; @@ -2126,7 +2135,8 @@ pointer_cursor_surface_configure(struct weston_surface *es, y = wl_fixed_to_int(seat->seat.pointer->y) - seat->hotspot_y; weston_surface_configure(seat->sprite, x, y, - es->buffer->width, es->buffer->height); + es->buffer_ref.buffer->width, + es->buffer_ref.buffer->height); empty_region(&es->pending.input); @@ -2192,7 +2202,7 @@ pointer_set_cursor(struct wl_client *client, struct wl_resource *resource, seat->hotspot_x = x; seat->hotspot_y = y; - if (surface->buffer) + if (surface->buffer_ref.buffer) pointer_cursor_surface_configure(surface, 0, 0); } @@ -2567,7 +2577,8 @@ drag_surface_configure(struct weston_surface *es, int32_t sx, int32_t sy) weston_surface_configure(es, es->geometry.x + sx, es->geometry.y + sy, - es->buffer->width, es->buffer->height); + es->buffer_ref.buffer->width, + es->buffer_ref.buffer->height); } static int @@ -2615,7 +2626,7 @@ device_map_drag_surface(struct weston_seat *seat) struct wl_list *list; if (weston_surface_is_mapped(seat->drag_surface) || - !seat->drag_surface->buffer) + !seat->drag_surface->buffer_ref.buffer) return; if (seat->sprite && weston_surface_is_mapped(seat->sprite)) diff --git a/src/compositor.h b/src/compositor.h index ff7c932f..53f9cfb7 100644 --- a/src/compositor.h +++ b/src/compositor.h @@ -345,6 +345,11 @@ struct weston_compositor { struct weston_xkb_info xkb_info; }; +struct weston_buffer_reference { + struct wl_buffer *buffer; + struct wl_listener destroy_listener; +}; + struct weston_region { struct wl_resource resource; pixman_region32_t region; @@ -437,8 +442,7 @@ struct weston_surface { struct wl_list frame_callback_list; - struct wl_buffer *buffer; - struct wl_listener buffer_destroy_listener; + struct weston_buffer_reference buffer_ref; uint32_t buffer_transform; /* All the pending state, that wl_surface.commit will apply. */ @@ -686,6 +690,10 @@ weston_surface_unmap(struct weston_surface *surface); void weston_buffer_post_release(struct wl_buffer *buffer); +void +weston_buffer_reference(struct weston_buffer_reference *ref, + struct wl_buffer *buffer); + uint32_t weston_compositor_get_time(void); diff --git a/src/gl-renderer.c b/src/gl-renderer.c index d459c210..f5814822 100644 --- a/src/gl-renderer.c +++ b/src/gl-renderer.c @@ -1038,6 +1038,7 @@ gl_renderer_flush_damage(struct weston_surface *surface) { struct gl_renderer *gr = get_renderer(surface->compositor); struct gl_surface_state *gs = get_surface_state(surface); + struct wl_buffer *buffer = surface->buffer_ref.buffer; #ifdef GL_UNPACK_ROW_LENGTH pixman_box32_t *rectangles; @@ -1061,9 +1062,9 @@ gl_renderer_flush_damage(struct weston_surface *surface) if (!gr->has_unpack_subimage) { glTexImage2D(GL_TEXTURE_2D, 0, GL_BGRA_EXT, - surface->pitch, surface->buffer->height, 0, + surface->pitch, buffer->height, 0, GL_BGRA_EXT, GL_UNSIGNED_BYTE, - wl_shm_buffer_get_data(surface->buffer)); + wl_shm_buffer_get_data(buffer)); goto done; } @@ -1071,7 +1072,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(buffer); rectangles = pixman_region32_rectangles(&surface->texture_damage, &n); for (i = 0; i < n; i++) { pixman_box32_t r; diff --git a/src/shell.c b/src/shell.c index 28fe6cb9..232d4133 100644 --- a/src/shell.c +++ b/src/shell.c @@ -1667,7 +1667,7 @@ shell_configure_fullscreen(struct shell_surface *shsurf) switch (shsurf->fullscreen.type) { case WL_SHELL_SURFACE_FULLSCREEN_METHOD_DEFAULT: - if (surface->buffer) + if (surface->buffer_ref.buffer) center_on_output(surface, shsurf->fullscreen_output); break; case WL_SHELL_SURFACE_FULLSCREEN_METHOD_SCALE: diff --git a/src/tablet-shell.c b/src/tablet-shell.c index 36237360..9e88e27e 100644 --- a/src/tablet-shell.c +++ b/src/tablet-shell.c @@ -132,8 +132,8 @@ tablet_shell_surface_configure(struct weston_surface *surface, if (weston_surface_is_mapped(surface)) return; - width = surface->buffer->width; - height = surface->buffer->height; + width = surface->buffer_ref.buffer->width; + height = surface->buffer_ref.buffer->height; weston_surface_configure(surface, 0, 0, width, height);