<div dir="ltr">On 1 October 2013 18:23, Ian Romanick <span dir="ltr"><<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</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 10/01/2013 03:21 PM, Paul Berry wrote:<br>
> In commit 5b1dff3 (Geometry shaders: Test gl_ClipDistance input (GLSL<br>
> 1.50)) I accidentally used this syntax to redeclare the<br>
> gl_ClipDistance fragment shader input:<br>
><br>
>     in gl_PerVertex {<br>
>       float gl_ClipDistance[8];<br>
>     };<br>
><br>
> This is incorrect--gl_PerVertex is only meaningful in vertex and<br>
> geometry shaders.  In fragment shaders gl_ClipDistance isn't in any<br>
> interface block.<br>
<br>
</div>Oops.  I overlooked that too.<br>
<br>
Reviewed-by: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a>><br>
<br>
Without this fix, does this test pass on NVIDIA?<br></blockquote><div><br></div><div>Oh, argh.  I'm glad you asked, because running the test on NVIDIA revealed that this patch is bogus.  Instead of replacing<br><br></div>
<div>in gl_PerVertex {<br></div><div>  float gl_ClipDistance[8];<br></div><div>};<br><br></div><div>with:<br><br></div><div>float gl_ClipDistance[8];<br><br></div><div>I should have replaced it with:<br><br></div><div>in float gl_ClipDistance[8];<br>
<br><br></div><div>NVIDIA's linux driver (version 313.18) behaves as follows:<br></div><div><br></div><div>With piglit master, the compiler accepts the shader but the test fails.<br></div><div>With the bogus patch, the compiler accepts the shader but the test fails.<br>
</div><div>With a corrected patch, the compiler accepts the shader and the test passes.<br></div><div><br></div><div>Mesa (using my current work-in-progress "gs" branch) behaves as follows:<br><br></div><div>With piglit master, it crashes due to known bugs in my work-in-progress branch.<br>
</div><div>With the bogus patch, the compiler accepts the shader and the test passes, due to a previously unknown bug on Mesa master.<br></div><div>With the corrected patch, the compiler accepts the shader and the test passes.<br>
<br></div><div><br></div><div>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. <br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div class="HOEnZb"><div class="h5"><br>
> ---<br>
>  .../glsl-1.50/execution/geometry/clip-distance-bulk-copy.shader_test  | 4 +---<br>
>  1 file changed, 1 insertion(+), 3 deletions(-)<br>
><br>
> 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<br>
> index ef21260..856f436 100644<br>
> --- a/tests/spec/glsl-1.50/execution/geometry/clip-distance-bulk-copy.shader_test<br>
> +++ b/tests/spec/glsl-1.50/execution/geometry/clip-distance-bulk-copy.shader_test<br>
> @@ -56,9 +56,7 @@ void main()<br>
>  [fragment shader]<br>
>  #version 150<br>
><br>
> -in gl_PerVertex {<br>
> -  float gl_ClipDistance[8];<br>
> -};<br>
> +float gl_ClipDistance[8];<br>
>  in float offset_to_fs;<br>
><br>
>  void main()<br>
><br>
<br>
</div></div></blockquote></div><br></div></div>