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

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