UBO + SSBO bug

[conversy]

Hi,

It's been a long time since I started chasing a bug with Shader Storage Buffer Object: if I use a mix of UBOs and SSBOs, sometimes nothing would show up in the window.

I suspect that somehow bindBuffersToCurrentRenderEncoder uses a wrong index or offset value: using the loop increment seems to be suspect to me, but I may be completely wrong. Besides, BufferMap::buffer_base_index and Buffer::index are set, but never used, which seems suspect too.
I tried various combinations of indexand offset with setVertexBuffer:offset:atIndex: to no avail.

I could finally edit test_2D_textures to replicate it (below). This version does not work. To make it work, turn the SSBO back to a UBO (there are 3 lines of code to change, all prefixed with // BUG:). Other combinations of SSBO and UBO do work, the bug might be due to how SPIRV arranges the order of the table entries in a first place (?) (see spirv log below).

@guymadison I figured that you could have a look at it and try to fix it? You may be much faster than me...

[EDIT] I suspect this is at the root of other bugs such as this one. This may be a showstopper for any moderately complex shaders. I could be wrong of course.

[EDIT2] I could find why it's buggy: in the vertex shader below, scale is not used, hence spvc-cross does not emit anything for its buffer. Setting the buffer will thus overwrite the next one... if scale is used (e.g. multiply the coordinates with it and a 1.0 value), everything is ok. So we need to know which buffer is actually used, and not send it in bindBuffersToCurrentRenderEncoder if it's not... And I do not know how to do that.

int test_2D_textures(GLFWwindow* window, int width, int height)
{
    GLuint vbo = 0, col_vbo = 0, tex_vbo = 0, mat_ubo = 0, scale_ubo = 0, col_att_ubo = 0;

    const char* vertex_shader =
    GLSL(450 core,
        layout(location = 0) in vec3 position;
        layout(location = 1) in vec3 in_color;
        layout(location = 2) in vec2 in_texcords;

        layout(location = 0) out vec4 out_color;
        layout(location = 1) out vec2 out_texcoords;

        layout(binding = 0)
        //uniform           // BUG: uncomment this to make it work
        readonly buffer     // BUG: comment this to make it work
        matrices
        {
            mat4 rotMatrix;
        };

        layout(binding = 1)
        uniform
        //readonly buffer
        scale
        {
               float pos_scale;
        };

        void main() {
            gl_Position = rotMatrix * vec4(position, 1.0);
            out_color = vec4(in_color, 1.0);
            out_texcoords = in_texcords;
        }
    );

    const char* fragment_shader =
    GLSL(450 core,
        layout(location = 0) in vec4 in_color;
        layout(location = 1) in vec2 in_texcords;

        layout(location = 0) out vec4 frag_colour;

        layout(binding = 2)
        uniform
        //readonly buffer
        color_att
        {
            float att;
        };

        uniform sampler2D image;

        void main() {
            vec4 tex_color = texture(image, in_texcords);

            // frag_colour = in_color * att;
            frag_colour = in_color * att * tex_color;
            //frag_colour = tex_color;
        }
    );

    float points[] = {
       0.0f,  0.5f,  0.0f,
       0.5f, -0.5f,  0.0f,
      -0.5f, -0.5f,  0.0f
    };

    float color[] = {
        1.0f, 0.0f, 0.0f,
        0.0f, 1.0f, 0.0f,
        0.0f, 0.0f, 1.0f,
    };

    float texcoords[] = {
        0.5f, 0.0f,
        0.0f, 1.0f,
        1.0f, 1.0f,
    };

    glGenBuffers(1, &vbo);
    glBindBuffer(GL_ARRAY_BUFFER, vbo);
    glBufferData(GL_ARRAY_BUFFER, 9 * sizeof(float), points, GL_STATIC_DRAW);
    glBindBuffer(GL_ARRAY_BUFFER, 0);

    glGenBuffers(1, &col_vbo);
    glBindBuffer(GL_ARRAY_BUFFER, col_vbo);
    glBufferData(GL_ARRAY_BUFFER, 9 * sizeof(float), color, GL_STATIC_DRAW);
    glBindBuffer(GL_ARRAY_BUFFER, 0);

    glGenBuffers(1, &tex_vbo);
    glBindBuffer(GL_ARRAY_BUFFER, tex_vbo);
    glBufferData(GL_ARRAY_BUFFER, 6 * sizeof(float), texcoords, GL_STATIC_DRAW);
    glBindBuffer(GL_ARRAY_BUFFER, 0);

    //GL_UNIFORM_BUFFER;
    //GL_SHADER_STORAGE_BUFFER;
    GLenum mat_target = GL_SHADER_STORAGE_BUFFER; // BUG: replace with GL_UNIFORM_BUFFER to make it work
    GLenum scale_target = GL_UNIFORM_BUFFER;
    GLenum col_target = GL_UNIFORM_BUFFER;

    mat4  rotZ = glm::identity<mat4>();

    float angle = M_1_PI / 6;

    rotZ = glm::rotate(glm::identity<mat4>(), angle, glm::vec3(0, 0, 1));

    glGenBuffers(1, &mat_ubo);
    glBindBuffer(mat_target, mat_ubo);
    glBufferData(mat_target, sizeof(mat4),&rotZ[0][0], GL_STATIC_DRAW);
    glBindBuffer(mat_target, 0);

    float scale = -1.0;
    glGenBuffers(1, &scale_ubo);
    glBindBuffer(scale_target, scale_ubo);
    glBufferData(scale_target, sizeof(float), &scale, GL_STATIC_DRAW);
    glBindBuffer(scale_target, 0);

    float att = 1.0;
    glGenBuffers(1, &col_att_ubo);
    glBindBuffer(col_target, col_att_ubo);
    glBufferData(col_target, sizeof(float), &att, GL_STATIC_DRAW);
    glBindBuffer(col_target, 0);

    GLuint vao = 0;
    glCreateVertexArrays(1, &vao);
    glBindVertexArray(vao);

    glEnableVertexAttribArray(0);
    glBindBuffer(GL_ARRAY_BUFFER, vbo);
    glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 0, NULL);

    glEnableVertexAttribArray(1);
    glBindBuffer(GL_ARRAY_BUFFER, col_vbo);
    glVertexAttribPointer(1, 3, GL_FLOAT, GL_FALSE, 0, NULL);

    glEnableVertexAttribArray(2);
    glBindBuffer(GL_ARRAY_BUFFER, tex_vbo);
    glVertexAttribPointer(2, 2, GL_FLOAT, GL_FALSE, 0, NULL);

    // clear currently bound buffer
    glBindBuffer(GL_ARRAY_BUFFER, 0);

    GLuint vs = glCreateShader(GL_VERTEX_SHADER);
    glShaderSource(vs, 1, &vertex_shader, NULL);
    glCompileShader(vs);
    GLuint fs = glCreateShader(GL_FRAGMENT_SHADER);
    glShaderSource(fs, 1, &fragment_shader, NULL);
    glCompileShader(fs);

    GLuint shader_program = glCreateProgram();
    glAttachShader(shader_program, fs);
    glAttachShader(shader_program, vs);
    glLinkProgram(shader_program);
    glUseProgram(shader_program);

    // GLuint matrices_loc = glGetUniformBlockIndex(shader_program, "matrices");
    // assert(matrices_loc == 0);

    // GLuint scale_loc = glGetUniformBlockIndex(shader_program, "scale");
    // assert(scale_loc == 1);

    // GLuint color_att_loc = glGetUniformBlockIndex(shader_program, "color_att");
    // assert(color_att_loc == 2);

    glBindBufferBase(mat_target, 0, mat_ubo);
    glBindBufferBase(scale_target, 1, scale_ubo);
    glBindBufferBase(col_target, 2, col_att_ubo);

    glActiveTexture(GL_TEXTURE0);

    GLuint tex;
    glGenTextures(1, &tex);
    glBindTexture(GL_TEXTURE_2D, tex);
    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, 256, 256, 0, GL_RGBA, GL_UNSIGNED_BYTE,
                 genTexturePixels(GL_RGBA, GL_UNSIGNED_BYTE, 0x10, 256,256));

    // GLuint tex2;
    // glGenTextures(1, &tex2);
    // glBindTexture(GL_TEXTURE_2D, tex2);
    // glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, 256, 512, 0, GL_RGBA, GL_UNSIGNED_BYTE,
    //              genTexturePixels(GL_RGBA, GL_UNSIGNED_BYTE, 0x10, 256,512));

    glViewport(0, 0, width, height);

    glBindVertexArray(vao);

    glUseProgram(shader_program);

    glClearColor(0.2, 0.2, 0.2, 0.0);

    float att_delta = 0.01;

    while (!glfwWindowShouldClose(window))
    {
        //GLsync  sync;

        glClear(GL_COLOR_BUFFER_BIT);

        // glBindTexture(GL_TEXTURE_2D, tex2);
        // glDrawArrays(GL_TRIANGLES, 0, 3);
        glBindTexture(GL_TEXTURE_2D, tex);
        glDrawArrays(GL_TRIANGLES, 0, 3);

        //sync = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0);

        SWAP_BUFFERS;

        angle += (M_PI / 180);

        rotZ = glm::rotate(glm::identity<mat4>(), angle, glm::vec3(0, 0, 1));

        //glWaitSync(sync, 0, GL_TIMEOUT_IGNORED);

        glBindBuffer(mat_target, mat_ubo);
        glBufferSubData(mat_target, 0, sizeof(mat4), &rotZ[0][0]);
        glBindBuffer(mat_target, 0);

        att += att_delta;
        if (att > 1.0)
        {
            att = 1.0;
            att_delta *= -1.0;
        }
        else if (att < 0.0)
        {
            att = 0.0;
            att_delta *= -1.0;
        }

        glBindBuffer(col_target, col_att_ubo);
        glBufferSubData(col_target, 0, sizeof(float), &att);
        glBindBuffer(col_target, 0);

        glfwPollEvents();
    }

    return 0;
}

Not working:

res_type:  UNIFORM_BUFFER ID: 50	BaseTypeID: 48	 TypeID: 49	 Name:                scale	Set: 0	Binding: 1	Location: 0	Index: 0	Uniform: 0
res_type:  STORAGE_BUFFER ID: 19	BaseTypeID: 17	 TypeID: 18	 Name:             matrices	Set: 0	Binding: 0	Location: 0	Index: 0	Uniform: 0
res_type:     STAGE_INPUT ID: 25	BaseTypeID: 23	 TypeID: 24	 Name:             position	Set: 0	Binding: 0	Location: 0	Index: 0	Uniform: 0
res_type:     STAGE_INPUT ID: 36	BaseTypeID: 23	 TypeID: 24	 Name:             in_color	Set: 0	Binding: 0	Location: 1	Index: 0	Uniform: 0
res_type:     STAGE_INPUT ID: 46	BaseTypeID: 42	 TypeID: 45	 Name:          in_texcords	Set: 0	Binding: 0	Location: 2	Index: 0	Uniform: 0
res_type:    STAGE_OUTPUT ID: 35	BaseTypeID: 7	 TypeID: 33	 Name:            out_color	Set: 0	Binding: 0	Location: 0	Index: 0	Uniform: 0
res_type:    STAGE_OUTPUT ID: 44	BaseTypeID: 42	 TypeID: 43	 Name:        out_texcoords	Set: 0	Binding: 0	Location: 1	Index: 0	Uniform: 0
res_type:  UNIFORM_BUFFER ID: 27	BaseTypeID: 25	 TypeID: 26	 Name:            color_att	Set: 0	Binding: 2	Location: 0	Index: 0	Uniform: 0
res_type:     STAGE_INPUT ID: 17	BaseTypeID: 15	 TypeID: 16	 Name:          in_texcords	Set: 0	Binding: 0	Location: 1	Index: 0	Uniform: 0
res_type:     STAGE_INPUT ID: 23	BaseTypeID: 7	 TypeID: 22	 Name:             in_color	Set: 0	Binding: 0	Location: 0	Index: 0	Uniform: 0
res_type:    STAGE_OUTPUT ID: 21	BaseTypeID: 7	 TypeID: 20	 Name:          frag_colour	Set: 0	Binding: 0	Location: 0	Index: 0	Uniform: 0
res_type:   SAMPLED_IMAGE ID: 13	BaseTypeID: 11	 TypeID: 12	 Name:                image	Set: 0	Binding: 0	Location: 0	Index: 0	Uniform: 0

Working: (all UBOs):

res_type:  UNIFORM_BUFFER ID: 19	BaseTypeID: 17	 TypeID: 18	 Name:             matrices	Set: 0	Binding: 0	Location: 0	Index: 0	Uniform: 0
res_type:  UNIFORM_BUFFER ID: 50	BaseTypeID: 48	 TypeID: 49	 Name:                scale	Set: 0	Binding: 1	Location: 0	Index: 0	Uniform: 0
res_type:     STAGE_INPUT ID: 25	BaseTypeID: 23	 TypeID: 24	 Name:             position	Set: 0	Binding: 0	Location: 0	Index: 0	Uniform: 0
res_type:     STAGE_INPUT ID: 36	BaseTypeID: 23	 TypeID: 24	 Name:             in_color	Set: 0	Binding: 0	Location: 1	Index: 0	Uniform: 0
res_type:     STAGE_INPUT ID: 46	BaseTypeID: 42	 TypeID: 45	 Name:          in_texcords	Set: 0	Binding: 0	Location: 2	Index: 0	Uniform: 0
res_type:    STAGE_OUTPUT ID: 35	BaseTypeID: 7	 TypeID: 33	 Name:            out_color	Set: 0	Binding: 0	Location: 0	Index: 0	Uniform: 0
res_type:    STAGE_OUTPUT ID: 44	BaseTypeID: 42	 TypeID: 43	 Name:        out_texcoords	Set: 0	Binding: 0	Location: 1	Index: 0	Uniform: 0
res_type:  UNIFORM_BUFFER ID: 27	BaseTypeID: 25	 TypeID: 26	 Name:            color_att	Set: 0	Binding: 2	Location: 0	Index: 0	Uniform: 0
res_type:     STAGE_INPUT ID: 17	BaseTypeID: 15	 TypeID: 16	 Name:          in_texcords	Set: 0	Binding: 0	Location: 1	Index: 0	Uniform: 0
res_type:     STAGE_INPUT ID: 23	BaseTypeID: 7	 TypeID: 22	 Name:             in_color	Set: 0	Binding: 0	Location: 0	Index: 0	Uniform: 0
res_type:    STAGE_OUTPUT ID: 21	BaseTypeID: 7	 TypeID: 20	 Name:          frag_colour	Set: 0	Binding: 0	Location: 0	Index: 0	Uniform: 0
res_type:   SAMPLED_IMAGE ID: 13	BaseTypeID: 11	 TypeID: 12	 Name:                image	Set: 0	Binding: 0	Location: 0	Index: 0	Uniform: 0

[conversy]

ok so using spvc_compiler_create_shader_resources_for_active_variables instead of spvc_compiler_create_shader_resources solves this bug.
Now, I do not know if it's the right thing to do, so I won't close this issue until I figure out why I'm still having problems with my own, more complex shader that mixes UBO and SSBO.

[darkaegisagain]

Thanks, I have been painting a house this week… then I am moving my office out to my new shed (which is more than a shed, with power / AC,Heat / etc), I should be able to take a look on Monday. Mike

…

On Jul 22, 2023, at 6:59 AM, conversy ***@***.***> wrote: ok so using spvc_compiler_create_shader_resources_for_active_variables instead of spvc_compiler_create_shader_resources solves this bug. Now, I do not know if it's the right thing to do, so I won't close this issue until I figure out why I'm still having problems with my own, more complex shader that mixes UBO and SSBO. — Reply to this email directly, view it on GitHub <#73 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACOKD34CXOEEEL4MDDONR33XRPME5ANCNFSM6AAAAAA2M4QC4Q>. You are receiving this because you are subscribed to this thread.

[conversy]

Good to hear from you, Mike!
I have refactored the code a bit, I may push it before Monday.
After that I have another bug for you ;-)

[guymadison]

Good to hear from you also… normally I don’t code much in the summer… I have loads of tasks outside that need to get done before the rain starts again! (And vacations..)

…

On Jul 22, 2023, at 8:47 AM, conversy ***@***.***> wrote: Good to hear from you, Mike! I have refactored the code a bit, I may push it before Monday. After that I have another bug for you ;-) — Reply to this email directly, view it on GitHub <#73 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABKYN3IIBOMHTLMJTSE6DFDXRPY2PANCNFSM6AAAAAA2M4QC4Q>. You are receiving this because you were mentioned.

[conversy]

ok, so I pushed many things in the glReadPixels branch (unfortunately I forgot to switch to the main one, I may try to cherry-pick into main tomorrow).
Among many things I tried to make sure the experience of compiling and running the lib and the test is straightforward, as well as trying to make a more complex application (e.g. I removed the assert(0) line in deleteHash, increased the maximum number of textures etc.).

One of the last commits is a fix (with spvc_compiler_create_shader_resources_for_active_variables) for the bug in the program I put above. However, it does not fix the mixing between UBOs + SSBOs. I think the problem is that in bindBuffersToCurrentRenderEncoder, the index of setVertexBuffer may not be correct because of the order the buffers are mapped in mapGLBuffersToMTLBufferMap. The UBOs and SSBOs may be interleaved in the MTL program, but the MGL code considers that UBOs should be done first, then SSBOs. I do not have a fix yet.

[darkaegisagain]

I will add your example to the main branch and fix it there.