Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KHR_vulkan_glsl: Does not recognize interface block array differences. #514

Closed
NicolBolas opened this issue Jun 19, 2017 · 2 comments
Closed

Comments

@NicolBolas
Copy link

In OpenGL's GLSL, arrays of interface blocks take up sequential binding indices. In Vulkan however, arrays of interface block only take up one binding point in SPIR-V. Or at least, they are allowed to.

The KHR_vulkan_glsl extension recognizes that arrays of opaque types are stored in one binding point. But it never changes the paragraph in 4.4.5 that discusses arrays of interface blocks (UBOs/SSBOs). By a strict reading of the specification, this means that an array of uniform blocks:

layout(set = 0, binding = 2) uniform ArrayBlock
{
  ...
} array_block[5];

This array should take up binding points 2, 3, 4, 5, and 6. in the descriptor set. But while this is valid, it is not necessary, as both SPIR-V and Vulkan have provisions for allowing such a uniform block to take up just one binding point. There does not appear to be anything in the Vulkan specification which prevents a descriptor set layout from specifying a descriptorCount greater than 1 for descriptors of type UNIFORM_BUFFER. And there is nothing in the SPIR-V specification which prevents a Block-decorated object from having an array count.

I have not investigated how glslangValidator compiles arrays of uniform/storage blocks.

@NicolBolas
Copy link
Author

I've investigated how glslangValidator generates code for this. The shader:

#version 450 core

layout(set = 0, binding = 0) uniform Block
{
	int i;
	float j;
} block[5];

void main()
{
	gl_Position = vec4(0, 0, 0, 1);
}

Becomes:

                              Capability Shader
               1:             ExtInstImport  "GLSL.std.450"
                              MemoryModel Logical GLSL450
                              EntryPoint Vertex 4  "main" 13
                              Source GLSL 450
                              Name 4  "main"
                              Name 11  "gl_PerVertex"
                              MemberName 11(gl_PerVertex) 0  "gl_Position"
                              MemberName 11(gl_PerVertex) 1  "gl_PointSize"
                              MemberName 11(gl_PerVertex) 2  "gl_ClipDistance"
                              MemberName 11(gl_PerVertex) 3  "gl_CullDistance"
                              Name 13  ""
                              Name 21  "Block"
                              MemberName 21(Block) 0  "i"
                              MemberName 21(Block) 1  "j"
                              Name 25  "block"
                              MemberDecorate 11(gl_PerVertex) 0 BuiltIn Position
                              MemberDecorate 11(gl_PerVertex) 1 BuiltIn PointSize
                              MemberDecorate 11(gl_PerVertex) 2 BuiltIn ClipDistance
                              MemberDecorate 11(gl_PerVertex) 3 BuiltIn CullDistance
                              Decorate 11(gl_PerVertex) Block
                              MemberDecorate 21(Block) 0 Offset 0
                              MemberDecorate 21(Block) 1 Offset 4
                              Decorate 21(Block) Block
                              Decorate 25(block) DescriptorSet 0
                              Decorate 25(block) Binding 0
               2:             TypeVoid
               3:             TypeFunction 2
               6:             TypeFloat 32
               7:             TypeVector 6(float) 4
               8:             TypeInt 32 0
               9:      8(int) Constant 1
              10:             TypeArray 6(float) 9
11(gl_PerVertex):             TypeStruct 7(fvec4) 6(float) 10 10
              12:             TypePointer Output 11(gl_PerVertex)
              13:     12(ptr) Variable Output
              14:             TypeInt 32 1
              15:     14(int) Constant 0
              16:    6(float) Constant 0
              17:    6(float) Constant 1065353216
              18:    7(fvec4) ConstantComposite 16 16 16 17
              19:             TypePointer Output 7(fvec4)
       21(Block):             TypeStruct 14(int) 6(float)
              22:      8(int) Constant 5
              23:             TypeArray 21(Block) 22
              24:             TypePointer Uniform 23
       25(block):     24(ptr) Variable Uniform
         4(main):           2 Function None 3
               5:             Label
              20:     19(ptr) AccessChain 13 15
                              Store 20 18
                              Return
                              FunctionEnd

This clearly creates a single binding point, which is an array of Block-declared structs. So that matches with what SPIR-V and Vulkan would expect.

But it doesn't match with section 4.4.5 of the GLSL specification, which KHR_vulkan_glsl does not modify:

If the binding identifier is used with a uniform or shader storage block instanced as an array, the first
element of the array takes the specified block binding and each subsequent element takes the next
consecutive uniform block binding point. For an array of arrays, each element (e.g., 6 elements for a[2]
[3]) gets a binding point, and they are ordered per the array of array ordering described in section 4.1.9
“Arrays.”

@oddhack
Copy link
Contributor

oddhack commented Jul 22, 2017

Should be fixed in the 1.0.56 spec update.

@oddhack oddhack closed this as completed Jul 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants