[Mesa-dev] [PATCH 2/3] meta: Properly save/restore GL_FRAMEBUFFER_SRGB in Meta.
Brian Paul
brianp at vmware.com
Tue Sep 25 12:03:22 PDT 2012
On 09/25/2012 12:59 PM, Paul Berry wrote:
> On 25 September 2012 06:53, Brian Paul <brianp at vmware.com
> <mailto:brianp at vmware.com>> wrote:
>
> 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 <mailto:brianp at vmware.com>>
>
>
> I have checked all the _mesa_meta_begin() calls, but if someone wants
> to verify that my reasoning is correct, I would appreciate it.
> Specifically, I reasoned that:
>
> - The calls to _mesa_meta_begin() in _mesa_meta_CopyPixels(),
> _mesa_meta_DrawPixels(), _mesa_meta_Bitmap(), and _mesa_meta_DrawTex()
> should *not* include MESA_META_FRAMEBUFFER_SRGB, because fragments
> produced by these calls are supposed to undergo normal fragment
> processing.
>
> - The call to _mesa_meta_begin() in _mesa_meta_GenerateMipmap()
> *should* include MESA_META_FRAMEBUFFER_SRGB, because sRGB encoding
> during mipmap generation would cause different miplevels to have
> different brightness.
>
> - The calls to _mesa_meta_begin() in _mesa_meta_CopyTexSubImage()
> don't need to include MESA_META_FRAMEBUFFER_SRGB, because the only
> operations that this function performs while meta is active are
> ReadPixels() and TexSubImage(), which won't be affected by the
> FRAMEBUFFER_SRGB setting.
>
> - The call to _mesa_meta_begin() in decompress_texture_image()
> *should* include MESA_META_FRAMEBUFFER_SRGB, since the behaviour of
> textures shouldn't depend on the setting of MESA_META_FRAMEBUFFER_SRGB.
>
> Do those sound right? I'm confident about the others.
Yeah, I agree with this.
-Brian
More information about the mesa-dev
mailing list