On 25 September 2012 06:53, Brian Paul <bri...@vmware.com
<mailto:bri...@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 <bri...@vmware.com <mailto:bri...@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.
