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

Submitted by Kenneth Graunke on Aug. 4, 2016, 11:14 p.m.

Details

Message ID 20160804231451.9994-10-kenneth@whitecape.org
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Piglit

Not browsing as part of any series.

Commit Message

Kenneth Graunke Aug. 4, 2016, 11:14 p.m.
Just avoid whacking GL_FRAMEBUFFER_SRGB altogether, so we respect
the application's setting.  This appears to work.

Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
---
 src/mesa/drivers/common/meta_blit.c | 80 +++++++++++++------------------------
 1 file changed, 28 insertions(+), 52 deletions(-)

Metaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!!!

Patch hide | download patch | download mbox

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.
     */
-   _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);

Comments

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@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);
>