[Piglit] [PATCH] arb_shader_storage_buffer_object: add memory qualifers test

[Timothy Arceri]

On Mon, 2015-08-17 at 08:06 +0300, Tapani Pälli wrote:
> 
> On 08/15/2015 03:43 AM, Timothy Arceri wrote:
> > On Thu, 2015-08-13 at 10:57 +0300, Tapani Pälli wrote:
> > > Test checks that parser is able to deal with memory qualifiers given
> > > to shader storage block.
> > > 
> > > Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> > > ---
> > >   .../preprocessor/memory-qualifiers.vert                      | 12
> > > ++++++++++++
> > >   1 file changed, 12 insertions(+)
> > >   create mode 100644
> > > tests/spec/arb_shader_storage_buffer_object/preprocessor/memory
> > > -qualifiers.vert
> > 
> > Please move this to the compiler directory this isnt a preprocessor test.
> 
> Our preprocessor currently chokes on this test so that's why I put it 
> here :)
> 
> > Also maybe call this buffer-memory-qualifiers.vert to distingish it from
> > qualifiers on members.
> 
> Yes, this was my thought too, I'll rename it.
> 
> > > 
> > > diff --git 
> > > a/tests/spec/arb_shader_storage_buffer_object/preprocessor/memory
> > > -qualifiers.vert
> > > b/tests/spec/arb_shader_storage_buffer_object/preprocessor/memory
> > > -qualifiers.vert
> > > new file mode 100644
> > > index 0000000..f996cee
> > > --- /dev/null
> > > +++ b/tests/spec/arb_shader_storage_buffer_object/preprocessor/memory
> > > -qualifiers.vert
> > > @@ -0,0 +1,12 @@
> > > +// [config]
> > > +// expect_result: pass
> > > +// glsl_version: 3.30
> > > +// require_extensions: GL_ARB_shader_storage_buffer_object
> > > +// [end config]
> > > +
> > > +#version 330
> > > +#extension GL_ARB_shader_storage_buffer_object: require
> > > +coherent restrict volatile buffer Buffer {
> > > +	vec4 foo;
> > > +} buf;
> > > +int foo(void) { return 1; }
> > 
> > I think the last line would look better as
> > 
> > vec4 foo(void) { return buf.foo; }
> 
> Well, it won't matter, the purpose of the test is to test that memory 
> qualifiers get parsed and don't cause errors.

Right but a lot of the time piglit tests catch errors they were not designed
for since we dont really have enough tests. I couldn't see any other compile
tests that do this simple access with a buffer name.

That said the test is fine without it its just a suggestion :)

> 
> > With those few comments addressed
> > 
> > Reviewed-by: Timothy Arceri <t_arceri at yahoo.com.au>
> >