[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