<div dir="ltr">On 8 September 2013 19:42, 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 09/07/2013 09:01 PM, Paul Berry wrote:<br>
> On 4 September 2013 12:57, Ian Romanick <<a href="mailto:idr@freedesktop.org">idr@freedesktop.org</a><br>
</div><div class="im">> <mailto:<a href="mailto:idr@freedesktop.org">idr@freedesktop.org</a>>> wrote:<br>
><br>
> From: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a><br>
</div>> <mailto:<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a>>><br>
<div class="im">><br>
> Separate shader objects can either connect outputs to inputs by matching<br>
> names (rendezvous by name) or by assigning explicit locations via the<br>
> layout(location=...) (rendezvous by location). This is a simple test<br>
> that a vertex shader with a different name than the fragment shader<br>
> input can by matched by the location.<br>
><br>
> v2: Fix some dumb bugs in the test. Use multiple values to ensure we're<br>
> not matching using rendezvous-by-dumb-luck.<br>
><br>
> Signed-off-by: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a><br>
</div>> <mailto:<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a>>><br>
<div><div class="h5">> ---<br>
> tests/all.tests | 1 +<br>
> .../arb_separate_shader_objects/CMakeLists.gl.txt | 1 +<br>
> .../rendezvous_by_location.c | 97<br>
> ++++++++++++++++++++++<br>
> 3 files changed, 99 insertions(+)<br>
> create mode 100644<br>
> tests/spec/arb_separate_shader_objects/rendezvous_by_location.c<br>
><br>
> diff --git a/tests/all.tests b/tests/all.tests<br>
> index ceefaf3..a8e389c 100644<br>
> --- a/tests/all.tests<br>
> +++ b/tests/all.tests<br>
> @@ -1241,6 +1241,7 @@<br>
> arb_separate_shader_objects['GetProgramPipelineiv'] =<br>
> concurrent_test('arb_separ<br>
> arb_separate_shader_objects['IsProgramPipeline'] =<br>
> concurrent_test('arb_separate_shader_object-IsProgramPipeline')<br>
> arb_separate_shader_objects['UseProgramStages - non-separable<br>
> program'] =<br>
> concurrent_test('arb_separate_shader_object-UseProgramStages-non-separable')<br>
> arb_separate_shader_objects['Mix BindProgramPipeline and<br>
> UseProgram'] =<br>
> concurrent_test('arb_separate_shader_object-mix_pipeline_useprogram')<br>
> +arb_separate_shader_objects['Rendezvous by location'] =<br>
> plain_test('arb_separate_shader_object-rendezvous_by_location -fbo')<br>
> arb_separate_shader_objects['ValidateProgramPipeline'] =<br>
> concurrent_test('arb_separate_shader_object-ValidateProgramPipeline')<br>
><br>
> # Group ARB_sampler_objects<br>
> diff --git<br>
> a/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt<br>
> b/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt<br>
> index ad22987..66176f1 100644<br>
> --- a/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt<br>
> +++ b/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt<br>
> @@ -12,5 +12,6 @@ link_libraries (<br>
> piglit_add_executable<br>
> (arb_separate_shader_object-GetProgramPipelineiv GetProgramPipelineiv.c)<br>
> piglit_add_executable (arb_separate_shader_object-IsProgramPipeline<br>
> IsProgramPipeline.c)<br>
> piglit_add_executable<br>
> (arb_separate_shader_object-mix_pipeline_useprogram<br>
> mix_pipeline_useprogram.c)<br>
> +piglit_add_executable<br>
> (arb_separate_shader_object-rendezvous_by_location<br>
> rendezvous_by_location.c)<br>
> piglit_add_executable<br>
> (arb_separate_shader_object-UseProgramStages-non-separable<br>
> UseProgramStages-non-separable.c)<br>
> piglit_add_executable<br>
> (arb_separate_shader_object-ValidateProgramPipeline<br>
> ValidateProgramPipeline.c)<br>
> diff --git<br>
> a/tests/spec/arb_separate_shader_objects/rendezvous_by_location.c<br>
> b/tests/spec/arb_separate_shader_objects/rendezvous_by_location.c<br>
> new file mode 100644<br>
> index 0000000..00a8cf9<br>
> --- /dev/null<br>
> +++ b/tests/spec/arb_separate_shader_objects/rendezvous_by_location.c<br>
> @@ -0,0 +1,97 @@<br>
> +/*<br>
> + * Copyright © 2013 Intel Corporation<br>
> + *<br>
> + * Permission is hereby granted, free of charge, to any person<br>
> obtaining a<br>
> + * copy of this software and associated documentation files (the<br>
> "Software"),<br>
> + * to deal in the Software without restriction, including without<br>
> limitation<br>
> + * the rights to use, copy, modify, merge, publish, distribute,<br>
> sublicense,<br>
> + * and/or sell copies of the Software, and to permit persons to<br>
> whom the<br>
> + * Software is furnished to do so, subject to the following conditions:<br>
> + *<br>
> + * The above copyright notice and this permission notice (including<br>
> the next<br>
> + * paragraph) shall be included in all copies or substantial<br>
> portions of the<br>
> + * Software.<br>
> + *<br>
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,<br>
> EXPRESS OR<br>
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF<br>
> MERCHANTABILITY,<br>
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO<br>
> EVENT SHALL<br>
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,<br>
> DAMAGES OR OTHER<br>
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,<br>
> ARISING<br>
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR<br>
> OTHER DEALINGS<br>
> + * IN THE SOFTWARE.<br>
> + */<br>
> +<br>
> +#include "piglit-util-gl-common.h"<br>
> +<br>
> +PIGLIT_GL_TEST_CONFIG_BEGIN<br>
> +<br>
> + config.supports_gl_compat_version = 10;<br>
> + config.window_visual = PIGLIT_GL_VISUAL_RGB |<br>
> PIGLIT_GL_VISUAL_DOUBLE;<br>
> +<br>
> +PIGLIT_GL_TEST_CONFIG_END<br>
> +<br>
> +static const char *vs_code =<br>
> + "#version 110\n"<br>
> + "#extension GL_ARB_separate_shader_objects: require\n"<br>
> + "layout(location = 2) out vec3 a;\n"<br>
> + "layout(location = 3) out vec3 b;\n"<br>
> + "void main()\n"<br>
> + "{\n"<br>
> + " gl_Position = gl_Vertex;\n"<br>
> + " a = vec3(0, 0, 1);\n"<br>
> + " b = vec3(1, 0, 0);\n"<br>
> + "}\n"<br>
> + ;<br>
> +<br>
> +static const char *fs_code =<br>
> + "#version 110\n"<br>
> + "#extension GL_ARB_separate_shader_objects: require\n"<br>
> + "layout(location = 3) in vec3 a;\n"<br>
> + "layout(location = 2) in vec3 b;\n"<br>
> + "void main()\n"<br>
> + "{\n"<br>
> + " gl_FragColor = vec4(cross(a, b), 1);\n"<br>
><br>
><br>
> This should be cross(b, a). As written you're computing cross(vec3(1,<br>
> 0, 0), vec3(0, 0, 1)), which equals vec3(0, -1, 0).<br>
<br>
</div></div>D'oh... I was thinking in NDC (where positive Z goes away from the<br>
screen) instead of "real" coordinates. Math is hard.<br>
<div class="im"><br>
> You're usually really good about testing your code before submitting it,<br>
> so I'm wondering: does this test pass on catalyst? If so that's really<br>
> bad because it means catalyst is doing rendezvous by name in spite of<br>
> locations being specified.<br>
<br>
</div>I think I had asked Ken to test on AMD (I only have GL 3.3 NVIDIA<br>
hardware handy), but I don't think he ever did. I may have just tested<br>
on Mesa where it spectacularly failed regardless. :)<br>
<div class="im"><br>
> Incidentally, I found that in order to run this test on NVIDIA, I had to<br>
> change the shaders to use #version 140, and I had to replace gl_Vertex<br>
> with a user-defined vertex attribute. I believe those are both<br>
> necessary because of NVIDIA bugs, but it might be nice to go ahead and<br>
> do them anyway, just so that it's easier to validate the test against<br>
> existing implementations. But I don't feel terribly strongly about it.<br>
<br>
</div>You had to replace gl_Vertex, but it let you use gl_FragColor? Would<br>
1.30 also work on NVIDIA? I suspect they don't enable the 'in' and<br>
'out' keywords based on extensions. I'll add separate compile tests for<br>
that. :)<br></blockquote><div><br></div><div>Ok, I investigated more thoroughly. It appears that:<br><br></div><div>- In GLSL 1.40 and above, NVIDIA prohibits the use of gl_Vertex. In GLSL 1.30, gl_Vertex is allowed but generates a warning.<br>
<br></div><div>- gl_FragColor is allowed but generates a warning in both GLSL 1.30 and 1.40.<br><br></div><div>- In GLSL 1.30, NVIDIA handles "in" and "out" just fine, but it fails to parse the "layout" keyword properly. I found a workaround to this, but it's stupid: if I add "#extension GL_ARB_fragment_coord_conventions: require" to *both* the vs and gs, then they can be GLSL 1.30 and they work properly. I also tried using "#extension GL_ARB_explicit_attrib_location: require" instead, and it doesn't work.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
The spec also says that layout() cannot be applied to the old keywords<br>
(attribute or varying).<br>
<br>
Since Mesa is going to enable this extension on all drivers, I'd like to<br>
have as many of the tests run on as many drivers as possible. Very few<br>
Mesa drivers support GLSL 1.40. Maybe I should dynamically set the version?<br></blockquote><div><br></div><div>Given what I've discovered about what the NVIDIA driver allows, I think that's probably the best bet.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
> With the order of arguments to cross() fixed, this patch is:<br>
><br>
> Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a><br>
</div>> <mailto:<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>>><br>
<div><div class="h5">><br>
><br>
> + "}\n"<br>
> + ;<br>
> +<br>
> +enum piglit_result<br>
> +piglit_display(void)<br>
> +{<br>
> + static const float expected[] = {<br>
> + 0.0f, 1.0f, 0.0f, 1.0f<br>
> + };<br>
> + bool pass;<br>
> +<br>
> + piglit_draw_rect(-1, -1, 2, 2);<br>
> + pass = piglit_probe_rect_rgba(0, 0, piglit_width, piglit_height,<br>
> + expected);<br>
> +<br>
> + piglit_present_results();<br>
> + return pass ? PIGLIT_PASS : PIGLIT_FAIL;<br>
> +}<br>
> +<br>
> +void piglit_init(int argc, char **argv)<br>
> +{<br>
> + GLuint vs_prog;<br>
> + GLuint fs_prog;<br>
> + GLuint pipeline;<br>
> +<br>
> + piglit_require_vertex_shader();<br>
> + piglit_require_extension("GL_ARB_separate_shader_objects");<br>
> +<br>
> + vs_prog = glCreateShaderProgramv(GL_VERTEX_SHADER, 1, &vs_code);<br>
> + piglit_link_check_status(vs_prog);<br>
> +<br>
> + fs_prog = glCreateShaderProgramv(GL_FRAGMENT_SHADER, 1,<br>
> &fs_code);<br>
> + piglit_link_check_status(fs_prog);<br>
> +<br>
> + glGenProgramPipelines(1, &pipeline);<br>
> + glUseProgramStages(pipeline, GL_VERTEX_SHADER_BIT, vs_prog);<br>
> + glUseProgramStages(pipeline, GL_FRAGMENT_SHADER_BIT, fs_prog);<br>
> + piglit_program_pipeline_check_status(pipeline);<br>
> +<br>
> + if (!piglit_check_gl_error(0))<br>
> + piglit_report_result(PIGLIT_FAIL);<br>
> +<br>
> + glBindProgramPipeline(pipeline);<br>
> +}<br>
> --<br>
> 1.8.1.4<br>
><br>
> _______________________________________________<br>
> Piglit mailing list<br>
</div></div>> <a href="mailto:Piglit@lists.freedesktop.org">Piglit@lists.freedesktop.org</a> <mailto:<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>
<br>
</blockquote></div><br></div></div>