<div dir="ltr">On 16 October 2013 00:26, Jordan Justen <span dir="ltr"><<a href="mailto:jljusten@gmail.com" target="_blank">jljusten@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On Tue, Oct 8, 2013 at 11:29 AM, Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> wrote:<br>
> Although it is not explicitly stated in the GLSL spec, all examples of<br>
> redeclaring built-in in/out variables (such as gl_ClipDistance)<br>
> include the "in" or "out" qualifier, so it seems like failure to do so<br>
> should cause a compile error.<br>
><br>
> At present:<br>
><br>
> - Mesa (as of commit a50c5f8) does not generate an error.<br>
><br>
> - The NVIDIA proprietary driver for Linux (version 313.18) does not<br>
> generate an error; however, if gl_ClipDistance is redeclared in the<br>
> fragment shader without specifying the "in" keyword, it fails to<br>
> work properly.<br>
<br>
</div>By 'fails to work properly', do you mean that it is unlikely that an<br>
app could be relying on this behavior? If so, it seems fine to add<br>
this test and make mesa generate a compile error.<br></blockquote><div><br></div><div>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.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Regarding 'out' with the VS, do you think you've tested it enough to<br>
be sure that an app also could not accidentally be relying on this<br>
behavior?<br></blockquote><div><br></div><div>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.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
If we are pretty confident that apps won't break, then<br>
Reviewed-by: Jordan Justen <<a href="mailto:jordan.l.justen@intel.com">jordan.l.justen@intel.com</a>><br>
<br>
Also, perhaps we should log a spec bug to ask the spec to clarify this?<br></blockquote><div><br></div><div>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.<br>
<br></div><br><div>Anyone else want to weigh in with an opinion on this? Idr?<br></div><br><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
-Jordan<br>
<div><div class="h5"><br>
> ---<br>
> .../clip-distance-redeclare-without-inout.frag | 21 +++++++++++++++++++++<br>
> .../clip-distance-redeclare-without-inout.vert | 21 +++++++++++++++++++++<br>
> 2 files changed, 42 insertions(+)<br>
> create mode 100644 tests/spec/glsl-1.30/compiler/clipping/clip-distance-redeclare-without-inout.frag<br>
> create mode 100644 tests/spec/glsl-1.30/compiler/clipping/clip-distance-redeclare-without-inout.vert<br>
><br>
> 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<br>
> new file mode 100644<br>
> index 0000000..60f0650<br>
> --- /dev/null<br>
> +++ b/tests/spec/glsl-1.30/compiler/clipping/clip-distance-redeclare-without-inout.frag<br>
> @@ -0,0 +1,21 @@<br>
> +/* [config]<br>
> + * expect_result: fail<br>
> + * glsl_version: 1.30<br>
> + * check_link: true<br>
> + * [end config]<br>
> + *<br>
> + * Although it is not explicitly stated in the GLSL spec, in all<br>
> + * examples, redeclarations of built-in variables (such as<br>
> + * gl_ClipDistance) preserve the in/out qualifiers.<br>
> + *<br>
> + * This test verifies that a shader is rejected if it tries to<br>
> + * redeclare gl_ClipDistance without an in/out qualifier.<br>
> + */<br>
> +#version 130<br>
> +<br>
> +float gl_ClipDistance[3];<br>
> +<br>
> +void main()<br>
> +{<br>
> + gl_FragColor = vec4(0.0);<br>
> +}<br>
> 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<br>
> new file mode 100644<br>
> index 0000000..d385cc7<br>
> --- /dev/null<br>
> +++ b/tests/spec/glsl-1.30/compiler/clipping/clip-distance-redeclare-without-inout.vert<br>
> @@ -0,0 +1,21 @@<br>
> +/* [config]<br>
> + * expect_result: fail<br>
> + * glsl_version: 1.30<br>
> + * check_link: true<br>
> + * [end config]<br>
> + *<br>
> + * Although it is not explicitly stated in the GLSL spec, in all<br>
> + * examples, redeclarations of built-in variables (such as<br>
> + * gl_ClipDistance) preserve the in/out qualifiers.<br>
> + *<br>
> + * This test verifies that a shader is rejected if it tries to<br>
> + * redeclare gl_ClipDistance without an in/out qualifier.<br>
> + */<br>
> +#version 130<br>
> +<br>
> +float gl_ClipDistance[3];<br>
> +<br>
> +void main()<br>
> +{<br>
> + gl_Position = vec4(0.0);<br>
> +}<br>
> --<br>
> 1.8.4<br>
><br>
</div></div>> _______________________________________________<br>
> Piglit mailing list<br>
> <a href="mailto:Piglit@lists.freedesktop.org">Piglit@lists.freedesktop.org</a><br>
> <a href="http://lists.freedesktop.org/mailman/listinfo/piglit" target="_blank">http://lists.freedesktop.org/mailman/listinfo/piglit</a><br>
</blockquote></div><br></div></div>