[Mesa-dev] [PATCH] meta: Make BlitFramebuffer() do sRGB encoding in ES 3.x.

Jordan Justen jordan.l.justen at intel.com
Fri Mar 18 15:40:35 UTC 2016


Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

On 2016-03-15 00:49:47, Kenneth Graunke wrote:
> 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.
> 
> Fixes scores of dEQP-GLES3.functional.fbo.blit.conversion.*srgb* tests.
> 
> 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 <kenneth at whitecape.org>
> ---
>  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 5d80f7d..58205e7 100644
> --- a/src/mesa/drivers/common/meta_blit.c
> +++ b/src/mesa/drivers/common/meta_blit.c
> @@ -594,6 +594,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];
> @@ -706,7 +707,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
> @@ -720,18 +721,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 18b9681..9402a46 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 dfd3327..62c3fce 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.7.3
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list