From 0378d143d9517c7508b64b81e9267a29ab1951aa Mon Sep 17 00:00:00 2001 From: emersion Date: Wed, 13 Jun 2018 19:38:10 +0100 Subject: [PATCH 1/4] surface: remove wlr_surface.texture The texture is managed by the surface's wlr_buffer now. In particular, the buffer can destroy the texture early if it becomes invalid. --- include/wlr/types/wlr_buffer.h | 11 +++++++++-- include/wlr/types/wlr_surface.h | 14 +++++++++++++- rootston/output.c | 6 +++--- types/wlr_output.c | 10 +++++----- types/wlr_surface.c | 12 ++++++++---- 5 files changed, 38 insertions(+), 15 deletions(-) diff --git a/include/wlr/types/wlr_buffer.h b/include/wlr/types/wlr_buffer.h index fc348a1c..eabc8b51 100644 --- a/include/wlr/types/wlr_buffer.h +++ b/include/wlr/types/wlr_buffer.h @@ -8,8 +8,15 @@ * A client buffer. */ struct wlr_buffer { - struct wl_resource *resource; // can be NULL - struct wlr_texture *texture; // can be NULL + /** + * The buffer resource, if any. Will be NULL if the client destroys it. + */ + struct wl_resource *resource; + /** + * The buffer's texture, if any. A buffer will not have a texture if the + * client destroys the buffer before it has been released. + */ + struct wlr_texture *texture; bool released; size_t n_refs; diff --git a/include/wlr/types/wlr_surface.h b/include/wlr/types/wlr_surface.h index 64503e78..8517934a 100644 --- a/include/wlr/types/wlr_surface.h +++ b/include/wlr/types/wlr_surface.h @@ -69,8 +69,13 @@ struct wlr_subsurface { struct wlr_surface { struct wl_resource *resource; struct wlr_renderer *renderer; + /** + * The surface's buffer, if any. A surface has an attached buffer when it + * commits with a non-null buffer in its pending state. A surface will not + * have a buffer if it has never committed one, has committed a null buffer, + * or something went wrong with uploading the buffer. + */ struct wlr_buffer *buffer; - struct wlr_texture *texture; struct wlr_surface_state *current, *pending; const char *role; // the lifetime-bound role or null @@ -125,6 +130,13 @@ int wlr_surface_set_role(struct wlr_surface *surface, const char *role, */ bool wlr_surface_has_buffer(struct wlr_surface *surface); +/** + * Get the texture of the buffer currently attached to this surface. Returns + * NULL if no buffer is currently attached or if something went wrong with + * uploading the buffer. + */ +struct wlr_texture *wlr_surface_get_texture(struct wlr_surface *surface); + /** * Create a new subsurface resource with the provided new ID. If `resource_list` * is non-NULL, adds the subsurface's resource to the list. diff --git a/rootston/output.c b/rootston/output.c index faa808d1..353d431f 100644 --- a/rootston/output.c +++ b/rootston/output.c @@ -189,7 +189,8 @@ static void render_surface(struct wlr_surface *surface, int sx, int sy, struct roots_output *output = data->output; float rotation = data->layout.rotation; - if (!wlr_surface_has_buffer(surface)) { + struct wlr_texture *texture = wlr_surface_get_texture(surface); + if (texture == NULL) { return; } @@ -230,8 +231,7 @@ static void render_surface(struct wlr_surface *surface, int sx, int sy, pixman_box32_t *rects = pixman_region32_rectangles(&damage, &nrects); for (int i = 0; i < nrects; ++i) { scissor_output(output, &rects[i]); - wlr_render_texture_with_matrix(renderer, surface->texture, matrix, - data->alpha); + wlr_render_texture_with_matrix(renderer, texture, matrix, data->alpha); } damage_finish: diff --git a/types/wlr_output.c b/types/wlr_output.c index a5a6d0eb..7befb651 100644 --- a/types/wlr_output.c +++ b/types/wlr_output.c @@ -367,7 +367,8 @@ static void output_fullscreen_surface_render(struct wlr_output *output, struct wlr_renderer *renderer = wlr_backend_get_renderer(output->backend); assert(renderer); - if (!wlr_surface_has_buffer(surface)) { + struct wlr_texture *texture = wlr_surface_get_texture(surface); + if (texture == NULL) { wlr_renderer_clear(renderer, (float[]){0, 0, 0, 1}); return; } @@ -386,8 +387,7 @@ static void output_fullscreen_surface_render(struct wlr_output *output, for (int i = 0; i < nrects; ++i) { output_scissor(output, &rects[i]); wlr_renderer_clear(renderer, (float[]){0, 0, 0, 1}); - wlr_render_texture_with_matrix(surface->renderer, surface->texture, - matrix, 1.0f); + wlr_render_texture_with_matrix(surface->renderer, texture, matrix, 1.0f); } wlr_renderer_scissor(renderer, NULL); @@ -418,7 +418,7 @@ static void output_cursor_render(struct wlr_output_cursor *cursor, struct wlr_texture *texture = cursor->texture; if (cursor->surface != NULL) { - texture = cursor->surface->texture; + texture = wlr_surface_get_texture(cursor->surface); } if (texture == NULL) { return; @@ -700,7 +700,7 @@ static bool output_cursor_attempt_hardware(struct wlr_output_cursor *cursor) { enum wl_output_transform transform = WL_OUTPUT_TRANSFORM_NORMAL; struct wlr_texture *texture = cursor->texture; if (cursor->surface != NULL) { - texture = cursor->surface->texture; + texture = wlr_surface_get_texture(cursor->surface); scale = cursor->surface->current->scale; transform = cursor->surface->current->transform; } diff --git a/types/wlr_surface.c b/types/wlr_surface.c index 7d8da02f..af1e9446 100644 --- a/types/wlr_surface.c +++ b/types/wlr_surface.c @@ -332,7 +332,6 @@ static void surface_apply_damage(struct wlr_surface *surface, // NULL commit wlr_buffer_unref(surface->buffer); surface->buffer = NULL; - surface->texture = NULL; return; } @@ -362,7 +361,6 @@ static void surface_apply_damage(struct wlr_surface *surface, wlr_buffer_unref(surface->buffer); surface->buffer = NULL; - surface->texture = NULL; struct wlr_buffer *buffer = wlr_buffer_create(surface->renderer, resource); if (buffer == NULL) { @@ -371,7 +369,6 @@ static void surface_apply_damage(struct wlr_surface *surface, } surface->buffer = buffer; - surface->texture = buffer->texture; } static void surface_commit_pending(struct wlr_surface *surface) { @@ -660,8 +657,15 @@ struct wlr_surface *wlr_surface_create(struct wl_client *client, return surface; } +struct wlr_texture *wlr_surface_get_texture(struct wlr_surface *surface) { + if (surface->buffer == NULL) { + return NULL; + } + return surface->buffer->texture; +} + bool wlr_surface_has_buffer(struct wlr_surface *surface) { - return surface->texture != NULL; + return wlr_surface_get_texture(surface) != NULL; } int wlr_surface_set_role(struct wlr_surface *surface, const char *role, From 3a2ef75d3af98e2a6e87c12b3236ff63769f8bde Mon Sep 17 00:00:00 2001 From: emersion Date: Thu, 14 Jun 2018 08:51:38 +0100 Subject: [PATCH 2/4] Add back wlr_buffer This reverts commit d27eeaa14c9a35c709f09de862aa6d4f0ef9ff83. --- include/wlr/types/wlr_buffer.h | 56 ++++++++++ include/wlr/types/wlr_surface.h | 1 + types/meson.build | 1 + types/wlr_buffer.c | 187 ++++++++++++++++++++++++++++++++ types/wlr_surface.c | 147 ++++++++----------------- 5 files changed, 290 insertions(+), 102 deletions(-) create mode 100644 include/wlr/types/wlr_buffer.h create mode 100644 types/wlr_buffer.c diff --git a/include/wlr/types/wlr_buffer.h b/include/wlr/types/wlr_buffer.h new file mode 100644 index 00000000..fc348a1c --- /dev/null +++ b/include/wlr/types/wlr_buffer.h @@ -0,0 +1,56 @@ +#ifndef WLR_TYPES_WLR_BUFFER_H +#define WLR_TYPES_WLR_BUFFER_H + +#include +#include + +/** + * A client buffer. + */ +struct wlr_buffer { + struct wl_resource *resource; // can be NULL + struct wlr_texture *texture; // can be NULL + bool released; + size_t n_refs; + + struct wl_listener resource_destroy; +}; + +struct wlr_renderer; + +/** + * Check if a resource is a wl_buffer resource. + */ +bool wlr_resource_is_buffer(struct wl_resource *resource); +/** + * Get the size of a wl_buffer resource. + */ +bool wlr_buffer_get_resource_size(struct wl_resource *resource, + struct wlr_renderer *renderer, int *width, int *height); + +/** + * Upload a buffer to the GPU and reference it. + */ +struct wlr_buffer *wlr_buffer_create(struct wlr_renderer *renderer, + struct wl_resource *resource); +/** + * Reference the buffer. + */ +struct wlr_buffer *wlr_buffer_ref(struct wlr_buffer *buffer); +/** + * Unreference the buffer. After this call, `buffer` may not be accessed + * anymore. + */ +void wlr_buffer_unref(struct wlr_buffer *buffer); +/** + * Try to update the buffer's content. On success, returns the updated buffer + * and destroys the provided `buffer`. On error, `buffer` is intact and NULL is + * returned. + * + * Fails if there's more than one reference to the buffer or if the texture + * isn't mutable. + */ +struct wlr_buffer *wlr_buffer_apply_damage(struct wlr_buffer *buffer, + struct wl_resource *resource, pixman_region32_t *damage); + +#endif diff --git a/include/wlr/types/wlr_surface.h b/include/wlr/types/wlr_surface.h index 526e4e2c..64503e78 100644 --- a/include/wlr/types/wlr_surface.h +++ b/include/wlr/types/wlr_surface.h @@ -69,6 +69,7 @@ struct wlr_subsurface { struct wlr_surface { struct wl_resource *resource; struct wlr_renderer *renderer; + struct wlr_buffer *buffer; struct wlr_texture *texture; struct wlr_surface_state *current, *pending; const char *role; // the lifetime-bound role or null diff --git a/types/meson.build b/types/meson.build index f9f5b469..87f21c55 100644 --- a/types/meson.build +++ b/types/meson.build @@ -20,6 +20,7 @@ lib_wlr_types = static_library( 'xdg_shell_v6/wlr_xdg_surface_v6.c', 'xdg_shell_v6/wlr_xdg_toplevel_v6.c', 'wlr_box.c', + 'wlr_buffer.c', 'wlr_compositor.c', 'wlr_cursor.c', 'wlr_gamma_control.c', diff --git a/types/wlr_buffer.c b/types/wlr_buffer.c new file mode 100644 index 00000000..82a359f0 --- /dev/null +++ b/types/wlr_buffer.c @@ -0,0 +1,187 @@ +#include +#include +#include +#include +#include +#include + +bool wlr_resource_is_buffer(struct wl_resource *resource) { + return strcmp(wl_resource_get_class(resource), wl_buffer_interface.name) == 0; +} + +bool wlr_buffer_get_resource_size(struct wl_resource *resource, + struct wlr_renderer *renderer, int *width, int *height) { + assert(wlr_resource_is_buffer(resource)); + + struct wl_shm_buffer *shm_buf = wl_shm_buffer_get(resource); + if (shm_buf != NULL) { + *width = wl_shm_buffer_get_width(shm_buf); + *height = wl_shm_buffer_get_height(shm_buf); + } else if (wlr_renderer_resource_is_wl_drm_buffer(renderer, + resource)) { + wlr_renderer_wl_drm_buffer_get_size(renderer, resource, + width, height); + } else if (wlr_dmabuf_resource_is_buffer(resource)) { + struct wlr_dmabuf_buffer *dmabuf = + wlr_dmabuf_buffer_from_buffer_resource(resource); + *width = dmabuf->attributes.width; + *height = dmabuf->attributes.height; + } else { + *width = *height = 0; + return false; + } + + return true; +} + + +static void buffer_resource_handle_destroy(struct wl_listener *listener, + void *data) { + struct wlr_buffer *buffer = + wl_container_of(listener, buffer, resource_destroy); + wl_list_remove(&buffer->resource_destroy.link); + wl_list_init(&buffer->resource_destroy.link); + buffer->resource = NULL; + + if (!buffer->released) { + // The texture becomes invalid + wlr_texture_destroy(buffer->texture); + buffer->texture = NULL; + } +} + +struct wlr_buffer *wlr_buffer_create(struct wlr_renderer *renderer, + struct wl_resource *resource) { + assert(wlr_resource_is_buffer(resource)); + + struct wlr_texture *texture = NULL; + bool released = false; + + struct wl_shm_buffer *shm_buf = wl_shm_buffer_get(resource); + if (shm_buf != NULL) { + enum wl_shm_format fmt = wl_shm_buffer_get_format(shm_buf); + int32_t stride = wl_shm_buffer_get_stride(shm_buf); + int32_t width = wl_shm_buffer_get_width(shm_buf); + int32_t height = wl_shm_buffer_get_height(shm_buf); + + wl_shm_buffer_begin_access(shm_buf); + void *data = wl_shm_buffer_get_data(shm_buf); + texture = wlr_texture_from_pixels(renderer, fmt, stride, + width, height, data); + wl_shm_buffer_end_access(shm_buf); + + // We have uploaded the data, we don't need to access the wl_buffer + // anymore + wl_buffer_send_release(resource); + released = true; + } else if (wlr_renderer_resource_is_wl_drm_buffer(renderer, resource)) { + texture = wlr_texture_from_wl_drm(renderer, resource); + } else if (wlr_dmabuf_resource_is_buffer(resource)) { + struct wlr_dmabuf_buffer *dmabuf = + wlr_dmabuf_buffer_from_buffer_resource(resource); + texture = wlr_texture_from_dmabuf(renderer, &dmabuf->attributes); + } else { + wlr_log(L_ERROR, "Cannot upload texture: unknown buffer type"); + return NULL; + } + + if (texture == NULL) { + wlr_log(L_ERROR, "Failed to upload texture"); + return NULL; + } + + struct wlr_buffer *buffer = calloc(1, sizeof(struct wlr_buffer)); + if (buffer == NULL) { + return NULL; + } + buffer->resource = resource; + buffer->texture = texture; + buffer->released = released; + buffer->n_refs = 1; + + wl_resource_add_destroy_listener(resource, &buffer->resource_destroy); + buffer->resource_destroy.notify = buffer_resource_handle_destroy; + + return buffer; +} + +struct wlr_buffer *wlr_buffer_ref(struct wlr_buffer *buffer) { + buffer->n_refs++; + return buffer; +} + +void wlr_buffer_unref(struct wlr_buffer *buffer) { + if (buffer == NULL) { + return; + } + + assert(buffer->n_refs > 0); + buffer->n_refs--; + if (buffer->n_refs > 0) { + return; + } + + if (!buffer->released && buffer->resource != NULL) { + wl_buffer_send_release(buffer->resource); + } + + wl_list_remove(&buffer->resource_destroy.link); + wlr_texture_destroy(buffer->texture); + free(buffer); +} + +struct wlr_buffer *wlr_buffer_apply_damage(struct wlr_buffer *buffer, + struct wl_resource *resource, pixman_region32_t *damage) { + assert(wlr_resource_is_buffer(resource)); + + if (buffer->n_refs > 1) { + // Someone else still has a reference to the buffer + return NULL; + } + + struct wl_shm_buffer *shm_buf = wl_shm_buffer_get(resource); + if (shm_buf == NULL) { + // Uploading only damaged regions only works for wl_shm buffers + return NULL; + } + + enum wl_shm_format fmt = wl_shm_buffer_get_format(shm_buf); + int32_t stride = wl_shm_buffer_get_stride(shm_buf); + int32_t width = wl_shm_buffer_get_width(shm_buf); + int32_t height = wl_shm_buffer_get_height(shm_buf); + + int32_t texture_width, texture_height; + wlr_texture_get_size(buffer->texture, &texture_width, &texture_height); + if (width != texture_width || height != texture_height) { + return NULL; + } + + wl_shm_buffer_begin_access(shm_buf); + void *data = wl_shm_buffer_get_data(shm_buf); + + int n; + pixman_box32_t *rects = pixman_region32_rectangles(damage, &n); + for (int i = 0; i < n; ++i) { + pixman_box32_t *r = &rects[i]; + if (!wlr_texture_write_pixels(buffer->texture, fmt, stride, + r->x2 - r->x1, r->y2 - r->y1, r->x1, r->y1, + r->x1, r->y1, data)) { + wl_shm_buffer_end_access(shm_buf); + return NULL; + } + } + + wl_shm_buffer_end_access(shm_buf); + + // We have uploaded the data, we don't need to access the wl_buffer + // anymore + wl_buffer_send_release(resource); + + wl_list_remove(&buffer->resource_destroy.link); + wl_resource_add_destroy_listener(resource, &buffer->resource_destroy); + buffer->resource_destroy.notify = buffer_resource_handle_destroy; + + buffer->resource = resource; + buffer->released = true; + return buffer; +} diff --git a/types/wlr_surface.c b/types/wlr_surface.c index fca4e847..7d8da02f 100644 --- a/types/wlr_surface.c +++ b/types/wlr_surface.c @@ -1,10 +1,9 @@ #include #include #include -#include #include +#include #include -#include #include #include #include @@ -46,13 +45,6 @@ static void surface_handle_buffer_destroy(struct wl_listener *listener, surface_state_reset_buffer(state); } -static void surface_state_release_buffer(struct wlr_surface_state *state) { - if (state->buffer) { - wl_buffer_send_release(state->buffer); - surface_state_reset_buffer(state); - } -} - static void surface_state_set_buffer(struct wlr_surface_state *state, struct wl_resource *buffer) { surface_state_reset_buffer(state); @@ -175,24 +167,8 @@ static bool surface_update_size(struct wlr_surface *surface, int scale = state->scale; enum wl_output_transform transform = state->transform; - struct wl_shm_buffer *buf = wl_shm_buffer_get(state->buffer); - if (buf != NULL) { - state->buffer_width = wl_shm_buffer_get_width(buf); - state->buffer_height = wl_shm_buffer_get_height(buf); - } else if (wlr_renderer_resource_is_wl_drm_buffer(surface->renderer, - state->buffer)) { - wlr_renderer_wl_drm_buffer_get_size(surface->renderer, state->buffer, - &state->buffer_width, &state->buffer_height); - } else if (wlr_dmabuf_resource_is_buffer(state->buffer)) { - struct wlr_dmabuf_buffer *dmabuf = - wlr_dmabuf_buffer_from_buffer_resource(state->buffer); - state->buffer_width = dmabuf->attributes.width; - state->buffer_height = dmabuf->attributes.height; - } else { - wlr_log(L_ERROR, "Unknown buffer handle attached"); - state->buffer_width = 0; - state->buffer_height = 0; - } + wlr_buffer_get_resource_size(state->buffer, surface->renderer, + &state->buffer_width, &state->buffer_height); int width = state->buffer_width / scale; int height = state->buffer_height / scale; @@ -243,7 +219,6 @@ static void surface_move_state(struct wlr_surface *surface, update_size = true; } if ((next->invalid & WLR_SURFACE_INVALID_BUFFER)) { - surface_state_release_buffer(state); surface_state_set_buffer(state, next->buffer); surface_state_reset_buffer(next); state->sx = next->sx; @@ -351,91 +326,60 @@ static void surface_damage_subsurfaces(struct wlr_subsurface *subsurface) { } static void surface_apply_damage(struct wlr_surface *surface, - bool invalid_buffer, bool reupload_buffer) { + bool invalid_buffer) { struct wl_resource *resource = surface->current->buffer; if (resource == NULL) { + // NULL commit + wlr_buffer_unref(surface->buffer); + surface->buffer = NULL; + surface->texture = NULL; return; } - struct wl_shm_buffer *buf = wl_shm_buffer_get(resource); - if (buf != NULL) { - wl_shm_buffer_begin_access(buf); - - enum wl_shm_format fmt = wl_shm_buffer_get_format(buf); - int32_t stride = wl_shm_buffer_get_stride(buf); - int32_t width = wl_shm_buffer_get_width(buf); - int32_t height = wl_shm_buffer_get_height(buf); - void *data = wl_shm_buffer_get_data(buf); - - if (surface->texture == NULL || reupload_buffer) { - wlr_texture_destroy(surface->texture); - surface->texture = wlr_texture_from_pixels(surface->renderer, fmt, - stride, width, height, data); - } else { - pixman_region32_t damage; - pixman_region32_init(&damage); - pixman_region32_copy(&damage, &surface->current->buffer_damage); - pixman_region32_intersect_rect(&damage, &damage, 0, 0, - surface->current->buffer_width, - surface->current->buffer_height); - - int n; - pixman_box32_t *rects = pixman_region32_rectangles(&damage, &n); - for (int i = 0; i < n; ++i) { - pixman_box32_t *r = &rects[i]; - if (!wlr_texture_write_pixels(surface->texture, fmt, stride, - r->x2 - r->x1, r->y2 - r->y1, r->x1, r->y1, - r->x1, r->y1, data)) { - break; - } - } - - pixman_region32_fini(&damage); - } - - wl_shm_buffer_end_access(buf); - - // We've uploaded the wl_shm_buffer data to the GPU, we won't access the - // wl_buffer anymore - surface_state_release_buffer(surface->current); - } else if (invalid_buffer || reupload_buffer) { - wlr_texture_destroy(surface->texture); - - if (wlr_renderer_resource_is_wl_drm_buffer(surface->renderer, resource)) { - surface->texture = - wlr_texture_from_wl_drm(surface->renderer, resource); - } else if (wlr_dmabuf_resource_is_buffer(resource)) { - struct wlr_dmabuf_buffer *dmabuf = - wlr_dmabuf_buffer_from_buffer_resource(resource); - surface->texture = - wlr_texture_from_dmabuf(surface->renderer, &dmabuf->attributes); - } else { - surface->texture = NULL; - wlr_log(L_ERROR, "Unknown buffer handle attached"); - } - - // Don't release the wl_buffer yet: since the texture is shared with the - // client, we'll access the wl_buffer when rendering + if (surface->buffer != NULL && !surface->buffer->released && + !invalid_buffer) { + // The buffer is still the same, no need to re-upload + return; } + + if (surface->buffer != NULL && surface->buffer->released) { + pixman_region32_t damage; + pixman_region32_init(&damage); + pixman_region32_copy(&damage, &surface->current->buffer_damage); + pixman_region32_intersect_rect(&damage, &damage, 0, 0, + surface->current->buffer_width, surface->current->buffer_height); + + struct wlr_buffer *updated_buffer = + wlr_buffer_apply_damage(surface->buffer, resource, &damage); + + pixman_region32_fini(&damage); + + if (updated_buffer != NULL) { + surface->buffer = updated_buffer; + return; + } + } + + wlr_buffer_unref(surface->buffer); + surface->buffer = NULL; + surface->texture = NULL; + + struct wlr_buffer *buffer = wlr_buffer_create(surface->renderer, resource); + if (buffer == NULL) { + wlr_log(L_ERROR, "Failed to upload buffer"); + return; + } + + surface->buffer = buffer; + surface->texture = buffer->texture; } static void surface_commit_pending(struct wlr_surface *surface) { - int32_t oldw = surface->current->buffer_width; - int32_t oldh = surface->current->buffer_height; - bool invalid_buffer = surface->pending->invalid & WLR_SURFACE_INVALID_BUFFER; - bool null_buffer_commit = invalid_buffer && surface->pending->buffer == NULL; surface_move_state(surface, surface->pending, surface->current); - if (null_buffer_commit) { - wlr_texture_destroy(surface->texture); - surface->texture = NULL; - } - - bool reupload_buffer = oldw != surface->current->buffer_width || - oldh != surface->current->buffer_height; - surface_apply_damage(surface, invalid_buffer, reupload_buffer); + surface_apply_damage(surface, invalid_buffer); // commit subsurface order struct wlr_subsurface *subsurface; @@ -657,10 +601,9 @@ static void surface_handle_resource_destroy(struct wl_resource *resource) { wl_list_remove(wl_resource_get_link(surface->resource)); wl_list_remove(&surface->renderer_destroy.link); - wlr_texture_destroy(surface->texture); surface_state_destroy(surface->pending); surface_state_destroy(surface->current); - + wlr_buffer_unref(surface->buffer); free(surface); } From da114d501322fe2f1698d19a11d6dc298000dfe4 Mon Sep 17 00:00:00 2001 From: emersion Date: Thu, 14 Jun 2018 10:15:14 +0100 Subject: [PATCH 3/4] buffer: don't destroy DMA-BUF textures with wl_buffer After some discussions on #wayland, it seems that as soon as you hold a reference to a DMA-BUF (via EGLImage for instance), the underlying memory won't get free'd. The client is allowed to re-use the DMA-BUF and upload something else to it though. --- types/wlr_buffer.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/types/wlr_buffer.c b/types/wlr_buffer.c index 82a359f0..ccff7b46 100644 --- a/types/wlr_buffer.c +++ b/types/wlr_buffer.c @@ -43,11 +43,12 @@ static void buffer_resource_handle_destroy(struct wl_listener *listener, wl_list_init(&buffer->resource_destroy.link); buffer->resource = NULL; - if (!buffer->released) { - // The texture becomes invalid - wlr_texture_destroy(buffer->texture); - buffer->texture = NULL; - } + // At this point, if the wl_buffer comes from linux-dmabuf or wl_drm, we + // still haven't released it (ie. we'll read it in the future) but the + // client destroyed it. Reading the texture itself should be fine because + // we still hold a reference to the DMA-BUF via the texture. However the + // client could decide to re-use the same DMA-BUF for something else, in + // which case we'll read garbage. We decide to accept this risk. } struct wlr_buffer *wlr_buffer_create(struct wlr_renderer *renderer, @@ -80,6 +81,10 @@ struct wlr_buffer *wlr_buffer_create(struct wlr_renderer *renderer, struct wlr_dmabuf_buffer *dmabuf = wlr_dmabuf_buffer_from_buffer_resource(resource); texture = wlr_texture_from_dmabuf(renderer, &dmabuf->attributes); + + // We have imported the DMA-BUF, but we need to prevent the client from + // re-using the same DMA-BUF for the next frames, so we don't release + // the buffer yet. } else { wlr_log(L_ERROR, "Cannot upload texture: unknown buffer type"); return NULL; From 225aa815b00502c2d91897c1ac2c4c5c65f82ca5 Mon Sep 17 00:00:00 2001 From: emersion Date: Sat, 16 Jun 2018 19:01:13 +0100 Subject: [PATCH 4/4] buffer: fix wlr_texture leak on failed alloc --- types/wlr_buffer.c | 1 + 1 file changed, 1 insertion(+) diff --git a/types/wlr_buffer.c b/types/wlr_buffer.c index ccff7b46..673d7088 100644 --- a/types/wlr_buffer.c +++ b/types/wlr_buffer.c @@ -97,6 +97,7 @@ struct wlr_buffer *wlr_buffer_create(struct wlr_renderer *renderer, struct wlr_buffer *buffer = calloc(1, sizeof(struct wlr_buffer)); if (buffer == NULL) { + wlr_texture_destroy(texture); return NULL; } buffer->resource = resource;