From a04cfca4da42d1cc01047c1cd9e60ef504beae98 Mon Sep 17 00:00:00 2001 From: Simon Ser Date: Tue, 16 Nov 2021 22:51:06 +0100 Subject: [PATCH] Remove support for DMA-BUF flags They are never used in practice, which makes all of our flag handling effectively dead code. Also, APIs such as KMS don't provide a good way to deal with the flags. Let's just fail the DMA-BUF import when clients provide flags. --- backend/drm/renderer.c | 6 ------ backend/wayland/output.c | 12 +----------- backend/x11/output.c | 4 ---- include/render/gles2.h | 2 -- include/render/vulkan.h | 1 - include/wlr/render/dmabuf.h | 7 ------- include/wlr/render/gles2.h | 1 - render/gles2/renderer.c | 4 ---- render/gles2/shaders.c | 7 +------ render/gles2/texture.c | 3 --- render/vulkan/renderer.c | 14 -------------- render/vulkan/texture.c | 13 ------------- types/wlr_export_dmabuf_v1.c | 2 +- types/wlr_linux_dmabuf_v1.c | 6 +++++- types/wlr_screencopy_v1.c | 13 ++++--------- 15 files changed, 12 insertions(+), 83 deletions(-) diff --git a/backend/drm/renderer.c b/backend/drm/renderer.c index 40e068cd..6c26558f 100644 --- a/backend/drm/renderer.c +++ b/backend/drm/renderer.c @@ -282,12 +282,6 @@ static struct wlr_drm_fb *drm_fb_create(struct wlr_drm_backend *drm, goto error_get_dmabuf; } - if (attribs.flags != 0) { - wlr_log(WLR_DEBUG, "Buffer with DMA-BUF flags 0x%"PRIX32" cannot be " - "scanned out", attribs.flags); - goto error_get_dmabuf; - } - if (formats && !wlr_drm_format_set_has(formats, attribs.format, attribs.modifier)) { // The format isn't supported by the plane. Try stripping the alpha diff --git a/backend/wayland/output.c b/backend/wayland/output.c index 8d8f036c..3fa86a3c 100644 --- a/backend/wayland/output.c +++ b/backend/wayland/output.c @@ -166,18 +166,8 @@ static struct wl_buffer *import_dmabuf(struct wlr_wl_backend *wl, dmabuf->offset[i], dmabuf->stride[i], modifier_hi, modifier_lo); } - uint32_t flags = 0; - if (dmabuf->flags & WLR_DMABUF_ATTRIBUTES_FLAGS_Y_INVERT) { - flags |= ZWP_LINUX_BUFFER_PARAMS_V1_FLAGS_Y_INVERT; - } - if (dmabuf->flags & WLR_DMABUF_ATTRIBUTES_FLAGS_INTERLACED) { - flags |= ZWP_LINUX_BUFFER_PARAMS_V1_FLAGS_INTERLACED; - } - if (dmabuf->flags & WLR_DMABUF_ATTRIBUTES_FLAGS_BOTTOM_FIRST) { - flags |= ZWP_LINUX_BUFFER_PARAMS_V1_FLAGS_BOTTOM_FIRST; - } struct wl_buffer *wl_buffer = zwp_linux_buffer_params_v1_create_immed( - params, dmabuf->width, dmabuf->height, dmabuf->format, flags); + params, dmabuf->width, dmabuf->height, dmabuf->format, 0); // TODO: handle create() errors return wl_buffer; } diff --git a/backend/x11/output.c b/backend/x11/output.c index 3ad7b4a4..76172582 100644 --- a/backend/x11/output.c +++ b/backend/x11/output.c @@ -140,10 +140,6 @@ static xcb_pixmap_t import_dmabuf(struct wlr_x11_output *output, return XCB_PIXMAP_NONE; } - if (dmabuf->flags != 0) { - return XCB_PIXMAP_NONE; - } - // xcb closes the FDs after sending them, so we need to dup them here struct wlr_dmabuf_attributes dup_attrs = {0}; if (!wlr_dmabuf_attributes_copy(&dup_attrs, dmabuf)) { diff --git a/include/render/gles2.h b/include/render/gles2.h index fb2de1ab..245b2804 100644 --- a/include/render/gles2.h +++ b/include/render/gles2.h @@ -24,7 +24,6 @@ struct wlr_gles2_pixel_format { struct wlr_gles2_tex_shader { GLuint program; GLint proj; - GLint invert_y; GLint tex; GLint alpha; GLint pos_attrib; @@ -101,7 +100,6 @@ struct wlr_gles2_texture { EGLImageKHR image; - bool inverted_y; bool has_alpha; // Only affects target == GL_TEXTURE_2D diff --git a/include/render/vulkan.h b/include/render/vulkan.h index aa40198c..1cba5a19 100644 --- a/include/render/vulkan.h +++ b/include/render/vulkan.h @@ -246,7 +246,6 @@ struct wlr_vk_texture { bool dmabuf_imported; bool owned; // if dmabuf_imported: whether we have ownership of the image bool transitioned; // if dma_imported: whether we transitioned it away from preinit - bool invert_y; // if dma_imported: whether we must flip y struct wl_list foreign_link; struct wl_list destroy_link; struct wl_list link; // wlr_gles2_renderer.textures diff --git a/include/wlr/render/dmabuf.h b/include/wlr/render/dmabuf.h index 75892d30..76aad629 100644 --- a/include/wlr/render/dmabuf.h +++ b/include/wlr/render/dmabuf.h @@ -14,16 +14,9 @@ #define WLR_DMABUF_MAX_PLANES 4 -enum wlr_dmabuf_attributes_flags { - WLR_DMABUF_ATTRIBUTES_FLAGS_Y_INVERT = 1 << 0, - WLR_DMABUF_ATTRIBUTES_FLAGS_INTERLACED = 1 << 1, - WLR_DMABUF_ATTRIBUTES_FLAGS_BOTTOM_FIRST = 1 << 2, -}; - struct wlr_dmabuf_attributes { int32_t width, height; uint32_t format; - uint32_t flags; // enum wlr_dmabuf_attributes_flags uint64_t modifier; int n_planes; diff --git a/include/wlr/render/gles2.h b/include/wlr/render/gles2.h index dabe49dd..e6844ce9 100644 --- a/include/wlr/render/gles2.h +++ b/include/wlr/render/gles2.h @@ -30,7 +30,6 @@ struct wlr_gles2_texture_attribs { GLenum target; /* either GL_TEXTURE_2D or GL_TEXTURE_EXTERNAL_OES */ GLuint tex; - bool inverted_y; bool has_alpha; }; diff --git a/render/gles2/renderer.c b/render/gles2/renderer.c index 01efaf1d..527d85bf 100644 --- a/render/gles2/renderer.c +++ b/render/gles2/renderer.c @@ -309,7 +309,6 @@ static bool gles2_render_subtexture_with_matrix( glUseProgram(shader->program); glUniformMatrix3fv(shader->proj, 1, GL_FALSE, gl_matrix); - glUniform1i(shader->invert_y, texture->inverted_y); glUniform1i(shader->tex, 0); glUniform1f(shader->alpha, alpha); @@ -810,7 +809,6 @@ struct wlr_renderer *wlr_gles2_renderer_create(struct wlr_egl *egl) { goto error; } renderer->shaders.tex_rgba.proj = glGetUniformLocation(prog, "proj"); - renderer->shaders.tex_rgba.invert_y = glGetUniformLocation(prog, "invert_y"); renderer->shaders.tex_rgba.tex = glGetUniformLocation(prog, "tex"); renderer->shaders.tex_rgba.alpha = glGetUniformLocation(prog, "alpha"); renderer->shaders.tex_rgba.pos_attrib = glGetAttribLocation(prog, "pos"); @@ -822,7 +820,6 @@ struct wlr_renderer *wlr_gles2_renderer_create(struct wlr_egl *egl) { goto error; } renderer->shaders.tex_rgbx.proj = glGetUniformLocation(prog, "proj"); - renderer->shaders.tex_rgbx.invert_y = glGetUniformLocation(prog, "invert_y"); renderer->shaders.tex_rgbx.tex = glGetUniformLocation(prog, "tex"); renderer->shaders.tex_rgbx.alpha = glGetUniformLocation(prog, "alpha"); renderer->shaders.tex_rgbx.pos_attrib = glGetAttribLocation(prog, "pos"); @@ -835,7 +832,6 @@ struct wlr_renderer *wlr_gles2_renderer_create(struct wlr_egl *egl) { goto error; } renderer->shaders.tex_ext.proj = glGetUniformLocation(prog, "proj"); - renderer->shaders.tex_ext.invert_y = glGetUniformLocation(prog, "invert_y"); renderer->shaders.tex_ext.tex = glGetUniformLocation(prog, "tex"); renderer->shaders.tex_ext.alpha = glGetUniformLocation(prog, "alpha"); renderer->shaders.tex_ext.pos_attrib = glGetAttribLocation(prog, "pos"); diff --git a/render/gles2/shaders.c b/render/gles2/shaders.c index d854b270..7898059e 100644 --- a/render/gles2/shaders.c +++ b/render/gles2/shaders.c @@ -28,18 +28,13 @@ const GLchar quad_fragment_src[] = // Textured quads const GLchar tex_vertex_src[] = "uniform mat3 proj;\n" -"uniform bool invert_y;\n" "attribute vec2 pos;\n" "attribute vec2 texcoord;\n" "varying vec2 v_texcoord;\n" "\n" "void main() {\n" " gl_Position = vec4(proj * vec3(pos, 1.0), 1.0);\n" -" if (invert_y) {\n" -" v_texcoord = vec2(texcoord.x, 1.0 - texcoord.y);\n" -" } else {\n" -" v_texcoord = texcoord;\n" -" }\n" +" v_texcoord = texcoord;\n" "}\n"; const GLchar tex_fragment_src_rgba[] = diff --git a/render/gles2/texture.c b/render/gles2/texture.c index 293b7a19..8d6f3fc2 100644 --- a/render/gles2/texture.c +++ b/render/gles2/texture.c @@ -250,8 +250,6 @@ static struct wlr_texture *gles2_texture_from_dmabuf( return NULL; } texture->drm_format = DRM_FORMAT_INVALID; // texture can't be written anyways - texture->inverted_y = - (attribs->flags & WLR_DMABUF_ATTRIBUTES_FLAGS_Y_INVERT) != 0; const struct wlr_pixel_format_info *drm_fmt = drm_get_pixel_format_info(attribs->format); @@ -363,6 +361,5 @@ void wlr_gles2_texture_get_attribs(struct wlr_texture *wlr_texture, memset(attribs, 0, sizeof(*attribs)); attribs->target = texture->target; attribs->tex = texture->tex; - attribs->inverted_y = texture->inverted_y; attribs->has_alpha = texture->has_alpha; } diff --git a/render/vulkan/renderer.c b/render/vulkan/renderer.c index 31ce0b76..a1d8d41e 100644 --- a/render/vulkan/renderer.c +++ b/render/vulkan/renderer.c @@ -433,15 +433,6 @@ static struct wlr_vk_render_buffer *create_render_buffer( wlr_log(WLR_DEBUG, "vulkan create_render_buffer: %.4s, %dx%d", (const char*) &dmabuf.format, dmabuf.width, dmabuf.height); - // NOTE: we could at least support WLR_DMABUF_ATTRIBUTES_FLAGS_Y_INVERT - // if it is needed by anyone. Can be implemented using negative viewport - // height or flipping matrix. - if (dmabuf.flags != 0) { - wlr_log(WLR_ERROR, "dmabuf flags %x not supported/implemented on vulkan", - dmabuf.flags); - goto error_buffer; - } - buffer->image = vulkan_import_dmabuf(renderer, &dmabuf, buffer->memories, &buffer->mem_count, true); if (!buffer->image) { @@ -789,11 +780,6 @@ static bool vulkan_render_subtexture_with_matrix(struct wlr_renderer *wlr_render vert_pcr_data.uv_size[0] = box->width / wlr_texture->width; vert_pcr_data.uv_size[1] = box->height / wlr_texture->height; - if (texture->invert_y) { - vert_pcr_data.uv_off[1] += vert_pcr_data.uv_size[1]; - vert_pcr_data.uv_size[1] = -vert_pcr_data.uv_size[1]; - } - vkCmdPushConstants(cb, renderer->pipe_layout, VK_SHADER_STAGE_VERTEX_BIT, 0, sizeof(vert_pcr_data), &vert_pcr_data); vkCmdPushConstants(cb, renderer->pipe_layout, diff --git a/render/vulkan/texture.c b/render/vulkan/texture.c index f6fbec5a..76c37011 100644 --- a/render/vulkan/texture.c +++ b/render/vulkan/texture.c @@ -605,19 +605,6 @@ static struct wlr_texture *vulkan_texture_from_dmabuf(struct wlr_renderer *wlr_r goto error; } - uint32_t flags = attribs->flags; - if (flags & WLR_DMABUF_ATTRIBUTES_FLAGS_Y_INVERT) { - texture->invert_y = true; - flags &= ~WLR_DMABUF_ATTRIBUTES_FLAGS_Y_INVERT; - } - - if (flags != 0) { - wlr_log(WLR_ERROR, "dmabuf flags %x not supported/implemented on vulkan", - attribs->flags); - // NOTE: should probably make this a critical error in future - // return VK_NULL_HANDLE; - } - const struct wlr_pixel_format_info *format_info = drm_get_pixel_format_info(attribs->format); assert(format_info); diff --git a/types/wlr_export_dmabuf_v1.c b/types/wlr_export_dmabuf_v1.c index 954a3270..d26a9a45 100644 --- a/types/wlr_export_dmabuf_v1.c +++ b/types/wlr_export_dmabuf_v1.c @@ -77,7 +77,7 @@ static void frame_output_handle_commit(struct wl_listener *listener, uint32_t mod_high = attribs.modifier >> 32; uint32_t mod_low = attribs.modifier & 0xFFFFFFFF; zwlr_export_dmabuf_frame_v1_send_frame(frame->resource, - attribs.width, attribs.height, 0, 0, attribs.flags, frame_flags, + attribs.width, attribs.height, 0, 0, 0, frame_flags, attribs.format, mod_high, mod_low, attribs.n_planes); for (int i = 0; i < attribs.n_planes; ++i) { diff --git a/types/wlr_linux_dmabuf_v1.c b/types/wlr_linux_dmabuf_v1.c index b111a721..9b4cdd44 100644 --- a/types/wlr_linux_dmabuf_v1.c +++ b/types/wlr_linux_dmabuf_v1.c @@ -215,10 +215,14 @@ static void params_create_common(struct wl_resource *params_resource, goto err_out; } + if (flags != 0) { + wlr_log(WLR_ERROR, "dmabuf flags aren't supported"); + goto err_failed; + } + attribs.width = width; attribs.height = height; attribs.format = format; - attribs.flags = flags; if (width < 1 || height < 1) { wl_resource_post_error(params_resource, diff --git a/types/wlr_screencopy_v1.c b/types/wlr_screencopy_v1.c index 7c7d7e95..3f3d3bc5 100644 --- a/types/wlr_screencopy_v1.c +++ b/types/wlr_screencopy_v1.c @@ -254,8 +254,7 @@ error_src_tex: return false; } -static bool frame_dma_copy(struct wlr_screencopy_frame_v1 *frame, - uint32_t *flags) { +static bool frame_dma_copy(struct wlr_screencopy_frame_v1 *frame) { struct wlr_dmabuf_v1_buffer *dma_buffer = frame->dma_buffer; struct wlr_output *output = frame->output; struct wlr_renderer *renderer = wlr_backend_get_renderer(output->backend); @@ -268,11 +267,7 @@ static bool frame_dma_copy(struct wlr_screencopy_frame_v1 *frame, return false; } - bool ok = blit_dmabuf(renderer, dma_buffer, output->front_buffer); - *flags = dma_buffer->attributes.flags & WLR_DMABUF_ATTRIBUTES_FLAGS_Y_INVERT ? - ZWLR_SCREENCOPY_FRAME_V1_FLAGS_Y_INVERT : 0; - - return ok; + return blit_dmabuf(renderer, dma_buffer, output->front_buffer); } static void frame_handle_output_commit(struct wl_listener *listener, @@ -282,7 +277,6 @@ static void frame_handle_output_commit(struct wl_listener *listener, struct wlr_output_event_commit *event = data; struct wlr_output *output = frame->output; struct wlr_renderer *renderer = wlr_backend_get_renderer(output->backend); - uint32_t flags; assert(renderer); if (!(event->committed & WLR_OUTPUT_STATE_BUFFER)) { @@ -304,8 +298,9 @@ static void frame_handle_output_commit(struct wl_listener *listener, wl_list_remove(&frame->output_commit.link); wl_list_init(&frame->output_commit.link); + uint32_t flags = 0; bool ok = frame->shm_buffer ? - frame_shm_copy(frame, &flags) : frame_dma_copy(frame, &flags); + frame_shm_copy(frame, &flags) : frame_dma_copy(frame); if (!ok) { zwlr_screencopy_frame_v1_send_failed(frame->resource); frame_destroy(frame);