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

vulkano-shaders generate different types with the same name in one namespace #1530

Open
p0lunin opened this issue Apr 1, 2021 · 7 comments

Comments

@p0lunin
Copy link

p0lunin commented Apr 1, 2021

For the following code:

struct Foo { uint a; };
layout(std140, set = 0, binding = 0) readonly buffer A{
    Foo foo[];
};
layout(set = 1, binding = 0) readonly buffer B {
    Foo foo[];
};
void main() {}

vulkano-shaders generates 2 types with name Foo. I think this is because shader uses the same structure with different memory layouts std140 and shared (uses by default).

@Rua
Copy link
Contributor

Rua commented Apr 2, 2021

Clearly these need to have different names, but what should they be?

@p0lunin
Copy link
Author

p0lunin commented Apr 2, 2021

Maybe macro will create namespaces with memory layout as name? e.g. types::std140::Foo and types::packed::Foo.

@Rua
Copy link
Contributor

Rua commented Apr 2, 2021

What about the names A and B? How come those aren't used by Vulkano-shaders?

@Rua
Copy link
Contributor

Rua commented Aug 6, 2022

I accidentally stumbled upon this old issue. I'll elaborate a bit more for future reference. Given this definition in GLSL:

struct Foo {
    float bar;
};

layout(push_constant) uniform PushConstants {
    Foo x;
} pc;

layout(binding = 0) uniform Uniforms {
    Foo x;
} uni;

The shader compiler will generate this SPIR-V output:

               OpName %Foo "Foo"
        %Foo = OpTypeStruct %float
%PushConstants = OpTypeStruct %Foo
%_ptr_PushConstant_PushConstants = OpTypePointer PushConstant %PushConstants
         %pc = OpVariable %_ptr_PushConstant_PushConstants PushConstant
               OpName %Foo_0 "Foo"
      %Foo_0 = OpTypeStruct %float
   %Uniforms = OpTypeStruct %Foo_0
%_ptr_Uniform_Uniforms = OpTypePointer Uniform %Uniforms
        %uni = OpVariable %_ptr_Uniform_Uniforms Uniform

Two different structures with the name Foo are defined in the SPIR-V; one for the push constants and another copy for the uniform buffer. The same thing happens with a uniform buffer combined with any other kind of input variable, such as a storage buffer, as long as they use the same GLSL struct in their definition. This happens because uniform buffers use the extended alignment (std140) by default, instead of the base alignment (std430) used by everything else. The difference in layout becomes more clear when bar is turned into an array float bar[2]:

               OpDecorate %_arr_float_uint_2 ArrayStride 4
%_arr_float_uint_2 = OpTypeArray %float %uint_2
        %Foo = OpTypeStruct %_arr_float_uint_2
               OpDecorate %_arr_float_uint_2_0 ArrayStride 16
%_arr_float_uint_2_0 = OpTypeArray %float %uint_2
      %Foo_0 = OpTypeStruct %_arr_float_uint_2_0

Now, the two Foos aren't exact copies of each other anymore, but are actually different structs with a different layout.

Vulkano-shaders can't deal with this situation at the moment. When presented with two different structs named Foo, it will happily generate two Rust structs also named Foo, which of course leads to a compile error.

@marc0246
Copy link
Contributor

marc0246 commented Apr 5, 2023

This was fixed in #2132.

@marc0246 marc0246 closed this as completed Apr 5, 2023
@Rua Rua reopened this Apr 5, 2023
@Rua
Copy link
Contributor

Rua commented Apr 5, 2023

After some Discord discussion we concluded that the issue is only fixed in the case of duplicate structs. It's not fixed if the structs actually have different definitions, like in my last example above.

@marc0246
Copy link
Contributor

marc0246 commented Apr 5, 2023

For context, the way I see it is that as long as we have a stuct registration mechanism in place which tracks structs across different shaders, this is very difficult (maybe impossible?) to solve completely. We could append a number at the end of the struct name inside a shader, but what if two different shaders have the same type of collision, but their numberings end up being different?

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

4 participants