[Mesa-dev] [PATCH 2/3] meta: Properly save/restore GL_FRAMEBUFFER_SRGB in Meta.

Brian Paul brianp at vmware.com
Tue Sep 25 06:53:04 PDT 2012


On 09/24/2012 05:49 PM, Paul Berry wrote:
> Previously, meta logic was saving and restoring the value of
> GL_FRAMEBUFFER_SRGB in an ad-hoc fashion.  As a result, it was not
> properly disabled and/or restored for some meta operations.
>
> This patch causes GL_FRAMEBUFFER_SRGB to be saved/restored in the
> conventional way of meta-ops (using _mesa_meta_begin() and
> _mesa_meta_end()).  It is now reliably saved/restored for
> _mesa_meta_BlitFramebuffer, _mesa_meta_GenerateMipmap, and
> decompress_texture_image, and preserved for all other meta ops.
>
> Fixes piglit tests "ARB_framebuffer_sRGB/blit renderbuffer
> {linear_to_srgb,srgb} scaled {disabled,enabled}".
> ---
>   src/mesa/drivers/common/meta.c | 47 ++++++++++++++++++------------------------
>   src/mesa/drivers/common/meta.h |  1 +
>   2 files changed, 21 insertions(+), 27 deletions(-)
>
> diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
> index 28a79b0..6689337 100644
> --- a/src/mesa/drivers/common/meta.c
> +++ b/src/mesa/drivers/common/meta.c
> @@ -187,6 +187,9 @@ struct save_state
>      /** MESA_META_MULTISAMPLE */
>      GLboolean MultisampleEnabled;
>
> +   /** MESA_META_FRAMEBUFFER_SRGB */
> +   GLboolean sRGBEnabled;
> +
>      /** Miscellaneous (always disabled) */
>      GLboolean Lighting;
>      GLboolean RasterDiscard;
> @@ -773,6 +776,12 @@ _mesa_meta_begin(struct gl_context *ctx, GLbitfield state)
>            _mesa_set_multisample(ctx, GL_FALSE);
>      }
>
> +   if (state&  MESA_META_FRAMEBUFFER_SRGB) {
> +      save->sRGBEnabled = ctx->Color.sRGBEnabled;
> +      if (ctx->Color.sRGBEnabled)
> +         _mesa_set_framebuffer_srgb(ctx, GL_FALSE);
> +   }
> +
>      /* misc */
>      {
>         save->Lighting = ctx->Light.Enabled;
> @@ -1075,6 +1084,11 @@ _mesa_meta_end(struct gl_context *ctx)
>            _mesa_set_multisample(ctx, save->MultisampleEnabled);
>      }
>
> +   if (state&  MESA_META_FRAMEBUFFER_SRGB) {
> +      if (ctx->Color.sRGBEnabled != save->sRGBEnabled)
> +         _mesa_set_framebuffer_srgb(ctx, save->sRGBEnabled);
> +   }
> +
>      /* misc */
>      if (save->Lighting) {
>         _mesa_set_enable(ctx, GL_LIGHTING, GL_TRUE);
> @@ -1394,7 +1408,6 @@ blitframebuffer_texture(struct gl_context *ctx,
>            const GLuint srcLevel = readAtt->TextureLevel;
>            const GLint baseLevelSave = texObj->BaseLevel;
>            const GLint maxLevelSave = texObj->MaxLevel;
> -         const GLenum fbo_srgb_save = ctx->Color.sRGBEnabled;
>            const GLenum target = texObj->Target;
>            GLuint sampler, samplerSave =
>               ctx->Texture.Unit[ctx->Texture.CurrentUnit].Sampler ?
> @@ -1433,15 +1446,14 @@ blitframebuffer_texture(struct gl_context *ctx,
>            _mesa_SamplerParameteri(sampler, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
>            _mesa_SamplerParameteri(sampler, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
>
> -	 /* Always do our blits with no sRGB decode or encode.*/
> +	 /* Always do our blits with no sRGB decode or encode.  Note that
> +          * GL_FRAMEBUFFER_SRGB has already been disabled by
> +          * _mesa_meta_begin().
> +          */
>   	 if (ctx->Extensions.EXT_texture_sRGB_decode) {
>   	    _mesa_SamplerParameteri(sampler, GL_TEXTURE_SRGB_DECODE_EXT,
>   				GL_SKIP_DECODE_EXT);
>   	 }
> -         if ((_mesa_is_desktop_gl(ctx)&&  ctx->Extensions.EXT_framebuffer_sRGB)
> -             || _mesa_is_gles3(ctx)) {
> -            _mesa_set_enable(ctx, GL_FRAMEBUFFER_SRGB_EXT, GL_FALSE);
> -         }
>
>            _mesa_TexEnvi(GL_TEXTURE_ENV, GL_TEXTURE_ENV_MODE, GL_REPLACE);
>            _mesa_set_enable(ctx, target, GL_TRUE);
> @@ -1500,9 +1512,6 @@ blitframebuffer_texture(struct gl_context *ctx,
>               _mesa_TexParameteri(target, GL_TEXTURE_BASE_LEVEL, baseLevelSave);
>               _mesa_TexParameteri(target, GL_TEXTURE_MAX_LEVEL, maxLevelSave);
>            }
> -	 if (ctx->Extensions.EXT_framebuffer_sRGB&&  fbo_srgb_save) {
> -	    _mesa_set_enable(ctx, GL_FRAMEBUFFER_SRGB_EXT, GL_TRUE);
> -	 }
>
>            _mesa_BindSampler(ctx->Texture.CurrentUnit, samplerSave);
>            _mesa_DeleteSamplers(1,&sampler);
> @@ -1713,7 +1722,8 @@ _mesa_meta_Clear(struct gl_context *ctx, GLbitfield buffers)
>      GLbitfield metaSave = (MESA_META_ALL -
>   			  MESA_META_SCISSOR -
>   			  MESA_META_PIXEL_STORE -
> -			  MESA_META_CONDITIONAL_RENDER);
> +			  MESA_META_CONDITIONAL_RENDER -
> +                          MESA_META_FRAMEBUFFER_SRGB);
>      const GLuint stencilMax = (1<<  ctx->DrawBuffer->Visual.stencilBits) - 1;
>
>      if (buffers&  BUFFER_BITS_COLOR) {
> @@ -3236,7 +3246,6 @@ _mesa_meta_GenerateMipmap(struct gl_context *ctx, GLenum target,
>      const GLuint maxLevel = texObj->MaxLevel;
>      const GLint maxLevelSave = texObj->MaxLevel;
>      const GLboolean genMipmapSave = texObj->GenerateMipmap;
> -   const GLenum srgbBufferSave = ctx->Color.sRGBEnabled;
>      const GLuint fboSave = ctx->DrawBuffer->Name;
>      const GLuint currentTexUnitSave = ctx->Texture.CurrentUnit;
>      const GLboolean use_glsl_version = ctx->Extensions.ARB_vertex_shader&&
> @@ -3330,12 +3339,6 @@ _mesa_meta_GenerateMipmap(struct gl_context *ctx, GLenum target,
>      else
>         assert(!genMipmapSave);
>
> -   if ((ctx->Extensions.EXT_framebuffer_sRGB&&
> -        _mesa_is_desktop_gl(ctx)) ||
> -       _mesa_is_gles3(ctx)) {
> -      _mesa_set_enable(ctx, GL_FRAMEBUFFER_SRGB_EXT, GL_FALSE);
> -   }
> -
>     /* Setup texture coordinates */
>      setup_texture_coords(faceTarget,
>                           slice,
> @@ -3452,10 +3455,6 @@ _mesa_meta_GenerateMipmap(struct gl_context *ctx, GLenum target,
>         _mesa_DrawArrays(GL_TRIANGLE_FAN, 0, 4);
>      }
>
> -   if (ctx->Extensions.EXT_framebuffer_sRGB&&  srgbBufferSave) {
> -      _mesa_set_enable(ctx, GL_FRAMEBUFFER_SRGB_EXT, GL_TRUE);
> -   }
> -
>      _mesa_lock_texture(ctx, texObj); /* relock */
>
>      _mesa_BindSampler(ctx->Texture.CurrentUnit, samplerSave);
> @@ -3734,12 +3733,6 @@ decompress_texture_image(struct gl_context *ctx,
>            _mesa_TexParameteri(target, GL_TEXTURE_MAX_LEVEL, texImage->Level);
>         }
>
> -      /* No sRGB decode or encode.*/
> -      if ((_mesa_is_desktop_gl(ctx)&&  ctx->Extensions.EXT_framebuffer_sRGB)
> -          || _mesa_is_gles3(ctx)) {
> -         _mesa_set_enable(ctx, GL_FRAMEBUFFER_SRGB_EXT, GL_FALSE);
> -      }
> -
>         /* render quad w/ texture into renderbuffer */
>         _mesa_DrawArrays(GL_TRIANGLE_FAN, 0, 4);
>
> diff --git a/src/mesa/drivers/common/meta.h b/src/mesa/drivers/common/meta.h
> index d8dfb56..6ffc5b5 100644
> --- a/src/mesa/drivers/common/meta.h
> +++ b/src/mesa/drivers/common/meta.h
> @@ -56,6 +56,7 @@
>   #define MESA_META_CLIP                  0x40000
>   #define MESA_META_SELECT_FEEDBACK       0x80000
>   #define MESA_META_MULTISAMPLE          0x100000
> +#define MESA_META_FRAMEBUFFER_SRGB     0x200000
>   /**\}*/
>
>   extern void

Assuming you've checked all the _mesa_meta_begin() calls to check if 
the new MESA_META_FRAMEBUFFER_SRGB flag is needed,

Reviewed-by: Brian Paul <brianp at vmware.com>



More information about the mesa-dev mailing list