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

[Nanley Chery]

On Mon, Jul 18, 2016 at 03:59:19PM -0700, Nanley Chery wrote:
> 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?
> 

(For the list,)
We reached the shared understanding that the test would not make sense
if we skipped sRGB decode on the compressed texture, but not the
decompressed texture.

- Nanley

> - 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