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

GLSL: Uniform texel buffer translation to SPIR-V #1096

Closed
chaoticbob opened this issue Oct 10, 2017 · 6 comments
Closed

GLSL: Uniform texel buffer translation to SPIR-V #1096

chaoticbob opened this issue Oct 10, 2017 · 6 comments

Comments

@chaoticbob
Copy link
Contributor

chaoticbob commented Oct 10, 2017

Question about how GLSL uniform texel buffers gets translated to SPIR-V.

According to the Vulkan spec (13.1.5), this:

layout (set=m, binding=n) uniform samplerBuffer myUniformTexelBuffer;

translates to something like this:

          %6 = OpTypeFloat 32
          %7 = OpTypeImage %6 Buffer 0 0 0 1 Unknown
          %8 = OpTypePointer UniformConstant %7
          %9 = OpVariable %8 UniformConstant

However, it seems that glslang slips in an additional OpTypeSampledImage:

         %23 = OpTypeFloat 32
         %26 = OpTypeImage %23 Buffer 0 0 0 1 Unknown
         %27 = OpTypeSampledImage %26
         %28 = OpTypePointer UniformConstant %27
         %29 = OpVariable %28 UniformConstant

Am curious as to why this is?

@johnkslang
Copy link
Member

What is see is this sequence of logic:

  1. a samplerBuffer is read in GLSL through texelFetch
  2. in KHR_vulkan_glsl, the suggested mapping is texelFetch -> OpImageFetch
  3. in SPIR-V:

OpImageFetch
Fetch a single texel from a sampled image.

So, using a sampled image looks correct, on the language side. If this is truly what the API spec is discussing, perhaps a specification is wrong somewhere. It does appear though that glslang currently does what GLSL, KHR_vulkan_glsl, and SPIR-V specifications indicate.

What would help is if these examples include the instruction that accesses the data, as declarations on their own don't tell the whole story.

@chaoticbob
Copy link
Contributor Author

Apologies for the delay, here's an example shader:

#version 450
#extension GL_ARB_separate_shader_objects : enable
#extension GL_ARB_shading_language_420pack : enable
#pragma shader_stage(vertex)
layout (set = 0, binding = 1) uniform isamplerBuffer texel_buffer_i;
layout (set = 0, binding = 2) uniform samplerBuffer texel_buffer_f;

void main() {
  int i = texelFetch(texel_buffer_i, gl_InstanceIndex).x;
  float f = texelFetch(texel_buffer_f, gl_InstanceIndex).x;
  gl_Position = vec4(i, f, 0, 1);
}

@johnkslang
Copy link
Member

I mean the examples in the specification show the declarations, but not the instructions that use them, leaving too much to the imagination.

To do the actual access, glslang then emits an OpImage, to extract the image from the combining sampler/image.

I think this is all okay. It is historically how GLSL works. The variables your example declared are combined sampler/image.

@johnkslang
Copy link
Member

Perhaps if you declare like the following it is what you want?

layout (set = 0, binding = 2) uniform textureBuffer texel_buffer_f;

That makes an image not combined with a sampler, and then OpImage is not needed to extact the image.

@chaoticbob
Copy link
Contributor Author

@johnkslang thanks for the follow up. I was going strictly on what the spec contained and wasn't aware that textureBuffer existed. Learn something new everyday.

For this case, I'm more concerned about SPIR-V reflection wrt Vulkan. I wasn't questioning the correctness of what glslang emits. Just got a bit confused between what was in the resulting SPIR-V and the Vulkan spec.

@johnkslang
Copy link
Member

Well, I'm not terribly happy the Vulkan spec mixed its metaphors here; that doesn't help.

New types like textureBuffer were added to GLSL to support Vulkan.

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