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

2016-08-08 Thread 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 
> ---
>  src/mesa/drivers/common/meta_blit.c | 80 
> +
>  1 file changed, 28 insertions(+), 52 deletions(-)
> 
> Metaaa!!!
> 
> 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 sa

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

2016-08-04 Thread Kenneth Graunke
Just avoid whacking GL_FRAMEBUFFER_SRGB altogether, so we respect
the application's setting.  This appears to work.

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

Metaaa!!!

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 &&
+