On 16 October 2013 00:26, Jordan Justen <[email protected]> wrote: > On Tue, Oct 8, 2013 at 11:29 AM, Paul Berry <[email protected]> > 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 <[email protected]> > > 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 > > [email protected] > > http://lists.freedesktop.org/mailman/listinfo/piglit >
_______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
