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

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

Thanks for the quick feedback! I'll resend the patch with the
requested fixes shortly.

Stefan

Am 2015-07-30 um 15:30 schrieb 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:
>> 
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJVuisUAAoJEN0/YqbEcdMwwl4P/Awl2F5yQyO0gmxzwwTOj4Cx
3yReuJmJj++rJkP8ZnYOD4ATOSwqfVFvwRvV5i2s/UhtqO5DDmS65IJ92qVLbgzH
As7QnUdoIFGlSB5vZv9KljQJhRFhrllod6xKYWGY8VtE4f8imri6vs6/2TperDHv
tkLF50UJX/JWo96aQDD+PuMPF02EcvpN1AJIvbqhswtm30T5a2QNRiiXFcEdoecQ
KbKKhKdoivLZ57ltyeeI2aCBVWm/EfyqX+wS4aRYLWlj8e2dQVHfPqeVcw8vfNPB
lIF8qDjzuX5kgy6KrtKbKKcMLMIVyzs+pWJAuCBkOlbvxEIm8zldlyh5ClGquU/o
BBa45KtKotzHnddmKdDl7MQBh9J1zSygGGbiHCUi64JbtAi6Kco4qmI7RRKl0dVa
aAzU/6eZV34Gving8SK80yvDAYdBRJ39m1xRnXHdhTym64ZfA4jH7gS6PBpyY9AQ
ZIQwtnUQogdgl+fsNR6e3BTEjRnFb8qQHk+yboOhRrsfd6quLmbUXItKleTWeYW4
UtZq1HL/omeTMetgKIT7ZwQAOJ7Exasz8w/tQmqyGjhc3fuIqye+gKb38sOAeGdZ
tdMYLK81/e5msujNcXUNmAr3yAtkFuFxPa/lHs1KHN95ZeDTfk2g6KE6fv6vazZg
/3JHWSX18t2PI53dQMR1
=nLxG
-----END PGP SIGNATURE-----