[Piglit] [PATCH] astc: avoid deleting a random texture

[Ian Romanick]

On 11/23/2015 12:43 PM, Ilia Mirkin wrote:
> On Mon, Nov 23, 2015 at 3:34 PM, Ian Romanick <idr at freedesktop.org> wrote:
>> On 11/21/2015 04:08 PM, Ilia Mirkin wrote:
>>> In the check_error case, decompressed_tex is completely uninitialized
>>> and might point to any texture. This can wreak various havoc.
>>
>> I might suggest a different approach.  In the check_error case, ensure
>> that decompress_texture is zero.  Remove the check_error parameter, and
>> s/!check_error/decompress_texture ==
>> 0/g;s/check_error/decompress_texture != 0/g.
> 
> Is it legal to delete texture 0? Or can texture 0 be a real thing?
> Otherwise I can just initialize it to that below.

It's similar to free(NULL) in that respect.  From the glDeleteTextures
man page:

       glDeleteTextures silently ignores 0's and names that do not correspond
       to existing textures.

> Separately, how would you feel about making the error color decoding
> check failing return a warn instead of a fail? I can't get Adreno A420
> to spit it out... haven't tried on blob, perhaps I'm missing some bit
> of programming. I just get black with the HDR data instead of the
> error color.

Hmm... I don't know how this particular test is structured.  When we've
had cases like this in the past, we've split the test into subcases.
One (or several) subcase checks the things that the driver in question
can pass, and one, single subcase checks the thing that the driver
fails.

If this test is conducive that kind of a split (perhaps by running it
twice with different data), that would be my preference.

>> It would also be cool if this test used piglit_draw_rect_tex instead of
>> open-coding it.
> 
> It would be even cooler if this test passed for me with -auto as well
> as without it... for now it only passes without -auto for me. I'm at a
> total loss as to what the difference is. I'm blaming DRI3 and GLAMOR.
> Anyways, I'm not in the business of making every piglit test pretty :)
> "It was like that when I got there."

Fair enough.  You can't blame me for trying.

>   -ilia
> 
>>
>>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>> ---
>>>  tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> 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 20f2415..d9c1c30 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
>>> @@ -213,7 +213,8 @@ bool draw_compare_levels(bool check_error, bool check_srgb,
>>>
>>>       /* Delete bound textures */
>>>       glDeleteTextures(1, &compressed_tex);
>>> -     glDeleteTextures(1, &decompressed_tex);
>>> +     if (!check_error)
>>> +             glDeleteTextures(1, &decompressed_tex);
>>>
>>>       piglit_present_results();
>>>       return pass;
>>>