[Piglit] [PATCH] tex-miplevel-selection: only require glsl 1.30 for textureOffset 2DArrayShadow

[Roland Scheidegger]

Am 19.04.2016 um 19:27 schrieb Jose Fonseca:
> On 19/04/16 18:21, Roland Scheidegger wrote:
>> Am 19.04.2016 um 18:41 schrieb Jose Fonseca:
>>> On 19/04/16 01:32, sroland at vmware.com wrote:
>>>> From: Roland Scheidegger <sroland at vmware.com>
>>>>
>>>> The spec doesn't really say this should work in older versions. It was
>>>> first
>>>> added in glsl 4.30, mentioning it was forgotten (initially part of
>>>> EXT_gpu_shader4, hence should have been added with 1.30), but with the
>>>> wrong
>>>> syntax. Finally fixed in glsl 4.40.
>>>> It does, however, work with nvidia blob with version 130 directive.
>>>> Also works with llvmpipe (with mesa fix).
>>>> ---
>>>>    tests/texturing/tex-miplevel-selection.c | 6 ------
>>>>    1 file changed, 6 deletions(-)
>>>>
>>>> diff --git a/tests/texturing/tex-miplevel-selection.c
>>>> b/tests/texturing/tex-miplevel-selection.c
>>>> index 959bab2..59030b5 100644
>>>> --- a/tests/texturing/tex-miplevel-selection.c
>>>> +++ b/tests/texturing/tex-miplevel-selection.c
>>>> @@ -322,12 +322,6 @@ piglit_init(int argc, char **argv)
>>>>        }
>>>>        piglit_require_gl_version(NEED_GL3(test) ? 30 : 14);
>>>>
>>>> -    if (target == TEX_2D_ARRAY_SHADOW &&
>>>> -        test == GL3_TEXTURE_OFFSET) {
>>>> -        piglit_require_GLSL_version(430);
>>>> -        version = "430";
>>>> -    }
>>>> -
>>>>        switch (target) {
>>>>        case TEX_1D:
>>>>            gltarget = GL_TEXTURE_1D;
>>>>
>>>
>>> One can't blame IHVs for taking the spec to the letter, instead of
>>> guessing intent.
>> You are right, seems iffy (albeit if you take the spec _really_
>> literally,
>> it won't work with 430 neither due to the bogus syntax there).
>>
>>> An addition to the change above, how about marking the test as a SKIP
>>> when the IHV throws a error compiling the GLSL (and maybe put a friendly
>>> warning), but still FAIL if the does not throw a compilation error and
>>> produces the wrong results?
>>
>> I think this change can just be dropped. (The test is only about
>> miplevel selection, so the offsets don't really matter all that much.)
>> I mostly just used this quick change to confirm that nvidia supports it
>> all the way down to 130, and to verify the mesa glsl change works.
>>
>> Roland
>>
> 
> As you think best.
> 
> But given the ambiguity in the spec, I think it would be nice to have
> some test in Piglit for it.  In the hope vendors converge, and also to
> quickly check when they don't.
> 
> If the tex-miplevel-selection is the easiest for doing it that, I'd say
> go for it.
> 
> Eitherway, FWIW, the patch is
> 
> Acked-by: Jose Fonseca <jfonseca at vmware.com>

I quickly tried to do what you suggested, but due to the way this code
works and the helpers used, we can't actually easily make the test not
fail when compilation fails.
So, maybe a different test would be best, but I'm not working on any.

Roland