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