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