[Piglit] [PATCH 6/6] Convert some raw glReadPixels to use piglit probes

Paul Berry stereotype441 at gmail.com
Sun Aug 26 12:55:00 PDT 2012


On 25 August 2012 01:12, Chris Forbes <chrisf at ijw.co.nz> wrote:

>
> Signed-off-by: Chris Forbes <chrisf at ijw.co.nz>
> ---
>  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 <stereotype441 at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20120826/74e58f61/attachment.html>


More information about the Piglit mailing list