Imgui: Custom ImDrawVert layout: not cleared/constructed

[ocornut]

Followup on discussion in #1172, @Rubonnek forked the discussion with an issue which seems to have nothing to do with the original post. So I am resuming it here.
The posts related to this starts from
https://github.com/ocornut/imgui/issues/1172#issuecomment-314933325

Basically the issue is that if you redefine the layout for ImDrawVert, the extra field don't get initialized.
e.g.

#define IMGUI_OVERRIDE_DRAWVERT_STRUCT_LAYOUT \
struct ImDrawVert \
{ \
    ImVec2 pos; \
    float z = 1000.f; \
    ImU32 col; \
    ImVec2 uv; \
};

And we don't call memset() nor constructor on that type.

@thedmd the function to modify would be ImDrawList::PrimReserve rather than ImDrawList::PrimWriteVtx

1. To clarify we can't use <type_traits>, that would betray a lot of the fundation of imgui and several users wouldn't like it at all.
2. As written in my previous post, we don't want to do this in ImVector::reserve(), but rather in the ImDrawList code calling reserve aka ImDrawList::PrimReserve. ImVector intentionally doesn't handle constructor/destruct and if we are going to introduce that sort of thing (along with using copy/= operators etc.) we should do it everywhere and not just in reseve(). So only touching reserve() is definitively not the right thing to do.
3. The default ImDrawVtx isn't is_trivially_default_constructible afaik as the individual ImVec2 have constructors (that we don't rely on here).

We can either:
A. call constructor in ImDrawList::PrimReserve
B. call memset in ImDrawList::PrimReserve (this is technically incorrect, maybe faster)
C. ask the user to clear their field in their Render function with a comment where IMGUI_OVERRIDE_DRAWVERT_STRUCT_LAYOUT is documented.

For now I have added a comment for C and I suspect it may be a good solution. It's an omission that such comment was forgotten but adding it in uppercase (as I am doing now) there's no way someone can miss it.

[ocornut]

For reference, PrimReserve with (B)

void ImDrawList::PrimReserve(int idx_count, int vtx_count)
{
    ImDrawCmd& draw_cmd = CmdBuffer.Data[CmdBuffer.Size-1];
    draw_cmd.ElemCount += idx_count;

    int vtx_buffer_old_size = VtxBuffer.Size;
    int vtx_buffer_new_size = vtx_buffer_old_size + vtx_count;
    if (vtx_buffer_new_size > VtxBuffer.Capacity)
    {
        int vtx_buffer_old_capacity = VtxBuffer.Capacity;
        VtxBuffer.reserve(VtxBuffer._grow_capacity(vtx_buffer_new_size));
        // when our underlying ImDrawVert has extra fields, we memset on growing capacity (vs on growing size) for efficiency
        if (sizeof(ImDrawVert) > 20)
            memset(VtxBuffer.Data + vtx_buffer_old_capacity, 0, (VtxBuffer.Capacity - vtx_buffer_old_capacity) * sizeof(ImDrawVert));
    }
    VtxBuffer.Size = vtx_buffer_new_size;
    _VtxWritePtr = VtxBuffer.Data + vtx_buffer_old_size;

    int idx_buffer_old_size = IdxBuffer.Size;
    IdxBuffer.resize(idx_buffer_old_size + idx_count);
    _IdxWritePtr = IdxBuffer.Data + idx_buffer_old_size;
}

[thedmd]

@ocornut Thanks for pointing out that to me.

[paulsapps]

Hmm could it be optional? If the user always overwrites their own fields (which I guess they will otherwise what are they for?). Then isn't the memset a waste of time?

[Rubonnek]

I agree with @paulsapps -- I think the memset should be optional. I ended up supplying the z value on each vertex before passing the vertex buffer to the Cocos2D-X renderer and everything looks fine now.

With the comment added on 8261d9ff30b0ef409c2ebdc324b2f5234518e5e4, newcomers like myself will know they need to manually assign the values on those extra fields.

[ocornut]

OK based on feedback above and since you are the person who came up with the issue, opting for the solution for only leaving a comment. Sorry that the lack of constructor made you waste time.

Here's the comment:

// You can override the vertex format layout by defining IMGUI_OVERRIDE_DRAWVERT_STRUCT_LAYOUT in imconfig.h
// The code expect ImVec2 pos (8 bytes), ImVec2 uv (8 bytes), ImU32 col (4 bytes), but you can re-order them or add other fields as needed to simplify integration in your engine.
// The type has to be described within the macro (you can either declare the struct or use a typedef)
// NOTE: IMGUI DOESN'T CLEAR THE STRUCTURE AND DOESN'T CALL A CONSTRUCTOR SO ANY CUSTOM FIELD WILL BE UNINITIALIZED. IF YOU ADD EXTRA FIELDS (SUCH AS A 'Z' COORDINATES) YOU WILL NEED TO CLEAR THEM DURING RENDER OR TO IGNORE THEM. 
IMGUI_OVERRIDE_DRAWVERT_STRUCT_LAYOUT;

[Rubonnek]

@ocornut It's completely fine. The time wasted is nothing compared to the time it would take me to make a library like this. And, in fact, thank you for such an amazing library.

[or75]

https://github.com/ocornut/imgui/issues/1172#issuecomment-318816341