[Piglit] [PATCH] gs: Don't try to use gl_PerVertex when redeclaring fragment shader inputs.

[Paul Berry]

On 1 October 2013 18:23, Ian Romanick <idr at freedesktop.org> wrote:

> On 10/01/2013 03:21 PM, Paul Berry wrote:
> > In commit 5b1dff3 (Geometry shaders: Test gl_ClipDistance input (GLSL
> > 1.50)) I accidentally used this syntax to redeclare the
> > gl_ClipDistance fragment shader input:
> >
> >     in gl_PerVertex {
> >       float gl_ClipDistance[8];
> >     };
> >
> > This is incorrect--gl_PerVertex is only meaningful in vertex and
> > geometry shaders.  In fragment shaders gl_ClipDistance isn't in any
> > interface block.
>
> Oops.  I overlooked that too.
>
> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
>
> Without this fix, does this test pass on NVIDIA?
>

Oh, argh.  I'm glad you asked, because running the test on NVIDIA revealed
that this patch is bogus.  Instead of replacing

in gl_PerVertex {
  float gl_ClipDistance[8];
};

with:

float gl_ClipDistance[8];

I should have replaced it with:

in float gl_ClipDistance[8];


NVIDIA's linux driver (version 313.18) behaves as follows:

With piglit master, the compiler accepts the shader but the test fails.
With the bogus patch, the compiler accepts the shader but the test fails.
With a corrected patch, the compiler accepts the shader and the test passes.

Mesa (using my current work-in-progress "gs" branch) behaves as follows:

With piglit master, it crashes due to known bugs in my work-in-progress
branch.
With the bogus patch, the compiler accepts the shader and the test passes,
due to a previously unknown bug on Mesa master.
With the corrected patch, the compiler accepts the shader and the test
passes.


What a mess.  I'll follow up with a corrected version of this patch, and
I'll file a report in bugzilla for the previously unknown Mesa bug.


>
> > ---
> >  .../glsl-1.50/execution/geometry/clip-distance-bulk-copy.shader_test  |
> 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git
> a/tests/spec/glsl-1.50/execution/geometry/clip-distance-bulk-copy.shader_test
> b/tests/spec/glsl-1.50/execution/geometry/clip-distance-bulk-copy.shader_test
> > index ef21260..856f436 100644
> > ---
> a/tests/spec/glsl-1.50/execution/geometry/clip-distance-bulk-copy.shader_test
> > +++
> b/tests/spec/glsl-1.50/execution/geometry/clip-distance-bulk-copy.shader_test
> > @@ -56,9 +56,7 @@ void main()
> >  [fragment shader]
> >  #version 150
> >
> > -in gl_PerVertex {
> > -  float gl_ClipDistance[8];
> > -};
> > +float gl_ClipDistance[8];
> >  in float offset_to_fs;
> >
> >  void main()
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20131001/fb5ec1fa/attachment.html>