Am 19.03.2014 10:44, schrieb Kenneth Graunke:
> Traditionally, we've implemented fragment color clamping by emitting
> "mov.sat" in the fragment shader's final color write code, clamping
> the output to [0, 1] prior to blending. (When clamping is off, we
> use a regular "mov".) This means we have to recompile based on the
> GL_CLAMP_FRAGMENT_COLOR state, and because GL_FIXED_ONLY is an option,
> we need to consider the render target format.
>
> Modern applications frequently use both UNORM buffers and FLOAT buffers
> with color clamping disabled. (FLOAT with clamping explicitly enabled
> and SNORM buffers appear to be less common.) With all UNORM buffers,
> ctx->Color._ClampFragmentColor is true, while it's false in the typical
> unclamped FLOAT case.
>
> In other words, the two most common cases disagree about whether we
> should emit saturates. This meant basically all modern applications
> needed fragment shader recompiles.
Are you sure? mesa does not seem to enable color clamps if there's all
unorm buffers.
void
_mesa_update_clamp_fragment_color(struct gl_context *ctx)
{
struct gl_framebuffer *fb = ctx->DrawBuffer;
/* Don't clamp if:
* - there is no colorbuffer
* - all colorbuffers are unsigned normalized, so clamping has no effect
* - there is an integer colorbuffer
*/
if (!fb || !fb->_HasSNormOrFloatColorBuffer || fb->_IntegerColor)
ctx->Color._ClampFragmentColor = GL_FALSE;
else
ctx->Color._ClampFragmentColor = _mesa_get_clamp_fragment_color(ctx);
}
>
> It turns out that the saturates are superfluous for UNORM buffers. We
> always configure BLEND_STATE to enable pre-blend color clamping, using
> the render target format's range. This means the color calculator will
> already clamp all UNORM buffers to [0, 1].
>
> By dropping the saturates for the UNORM case, key->clamp_fragment_color
> can simply be false in the two most common cases, giving us a clear
> value to guess in the precompile.
>
> Unfortunately, it doesn't look like we can rely on the color calculator
> to implement fragment color clamping for non-UNORM formats:
>
> For SNORM, the color calculator clamps to [-1, 1] while OpenGL requires
> [0, 1] for some reason. (Technically, the ARB_color_buffer_float spec
> does specify [-1, 1], but every other specification contradicts that.)
> Requesting BRW_RENDERTARGET_CLAMPRANGE_UNORM is not allowed.
>
> Although we are allowed to request [0, 1] pre-blend clamping for FLOAT
> formats, we are also required to enable post-blend clamping, and it uses
> the same range. I believe this is incorrect for OpenGL rules---at the
> very least, no values appear to satisfy Piglit's
> arb_color_buffer_float-render test.
I agree color clamping is a mess. It doesn't help that GL has TWO clamps
- one for shader outputs (this is what will clamp to 0/1 apparently even
for signed normalized buffers when enabled), plus the always active
pre-blend clamp (this one will more meaningfully clamp to -1/1 for
signed normalized, though only mentioned correctly in 4.2+ - apparently
this is what your hw will do). At least the former clamp is legacy only...
I'm not really convinced the _ClampFragmentColor bit is actually fully
correct neither, I think the evaluation if clamping is needed or not
would need to be done per color buffer.
Roland
>
> Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
> ---
> src/mesa/drivers/dri/i965/brw_fs.cpp | 2 --
> src/mesa/drivers/dri/i965/brw_wm.c | 3 ++-
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 1b32d63..bd44952 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -3616,8 +3616,6 @@ brw_fs_precompile(struct gl_context *ctx, struct
> gl_shader_program *prog)
> BRW_FS_VARYING_INPUT_MASK) > 16)
> key.input_slots_valid = fp->Base.InputsRead | VARYING_BIT_POS;
>
> - key.clamp_fragment_color = ctx->API == API_OPENGL_COMPAT;
> -
> unsigned sampler_count = _mesa_fls(fp->Base.SamplersUsed);
> for (unsigned i = 0; i < sampler_count; i++) {
> if (fp->Base.ShadowSamplers & (1 << i)) {
> diff --git a/src/mesa/drivers/dri/i965/brw_wm.c
> b/src/mesa/drivers/dri/i965/brw_wm.c
> index 0d0d6ec..baf001f 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm.c
> @@ -488,7 +488,8 @@ static void brw_wm_populate_key( struct brw_context *brw,
> key->flat_shade = (ctx->Light.ShadeModel == GL_FLAT);
>
> /* _NEW_FRAG_CLAMP | _NEW_BUFFERS */
> - key->clamp_fragment_color = ctx->Color._ClampFragmentColor;
> + key->clamp_fragment_color = ctx->Color._ClampFragmentColor &&
> + ctx->DrawBuffer->_HasSNormOrFloatColorBuffer;
>
> /* _NEW_TEXTURE */
> brw_populate_sampler_prog_key_data(ctx, prog, brw->wm.base.sampler_count,
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
