[Piglit] [PATCH] Test that there are 8 clip planes/distances in GLSL 1.30.

[Paul Berry]

On 19 September 2011 10:49, Ian Romanick <idr at freedesktop.org> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 09/13/2011 02:54 PM, Paul Berry wrote:
> > The clipping tests previously in Piglit only tested the first 6
> > clipping planes, since prior to GLSL 1.30, a conforming
> > implementation was allowed to only support 6 clipping planes.
> > However, in GLSL 1.30, the minimum value of gl_MaxClipDistances and
> > gl_MaxClipPlanes was increased to 8.
> >
> > This patch adds tests to verify that if the implementation reports
> > a GLSL version of at least 1.30, gl_MaxClipDistances and
> > gl_MaxClipPlanes are both at least 8.  It also modifies
> > vs-clip-distance-all-planes-enabled.shader_test to exercise all 8
> > clipping planes.
> >
> > The previous implementation of
> > vs-clip-distance-all-planes-enabled.shader_test used to also
> > verify, as a side effect, that gl_ClipDistance could be implicitly
> > sized.  The new version does not (it requires gl_ClipDistance to be
> > explicitly sized, since it indexes into it using a loop variable).
> > So I've added a new test,
> > vs-clip-distance-implicitly-sized.shader_test, to verify this.
> >
> > Tested using the nVidia proprietary Linux driver.
>
> I made some comments about max-clip-distances.shader_test (which also
> apply to max-clip-planes.shader_test) below.
>
> The changes to shader_runner and the other tests look fine.  You can
> consider them
>
> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
>
> if you want to split them out and push them sooner rather than later.
>
> > --- tests/shaders/shader_runner.c                      |    2 +
> > .../clipping/max-clip-distances.shader_test        |   54
> > ++++++++++++++ .../execution/clipping/max-clip-planes.shader_test |
> > 54 ++++++++++++++
> > ...vs-clip-distance-all-planes-enabled.shader_test |   76
> > ++++++++++----------
> > .../vs-clip-distance-implicitly-sized.shader_test  |   67
> > +++++++++++++++++ tests/util/glew.h
> > |    2 + 6 files changed, 218 insertions(+), 37 deletions(-) create
> > mode 100644
> > tests/spec/glsl-1.30/execution/clipping/max-clip-distances.shader_test
> >
> >
> create mode 100644
> tests/spec/glsl-1.30/execution/clipping/max-clip-planes.shader_test
> > create mode 100644
> >
> tests/spec/glsl-1.30/execution/clipping/vs-clip-distance-implicitly-sized.shader_test
> >
> >  diff --git a/tests/shaders/shader_runner.c
> > b/tests/shaders/shader_runner.c index cde5ae3..a46ea4d 100644 ---
> > a/tests/shaders/shader_runner.c +++
> > b/tests/shaders/shader_runner.c @@ -819,6 +819,8 @@ struct
> > enable_table { { "GL_CLIP_PLANE3", GL_CLIP_PLANE3 }, {
> > "GL_CLIP_PLANE4", GL_CLIP_PLANE4 }, { "GL_CLIP_PLANE5",
> > GL_CLIP_PLANE5 }, +   { "GL_CLIP_PLANE6", GL_CLIP_PLANE6 }, + {
> > "GL_CLIP_PLANE7", GL_CLIP_PLANE7 }, { NULL, 0 } };
> >
> > diff --git
> > a/tests/spec/glsl-1.30/execution/clipping/max-clip-distances.shader_test
> > b/tests/spec/glsl-1.30/execution/clipping/max-clip-distances.shader_test
> >
> >
> new file mode 100644
> > index 0000000..d593fbf --- /dev/null +++
> > b/tests/spec/glsl-1.30/execution/clipping/max-clip-distances.shader_test
> >
> >
> @@ -0,0 +1,54 @@
> > +# [description] +# Verify correct behavior of
> > gl_MaxClipDistances. +# +# From section the GLSL 1.30 spec, section
> > 7.4 (Built-In Constants): +# +#   "The following built-in constants
> > are provided to vertex and +#   fragment shaders. The actual values
> > used are implementation +#   dependent, but must be at least the
> > value shown. +# +#   ... +# +#   const int gl_MaxClipDistances =
> > 8;" +# +# This test verifies that gl_MaxClipDistances has the same
> > value in +# both the vertex and fragment shader, and that this
> > value is at least +# 8. + +[require] +GLSL >= 1.30 + +[vertex
> > shader] +#version 130 + +out float maxClipDistances; + +void
> > main() +{ +   gl_Position = gl_Vertex; + +    maxClipDistances =
> > gl_MaxClipDistances; +} + +[fragment shader] +#version 130 + +in
> > float maxClipDistances; + +void main() +{ +   if (maxClipDistances !=
> > gl_MaxClipDistances) {
>
> This is valid, but probably not what we actually want.  Shouldn't we
> pass maxClipDistances as an int?  It means that the 'flat'
> interpolation qualifier will have to be used, and that may cause the
> test to fail on Mesa now even though we set gl_MaxClipDistances correctly.
>

Hmm, yeah, using ints and the 'flat' qualifier would be a better way to do
this.  You're right that this causes the tests to fail right now on Mesa.  I
assume this is because passing 'int' data between VS and FS doesn't work
properly yet?


>
> Also, this test is really checking that gl_MaxClipDistances is set the
> same in the VS and FS.  I'm not sure how useful that is.  It seems
> like we want to query GL_MAX_CLIP_DISTANCES from the API and pass that
> as a uniform.  Maybe that's an additional test?
>

You're right--this would be more useful.  Ok, I'll commit the tests that got
your Reviewed-by, and I'll submit a follow-up patch to test that
GL_MAX_CLIP_DISTANCES matches the value of gl_MaxClipDistances in both the
VS and FS.

Paul
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20110919/4c805c69/attachment.htm>