On 11 November 2015 at 21:10, Timothy Arceri <[email protected]> wrote: > On Wed, 2015-11-11 at 18:14 +0000, Emil Velikov wrote: >> From: Emil Velikov <[email protected]> >> >> As per the spec hunk: >> "The specified offset must be a >> multiple of the base alignment of the type of the block member it >> qualifies, or a compile-time error results." >> >> v2: >> - Fix typo - enhanced-layout > enhanced-layouts >> - Prefix uniform tests with ubo >> - Add ssbo equivalent tests >> - Quote the correct spec hunk in commit msg >> >> v3: >> - Remove trailing whitespace (Tim) >> - Drop glsl versoin to 1.40 for the ssbo tests (Tim, Ilia) >> - Change check_link to true to match below test results (Tim) > > Again I think you had this right the first time, the spec clearly says compile > -time error. Sorry for the confusion. > Nothing to apologise for. I've completely missed your reply on the topic :-\ Will revert to previous behaviour.
>> >> Test results (Tim): >> Nvidia GeForce 840M - NVIDIA 352.41: pass >> >> Signed-off-by: Emil Velikov <[email protected]> >> --- >> .../ssbo-offset-multiple-of-base-member-align.vert | 27 >> ++++++++++++++++++++++ >> .../ubo-offset-multiple-of-base-member-align.vert | 26 >> +++++++++++++++++++++ >> 2 files changed, 53 insertions(+) >> create mode 100644 tests/spec/arb_enhanced_layouts/compiler/explicit >> -offsets/ssbo-offset-multiple-of-base-member-align.vert >> create mode 100644 tests/spec/arb_enhanced_layouts/compiler/explicit >> -offsets/ubo-offset-multiple-of-base-member-align.vert >> >> diff --git a/tests/spec/arb_enhanced_layouts/compiler/explicit-offsets/ssbo >> -offset-multiple-of-base-member-align.vert >> b/tests/spec/arb_enhanced_layouts/compiler/explicit-offsets/ssbo-offset >> -multiple-of-base-member-align.vert >> new file mode 100644 >> index 0000000..231d445 >> --- /dev/null >> +++ b/tests/spec/arb_enhanced_layouts/compiler/explicit-offsets/ssbo-offset >> -multiple-of-base-member-align.vert >> @@ -0,0 +1,27 @@ >> +// [config] >> +// expect_result: fail >> +// glsl_version: 1.40 >> +// require_extensions: GL_ARB_enhanced_layouts >> GL_ARB_shader_storage_buffer_object >> +// check_link: true >> +// [end config] >> +// >> +// ARB_enhanced_layouts spec says: >> +// "The specified offset must be a >> +// multiple of the base alignment of the type of the block member it >> +// qualifies, or a compile-time error results." >> +// >> +// Tests for successful compilation, when the block is of std140 layout. >> +// >> + >> +#version 140 >> +#extension GL_ARB_enhanced_layouts : enable >> +#extension GL_ARB_shader_storage_buffer_object : enable >> + >> +layout(std430) buffer b { >> + layout(offset = 0) float f1; >> + layout(offset = 2) float f2; // Wrong: offset must be aligned to > > > I think this should be: > > layout(offset = 6) float f2; > > Otherwise the test could pass because the offset lies within the previous > member of the block not because the alignment is wrong. > I believe you mean "Otherwise the test _should fail_ ...." in the above. Although you're spot on - we should be checking the offset alignment here, not if members overlap. > Same goes for UBOs. > > With these changes and removing the link time check. > > Reviewed-by: Timothy Arceri <[email protected]> > Thanks Emil _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
