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

2016-08-01 Thread 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  wrote:
> 
> > On Mon, Aug 01, 2016 at 03:16:17PM -0700, Anuj Phogat wrote:
> > > Cc: Nanley Chery 
> > > Signed-off-by: Anuj Phogat 
> > > ---
> > >  .../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
> >
> > >   block_dims++;
> > >   }
> > >   return PIGLIT_PASS;
> > > --
> > > 2.5.5
> > >
> > > ___
> > > Piglit mailing list
> > > Piglit@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/piglit
> >
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

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

2016-08-01 Thread Anuj Phogat 
On Mon, Aug 1, 2016 at 3:36 PM, Nanley Chery  wrote:

> On Mon, Aug 01, 2016 at 03:16:17PM -0700, Anuj Phogat wrote:
> > Cc: Nanley Chery 
> > Signed-off-by: Anuj Phogat 
> > ---
> >  .../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.
​


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

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

2016-08-01 Thread Nanley Chery 
On Mon, Aug 01, 2016 at 03:16:17PM -0700, Anuj Phogat wrote:
> Cc: Nanley Chery 
> Signed-off-by: Anuj Phogat 
> ---
>  .../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.

- Nanley

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

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

2016-08-01 Thread Anuj Phogat 
Cc: Nanley Chery 
Signed-off-by: Anuj Phogat 
---
 .../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);
+
block_dims++;
}
return PIGLIT_PASS;
-- 
2.5.5

___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit