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

Reading uint8_t from storage buffers adds (unnecessarily) UniformAndStorageBuffer8BitAccess capability #1539

Closed
zeux opened this issue Oct 20, 2018 · 0 comments · Fixed by #1542

Comments

@zeux
Copy link
Contributor

zeux commented Oct 20, 2018

(yes, this is me and my uint8_t shader again)

This shader:

#version 450

#extension GL_EXT_shader_16bit_storage: require
#extension GL_EXT_shader_8bit_storage: require

struct Vertex
{
	float vx, vy, vz;
	uint8_t nx, ny, nz, nw;
	float tu, tv;
};

layout(binding = 0) readonly buffer Vertices
{
	Vertex vertices[];
};

layout(location = 0) out vec4 color;

void main()
{
	vec3 position = vec3(vertices[gl_VertexIndex].vx, vertices[gl_VertexIndex].vy, vertices[gl_VertexIndex].vz);
	vec3 normal = vec3(int(vertices[gl_VertexIndex].nx), int(vertices[gl_VertexIndex].ny), int(vertices[gl_VertexIndex].nz)) / 127.0 - 1.0;
	vec2 texcoord = vec2(vertices[gl_VertexIndex].tu, vertices[gl_VertexIndex].tv);

	gl_Position = vec4(position * vec3(1, 1, 0.5) + vec3(0, 0, 0.5), 1.0);

	color = vec4(normal * 0.5 + vec3(0.5), 1.0);
}

Gets compiled into a shader with these capabilities:

               OpCapability Shader
               OpCapability StorageBuffer8BitAccess
               OpCapability UniformAndStorageBuffer8BitAccess

It seems to me that UniformAndStorageBuffer8BitAccess is unnecessary; I hit this because on the C++ side, you can enable these features separately, and I was only enabling 8-bit storage buffer access which seems correct to me. I don't think glslang should be adding both and instead should only add StorageBuffer8BitAccess.

After talking to @sheredom it seems like there might be some subtle differences here between SPIRV 1.0 and SPIRV 1.3 that I didn't fully grasp, but selecting SPIRV 1.3 (via --target-env vulkan1.1) still produces a shader that requires both capabilities.

@zeux zeux changed the title Reading uint8_t from storage buffers adds (redundantly) UniformAndStorageBuffer8BitAccess capability Reading uint8_t from storage buffers adds (unnecessarily) UniformAndStorageBuffer8BitAccess capability Oct 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant