On 31 August 2012 16:06, Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>> wrote: <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> The ARB_explicit_attrib_location tests layout-12.frag and layout-13.frag were incorrect: they both used index == 2 (which is invalid) and expected the compile to succeed. Although the extension spec doesn't state anything about generating an error on indices other than 0 or 1, the GLSL 4.30 spec clarifies this as a compile error. This matches the behavior of the API call defined in ARB_blend_func_extended, glBindFragDataLocationIndexed, which generates INVALID_VALUE for indices other than 0 or 1. This patch changes layout-13.frag to use index = 1 so it will actually test the intended behavior. layout-12.frag was just bogus: the comment said "Even though the specified locations overlap [...]" but the test only had one variable. This patch turns it into a negative test to ensure index == 2 is a compile error. This patch also adds layout-14.frag, a test to check that index < 0 is also a compile error. </blockquote><div> Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> --- .../1.20/compiler/layout-12.frag | 27 ++++++---------------- .../1.20/compiler/layout-13.frag | 4 ++-- .../1.20/compiler/layout-14.frag | 21 +++++++++++++++++ 3 files changed, 30 insertions(+), 22 deletions(-) create mode 100644 tests/spec/arb_explicit_attrib_location/1.20/compiler/layout-14.frag diff --git a/tests/spec/arb_explicit_attrib_location/1.20/compiler/layout-12.frag b/tests/spec/arb_explicit_attrib_location/1.20/compiler/layout-12.frag index d337eb5..e7dab5d 100644 --- a/tests/spec/arb_explicit_attrib_location/1.20/compiler/layout-12.frag +++ b/tests/spec/arb_explicit_attrib_location/1.20/compiler/layout-12.frag @@ -1,28 +1,15 @@ // [config] -// expect_result: pass +// expect_result: fail // glsl_version: 1.20 // require_extensions: GL_ARB_explicit_attrib_location GL_ARB_blend_func_extended // [end config] // -// Even though the specified locations overlap, the spec says that a *link* -// error is generated. The changes to section 3.9.2 say: -// -// "Output binding assignments will cause LinkProgram to fail: -// -// * if the number of active outputs is greater than the value of -// MAX_DRAW_BUFFERS; -// -// * if the program has an active output assigned to a location -// greater than or equal to the value of -// MAX_DUAL_SOURCE_DRAW_BUFFERS and has an active output assigned -// an index greater than or equal to one; -// -// * if more than one varying out variable is bound to the same -// number and index; or -// -// * if the explicit binding assigments do not leave enough space -// for the linker to automatically assign a location for a varying -// out array, which requires multiple contiguous locations." +// While the GL_ARB_explicit_attrib_location specification does not say +// anything about generating errors for invalid indices, the GLSL 4.30 +// spec clarifies this: "It is also a compile-time error if a fragment shader +// sets a layout index to less than 0 or greater than 1." This matches the +// behavior of the equivalent API call, glBindFragDataLocationIndexed, which +// generates an INVALID_VALUE error if <index> is not 0 or 1. #version 120 #extension GL_ARB_explicit_attrib_location: require diff --git a/tests/spec/arb_explicit_attrib_location/1.20/compiler/layout-13.frag b/tests/spec/arb_explicit_attrib_location/1.20/compiler/layout-13.frag index 3124e7f..a57a108 100644 --- a/tests/spec/arb_explicit_attrib_location/1.20/compiler/layout-13.frag +++ b/tests/spec/arb_explicit_attrib_location/1.20/compiler/layout-13.frag @@ -26,8 +26,8 @@ #version 120 #extension GL_ARB_explicit_attrib_location: require -layout(location = 0, index = 2) out vec4 color; -layout(location = 0, index = 2) out vec4 factor; +layout(location = 0, index = 1) out vec4 color; +layout(location = 0, index = 1) out vec4 factor; void main() { diff --git a/tests/spec/arb_explicit_attrib_location/1.20/compiler/layout-14.frag b/tests/spec/arb_explicit_attrib_location/1.20/compiler/layout-14.frag new file mode 100644 index 0000000..045fb8e --- /dev/null +++ b/tests/spec/arb_explicit_attrib_location/1.20/compiler/layout-14.frag @@ -0,0 +1,21 @@ +// [config] +// expect_result: fail +// glsl_version: 1.20 +// require_extensions: GL_ARB_explicit_attrib_location GL_ARB_blend_func_extended +// [end config] +// +// While the GL_ARB_explicit_attrib_location specification does not say +// anything about generating errors for invalid indices, the GLSL 4.30 +// spec clarifies this: "It is also a compile-time error if a fragment shader +// sets a layout index to less than 0 or greater than 1." This matches the +// behavior of the equivalent API call, glBindFragDataLocationIndexed, which +// generates an INVALID_VALUE error if <index> is not 0 or 1. + +#version 120 +#extension GL_ARB_explicit_attrib_location: require +layout(location = 0, index = -5) out vec4 color; + +void main() +{ + color = vec4(1.0); +} -- 1.7.11.4 _______________________________________________ Piglit mailing list <a href="mailto:Piglit@lists.freedesktop.org">Piglit@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/piglit" target="_blank">http://lists.freedesktop.org/mailman/listinfo/piglit</a> </blockquote></div>