[Piglit] [PATCH v2 3/5] arb-enhanced-layouts: explicit-offset: relative offset values

[Timothy Arceri]

On Mon, 2015-11-02 at 16:28 +0000, Emil Velikov wrote:
> On 1 November 2015 at 22:03, Timothy Arceri
> <timothy.arceri at collabora.com> wrote:
> > On Tue, 2015-10-27 at 15:34 +0000, Emil Velikov wrote:
> > > From: Emil Velikov <emil.velikov at collabora.com>
> > > 
> > > Check if one member is (attempted to be) positioned on top of another,
> > > and that the assigned offset(s) increase naturally.
> > > 
> > > v2:
> > >  - Fix typo - enhanced-layout > enhanced-layouts
> > >  - Prefix uniform tests with ubo
> > >  - Add ssbo equivalent tests
> > > 
> > > Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> > > ---
> > >  .../explicit-offsets/ssbo-decreasing-offset.vert   | 29
> > > +++++++++++++++++++++
> > >  .../ssbo-members-stamping-each-other.vert          | 30
> > > ++++++++++++++++++++++
> > >  .../ssbo-multiple-members-same-offset.vert         | 30
> > > ++++++++++++++++++++++
> > >  .../explicit-offsets/ubo-decreasing-offset.vert    | 27
> > > +++++++++++++++++++
> > >  .../ubo-members-stamping-each-other.vert           | 29
> > > +++++++++++++++++++++
> > >  .../ubo-multiple-members-same-offset.vert          | 29
> > > +++++++++++++++++++++
> > >  6 files changed, 174 insertions(+)
> > >  create mode 100644 tests/spec/arb_enhanced_layouts/compiler/explicit
> > > -offsets/ssbo-decreasing-offset.vert
> > >  create mode 100644 tests/spec/arb_enhanced_layouts/compiler/explicit
> > > -offsets/ssbo-members-stamping-each-other.vert
> > >  create mode 100644 tests/spec/arb_enhanced_layouts/compiler/explicit
> > > -offsets/ssbo-multiple-members-same-offset.vert
> > >  create mode 100644 tests/spec/arb_enhanced_layouts/compiler/explicit
> > > -offsets/ubo-decreasing-offset.vert
> > >  create mode 100644 tests/spec/arb_enhanced_layouts/compiler/explicit
> > > -offsets/ubo-members-stamping-each-other.vert
> > >  create mode 100644 tests/spec/arb_enhanced_layouts/compiler/explicit
> > > -offsets/ubo-multiple-members-same-offset.vert
> > > 
> > > diff --git a/tests/spec/arb_enhanced_layouts/compiler/explicit
> > > -offsets/ssbo
> > > -decreasing-offset.vert
> > > b/tests/spec/arb_enhanced_layouts/compiler/explicit
> > > -offsets/ssbo-decreasing-offset.vert
> > > new file mode 100644
> > > index 0000000..ba3678a
> > > --- /dev/null
> > > +++ b/tests/spec/arb_enhanced_layouts/compiler/explicit-offsets/ssbo
> > > -decreasing-offset.vert
> > > @@ -0,0 +1,29 @@
> > > +// [config]
> > > +// expect_result: fail
> > > +// glsl_version: 4.30
> > > +// require_extensions: GL_ARB_enhanced_layouts
> > > GL_ARB_shader_storage_buffer_object
> > > +// check_link: false
> > > +// [end config]
> > > +//
> > > +// ARB_enhanced_layouts spec says:
> > > +//    "It is a compile-time error to
> > > +//    specify an *offset* that is smaller than the offset of the
> > > previous
> > > +//    member in the block..."
> > > +//
> > > +// Tests whether assigning a smaller offset for sequential member
> > > triggers
> > > +// a compile-time error.
> > > +//
> > > +
> > > +#version 430
> > > +#extension GL_ARB_enhanced_layouts : enable
> > > +#extension GL_ARB_shader_storage_buffer_object : enable
> > > +
> > > +
> > > +layout(std430) buffer b {
> > > +       layout(offset = 32) vec4 var1;
> > > +       layout(offset = 0) vec4 var2; // Wrong: offset but be larger
> > > than
> > > one of previous member
> > 
> > "offset must be larger than that of a previous member" ?
> > 
> This sounds a lot better.
> 
> ...
> > > +// Tests whether assigning the same offsets for multiple members
> > > trigger
> > > +// a compile-time error.
> > > +// XXX: fuzz for other (all?) types ?
> > 
> > Remove the XXX comment? Once this is commited its highly unlikely anyone
> > will
> > notice it or care.
> > 
> It's meant to inspire some input during the review stage. And see how
> deeply people read through the test ;-)
> Will drop the comment.
> ...
> > > +// Tests whether assigning the same offsets for multiple members
> > > trigger
> > > +// a compile-time error.
> > > +// Note: not explicitly mentioned in the spec.
> > 
> > You can remove the "Note" as the spec covers this with "that lies within
> > the
> > previous member of the block" right?
> > 
> Keyword here is "explicit" - the spec covers overlapping members in
> ''generic' way without going into details about this case :-) Which
> makes we wonder - should we even bother with this (and the ssbo) test
> ?

I would leave them they don't hurt, up to you.

> 
> Thanks
> Emil