[Piglit] [PATCH 2/5] khr_texture_compression_astc: Skip the decoding for sRGB textures

[Nanley Chery]

On Mon, Jul 18, 2016 at 03:39:22PM -0700, Nanley Chery wrote:
> On Thu, Jul 07, 2016 at 04:28:10PM -0700, Anuj Phogat wrote:
> > Both compressed and decompressed sRGB textures in the test are decoded
> > to linear color by sampler. The loss of precision during this conversion
> > caused the subtest failure. It can be fixed by avoiding the sRGB decoding
> > for sRGB textures.
> > 
> > Skipping the decoding in non-sRGB cases isn't serving any purpose.
> > 
> > Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> 
> Thanks for catching this mistake! This patch is,
> Reviewed-by: Nanley Chery <nanley.g.chery at intel.com>
> 

I unfortunately may have spoken too soon. The code for skipping ASTC sRGB
decode via glTexParameteri seems incorrect. I'm not so sure we can
substitute the sRGB format with the non-sRGB format as they are stored
differently on-disk.

> > ---
> >  .../spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c b/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c
> > index 6429c2e..5126820 100644
> > --- a/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c
> > +++ b/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c
> > @@ -171,7 +171,7 @@ bool draw_compare_levels(bool check_error, bool check_srgb,
> >  
> >  		/* Draw miplevel of compressed texture. */
> >  		glBindTexture(GL_TEXTURE_2D, compressed_tex);
> > -		if (!check_srgb)
> > +		if (check_srgb)

Given what I said above, I'm not sure we can call glTexParameteri for
the compressed texture. The rest of the patch makes sense however. Does
the subtest still pass if you remove the glTexParameteri call at this
point?

- Nanley

> >  			glTexParameteri(GL_TEXTURE_2D,
> >  					GL_TEXTURE_SRGB_DECODE_EXT,
> >  					GL_SKIP_DECODE_EXT);
> > @@ -181,7 +181,7 @@ bool draw_compare_levels(bool check_error, bool check_srgb,
> >  		/* Draw miplevel of decompressed texture. */
> >  		if (!check_error) {
> >  			glBindTexture(GL_TEXTURE_2D, decompressed_tex);
> > -			if (!check_srgb)
> > +			if (check_srgb)
> >  				glTexParameteri(GL_TEXTURE_2D,
> >  						GL_TEXTURE_SRGB_DECODE_EXT,
> >  						GL_SKIP_DECODE_EXT);
> > @@ -244,7 +244,7 @@ test_miptrees(void* input_type)
> >  		"12x12"
> >  	};
> >  
> > -	if (!is_srgb_test)
> > +	if (is_srgb_test)
> >  		piglit_require_extension("GL_EXT_texture_sRGB_decode");
> >  
> >  	GLint pixel_offset_loc = glGetUniformLocation(prog, "pixel_offset");
> > -- 
> > 2.5.5
> > 
> > _______________________________________________
> > Piglit mailing list
> > Piglit at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/piglit