[Piglit] [PATCH 4/7] astc_sliced_3d: Move slice 'for' loop further down

[Nanley Chery]

OThis patch is,n Mon, Aug 01, 2016 at 04:13:21PM -0700, Anuj Phogat wrote:
> On Mon, Aug 1, 2016 at 3:36 PM, Nanley Chery <nanleychery at gmail.com> wrote:
> 
> > On Mon, Aug 01, 2016 at 03:16:17PM -0700, Anuj Phogat wrote:
> > > Cc: Nanley Chery <nanley.g.chery at intel.com>
> > > Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> > > ---
> > >  .../khr_compressed_astc-sliced-3d-miptree.c        | 39
> > +++++++++++-----------
> > >  1 file changed, 19 insertions(+), 20 deletions(-)
> > >
> > > diff --git
> > a/tests/spec/khr_texture_compression_astc/khr_compressed_astc-sliced-3d-miptree.c
> > b/tests/spec/khr_texture_compression_astc/khr_compressed_astc-sliced-3d-miptree.c
> > > index d833bac..cb884e9 100644
> > > ---
> > a/tests/spec/khr_texture_compression_astc/khr_compressed_astc-sliced-3d-miptree.c
> > > +++
> > b/tests/spec/khr_texture_compression_astc/khr_compressed_astc-sliced-3d-miptree.c
> > > @@ -216,10 +216,6 @@ bool draw_compare_levels(bool check_error,
> > >                       y += h;
> > >       }
> > >
> > > -     /* Delete bound textures */
> > > -     glDeleteTextures(1, &compressed_tex);
> > > -     glDeleteTextures(1, &decompressed_tex);
> > > -
> > >       piglit_present_results();
> > >       return result;
> > >  }
> > > @@ -279,23 +275,21 @@ test_miptrees(void* input_type)
> > >               piglit_set_tolerance_for_bits(8, 8, 8, 8);
> > >
> > >       for ( ; block_dims < ARRAY_SIZE(block_dim_str); block_dims++) {
> > > +             /* Texture objects. */
> > > +             GLuint tex_compressed = 0;
> > > +             GLuint tex_decompressed = 0;
> > > +
> > > +             /* Load texture for current submode and block size */
> > > +             load_texture("compressed/SLICED3D", tests[subtest],
> > > +                             block_dim_str[block_dims],
> > > +                             &tex_compressed);
> > > +             if (!check_error) {
> > > +                     load_texture("decompressed/SLICED3D",
> > > +                                  tests[subtest],
> > > +                                  block_dim_str[block_dims],
> > > +                                  &tex_decompressed);
> > > +             }
> > >               for (slice = 0 ; slice < LEVEL0_DEPTH; slice++) {
> > > -
> > > -                     /* Texture objects. */
> > > -                     GLuint tex_compressed = 0;
> > > -                     GLuint tex_decompressed = 0;
> > > -
> > > -                     /* Load texture for current submode and block size
> > */
> > > -                     load_texture("compressed/SLICED3D", tests[subtest],
> > > -                                     block_dim_str[block_dims],
> > > -                                     &tex_compressed);
> > > -                     if (!check_error) {
> > > -                             load_texture("decompressed/SLICED3D",
> > > -                                          tests[subtest],
> > > -                                          block_dim_str[block_dims],
> > > -                                          &tex_decompressed);
> > > -                     }
> > > -
> > >                       /* Draw and compare each level of the two textures
> > */
> > >                       glClear(GL_COLOR_BUFFER_BIT);
> > >                       if (!draw_compare_levels(check_error,
> > > @@ -311,6 +305,11 @@ test_miptrees(void* input_type)
> > >                               return PIGLIT_FAIL;
> > >                       }
> > >               }
> > > +             /* Delete bound textures */
> > > +             glDeleteTextures(1, &tex_compressed);
> > > +             if (!check_error)
> > > +                     glDeleteTextures(1, &tex_decompressed);
> > > +
> >
> > This patch does more than the commit title says. I think you should
> > mention the purpose of this (and the related) hunk or leave it out.
> >
> > Earlier the test was calling the load_texture() for each slice and deleting
> the created textures inside the slice 'for' loop (in draw_compare_levels()).
> If we want to avoid reloading
> the
> textures for each slice,
> ​we've to take the
> glDeleteTextures
> ​() calls out of
> slice 'for' loop.​
> ​ I can update the commit
> message​ with above details.
> ​

Thanks for the explanation. I didn't expect this change to be required,
but it makes sense.

With or without the explanation, this patch is,
Reviewed-by: Nanley Chery <nanley.g.chery at intel.com>

> 
> 
> > - Nanley
> >
> > >               block_dims++;
> > >       }
> > >       return PIGLIT_PASS;
> > > --
> > > 2.5.5
> > >
> > > _______________________________________________
> > > Piglit mailing list
> > > Piglit at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/piglit
> >