[Piglit] [PATCH] arb_shader_atomic_counters-semantics: test different binding points

Ian Romanick idr at freedesktop.org
Thu May 14 11:18:49 PDT 2015


On 04/28/2015 12:57 AM, Alejandro PiƱeiro wrote:
> Piglit was not detecting that non-zero bindings was not properly
> working.
> ---
> 
> Mesa patch and bug information here:
> http://lists.freedesktop.org/archives/mesa-dev/2015-April/082736.html
> 
> Note that until this patch gets reviewed and committed, the subtests
> added on this commit would fail.
> 
> On the thread it was suggested to add the new subtests
> at buffer-binding, as it already includes glBindBufferBase and
> glBindBufferRange tests. But this set of tests doesn't include
> any comparison with the image and final value of the atomics, that
> was how the bug was detected.
> 
> For that reason I added the subtests at semantics.c. I keep the
> previous subtests, and add some extra using a non-zero binding.
> 
> 
>  tests/spec/arb_shader_atomic_counters/semantics.c | 106 +++++++++++++++-------
>  1 file changed, 74 insertions(+), 32 deletions(-)
> 
> diff --git a/tests/spec/arb_shader_atomic_counters/semantics.c b/tests/spec/arb_shader_atomic_counters/semantics.c
> index 694f1fe..42679e5 100644
> --- a/tests/spec/arb_shader_atomic_counters/semantics.c
> +++ b/tests/spec/arb_shader_atomic_counters/semantics.c
> @@ -24,7 +24,8 @@
>  /** @file semantics.c
>   *
>   * Tests that the atomic built-in functions have the expected effects
> - * on memory and return the expected results.
> + * on memory and return the expected results, with different binding
> + * points.
>   */
>  
>  #include "common.h"
> @@ -40,7 +41,7 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
>  PIGLIT_GL_TEST_CONFIG_END
>  
>  static bool
> -run_test_vertex(void)
> +run_test_vertex(GLuint binding)
>  {
>          const char *fs_source = "#version 140\n"
>                  "flat in ivec4 vcolor;\n"
> @@ -48,10 +49,10 @@ run_test_vertex(void)
>                  "void main() {\n"
>                  "       fcolor = vcolor;\n"
>                  "}\n";
> -        const char *vs_source = "#version 140\n"
> +        const char *vs_template = "#version 140\n"
>                  "#extension GL_ARB_shader_atomic_counters : enable\n"
>                  "\n"
> -                "layout(binding = 0, offset = 0) uniform atomic_uint x;\n"
> +                "layout(binding = %i, offset = 0) uniform atomic_uint x;\n"
>                  "in vec4 piglit_vertex;\n"
>                  "flat out ivec4 vcolor;\n"
>                  "\n"
> @@ -67,25 +68,33 @@ run_test_vertex(void)
>          const uint32_t expected_color[] = { 0xfffffffe, 0xfffffffe,
>                                              0xffffffff, 0x0 };
>          GLuint prog = glCreateProgram();
> -        bool ret =
> -                atomic_counters_compile(prog, GL_FRAGMENT_SHADER, fs_source) &&
> -                atomic_counters_compile(prog, GL_VERTEX_SHADER, vs_source) &&
> -                atomic_counters_draw_point(prog, 1, start_buffer) &&
> -                piglit_probe_rect_rgba_uint(0, 0, 1, 1, expected_color) &&
> -                atomic_counters_probe_buffer(0, 1, expected_buffer);
> +        bool ret;
> +        char *vs_source;
> +        bool asprintf_ret;
> +
> +        asprintf_ret = asprintf(&vs_source, vs_template, binding);
> +        assert(asprintf_ret > 0);
> +
> +        ret =
> +          atomic_counters_compile(prog, GL_FRAGMENT_SHADER, fs_source) &&
> +          atomic_counters_compile(prog, GL_VERTEX_SHADER, vs_source) &&
> +          atomic_counters_draw_point(prog, 1, start_buffer) &&
> +          piglit_probe_rect_rgba_uint(0, 0, 1, 1, expected_color) &&
> +          atomic_counters_probe_buffer(0, 1, expected_buffer);
>  
> +        free(vs_source);
>          glDeleteProgram(prog);
>          return ret;
>  }
>  
>  static bool
> -run_test_fragment(void)
> +run_test_fragment(GLuint binding)
>  {
> -        const char *fs_source = "#version 140\n"
> +        const char *fs_template = "#version 140\n"
>                  "#extension GL_ARB_shader_atomic_counters : enable\n"
>                  "\n"
>                  "out ivec4 fcolor;\n"
> -                "layout(binding = 0, offset = 0) uniform atomic_uint x;\n"
> +                "layout(binding = %i, offset = 0) uniform atomic_uint x;\n"
>                  "\n"
>                  "void main() {\n"
>                  "       fcolor.x = int(atomicCounterDecrement(x));\n"
> @@ -106,19 +115,27 @@ run_test_fragment(void)
>          const uint32_t expected_color[] = { 0xfffffffe, 0xfffffffe,
>                                              0xffffffff, 0x0 };
>          GLuint prog = glCreateProgram();
> -        bool ret =
> -                atomic_counters_compile(prog, GL_FRAGMENT_SHADER, fs_source) &&
> -                atomic_counters_compile(prog, GL_VERTEX_SHADER, vs_source) &&
> -                atomic_counters_draw_point(prog, 1, start_buffer) &&
> -                piglit_probe_rect_rgba_uint(0, 0, 1, 1, expected_color) &&
> -                atomic_counters_probe_buffer(0, 1, expected_buffer);
> +        bool ret;
> +        char *fs_source;
> +        bool asprintf_ret;
> +
> +        asprintf_ret = asprintf(&fs_source, fs_template, binding);
> +        assert(asprintf_ret > 0);
>  
> +        ret =
> +          atomic_counters_compile(prog, GL_FRAGMENT_SHADER, fs_source) &&
> +          atomic_counters_compile(prog, GL_VERTEX_SHADER, vs_source) &&
> +          atomic_counters_draw_point(prog, 1, start_buffer) &&
> +          piglit_probe_rect_rgba_uint(0, 0, 1, 1, expected_color) &&
> +          atomic_counters_probe_buffer(0, 1, expected_buffer);
> +
> +        free(fs_source);
>          glDeleteProgram(prog);
>          return ret;
>  }
>  
>  static bool
> -run_test_geometry(void)
> +run_test_geometry(GLuint binding)
>  {
>          const char *fs_source = "#version 140\n"
>                  "flat in ivec4 gcolor;\n"
> @@ -126,7 +143,7 @@ run_test_geometry(void)
>                  "void main() {\n"
>                  "       fcolor = gcolor;\n"
>                  "}\n";
> -        const char *gs_source = "#version 150\n"
> +        const char *gs_template = "#version 150\n"
>                  "#extension GL_ARB_shader_atomic_counters : enable\n"
>                  "\n"
>                  "layout(points) in;\n"
> @@ -134,7 +151,7 @@ run_test_geometry(void)
>                  "\n"
>                  "flat out ivec4 gcolor;\n"
>                  "\n"
> -                "layout(binding = 0, offset = 0) uniform atomic_uint x;\n"
> +                "layout(binding = %i, offset = 0) uniform atomic_uint x;\n"
>                  "\n"
>                  "void main() {\n"
>                  "       gl_Position = gl_in[0].gl_Position;\n"
> @@ -157,14 +174,22 @@ run_test_geometry(void)
>          const uint32_t expected_color[] = { 0xfffffffe, 0xfffffffe,
>                                              0xffffffff, 0x0 };
>          GLuint prog = glCreateProgram();
> -        bool ret =
> -                atomic_counters_compile(prog, GL_FRAGMENT_SHADER, fs_source) &&
> -                atomic_counters_compile(prog, GL_GEOMETRY_SHADER, gs_source) &&
> -                atomic_counters_compile(prog, GL_VERTEX_SHADER, vs_source) &&
> -                atomic_counters_draw_point(prog, 1, start_buffer) &&
> -                piglit_probe_rect_rgba_uint(0, 0, 1, 1, expected_color) &&
> -                atomic_counters_probe_buffer(0, 1, expected_buffer);
> +        bool ret;
> +        char *gs_source;
> +        bool asprintf_ret;

asprintf returns int.

> +
> +        asprintf_ret = asprintf(&gs_source, gs_template, binding);
> +        assert(asprintf_ret > 0);

Realistically, this should return some value > strlen(gs_template), but
I guess we're not testing asprintf. :)

>  
> +        ret =
> +          atomic_counters_compile(prog, GL_FRAGMENT_SHADER, fs_source) &&
> +          atomic_counters_compile(prog, GL_GEOMETRY_SHADER, gs_source) &&
> +          atomic_counters_compile(prog, GL_VERTEX_SHADER, vs_source) &&
> +          atomic_counters_draw_point(prog, 1, start_buffer) &&
> +          piglit_probe_rect_rgba_uint(0, 0, 1, 1, expected_color) &&
> +          atomic_counters_probe_buffer(0, 1, expected_buffer);
> +
> +        free(gs_source);
>          glDeleteProgram(prog);
>          return ret;
>  }
> @@ -350,15 +375,15 @@ piglit_init(int argc, char **argv)
>  
>          atomic_counters_subtest(&status, GL_FRAGMENT_SHADER,
>                                  "Fragment shader atomic built-in semantics",
> -                                run_test_fragment);
> +                                run_test_fragment, 0);
>  
>          atomic_counters_subtest(&status, GL_VERTEX_SHADER,
>                                  "Vertex shader atomic built-in semantics",
> -                                run_test_vertex);
> +                                run_test_vertex, 0);
>  
>          atomic_counters_subtest(&status, GL_GEOMETRY_SHADER,
>                                  "Geometry shader atomic built-in semantics",
> -                                run_test_geometry);
> +                                run_test_geometry, 0);
>  
>          atomic_counters_subtest(&status, GL_TESS_CONTROL_SHADER,
>                                  "Tessellation control shader atomic built-in "
> @@ -370,6 +395,23 @@ piglit_init(int argc, char **argv)
>                                  "semantics",
>                                  run_test_tess_evaluation);
>  

This should check the value of GL_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS.
The minimum value is 1, so this could fail on some valid
implementations.  I think it would be better to just try all the values.

    GLint max_bindings;

    glGetIntegerv(GL_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS, &max_bindings);
    for (i = 0; i < max_bindings; i++) {
        ...
    }

Maybe clamp the maximum iterations to 8 or 16 in case someone advertises
a limit of MAXINT or something.

> +        glBindBufferBase(GL_ATOMIC_COUNTER_BUFFER, 1, buffer);
> +
> +        atomic_counters_subtest(&status, GL_FRAGMENT_SHADER,
> +                                "Fragment shader atomic built-in semantics "
> +                                "with binding non-zero",
> +                                run_test_fragment, 1);
> +
> +        atomic_counters_subtest(&status, GL_VERTEX_SHADER,
> +                                "Vertex shader atomic built-in semantics "
> +                                "with binding non-zero",
> +                                run_test_vertex, 1);
> +
> +        atomic_counters_subtest(&status, GL_GEOMETRY_SHADER,
> +                                "Geometry shader atomic built-in semantics "
> +                                "with binding non-zero",
> +                                run_test_geometry, 1);
> +
>          piglit_report_result(status);
>  }
>  
> 



More information about the Piglit mailing list