[Piglit] [PATCH V2] arb_shading_language_420pack: test multiple layout qualifiers on the same line

[Matt Turner]

On Wed, Jan 13, 2016 at 9:45 PM, Timothy Arceri
<timothy.arceri at 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 think this indicates an actual behavioral
change and not a clarification meant to be applied to earlier shader
versions.

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.