nir/lower_blend: fix snorm factor clamping
The spec says (emphasis mine): If the color attachment is fixed-point, the components of the source and destination values **AND BLEND FACTORS** are each clamped to [0,1] or [-1,1] respectively for an unsigned normalized or signed normalized color attachment prior to evaluating the blend operations. If the color attachment is floating-point, no clamping occurs. However, neither the CTS nor any hardware implement this semantic. For unsigned normalized formats, the definitions are roughly equivalent (except perhaps around constant colours). 0 <= x <= 1 implies that 0 <= 1 - x <= 1. Therefore if the source/destination colours are clamped to [0, 1], then their complements are also in [0, 1], so clamping any blend factor (except constant colour) has no effect if the source/dest were already clamped. For signed normalized formats, however, this difference matters. -1 <= x <= 1 implies that 0 <= 1 - x <= 2... so to implement the spec text faithfully, we would need to clamp again the complemented colour blend factors to return back to signed normalized range. Software blending implementations can of course do that... but doing so causes CTS fails, as the CTS reference renderer does not do this. This commit adjusts nir_lower_blend to match what actual hardware does, what CTS requires, and what the spec should have said. See https://gitlab.khronos.org/vulkan/vulkan/-/issues/4293 for the spec resolution. Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io> Reviewed-by: Adam Jackson <ajax@redhat.com> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/35519>
This commit is contained in:
Alyssa Rosenzweig
committed by
Marge Bot
Marge Bot
parent
09d6427c13
commit
d37bf148d2
@@ -150,47 +150,6 @@ nir_fsat_to_format(nir_builder *b, nir_def *x, enum pipe_format format)
|
||||
return x;
|
||||
}
|
||||
|
||||
/*
|
||||
* The spec says we need to clamp blend factors. However, we don't want to clamp
|
||||
* unnecessarily, as the clamp might not be optimized out. Check whether
|
||||
* clamping a blend factor is needed.
|
||||
*/
|
||||
static bool
|
||||
should_clamp_factor(enum pipe_blendfactor factor, bool snorm)
|
||||
{
|
||||
switch (util_blendfactor_without_invert(factor)) {
|
||||
case PIPE_BLENDFACTOR_ONE:
|
||||
/* 0, 1 are in [0, 1] and [-1, 1] */
|
||||
return false;
|
||||
|
||||
case PIPE_BLENDFACTOR_SRC_COLOR:
|
||||
case PIPE_BLENDFACTOR_SRC1_COLOR:
|
||||
case PIPE_BLENDFACTOR_DST_COLOR:
|
||||
case PIPE_BLENDFACTOR_SRC_ALPHA:
|
||||
case PIPE_BLENDFACTOR_SRC1_ALPHA:
|
||||
case PIPE_BLENDFACTOR_DST_ALPHA:
|
||||
/* Colours are already clamped. For unorm, the complement of something
|
||||
* clamped is still clamped. But for snorm, this is not true. Clamp for
|
||||
* snorm only.
|
||||
*/
|
||||
return util_blendfactor_is_inverted(factor) && snorm;
|
||||
|
||||
case PIPE_BLENDFACTOR_CONST_COLOR:
|
||||
case PIPE_BLENDFACTOR_CONST_ALPHA:
|
||||
/* Constant colours are not yet clamped */
|
||||
return true;
|
||||
|
||||
case PIPE_BLENDFACTOR_SRC_ALPHA_SATURATE:
|
||||
/* For unorm, this is in bounds (and hence so is its complement). For
|
||||
* snorm, it may not be.
|
||||
*/
|
||||
return snorm;
|
||||
|
||||
default:
|
||||
unreachable("invalid blend factor");
|
||||
}
|
||||
}
|
||||
|
||||
static bool
|
||||
channel_uses_dest(nir_lower_blend_channel chan)
|
||||
{
|
||||
@@ -229,9 +188,6 @@ nir_blend_factor(
|
||||
if (util_blendfactor_is_inverted(factor))
|
||||
f = nir_fadd_imm(b, nir_fneg(b, f), 1.0);
|
||||
|
||||
if (should_clamp_factor(factor, util_format_is_snorm(format)))
|
||||
f = nir_fsat_to_format(b, f, format);
|
||||
|
||||
return nir_fmul(b, raw_scalar, f);
|
||||
}
|
||||
|
||||
@@ -407,20 +363,12 @@ nir_blend(
|
||||
/* Fixed-point framebuffers require their inputs clamped. */
|
||||
enum pipe_format format = options->format[rt];
|
||||
|
||||
/* From section 17.3.6 "Blending" of the OpenGL 4.5 spec:
|
||||
*
|
||||
* If the color buffer is fixed-point, the components of the source and
|
||||
* destination values and blend factors are each clamped to [0, 1] or
|
||||
* [-1, 1] respectively for an unsigned normalized or signed normalized
|
||||
* color buffer prior to evaluating the blend equation. If the color
|
||||
* buffer is floating-point, no clamping occurs.
|
||||
*
|
||||
* Blend factors are clamped at the time of their use to ensure we properly
|
||||
* clamp negative constant colours with signed normalized formats and
|
||||
* ONE_MINUS_CONSTANT_* factors. Notice that -1 is in [-1, 1] but 1 - (-1) =
|
||||
* 2 is not in [-1, 1] and should be clamped to 1.
|
||||
/* The input colours need to be clamped to the format. Contrary to the
|
||||
* OpenGL/Vulkan specs, it really is the inputs that get clamped and not the
|
||||
* intermediate blend factors. This matches the CTS and hardware behaviour.
|
||||
*/
|
||||
src = nir_fsat_to_format(b, src, format);
|
||||
bconst = nir_fsat_to_format(b, bconst, format);
|
||||
|
||||
if (src1)
|
||||
src1 = nir_fsat_to_format(b, src1, format);
|
||||
|
||||
@@ -97,7 +97,6 @@ spec@ext_framebuffer_object@fbo-blending-formats@GL_LUMINANCE8_ALPHA8,Fail
|
||||
spec@ext_framebuffer_object@fbo-blending-formats@GL_LUMINANCE8,Fail
|
||||
spec@ext_framebuffer_object@fbo-blending-formats@GL_LUMINANCE_ALPHA,Fail
|
||||
spec@ext_framebuffer_object@fbo-blending-formats@GL_LUMINANCE,Fail
|
||||
spec@ext_framebuffer_object@fbo-blending-snorm,Fail
|
||||
spec@ext_framebuffer_object@getteximage-formats init-by-clear-and-render,Fail
|
||||
spec@ext_framebuffer_object@getteximage-formats init-by-rendering,Fail
|
||||
spec@ext_transform_feedback2@draw-auto,Fail
|
||||
|
||||
@@ -87,7 +87,6 @@ spec@ext_framebuffer_object@fbo-blending-formats@GL_INTENSITY,Fail
|
||||
spec@ext_framebuffer_object@fbo-blending-formats@GL_LUMINANCE4,Fail
|
||||
spec@ext_framebuffer_object@fbo-blending-formats@GL_LUMINANCE8,Fail
|
||||
spec@ext_framebuffer_object@fbo-blending-formats@GL_LUMINANCE,Fail
|
||||
spec@ext_framebuffer_object@fbo-blending-snorm,Fail
|
||||
spec@ext_framebuffer_object@getteximage-formats init-by-clear-and-render,Fail
|
||||
spec@ext_framebuffer_object@getteximage-formats init-by-rendering,Fail
|
||||
spec@ext_transform_feedback2@draw-auto,Fail
|
||||
@@ -416,7 +415,6 @@ afbcp-spec@ext_framebuffer_object@fbo-blending-formats@GL_INTENSITY,Fail
|
||||
afbcp-spec@ext_framebuffer_object@fbo-blending-formats@GL_LUMINANCE4,Fail
|
||||
afbcp-spec@ext_framebuffer_object@fbo-blending-formats@GL_LUMINANCE8,Fail
|
||||
afbcp-spec@ext_framebuffer_object@fbo-blending-formats@GL_LUMINANCE,Fail
|
||||
afbcp-spec@ext_framebuffer_object@fbo-blending-snorm,Fail
|
||||
afbcp-spec@ext_framebuffer_object@getteximage-formats init-by-clear-and-render,Fail
|
||||
afbcp-spec@ext_framebuffer_object@getteximage-formats init-by-rendering,Fail
|
||||
afbcp-spec@ext_transform_feedback2@draw-auto,Fail
|
||||
|
||||
Reference in New Issue
Block a user