Re: [Piglit] [PATCH 1/6] shader_runner: Check feature support before querying GL_MAX_*.

2015-11-11 Thread Ilia Mirkin
On Wed, Nov 11, 2015 at 1:46 AM, Matt Turner  wrote:
> Otherwise, these will generate an error (to be noticed sometime later
> when glGetError() is called), often resulting in a test failure.
> ---
>  tests/shaders/shader_runner.c | 18 --
>  1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
> index 32ac7bd..4597b46 100644
> --- a/tests/shaders/shader_runner.c
> +++ b/tests/shaders/shader_runner.c
> @@ -3111,12 +3111,18 @@ piglit_init(int argc, char **argv)
> if (piglit_get_gl_version() >= 32)
> glGetIntegerv(GL_MAX_VERTEX_OUTPUT_COMPONENTS,
>   &gl_max_vertex_output_components);
> -   glGetIntegerv(GL_MAX_FRAGMENT_UNIFORM_COMPONENTS,
> - &gl_max_fragment_uniform_components);
> -   glGetIntegerv(GL_MAX_VERTEX_UNIFORM_COMPONENTS,
> - &gl_max_vertex_uniform_components);
> -   glGetIntegerv(GL_MAX_VARYING_COMPONENTS,
> - &gl_max_varying_components);
> +   if (piglit_get_gl_version() >= 20 ||
> +   piglit_is_extension_supported("GL_ARB_fragment_shader"))
> +   glGetIntegerv(GL_MAX_FRAGMENT_UNIFORM_COMPONENTS,
> + &gl_max_fragment_uniform_components);
> +   if (piglit_get_gl_version() >= 20 ||
> +   piglit_is_extension_supported("GL_ARB_vertex_shader"))
> +   glGetIntegerv(GL_MAX_VERTEX_UNIFORM_COMPONENTS,
> + &gl_max_vertex_uniform_components);
> +   if (piglit_get_gl_version() >= 30 ||
> +   piglit_is_extension_supported("GL_EXT_geometry_shader4"))

I'll admit to not having gone to check the specs, but you almost
certainly mean GL_EXT_gpu_shader4 here, no?

> +   glGetIntegerv(GL_MAX_VARYING_COMPONENTS,
> + &gl_max_varying_components);
> glGetIntegerv(GL_MAX_CLIP_PLANES, &gl_max_clip_planes);
>  #else
> glGetIntegerv(GL_MAX_FRAGMENT_UNIFORM_VECTORS,
> --
> 2.4.9
>
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 1/6] shader_runner: Check feature support before querying GL_MAX_*.

2015-11-11 Thread Matt Turner
On Wed, Nov 11, 2015 at 6:44 AM, Ilia Mirkin  wrote:
> On Wed, Nov 11, 2015 at 1:46 AM, Matt Turner  wrote:
>> Otherwise, these will generate an error (to be noticed sometime later
>> when glGetError() is called), often resulting in a test failure.
>> ---
>>  tests/shaders/shader_runner.c | 18 --
>>  1 file changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
>> index 32ac7bd..4597b46 100644
>> --- a/tests/shaders/shader_runner.c
>> +++ b/tests/shaders/shader_runner.c
>> @@ -3111,12 +3111,18 @@ piglit_init(int argc, char **argv)
>> if (piglit_get_gl_version() >= 32)
>> glGetIntegerv(GL_MAX_VERTEX_OUTPUT_COMPONENTS,
>>   &gl_max_vertex_output_components);
>> -   glGetIntegerv(GL_MAX_FRAGMENT_UNIFORM_COMPONENTS,
>> - &gl_max_fragment_uniform_components);
>> -   glGetIntegerv(GL_MAX_VERTEX_UNIFORM_COMPONENTS,
>> - &gl_max_vertex_uniform_components);
>> -   glGetIntegerv(GL_MAX_VARYING_COMPONENTS,
>> - &gl_max_varying_components);
>> +   if (piglit_get_gl_version() >= 20 ||
>> +   piglit_is_extension_supported("GL_ARB_fragment_shader"))
>> +   glGetIntegerv(GL_MAX_FRAGMENT_UNIFORM_COMPONENTS,
>> + &gl_max_fragment_uniform_components);
>> +   if (piglit_get_gl_version() >= 20 ||
>> +   piglit_is_extension_supported("GL_ARB_vertex_shader"))
>> +   glGetIntegerv(GL_MAX_VERTEX_UNIFORM_COMPONENTS,
>> + &gl_max_vertex_uniform_components);
>> +   if (piglit_get_gl_version() >= 30 ||
>> +   piglit_is_extension_supported("GL_EXT_geometry_shader4"))
>
> I'll admit to not having gone to check the specs, but you almost
> certainly mean GL_EXT_gpu_shader4 here, no?

I don't think so. I just looked at what defines
GL_MAX_VARYING_COMPONENTS{,_EXT} in GL/glext.h and it's GL 3.0 or
GL_EXT_geometry_shader4. GL_EXT_geometry_shader4's spec confirms it,
and GL_EXT_gpu_shader4 does not define MAX_VARYING_COMPONENTS.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 1/6] shader_runner: Check feature support before querying GL_MAX_*.

2015-11-11 Thread Ilia Mirkin
On Wed, Nov 11, 2015 at 2:05 PM, Matt Turner  wrote:
> On Wed, Nov 11, 2015 at 6:44 AM, Ilia Mirkin  wrote:
>> On Wed, Nov 11, 2015 at 1:46 AM, Matt Turner  wrote:
>>> Otherwise, these will generate an error (to be noticed sometime later
>>> when glGetError() is called), often resulting in a test failure.
>>> ---
>>>  tests/shaders/shader_runner.c | 18 --
>>>  1 file changed, 12 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
>>> index 32ac7bd..4597b46 100644
>>> --- a/tests/shaders/shader_runner.c
>>> +++ b/tests/shaders/shader_runner.c
>>> @@ -3111,12 +3111,18 @@ piglit_init(int argc, char **argv)
>>> if (piglit_get_gl_version() >= 32)
>>> glGetIntegerv(GL_MAX_VERTEX_OUTPUT_COMPONENTS,
>>>   &gl_max_vertex_output_components);
>>> -   glGetIntegerv(GL_MAX_FRAGMENT_UNIFORM_COMPONENTS,
>>> - &gl_max_fragment_uniform_components);
>>> -   glGetIntegerv(GL_MAX_VERTEX_UNIFORM_COMPONENTS,
>>> - &gl_max_vertex_uniform_components);
>>> -   glGetIntegerv(GL_MAX_VARYING_COMPONENTS,
>>> - &gl_max_varying_components);
>>> +   if (piglit_get_gl_version() >= 20 ||
>>> +   piglit_is_extension_supported("GL_ARB_fragment_shader"))
>>> +   glGetIntegerv(GL_MAX_FRAGMENT_UNIFORM_COMPONENTS,
>>> + &gl_max_fragment_uniform_components);
>>> +   if (piglit_get_gl_version() >= 20 ||
>>> +   piglit_is_extension_supported("GL_ARB_vertex_shader"))
>>> +   glGetIntegerv(GL_MAX_VERTEX_UNIFORM_COMPONENTS,
>>> + &gl_max_vertex_uniform_components);
>>> +   if (piglit_get_gl_version() >= 30 ||
>>> +   piglit_is_extension_supported("GL_EXT_geometry_shader4"))
>>
>> I'll admit to not having gone to check the specs, but you almost
>> certainly mean GL_EXT_gpu_shader4 here, no?
>
> I don't think so. I just looked at what defines
> GL_MAX_VARYING_COMPONENTS{,_EXT} in GL/glext.h and it's GL 3.0 or
> GL_EXT_geometry_shader4. GL_EXT_geometry_shader4's spec confirms it,
> and GL_EXT_gpu_shader4 does not define MAX_VARYING_COMPONENTS.

Indeed. I guess that's because VS and GS can output diff quantities of
data? Whatever. This patch is

Reviewed-by: Ilia Mirkin 
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 1/6] shader_runner: Check feature support before querying GL_MAX_*.

2015-11-11 Thread Kenneth Graunke
On Tuesday, November 10, 2015 10:46:18 PM Matt Turner wrote:
> Otherwise, these will generate an error (to be noticed sometime later
> when glGetError() is called), often resulting in a test failure.
> ---
>  tests/shaders/shader_runner.c | 18 --
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
> index 32ac7bd..4597b46 100644
> --- a/tests/shaders/shader_runner.c
> +++ b/tests/shaders/shader_runner.c
> @@ -3111,12 +3111,18 @@ piglit_init(int argc, char **argv)
>   if (piglit_get_gl_version() >= 32)
>   glGetIntegerv(GL_MAX_VERTEX_OUTPUT_COMPONENTS,
> &gl_max_vertex_output_components);
> - glGetIntegerv(GL_MAX_FRAGMENT_UNIFORM_COMPONENTS,
> -   &gl_max_fragment_uniform_components);
> - glGetIntegerv(GL_MAX_VERTEX_UNIFORM_COMPONENTS,
> -   &gl_max_vertex_uniform_components);
> - glGetIntegerv(GL_MAX_VARYING_COMPONENTS,
> -   &gl_max_varying_components);
> + if (piglit_get_gl_version() >= 20 ||
> + piglit_is_extension_supported("GL_ARB_fragment_shader"))
> + glGetIntegerv(GL_MAX_FRAGMENT_UNIFORM_COMPONENTS,
> +   &gl_max_fragment_uniform_components);
> + if (piglit_get_gl_version() >= 20 ||
> + piglit_is_extension_supported("GL_ARB_vertex_shader"))
> + glGetIntegerv(GL_MAX_VERTEX_UNIFORM_COMPONENTS,
> +   &gl_max_vertex_uniform_components);

Above this, we call piglit_require_GLSL()...it seems like that
ought to be causing us to skip in this case.  Maybe we don't
return early enough?

There's also piglit_require_vertex_shader() and
piglit_require_fragment_shader() if those are useful to you...

> + if (piglit_get_gl_version() >= 30 ||
> + piglit_is_extension_supported("GL_EXT_geometry_shader4"))
> + glGetIntegerv(GL_MAX_VARYING_COMPONENTS,
> +   &gl_max_varying_components);

There's also GL_ARB_geometry_shader4, which introduces this.

>   glGetIntegerv(GL_MAX_CLIP_PLANES, &gl_max_clip_planes);
>  #else
>   glGetIntegerv(GL_MAX_FRAGMENT_UNIFORM_VECTORS,
> 


signature.asc
Description: This is a digitally signed message part.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 1/6] shader_runner: Check feature support before querying GL_MAX_*.

2015-11-13 Thread Matt Turner
On Wed, Nov 11, 2015 at 4:47 PM, Kenneth Graunke  wrote:
> On Tuesday, November 10, 2015 10:46:18 PM Matt Turner wrote:
>> Otherwise, these will generate an error (to be noticed sometime later
>> when glGetError() is called), often resulting in a test failure.
>> ---
>>  tests/shaders/shader_runner.c | 18 --
>>  1 file changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
>> index 32ac7bd..4597b46 100644
>> --- a/tests/shaders/shader_runner.c
>> +++ b/tests/shaders/shader_runner.c
>> @@ -3111,12 +3111,18 @@ piglit_init(int argc, char **argv)
>>   if (piglit_get_gl_version() >= 32)
>>   glGetIntegerv(GL_MAX_VERTEX_OUTPUT_COMPONENTS,
>> &gl_max_vertex_output_components);
>> - glGetIntegerv(GL_MAX_FRAGMENT_UNIFORM_COMPONENTS,
>> -   &gl_max_fragment_uniform_components);
>> - glGetIntegerv(GL_MAX_VERTEX_UNIFORM_COMPONENTS,
>> -   &gl_max_vertex_uniform_components);
>> - glGetIntegerv(GL_MAX_VARYING_COMPONENTS,
>> -   &gl_max_varying_components);
>> + if (piglit_get_gl_version() >= 20 ||
>> + piglit_is_extension_supported("GL_ARB_fragment_shader"))
>> + glGetIntegerv(GL_MAX_FRAGMENT_UNIFORM_COMPONENTS,
>> +   &gl_max_fragment_uniform_components);
>> + if (piglit_get_gl_version() >= 20 ||
>> + piglit_is_extension_supported("GL_ARB_vertex_shader"))
>> + glGetIntegerv(GL_MAX_VERTEX_UNIFORM_COMPONENTS,
>> +   &gl_max_vertex_uniform_components);
>
> Above this, we call piglit_require_GLSL()...it seems like that
> ought to be causing us to skip in this case.  Maybe we don't
> return early enough?

We talked about this on IRC, but for the mailing list:

The problem is that piglit_require_GLSL() says "all okay" if you have
GL 2.0 or (GL_ARB_shader_objects and GL_ARB_shading_language_100), and
R200 exposes both of those extensions, even though it doesn't expose
the actual extensions you need for shading... ARB_fragment_shader or
ARB_vertex_shader.

And moreover, since we support fragment program and vertex program
shader_tests, we don't actually want to be requiring GLSL anyway.

I'll work on sorting that out, but I think t his patch is good as is
(with the GL_ARB_geometry_shader4 change).
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit