[Piglit] [PATCH 6/6] gs: Adapt xfb/intervening-read.c for geometry shaders.

Paul Berry stereotype441 at gmail.com
Fri Aug 9 10:48:14 PDT 2013


On 8 August 2013 15:02, Ian Romanick <idr at freedesktop.org> wrote:

> On 08/07/2013 01:28 PM, Paul Berry wrote:
>
>> The transform feedback test "intervening-read" can now be invoked with
>> a "use_gs" argument, that causes it to use a geometry shader.
>> ---
>>   tests/all.tests                                    |   7 +-
>>   .../spec/ext_transform_**feedback/intervening-read.c | 129
>> +++++++++++++++++++--
>>   .../ext_transform_feedback/**overflow-edge-cases.c   |   2 +-
>>   tests/util/piglit-framework-**gl.c                   |   3 +-
>>   4 files changed, 126 insertions(+), 15 deletions(-)
>>
>> diff --git a/tests/all.tests b/tests/all.tests
>> index b1df541..2e483ef 100644
>> --- a/tests/all.tests
>> +++ b/tests/all.tests
>> @@ -2024,9 +2024,10 @@ for mode in ['main_binding', 'indexed_binding',
>> 'buffer_start', 'buffer_size']:
>>                   'ext_transform_feedback-{0}'.**format(test_name))
>>   ext_transform_feedback['**immediate-reuse'] = concurrent_test('ext_**
>> transform_feedback-immediate-**reuse')
>>   for mode in ['output', 'prims_generated', 'prims_written']:
>> -        test_name = 'intervening-read {0}'.format(mode)
>> -        ext_transform_feedback[test_**name] = concurrent_test(
>> -                'ext_transform_feedback-{0}'.**format(test_name))
>> +        for use_gs in ['', ' use_gs']:
>> +                test_name = 'intervening-read {0}{1}'.format(mode,
>> use_gs)
>> +                ext_transform_feedback[test_**name] = concurrent_test(
>> +                        'ext_transform_feedback-{0}'.**
>> format(test_name))
>>   ext_transform_feedback['max-**varyings'] = concurrent_test('ext_**
>> transform_feedback-max-**varyings')
>>   ext_transform_feedback['**nonflat-integral'] = concurrent_test('ext_**
>> transform_feedback-nonflat-**integral')
>>   ext_transform_feedback['**overflow-edge-cases'] = concurrent_test('ext_
>> **transform_feedback-overflow-**edge-cases')
>> diff --git a/tests/spec/ext_transform_**feedback/intervening-read.c
>> b/tests/spec/ext_transform_**feedback/intervening-read.c
>> index 7d7eece..a6baaf0 100644
>> --- a/tests/spec/ext_transform_**feedback/intervening-read.c
>> +++ b/tests/spec/ext_transform_**feedback/intervening-read.c
>> @@ -44,14 +44,30 @@
>>    * but it requests that no more than 9 vertices be written to it.
>>    * This allows us to verify that the intervening glReadPixels call
>>    * doesn't interfere with overflow checking.
>> + *
>> + * The optional argument "use_gs" causes the test to use a geometry
>> + * shader.  When this argument is given, the number of vertices output
>> + * by the geometry shader is in general different from the number of
>> + * vertices sent down the pipeline by the glDrawArrays() command.
>> + * Thus, the test verifies that the implementation uses the
>> + * post-geometry-shader vertex count to figure out where to resume
>> + * transform feedback after the glReadPixels call.
>>    */
>>
>>   #include "piglit-util-gl-common.h"
>>
>> +static bool use_gs;
>> +
>>   PIGLIT_GL_TEST_CONFIG_BEGIN
>>
>> -       config.supports_gl_compat_**version = 10;
>> -       config.supports_gl_core_**version = 31;
>> +       use_gs = PIGLIT_STRIP_ARG("use_gs");
>> +       if (use_gs) {
>> +               config.supports_gl_compat_**version = 32;
>> +               config.supports_gl_core_**version = 32;
>> +       } else {
>> +               config.supports_gl_compat_**version = 10;
>> +               config.supports_gl_core_**version = 31;
>> +       }
>>
>>         config.window_width = 64;
>>         config.window_height = 32;
>> @@ -65,7 +81,10 @@ static enum test_mode {
>>         TEST_MODE_PRIMS_WRITTEN
>>   } test_mode;
>>
>> -static const char *vstext =
>> +/**
>> + * Vertex shader used when use_gs is false.
>> + */
>> +static const char *vstext_nogs =
>>         "attribute vec4 in_position;\n"
>>         "attribute vec4 in_color;\n"
>>         "varying vec4 out_position;\n"
>> @@ -78,7 +97,10 @@ static const char *vstext =
>>         "  out_color = in_color;\n"
>>         "}\n";
>>
>> -static const char *fstext =
>> +/**
>> + * Fragment shader used when use_gs is false.
>> + */
>> +static const char *fstext_nogs =
>>         "varying vec4 out_color;\n"
>>         "\n"
>>         "void main()\n"
>> @@ -86,6 +108,73 @@ static const char *fstext =
>>         "  gl_FragColor = out_color;\n"
>>         "}\n";
>>
>> +/**
>> + * Vertex shader used when use_gs is true.
>> + */
>> +static const char *vstext_gs =
>> +       "#version 150\n"
>> +       "in vec4 in_color;\n"
>> +       "out vec4 color_to_gs;\n"
>> +       "\n"
>> +       "void main()\n"
>> +       "{\n"
>> +       "  color_to_gs = in_color;\n"
>> +       "}\n";
>> +
>> +/**
>> + * Geometry shader used when use_gs is true.
>> + */
>> +static const char *gstext_gs =
>> +       "#version 150\n"
>> +       "layout(points) in;\n"
>> +       "layout(triangle_strip, max_vertices=6) out;\n"
>> +       "uniform int start_index;\n"
>> +       "in vec4 color_to_gs[1];\n"
>> +       "out vec4 out_position;\n"
>> +       "out vec4 out_color;\n"
>> +       "\n"
>> +       "void main()\n"
>> +       "{\n"
>> +       "  vec2 positions[12] = vec2[12](\n"
>>
>
> I might be inclined to use
>
>         "  const vec2 positions[] = vec2[12](\n"
>
> But that's a nearly infinitesimal nit.


I can go along with that.


>
>
>  +       "    vec2(-1.0, -1.0),\n"
>> +       "    vec2( 0.0, -1.0),\n"
>> +       "    vec2(-1.0,  1.0),\n"
>> +       "    vec2(-1.0,  1.0),\n"
>> +       "    vec2( 0.0, -1.0),\n"
>> +       "    vec2( 0.0,  1.0),\n"
>> +       "    vec2( 0.0, -1.0),\n"
>> +       "    vec2( 1.0, -1.0),\n"
>> +       "    vec2( 0.0,  1.0),\n"
>> +       "    vec2( 0.0,  1.0),\n"
>> +       "    vec2( 1.0, -1.0),\n"
>> +       "    vec2( 1.0,  1.0)\n"
>> +       "  );\n"
>> +       "  int index = start_index;\n"
>> +       "  for (int i = 0; i < 6; i++) {\n"
>> +       "    for (int j = 0; j < 3; j++) {\n"
>> +       "      vec4 position = vec4(positions[index], 0.0, 1.0);\n"
>>
>
> positions only has elements [0..11], but it looks like the loop will
> access elements [start_index..start_index+17].  Am I reading this right?


Whoops, that first loop was supposed to be "for (int i = 0; i < 2; i++)".
The only reason the test wasn't failing was because the geometry shader
declared max_vertices=6, and my work-in-progress i965 gs implementation
discards extra vertices beyond max_vertices.  I'll fix that.


>
>
>  +       "      gl_Position = position;\n"
>> +       "      out_position = position;\n"
>> +       "      out_color = color_to_gs[0];\n"
>> +       "      EmitVertex();\n"
>> +       "      index++;\n"
>> +       "    }\n"
>> +       "    EndPrimitive();\n"
>> +       "  }\n"
>> +       "}\n";
>> +
>> +/**
>> + * Fragment shader used when use_gs is false.
>> + */
>> +static const char *fstext_gs =
>> +       "#version 150\n"
>> +       "in vec4 out_color;\n"
>> +       "\n"
>> +       "void main()\n"
>> +       "{\n"
>> +       "  gl_FragColor = out_color;\n"
>> +       "}\n";
>> +
>>   static const char *varyings[] = { "out_position", "out_color" };
>>
>>   static GLuint xfb_buf, vao, array_buf;
>> @@ -106,7 +195,7 @@ print_usage_and_exit(char *prog_name)
>>   void
>>   piglit_init(int argc, char **argv)
>>   {
>> -       GLuint vs, fs;
>> +       GLuint vs, gs, fs;
>>
>>         /* Interpret command line args */
>>         if (argc != 2)
>> @@ -123,12 +212,22 @@ piglit_init(int argc, char **argv)
>>         piglit_require_GLSL();
>>         piglit_require_transform_**feedback();
>>
>> -       vs = piglit_compile_shader_text(GL_**VERTEX_SHADER, vstext);
>> -       fs = piglit_compile_shader_text(GL_**FRAGMENT_SHADER, fstext);
>> +       if (use_gs) {
>> +               vs = piglit_compile_shader_text(GL_**VERTEX_SHADER,
>> vstext_gs);
>> +               gs = piglit_compile_shader_text(GL_**GEOMETRY_SHADER,
>> gstext_gs);
>> +               fs = piglit_compile_shader_text(GL_**FRAGMENT_SHADER,
>> fstext_gs);
>> +       } else {
>> +               vs = piglit_compile_shader_text(GL_**VERTEX_SHADER,
>> vstext_nogs);
>> +               fs = piglit_compile_shader_text(GL_**FRAGMENT_SHADER,
>> +                                               fstext_nogs);
>> +       }
>>         prog = glCreateProgram();
>>         glAttachShader(prog, vs);
>> +       if (use_gs)
>> +               glAttachShader(prog, gs);
>>         glAttachShader(prog, fs);
>> -       glBindAttribLocation(prog, 0, "in_position");
>> +       if (!use_gs)
>> +               glBindAttribLocation(prog, 0, "in_position");
>>         glBindAttribLocation(prog, 1, "in_color");
>>         glTransformFeedbackVaryings(**prog, 2, varyings,
>> GL_INTERLEAVED_ATTRIBS);
>>         glLinkProgram(prog);
>> @@ -223,14 +322,24 @@ piglit_display(void)
>>         }
>>
>>         /* First draw call */
>> -       glDrawArrays(GL_TRIANGLES, 0, 6);
>> +       if (use_gs) {
>> +               glUniform1i(**glGetUniformLocation(prog, "start_index"),
>> 0);
>> +               glDrawArrays(GL_POINTS, 0, 1);
>> +       } else {
>> +               glDrawArrays(GL_TRIANGLES, 0, 6);
>> +       }
>>
>>         /* Read pixels */
>>         pass = piglit_probe_rect_rgba(0, 0, piglit_width / 2,
>> piglit_height,
>>                                       vertex_input[0].color) && pass;
>>
>>         /* Second draw call */
>> -       glDrawArrays(GL_TRIANGLES, 6, 6);
>> +       if (use_gs) {
>> +               glUniform1i(**glGetUniformLocation(prog, "start_index"),
>> 6);
>> +               glDrawArrays(GL_POINTS, 6, 1);
>> +       } else {
>> +               glDrawArrays(GL_TRIANGLES, 6, 6);
>> +       }
>>
>>         /* Finish transform feedback and test correct behavior. */
>>         glEndTransformFeedback();
>> diff --git a/tests/spec/ext_transform_**feedback/overflow-edge-cases.c
>> b/tests/spec/ext_transform_**feedback/overflow-edge-cases.c
>> index e29e20b..32c27bc 100644
>> --- a/tests/spec/ext_transform_**feedback/overflow-edge-cases.c
>> +++ b/tests/spec/ext_transform_**feedback/overflow-edge-cases.c
>> @@ -40,7 +40,7 @@
>>    * - GL_TRANSFORM_FEEDBACK_**PRIMITIVES_WRITTEN is set correctly.
>>    * - GL_PRIMITIVES_GENERATED is set correctly.
>>    *
>> - * The optional argument "use_gs" uses the test to use a geometry
>> + * The optional argument "use_gs" causes the test to use a geometry
>>    * shader.  When this argument is given, the number of vertices output
>>    * by the geometry shader is in general different from the number of
>>    * vertices sent down the pipeline by the glDrawArrays() command.
>>
>
> I think this hunk belongs squashed with patch 4.


Oops, you're right.


>
>
>  diff --git a/tests/util/piglit-framework-**gl.c
>> b/tests/util/piglit-framework-**gl.c
>> index 1ce3a07..85dcac0 100644
>> --- a/tests/util/piglit-framework-**gl.c
>> +++ b/tests/util/piglit-framework-**gl.c
>> @@ -188,7 +188,8 @@ piglit_strip_arg(int *argc, char *argv[], const char
>> *arg)
>>         int i;
>>         for (i = 1; i < *argc; i++) {
>>                 if (!strcmp(argv[i], arg)) {
>> -                       delete_arg(argv, *argc--, i);
>> +                       delete_arg(argv, *argc, i);
>> +                       *argc -= 1;
>>                         return true;
>>                 }
>>         }
>>
>>
> This last hunk seems spurious.
>
>
Actually, it belongs in patch 1.  Since postfix operators take precedence,
"*argc--" gets interpreted as "*(argc--)", which isn't what we want.
Rather than go with the awkward parenthesization "(*argc)--" I decided to
use "-=" to avoid confusion, as process_args() does.

I'll move this hunk to where it belongs.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20130809/cfd0d604/attachment-0001.html>


More information about the Piglit mailing list