[Mesa-dev] [PATCH] Allow setting GL_TEXTURE_COMPARE_MODE on a sampler object without ARB_shadow support (v2).

[Brian Paul]

Can you try to shorten the subject line to 75 chars or less?  It should 
also start with "mesa:".  Perhaps:

"mesa: disable texture compare mode error checking for Wine"


On 07/30/2015 07:04 AM, Stefan Dösinger wrote:
> Version 2: Return GL_FALSE if ARB_shadow is unsupported instead of pretending to store the value as

Please word wrap that line to 75 chars or less.

> suggested by Brian Paul.
>
> This fixes a GL error warning on r200 in Wine.
>
> The GL_ARB_sampler_objects extension does not specify a dependency on
> GL_ARB_shadow or GL_ARB_depth_texture for this value. Just set the value
> and don't do anything else. It won't matter without a depth texture
> being assigned anyway.

This comment doesn't reflect what the code is really doing.  The code is 
not setting any values.


> ---
>   src/mesa/main/samplerobj.c | 28 ++++++++++------------------
>   1 file changed, 10 insertions(+), 18 deletions(-)
>
> diff --git a/src/mesa/main/samplerobj.c b/src/mesa/main/samplerobj.c
> index 32180fb1..f5d2077 100644
> --- a/src/mesa/main/samplerobj.c
> +++ b/src/mesa/main/samplerobj.c
> @@ -634,8 +634,12 @@ static GLuint
>   set_sampler_compare_mode(struct gl_context *ctx,
>                            struct gl_sampler_object *samp, GLint param)
>   {
> +   /* No state change, no error. GL_ARB_sampler_objects does not
> +    * have a dependency on GL_ARB_shadow for these settings. They
> +    * don't have any effect without shadow textures anyway.
> +    * This matters for Wine on old GPUs, e.g. r200. */

Please put closing */ on next line.  Actually, the comment could be 
simplified:

/* If GL_ARB_shadow is not supported, don't report an error.  The
  * sampler object extension spec isn't clear on this extension interaction.
  * Silences errors with Wine on older GPUs such as R200.
  */


>      if (!ctx->Extensions.ARB_shadow)
> -      return INVALID_PNAME;
> +      return GL_FALSE;
>
>      if (samp->CompareMode == param)
>         return GL_FALSE;
> @@ -655,8 +659,12 @@ static GLuint
>   set_sampler_compare_func(struct gl_context *ctx,
>                            struct gl_sampler_object *samp, GLint param)
>   {
> +   /* No state change, no error. GL_ARB_sampler_objects does not
> +    * have a dependency on GL_ARB_shadow for these settings. They
> +    * don't have any effect without shadow textures anyway.
> +    * This matters for Wine on old GPUs, e.g. r200. */
>      if (!ctx->Extensions.ARB_shadow)
> -      return INVALID_PNAME;
> +      return GL_FALSE;
>
>      if (samp->CompareFunc == param)
>         return GL_FALSE;
> @@ -1342,13 +1350,9 @@ _mesa_GetSamplerParameteriv(GLuint sampler, GLenum pname, GLint *params)
>         *params = IROUND(sampObj->LodBias);
>         break;
>      case GL_TEXTURE_COMPARE_MODE:
> -      if (!ctx->Extensions.ARB_shadow)
> -         goto invalid_pname;
>         *params = sampObj->CompareMode;
>         break;
>      case GL_TEXTURE_COMPARE_FUNC:
> -      if (!ctx->Extensions.ARB_shadow)
> -         goto invalid_pname;
>         *params = sampObj->CompareFunc;
>         break;
>      case GL_TEXTURE_MAX_ANISOTROPY_EXT:
> @@ -1431,13 +1435,9 @@ _mesa_GetSamplerParameterfv(GLuint sampler, GLenum pname, GLfloat *params)
>         *params = sampObj->LodBias;
>         break;
>      case GL_TEXTURE_COMPARE_MODE:
> -      if (!ctx->Extensions.ARB_shadow)
> -         goto invalid_pname;
>         *params = (GLfloat) sampObj->CompareMode;
>         break;
>      case GL_TEXTURE_COMPARE_FUNC:
> -      if (!ctx->Extensions.ARB_shadow)
> -         goto invalid_pname;
>         *params = (GLfloat) sampObj->CompareFunc;
>         break;
>      case GL_TEXTURE_MAX_ANISOTROPY_EXT:
> @@ -1510,13 +1510,9 @@ _mesa_GetSamplerParameterIiv(GLuint sampler, GLenum pname, GLint *params)
>         *params = (GLint) sampObj->LodBias;
>         break;
>      case GL_TEXTURE_COMPARE_MODE:
> -      if (!ctx->Extensions.ARB_shadow)
> -         goto invalid_pname;
>         *params = sampObj->CompareMode;
>         break;
>      case GL_TEXTURE_COMPARE_FUNC:
> -      if (!ctx->Extensions.ARB_shadow)
> -         goto invalid_pname;
>         *params = sampObj->CompareFunc;
>         break;
>      case GL_TEXTURE_MAX_ANISOTROPY_EXT:
> @@ -1589,13 +1585,9 @@ _mesa_GetSamplerParameterIuiv(GLuint sampler, GLenum pname, GLuint *params)
>         *params = (GLuint) sampObj->LodBias;
>         break;
>      case GL_TEXTURE_COMPARE_MODE:
> -      if (!ctx->Extensions.ARB_shadow)
> -         goto invalid_pname;
>         *params = sampObj->CompareMode;
>         break;
>      case GL_TEXTURE_COMPARE_FUNC:
> -      if (!ctx->Extensions.ARB_shadow)
> -         goto invalid_pname;
>         *params = sampObj->CompareFunc;
>         break;
>      case GL_TEXTURE_MAX_ANISOTROPY_EXT:
>