sokol_gfx: update a stream buffer and draw it multiple times during the same pass

[Alzathar]

First of all, thanks a lot for the development of sokol!

I successfully used sokol with the proposed samples and I am now doing complementary tests with OpenGL 3.3.

One of this test is to update a stream buffer and draw it several times during the same pass. However, it does not work because one assertion (line 9009) is not valid in the function sg_update_buffer. If I comment this assertion, it works without problems.

Why is it not allowed to update a stream buffer multiple times during the same pass?

[floooh]

I've added this restriction to have a simple and consistent way to avoid unintended blocking where the CPU needs to wait for the GPU because the GPU is currently drawing from the buffer (or image).

In the GL and Metal backend, dynamic buffers and images have multiple internal buffers which sokol rotates through to avoid blocking (in D3D11, LOCK_DISCARD is used, which does the same internally).

If you would update the same buffer multiple times per frame, it would rotate through the internal buffers all in the same frame, and the CPU would need to wait for the GPU.

When you remove the assert, it appears to work in the GL backend, but the CPU might stall waiting for the GPU (depending on whether or not the GL driver does any magic under the hood, but that's not reliable, and definitely doesn't work on WebGL). On Metal it might also appear to work, but you may get random rendering artefacts because the CPU scribbled over buffers that are currently in use by the GPU.

[Alzathar]

Thank you for these explanations. I better understand the reason why you use the global buffers vertices and indices in the sokol-samples imgui GLFW sample.

What do you think about the possibility to remove these global buffers? In the OpenGL3 example furnished with ImGui 1.66WIP (imgui_impl_opengl3.cpp:210), the GPU buffer is updated for each ImGui command list and then several draw commands are called based on the listed ImGui command buffers.

So instead of the proposed function void imgui_draw_cb(ImDrawData* draw_data), do you think it would be possible (with some modifications in sokol_gfx.h) to do something like the following snippet? In this case the GPU buffer is updated for each command list.

// This sample might not compile. It is only to show the difference with the existing code
void imgui_draw_cb(ImDrawData* draw_data) {
    assert(draw_data);
    if (draw_data->CmdListsCount == 0) {
        return;
    }
    sg_apply_draw_state(&draw_state);
    sg_apply_uniform_block(SG_SHADERSTAGE_VS, 0, &vs_params, sizeof(vs_params));
    int base_element = 0;
    for (int cl_index = 0 ; cl_index < draw_data->CmdListsCount ; ++cl_index) {
        const ImDrawList *cmd_list = draw_data->CmdLists[cl_index];
        // The next line triggers the assertion.
        sg_update_buffer(draw_state.vertex_buffers[0], cmd_list->VtxBuffer.Data, cmd_list->VtxBuffer.Size * sizeof(ImDrawVert));
        sg_update_buffer(draw_state.index_buffer, cmd_list->IdxBuffer.Data, cmd_list->IdxBuffer.Size * sizeof(ImDrawIdx));
        int base_element = 0;
        for (const ImDrawCmd& pcmd : cmd_list->CmdBuffer) {
            if (pcmd.UserCallback) {
                pcmd.UserCallback(cmd_list, &pcmd);
            }
            else {
                const int sx = (int) pcmd.ClipRect.x;
                const int sy = (int) pcmd.ClipRect.y;
                const int sw = (int) (pcmd.ClipRect.z - pcmd.ClipRect.x);
                const int sh = (int) (pcmd.ClipRect.w - pcmd.ClipRect.y);
                sg_apply_scissor_rect(sx, sy, sw, sh, true);
                sg_draw(base_element, pcmd.ElemCount, 1);
            }
            base_element += pcmd.ElemCount;
        }
    }
}

This would has the advantage to remove the memory copy step and to simplify the modification of the code to use unsigned 32-bit indices.

[floooh]

Ok, I'm just making up things now, not sure if it makes sense yet:

To eliminate the inbetween copy step I think the following could work: have a new sg_append_buffer() function which appends a chunk of "subdata" to a buffer and keeps track of the current end-of-data position in the buffer (this position would be reset at frame start). The application can call sg_append_buffer() multiple times per frame, but it must not "overflow" the buffer.

Sokol can already now render from a partially filled buffer (as long as the drawing operation stays within the currently "valid data" range in the buffer), and there's a relatively new feature where you can set an offset into the buffer in sg_draw_state.

The ImGui draw loop would then look like this:

    for (int cl_index = 0 ; cl_index < draw_data->CmdListsCount ; ++cl_index) {
        const ImDrawList *cmd_list = draw_data->CmdLists[cl_index];
        int vb_offset = sg_append_buffer(draw_state.vertex_buffers[0], cmd_list->VtxBuffer.Data, cmd_list->VtxBuffer.Size * sizeof(ImDrawVert));
        int ib_offset = sg_append_buffer(draw_state.index_buffer, cmd_list->IdxBuffer.Data, cmd_list->IdxBuffer.Size * sizeof(ImDrawIdx));
        draw_state.vertex_buffer_offsets[0] = vb_offset;
        draw_state.index_buffer_offset = ib_offset;
        sg_apply_draw_state(&draw_state);
        int base_element = 0;
        for (const ImDrawCmd& pcmd : cmd_list->CmdBuffer) {
            if (pcmd.UserCallback) {
                pcmd.UserCallback(cmd_list, &pcmd);
            }
            else {
                const int sx = (int) pcmd.ClipRect.x;
                const int sy = (int) pcmd.ClipRect.y;
                const int sw = (int) (pcmd.ClipRect.z - pcmd.ClipRect.x);
                const int sh = (int) (pcmd.ClipRect.w - pcmd.ClipRect.y);
                sg_apply_scissor_rect(sx, sy, sw, sh, true);
                sg_draw(base_element, pcmd.ElemCount, 1);
                base_element += pcmd.ElemCount;
            }
        }
    }

This would both eliminate the intermediate copy, and the additions on the indices while copying the indices...

You would still have the same buffer rotation system under the hood, the only difference is that you add data to the dynamic buffers in chunks, and issue a draw command on that chunk...

I think that looks pretty neat :) I'll write a separate implementation ticket.

PS: I need to check whether this sort of updating is possible in WebGL too. It would definitely work in Metal, and most likely in D3D11 though. So no promises yet.

[Alzathar]

Indeed that's pretty neat :)

I guess the definition of the [_]sg_append_buffer would be quite the same than the definition of the [_]sg_update_buffer except you have an internal offset that update the right "subdata" in the GPU buffer.

[floooh]

Yes the code would look very similar, I just don't want to break the sg_update_buffer() API, and theoretically at least, sg_update_buffer() might use a more efficient way to update the buffer data when it is guaranteed that the same buffer won't be touched again until it is rendered.

[Alzathar]

I don't really know the internal behaviour of sokol. Please forget me if the following code is a nonsense for you :-)

The following code propose an implementation of sg_append_buffer for the OpenGL backend. It it the mechanism you have in mind?

// New member in the _sg_buffer structure
// - offset: new member that is incremented each time the
//           sg_update_buffer function is called.
// - used_slots: new member that count the number of slots used in this frame
// New enum item in _sg_validate_error
// - _SG_VALIDATE_APDBUF_USAGE
// - _SG_VALIDATE_APDBUF_ONCE
// - _SG_VALIDATE_APDBUF_SIZE

// This code is quite the same than _sg_validate_update_buffer.
// Only one validation differs to verify 
_SOKOL_PRIVATE bool _sg_validate_append_buffer(const _sg_buffer* buf, const void* data, int size) {
    #if !defined(SOKOL_DEBUG)
        _SOKOL_UNUSED(buf);
        _SOKOL_UNUSED(data);
        _SOKOL_UNUSED(size);
        return true;
    #else
        SOKOL_ASSERT(buf && data);
        SOKOL_VALIDATE_BEGIN();
        SOKOL_VALIDATE(buf->usage != SG_USAGE_IMMUTABLE, _SG_VALIDATE_APDBUF_USAGE);
        SOKOL_VALIDATE(buf->used_slots <= SG_NUM_INFLIGHT_FRAMES && buf->size >= buf->offset + size, _SG_VALIDATE_APDBUF_SIZE);
        SOKOL_VALIDATE(buf->upd_frame_index != _sg.frame_index, _SG_VALIDATE_APDBUF_ONCE);
        return SOKOL_VALIDATE_END();
    #endif
}

// Note: OpenGL implementation
// This implementation proposes to append data to the internal slots using the
// following rules:
//  - update the current slot while the current_size + data_size does not
//    exceed the buffer_size associated with the current slot
//  - if current_size + data_size > buffer_size, use another slot
// TBD: How to reset it when the function sg_end_pass is called?
//      Can we loop in the pool to pass through the buffer and reset 'offset'?
_SOKOL_PRIVATE bool _sg_append_buffer(_sg_buffer* buf, const void* data_ptr, int data_size) {
    SOKOL_ASSERT(buf && data_ptr && (data_size > 0));
    if (data_size + buf->offset >= buf->size) {
        ++buf->active_slot;
        ++buf->used_slots;
        buf->offset = 0;
    }
    if (buf->active_slot >= buf->num_slots) {
        buf->active_slot = 0;
    }
    GLenum gl_tgt = _sg_gl_buffer_target(buf->type);
    SOKOL_ASSERT(buf->active_slot < SG_NUM_INFLIGHT_FRAMES);
    GLuint gl_buf = buf->gl_buf[buf->active_slot];
    SOKOL_ASSERT(gl_buf);
    _SG_GL_CHECK_ERROR();
    glBindBuffer(gl_tgt, gl_buf);
    glBufferSubData(gl_tgt, buf->offset, data_size, data_ptr);
    _SG_GL_CHECK_ERROR();
    buf->offset += data_size;
    return buf->used_slots >= SG_NUM_INFLIGHT_FRAMES;
}

void sg_update_buffer(sg_buffer buf_id, const void* data, int num_bytes) {
    if (num_bytes == 0) {
        return;
    }
    _sg_buffer* buf = _sg_lookup_buffer(&_sg.pools, buf_id.id);
    if (!(buf && buf->slot.state == SG_RESOURCESTATE_VALID)) {
        return;
    }
    if (_sg_validate_append_buffer(buf, data, num_bytes)) {
        SOKOL_ASSERT(buf->upd_frame_index != _sg.frame_index);
        if (_sg_append_buffer(buf, data, num_bytes)) {
          // All the slots are used, it is no more possible to update/append data in the buffer.
          buf->upd_frame_index = _sg.frame_index;
        }
    }
}

[floooh]

I already started working on it, the Metal version is done, I'll try to work on the GL version today.

You can follow the changes in this (work in progress) PR: https://github.com/floooh/sokol/pull/86