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. 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. 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, -- 1.9.0 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev