On Wed, 2015-11-25 at 21:36 +0100, gregory hainaut wrote:
> On Wed, 25 Nov 2015 13:45:51 +1100
> Timothy Arceri <timothy.arc...@collabora.com> wrote:
>
> > From: Timothy Arceri <t_arc...@yahoo.com.au>
> >
> > This tests a bug in Mesa where explicit locations are not taken
> > into
> > account when assigning varying locations which results in two
> > inputs/outputs being given the same location.
> >
> > Test results:
> > Nvidia GeForce 840M - NVIDIA 352.41: pass
> > i965 - Mesa 11.1-dev: fail
> >
> > V3:
> > - use helper function to format and link shaders
> > - add array and arrays of arrays subtests
> > - allow test to be run concurrently
> >
> > V2: use pick_a_glsl_version() helper
> >
> > Cc: Gregory Hainaut <gregory.hain...@gmail.com>
> > ---
> > tests/all.py | 2 +
> > .../arb_separate_shader_objects/CMakeLists.gl.txt | 1 +
> > .../mixed_explicit_and_non_explicit_locations.c | 343
> > +++++++++++++++++++++
> > 3 files changed, 346 insertions(+)
> > create mode 100644
> > tests/spec/arb_separate_shader_objects/mixed_explicit_and_non_expli
> > cit_locations.c
> >
> > diff --git a/tests/all.py b/tests/all.py
> > index 07e3599..7689796 100644
> > --- a/tests/all.py
> > +++ b/tests/all.py
> > @@ -2161,6 +2161,8 @@ with profile.group_manager(
> > 'Rendezvous by name')
> > g(['arb_separate_shader_object
> > -rendezvous_by_name_interpolation'],
> > 'Rendezvous by name with multiple interpolation qualifier')
> > + g(['arb_separate_shader_object
> > -mixed_explicit_and_non_explicit_locations'],
> > + 'Mixed explicit and non-explicit locations')
> > g(['arb_separate_shader_object-rendezvous_by_location', '
> > -fbo'],
> > 'Rendezvous by location', run_concurrent=False)
> > g(['arb_separate_shader_object-rendezvous_by_location-5
> > -stages'],
> > diff --git
> > a/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt
> > b/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt
> > index b4fce73..8e4011f 100644
> > --- a/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt
> > +++ b/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt
> > @@ -15,6 +15,7 @@ piglit_add_executable (arb_separate_shader_object
> > -compat-builtins compat-builtin
> > piglit_add_executable (arb_separate_shader_object
> > -explicit_locations_and_transform_feedback
> > explicit_locations_and_transform_feedback.c)
> > 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
> > -mixed_explicit_and_non_explicit_locations
> > mixed_explicit_and_non_explicit_locations.c sso-common.c)
> > piglit_add_executable (arb_separate_shader_object-ProgramUniform
> > -coverage ProgramUniform-coverage.c)
> > piglit_add_executable (arb_separate_shader_object
> > -rendezvous_by_location rendezvous_by_location.c sso-common.c)
> > piglit_add_executable (arb_separate_shader_object
> > -rendezvous_by_location-3-stages rendezvous_by_location-3-stages.c)
> > diff --git
> > a/tests/spec/arb_separate_shader_objects/mixed_explicit_and_non_exp
> > licit_locations.c
> > b/tests/spec/arb_separate_shader_objects/mixed_explicit_and_non_exp
> > licit_locations.c
> > new file mode 100644
> > index 0000000..3bca2bc
> > --- /dev/null
> > +++
> > b/tests/spec/arb_separate_shader_objects/mixed_explicit_and_non_exp
> > licit_locations.c
> > @@ -0,0 +1,343 @@
> > +/*
> > + * Copyright © 2015 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.
> > + */
> > +
> > +/**
> > + * This tests a bug in Mesa where explicit locations are not taken
> > into
> > + * account when assigning varying locations which results in two
> > + * inputs/outputs being given the same location.
> > + */
> > +#include "piglit-util-gl.h"
> > +#include "sso-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 GLuint pipeline = 0;
> > +static GLuint pipeline_arrays = 0;
> > +static GLuint pipeline_arrays_of_arrays = 0;
> > +
> > +static const char *vs_code_template =
> > + "#version %d\n"
> > + "#extension GL_ARB_separate_shader_objects: require\n"
> > + "#extension GL_ARB_explicit_attrib_location: require\n"
> > + "\n"
> > + "layout(location = 0) in vec4 piglit_vertex;\n"
> > + "\n"
> > + "layout(location = 0) out vec3 a;\n"
> > + "out vec3 d;\n"
> > + "out vec3 e;\n"
> > + "layout(location = 1) out vec3 b;\n"
> > + "out vec3 f;\n"
> > + "layout(location = 2) out vec3 c;\n"
> > + "\n"
> > + "void main()\n"
> > + "{\n"
> > + " gl_Position = piglit_vertex;\n"
> > + " a = vec3(0.25, 0, 0);\n"
> > + " b = vec3(0, 0.25, 0);\n"
> > + " c = vec3(0, 0, 0.25);\n"
> > + " d = vec3(0.5, 0, 0);\n"
> > + " e = vec3(0, 0.5, 0);\n"
> > + " f = vec3(0, 0, 0.5);\n"
> > + "}\n"
> > + ;
> > +
> > +static const char *fs_code_template =
> > + "#version %d\n"
> > + "#extension GL_ARB_separate_shader_objects: require\n"
> > + "#extension GL_ARB_explicit_attrib_location: enable\n"
> > + "\n"
> > + "#if __VERSION__ >= 130\n"
> > + "layout(location = 0) out vec4 out_color;\n"
> > + "#else\n"
> > + "#define out_color gl_FragColor\n"
> > + "#endif\n"
> > + "\n"
> > + "layout(location = 0) in vec3 a; /* should get vec3(0.25,
> > 0, 0) */\n"
> > + "in vec3 d; /* should get vec3(0.5,
> > 0, 0) */\n"
> > + "in vec3 e; /* should get vec3(0,
> > 0.5, 0) */\n"
> > + "layout(location = 1) in vec3 b; /* should get vec3(0,
> > 0.25, 0) */\n"
> > + "in vec3 f; /* should get vec3(0, 0,
> > 0.5) */\n"
> > + "layout(location = 2) in vec3 c; /* should get vec3(0, 0,
> > 0.25) */\n"
> > + "\n"
> > + "void main()\n"
> > + "{\n"
> > + " out_color = vec4(a.x + d.x, b.y + e.y, c.z + f.z,
> > 1);\n"
> > + "}\n"
> > + ;
> > +
> > +static const char *vs_arrays_code_template =
> > + "#version %d\n"
> > + "#extension GL_ARB_separate_shader_objects: require\n"
> > + "#extension GL_ARB_explicit_attrib_location: require\n"
> > + "\n"
> > + "layout(location = 0) in vec4 piglit_vertex;\n"
> > + "\n"
> > + "out vec3 c[2];\n"
> > + "layout(location = 1) out vec3 a;\n"
> > + "layout(location = 2) out vec3 b[2];\n"
> > + "out vec3 d;\n"
> > + "\n"
> > + "void main()\n"
> > + "{\n"
> > + " gl_Position = piglit_vertex;\n"
> > + " a = vec3(0.25, 0, 0);\n"
> > + " b[0] = vec3(0, 0.25, 0);\n"
> > + " b[1] = vec3(0, 0, 0.25);\n"
> > + " c[0] = vec3(0.5, 0, 0);\n"
> > + " c[1] = vec3(0, 0.5, 0);\n"
> > + " d = vec3(0, 0, 0.5);\n"
> > + "}\n"
> > + ;
> > +
> > +static const char *fs_arrays_code_template =
> > + "#version %d\n"
> > + "#extension GL_ARB_separate_shader_objects: require\n"
> > + "#extension GL_ARB_explicit_attrib_location: enable\n"
> > + "\n"
> > + "#if __VERSION__ >= 130\n"
> > + "layout(location = 0) out vec4 out_color;\n"
> > + "#else\n"
> > + "#define out_color gl_FragColor\n"
> > + "#endif\n"
> > + "\n"
> > + "in vec3 c[2]; /* should get
> > vec3(0.5, 0, 0)\n"
> > + " * and vec3(0, 0.5,
> > 0)\n"
> > + " */\n"
> > + "layout(location = 1) in vec3 a; /* should get
> > vec3(0.25, 0, 0) */\n"
> > + "layout(location = 2) in vec3 b[2]; /* should get vec3(0,
> > 0.25, 0)\n"
> > + " * and vec3(0, 0,
> > 0.25)\n"
> > + " */\n"
> > + "in vec3 d; /* should get vec3(0,
> > 0, 0.5) */\n"
> > + "\n"
> > + "void main()\n"
> > + "{\n"
> > + " out_color = vec4(a.x + c[0].x, b[0].y + c[1].y,
> > b[1].z + d.z, 1);\n"
> > + "}\n"
> > + ;
> > +
> > +static const char *vs_aoa_code_template =
> > + "#version %d\n"
> > + "#extension GL_ARB_separate_shader_objects: require\n"
> > + "#extension GL_ARB_explicit_attrib_location: require\n"
> > + "#extension GL_ARB_arrays_of_arrays: require\n"
> > + "\n"
> > + "layout(location = 0) in vec4 piglit_vertex;\n"
> > + "\n"
> > + "out vec3 c[2][2];\n"
> > + "layout(location = 2) out vec3 a[2][2];\n"
> > + "out vec3 d;\n"
> > + "layout(location = 8) out vec3 b;\n"
> > + "\n"
> > + "void main()\n"
> > + "{\n"
> > + " gl_Position = piglit_vertex;\n"
> > + " a[0][0] = vec3(0.25, 0, 0);\n"
> > + " a[0][1] = vec3(0, 0.25, 0);\n"
> > + " a[1][0] = vec3(0.125, 0, 0);\n"
> > + " a[1][1] = vec3(0, 0.125, 0);\n"
> > + " b = vec3(0, 0, 0.25);\n"
> > + " c[0][0] = vec3(0.5, 0, 0);\n"
> > + " c[0][1] = vec3(0, 0.5, 0);\n"
> > + " c[1][0] = vec3(0.0625, 0, 0);\n"
> > + " c[1][1] = vec3(0, 0.0625, 0);\n"
> > + " d = vec3(0, 0, 0.5);\n"
> > + "}\n"
> > + ;
> > +
> > +static const char *fs_aoa_code_template =
> > + "#version %d\n"
> > + "#extension GL_ARB_separate_shader_objects: require\n"
> > + "#extension GL_ARB_explicit_attrib_location: enable\n"
> > + "#extension GL_ARB_arrays_of_arrays: require\n"
> > + "\n"
> > + "#if __VERSION__ >= 130\n"
> > + "layout(location = 0) out vec4 out_color;\n"
> > + "#else\n"
> > + "#define out_color gl_FragColor\n"
> > + "#endif\n"
> > + "\n"
> > + "in vec3 c[2][2];\n"
> > + "layout(location = 2) in vec3 a[2][2];\n"
> > + "layout(location = 8) in vec3 b;\n"
> > + "in vec3 d;\n"
> > + "\n"
> > + "void main()\n"
> > + "{\n"
> > + " float red = a[0][0].x + a[1][0].x + c[0][0].x +
> > c[1][0].x;\n"
> > + " float green = a[0][1].y + a[1][1].y + c[0][1].y +
> > c[1][1].y;\n"
> > + " float blue = b.z + d.z;\n"
> > + " out_color = vec4(red, green, blue, 1);\n"
> > + "}\n"
> > + ;
> > +
> > +enum piglit_result
> > +piglit_display(void)
> > +{
> > + static const float expected[] = {
> > + 0.75f, 0.75f, 0.75f, 1.0f
> > + };
> > + static const float expected_aoa[] = {
> > + 0.9375f, 0.9375f, 0.75f, 1.0f
> > + };
> > + bool pass, pass1, pass2, pass3;
> > + int h_width = piglit_width / 2;
> > + int h_height = piglit_height / 2;
> > +
> > + glClearColor(0.1f, 0.1f, 0.1f, 0.1f);
> > + glClear(GL_COLOR_BUFFER_BIT);
> > +
> > + /*
> > + * Test 1: Test for overlap of location assignment for
> > varying.
> > + */
> > + glBindProgramPipeline(pipeline);
> > + piglit_draw_rect(-1, -1, 1, 1);
> > +
> > + /*
> > + * Test 2: Test for overlap of location assignment for
> > varying arrays.
> > + */
> > + if (pipeline_arrays) {
> > + glBindProgramPipeline(pipeline_arrays);
> > + piglit_draw_rect(-1, 0, 1, 1);
> > + }
> > +
> > + /*
> > + * Test 3: Test for overlap of location assignment for
> > varying
> > + * arrays of arrays.
> > + */
> > + if (pipeline_arrays_of_arrays) {
> > + glBindProgramPipeline(pipeline_arrays_of_arrays);
> > + piglit_draw_rect(0, -1, 1, 2);
> > + }
> > +
> > + /*
> > + * Probe and report result
> > + */
> > + pass1 = piglit_probe_rect_rgba(0, 0, h_width, h_height,
> > expected);
> > + pass2 = !pipeline_arrays ||
> > + piglit_probe_rect_rgba(0, h_height, h_width,
> > h_height,
> > + expected);
> > + pass3 = !pipeline_arrays_of_arrays ||
> > + piglit_probe_rect_rgba(h_width, h_height, h_width,
> > h_height,
> > + expected_aoa);
> > +
> > + piglit_present_results();
> > +
> > + pass = pass1 & pass2 & pass3;
> > +
> > + piglit_report_subtest_result(pass1 ? PIGLIT_PASS :
> > PIGLIT_FAIL,
> > + "Varying location assignment overlap");
> > +
> > + if (pipeline_arrays) {
> > + piglit_report_subtest_result(pass2 ? PIGLIT_PASS :
> > PIGLIT_FAIL,
> > + "Varying arrays location assignment
> > overlap");
> > + }
> > +
> > + if (pipeline_arrays_of_arrays) {
> > + piglit_report_subtest_result(pass3 ? PIGLIT_PASS :
> > PIGLIT_FAIL,
> > + "Varying arrays of arrays location
> > assignment "
> > + "overlap");
> > + }
> > +
> > + piglit_present_results();
> > + return pass ? PIGLIT_PASS : PIGLIT_FAIL;
> > +}
> > +
> > +void piglit_init(int argc, char **argv)
> > +{
> > + unsigned glsl_version;
> > + GLuint vs_prog, vs_prog_arrays, vs_prog_arrays_of_arrays;
> > + GLuint fs_prog, fs_prog_arrays, fs_prog_arrays_of_arrays;
> > +
> > + piglit_require_vertex_shader();
> > + piglit_require_fragment_shader();
> > + piglit_require_extension("GL_ARB_separate_shader_objects")
> > ;
> > + piglit_require_extension("GL_ARB_explicit_attrib_location"
> > );
> > +
> > + glsl_version = pick_a_glsl_version();
> > +
> > + /* Link and build pipeline for Varying test */
> > + vs_prog = format_and_link_program(GL_VERTEX_SHADER,
> > vs_code_template,
> > + glsl_version);
> > +
> > + fs_prog = format_and_link_program(GL_FRAGMENT_SHADER,
> > + fs_code_template,
> > glsl_version);
> > +
> > + 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);
> > +
> > + /* Link and build pipeline for Varying arrays test */
> > + if (glsl_version >= 110) {
> > + vs_prog_arrays =
> > format_and_link_program(GL_VERTEX_SHADER,
> > + vs_arrays_
> > code_template,
> > + glsl_versi
> > on);
> > +
> > + fs_prog_arrays =
> > format_and_link_program(GL_FRAGMENT_SHADER,
> > + fs_arrays_
> > code_template,
> > + glsl_versi
> > on);
> > +
> > + glGenProgramPipelines(1, &pipeline_arrays);
> > + glUseProgramStages(pipeline_arrays,
> > + GL_VERTEX_SHADER_BIT,
> > + vs_prog_arrays);
> > + glUseProgramStages(pipeline_arrays,
> > + GL_FRAGMENT_SHADER_BIT,
> > + fs_prog_arrays);
> > + piglit_program_pipeline_check_status(pipeline_arra
> > ys);
> > + }
> > +
> > + /* Link and build pipeline for Varying arrays test */
> > + if
> > (piglit_is_extension_supported("GL_ARB_arrays_of_arrays")) {
> > + vs_prog_arrays_of_arrays =
> > + format_and_link_program(GL_VERTEX_SHADER,
> > + vs_aoa_code_templa
> > te,
> > + glsl_version);
> > +
> > + fs_prog_arrays_of_arrays =
> > + format_and_link_program(GL_FRAGMENT_SHADER
> > ,
> > + fs_aoa_code_templa
> > te,
> > + glsl_version);
> > +
> > + glGenProgramPipelines(1,
> > &pipeline_arrays_of_arrays);
> > + glUseProgramStages(pipeline_arrays_of_arrays,
> > + GL_VERTEX_SHADER_BIT,
> > + vs_prog_arrays_of_arrays);
> > + glUseProgramStages(pipeline_arrays_of_arrays,
> > + GL_FRAGMENT_SHADER_BIT,
> > + fs_prog_arrays_of_arrays);
> > + piglit_program_pipeline_check_status(pipeline_arra
> > ys_of_arrays);
> > + }
> > +
> > + if (!piglit_check_gl_error(0))
> > + piglit_report_result(PIGLIT_FAIL);
> > +}
>
> Hello Timothy,
>
> There are a couple of indentation issue (in shader code template).
Will fix.
>
> IMHO, it would be better to just request GLSL 110 (or the equivalent
> gl
> version).
Whoops that should be glsl_version > 110 for the arrays test. But yes I
think your right we need a minimum of 120 to use the in/out in place of
varying anyway.
Will change to make the test require 120 and drop the extra checks for
the arrays shaders.
> As a side note, a nice common function to Generate, Use VS/FS,
> check the pipeline state could be a good idea too.
Will do this too.
>
> Otherwise, it seems good to me.
Thanks for taking a look.
>
> Best regards,
> Gregory
> _______________________________________________
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
_______________________________________________
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
