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

Transpilation to MSL from SPIRV has uniforms and storage buffers flipped #78

Closed
christopherliu830 opened this issue Dec 30, 2024 · 2 comments · Fixed by #87
Closed

Transpilation to MSL from SPIRV has uniforms and storage buffers flipped #78

christopherliu830 opened this issue Dec 30, 2024 · 2 comments · Fixed by #87

Comments

@christopherliu830
Copy link

My game has a storage buffer for model matrices and a uniform buffer for global scene data. Given this sample glsl file:

#version 450

layout(std430, binding = 0, set = 0) readonly buffer Objects {
    mat4 _data[];
} objects;

layout(std140, binding = 0, set = 1) uniform Scene
{
    mat4 view_proj;
} scene;

layout(location = 0) in vec3 input_position;

void main()
{
    gl_Position = scene.view_proj * objects._data[gl_InstanceIndex] * vec4(input_position, 1.0);
}

Then running these commands:

glslangValidator -V standard.vert
shadercross vert.spv -t vertex -d msl -o vert.metal 

I get this output:

#include <metal_stdlib>
#include <simd/simd.h>

using namespace metal;

struct Scene
{
    float4x4 view_proj;
};

struct Objects
{
    float4x4 _data[1];
};

struct main0_out
{
    float4 gl_Position [[position]];
};

struct main0_in
{
    float3 input_position [[attribute(0)]];
};

vertex main0_out main0(main0_in in [[stage_in]], const device Objects& objects [[buffer(0)]], constant Scene& scene [[buffer(1)]], uint gl_InstanceIndex [[instance_id]])
{
    main0_out out = {};
    out.gl_Position = (scene.view_proj * objects._data[int(gl_InstanceIndex)]) * float4(in.input_position, 1.0);
    return out;
}

I believe that scene [[buffer(1)]] and objects [[buffer(0)]] should be flipped, as per SDL_gpu documentation:

/*
 * For MSL/metallib, use the following order:
 *
 * - [[texture]]: Sampled textures, followed by storage textures
 * - [[sampler]]: Samplers with indices corresponding to the sampled textures
 * - [[buffer]]: Uniform buffers, followed by storage buffers. Vertex buffer 0
 *   is bound at [[buffer(14)]], vertex buffer 1 at [[buffer(15)]], and so on.
 *   Rather than manually authoring vertex buffer indices, use the
 *   [[stage_in]] attribute which will automatically use the vertex input
 *   information from the SDL_GPUGraphicsPipeline.
 */

And when I swap 1s and 0s for the buffer indices, my shader works correctly.

@karabatov
Copy link

Hit the same issue where I have 3 structured buffers and 1 uniform in the vertex shader and 2 structured buffers and 1 uniform in the fragment shader.

The uniform is declared as buffer(3) in MSL instead of buffer(0) and as a result the bindings are shifted down by one: should be uniform (0) followed by 3 buffers.

Screenshot 2025-01-11 at 18 03 42

SDL itself sets the uniform in slot 0 as expected (the unnamed buffer is the uniform):
Screenshot 2025-01-11 at 18 05 45

Exactly the same happens in the fragment shader:
Screenshot 2025-01-11 at 18 07 16

When I manually go to the MSL transpiled file and up-shift the structured buffers by 1 and put the uniform as buffer(0) everything starts working.

@thatcosmonaut
Copy link
Collaborator

Should be fixed by #87 if you wouldn't mind testing.

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.

3 participants