<div dir="ltr">Looks good overall. Just a bunch of nit-picks below... On Mon, Aug 24, 2015 at 5:19 PM, Juliet Fru <<a href="mailto:julietfru@gmail.com" target="_blank">julietfru@gmail.com</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">--- tests/all.py | 1 + tests/spec/gl-1.0/CMakeLists.gl.txt | 1 + tests/spec/gl-1.0/no-op-paths.c | 370 ++++++++++++++++++++++++++++++++++++ 3 files changed, 372 insertions(+) create mode 100644 tests/spec/gl-1.0/no-op-paths.c diff --git a/tests/all.py b/tests/all.py index fe088f5..a9822d3 100644 --- a/tests/all.py +++ b/tests/all.py @@ -1000,6 +1000,7 @@ with profile.group_manager( g(['gl-1.0-ortho-pos']) g(['gl-1.0-readpixsanity']) g(['gl-1.0-logicop']) + g(['gl-1.0-no-op-paths']) with profile.group_manager( PiglitGLTest, diff --git a/tests/spec/gl-1.0/CMakeLists.gl.txt b/tests/spec/gl-1.0/CMakeLists.gl.txt index d04b835..7a7f508 100644 --- a/tests/spec/gl-1.0/CMakeLists.gl.txt +++ b/tests/spec/gl-1.0/CMakeLists.gl.txt @@ -22,6 +22,7 @@ piglit_add_executable (gl-1.0-front-invalidate-back front-invalidate-back.c) piglit_add_executable (gl-1.0-logicop logicop.c) piglit_add_executable (gl-1.0-long-dlist long-dlist.c) piglit_add_executable (gl-1.0-ortho-pos orthpos.c) +piglit_add_executable (gl-1.0-no-op-paths no-op-paths.c) piglit_add_executable (gl-1.0-polygon-line-aa polygon-line-aa.c) piglit_add_executable (gl-1.0-push-no-attribs push-no-attribs.c) piglit_add_executable (gl-1.0-readpixsanity readpix.c) diff --git a/tests/spec/gl-1.0/no-op-paths.c b/tests/spec/gl-1.0/no-op-paths.c new file mode 100644 index 0000000..9a76baf --- /dev/null +++ b/tests/spec/gl-1.0/no-op-paths.c @@ -0,0 +1,370 @@ +/* BEGIN_COPYRIGHT -*- glean -*- + * + * Copyright (C) 1999 Allen Akin All Rights Reserved. + * Copyright (C) 2015 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. + * + * END_COPYRIGHT + */ +/** @file paths.c + * + * Test basic GL rendering paths. + * + * + * This test verifies that basic, trival OpenGL paths work as expected. + * For example, glAlphaFunc(GL_GEQUAL, 0.0) should always pass and + * glAlphaFunc(GL_LESS, 0.0) should always fail. We setup trivial + * pass and fail conditions for each of alpha test, blending, color mask, + * depth test, logic ops, scissor, stencil, stipple, and texture and + * make sure they work as expected. We also setup trival-pass for all + * these paths simultaneously and test that as well. + * + * To test for pass/fail we examine the color buffer for white or black, + * respectively. + * </blockquote><div> </div><div>Here, I'd add an additional comment saying that this Piglit test is based on the original Glean tpaths.cpp test. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + */ + +#include "piglit-util-gl.h" + +#include <stdlib.h> </blockquote><div> </div><div>Is stdlib.h really needed? </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + + +PIGLIT_GL_TEST_CONFIG_BEGIN + + config.supports_gl_compat_version = 10; + + config.window_visual = PIGLIT_GL_VISUAL_RGBA | + PIGLIT_GL_VISUAL_DOUBLE | PIGLIT_GL_VISUAL_DEPTH; + +PIGLIT_GL_TEST_CONFIG_END + + +typedef enum { + ALPHA, + BLEND, + COLOR_MASK, + DEPTH, + LOGIC, + SCISSOR, + STENCIL, + STIPPLE, + TEXTURE, + ZZZ /* end-of-list token */ +} Path; + +typedef enum { + DISABLE, + ALWAYS_PASS, + ALWAYS_FAIL + </blockquote><div> </div><div>Unneeded empty line. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> +} State; + + +const char * </blockquote><div> </div><div>Could be static. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> +path_name(Path path) { </blockquote><div> </div><div>Opening brace goes on next line. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + + switch (path) { + case ALPHA: + return "Alpha Test"; </blockquote><div> </div><div>We use 8-column tabs for indenting in Piglit, not spaces. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + case BLEND: + return "Blending"; + case COLOR_MASK: + return "Color Mask"; + case DEPTH: + return "Depth Test"; + case LOGIC: + return "LogicOp"; + case SCISSOR: + return "Scissor Test"; + case STENCIL: + return "Stencil Test"; + case STIPPLE: + return "Polygon Stipple"; + case TEXTURE: + return "Modulated Texture"; + case ZZZ: + return "paths"; + default: + return "???"; + } +} + + +void +set_path_state(Path path, State state) { </blockquote><div> </div><div>Same comments as above. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + + int i; + switch (path) { + case ALPHA: + if (state == ALWAYS_PASS) { + glAlphaFunc(GL_GEQUAL, 0.0); + glEnable(GL_ALPHA_TEST); + } + else if (state == ALWAYS_FAIL) { + glAlphaFunc(GL_GREATER, 1.0); + glEnable(GL_ALPHA_TEST); + } + else { + glDisable(GL_ALPHA_TEST); + } + break; + case BLEND: + if (state == ALWAYS_PASS) { + glBlendFunc(GL_ONE, GL_ZERO); + glEnable(GL_BLEND); + } + else if (state == ALWAYS_FAIL) { + glBlendFunc(GL_ZERO, GL_ONE); + glEnable(GL_BLEND); + } + else { + glDisable(GL_BLEND); + } + break; + case COLOR_MASK: + if (state == ALWAYS_PASS) { + glColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE); + } + else if (state == ALWAYS_FAIL) { + glColorMask(GL_FALSE, GL_FALSE, GL_FALSE, GL_FALSE); + } + else { + glColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE); + } + break; + case DEPTH: + if (state == ALWAYS_PASS) { + glDepthFunc(GL_ALWAYS); + glEnable(GL_DEPTH_TEST); + } + else if (state == ALWAYS_FAIL) { + glDepthFunc(GL_NEVER); + glEnable(GL_DEPTH_TEST); + } + else { + glDisable(GL_DEPTH_TEST); + } + break; + case LOGIC: + if (state == ALWAYS_PASS) { + glLogicOp(GL_OR); + glEnable(GL_COLOR_LOGIC_OP); + } + else if (state == ALWAYS_FAIL) { + glLogicOp(GL_AND); + glEnable(GL_COLOR_LOGIC_OP); + } + else { + glDisable(GL_COLOR_LOGIC_OP); + } + break; + case SCISSOR: + if (state == ALWAYS_PASS) { + glScissor(0, 0, 10, 10); + glEnable(GL_SCISSOR_TEST); + } + else if (state == ALWAYS_FAIL) { + glScissor(0, 0, 0, 0); + glEnable(GL_SCISSOR_TEST); + } + else { + glDisable(GL_SCISSOR_TEST); + } + break; + case STENCIL: + if (state == ALWAYS_PASS) { + /* pass if reference <= stencil value (ref = 0) */ + glStencilFunc(GL_LEQUAL, 0, ~0); + glEnable(GL_STENCIL_TEST); + } + else if (state == ALWAYS_FAIL) { + /* pass if reference > stencil value (ref = 0) */ + glStencilFunc(GL_GREATER, 0, ~0); + glEnable(GL_STENCIL_TEST); + } + else { + glDisable(GL_STENCIL_TEST); + } + break; + case STIPPLE: + if (state == ALWAYS_PASS) { + GLubyte stipple[4*32]; + for (i = 0; i < 4*32; i++) + stipple[i] = 0xff; + glPolygonStipple(stipple); + glEnable(GL_POLYGON_STIPPLE); + } + else if (state == ALWAYS_FAIL) { + GLubyte stipple[4*32]; + for (i = 0; i < 4*32; i++) + stipple[i] = 0x0; + glPolygonStipple(stipple); + glEnable(GL_POLYGON_STIPPLE); + } + else { + glDisable(GL_POLYGON_STIPPLE); + } + break; + case TEXTURE: + if (state == DISABLE) { + glDisable(GL_TEXTURE_2D); + } + else { + GLubyte val = (state == ALWAYS_PASS) ? 0xff : 0x00; + GLubyte texImage[4*4*4]; + for (i = 0; i < 4*4*4; i++) + texImage[i] = val; + glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 4, 4, 0, + GL_RGBA, GL_UNSIGNED_BYTE, texImage); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, + GL_NEAREST); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, + GL_NEAREST); + glTexEnvi(GL_TEXTURE_ENV, GL_TEXTURE_ENV_MODE, + GL_MODULATE); + glEnable(GL_TEXTURE_2D); + } + break; + default: + abort(); /* error */ + } +} + + +void +piglit_init(int argc, char **argv) +{ + /* No init */ +} /* piglit_init */ </blockquote><div> </div><div>You can drop the closing /* piglit_init */ comment. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + + +enum piglit_result +piglit_display(void) +{ + bool pass = true; + Path p, paths[1000]; + int i, numPaths = 0; + + /* draw 10x10 pixel quads */ + glViewport(0, 0, 10, 10); + + glDisable(GL_DITHER); + + /* Build the list of paths to exercise. + * Skip depth test if we have no depth buffer. */ + /* Skip stencil test if we have no stencil buffer. */ + for (p = ALPHA; p != ZZZ; p = (Path) (p + 1)) { + paths[numPaths++] = p; + } + + /* test always-pass paths */ + for (i = 0; i < numPaths; i++) { + glClear(GL_COLOR_BUFFER_BIT); + + set_path_state(paths[i], ALWAYS_PASS); + + /* draw polygon */ + glColor4f(1, 1, 1, 1); + glBegin(GL_POLYGON); + glVertex2f(-1, -1); + glVertex2f( 1, -1); + glVertex2f( 1, 1); + glVertex2f(-1, 1); + glEnd(); </blockquote><div> </div><div>I think there's a piglit_draw_rect() function that could be used there. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + + set_path_state(paths[i], DISABLE); + + /* test buffer */ + GLfloat pixel[3]; + glReadPixels(4, 4, 1, 1, GL_RGB, GL_FLOAT, pixel); + if (pixel[0] != 1.0 || pixel[1] != 1.0 || pixel[2] != 1.0) { </blockquote><div> </div><div>I think you can replace that code with piglit_probe_rect_rgb(). </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + printf("\nFAIL: %s, should have had no effect" + " (1, 1, 1) but actually modified the fragment" + " ( %f, %f, %f)\n", path_name(paths[i]), pixel[0], pixel[1], pixel[2]); + return PIGLIT_FAIL; + } + } + + /* enable all always-pass paths */ + { + glClear(GL_COLOR_BUFFER_BIT); + + for (i = 0; i < numPaths; i++) { + set_path_state(paths[i], ALWAYS_PASS); + } + + /* draw polygon */ + glColor4f(1, 1, 1, 1); + glBegin(GL_POLYGON); + glVertex2f(-1, -1); + glVertex2f( 1, -1); + glVertex2f( 1, 1); + glVertex2f(-1, 1); + glEnd(); </blockquote><div> </div><div>piglit_draw_rect() </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + + for (i = 0; i < numPaths; i++) { + set_path_state(paths[i], DISABLE); + } + + /* test buffer */ + GLfloat pixel[3]; + glReadPixels(4, 4, 1, 1, GL_RGB, GL_FLOAT, pixel); + if (pixel[0] != 1.0 || pixel[1] != 1.0 || pixel[2] != 1.0) { </blockquote><div> </div><div>piglit_probe_rect_rgb() </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + printf("\nFAIL: %s, should have had no effect" + " (1, 1, 1) but actually modified the fragment" + " ( %f, %f, %f)\n", path_name(paths[i]), pixel[0], pixel[1], pixel[2]); + return PIGLIT_FAIL; + } + } + + /* test never-pass paths */ + for (i = 0; i < numPaths; i++) { + glClear(GL_COLOR_BUFFER_BIT); + + set_path_state(paths[i], ALWAYS_FAIL); + + /* draw polygon */ + glColor4f(1, 1, 1, 1); + glBegin(GL_POLYGON); + glVertex2f(-1, -1); + glVertex2f( 1, -1); + glVertex2f( 1, 1); + glVertex2f(-1, 1); + glEnd(); + + set_path_state(paths[i], DISABLE); + + /* test buffer */ + GLfloat pixel[3]; + glReadPixels(4, 4, 1, 1, GL_RGB, GL_FLOAT, pixel); + if (pixel[0] != 0.0 || pixel[1] != 0.0 || pixel[2] != 0.0) { + printf("\nFAIL: %s, should have had no effect" + " (1, 1, 1) but actually modified the fragment" + " ( %f, %f, %f)\n", path_name(paths[i]), pixel[0], pixel[1], pixel[2]); + return PIGLIT_FAIL; + } + } + + /* success */ + pass = true; + + return pass ? PIGLIT_PASS : PIGLIT_FAIL; </blockquote><div> </div><div>Remove the 'pass' var and just return PIGLIT_PASS here. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> +} -- 2.4.3 </blockquote></div> </div></div>