Re: [Piglit] [PATCH 1/2] arb_shading_language_420pack: check different binding points

[Matt Turner]

Reviewed-by: Matt Turner 
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH 1/2] arb_shading_language_420pack: check different binding points

[Andres Gomez]

This adds tests to check that a link error is expected when specifying
different binding points for matching Uniform Blocks, Shader Storage
Blocks or Opaque-Uniforms across multiple compilation units.

From page 93 (page 110 of the PDF) of the GL 4.2 (Core Profile) spec:

  " 2.11.7 Uniform Variables

...

Uniform Blocks

...

When a named uniform block is declared by multiple shaders in a
program, it must be declared identically in each shader. The
uniforms within the block must be declared with the same names and
types, and in the same order. If a program contains multiple
shaders with different declarations for the same named uniform
block differs between shader, the program will fail to link."

From page 129 (page 150 of the PDF) of the GL 4.3 (Core Profile) spec:

  " 7.8 Shader Buffer Variables and Shader Storage Blocks

...

When a named shader storage block is declared by multiple shaders
in a program, it must be declared identically in each shader. The
buffer variables within the block must be declared with the same
names, types, qualification, and declaration order. If a program
contains multiple shaders with different declarations for the same
named shader storage block, the program will fail to link."

From page 60 (page 66 of the PDF) of the GLSL 4.20 spec, v11:

  " A link error will result if two compilation units in a program
specify different integer-constant bindings for the same
opaque-uniform name. However, it is not an error to specify a
binding on some but not all declarations for the same name, as
shown in the examples below."

Although these restrictions are not included in the
ARB_shading_language_420pack spec, it is reasonable to believe that it
applies to it too.

v2:
- Added GL minimum version restrictions.
- Corrected GLSL minimum version restrictions.
- Corrected the outcome and documentation for the linking failures
  on UBOs and SSBOs.
- Added tests for UBOs and SSBOs without instance names.

Signed-off-by: Andres Gomez 
Cc: Matt Turner 
Cc: Ian Romanick 
---
 .../linker/different-bindings-image2D.shader_test  | 57 ++
 .../different-bindings-sampler2D.shader_test   | 53 +
 ...ngs-shader-storage-blocks-instanced.shader_test | 66 +
 ...rent-bindings-shader-storage-blocks.shader_test | 66 +
 ...t-bindings-uniform-blocks-instanced.shader_test | 69 ++
 .../different-bindings-uniform-blocks.shader_test  | 69 ++
 6 files changed, 380 insertions(+)
 create mode 100644 
tests/spec/arb_shading_language_420pack/linker/different-bindings-image2D.shader_test
 create mode 100644 
tests/spec/arb_shading_language_420pack/linker/different-bindings-sampler2D.shader_test
 create mode 100644 
tests/spec/arb_shading_language_420pack/linker/different-bindings-shader-storage-blocks-instanced.shader_test
 create mode 100644 
tests/spec/arb_shading_language_420pack/linker/different-bindings-shader-storage-blocks.shader_test
 create mode 100644 
tests/spec/arb_shading_language_420pack/linker/different-bindings-uniform-blocks-instanced.shader_test
 create mode 100644 
tests/spec/arb_shading_language_420pack/linker/different-bindings-uniform-blocks.shader_test

diff --git 
a/tests/spec/arb_shading_language_420pack/linker/different-bindings-image2D.shader_test
 
b/tests/spec/arb_shading_language_420pack/linker/different-bindings-image2D.shader_test
new file mode 100644
index 0..27ccda57b
--- /dev/null
+++ 
b/tests/spec/arb_shading_language_420pack/linker/different-bindings-image2D.shader_test
@@ -0,0 +1,57 @@
+/* The GLSL 4.20 spec, v11, says:
+ *
+ * "A link error will result if two compilation units in a program
+ *  specify different integer-constant bindings for the same
+ *  opaque-uniform name. However, it is not an error to specify a
+ *  binding on some but not all declarations for the same name, as
+ *  shown in the examples below."
+ *
+ * Although this restriction is not included in the
+ * ARB_shading_language_420pack spec, it is reasonable to believe that
+ * it applies to it too.
+ *
+ * Verify that a link error happens when using different binding
+ * points for an opaque type (image2D) with the same name in
+ * different compilation units.
+ */
+
+[require]
+GL >= 3.00
+GLSL >= 1.30
+GL_ARB_shading_language_420pack
+GL_ARB_shader_image_load_store
+
+[vertex shader]
+#version 130
+#extension GL_ARB_shading_language_420pack: require
+#extension GL_ARB_shader_image_load_store: require
+
+layout (rgba8, binding = 0) uniform image2D img;
+
+in vec4 piglit_vertex;
+out vec4 vs_fs;
+
+void main()
+{
+   vs_fs = imageLoad(img, ivec2(gl_Vertex.xy));
+   gl_Position = piglit_vertex;
+}
+
+[fragment shader]
+#version 130
+#extension GL_ARB_shading_language_420pack: require
+#extension 

[Piglit] [PATCH 1/2] arb_shading_language_420pack: check different binding points

[Andres Gomez]

This adds tests to check that, in the case of Uniform Blocks and
Shader Storage Blocks, it is allowed to specify different binding
points among the compilation units for blocks with the same name
while, in the case of Opaque-Uniforms, a link error is expected.

From page 60 (page 66 of the PDF) of the GLSL 4.20 spec, v11:

  " A link error will result if two compilation units in a program
specify different integer-constant bindings for the same
opaque-uniform name. However, it is not an error to specify a
binding on some but not all declarations for the same name, as
shown in the examples below."

Although this restriction is not included in the
ARB_shading_language_420pack spec, it is reasonable to believe that it
applies to it too.

Signed-off-by: Andres Gomez 
Cc: Matt Turner 
Cc: Kenneth Graunke 
---
 .../linker/different-bindings-image2D.shader_test  | 56 
 .../different-bindings-sampler2D.shader_test   | 53 +++
 ...rent-bindings-shader-storage-blocks.shader_test | 59 ++
 .../different-bindings-uniform-blocks.shader_test  | 56 
 4 files changed, 224 insertions(+)
 create mode 100644 
tests/spec/arb_shading_language_420pack/linker/different-bindings-image2D.shader_test
 create mode 100644 
tests/spec/arb_shading_language_420pack/linker/different-bindings-sampler2D.shader_test
 create mode 100644 
tests/spec/arb_shading_language_420pack/linker/different-bindings-shader-storage-blocks.shader_test
 create mode 100644 
tests/spec/arb_shading_language_420pack/linker/different-bindings-uniform-blocks.shader_test

diff --git 
a/tests/spec/arb_shading_language_420pack/linker/different-bindings-image2D.shader_test
 
b/tests/spec/arb_shading_language_420pack/linker/different-bindings-image2D.shader_test
new file mode 100644
index 0..97579678e
--- /dev/null
+++ 
b/tests/spec/arb_shading_language_420pack/linker/different-bindings-image2D.shader_test
@@ -0,0 +1,56 @@
+/* The GLSL 4.20 spec, v11, says:
+ *
+ * "A link error will result if two compilation units in a program
+ *  specify different integer-constant bindings for the same
+ *  opaque-uniform name. However, it is not an error to specify a
+ *  binding on some but not all declarations for the same name, as
+ *  shown in the examples below."
+ *
+ * Although this restriction is not included in the
+ * ARB_shading_language_420pack spec, it is reasonable to believe that
+ * it applies to it too.
+ *
+ * Verify that a link error happens when using different binding
+ * points for an opaque type (image2D) with the same name in
+ * different compilation units.
+ */
+
+[require]
+GLSL >= 1.30
+GL_ARB_shading_language_420pack
+GL_ARB_shader_image_load_store
+
+[vertex shader]
+#version 130
+#extension GL_ARB_shading_language_420pack: require
+#extension GL_ARB_shader_image_load_store: require
+
+layout (rgba8, binding = 0) uniform image2D img;
+
+in vec4 piglit_vertex;
+out vec4 vs_fs;
+
+void main()
+{
+   vs_fs = imageLoad(img, ivec2(gl_Vertex.xy));
+   gl_Position = piglit_vertex;
+}
+
+[fragment shader]
+#version 130
+#extension GL_ARB_shading_language_420pack: require
+#extension GL_ARB_shader_image_load_store: require
+
+layout (rgba8, binding = 1) uniform image2D img;
+
+uniform ivec4 cst;
+in  vec4 vs_fs;
+out vec4 fs_out;
+
+void main()
+{
+   fs_out = vs_fs * imageLoad(img, cst.xy).x;
+}
+
+[test]
+link error
diff --git 
a/tests/spec/arb_shading_language_420pack/linker/different-bindings-sampler2D.shader_test
 
b/tests/spec/arb_shading_language_420pack/linker/different-bindings-sampler2D.shader_test
new file mode 100644
index 0..37a388be6
--- /dev/null
+++ 
b/tests/spec/arb_shading_language_420pack/linker/different-bindings-sampler2D.shader_test
@@ -0,0 +1,53 @@
+/* The GLSL 4.20 spec, v11, says:
+ *
+ * "A link error will result if two compilation units in a program
+ *  specify different integer-constant bindings for the same
+ *  opaque-uniform name. However, it is not an error to specify a
+ *  binding on some but not all declarations for the same name, as
+ *  shown in the examples below."
+ *
+ * Although this restriction is not included in the
+ * ARB_shading_language_420pack spec, it is reasonable to believe that
+ * it applies to it too.
+ *
+ * Verify that a link error happens when using different binding
+ * points for an opaque type (sampler2D) with the same name in
+ * different compilation units.
+ */
+
+[require]
+GLSL >= 1.30
+GL_ARB_shading_language_420pack
+
+[vertex shader]
+#version 130
+#extension GL_ARB_shading_language_420pack: require
+
+layout (binding = 0) uniform sampler2D tex;
+
+in vec4 piglit_vertex;
+out vec4 vs_fs;
+
+void main()
+{
+   vs_fs = texture2D(tex, gl_Vertex.xy);
+   gl_Position = piglit_vertex;
+}
+
+[fragment shader]
+#version 130
+#extension GL_ARB_shading_language_420pack: