[Mesa-dev] [PATCH v2 07/21] mesa/samplerobj: Support EXT_texture_sRGB_decode

[Kenneth Graunke]

On 06/12/2012 11:38 AM, Pauli Nieminen wrote:
> sRGBDecode state is part of sampler object state but mesa was missing
> handlers to access the state. This patch adds the support for required
> state changes and queries.
> 
> GL_EXT_texture_sRGB_decode issue 4:
> "4) Should we add forward-looking support for ARB_sampler_objects?
> 
>         RESOLVED: YES
> 
>         If ARB_sampler_objects exists in the implementation, the sampler
>         objects should also include this parameter per sampler."
> 
> Signed-off-by: Pauli Nieminen <pauli.nieminen at linux.intel.com>
> Reviewed-by: Brian Paul <brianp at vmware.com>
> ---
>  src/mesa/main/samplerobj.c |   56 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/src/mesa/main/samplerobj.c b/src/mesa/main/samplerobj.c
> index 8c54c9a..f276296 100644
> --- a/src/mesa/main/samplerobj.c
> +++ b/src/mesa/main/samplerobj.c
> @@ -582,6 +582,24 @@ set_sampler_cube_map_seamless(struct gl_context *ctx,
>     return GL_TRUE;
>  }
>  
> +static GLuint
> +set_sampler_srgb_decode(struct gl_context *ctx,
> +                              struct gl_sampler_object *samp, GLenum param)
> +{
> +   if (!ctx->Extensions.EXT_texture_sRGB_decode)
> +      return INVALID_PNAME;
> +
> +   if (samp->sRGBDecode == param)
> +      return GL_FALSE;
> +
> +   if (param != GL_DECODE_EXT && param != GL_SKIP_DECODE_EXT)
> +      return INVALID_VALUE;

The correct return value for glSamplerParameter when passed an invalid
pname is GL_INVALID_ENUM, not GL_INVALID_VALUE.  From the GL 4.0 core
spec, page 177: "An INVALID_ENUM error is generated if pname is not the
name of a parameter accepted by SamplerParameter*."

To make that happen, you should return INVALID_PARAM here, and the code
at the bottom of _mesa_SamplerParameter* will make translate it.

> +   flush(ctx);
> +   samp->sRGBDecode = param;
> +   return GL_TRUE;
> +}
> +
>  
>  static void GLAPIENTRY
>  _mesa_SamplerParameteri(GLuint sampler, GLenum pname, GLint param)
> @@ -634,6 +652,9 @@ _mesa_SamplerParameteri(GLuint sampler, GLenum pname, GLint param)
>     case GL_TEXTURE_CUBE_MAP_SEAMLESS:
>        res = set_sampler_cube_map_seamless(ctx, sampObj, param);
>        break;
> +   case GL_TEXTURE_SRGB_DECODE_EXT:
> +      res = set_sampler_srgb_decode(ctx, sampObj, param);
> +      break;
>     case GL_TEXTURE_BORDER_COLOR:
>        /* fall-through */
>     default:
> @@ -718,6 +739,9 @@ _mesa_SamplerParameterf(GLuint sampler, GLenum pname, GLfloat param)
>     case GL_TEXTURE_CUBE_MAP_SEAMLESS:
>        res = set_sampler_cube_map_seamless(ctx, sampObj, (GLboolean) param);
>        break;
> +   case GL_TEXTURE_SRGB_DECODE_EXT:
> +      res = set_sampler_srgb_decode(ctx, sampObj, (GLenum) param);
> +      break;
>     case GL_TEXTURE_BORDER_COLOR:
>        /* fall-through */
>     default:
> @@ -799,6 +823,9 @@ _mesa_SamplerParameteriv(GLuint sampler, GLenum pname, const GLint *params)
>     case GL_TEXTURE_CUBE_MAP_SEAMLESS:
>        res = set_sampler_cube_map_seamless(ctx, sampObj, params[0]);
>        break;
> +   case GL_TEXTURE_SRGB_DECODE_EXT:
> +      res = set_sampler_srgb_decode(ctx, sampObj, params[0]);
> +      break;
>     case GL_TEXTURE_BORDER_COLOR:
>        {
>           GLfloat c[4];
> @@ -890,6 +917,9 @@ _mesa_SamplerParameterfv(GLuint sampler, GLenum pname, const GLfloat *params)
>     case GL_TEXTURE_CUBE_MAP_SEAMLESS:
>        res = set_sampler_cube_map_seamless(ctx, sampObj, (GLboolean) params[0]);
>        break;
> +   case GL_TEXTURE_SRGB_DECODE_EXT:
> +      res = set_sampler_srgb_decode(ctx, sampObj, (GLenum) params[0]);
> +      break;
>     case GL_TEXTURE_BORDER_COLOR:
>        res = set_sampler_border_colorf(ctx, sampObj, params);
>        break;
> @@ -972,6 +1002,9 @@ _mesa_SamplerParameterIiv(GLuint sampler, GLenum pname, const GLint *params)
>     case GL_TEXTURE_CUBE_MAP_SEAMLESS:
>        res = set_sampler_cube_map_seamless(ctx, sampObj, params[0]);
>        break;
> +   case GL_TEXTURE_SRGB_DECODE_EXT:
> +      res = set_sampler_srgb_decode(ctx, sampObj, (GLenum) params[0]);
> +      break;
>     case GL_TEXTURE_BORDER_COLOR:
>        res = set_sampler_border_colori(ctx, sampObj, params);
>        break;
> @@ -1055,6 +1088,9 @@ _mesa_SamplerParameterIuiv(GLuint sampler, GLenum pname, const GLuint *params)
>     case GL_TEXTURE_CUBE_MAP_SEAMLESS:
>        res = set_sampler_cube_map_seamless(ctx, sampObj, params[0]);
>        break;
> +   case GL_TEXTURE_SRGB_DECODE_EXT:
> +      res = set_sampler_srgb_decode(ctx, sampObj, (GLenum) params[0]);
> +      break;
>     case GL_TEXTURE_BORDER_COLOR:
>        res = set_sampler_border_colorui(ctx, sampObj, params);
>        break;
> @@ -1149,6 +1185,11 @@ _mesa_GetSamplerParameteriv(GLuint sampler, GLenum pname, GLint *params)
>           goto invalid_pname;
>        *params = sampObj->CubeMapSeamless;
>        break;
> +   case GL_TEXTURE_SRGB_DECODE_EXT:
> +      if (!ctx->Extensions.EXT_texture_sRGB_decode)
> +         goto invalid_pname;
> +      *params = (GLenum) sampObj->sRGBDecode;

You can drop the cast here; sampObj->sRGBDecode is already a GLenum.

> +      break;
>     default:
>        goto invalid_pname;
>     }
> @@ -1222,6 +1263,11 @@ _mesa_GetSamplerParameterfv(GLuint sampler, GLenum pname, GLfloat *params)
>           goto invalid_pname;
>        *params = (GLfloat) sampObj->CubeMapSeamless;
>        break;
> +   case GL_TEXTURE_SRGB_DECODE_EXT:
> +      if (!ctx->Extensions.EXT_texture_sRGB_decode)
> +         goto invalid_pname;
> +      *params = (GLfloat) sampObj->sRGBDecode;
> +      break;
>     default:
>        goto invalid_pname;
>     }
> @@ -1296,6 +1342,11 @@ _mesa_GetSamplerParameterIiv(GLuint sampler, GLenum pname, GLint *params)
>           goto invalid_pname;
>        *params = sampObj->CubeMapSeamless;
>        break;
> +   case GL_TEXTURE_SRGB_DECODE_EXT:
> +      if (!ctx->Extensions.EXT_texture_sRGB_decode)
> +         goto invalid_pname;
> +      *params = (GLenum) sampObj->sRGBDecode;

Ditto.  No need for the cast.

> +      break;
>     default:
>        goto invalid_pname;
>     }
> @@ -1370,6 +1421,11 @@ _mesa_GetSamplerParameterIuiv(GLuint sampler, GLenum pname, GLuint *params)
>           goto invalid_pname;
>        *params = sampObj->CubeMapSeamless;
>        break;
> +   case GL_TEXTURE_SRGB_DECODE_EXT:
> +      if (!ctx->Extensions.EXT_texture_sRGB_decode)
> +         goto invalid_pname;
> +      *params = (GLenum) sampObj->sRGBDecode;

Ditto.  No need for the cast.

> +      break;
>     default:
>        goto invalid_pname;
>     }

Other than those minor changes, this patch looks good.

With the INVALID_PARAM fix:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>