[Piglit] [RFC PATCH] Test that in/out qualifiers are required when redeclaring gl_ClipDistance.

[Paul Berry]

On 16 October 2013 00:26, Jordan Justen <jljusten at gmail.com> wrote:

> On Tue, Oct 8, 2013 at 11:29 AM, Paul Berry <stereotype441 at gmail.com>
> wrote:
> > Although it is not explicitly stated in the GLSL spec, all examples of
> > redeclaring built-in in/out variables (such as gl_ClipDistance)
> > include the "in" or "out" qualifier, so it seems like failure to do so
> > should cause a compile error.
> >
> > At present:
> >
> > - Mesa (as of commit a50c5f8) does not generate an error.
> >
> > - The NVIDIA proprietary driver for Linux (version 313.18) does not
> >   generate an error; however, if gl_ClipDistance is redeclared in the
> >   fragment shader without specifying the "in" keyword, it fails to
> >   work properly.
>
> By 'fails to work properly', do you mean that it is unlikely that an
> app could be relying on this behavior? If so, it seems fine to add
> this test and make mesa generate a compile error.
>

Without the "in" keyword, gl_ClipDistance receives garbage data.  So yeah,
I think it's unlikely that there's an app out there that relies on this.


>
> Regarding 'out' with the VS, do you think you've tested it enough to
> be sure that an app also could not accidentally be relying on this
> behavior?
>

Unfortunately, on NVIDIA, forgetting "out" when redeclaring gl_ClipDistance
in the VS doesn't appear to lead to any problems.  So I can't rule out the
possibility that an app out there somewhere relies on it.


>
> If we are pretty confident that apps won't break, then
> Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>
>
> Also, perhaps we should log a spec bug to ask the spec to clarify this?
>

IMHO, the intent is already clear from the fact that all the redeclaration
examples in the spec include the in/out qualifier.  The fact that the
NVIDIA compiler sometimes misbehaves if in/out is absent seems like
adequate confirmation to me.  Spec bugs frequently take weeks or months to
get resolved, so I guess I'm having trouble convincing myself that it's
worth it in this case.


Anyone else want to weigh in with an opinion on this?  Idr?




>
> -Jordan
>
> > ---
> >  .../clip-distance-redeclare-without-inout.frag      | 21
> +++++++++++++++++++++
> >  .../clip-distance-redeclare-without-inout.vert      | 21
> +++++++++++++++++++++
> >  2 files changed, 42 insertions(+)
> >  create mode 100644
> tests/spec/glsl-1.30/compiler/clipping/clip-distance-redeclare-without-inout.frag
> >  create mode 100644
> tests/spec/glsl-1.30/compiler/clipping/clip-distance-redeclare-without-inout.vert
> >
> > diff --git
> a/tests/spec/glsl-1.30/compiler/clipping/clip-distance-redeclare-without-inout.frag
> b/tests/spec/glsl-1.30/compiler/clipping/clip-distance-redeclare-without-inout.frag
> > new file mode 100644
> > index 0000000..60f0650
> > --- /dev/null
> > +++
> b/tests/spec/glsl-1.30/compiler/clipping/clip-distance-redeclare-without-inout.frag
> > @@ -0,0 +1,21 @@
> > +/* [config]
> > + * expect_result: fail
> > + * glsl_version: 1.30
> > + * check_link: true
> > + * [end config]
> > + *
> > + * Although it is not explicitly stated in the GLSL spec, in all
> > + * examples, redeclarations of built-in variables (such as
> > + * gl_ClipDistance) preserve the in/out qualifiers.
> > + *
> > + * This test verifies that a shader is rejected if it tries to
> > + * redeclare gl_ClipDistance without an in/out qualifier.
> > + */
> > +#version 130
> > +
> > +float gl_ClipDistance[3];
> > +
> > +void main()
> > +{
> > +  gl_FragColor = vec4(0.0);
> > +}
> > diff --git
> a/tests/spec/glsl-1.30/compiler/clipping/clip-distance-redeclare-without-inout.vert
> b/tests/spec/glsl-1.30/compiler/clipping/clip-distance-redeclare-without-inout.vert
> > new file mode 100644
> > index 0000000..d385cc7
> > --- /dev/null
> > +++
> b/tests/spec/glsl-1.30/compiler/clipping/clip-distance-redeclare-without-inout.vert
> > @@ -0,0 +1,21 @@
> > +/* [config]
> > + * expect_result: fail
> > + * glsl_version: 1.30
> > + * check_link: true
> > + * [end config]
> > + *
> > + * Although it is not explicitly stated in the GLSL spec, in all
> > + * examples, redeclarations of built-in variables (such as
> > + * gl_ClipDistance) preserve the in/out qualifiers.
> > + *
> > + * This test verifies that a shader is rejected if it tries to
> > + * redeclare gl_ClipDistance without an in/out qualifier.
> > + */
> > +#version 130
> > +
> > +float gl_ClipDistance[3];
> > +
> > +void main()
> > +{
> > +  gl_Position = vec4(0.0);
> > +}
> > --
> > 1.8.4
> >
> > _______________________________________________
> > Piglit mailing list
> > Piglit at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/piglit
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20131016/9909d9f2/attachment.html>