<div dir="ltr">On 30 August 2013 17:28, Nicholas Mack <span dir="ltr"><<a href="mailto:nichmack@gmail.com" target="_blank">nichmack@gmail.com</a>></span> wrote:<br><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">
v2: condense all previous tests into a single file and move it to correct location<br>
---<br>
tests/all.tests | 6 +<br>
.../glsl-1.50/execution/geometry/CMakeLists.gl.txt | 1 +<br>
.../execution/geometry/gs-mismatch-prim-type.c | 202 +++++++++++++++++++++<br>
3 files changed, 209 insertions(+)<br>
create mode 100644 tests/spec/glsl-1.50/execution/geometry/gs-mismatch-prim-type.c<br>
<br>
diff --git a/tests/all.tests b/tests/all.tests<br>
index 7ab841e..da4e2e7 100644<br>
--- a/tests/all.tests<br>
+++ b/tests/all.tests<br>
@@ -955,6 +955,12 @@ for prim_type in ['GL_POINTS', 'GL_LINE_LOOP', 'GL_LINE_STRIP', 'GL_LINES',<br>
'glsl-1.50-geometry-primitive-types {0}'.format(<br>
prim_type))<br>
<br>
+for layout_type in ['points', 'lines', 'lines_adjacency', 'triangles',<br>
+ 'triangles_adjacency']:<br>
+ add_concurrent_test(spec['glsl-1.50'],<br>
+ 'glsl-1.50-gs-mismatch-prim-type {0}'.format(<br>
+ layout_type))<br>
+<br>
spec['glsl-3.30'] = Group()<br>
import_glsl_parser_tests(spec['glsl-3.30'],<br>
os.path.join(testsDir, 'spec', 'glsl-3.30'),<br>
diff --git a/tests/spec/glsl-1.50/execution/geometry/CMakeLists.gl.txt b/tests/spec/glsl-1.50/execution/geometry/CMakeLists.gl.txt<br>
index 202fcd2..b7965a8 100644<br>
--- a/tests/spec/glsl-1.50/execution/geometry/CMakeLists.gl.txt<br>
+++ b/tests/spec/glsl-1.50/execution/geometry/CMakeLists.gl.txt<br>
@@ -12,3 +12,4 @@ ${OPENGL_glu_LIBRARY}<br>
<br>
piglit_add_executable (glsl-1.50-geometry-end-primitive end-primitive.c)<br>
piglit_add_executable (glsl-1.50-geometry-primitive-types primitive-types.c)<br>
+piglit_add_executable (glsl-1.50-gs-mismatch-prim-type gs-mismatch-prim-type.c)<br>
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<br>
new file mode 100644<br>
index 0000000..83edf0a<br>
--- /dev/null<br>
+++ b/tests/spec/glsl-1.50/execution/geometry/gs-mismatch-prim-type.c<br>
@@ -0,0 +1,202 @@<br>
<div><div class="h5">+/**<br>
+ * Copyright © 2013 Intel Corporation<br>
+ *<br>
+ * Permission is hereby granted, free of charge, to any person obtaining a<br>
+ * copy of this software and associated documentation files (the "Software"),<br>
+ * to deal in the Software without restriction, including without limitation<br>
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,<br>
+ * and/or sell copies of the Software, and to permit persons to whom the<br>
+ * Software is furnished to do so, subject to the following conditions:<br>
+ *<br>
+ * The above copyright notice and this permission notice (including the next<br>
+ * paragraph) shall be included in all copies or substantial portions of the<br>
+ * Software.<br>
+ *<br>
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR<br>
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,<br>
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL<br>
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER<br>
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING<br>
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS<br>
+ * IN THE SOFTWARE.<br>
+ */<br>
+<br>
+/**<br>
+ * From the GLSL 3.2 spec, section 2.12.1 (Geometry Shader Input Primitives):<br>
+ *<br>
+ * "If a geometry shader is active, any command that transfers vertices to the<br>
+ * GL will generate an INVALID_OPERATION error if the primitive mode parameter<br>
+ * is incompatible with the input primitive type of the currently active<br>
+ * program object, as discussed below."<br>
+ *<br>
+ * "Geometry shaders that operate on points are valid only for the POINTS<br>
+ * primitive type."<br>
</div></div><div class="im">+ * "Geometry shaders that operate on line segments are valid only for the LINES,<br>
+ * LINE_STRIP, and LINE_LOOP primitive types."<br>
</div><div class="im">+ * "Geometry shaders that operate on line segments with adjacent vertices are<br>
+ * valid only for the LINES_ADJACENCY and LINE_STRIP_ADJACENCY primitive<br>
+ * types."<br>
</div><div class="im">+ * "Geometry shaders that operate on triangles are valid for the TRIANGLES,<br>
+ * TRIANGLE_STRIP and TRIANGLE_FAN primitive types."<br>
</div><div class="im">+ * "Geometry shaders that operate on triangles with adjacent vertices are valid<br>
+ * for the TRIANGLES_ADJACENCY and TRIANGLE_STRIP_ADJACENCY primitive types."<br>
+ */<br>
+<br>
+#include "piglit-util-gl-common.h"<br>
+<br>
+PIGLIT_GL_TEST_CONFIG_BEGIN<br>
+<br>
+ config.supports_gl_compat_version = 32;<br>
+ config.supports_gl_core_version = 32;<br>
+<br>
+ config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DOUBLE;<br>
+<br>
+PIGLIT_GL_TEST_CONFIG_END<br>
+<br>
+static const char *vstext =<br>
+ "#version 150\n"<br>
+ "in vec3 vertex;\n"<br>
+ "out vec3 pos;\n"<br>
+ "void main() {\n"<br>
+ " gl_Position = vec4(vertex, 1.);\n"<br>
+ " pos = vertex;\n"<br>
+ "}\n";<br>
+<br>
</div>+static const char *gstemplate =<br>
+ "#version 150\n"<br>
+ "#define LAYOUT_TYPE %s\n"<br>
+ "layout(LAYOUT_TYPE) in;\n"<br>
<div><div class="h5">+ "layout(triangles, max_vertices = 3) out;\n"<br>
+ "in vec3 pos[];\n"<br>
+ "void main() {\n"<br>
+ " for(int i = 0; i < pos.length(); i++) {\n"<br>
+ " gl_Position = vec4(pos[i], 1.);\n"<br>
+ " EmitVertex();\n"<br>
+ " }\n"<br>
+ "}\n";<br>
+<br>
+static const char *fstext =<br>
+ "#version 150\n"<br>
+ "out vec4 color;\n"<br>
+ "void main() {\n"<br>
+ " color = vec4(1.);\n"<br>
+ "}\n";<br></div></div></blockquote><div><br></div><div>There's spurious end-of-line whitespace in the declaration of vstext, gstemplate, and fstext.<br></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">
+<br>
+static GLuint vao;<br>
+static GLuint vertBuff;<br>
+static GLuint indexBuf;<br>
+<br>
+static GLfloat vertices[] = {<br>
+ -1.0, 1.0, 0.0,<br>
+ 1.0, 1.0, 0.0,<br>
+ 1.0,-1.0, 0.0,<br>
+ -1.0,-1.0, 0.0<br>
+};<br>
+static GLsizei vertSize = sizeof(vertices);<br>
+<br>
+static GLuint indices[] = {<br>
+ 0, 1, 2, 0, 2, 3<br>
+};<br>
+static GLsizei indSize = sizeof(indices);<br></div></div></blockquote><div><br></div><div>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.<br>
<br>I won't be a stickler about it, though, since you started writing this test before Ian's series landed :)<br></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">
+<br>
+static GLuint prog;<br>
+<br>
</div></div>+static const struct test_set<br>
+{<br>
+ GLenum prim_type;<br>
+ const char *layout_type;<br>
+} tests[] = {<br>
+ {GL_POINTS, "points"},<br>
+ {GL_LINES, "lines"},<br>
+ {GL_LINE_STRIP, "lines"},<br>
+ {GL_LINE_LOOP, "lines"},<br>
+ {GL_LINES_ADJACENCY, "lines_adjacency"},<br>
+ {GL_LINE_STRIP_ADJACENCY, "lines_adjacency"},<br>
+ {GL_TRIANGLES, "triangles"},<br>
+ {GL_TRIANGLE_STRIP, "triangles"},<br>
+ {GL_TRIANGLE_FAN, "triangles"},<br>
+ {GL_TRIANGLES_ADJACENCY, "triangles_adjacency"},<br>
+ {GL_TRIANGLE_STRIP_ADJACENCY, "triangles_adjacency"}<br>
+};<br>
+<br>
+static char layout[25];<br>
<div class="im">+<br>
+void<br>
+piglit_init(int argc, char **argv)<br>
+{<br>
+ GLuint vs = 0, gs = 0, fs = 0;<br>
</div>+ char* gstext = NULL;<br>
+ GLuint vertIndex;<br>
+<br>
+ /* Parse params */<br>
+ if (argc != 2) {<br>
+ printf("%s failed\n", argv[0]);<br>
+ piglit_report_result(PIGLIT_FAIL);<br>
+ }<br>
+<br>
+ strcpy(layout, argv[1]);<br></blockquote><div><br></div><div>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.<br>
</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">
+ if (layout == NULL) {<br>
+ printf("%s failed\n", argv[0]);<br>
+ piglit_report_result(PIGLIT_FAIL);<br>
+ }<br></blockquote><div><br></div><div>I'm not sure what you're trying to achieve here. layout can never be null because it's a static char array.<br><br></div><div>I'd recommend changing layout to a "const char *", and then just doing<br>
<br></div><div>layout = argv[1];<br><br>instead of the strcpy().<br></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">
+<br>
<div class="im">+ prog = glCreateProgram();<br>
+ vs = piglit_compile_shader_text(GL_VERTEX_SHADER, vstext);<br>
</div>+ asprintf(&gstext, gstemplate, layout);<br>
<div class="im">+ gs = piglit_compile_shader_text(GL_GEOMETRY_SHADER, gstext);<br>
+ fs = piglit_compile_shader_text(GL_FRAGMENT_SHADER, fstext);<br>
+ glAttachShader(prog, vs);<br>
+ glAttachShader(prog, gs);<br>
+ glAttachShader(prog, fs);<br>
</div>+ free(gstext);<br>
<div><div class="h5">+<br>
+ glLinkProgram(prog);<br>
+ if(!piglit_link_check_status(prog)){<br>
+ glDeleteProgram(prog);<br>
+ piglit_report_result(PIGLIT_FAIL);<br>
+ }<br>
+<br>
+ glUseProgram(prog);<br>
+<br>
+ glGenBuffers(1, &vertBuff);<br>
+ glBindBuffer(GL_ARRAY_BUFFER, vertBuff);<br>
+ glBufferData(GL_ARRAY_BUFFER, vertSize, vertices, GL_STATIC_DRAW);<br>
+<br>
+ glGenBuffers(1, &indexBuf);<br>
+ glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, indexBuf);<br>
+ glBufferData(GL_ELEMENT_ARRAY_BUFFER, indSize,<br>
+ indices, GL_STATIC_DRAW);<br>
+<br>
+ glGenVertexArrays(1, &vao);<br>
+ glBindVertexArray(vao);<br>
+<br>
+ vertIndex = glGetAttribLocation(prog, "vertex");<br>
+<br>
+ glBindBuffer(GL_ARRAY_BUFFER, vertBuff);<br>
+ glEnableVertexAttribArray(vertIndex);<br>
+ glVertexAttribPointer(vertIndex, 3, GL_FLOAT, GL_FALSE, 0, 0);<br>
+}<br>
+<br>
</div></div><div class="im">+enum piglit_result<br>
+piglit_display(void)<br>
+{<br>
+ bool pass = true;<br>
+ int i = 0;<br>
+<br>
+ glClearColor(0.2, 0.2, 0.2, 1.0);<br>
+ glClear(GL_COLOR_BUFFER_BIT);<br>
+<br>
+ glBindVertexArray(vao);<br>
+ glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, indexBuf);<br>
+<br>
</div>+ for(i = 0; i < ARRAY_SIZE(tests); i++) {<br>
+ glDrawElements(tests[i].prim_type, ARRAY_SIZE(indices),<br>
+ GL_UNSIGNED_INT, NULL);<br>
+<br>
+ if(strcmp(layout, tests[i].layout_type) == 0)<br>
<div class="im">+ pass = piglit_check_gl_error(GL_NO_ERROR) && pass;<br>
</div>+ else pass = piglit_check_gl_error(GL_INVALID_OPERATION) & pass;<br></blockquote><div><br></div><div>There should be a newline after the "else".<br></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=""><div class="h5">+ }<br>
+<br>
+ return pass ? PIGLIT_PASS : PIGLIT_FAIL;<br>
+}<br>
--<br>
1.8.3.1<br></div></div></blockquote><div><br></div><div>With everything but the "draw rect" issue fixed, the patch is:<br><br></div><div>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>><br>
<br></div><div>Bonus points if you fix the "draw rect" issue too :)<br></div></div></div></div>