On Thu, 2016-01-14 at 09:18 -0800, Matt Turner wrote: > On Wed, Jan 13, 2016 at 9:45 PM, Timothy Arceri > <timothy.arc...@collabora.com> wrote: > > While the spec is taking about multiple layout qualifiers we > > interpret that > > to mean layout-qualifier-names can also occur multiple times within > > a single layout qualifier. > > > > In Section 4.4 (Layout Qualifiers) of the GLSL 4.40 spec it > > clarifies this: > > > > "More than one layout qualifier may appear in a single > > declaration. > > Additionally, the same layout-qualifier-name can occur multiple > > times > > within a layout qualifier or across multiple layout qualifiers > > in the > > same declaration" > > > > The results from the Nvidia binary are a little all over the place > > with these > > tests, most of the tests with arb_shading_language_420pack enabled > > fail but > > if the GLSL version is changed to 4.20 they pass. > > The only tests that pass with arb_shading_language_420pack enabled > > are the > > global tests but that seems to be because they do not detect the > > duplicates > > and fail the negative tests. > > > > V2: fix version mismatch in location tests, and prefix variable > > with out > > in global tests > > --- > > You seem to be interpreting this as a clarification on the earlier > spec text rather than as a behavioral change based on NVIDIA's > acceptance when using #version 420. I don't necessarily agree -- > their > drivers are known to accept out-of-spec shaders. > > In fact (as I'm sure you know) ARB_enhanced_layouts, explicitly > expands the paragraph cited to contain new text about > layout-qualifier-names.
I actually totally missed that, thanks. > I think this indicates an actual behavioral > change and not a clarification meant to be applied to earlier shader > versions. I spent most of my time searching the older specs trying to find something that says duplicates were not allowed in the first place ... I couldn't find anything it seems undefined and drivers default to not allowed. The only thing I could find were sections specificly allowing it, Section 4.3.8.1 (Input Layout Qualifiers) in the 4.10 spec: "Any or all of these identifiers may be specified one or more times in a single input layout declaration. If primitive mode, vertex spacing, or ordering is declared more than once in the tessellation evaluation shaders of a program, all such declarations must use the same identifier." I guess from that you can assume they are not normally allowed. Anyway I think enabling this for enhanced layouts makes the most sense as per your suggestion. I'll update the tests and Mesa patch thanks for the feedback. > > I think the tests have value, but I think that they should be testing > ARB_enhanced_layouts instead of 420pack. > > > ...e-qualifier-in-single-declaration-420-pack.vert | 22 > > +++++++++++++ > > ...ualifier-in-single-declaration-no-420-pack.vert | 21 > > ++++++++++++ > > ...n-qualifier-in-single-declaration-420-pack.vert | 33 > > +++++++++++++++++++ > > ...ualifier-in-single-declaration-no-420-pack.vert | 32 > > ++++++++++++++++++ > > ...iple-stream-in-single-declaration-420-pack.geom | 36 > > ++++++++++++++++++++ > > ...e-stream-in-single-declaration-no-420-pack.geom | 35 > > ++++++++++++++++++++ > > ...ream-in-single-global-declaration-420-pack.geom | 38 > > ++++++++++++++++++++++ > > ...m-in-single-global-declaration-no-420-pack.geom | 37 > > +++++++++++++++++++++ > > ...e-qualifier-in-single-declaration-420-pack.geom | 25 > > ++++++++++++++ > > ...ualifier-in-single-declaration-no-420-pack.geom | 24 > > ++++++++++++++ > > ...fier-in-single-global-declaration-420-pack.geom | 27 > > +++++++++++++++ > > ...r-in-single-global-declaration-no-420-pack.geom | 37 > > +++++++++++++++++++++ > > 12 files changed, 367 insertions(+) > > create mode 100644 > > tests/spec/arb_shading_language_420pack/compiler/layout > > -qualifiers/multiple-location-multiple-qualifier-in-single > > -declaration-420-pack.vert > > create mode 100644 > > tests/spec/arb_shading_language_420pack/compiler/layout > > -qualifiers/multiple-location-multiple-qualifier-in-single > > -declaration-no-420-pack.vert > > create mode 100644 > > tests/spec/arb_shading_language_420pack/compiler/layout > > -qualifiers/multiple-location-qualifier-in-single-declaration-420 > > -pack.vert > > create mode 100644 > > tests/spec/arb_shading_language_420pack/compiler/layout > > -qualifiers/multiple-location-qualifier-in-single-declaration-no > > -420-pack.vert > > create mode 100644 > > tests/spec/arb_shading_language_420pack/compiler/layout > > -qualifiers/multiple-stream-in-single-declaration-420-pack.geom > > create mode 100644 > > tests/spec/arb_shading_language_420pack/compiler/layout > > -qualifiers/multiple-stream-in-single-declaration-no-420-pack.geom > > create mode 100644 > > tests/spec/arb_shading_language_420pack/compiler/layout > > -qualifiers/multiple-stream-in-single-global-declaration-420 > > -pack.geom > > create mode 100644 > > tests/spec/arb_shading_language_420pack/compiler/layout > > -qualifiers/multiple-stream-in-single-global-declaration-no-420 > > -pack.geom > > create mode 100644 > > tests/spec/arb_shading_language_420pack/compiler/layout > > -qualifiers/multiple-stream-multiple-qualifier-in-single > > -declaration-420-pack.geom > > create mode 100644 > > tests/spec/arb_shading_language_420pack/compiler/layout > > -qualifiers/multiple-stream-multiple-qualifier-in-single > > -declaration-no-420-pack.geom > > create mode 100644 > > tests/spec/arb_shading_language_420pack/compiler/layout > > -qualifiers/multiple-stream-multiple-qualifier-in-single-global > > -declaration-420-pack.geom > > create mode 100644 > > tests/spec/arb_shading_language_420pack/compiler/layout > > -qualifiers/multiple-stream-multiple-qualifier-in-single-global > > -declaration-no-420-pack.geom > > > > diff --git > > a/tests/spec/arb_shading_language_420pack/compiler/layout > > -qualifiers/multiple-location-multiple-qualifier-in-single > > -declaration-420-pack.vert > > b/tests/spec/arb_shading_language_420pack/compiler/layout > > -qualifiers/multiple-location-multiple-qualifier-in-single > > -declaration-420-pack.vert > > new file mode 100644 > > index 0000000..9551476 > > --- /dev/null > > +++ b/tests/spec/arb_shading_language_420pack/compiler/layout > > -qualifiers/multiple-location-multiple-qualifier-in-single > > -declaration-420-pack.vert > > @@ -0,0 +1,22 @@ > > +// [config] > > +// expect_result: pass > > +// glsl_version: 1.40 > > +// require_extensions: GL_ARB_separate_shader_objects > > GL_ARB_shading_language_420pack > > +// check_link: false > > +// [end config] > > +// > > +// From the ARB_shading_language_420pack spec: > > +// > > +// "More than one layout qualifier may appear in a single > > declaration. If > > +// the same layout-qualifier-name occurs in multiple layout > > qualifiers for > > +// the same declaration, the last one overrides the former > > ones." > > + > > +#version 140 > > +#extension GL_ARB_separate_shader_objects : enable > > +#extension GL_ARB_shading_language_420pack: enable > > + > > +layout(location=2) layout(location=1) out vec3 var; > > + > > +void main() > > +{ > > +} > > diff --git > > a/tests/spec/arb_shading_language_420pack/compiler/layout > > -qualifiers/multiple-location-multiple-qualifier-in-single > > -declaration-no-420-pack.vert > > b/tests/spec/arb_shading_language_420pack/compiler/layout > > -qualifiers/multiple-location-multiple-qualifier-in-single > > -declaration-no-420-pack.vert > > new file mode 100644 > > index 0000000..e5cbeb4 > > --- /dev/null > > +++ b/tests/spec/arb_shading_language_420pack/compiler/layout > > -qualifiers/multiple-location-multiple-qualifier-in-single > > -declaration-no-420-pack.vert > > @@ -0,0 +1,21 @@ > > +// [config] > > +// expect_result: fail > > +// glsl_version: 1.40 > > +// require_extensions: GL_ARB_separate_shader_objects > > +// check_link: false > > +// [end config] > > +// > > +// From the ARB_shading_language_420pack spec: > > +// > > +// "More than one layout qualifier may appear in a single > > declaration. If > > +// the same layout-qualifier-name occurs in multiple layout > > qualifiers for > > +// the same declaration, the last one overrides the former > > ones." > > + > > +#version 140 > > +#extension GL_ARB_separate_shader_objects : enable > > + > > +layout(location=2) layout(location=1) out vec3 var; > > + > > +void main() > > +{ > > +} > > diff --git > > a/tests/spec/arb_shading_language_420pack/compiler/layout > > -qualifiers/multiple-location-qualifier-in-single-declaration-420 > > -pack.vert > > b/tests/spec/arb_shading_language_420pack/compiler/layout > > -qualifiers/multiple-location-qualifier-in-single-declaration-420 > > -pack.vert > > new file mode 100644 > > index 0000000..cff5d51 > > --- /dev/null > > +++ b/tests/spec/arb_shading_language_420pack/compiler/layout > > -qualifiers/multiple-location-qualifier-in-single-declaration-420 > > -pack.vert > > @@ -0,0 +1,33 @@ > > +// [config] > > +// expect_result: pass > > +// glsl_version: 1.40 > > +// require_extensions: GL_ARB_separate_shader_objects > > GL_ARB_shading_language_420pack > > +// check_link: false > > +// [end config] > > +// > > +// From the ARB_shading_language_420pack spec: > > +// > > +// "More than one layout qualifier may appear in a single > > declaration. If > > +// the same layout-qualifier-name occurs in multiple layout > > qualifiers for > > +// the same declaration, the last one overrides the former > > ones." > > +// > > +// While the spec is taking about multiple layout qualifiers we > > interpert that > > typo: interpret > > Occurs elsewhere as well. _______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit