[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