On Mon, Nov 23, 2015 at 1:28 PM, Ilia Mirkin <imir...@alum.mit.edu> wrote:
> On Mon, Nov 23, 2015 at 4:16 PM, Ian Romanick <i...@freedesktop.org> wrote: > > On 11/23/2015 12:43 PM, Ilia Mirkin wrote: > >> On Mon, Nov 23, 2015 at 3:34 PM, Ian Romanick <i...@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. > >>> > Thanks for noticing this issue. > >>> 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? > I actually have a patch sitting on the list which redoes some of the logic: http://lists.freedesktop.org/archives/piglit/2015-October/017743.html The test previously wasn't splitting up into subtests correctly. - Nanley
_______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit