On 25 August 2012 01:12, Chris Forbes <[email protected]> wrote:

>
> Signed-off-by: Chris Forbes <[email protected]>
> ---
>  tests/shaders/fp-incomplete-tex.c | 41
> +++++++--------------------------------
>  tests/shaders/fp-kil.c            | 41
> +++++++--------------------------------
>  tests/texturing/crossbar.c        | 30 ++++++----------------------
>  3 files changed, 20 insertions(+), 92 deletions(-)
>
> diff --git a/tests/shaders/fp-incomplete-tex.c
> b/tests/shaders/fp-incomplete-tex.c
> index 480aee2..458c440 100644
> --- a/tests/shaders/fp-incomplete-tex.c
> +++ b/tests/shaders/fp-incomplete-tex.c
> @@ -130,45 +130,18 @@ static const struct {
>
>  static int DoTest( void )
>  {
> -       int idx;
> -       GLfloat dmax;
> +       int idx = 0;
> +       int pass = GL_TRUE;
>
> -       dmax = 0;
> -
> -       idx = 0;
>         while(Probes[idx].name) {
> -               GLfloat probe[4];
> -               GLfloat delta[4];
> -               int i;
> -
> -               glReadPixels((int)(Probes[idx].x * piglit_width / 3),
> -                            (int)(Probes[idx].y * piglit_height / 2),
> -                            1, 1,
> -               GL_RGBA, GL_FLOAT, probe);
> -
> -               printf("%20s (%3.1f,%3.1f): %f,%f,%f,%f",
> -                      Probes[idx].name,
> -                      Probes[idx].x, Probes[idx].y,
> -                      probe[0], probe[1], probe[2], probe[3]);
> -
> -               for(i = 0; i < 4; ++i) {
> -                       delta[i] = probe[i] - Probes[idx].expected[i];
> -
> -                       if (delta[i] > dmax) dmax = delta[i];
> -                       else if (-delta[i] > dmax) dmax = -delta[i];
> -               }
> -
> -               printf("   Delta: %f,%f,%f,%f\n", delta[0], delta[1],
> delta[2], delta[3]);
> -
> +               pass &= piglit_probe_pixel_rgba(
> +                       (int)(Probes[idx].x * piglit_width / 3),
> +                       (int)(Probes[idx].y * piglit_height / 2),
> +                       Probes[idx].expected);
>

Minor nit-pick:  Our usual convention is to make "pass" a bool, and to
write this as "pass = piglit_probe_pixel_rgba(...) && pass", just so that
it's clear that there aren't any bitwise shenanigans going on.  Note that
it would probably make sense to also change the return type of DoTest(),
and the type of "pass" in piglit_display(), to bool as well.  I won't be a
stickler about it though.

This comment applies to fp-kil.c and crossbar.c as well.


>                 idx++;
>         }
>
> -       printf("Max delta: %f\n", dmax);
> -
> -       if (dmax >= 0.02)
> -               return 0;
> -       else
> -               return 1;
> +       return pass;
>  }
>
>
> diff --git a/tests/shaders/fp-kil.c b/tests/shaders/fp-kil.c
> index d9b78fe..6ba1f9d 100644
> --- a/tests/shaders/fp-kil.c
> +++ b/tests/shaders/fp-kil.c
> @@ -205,45 +205,18 @@ static const struct {
>
>  static int DoTest( void )
>  {
> -       int idx;
> -       GLfloat dmax;
> +       int idx = 0;
> +       int pass = GL_TRUE;
>
> -       dmax = 0;
> -
> -       idx = 0;
>         while(Probes[idx].name) {
> -               GLfloat probe[4];
> -               GLfloat delta[4];
> -               int i;
> -
> -               glReadPixels((int)(Probes[idx].x*piglit_width/2),
> -                            (int)(Probes[idx].y*piglit_height/2),
> -                            1, 1,
> -                            GL_RGBA, GL_FLOAT, probe);
> -
> -               printf("%20s (%3.1f,%3.1f): %f,%f,%f,%f",
> -                      Probes[idx].name,
> -                      Probes[idx].x, Probes[idx].y,
> -                      probe[0], probe[1], probe[2], probe[3]);
> -
> -               for(i = 0; i < 4; ++i) {
> -                       delta[i] = probe[i] - Probes[idx].expected[i];
> -
> -                       if (delta[i] > dmax) dmax = delta[i];
> -                       else if (-delta[i] > dmax) dmax = -delta[i];
> -               }
> -
> -               printf("   Delta: %f,%f,%f,%f\n", delta[0], delta[1],
> delta[2], delta[3]);
> -
> +               pass &= piglit_probe_pixel_rgba(
> +                       (int)(Probes[idx].x*piglit_width/2),
> +                   (int)(Probes[idx].y*piglit_height/2),
> +                       Probes[idx].expected);
>

Inconsistent indentation here.


>                 idx++;
>         }
>
> -       printf("Max delta: %f\n", dmax);
> -
> -       if (dmax >= 0.02)
> -               return 0;
> -       else
> -               return 1;
> +       return pass;
>  }
>
>  enum piglit_result
> diff --git a/tests/texturing/crossbar.c b/tests/texturing/crossbar.c
> index 497ff31..1cc716d 100644
> --- a/tests/texturing/crossbar.c
> +++ b/tests/texturing/crossbar.c
> @@ -112,35 +112,17 @@ static void DoFrame( void )
>
>  static int DoTest( void )
>  {
> +   const static float expected[] = {0.5f, 0.5f, 0.5f};
> +   int pass = GL_TRUE;
>     int i;
> -   GLfloat probe[NUM_TESTS+1][4];
> -   GLfloat dr, dg, db;
> -   GLfloat dmax;
>
> -   glReadBuffer( GL_BACK );
> -
> -   dmax = 0;
>     for( i = 0; i <= NUM_TESTS; ++i ) {
> -      glReadPixels(piglit_width*(2*i+1)/((NUM_TESTS+1)*2),
> piglit_height/2, 1, 1, GL_RGBA, GL_FLOAT, probe[i]);
> -      printf("Probe %i: %f,%f,%f\n", i, probe[i][0], probe[i][1],
> probe[i][2]);
> -      dr = probe[i][0] - 0.5f;
> -      dg = probe[i][1] - 0.5f;
> -      db = probe[i][2] - 0.5f;
> -      printf("   Delta: %f,%f,%f\n", dr, dg, db);
> -      if (dr > dmax) dmax = dr;
> -      else if (-dr > dmax) dmax = -dr;
> -      if (dg > dmax) dmax = dg;
> -      else if (-dg > dmax) dmax = -dg;
> -      if (db > dmax) dmax = db;
> -      else if (-db > dmax) dmax = -db;
> +          pass &=
> piglit_probe_pixel_rgb(piglit_width*(2*i+1)/((NUM_TESTS+1)*2),
> +                          piglit_height/2,
> +                          expected);
>     }
>
> -   printf("Max delta: %f\n", dmax);
> -
> -   if (dmax >= 0.07) // roughly 1/128
>

Wow, that's gotta be the worst approximation to 1/128 I've ever seen.  Glad
to see it's getting deleted :)


> -      return 0;
> -   else
> -      return 1;
> +   return pass;
>  }
>
>  enum piglit_result
> --
> 1.7.12
>

With the indentation fixed, this patch is:

Reviewed-by: Paul Berry <[email protected]>
_______________________________________________
Piglit mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/piglit

Reply via email to