Quoting Ilia Mirkin (2018-11-20 18:28:52) > On Mon, Nov 19, 2018 at 4:24 PM Dylan Baker <[email protected]> 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
> > [email protected]
> > https://lists.freedesktop.org/mailman/listinfo/piglit
signature.asc
Description: signature
_______________________________________________ Piglit mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/piglit
