[Piglit] [PATCH v2] Test that geometry shaders input types must match the primitive mode
Paul Berry
stereotype441 at gmail.com
Tue Sep 3 10:08:43 PDT 2013
On 30 August 2013 17:28, Nicholas Mack <nichmack at gmail.com> wrote:
> v2: condense all previous tests into a single file and move it to correct
> location
> ---
> tests/all.tests | 6 +
> .../glsl-1.50/execution/geometry/CMakeLists.gl.txt | 1 +
> .../execution/geometry/gs-mismatch-prim-type.c | 202
> +++++++++++++++++++++
> 3 files changed, 209 insertions(+)
> create mode 100644
> tests/spec/glsl-1.50/execution/geometry/gs-mismatch-prim-type.c
>
> diff --git a/tests/all.tests b/tests/all.tests
> index 7ab841e..da4e2e7 100644
> --- a/tests/all.tests
> +++ b/tests/all.tests
> @@ -955,6 +955,12 @@ for prim_type in ['GL_POINTS', 'GL_LINE_LOOP',
> 'GL_LINE_STRIP', 'GL_LINES',
> 'glsl-1.50-geometry-primitive-types {0}'.format(
> prim_type))
>
> +for layout_type in ['points', 'lines', 'lines_adjacency', 'triangles',
> + 'triangles_adjacency']:
> + add_concurrent_test(spec['glsl-1.50'],
> + 'glsl-1.50-gs-mismatch-prim-type {0}'.format(
> + layout_type))
> +
> spec['glsl-3.30'] = Group()
> import_glsl_parser_tests(spec['glsl-3.30'],
> os.path.join(testsDir, 'spec', 'glsl-3.30'),
> diff --git a/tests/spec/glsl-1.50/execution/geometry/CMakeLists.gl.txt
> b/tests/spec/glsl-1.50/execution/geometry/CMakeLists.gl.txt
> index 202fcd2..b7965a8 100644
> --- a/tests/spec/glsl-1.50/execution/geometry/CMakeLists.gl.txt
> +++ b/tests/spec/glsl-1.50/execution/geometry/CMakeLists.gl.txt
> @@ -12,3 +12,4 @@ ${OPENGL_glu_LIBRARY}
>
> piglit_add_executable (glsl-1.50-geometry-end-primitive end-primitive.c)
> piglit_add_executable (glsl-1.50-geometry-primitive-types
> primitive-types.c)
> +piglit_add_executable (glsl-1.50-gs-mismatch-prim-type
> gs-mismatch-prim-type.c)
> diff --git
> a/tests/spec/glsl-1.50/execution/geometry/gs-mismatch-prim-type.c
> b/tests/spec/glsl-1.50/execution/geometry/gs-mismatch-prim-type.c
> new file mode 100644
> index 0000000..83edf0a
> --- /dev/null
> +++ b/tests/spec/glsl-1.50/execution/geometry/gs-mismatch-prim-type.c
> @@ -0,0 +1,202 @@
> +/**
> + * Copyright © 2013 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.
> + */
> +
> +/**
> + * From the GLSL 3.2 spec, section 2.12.1 (Geometry Shader Input
> Primitives):
> + *
> + * "If a geometry shader is active, any command that transfers vertices
> to the
> + * GL will generate an INVALID_OPERATION error if the primitive mode
> parameter
> + * is incompatible with the input primitive type of the currently active
> + * program object, as discussed below."
> + *
> + * "Geometry shaders that operate on points are valid only for the POINTS
> + * primitive type."
> + * "Geometry shaders that operate on line segments are valid only for the
> LINES,
> + * LINE_STRIP, and LINE_LOOP primitive types."
> + * "Geometry shaders that operate on line segments with adjacent vertices
> are
> + * valid only for the LINES_ADJACENCY and LINE_STRIP_ADJACENCY primitive
> + * types."
> + * "Geometry shaders that operate on triangles are valid for the
> TRIANGLES,
> + * TRIANGLE_STRIP and TRIANGLE_FAN primitive types."
> + * "Geometry shaders that operate on triangles with adjacent vertices are
> valid
> + * for the TRIANGLES_ADJACENCY and TRIANGLE_STRIP_ADJACENCY primitive
> types."
> + */
> +
> +#include "piglit-util-gl-common.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> + config.supports_gl_compat_version = 32;
> + config.supports_gl_core_version = 32;
> +
> + config.window_visual = PIGLIT_GL_VISUAL_RGB |
> PIGLIT_GL_VISUAL_DOUBLE;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +static const char *vstext =
> + "#version 150\n"
> + "in vec3 vertex;\n"
> + "out vec3 pos;\n"
> + "void main() {\n"
> + " gl_Position = vec4(vertex, 1.);\n"
> + " pos = vertex;\n"
> + "}\n";
> +
> +static const char *gstemplate =
> + "#version 150\n"
> + "#define LAYOUT_TYPE %s\n"
> + "layout(LAYOUT_TYPE) in;\n"
> + "layout(triangles, max_vertices = 3) out;\n"
> + "in vec3 pos[];\n"
> + "void main() {\n"
> + " for(int i = 0; i < pos.length(); i++) {\n"
> + " gl_Position = vec4(pos[i], 1.);\n"
> + " EmitVertex();\n"
> + " }\n"
> + "}\n";
> +
> +static const char *fstext =
> + "#version 150\n"
> + "out vec4 color;\n"
> + "void main() {\n"
> + " color = vec4(1.);\n"
> + "}\n";
>
There's spurious end-of-line whitespace in the declaration of vstext,
gstemplate, and fstext.
> +
> +static GLuint vao;
> +static GLuint vertBuff;
> +static GLuint indexBuf;
> +
> +static GLfloat vertices[] = {
> + -1.0, 1.0, 0.0,
> + 1.0, 1.0, 0.0,
> + 1.0,-1.0, 0.0,
> + -1.0,-1.0, 0.0
> +};
> +static GLsizei vertSize = sizeof(vertices);
> +
> +static GLuint indices[] = {
> + 0, 1, 2, 0, 2, 3
> +};
> +static GLsizei indSize = sizeof(indices);
>
FYI, now that Ian's "draw rect in core profile" series has landed, we
should be able to stop doing this and just use piglit_draw_rect(). All you
need to modify in your shader to make this work is to rename "vertex" to
"piglit_vertex" in your vertex shader.
I won't be a stickler about it, though, since you started writing this test
before Ian's series landed :)
> +
> +static GLuint prog;
> +
> +static const struct test_set
> +{
> + GLenum prim_type;
> + const char *layout_type;
> +} tests[] = {
> + {GL_POINTS, "points"},
> + {GL_LINES, "lines"},
> + {GL_LINE_STRIP, "lines"},
> + {GL_LINE_LOOP, "lines"},
> + {GL_LINES_ADJACENCY, "lines_adjacency"},
> + {GL_LINE_STRIP_ADJACENCY, "lines_adjacency"},
> + {GL_TRIANGLES, "triangles"},
> + {GL_TRIANGLE_STRIP, "triangles"},
> + {GL_TRIANGLE_FAN, "triangles"},
> + {GL_TRIANGLES_ADJACENCY, "triangles_adjacency"},
> + {GL_TRIANGLE_STRIP_ADJACENCY, "triangles_adjacency"}
> +};
> +
> +static char layout[25];
> +
> +void
> +piglit_init(int argc, char **argv)
> +{
> + GLuint vs = 0, gs = 0, fs = 0;
> + char* gstext = NULL;
> + GLuint vertIndex;
> +
> + /* Parse params */
> + if (argc != 2) {
> + printf("%s failed\n", argv[0]);
> + piglit_report_result(PIGLIT_FAIL);
> + }
> +
> + strcpy(layout, argv[1]);
>
Generally we aren't terribly careful about this in piglit because it's just
a testing library, but in any other project this would be a security risk
because if argv[1] is longer than 24 characters, the strcpy() will
overwrite memory belonging to other globals, which could have any effect,
up to and including execution of code from a malicious attacker.
> + if (layout == NULL) {
> + printf("%s failed\n", argv[0]);
> + piglit_report_result(PIGLIT_FAIL);
> + }
>
I'm not sure what you're trying to achieve here. layout can never be null
because it's a static char array.
I'd recommend changing layout to a "const char *", and then just doing
layout = argv[1];
instead of the strcpy().
> +
> + prog = glCreateProgram();
> + vs = piglit_compile_shader_text(GL_VERTEX_SHADER, vstext);
> + asprintf(&gstext, gstemplate, layout);
> + gs = piglit_compile_shader_text(GL_GEOMETRY_SHADER, gstext);
> + fs = piglit_compile_shader_text(GL_FRAGMENT_SHADER, fstext);
> + glAttachShader(prog, vs);
> + glAttachShader(prog, gs);
> + glAttachShader(prog, fs);
> + free(gstext);
> +
> + glLinkProgram(prog);
> + if(!piglit_link_check_status(prog)){
> + glDeleteProgram(prog);
> + piglit_report_result(PIGLIT_FAIL);
> + }
> +
> + glUseProgram(prog);
> +
> + glGenBuffers(1, &vertBuff);
> + glBindBuffer(GL_ARRAY_BUFFER, vertBuff);
> + glBufferData(GL_ARRAY_BUFFER, vertSize, vertices, GL_STATIC_DRAW);
> +
> + glGenBuffers(1, &indexBuf);
> + glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, indexBuf);
> + glBufferData(GL_ELEMENT_ARRAY_BUFFER, indSize,
> + indices, GL_STATIC_DRAW);
> +
> + glGenVertexArrays(1, &vao);
> + glBindVertexArray(vao);
> +
> + vertIndex = glGetAttribLocation(prog, "vertex");
> +
> + glBindBuffer(GL_ARRAY_BUFFER, vertBuff);
> + glEnableVertexAttribArray(vertIndex);
> + glVertexAttribPointer(vertIndex, 3, GL_FLOAT, GL_FALSE, 0, 0);
> +}
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> + bool pass = true;
> + int i = 0;
> +
> + glClearColor(0.2, 0.2, 0.2, 1.0);
> + glClear(GL_COLOR_BUFFER_BIT);
> +
> + glBindVertexArray(vao);
> + glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, indexBuf);
> +
> + for(i = 0; i < ARRAY_SIZE(tests); i++) {
> + glDrawElements(tests[i].prim_type, ARRAY_SIZE(indices),
> + GL_UNSIGNED_INT, NULL);
> +
> + if(strcmp(layout, tests[i].layout_type) == 0)
> + pass = piglit_check_gl_error(GL_NO_ERROR) && pass;
> + else pass = piglit_check_gl_error(GL_INVALID_OPERATION) &
> pass;
>
There should be a newline after the "else".
> + }
> +
> + return pass ? PIGLIT_PASS : PIGLIT_FAIL;
> +}
> --
> 1.8.3.1
>
With everything but the "draw rect" issue fixed, the patch is:
Reviewed-by: Paul Berry <stereotype441 at gmail.com>
Bonus points if you fix the "draw rect" issue too :)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20130903/22b8f905/attachment-0001.html>
More information about the Piglit
mailing list