<div dir="ltr">On 8 August 2013 15:02, Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <div class=""><div class="h5">On 08/07/2013 01:28 PM, Paul Berry wrote: <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> 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" </blockquote> </div></div> I might be inclined to use " const vec2 positions[] = vec2[12](\n" But that's a nearly infinitesimal nit.</blockquote><div> </div><div>I can go along with that. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <div class="im"> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + " 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" </blockquote> </div> 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?</blockquote><div> </div><div>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. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="h5"> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + " 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. </blockquote> </div></div> I think this hunk belongs squashed with patch 4.</blockquote><div> </div><div>Oops, you're right. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <div class="im"> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> 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; } } </blockquote> </div> This last hunk seems spurious. </blockquote></div> </div><div class="gmail_extra">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. </div><div class="gmail_extra">I'll move this hunk to where it belongs. </div></div>