From b81aefcc19564c558cb7a31f36b4f2c3ab5f22dc Mon Sep 17 00:00:00 2001 From: "Eric R. Smith" Date: Wed, 25 Sep 2024 15:12:32 -0300 Subject: [PATCH] mesa: when blitting between formats clear any unused components MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the state tracker chooses to implement one format with a more general one (e.g. GL_ALPHA implemented with GL_RGBA) we end up in a situation where some components should be ignored. Readpix handles this correctly, but blit does not, which means that if we blit between different formats we can end up writing garbage into some components. Work around this by adding an explicit swizzle to the pipe_blit_info struct, which can re-arrange elements and/or put 0 or 1 into appropriate channels, and use this to set the appropriate values into unused channels via the sampler view. Reviewed-by: Marek Olšák Reviewed-by: Faith Ekstrand Part-of: --- src/gallium/auxiliary/util/u_blitter.c | 7 +++++++ src/gallium/drivers/v3d/v3d_blit.c | 6 ++++-- src/gallium/include/pipe/p_state.h | 8 ++++++++ src/mesa/main/blit.c | 21 ++++++++++++++++++++- 4 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/gallium/auxiliary/util/u_blitter.c b/src/gallium/auxiliary/util/u_blitter.c index 4db49c38fd4..ecbcff36902 100644 --- a/src/gallium/auxiliary/util/u_blitter.c +++ b/src/gallium/auxiliary/util/u_blitter.c @@ -2254,6 +2254,7 @@ util_blitter_blit(struct blitter_context *blitter, struct pipe_context *pipe = ctx->base.pipe; struct pipe_surface *dst_view, dst_templ; struct pipe_sampler_view src_templ, *src_view; + const uint8_t *swizzle = info->swizzle_enable ? info->swizzle : NULL; /* Initialize the surface. */ util_blitter_default_dst_texture(&dst_templ, dst, info->dst.level, @@ -2264,6 +2265,12 @@ util_blitter_blit(struct blitter_context *blitter, /* Initialize the sampler view. */ util_blitter_default_src_texture(blitter, &src_templ, src, info->src.level); src_templ.format = info->src.format; + if (swizzle) { + src_templ.swizzle_r = swizzle[0]; + src_templ.swizzle_g = swizzle[1]; + src_templ.swizzle_b = swizzle[2]; + src_templ.swizzle_a = swizzle[3]; + } src_view = pipe->create_sampler_view(pipe, src, &src_templ); /* Copy. */ diff --git a/src/gallium/drivers/v3d/v3d_blit.c b/src/gallium/drivers/v3d/v3d_blit.c index e065775fbf4..378068b8e5a 100644 --- a/src/gallium/drivers/v3d/v3d_blit.c +++ b/src/gallium/drivers/v3d/v3d_blit.c @@ -260,6 +260,7 @@ v3d_tfu_blit(struct pipe_context *pctx, struct pipe_blit_info *info) return; if (info->scissor_enable || + info->swizzle_enable || info->dst.box.x != 0 || info->dst.box.y != 0 || info->dst.box.width != dst_width || @@ -328,7 +329,7 @@ check_tlb_blit_ok(struct v3d_device_info *devinfo, struct pipe_blit_info *info) assert ((is_color_blit && !is_depth_blit && !is_stencil_blit) || (!is_color_blit && (is_depth_blit || is_stencil_blit))); - if (info->scissor_enable) + if (info->scissor_enable || info->swizzle_enable) return false; if (info->src.box.x != info->dst.box.x || @@ -1101,7 +1102,8 @@ v3d_sand30_blit(struct pipe_context *pctx, struct pipe_blit_info *info) return; if (!(info->mask & PIPE_MASK_RGBA)) return; - + if (info->swizzle_enable) + return; assert(dst->base.format == src->base.format); assert(dst->tiled); diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h index 8a808a530e5..e1819e8c396 100644 --- a/src/gallium/include/pipe/p_state.h +++ b/src/gallium/include/pipe/p_state.h @@ -918,6 +918,14 @@ struct pipe_blit_info bool scissor_enable; struct pipe_scissor_state scissor; + /* Swizzling during a blit typically forces a slower + path, so it should be used only when necessary. It's + there mainly to support blitting between different formats + when one of them has been emulated (e.g. GL_ALPHA emulated + by GL_RGBA) */ + bool swizzle_enable; /**< swizzle is only applied if this is set */ + uint8_t swizzle[4]; /**< map to be applied while blitting */ + /* Window rectangles can either be inclusive or exclusive. */ bool window_rectangle_include; unsigned num_window_rectangles; diff --git a/src/mesa/main/blit.c b/src/mesa/main/blit.c index c71aa14170a..89646441bec 100644 --- a/src/mesa/main/blit.c +++ b/src/mesa/main/blit.c @@ -482,6 +482,7 @@ do_blit_framebuffer(struct gl_context *ctx, struct gl_renderbuffer_attachment *srcAtt = &readFB->Attachment[readFB->_ColorReadBufferIndex]; GLuint i; + GLenum src_base_fmt, dst_base_fmt; blit.mask = PIPE_MASK_RGBA; @@ -497,6 +498,7 @@ do_blit_framebuffer(struct gl_context *ctx, if (!srcObj || !srcObj->pt) { return; } + src_base_fmt = srcObj->Image[0][0]->_BaseFormat; blit.src.resource = srcObj->pt; blit.src.level = srcAtt->TextureLevel; @@ -519,7 +521,7 @@ do_blit_framebuffer(struct gl_context *ctx, return; srcSurf = srcRb->surface; - + src_base_fmt = srcRb->_BaseFormat; blit.src.resource = srcSurf->texture; blit.src.level = srcSurf->u.tex.level; blit.src.box.z = srcSurf->u.tex.first_layer; @@ -532,6 +534,7 @@ do_blit_framebuffer(struct gl_context *ctx, if (dstRb) { struct pipe_surface *dstSurf; + dst_base_fmt = dstRb->_BaseFormat; _mesa_update_renderbuffer_surface(ctx, dstRb); dstSurf = dstRb->surface; @@ -542,6 +545,22 @@ do_blit_framebuffer(struct gl_context *ctx, blit.dst.box.z = dstSurf->u.tex.first_layer; blit.dst.format = dstSurf->format; + if (dst_base_fmt != src_base_fmt) { + uint8_t map[6]; + /* we may have to add a swizzle to the blit */ + _mesa_compute_component_mapping(src_base_fmt, dst_base_fmt, map); + for (int i = 0; i < 4; i++) { + if (map[i] > MESA_FORMAT_SWIZZLE_W) { + blit.swizzle_enable = true; + blit.swizzle[i] = map[i]; + } else { + /* the swizzle has already been mostly applied, + so don't un-do it; we only want the 0's and 1's + inserted */ + blit.swizzle[i] = i; + } + } + } ctx->pipe->blit(ctx->pipe, &blit); dstRb->defined = true; /* front buffer tracking */ }