[Mesa-dev] [PATCH - Mesa 09/10] meta: Make Meta's BlitFramebuffer() follow the GL 4.4 sRGB rules.

[Ian Romanick]

On 08/04/2016 04:14 PM, Kenneth Graunke wrote:
> Just avoid whacking GL_FRAMEBUFFER_SRGB altogether, so we respect
> the application's setting.  This appears to work.
> 
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/common/meta_blit.c | 80 +++++++++++++------------------------
>  1 file changed, 28 insertions(+), 52 deletions(-)
> 
> Metaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!!!
> 
> diff --git a/src/mesa/drivers/common/meta_blit.c b/src/mesa/drivers/common/meta_blit.c
> index d6d3a42..8a79725 100644
> --- a/src/mesa/drivers/common/meta_blit.c
> +++ b/src/mesa/drivers/common/meta_blit.c
> @@ -606,7 +606,6 @@ 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];
> @@ -719,57 +718,32 @@ blitframebuffer_texture(struct gl_context *ctx,
>     fb_tex_blit.samp_obj = _mesa_meta_setup_sampler(ctx, texObj, target, filter,
>                                                     srcLevel);
>  
> -   /* 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
> -    * right colorspace.
> -    *
> -    * Our choice of not doing any net encode/decode is from the GL 3.0
> -    * specification:
> -    *
> -    *     "Blit operations bypass the fragment pipeline. The only fragment
> -    *      operations which affect a blit are the pixel ownership test and the
> -    *      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
> -    * (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) {
> -      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,
> -                                       src_srgb ? GL_DECODE_EXT
> -                                                : GL_SKIP_DECODE_EXT);
> -         _mesa_set_framebuffer_srgb(ctx, drawFb->Visual.sRGBCapable);
> -      } else {
> -         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(). */
> -         }
> -      }
> +      /* The GL 4.4 spec, section 18.3.1 ("Blitting Pixel Rectangles") says:
> +       *
> +       *    "When values are taken from the read buffer, if FRAMEBUFFER_SRGB
> +       *     is enabled and the value of FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING
> +       *     for the framebuffer attachment corresponding to the read buffer
> +       *     is SRGB (see section 9.2.3), 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 most of the fragment pipeline.  The only fragment
> +       *     operations which affect a blit are the pixel ownership test,
> +       *     the scissor test, and sRGB conversion (see section 17.3.9)."
> +       *
> +       * ES 3.0 contains nearly the exact same text, but omits the part
> +       * about GL_FRAMEBUFFER_SRGB as that doesn't exist in ES.  Mesa
> +       * defaults it to on for ES contexts, so we can safely check it.
> +       */
> +      const bool decode =
> +         ctx->Color.sRGBEnabled &&
> +         _mesa_get_format_color_encoding(rb->Format) == GL_SRGB;
> +
> +      _mesa_set_sampler_srgb_decode(ctx, fb_tex_blit.samp_obj,
> +                                    decode ? GL_DECODE_EXT
> +                                           : GL_SKIP_DECODE_EXT);
>     }
>  
>     if (!glsl_version) {
> @@ -1015,7 +989,9 @@ _mesa_meta_BlitFramebuffer(struct gl_context *ctx,
>     /* Only scissor affects blit, but we're doing to set a custom scissor if
>      * necessary anyway, so save/clear state.
>      */

Update the comment too... and s/doing to set/going to set/.

> -   _mesa_meta_begin(ctx, MESA_META_ALL & ~MESA_META_DRAW_BUFFERS);
> +   _mesa_meta_begin(ctx, MESA_META_ALL &
> +                         ~(MESA_META_DRAW_BUFFERS |
> +                           MESA_META_FRAMEBUFFER_SRGB));
>  
>     /* Dithering shouldn't be performed for glBlitFramebuffer */
>     _mesa_set_enable(ctx, GL_DITHER, GL_FALSE);
>