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

[Ilia Mirkin]

On Mon, Nov 23, 2015 at 4:16 PM, Ian Romanick <idr at freedesktop.org> wrote:
> 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.

Ah that's perfect. I should have checked the man page when I was
futzing with the test.

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

Right now there are 3 subtests (ldr, hdr, srgb), each of which tests
loading 3 sets of data (ldr, hdr, and ldr srgb). If there's no
astc_hdr support, the hdr subtest isn't run, and when loading hdr
data, it checks for the error color. Instead of 3 subtests, I could
split this up into 9 subtests for the full cross -- does that sound
reasonable, and then we'd pass the ldr-data ones and fail the hdr data
ones.

TBH I'm unsure what the diff between the ldr and hdr subtests is...
they seem identical except that HDR one skips if you don't hdr. But
otherwise it's identical to the LDR one from what I can tell. Also
from the looks of it the srgb test doesn't need the full cross either,
it only runs against the ldrs data. Urgh, I guess this test needs a
bit of a redo :( Perhaps we can sucker Nanley into fixing it up?

  -ilia