[Piglit] [PATCH] glsl-version-define-130: New test for glsl 1.30 existence and __VERSION__ reflecting glsl 1.30.

[Zhao, Jian J]

> -----Original Message-----
> From: Ian Romanick [mailto:idr at freedesktop.org]
> Sent: Friday, December 02, 2011 5:36 AM
> To: Zhao, Jian J
> Cc: piglit at lists.freedesktop.org; eric at anholt.net
> Subject: Re: [PATCH] glsl-version-define-130: New test for glsl 1.30 existence
> and __VERSION__ reflecting glsl 1.30.
> 
> On 11/29/2011 01:03 AM, jian.j.zhao at intel.com wrote:
> > From: Jian Zhao<jian.j.zhao at intel.com>
> >
> > According to the glsl spec page 7:
> >       "This document specifies only version 1.30 of the OpenGL
> >       Shading Language. It requires __VERSION__ to substitute
> >       130, and requires #version to accept only 130. If #version
> >       is declared with 110 or 120, the language accepted is a
> >       previous version of the shading language, which will be
> >       supported depending on the version and type of context in
> >       the OpenGL API."
> 
> This should be a comment in the test.  It should also say:
> 
> "If the supported GLSL version is higher than 130, __VERSION__ will have
> that value.  Dividing by 130 will result in a value > 1.0, and this will
> be clamped to 1.0 when it's written to the framebuffer."
> 
> This is why I suggested having a C test instead of a shader_runner test.
>   __VERSION__ should always be the value from
> glGetString(GL_SHADING_LANGUAGE_VERSION) regardless of the #version
> line
> in the shader.  *That* is what we should test.

If so, the __ VERSION__ should not be affected by #version line in the shader. I will add a C test for it. 

BTW, I find there are three piglit cases(glsl-version-define.shader_test, glsl-version-define-110.shader_test, glsl-version-define-120.shader_test) relate to the __VERSION__ seems wrong. Because if the __ VERSION__ has no relation with #version line, all of them should fail now, but they all pass now, which means the __VERSION__ in it just take the version from #version line. Can you have a look at it? Thanks. 

> 
> > v2: use in and out instead of some deprecated variables.
> > ---
> >   tests/shaders/glsl-version-define-130.shader_test |   22
> +++++++++++++++++++++
> >   1 files changed, 22 insertions(+), 0 deletions(-)
> >   create mode 100644 tests/shaders/glsl-version-define-130.shader_test
> >
> > diff --git a/tests/shaders/glsl-version-define-130.shader_test
> b/tests/shaders/glsl-version-define-130.shader_test
> > new file mode 100644
> > index 0000000..e0981a6
> > --- /dev/null
> > +++ b/tests/shaders/glsl-version-define-130.shader_test
> > @@ -0,0 +1,22 @@
> > +[require]
> > +GLSL>= 1.30
> > +
> > +[vertex shader]
> > +#version 130
> > +in vec4 position;
> 
> Shader runner doesn't have a way to handle attributes that aren't
> built-in.  If this works, it is only by luck.

OK. I will remove it. 

Best regards
ZhaoJian

> 
> > +void main()
> > +{
> > +	gl_Position = position;
> > +}
> > +
> > +[fragment shader]
> > +#version 130
> > +out vec4 FragColor;
> 
> Same comment as above.
> 
> > +void main()
> > +{
> > +	FragColor = vec4(0.0, float(__VERSION__) / 130.0, 0.0, 0.0);
> > +}
> > +
> > +[test]
> > +draw rect -1 -1 2 2
> > +probe rgba 1 1 0.0 1.0 0.0 0.0