From 08581e2e92385b2cdcb533d49fe7a3b0e6f60f3e Mon Sep 17 00:00:00 2001 From: Connor Abbott Date: Wed, 27 Sep 2023 14:17:19 +0200 Subject: [PATCH] zink: Rework color clamping and conversion Before this commit, zink_format_clamp_channel_color() ignored the format swizzle, so it was assuming that for emulated formats like alpha, alpha-luminance etc. that the color had already been swizzled to match the internal format rather than the emulated one. It's somewhat confusing that passing in e.g. A8_UNORM actually means R8_UNORM, and led to a bug when using VK_FORMAT_A8_UNORM for texture border colors because we didn't swizzle it back. It also wouldn't have worked for media formats like R10X6G10X6 due to the void channel in the middle. In order to fix this, we need to untangle the mess in its users. For convert_color() used when clearing, this means we now need to clamp and then swizzle instead of swizzle and then clamp, and we can drop the hack for A8_UNORM. For texture border colors, the state tracker duplicates colors for the emulated formats to help drivers, which zink was previously relying on, but fixing zink_format_clamp_channel_color() breaks this because it assumes that those duplicated colors are useless and clamps them. However, because we know the format we can just swizzle the border color ourself, which convert_color() was already doing. So, we pull that out into a common zink_convert_color() function that handles both clamping and format emulation, and have both clearing and border color handling use it. This fixes A8_UNORM in turnip+zink once we enable it. Part-of: --- src/gallium/drivers/zink/zink_clear.c | 48 ++++--------------------- src/gallium/drivers/zink/zink_context.c | 5 +-- src/gallium/drivers/zink/zink_format.c | 44 ++++++++++++++--------- src/gallium/drivers/zink/zink_screen.c | 38 ++++++++++++++++++++ src/gallium/drivers/zink/zink_screen.h | 5 +++ 5 files changed, 79 insertions(+), 61 deletions(-) diff --git a/src/gallium/drivers/zink/zink_clear.c b/src/gallium/drivers/zink/zink_clear.c index d9eb20ab27b..00f134e0a81 100644 --- a/src/gallium/drivers/zink/zink_clear.c +++ b/src/gallium/drivers/zink/zink_clear.c @@ -145,44 +145,6 @@ get_clear_data(struct zink_context *ctx, struct zink_framebuffer_clear *fb_clear return add_new_clear(fb_clear); } -static void -convert_color(struct pipe_surface *psurf, union pipe_color_union *color) -{ - struct zink_resource *res = zink_resource(psurf->texture); - const struct util_format_description *desc = util_format_description(psurf->format); - union pipe_color_union tmp = *color; - - if (zink_format_is_emulated_alpha(psurf->format)) { - if (util_format_is_alpha(psurf->format)) { - tmp.ui[0] = tmp.ui[3]; - tmp.ui[1] = 0; - tmp.ui[2] = 0; - tmp.ui[3] = 0; - } else if (util_format_is_luminance(psurf->format)) { - tmp.ui[1] = 0; - tmp.ui[2] = 0; - tmp.f[3] = 1.0; - } else if (util_format_is_luminance_alpha(psurf->format)) { - tmp.ui[1] = tmp.ui[3]; - tmp.ui[2] = 0; - tmp.f[3] = 1.0; - } else /* zink_format_is_red_alpha */ { - tmp.ui[1] = tmp.ui[3]; - tmp.ui[2] = 0; - tmp.ui[3] = 0; - } - memcpy(color, &tmp, sizeof(union pipe_color_union)); - } - for (unsigned i = 0; i < 4; i++) - zink_format_clamp_channel_color(desc, color, &tmp, i); - - /* manually swizzle R -> A for true A8 */ - if (res->format == VK_FORMAT_A8_UNORM_KHR) { - color->ui[3] = color->ui[0]; - color->ui[0] = 0; - } -} - void zink_clear(struct pipe_context *pctx, unsigned buffers, @@ -191,6 +153,7 @@ zink_clear(struct pipe_context *pctx, double depth, unsigned stencil) { struct zink_context *ctx = zink_context(pctx); + struct zink_screen *screen = zink_screen(pctx->screen); struct pipe_framebuffer_state *fb = &ctx->fb_state; struct zink_batch *batch = &ctx->batch; bool needs_rp = false; @@ -295,7 +258,7 @@ zink_clear(struct pipe_context *pctx, clear->conditional = ctx->render_condition_active; clear->has_scissor = needs_rp; memcpy(&clear->color, pcolor, sizeof(union pipe_color_union)); - convert_color(psurf, &clear->color); + zink_convert_color(screen, psurf->format, &clear->color, pcolor); if (scissor_state && needs_rp) clear->scissor = *scissor_state; if (zink_fb_clear_first_needs_explicit(fb_clear)) @@ -472,6 +435,7 @@ zink_clear_texture_dynamic(struct pipe_context *pctx, const void *data) { struct zink_context *ctx = zink_context(pctx); + struct zink_screen *screen = zink_screen(pctx->screen); struct zink_resource *res = zink_resource(pres); bool full_clear = 0 <= box->x && u_minify(pres->width0, level) >= box->x + box->width && @@ -495,12 +459,12 @@ zink_clear_texture_dynamic(struct pipe_context *pctx, info.renderArea.extent.height = box->height; info.layerCount = MAX2(box->depth, 1); - union pipe_color_union color; + union pipe_color_union color, tmp; float depth = 0.0; uint8_t stencil = 0; if (res->aspect & VK_IMAGE_ASPECT_COLOR_BIT) { - util_format_unpack_rgba(pres->format, color.ui, data, 1); - convert_color(surf, &color); + util_format_unpack_rgba(pres->format, tmp.ui, data, 1); + zink_convert_color(screen, surf->format, &color, &tmp); } else { if (res->aspect & VK_IMAGE_ASPECT_DEPTH_BIT) util_format_unpack_z_float(pres->format, &depth, data, 1); diff --git a/src/gallium/drivers/zink/zink_context.c b/src/gallium/drivers/zink/zink_context.c index eef41b8af3e..ac80f5a805e 100644 --- a/src/gallium/drivers/zink/zink_context.c +++ b/src/gallium/drivers/zink/zink_context.c @@ -508,10 +508,11 @@ zink_create_sampler_state(struct pipe_context *pctx, } } else { cbci.format = zink_get_format(screen, state->border_color_format); + union pipe_color_union color; for (unsigned i = 0; i < 4; i++) { - zink_format_clamp_channel_color(util_format_description(state->border_color_format), (void*)&cbci.customBorderColor, &state->border_color, i); - zink_format_clamp_channel_srgb(util_format_description(state->border_color_format), (void*)&cbci.customBorderColor, (void*)&cbci.customBorderColor, i); + zink_format_clamp_channel_srgb(util_format_description(state->border_color_format), &color, &state->border_color, i); } + zink_convert_color(screen, state->border_color_format, (void*)&cbci.customBorderColor, &color); } } cbci.pNext = sci.pNext; diff --git a/src/gallium/drivers/zink/zink_format.c b/src/gallium/drivers/zink/zink_format.c index 81485c6372f..2cf83275b86 100644 --- a/src/gallium/drivers/zink/zink_format.c +++ b/src/gallium/drivers/zink/zink_format.c @@ -381,13 +381,13 @@ zink_format_is_voidable_rgba_variant(enum pipe_format format) return true; } - void zink_format_clamp_channel_color(const struct util_format_description *desc, union pipe_color_union *dst, const union pipe_color_union *src, unsigned i) { int non_void = util_format_get_first_non_void_channel(desc->format); - switch (desc->channel[i].type) { - case UTIL_FORMAT_TYPE_VOID: + unsigned channel = desc->swizzle[i]; + + if (channel > PIPE_SWIZZLE_W || desc->channel[channel].type == UTIL_FORMAT_TYPE_VOID) { if (non_void != -1) { if (desc->channel[non_void].type == UTIL_FORMAT_TYPE_FLOAT) { dst->f[i] = uif(UINT32_MAX); @@ -402,20 +402,26 @@ zink_format_clamp_channel_color(const struct util_format_description *desc, unio } else { dst->ui[i] = src->ui[i]; } + return; + } + + switch (desc->channel[channel].type) { + case UTIL_FORMAT_TYPE_VOID: + unreachable("handled above"); break; case UTIL_FORMAT_TYPE_SIGNED: - if (desc->channel[i].normalized) + if (desc->channel[channel].normalized) dst->i[i] = src->i[i]; else { - dst->i[i] = MAX2(src->i[i], -(1<<(desc->channel[i].size - 1))); - dst->i[i] = MIN2(dst->i[i], (1 << (desc->channel[i].size - 1)) - 1); + dst->i[i] = MAX2(src->i[i], -(1<<(desc->channel[channel].size - 1))); + dst->i[i] = MIN2(dst->i[i], (1 << (desc->channel[channel].size - 1)) - 1); } break; case UTIL_FORMAT_TYPE_UNSIGNED: - if (desc->channel[i].normalized) + if (desc->channel[channel].normalized) dst->ui[i] = src->ui[i]; else - dst->ui[i] = MIN2(src->ui[i], BITFIELD_MASK(desc->channel[i].size)); + dst->ui[i] = MIN2(src->ui[i], BITFIELD_MASK(desc->channel[channel].size)); break; case UTIL_FORMAT_TYPE_FIXED: case UTIL_FORMAT_TYPE_FLOAT: @@ -427,14 +433,18 @@ zink_format_clamp_channel_color(const struct util_format_description *desc, unio void zink_format_clamp_channel_srgb(const struct util_format_description *desc, union pipe_color_union *dst, const union pipe_color_union *src, unsigned i) { - if (desc->colorspace != UTIL_FORMAT_COLORSPACE_SRGB) - return; - switch (desc->channel[i].type) { - case UTIL_FORMAT_TYPE_SIGNED: - case UTIL_FORMAT_TYPE_UNSIGNED: - dst->f[i] = CLAMP(src->f[i], 0.0, 1.0); - break; - default: - break; + unsigned channel = desc->swizzle[i]; + if (desc->colorspace == UTIL_FORMAT_COLORSPACE_SRGB && + channel <= PIPE_SWIZZLE_W) { + switch (desc->channel[channel].type) { + case UTIL_FORMAT_TYPE_SIGNED: + case UTIL_FORMAT_TYPE_UNSIGNED: + dst->f[i] = CLAMP(src->f[i], 0.0, 1.0); + return; + default: + break; + } } + + dst->ui[i] = src->ui[i]; } diff --git a/src/gallium/drivers/zink/zink_screen.c b/src/gallium/drivers/zink/zink_screen.c index df66a2ae1b9..4c8b19a8eb7 100644 --- a/src/gallium/drivers/zink/zink_screen.c +++ b/src/gallium/drivers/zink/zink_screen.c @@ -1829,6 +1829,44 @@ zink_get_format(struct zink_screen *screen, enum pipe_format format) return ret; } +void +zink_convert_color(const struct zink_screen *screen, enum pipe_format format, + union pipe_color_union *dst, + const union pipe_color_union *src) +{ + const struct util_format_description *desc = util_format_description(format); + union pipe_color_union tmp = *src; + + for (unsigned i = 0; i < 4; i++) + zink_format_clamp_channel_color(desc, &tmp, src, i); + + if (zink_format_is_emulated_alpha(format) && + /* Don't swizzle colors if the driver supports real A8_UNORM */ + (format != PIPE_FORMAT_A8_UNORM || + screen->driver_workarounds.missing_a8_unorm)) { + if (util_format_is_alpha(format)) { + tmp.ui[0] = tmp.ui[3]; + tmp.ui[1] = 0; + tmp.ui[2] = 0; + tmp.ui[3] = 0; + } else if (util_format_is_luminance(format)) { + tmp.ui[1] = 0; + tmp.ui[2] = 0; + tmp.f[3] = 1.0; + } else if (util_format_is_luminance_alpha(format)) { + tmp.ui[1] = tmp.ui[3]; + tmp.ui[2] = 0; + tmp.f[3] = 1.0; + } else /* zink_format_is_red_alpha */ { + tmp.ui[1] = tmp.ui[3]; + tmp.ui[2] = 0; + tmp.ui[3] = 0; + } + } + + memcpy(dst, &tmp, sizeof(union pipe_color_union)); +} + static bool check_have_device_time(struct zink_screen *screen) { diff --git a/src/gallium/drivers/zink/zink_screen.h b/src/gallium/drivers/zink/zink_screen.h index e68291c124b..69baa8e3fd0 100644 --- a/src/gallium/drivers/zink/zink_screen.h +++ b/src/gallium/drivers/zink/zink_screen.h @@ -132,6 +132,11 @@ zink_screen_import_dmabuf_semaphore(struct zink_screen *screen, struct zink_reso VkFormat zink_get_format(struct zink_screen *screen, enum pipe_format format); +void +zink_convert_color(const struct zink_screen *screen, enum pipe_format format, + union pipe_color_union *dst, + const union pipe_color_union *src); + bool zink_screen_timeline_wait(struct zink_screen *screen, uint64_t batch_id, uint64_t timeout);