On 11/20/2017 04:12 AM, Emil Velikov wrote:
> Hi Scott,
>
> I think there's a few missed checks if the extensions is enabled.
>
> On 14 November 2017 at 22:54, Scott D Phillips
> <scott.d.phill...@intel.com> wrote:
>
>> @@ -218,6 +218,7 @@ _legal_parameters(struct gl_context *ctx, GLenum target,
>> GLenum internalformat,
>> case GL_VIEW_COMPATIBILITY_CLASS:
>> case GL_NUM_TILING_TYPES_EXT:
>> case GL_TILING_TYPES_EXT:
>> + case GL_TEXTURE_REDUCTION_MODE_ARB:
> Here.
I have always thought that we should add negative tests when we enable a
new extension. Specifically, tests that try to use the new enums in
various valid places on drivers that do not support the extension.
There are a couple tests like this, but not a lot. The only one I could
find for sure was
tests/spec/arb_texture_buffer_object/negative-unsupported.c.
>> @@ -1464,6 +1489,9 @@ _mesa_GetSamplerParameteriv(GLuint sampler, GLenum
>> pname, GLint *params)
>> @@ -1536,6 +1564,9 @@ _mesa_GetSamplerParameterfv(GLuint sampler, GLenum
>> pname, GLfloat *params)
>> @@ -1608,6 +1639,9 @@ _mesa_GetSamplerParameterIiv(GLuint sampler, GLenum
>> pname, GLint *params)
>> @@ -1680,6 +1714,9 @@ _mesa_GetSamplerParameterIuiv(GLuint sampler, GLenum
>> pname, GLuint *params)
>> goto invalid_pname;
>> *params = (GLenum) sampObj->sRGBDecode;
>> break;
>> + case GL_TEXTURE_REDUCTION_MODE_ARB:
> And these four?
>
>> --- a/src/mesa/main/texparam.c
>> +++ b/src/mesa/main/texparam.c
>> @@ -630,6 +630,25 @@ set_tex_parameteri(struct gl_context *ctx,
>> }
>> goto invalid_pname;
>>
>> + case GL_TEXTURE_REDUCTION_MODE_ARB:
>> + if (ctx->Extensions.ARB_texture_filter_minmax) {
>> +
>> + if
>> (!_mesa_target_allows_setting_sampler_parameters(texObj->Target))
>> + goto invalid_enum;
>> +
>> + if (texObj->Sampler.ReductionMode == params[0])
>> + return GL_FALSE;
>> + if (params[0] == GL_MIN ||
>> + params[0] == GL_MAX ||
>> + params[0] == GL_WEIGHTED_AVERAGE_ARB) {
>> + flush(ctx);
>> + texObj->Sampler.ReductionMode = params[0];
>> + return GL_TRUE;
>> + }
>> + goto invalid_param;
>> + }
>> + goto invalid_pname;
>> +
> Using the 'return early' approach will make it easier to read ...
> although the function is inconsistent so meh.
> Just an example, I'm talking about:
>
> case GL_TEXTURE_REDUCTION_MODE_ARB:
> if (!ctx->Extensions.ARB_texture_filter_minmax)
> goto invalid_pname;
>
> if (!_mesa_target_allows_setting_sampler_parameters(texObj->Target))
> goto invalid_enum;
>
> if (texObj->Sampler.ReductionMode == params[0])
> return GL_FALSE;
>
> if (params[0] != GL_MIN &&
> params[0] != GL_MAX &&
> params[0] != GL_WEIGHTED_AVERAGE_ARB)
> goto invalid_param;
> }
>
> flush(ctx);
> texObj->Sampler.ReductionMode = params[0];
> return GL_TRUE;
>
>
> HTH
> Emil
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
