[Piglit] [PATCH] built-in-constants: check required GLSL versions for #extensions

[Juan A. Suarez Romero]

On Mon, 2017-11-13 at 13:24 -0500, Ilia Mirkin wrote:
> On Mon, Nov 13, 2017 at 12:48 PM, Juan A. Suarez Romero
> <jasuarez at igalia.com> wrote:
> > GL_OES_geometry_shader and GL_OES_tessellation_shader specifications
> > require OpenGL ES Shading Language 3.10.
> > 
> > GL_ARB_tessellation_shader specification requires GLSL 1.50.
> > ---
> >  tests/shaders/built-in-constants.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tests/shaders/built-in-constants.c b/tests/shaders/built-in-constants.c
> > index d470fe1bf..6ad26dccf 100644
> > --- a/tests/shaders/built-in-constants.c
> > +++ b/tests/shaders/built-in-constants.c
> > @@ -386,7 +386,7 @@ create_shader(GLenum type)
> >                         return 0;
> >         } else {
> >                 if (is_tessellation_type(type) &&
> > -                   (required_glsl_version < 400 &&
> > +                   (required_glsl_version < 150 ||
> >                      !piglit_is_extension_supported("GL_ARB_tessellation_shader")))
> 
> I believe this code was correct as-is. With your version, you'll
> disallow a shader with #version 400 when the impl doesn't advertise
> the separate GL_ARB_tessellation_shader (which there's no requirement
> it advertise, AFAIK).
> 

Yes, you're right. I think this hunk was added in the patchset by
mistake.


> The one thing is that it'll allow shaders through with
> required_glsl_version < 150 if the impl supports tess, but ... hard to
> care about that.
> 
> >                         return 0;
> > 
> > @@ -461,7 +461,7 @@ piglit_init(int argc, char **argv)
> >         /* Geometry shaders must use the #extension directive in GLSL ES
> >          * before version 3.20.
> >          */
> > -       if (es_shader && required_glsl_version < 320 &&
> > +       if (es_shader && required_glsl_version == 310 &&
> >             piglit_is_extension_supported("GL_OES_geometry_shader")) {
> >                 assert(num_required_extensions < ARRAY_SIZE(required_extensions));
> >                 required_extensions[num_required_extensions] =
> 
> What situation are you trying to fix here?
> 
> > @@ -473,7 +473,9 @@ piglit_init(int argc, char **argv)
> >         const char *const tess_ext_name = es_shader
> >                 ? "GL_OES_tessellation_shader"
> >                 : "GL_ARB_tessellation_shader";
> > -       if (piglit_is_extension_supported(tess_ext_name)) {
> > +       if (((es_shader && required_glsl_version == 310) ||
> > +           (!es_shader && required_glsl_version >= 150)) &&
> > +           piglit_is_extension_supported(tess_ext_name)) {
> >                 assert(num_required_extensions < ARRAY_SIZE(required_extensions));
> >                 required_extensions[num_required_extensions] =
> >                         strdup(tess_ext_name);
> 
> And here?
> 


First all, I'll push a new version of the patch, as I did some changes
there.


The fix I am trying to do is that in order to declare
OES_tessellation_shader or OES_geometry_shader extensions in the
shader, OpenGL ES Shading Language must be 3.10 or newer.

Thus we need to avoid adding those extensions in the shader code on
older versions.


	J.A.