From eee8a53906f72635423931430e667159c88613bb Mon Sep 17 00:00:00 2001 From: Kenneth Graunke Date: Tue, 8 Mar 2016 00:34:14 -0800 Subject: [PATCH] meta: Make BlitFramebuffer() do sRGB encoding in ES 3.x. According to the ES 3.0 and GL 4.4 specifications, glBlitFramebuffer is supposed to perform sRGB decoding and encoding whenever sRGB formats are in use. The ES 3.0 specification is completely clear, and has always stated this. However, the GL specification has changed behavior in 4.1, 4.2, and 4.4. The original behavior stated that no sRGB encoding should occur. The 4.4 behavior matches ES 3.0's wording. However, implementing the new behavior appears to break applications such as Left 4 Dead 2. This patch changes Meta to apply the ES 3.x rules in ES 3.x, but leaves OpenGL alone for now, to avoid breaking applications. Meta implements several other functions in terms of BlitFramebuffer, and many of those explicitly do not perform sRGB encoding. So, this patch explicitly disables sRGB encoding in those other functions, preserving the existing (correct) behavior. If you're from the future and are reading this, hi! Welcome to the "fun" of debugging sRGB problems! Best of luck! Signed-off-by: Kenneth Graunke Reviewed-by: Jordan Justen --- src/mesa/drivers/common/meta_blit.c | 43 ++++++++++++++++----- src/mesa/drivers/common/meta_copy_image.c | 3 ++ src/mesa/drivers/common/meta_tex_subimage.c | 6 +++ 3 files changed, 43 insertions(+), 9 deletions(-) diff --git a/src/mesa/drivers/common/meta_blit.c b/src/mesa/drivers/common/meta_blit.c index 0066f7f9184..6761238b014 100644 --- a/src/mesa/drivers/common/meta_blit.c +++ b/src/mesa/drivers/common/meta_blit.c @@ -597,6 +597,7 @@ blitframebuffer_texture(struct gl_context *ctx, GLenum filter, GLint flipX, GLint flipY, GLboolean glsl_version, GLboolean do_depth) { + struct save_state *save = &ctx->Meta->Save[ctx->Meta->SaveStackDepth - 1]; int att_index = do_depth ? BUFFER_DEPTH : readFb->_ColorReadBufferIndex; const struct gl_renderbuffer_attachment *readAtt = &readFb->Attachment[att_index]; @@ -709,7 +710,7 @@ blitframebuffer_texture(struct gl_context *ctx, fb_tex_blit.samp_obj = _mesa_meta_setup_sampler(ctx, texObj, target, filter, srcLevel); - /* Always do our blits with no net sRGB decode or encode. + /* For desktop GL, we do our blits with no net sRGB decode or encode. * * However, if both the src and dst can be srgb decode/encoded, enable them * so that we do any blending (from scaling or from MSAA resolves) in the @@ -723,18 +724,42 @@ blitframebuffer_texture(struct gl_context *ctx, * scissor test." * * The GL 4.4 specification disagrees and says that the sRGB part of the - * fragment pipeline applies, but this was found to break applications. + * fragment pipeline applies, but this was found to break applications + * (such as Left 4 Dead 2). + * + * However, for ES 3.0, we follow the specification and perform sRGB + * decoding and encoding. The specification has always been clear in + * the ES world, and hasn't changed over time. */ if (ctx->Extensions.EXT_texture_sRGB_decode) { - if (_mesa_get_format_color_encoding(rb->Format) == GL_SRGB && - drawFb->Visual.sRGBCapable) { + bool src_srgb = _mesa_get_format_color_encoding(rb->Format) == GL_SRGB; + if (save->API == API_OPENGLES2 && ctx->Version >= 30) { + /* From the ES 3.0.4 specification, page 198: + * "When values are taken from the read buffer, if the value of + * FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING for the framebuffer + * attachment corresponding to the read buffer is SRGB (see section + * 6.1.13), the red, green, and blue components are converted from + * the non-linear sRGB color space according to equation 3.24. + * + * When values are written to the draw buffers, blit operations + * bypass the fragment pipeline. The only fragment operations which + * affect a blit are the pixel ownership test, the scissor test, + * and sRGB conversion (see section 4.1.8)." + */ _mesa_set_sampler_srgb_decode(ctx, fb_tex_blit.samp_obj, - GL_DECODE_EXT); - _mesa_set_framebuffer_srgb(ctx, GL_TRUE); + src_srgb ? GL_DECODE_EXT + : GL_SKIP_DECODE_EXT); + _mesa_set_framebuffer_srgb(ctx, drawFb->Visual.sRGBCapable); } else { - _mesa_set_sampler_srgb_decode(ctx, fb_tex_blit.samp_obj, - GL_SKIP_DECODE_EXT); - /* set_framebuffer_srgb was set by _mesa_meta_begin(). */ + if (src_srgb && drawFb->Visual.sRGBCapable) { + _mesa_set_sampler_srgb_decode(ctx, fb_tex_blit.samp_obj, + GL_DECODE_EXT); + _mesa_set_framebuffer_srgb(ctx, GL_TRUE); + } else { + _mesa_set_sampler_srgb_decode(ctx, fb_tex_blit.samp_obj, + GL_SKIP_DECODE_EXT); + /* set_framebuffer_srgb was set by _mesa_meta_begin(). */ + } } } diff --git a/src/mesa/drivers/common/meta_copy_image.c b/src/mesa/drivers/common/meta_copy_image.c index 18b9681b710..9402a4652eb 100644 --- a/src/mesa/drivers/common/meta_copy_image.c +++ b/src/mesa/drivers/common/meta_copy_image.c @@ -269,6 +269,9 @@ _mesa_meta_CopyImageSubData_uncompressed(struct gl_context *ctx, if (status != GL_FRAMEBUFFER_COMPLETE) goto meta_end; + /* Explicitly disable sRGB encoding */ + ctx->DrawBuffer->Visual.sRGBCapable = false; + /* Since we've bound a new draw framebuffer, we need to update its * derived state -- _Xmin, etc -- for BlitFramebuffer's clipping to * be correct. diff --git a/src/mesa/drivers/common/meta_tex_subimage.c b/src/mesa/drivers/common/meta_tex_subimage.c index dfd3327dd55..62c3fce4249 100644 --- a/src/mesa/drivers/common/meta_tex_subimage.c +++ b/src/mesa/drivers/common/meta_tex_subimage.c @@ -263,6 +263,9 @@ _mesa_meta_pbo_TexSubImage(struct gl_context *ctx, GLuint dims, if (status != GL_FRAMEBUFFER_COMPLETE) goto fail; + /* Explicitly disable sRGB encoding */ + ctx->DrawBuffer->Visual.sRGBCapable = false; + _mesa_update_state(ctx); if (_mesa_meta_BlitFramebuffer(ctx, ctx->ReadBuffer, ctx->DrawBuffer, @@ -420,6 +423,9 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx, GLuint dims, if (status != GL_FRAMEBUFFER_COMPLETE) goto fail; + /* Explicitly disable sRGB encoding */ + ctx->DrawBuffer->Visual.sRGBCapable = false; + _mesa_update_state(ctx); if (_mesa_meta_BlitFramebuffer(ctx, ctx->ReadBuffer, ctx->DrawBuffer, -- 2.30.2