[Piglit] [PATCH 2/6] arb_bindless_texture: add uniform-block-initializer.frag compiler tests

[Samuel Pitoiset]

On 04/27/2017 02:20 AM, Timothy Arceri wrote:
> 
> On 27/04/17 08:52, Samuel Pitoiset wrote:
>> "Replace Section 4.1.7 (Samplers), p. 25"
>>
>>      "As uniforms in the default uniform block, samplers may be
>>       initialized only with the OpenGL API; they cannot be declared
>>       with an initializer in a shader."
>>
>> "Replace Section 4.1.X, (Images)"
>>
>>      "As uniforms in the default uniform block, images may be
>>       initialized only with the OpenGL API; they cannot be declared
>>       with an initializer in a shader."
> 
> 
> You are missing the bit to say where you are quoting this text from in 
> this commit message. I know it should be obvious but would be nice to 
> add for completeness.

Yeah, it looked obvious but I can add "The ARB_bindless_texture spec 
says:" for sure. :)

> 
>>
>> Just to be sure samplers/images are not too relaxed when bindless
>> texture is enabled.
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>   .../compiler/images/uniform-block-initializer.frag | 23 
>> ++++++++++++++++++++++
>>   .../samplers/uniform-block-initializer.frag        | 22 
>> +++++++++++++++++++++
>>   2 files changed, 45 insertions(+)
>>   create mode 100644 
>> tests/spec/arb_bindless_texture/compiler/images/uniform-block-initializer.frag 
>>
>>   create mode 100644 
>> tests/spec/arb_bindless_texture/compiler/samplers/uniform-block-initializer.frag 
>>
>>
>> diff --git 
>> a/tests/spec/arb_bindless_texture/compiler/images/uniform-block-initializer.frag 
>> b/tests/spec/arb_bindless_texture/compiler/images/uniform-block-initializer.frag 
>>
>> new file mode 100644
>> index 000000000..2148ea04a
>> --- /dev/null
>> +++ 
>> b/tests/spec/arb_bindless_texture/compiler/images/uniform-block-initializer.frag 
>>
>> @@ -0,0 +1,23 @@
>> +// [config]
>> +// expect_result: fail
>> +// glsl_version: 3.30
>> +// require_extensions: GL_ARB_bindless_texture 
>> GL_ARB_shader_image_load_store
>> +// [end config]
>> +
>> +#version 330
>> +#extension GL_ARB_bindless_texture: require
>> +#extension GL_ARB_shader_image_load_store: enable
>> +
>> +// The ARB_bindless_texture spec says:
>> +//
>> +//  "Replace Section 4.1.X, (Images)"
>> +//
>> +//  "As uniforms in the default uniform block, images may be initialized
>> +//   only with the OpenGL API; they cannot be declared with an 
>> initializer
>> +//   in a shader."
>> +
>> +writeonly uniform image2D img = 0 > +
>> +void main()
>> +{
>> +}
> 
> I don't really like tests like this. IMO when testing variable/uniforms 
> we should always make sure they are used. There is nothing in the spec 
> that says the uniform cannot be optimised away before validation is 
> done. I'm actually a little surprised that doesn't happen in this case, 
> my feeling is the test would fail on Nvidia where they are very 
> aggressive with optimisations. The spec also doesn't say if this is a 
> link or compile time error.

A compile-time error makes more sense to me.

I will update the test.

> 
> Either way I'd rather have the uniform used.
> 
> 
>> diff --git 
>> a/tests/spec/arb_bindless_texture/compiler/samplers/uniform-block-initializer.frag 
>> b/tests/spec/arb_bindless_texture/compiler/samplers/uniform-block-initializer.frag 
>>
>> new file mode 100644
>> index 000000000..d76ab8442
>> --- /dev/null
>> +++ 
>> b/tests/spec/arb_bindless_texture/compiler/samplers/uniform-block-initializer.frag 
>>
>> @@ -0,0 +1,22 @@
>> +// [config]
>> +// expect_result: fail
>> +// glsl_version: 3.30
>> +// require_extensions: GL_ARB_bindless_texture
>> +// [end config]
>> +
>> +#version 330
>> +#extension GL_ARB_bindless_texture: require
>> +
>> +// The ARB_bindless_texture spec says:
>> +//
>> +//  "Replace Section 4.1.7 (Samplers), p. 25"
>> +//
>> +//  "As uniforms in the default uniform block, samplers may be 
>> initialized
>> +//   only with the OpenGL API; they cannot be declared with an 
>> initializer
>> +//   in a shader."
>> +
>> +uniform sampler2D tex = 0;
>> +
>> +void main()
>> +{
>> +}
>>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit