Re: [Piglit] [PATCH 1/6] shader_runner: Check feature support before querying GL_MAX_*.
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
Re: [Piglit] [PATCH 1/6] shader_runner: Check feature support before querying GL_MAX_*.
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_*.
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_*.
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_*.
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
[Piglit] [PATCH 1/6] shader_runner: Check feature support before querying GL_MAX_*.
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")) + 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