glslang icon indicating copy to clipboard operation
glslang copied to clipboard

uniform structures and invalid locations

Open gbromov opened this issue 4 years ago • 4 comments

This example shader:

#version 430

struct B0 { mat4 M1; mat4 M2; };
layout(location=0) uniform B0 Var0;

struct B1 { vec4 E1; };
layout(location=1) uniform B1 Var1;

layout(location=0) in vec4 vs_Position;

void main( )
{
	gl_Position = Var0.M2 * vs_Position;
}

should not compile, since uniform location 1 is overlapped by Var0 and Var1. However it compiles just fine, and even worse - if you use spirv-opt (with -Os flag) on the generated binary, then decompose it you can clearly see that the main function uses the matrix at location 0 instead of location 1.

Result as generated by spirv-cross after passing the bytecode through spirv-opt:

#version 430
struct B0 { mat4 M2; };
layout(location = 0) uniform B0 Var0;
layout(location = 0) in vec4 vs_Position;
void main()
{
    gl_Position = Var0.M2 * vs_Position;
}

If I modify the source shader code to use arrays instead of structures, however, the tool properly identifies the overlay and yield error:

#version 430
layout(location=0) uniform mat4 Var0[2];
layout(location=1) uniform vec4 Var1[5];
layout(location=0) in vec4 vs_Position;
void main( )
{
	gl_Position = Var0[1] * vs_Position;
}

Results in:

ERROR: test.glsl:4: 'location' : overlapping use of location 1
ERROR: 1 compilation errors.  No code generated.

Also if I fix the locations so that the arrays don't overlap, the code produced by spirv-cross, after optimizing the bytecode is perfectly fine - the matrix used in the multiplication is the one at location 1 as expected.

gbromov avatar Jun 18 '20 06:06 gbromov

I don't quite understand yet. Is this for OpenGL? I know Vulkan only has bindings for uniform buffers, and was thinking that's true of all SPIR-V, but perhaps not. Also, I think the rules for locations are quite different for buffers than for in/out.

johnkslang avatar Jun 22 '20 07:06 johnkslang

Yes, this is for OpenGL. I am no expert in any way here - not even close, but judging by https://www.khronos.org/opengl/wiki/Uniform_(GLSL)/Explicit_Uniform_Location I believe there are no "real" uniform structures, but rather the compiler creates a uniform for each structure member, and calculate the locations for those, based on the first one given.

So in the first example with

struct B0 { mat4 M1; mat4 M2; };
layout(location=0) uniform B0 Var0;

I'd expect the result to have two uniforms - mat4 Var0.M1 at location 0 and mat4 Var0.M2 at location 1. Whether the compiler does that I am not sure how to tell, but after optimizing/decompiling the bytecode it's pretty obvious that mat4 Var0.M2 is at location 0 instead,

gbromov avatar Jun 22 '20 08:06 gbromov

Here's the normative language from the specification:

The location identifier can be used with default-block uniform variables and ... Individual elements of a uniform array are assigned consecutive locations with the first element taking location location. Default-block uniform variable declarations sharing the same location linked in the program have to match by name, type, qualifiers and arrayness... Valid locations for default-block uniform variable locations are in the range of 0 to the implementation-defined maximum number of uniform locations minus one...

Locations can be assigned to default-block uniform arrays and structures. The first inner-most scalar, vector or matrix member or element takes the specified location and the compiler assigns the next inner-most member or element the next incremental location value. Each subsequent inner-most member or element gets incremental locations for the entire structure or array. This rule applies to nested structures and arrays and gives each inner-most scalar, vector, or matrix member a unique location...

When generating SPIR-V for API’s that accept individual (default block) non-opaque uniform variables, it is a compile-time error to not include a location when declaring them.

It contradicts the link you provided a bit on the matching, but, yes, it looks like SPIR-V generation needs to support locations on default uniforms, and the part about counting up number of locations used is not yet implemented.

johnkslang avatar Jun 24 '20 14:06 johnkslang

I meet the same problem.

IMO, this kind of location overlapping error is unnecessary:

  • When we directly use GLSL shader in OpenGL, the driver would automatically distribute locations to default uniform variable's array elements, and they are maybe not sequential in the same array: the location of arr[1] may not be the location of arr[0] plus one. People almost always use glUniform*v to send values to uniform array.

  • When we want to convert the GLSL shader to SPIR-V shader, and use the SPIR-V shader in OpenGL, we cannot expect get the location of a single array element (OpenGL Specification 4.6, Core Profile, 7.3.1.1):

    For shaders constructed from SPIR-V binaries (that is with a state of SPIR_- V_BINARY equal to TRUE), variables may not have names associated with them, as the OpName and OpMemberName debug instructions are optional and may not be present in a SPIR-V module. When the Op*Name instructions are present, it is implementation-dependent if these are reported via the name reflection APIs. If no name reflection information is available, the name string associated with each active variable is the empty string (""). In this case, any queries that operate with a name as input will return INVALID_INDEX or -1 as appropriate, and any queries that return information about the name of a resource will report a name length of one (for the null character) and return an empty string with a length of zero.

    The only way to send data to this uniform array is to use glUniform*v.

  • When we want to convert the GLSL shader to SPIR-V shader, and use the SPIR-V shader in Vulkan, the location make no sense because Vulkan only allow to use uniform buffers. If we use the extension GL_EXT_vulkan_glsl_relaxed, the location also make no sense, because the extension collect all default uniforms to one uniform block.

In all cases above, it's no need for glslang to distribute locations for array elements or to assume the locations in an array is sequential (so they would be overlapping with other uniforms).

chirsz-ever avatar Dec 01 '21 08:12 chirsz-ever