[19/19] tests/ext_polygon_offset_clamp-draw: use subtest framework

Submitted by Dylan Baker on Nov. 19, 2018, 9:23 p.m.

Details

Message ID 20181119212345.21108-20-dylan@pnwbakers.com
State New
Headers show
Series "Convert a number of tests to use subtest framework" ( rev: 8 7 6 5 4 3 2 1 ) in Piglit

Not browsing as part of any series.

Commit Message

Dylan Baker Nov. 19, 2018, 9:23 p.m.
---
 tests/spec/ext_polygon_offset_clamp/draw.c | 121 ++++++++++++++-------
 1 file changed, 79 insertions(+), 42 deletions(-)

Patch hide | download patch | download mbox

diff --git a/tests/spec/ext_polygon_offset_clamp/draw.c b/tests/spec/ext_polygon_offset_clamp/draw.c
index 5c7382556..089b45425 100644
--- a/tests/spec/ext_polygon_offset_clamp/draw.c
+++ b/tests/spec/ext_polygon_offset_clamp/draw.c
@@ -39,47 +39,22 @@ 
 
 #include "piglit-util-gl.h"
 
-PIGLIT_GL_TEST_CONFIG_BEGIN
-#if PIGLIT_USE_OPENGL
-	config.supports_gl_compat_version = 21;
-#else
-	config.supports_gl_es_version = 20;
-#endif
-	config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DEPTH | PIGLIT_GL_VISUAL_DOUBLE;
-
-PIGLIT_GL_TEST_CONFIG_END
-
 GLint prog, color, zflip;
 
-enum piglit_result
-piglit_display(void)
-{
-	static const float blue[4] = {0, 0, 1, 1};
-	static const float red[4] = {1, 0, 0, 1};
-	static const float green[4] = {0, 1, 0, 1};
-
-	bool passa = true, passb = true;
-
-	glUseProgram(prog);
-
-	glViewport(0, 0, piglit_width, piglit_height);
-	glEnable(GL_DEPTH_TEST);
-	glEnable(GL_POLYGON_OFFSET_FILL);
-
-	glUniform1f(zflip, 1.0);
-	glClearColor(0, 0, 1, 1);
-#ifdef PIGLIT_USE_OPENGL
-	glClearDepth(0.5);
-#else
-	glClearDepthf(0.5);
-#endif
-	glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
+static const struct piglit_gl_test_config * piglit_config;
+static const float blue[4] = {0, 0, 1, 1};
+static const float red[4] = {1, 0, 0, 1};
+static const float green[4] = {0, 1, 0, 1};
 
+static enum piglit_result
+test_negative_clamp(void * unused)
+{
 	/* NOTE: It appears that at least nvidia hw will end up
 	 * wrapping around if the final z value goes below 0 (or
 	 * something). This can come up when testing without the
 	 * clamp.
 	 */
+	bool pass = true;
 
 	/* Draw red rectangle that slopes between 1 and 0.1. Use a
 	 * polygon offset with a high factor but small clamp
@@ -89,7 +64,7 @@  piglit_display(void)
 	glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
 	if (!piglit_probe_rect_rgba(0, 0, piglit_width, piglit_height, blue)) {
 		printf("  FAIL: red rect peeks over blue rect\n");
-		passa = false;
+		pass = false;
 	}
 
 	/* And now set the clamp such that all parts of the polygon
@@ -100,11 +75,21 @@  piglit_display(void)
 	glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
 	if (!piglit_probe_rect_rgba(0, 0, piglit_width, piglit_height, green)) {
 		printf("  FAIL: green rect does not cover blue rect\n");
-		passa = false;
+		pass = false;
 	}
 
-	piglit_report_subtest_result(passa ? PIGLIT_PASS : PIGLIT_FAIL,
-				     "negative clamp");
+	return pass ? PIGLIT_PASS : PIGLIT_FAIL;
+}
+
+static enum piglit_result
+test_positive_clamp(void * unused)
+{
+	/* NOTE: It appears that at least nvidia hw will end up
+	 * wrapping around if the final z value goes below 0 (or
+	 * something). This can come up when testing without the
+	 * clamp.
+	 */
+	bool pass = true;
 
 	/* Now try this again with the inverse approach and a positive
 	 * clamp value. The polygon will now slope between -1 and
@@ -121,7 +106,7 @@  piglit_display(void)
 	glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
 	if (!piglit_probe_rect_rgba(0, 0, piglit_width, piglit_height, blue)) {
 		printf("  FAIL: red rect peeks over blue rect\n");
-		passb = false;
+		pass = false;
 	}
 
 	/* And now set the clamp so that all parts of the polygon pass
@@ -132,15 +117,67 @@  piglit_display(void)
 	glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
 	if (!piglit_probe_rect_rgba(0, 0, piglit_width, piglit_height, green)) {
 		printf("  FAIL: green rect does not cover blue rect\n");
-		passb = false;
+		pass = false;
 	}
 
-	piglit_report_subtest_result(passb ? PIGLIT_PASS : PIGLIT_FAIL,
-				     "positive clamp");
+	return pass ? PIGLIT_PASS : PIGLIT_FAIL;
+}
+
+static const struct piglit_subtest tests[] = {
+	{
+		"negative clamp",
+		"neg_clamp",
+		test_negative_clamp,
+		NULL
+	},
+	{
+		"positive clamp",
+		"pos_clamp",
+		test_positive_clamp,
+		NULL
+	},
+	{ NULL }
+};
+
+PIGLIT_GL_TEST_CONFIG_BEGIN
+#if PIGLIT_USE_OPENGL
+	piglit_config = &config;
+	config.subtests = tests;
+	config.supports_gl_compat_version = 21;
+#else
+	config.supports_gl_es_version = 20;
+#endif
+	config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DEPTH | PIGLIT_GL_VISUAL_DOUBLE;
+
+PIGLIT_GL_TEST_CONFIG_END
+
+enum piglit_result
+piglit_display(void)
+{
+	glUseProgram(prog);
+
+	glViewport(0, 0, piglit_width, piglit_height);
+	glEnable(GL_DEPTH_TEST);
+	glEnable(GL_POLYGON_OFFSET_FILL);
+
+	glUniform1f(zflip, 1.0);
+	glClearColor(0, 0, 1, 1);
+#ifdef PIGLIT_USE_OPENGL
+	glClearDepth(0.5);
+#else
+	glClearDepthf(0.5);
+#endif
+	glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
 
+	enum piglit_result result = PIGLIT_PASS;
+	result = piglit_run_selected_subtests(
+		tests,
+		piglit_config->selected_subtests,
+		piglit_config->num_selected_subtests,
+		result);
 	piglit_present_results();
 
-	return (passa && passb) ? PIGLIT_PASS : PIGLIT_FAIL;
+	return result;
 }
 
 void

Comments

On Mon, Nov 19, 2018 at 4:24 PM Dylan Baker <dylan@pnwbakers.com> wrote:
>
> ---
>  tests/spec/ext_polygon_offset_clamp/draw.c | 121 ++++++++++++++-------
>  1 file changed, 79 insertions(+), 42 deletions(-)
>
> diff --git a/tests/spec/ext_polygon_offset_clamp/draw.c b/tests/spec/ext_polygon_offset_clamp/draw.c
> index 5c7382556..089b45425 100644
> --- a/tests/spec/ext_polygon_offset_clamp/draw.c
> +++ b/tests/spec/ext_polygon_offset_clamp/draw.c
> @@ -39,47 +39,22 @@
>
>  #include "piglit-util-gl.h"
>
> -PIGLIT_GL_TEST_CONFIG_BEGIN
> -#if PIGLIT_USE_OPENGL
> -       config.supports_gl_compat_version = 21;
> -#else
> -       config.supports_gl_es_version = 20;
> -#endif
> -       config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DEPTH | PIGLIT_GL_VISUAL_DOUBLE;
> -
> -PIGLIT_GL_TEST_CONFIG_END
> -
>  GLint prog, color, zflip;
>
> -enum piglit_result
> -piglit_display(void)
> -{
> -       static const float blue[4] = {0, 0, 1, 1};
> -       static const float red[4] = {1, 0, 0, 1};
> -       static const float green[4] = {0, 1, 0, 1};
> -
> -       bool passa = true, passb = true;
> -
> -       glUseProgram(prog);
> -
> -       glViewport(0, 0, piglit_width, piglit_height);
> -       glEnable(GL_DEPTH_TEST);
> -       glEnable(GL_POLYGON_OFFSET_FILL);
> -
> -       glUniform1f(zflip, 1.0);
> -       glClearColor(0, 0, 1, 1);
> -#ifdef PIGLIT_USE_OPENGL
> -       glClearDepth(0.5);
> -#else
> -       glClearDepthf(0.5);
> -#endif
> -       glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
> +static const struct piglit_gl_test_config * piglit_config;
> +static const float blue[4] = {0, 0, 1, 1};
> +static const float red[4] = {1, 0, 0, 1};
> +static const float green[4] = {0, 1, 0, 1};
>
> +static enum piglit_result
> +test_negative_clamp(void * unused)
> +{
>         /* NOTE: It appears that at least nvidia hw will end up
>          * wrapping around if the final z value goes below 0 (or
>          * something). This can come up when testing without the
>          * clamp.
>          */
> +       bool pass = true;
>
>         /* Draw red rectangle that slopes between 1 and 0.1. Use a
>          * polygon offset with a high factor but small clamp
> @@ -89,7 +64,7 @@ piglit_display(void)
>         glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
>         if (!piglit_probe_rect_rgba(0, 0, piglit_width, piglit_height, blue)) {
>                 printf("  FAIL: red rect peeks over blue rect\n");
> -               passa = false;
> +               pass = false;
>         }
>
>         /* And now set the clamp such that all parts of the polygon
> @@ -100,11 +75,21 @@ piglit_display(void)
>         glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
>         if (!piglit_probe_rect_rgba(0, 0, piglit_width, piglit_height, green)) {
>                 printf("  FAIL: green rect does not cover blue rect\n");
> -               passa = false;
> +               pass = false;
>         }
>
> -       piglit_report_subtest_result(passa ? PIGLIT_PASS : PIGLIT_FAIL,
> -                                    "negative clamp");
> +       return pass ? PIGLIT_PASS : PIGLIT_FAIL;
> +}
> +
> +static enum piglit_result
> +test_positive_clamp(void * unused)
> +{
> +       /* NOTE: It appears that at least nvidia hw will end up
> +        * wrapping around if the final z value goes below 0 (or
> +        * something). This can come up when testing without the
> +        * clamp.

Having this identical comment 2x seems ... unnecessary. Perhaps just
once and above the function?

> +        */
> +       bool pass = true;
>
>         /* Now try this again with the inverse approach and a positive
>          * clamp value. The polygon will now slope between -1 and
> @@ -121,7 +106,7 @@ piglit_display(void)
>         glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
>         if (!piglit_probe_rect_rgba(0, 0, piglit_width, piglit_height, blue)) {
>                 printf("  FAIL: red rect peeks over blue rect\n");
> -               passb = false;
> +               pass = false;
>         }
>
>         /* And now set the clamp so that all parts of the polygon pass
> @@ -132,15 +117,67 @@ piglit_display(void)
>         glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
>         if (!piglit_probe_rect_rgba(0, 0, piglit_width, piglit_height, green)) {
>                 printf("  FAIL: green rect does not cover blue rect\n");
> -               passb = false;
> +               pass = false;
>         }
>
> -       piglit_report_subtest_result(passb ? PIGLIT_PASS : PIGLIT_FAIL,
> -                                    "positive clamp");
> +       return pass ? PIGLIT_PASS : PIGLIT_FAIL;
> +}
> +
> +static const struct piglit_subtest tests[] = {
> +       {
> +               "negative clamp",
> +               "neg_clamp",
> +               test_negative_clamp,
> +               NULL
> +       },
> +       {
> +               "positive clamp",
> +               "pos_clamp",
> +               test_positive_clamp,
> +               NULL
> +       },
> +       { NULL }
> +};
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +#if PIGLIT_USE_OPENGL

Probably meant to put that a couple of lines down?

> +       piglit_config = &config;
> +       config.subtests = tests;
> +       config.supports_gl_compat_version = 21;
> +#else
> +       config.supports_gl_es_version = 20;
> +#endif
> +       config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DEPTH | PIGLIT_GL_VISUAL_DOUBLE;
> +
> +PIGLIT_GL_TEST_CONFIG_END

I don't have a constructive way to address this, but it's rather
unfortunate that this block has to move from being at the top, where
it is for every single piglit test, to the bottom.

> +
> +enum piglit_result
> +piglit_display(void)
> +{
> +       glUseProgram(prog);
> +
> +       glViewport(0, 0, piglit_width, piglit_height);
> +       glEnable(GL_DEPTH_TEST);
> +       glEnable(GL_POLYGON_OFFSET_FILL);
> +
> +       glUniform1f(zflip, 1.0);
> +       glClearColor(0, 0, 1, 1);
> +#ifdef PIGLIT_USE_OPENGL
> +       glClearDepth(0.5);
> +#else
> +       glClearDepthf(0.5);
> +#endif
> +       glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
>
> +       enum piglit_result result = PIGLIT_PASS;
> +       result = piglit_run_selected_subtests(
> +               tests,
> +               piglit_config->selected_subtests,
> +               piglit_config->num_selected_subtests,
> +               result);
>         piglit_present_results();
>
> -       return (passa && passb) ? PIGLIT_PASS : PIGLIT_FAIL;
> +       return result;
>  }
>
>  void
> --
> 2.19.1
>
> _______________________________________________
> Piglit mailing list
> Piglit@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
Quoting Ilia Mirkin (2018-11-20 18:28:52)
> On Mon, Nov 19, 2018 at 4:24 PM Dylan Baker <dylan@pnwbakers.com> wrote:
> >
> > ---
> >  tests/spec/ext_polygon_offset_clamp/draw.c | 121 ++++++++++++++-------
> >  1 file changed, 79 insertions(+), 42 deletions(-)
> >
> > diff --git a/tests/spec/ext_polygon_offset_clamp/draw.c b/tests/spec/ext_polygon_offset_clamp/draw.c
> > index 5c7382556..089b45425 100644
> > --- a/tests/spec/ext_polygon_offset_clamp/draw.c
> > +++ b/tests/spec/ext_polygon_offset_clamp/draw.c
> > @@ -39,47 +39,22 @@
> >
> >  #include "piglit-util-gl.h"
> >
> > -PIGLIT_GL_TEST_CONFIG_BEGIN
> > -#if PIGLIT_USE_OPENGL
> > -       config.supports_gl_compat_version = 21;
> > -#else
> > -       config.supports_gl_es_version = 20;
> > -#endif
> > -       config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DEPTH | PIGLIT_GL_VISUAL_DOUBLE;
> > -
> > -PIGLIT_GL_TEST_CONFIG_END
> > -
> >  GLint prog, color, zflip;
> >
> > -enum piglit_result
> > -piglit_display(void)
> > -{
> > -       static const float blue[4] = {0, 0, 1, 1};
> > -       static const float red[4] = {1, 0, 0, 1};
> > -       static const float green[4] = {0, 1, 0, 1};
> > -
> > -       bool passa = true, passb = true;
> > -
> > -       glUseProgram(prog);
> > -
> > -       glViewport(0, 0, piglit_width, piglit_height);
> > -       glEnable(GL_DEPTH_TEST);
> > -       glEnable(GL_POLYGON_OFFSET_FILL);
> > -
> > -       glUniform1f(zflip, 1.0);
> > -       glClearColor(0, 0, 1, 1);
> > -#ifdef PIGLIT_USE_OPENGL
> > -       glClearDepth(0.5);
> > -#else
> > -       glClearDepthf(0.5);
> > -#endif
> > -       glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
> > +static const struct piglit_gl_test_config * piglit_config;
> > +static const float blue[4] = {0, 0, 1, 1};
> > +static const float red[4] = {1, 0, 0, 1};
> > +static const float green[4] = {0, 1, 0, 1};
> >
> > +static enum piglit_result
> > +test_negative_clamp(void * unused)
> > +{
> >         /* NOTE: It appears that at least nvidia hw will end up
> >          * wrapping around if the final z value goes below 0 (or
> >          * something). This can come up when testing without the
> >          * clamp.
> >          */
> > +       bool pass = true;
> >
> >         /* Draw red rectangle that slopes between 1 and 0.1. Use a
> >          * polygon offset with a high factor but small clamp
> > @@ -89,7 +64,7 @@ piglit_display(void)
> >         glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
> >         if (!piglit_probe_rect_rgba(0, 0, piglit_width, piglit_height, blue)) {
> >                 printf("  FAIL: red rect peeks over blue rect\n");
> > -               passa = false;
> > +               pass = false;
> >         }
> >
> >         /* And now set the clamp such that all parts of the polygon
> > @@ -100,11 +75,21 @@ piglit_display(void)
> >         glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
> >         if (!piglit_probe_rect_rgba(0, 0, piglit_width, piglit_height, green)) {
> >                 printf("  FAIL: green rect does not cover blue rect\n");
> > -               passa = false;
> > +               pass = false;
> >         }
> >
> > -       piglit_report_subtest_result(passa ? PIGLIT_PASS : PIGLIT_FAIL,
> > -                                    "negative clamp");
> > +       return pass ? PIGLIT_PASS : PIGLIT_FAIL;
> > +}
> > +
> > +static enum piglit_result
> > +test_positive_clamp(void * unused)
> > +{
> > +       /* NOTE: It appears that at least nvidia hw will end up
> > +        * wrapping around if the final z value goes below 0 (or
> > +        * something). This can come up when testing without the
> > +        * clamp.
> 
> Having this identical comment 2x seems ... unnecessary. Perhaps just
> once and above the function?

sure, that sounds better.

> 
> > +        */
> > +       bool pass = true;
> >
> >         /* Now try this again with the inverse approach and a positive
> >          * clamp value. The polygon will now slope between -1 and
> > @@ -121,7 +106,7 @@ piglit_display(void)
> >         glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
> >         if (!piglit_probe_rect_rgba(0, 0, piglit_width, piglit_height, blue)) {
> >                 printf("  FAIL: red rect peeks over blue rect\n");
> > -               passb = false;
> > +               pass = false;
> >         }
> >
> >         /* And now set the clamp so that all parts of the polygon pass
> > @@ -132,15 +117,67 @@ piglit_display(void)
> >         glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
> >         if (!piglit_probe_rect_rgba(0, 0, piglit_width, piglit_height, green)) {
> >                 printf("  FAIL: green rect does not cover blue rect\n");
> > -               passb = false;
> > +               pass = false;
> >         }
> >
> > -       piglit_report_subtest_result(passb ? PIGLIT_PASS : PIGLIT_FAIL,
> > -                                    "positive clamp");
> > +       return pass ? PIGLIT_PASS : PIGLIT_FAIL;
> > +}
> > +
> > +static const struct piglit_subtest tests[] = {
> > +       {
> > +               "negative clamp",
> > +               "neg_clamp",
> > +               test_negative_clamp,
> > +               NULL
> > +       },
> > +       {
> > +               "positive clamp",
> > +               "pos_clamp",
> > +               test_positive_clamp,
> > +               NULL
> > +       },
> > +       { NULL }
> > +};
> > +
> > +PIGLIT_GL_TEST_CONFIG_BEGIN
> > +#if PIGLIT_USE_OPENGL
> 
> Probably meant to put that a couple of lines down?

yeah, this looks like rebase fail.

> 
> > +       piglit_config = &config;
> > +       config.subtests = tests;
> > +       config.supports_gl_compat_version = 21;
> > +#else
> > +       config.supports_gl_es_version = 20;
> > +#endif
> > +       config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DEPTH | PIGLIT_GL_VISUAL_DOUBLE;
> > +
> > +PIGLIT_GL_TEST_CONFIG_END
> 
> I don't have a constructive way to address this, but it's rather
> unfortunate that this block has to move from being at the top, where
> it is for every single piglit test, to the bottom.

I don't like it either, but I couldn't come up with any way to fix it without
fundamentally changing the way piglit tests are defined or something equally
invasive.

> 
> > +
> > +enum piglit_result
> > +piglit_display(void)
> > +{
> > +       glUseProgram(prog);
> > +
> > +       glViewport(0, 0, piglit_width, piglit_height);
> > +       glEnable(GL_DEPTH_TEST);
> > +       glEnable(GL_POLYGON_OFFSET_FILL);
> > +
> > +       glUniform1f(zflip, 1.0);
> > +       glClearColor(0, 0, 1, 1);
> > +#ifdef PIGLIT_USE_OPENGL
> > +       glClearDepth(0.5);
> > +#else
> > +       glClearDepthf(0.5);
> > +#endif
> > +       glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
> >
> > +       enum piglit_result result = PIGLIT_PASS;
> > +       result = piglit_run_selected_subtests(
> > +               tests,
> > +               piglit_config->selected_subtests,
> > +               piglit_config->num_selected_subtests,
> > +               result);
> >         piglit_present_results();
> >
> > -       return (passa && passb) ? PIGLIT_PASS : PIGLIT_FAIL;
> > +       return result;
> >  }
> >
> >  void
> > --
> > 2.19.1
> >
> > _______________________________________________
> > Piglit mailing list
> > Piglit@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/piglit