On 20 September 2011 14:51, Eric Anholt <<a href="mailto:eric@anholt.net">eric@anholt.net</a>> wrote: <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"> As fallout, test a couple more combinations. --- tests/all.tests | 29 ++++++----- tests/spec/gl-2.0/vertex-program-two-side.c | 73 +++++++++++++-------------- 2 files changed, 50 insertions(+), 52 deletions(-) diff --git a/tests/all.tests b/tests/all.tests index 94b3184..5404880 100644 --- a/tests/all.tests +++ b/tests/all.tests @@ -732,21 +732,22 @@ add_texwrap_test3(gl20, '2D', 'npot', 'proj') add_texwrap_test2(gl20, '3D', 'npot') add_texwrap_test3(gl20, '3D', 'npot', 'proj') add_plain_test(gl20, 'getattriblocation-conventional') -add_concurrent_test(gl20, 'vertex-program-two-side') +add_concurrent_test(gl20, 'vertex-program-two-side enabled front back front2 back2') +add_concurrent_test(gl20, 'vertex-program-two-side enabled front back') +add_concurrent_test(gl20, 'vertex-program-two-side enabled front2 back2') +add_concurrent_test(gl20, 'vertex-program-two-side enabled front front2') +add_concurrent_test(gl20, 'vertex-program-two-side enabled back back2') +add_concurrent_test(gl20, 'vertex-program-two-side enabled front') +add_concurrent_test(gl20, 'vertex-program-two-side enabled back') +add_concurrent_test(gl20, 'vertex-program-two-side enabled front2') +add_concurrent_test(gl20, 'vertex-program-two-side enabled back2') + +add_concurrent_test(gl20, 'vertex-program-two-side front back front2 back2') +add_concurrent_test(gl20, 'vertex-program-two-side front back') +add_concurrent_test(gl20, 'vertex-program-two-side front2 back2') +add_concurrent_test(gl20, 'vertex-program-two-side front front2') add_concurrent_test(gl20, 'vertex-program-two-side front') -add_concurrent_test(gl20, 'vertex-program-two-side back') -add_concurrent_test(gl20, 'vertex-program-two-side primary') -add_concurrent_test(gl20, 'vertex-program-two-side primary front') -add_concurrent_test(gl20, 'vertex-program-two-side primary back') -add_concurrent_test(gl20, 'vertex-program-two-side secondary') -add_concurrent_test(gl20, 'vertex-program-two-side secondary front') -add_concurrent_test(gl20, 'vertex-program-two-side secondary back') -add_concurrent_test(gl20, 'vertex-program-two-side disabled') -add_concurrent_test(gl20, 'vertex-program-two-side disabled front') -add_concurrent_test(gl20, 'vertex-program-two-side disabled primary') -add_concurrent_test(gl20, 'vertex-program-two-side disabled primary front') -add_concurrent_test(gl20, 'vertex-program-two-side disabled secondary') -add_concurrent_test(gl20, 'vertex-program-two-side disabled secondary front') +add_concurrent_test(gl20, 'vertex-program-two-side front2') </blockquote><div> There are a few nonsensical use cases that won't produce well-defined colors, but are probably worth testing just to make sure they don't cause a GPU hang or assertion failure, such as "enabled front back back2", or "back back2". In fact, I would argue that it's worth testing all 32 combinations of enable, front, back, front2, and back2 just to be on the safe side. </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"> # Group spec/glsl-1.00 spec['glsl-1.00'] = Group() diff --git a/tests/spec/gl-2.0/vertex-program-two-side.c b/tests/spec/gl-2.0/vertex-program-two-side.c index e57e508..611e1a9 100644 --- a/tests/spec/gl-2.0/vertex-program-two-side.c +++ b/tests/spec/gl-2.0/vertex-program-two-side.c @@ -50,11 +50,12 @@ int piglit_window_mode = GLUT_RGB | GLUT_ALPHA | GLUT_DOUBLE; static GLint prog; -static bool primary = true; -static bool secondary = true; -static bool enabled = true; -static bool front = true; -static bool back = true; +static bool enabled = false; +static bool front = false; +static bool back = false; +static bool front2 = false; +static bool back2 = false; + static float frontcolor[4] = {0.0, 0.5, 0.0, 0.0}; static float backcolor[4] = {0.0, 0.0, 0.5, 0.0}; static float secondary_frontcolor[4] = {0.0, 0.25, 0.0, 0.0}; @@ -78,51 +79,49 @@ piglit_display(void) int w = piglit_width / 2, h = piglit_height / 2; int x2 = piglit_width - w, y2 = piglit_height - h; bool pass = true; - bool draw_back = back || !enabled; glClearColor(0.5, 0.5, 0.5, 0.5); glClear(GL_COLOR_BUFFER_BIT); glUniform1i(draw_secondary_loc, false); - if (front && primary) + if (front) piglit_draw_rect(-1, 0, 1, 1); /* top left */ - if (draw_back && primary) + if (back || (front && !enabled)) piglit_draw_rect( 1, 0, -1, 1); /* top right */ glUniform1i(draw_secondary_loc, true); - if (front && secondary) + if (front2) piglit_draw_rect(-1, -1, 1, 1); /* bot left */ - if (draw_back && secondary) + if (back2 || (front2 && !enabled)) piglit_draw_rect( 1, -1, -1, 1); /* bot right */ </blockquote><div> Along the same lines of what I said above, I think it would be worth drawing all four rects regardless of the command-line arguments, just to make sure there isn't an assertion failure or GPU hang when drawing a rect with an undefined color. We can rely on the if tests below to ensure that we only *check* the colors in the cases where they are well-defined. </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"> if (front) { - if (primary) { - pass = pass && piglit_probe_rect_rgba(x1, y2, w, h, - frontcolor); - } - if (secondary) { - pass = pass && piglit_probe_rect_rgba(x1, y1, w, h, - secondary_frontcolor); - } + pass = pass && piglit_probe_rect_rgba(x1, y2, w, h, + frontcolor); + } + + if (front2) { + pass = pass && piglit_probe_rect_rgba(x1, y1, w, h, + secondary_frontcolor); } - if (back && enabled) { + if (enabled) { /* Two-sided: Get the back color/secondarycolor. */ - if (primary) { + if (back) { pass = pass && piglit_probe_rect_rgba(x2, y2, w, h, backcolor); } - if (secondary) { + if (back2) { pass = pass && piglit_probe_rect_rgba(x2, y1, w, h, secondary_backcolor); } - } else if (back) { + } else { /* Non-two-sided: Get the front color/secondarycolor. */ - if (primary) { + if (front) { pass = pass && piglit_probe_rect_rgba(x2, y2, w, h, frontcolor); } - if (secondary) { + if (front2) { pass = pass && piglit_probe_rect_rgba(x2, y1, w, h, secondary_frontcolor); } @@ -168,30 +167,28 @@ piglit_init(int argc, char **argv) printf("+-------------------------+------------------------+\n"); for (i = 1; i < argc; i++) { - if (strcmp(argv[i], "disabled") == 0) { - enabled = false; + if (strcmp(argv[i], "enabled") == 0) { + enabled = true; } else if (strcmp(argv[i], "front") == 0) { - back = false; + front = true; } else if (strcmp(argv[i], "back") == 0) { - front = false; - } else if (strcmp(argv[i], "primary") == 0) { - secondary = false; - } else if (strcmp(argv[i], "secondary") == 0) { - primary = false; + back = true; + } else if (strcmp(argv[i], "front2") == 0) { + front2 = true; + } else if (strcmp(argv[i], "back2") == 0) { + back2 = true; } else { fprintf(stderr, "unknown argument %s\n", argv[i]); } } - assert(enabled || front); - - if (front && primary) + if (front) setup_output(&vs_outputs[0], "gl_FrontColor", frontcolor); - if (back && primary) + if (back) setup_output(&vs_outputs[1], "gl_BackColor", backcolor); - if (front && secondary) + if (front2) setup_output(&vs_outputs[2], "gl_FrontSecondaryColor", secondary_frontcolor); - if (back && secondary) + if (back2) setup_output(&vs_outputs[3], "gl_BackSecondaryColor", secondary_backcolor); asprintf(&vs_source, -- 1.7.5.4 _______________________________________________ Piglit mailing list <a href="mailto:Piglit@lists.freedesktop.org">Piglit@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/piglit" target="_blank">http://lists.freedesktop.org/mailman/listinfo/piglit</a> </blockquote></div> Other than the two comments above, this series is Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>>