Double-buffered ImDrawData

[mpvsv]

I run two primary threads in my application; one handles world update at 60hz while another renders frames at however fast it's able. The world update thread keeps two versions of the graphics state that the graphics thread interpolates. Is there support for this mode of operation now? If not, would it be simple to add? (Interpolation isn't important for the GUI side of things. I'd be fine having double-buffered DrawData.)

[ocornut]

Hello, It’s not supported yet but there is a CloneOutput function you can use (a little slower) to duplicate an ImDrawList. For this and other features I would like to support double buffered ImDrawList per window, which would lead to double buffered ImDrawData.

[gviot]

Hey there,

I wanted to share a few things:

1. I think a simple "double buffering" feature in imgui wouldn't cover every use case. ideally I would like to see some kind of configurable n-buffering or improvement of CloneOutput.
   e.g. my engine (inspired a bit by "Parallelizing the Naughty Dog Engine Using Fibers") is multi threaded with N-world-frame-buffering (usually N = 10 in my case but configurable).
   The renderer is double buffered and can skip world frames in order to catch up and reduce latency.

2. I have managed to make imgui mostly work in this context by storing the command lists to each world frame. When the renderer skips a world frame it also skips an imgui frame so I still have a few minor issues like invalid framerate value in imgui but nothing problematic.
   In case it can help anyone, in order to save the data of each imgui frame i used both CloneOutput and this bit of code:

void set_imgui_data(ImDrawData* dat) {
	if (m_imgui_data.CmdLists != nullptr) {
		for (int i = 0; i < m_imgui_data.CmdListsCount; i++) {
			delete m_imgui_data.CmdLists[i];
		}
		delete m_imgui_data.CmdLists;
		m_imgui_data.CmdLists = nullptr;
	}
	m_imgui_data.Clear();
	m_imgui_data = *dat;
	m_imgui_data.CmdLists = new ImDrawList*[dat->CmdListsCount];
	for (int i = 0; i < dat->CmdListsCount; i++) {
		m_imgui_data.CmdLists[i] = dat->CmdLists[i]->CloneOutput();
	}
}

3. Since i was on the docking branch, I have updated more often and currently CloneOutput crashes 100% when called, so I wanted to share my fix, hopefully it can be upstreamed so i don't have to refix it with every update.
   The current code is:

ImDrawList* ImDrawList::CloneOutput() const
{
    ImDrawList* dst = IM_NEW(ImDrawList(NULL));
    ...
}

The NULL argument causes a crash when the constructor calls Clear so I changed it to

ImDrawList* ImDrawList::CloneOutput() const
{
    ImDrawList* dst = IM_NEW(ImDrawList(_Data));
    ...
}

[ocornut]

@gviot For now I have pushed the fix for the issue described in (3).

[ocornut]

Here's a suggested more optimal way to handle double buffering of ImDrawData.
Maybe people have been using CloneOutput() but it's not ideally efficient.
This implementation is a little more complex but allow avoiding a full copy of vertex data.

I wrote this on a whim and haven't tested it thoroughly. Would be interested in feedback.
I would consider making ImDrawDataSnapshot public API eventually, but I'm not sure I am fully grasp how multi-frames-in-flight custom backends would work with multi-viewports, and where would be the responsibility there.

Code

// Usage:
//  static ImDrawDataSnapshot snapshot; // Important: make persistent accross frames to reuse buffers.
//  snapshot.SnapUsingSwap(ImGui::GetDrawData(), ImGui::GetTime());
//  [...]
//  ImGui_ImplDX11_RenderDrawData(&snapshot.DrawData);

// FIXME: Could store an ID in ImDrawList to make this easier for user.
#include "imgui_internal.h" // ImPool<>, ImHashData

struct ImDrawDataSnapshotEntry
{
    ImDrawList*     SrcCopy = NULL;     // Drawlist owned by main context
    ImDrawList*     OurCopy = NULL;     // Our copy
    double          LastUsedTime = 0.0;
};

struct ImDrawDataSnapshot
{
    // Members
    ImDrawData                      DrawData;
    ImPool<ImDrawDataSnapshotEntry> Cache;
    float                           MemoryCompactTimer = 20.0f; // Discard unused data after 20 seconds

    // Functions
    ~ImDrawDataSnapshot()           { Clear(); }
    void                            Clear();
    void                            SnapUsingSwap(ImDrawData* src, double current_time); // Efficient snapshot by swapping data, meaning "src_list" is unusable.
    //void                          SnapUsingCopy(ImDrawData* src, double current_time); // Deep-copy snapshop

    // Internals
    ImGuiID                         GetDrawListID(ImDrawList* src_list) { return ImHashData(&src_list, sizeof(src_list)); }     // Hash pointer
    ImDrawDataSnapshotEntry*        GetOrAddEntry(ImDrawList* src_list) { return Cache.GetOrAddByKey(GetDrawListID(src_list)); }
};

void ImDrawDataSnapshot::Clear()
{
    for (int n = 0; n < Cache.GetMapSize(); n++)
        if (ImDrawDataSnapshotEntry* entry = Cache.TryGetMapData(n))
            IM_DELETE(entry->OurCopy);
    Cache.Clear();
    DrawData.Clear();
}

void ImDrawDataSnapshot::SnapUsingSwap(ImDrawData* src, double current_time)
{
    ImDrawData* dst = &DrawData;
    IM_ASSERT(src != dst && src->Valid);

    // Copy all fields except CmdLists[]
    ImVector<ImDrawList*> backup_draw_list;
    backup_draw_list.swap(src->CmdLists);
    IM_ASSERT(src->CmdLists.Data == NULL);
    *dst = *src;
    backup_draw_list.swap(src->CmdLists);

    // Swap and mark as used
    for (ImDrawList* src_list : src->CmdLists)
    {
        ImDrawDataSnapshotEntry* entry = GetOrAddEntry(src_list);
        if (entry->OurCopy == NULL)
        {
            entry->SrcCopy = src_list;
            entry->OurCopy = IM_NEW(ImDrawList)(src_list->_Data);
        }
        IM_ASSERT(entry->SrcCopy == src_list);
        entry->SrcCopy->CmdBuffer.swap(entry->OurCopy->CmdBuffer); // Cheap swap
        entry->SrcCopy->IdxBuffer.swap(entry->OurCopy->IdxBuffer);
        entry->SrcCopy->VtxBuffer.swap(entry->OurCopy->VtxBuffer);
        entry->SrcCopy->CmdBuffer.reserve(entry->OurCopy->CmdBuffer.Capacity); // Preserve bigger size to avoid reallocs for two consecutive frames
        entry->SrcCopy->IdxBuffer.reserve(entry->OurCopy->IdxBuffer.Capacity);
        entry->SrcCopy->VtxBuffer.reserve(entry->OurCopy->VtxBuffer.Capacity);
        entry->LastUsedTime = current_time;
        dst->CmdLists.push_back(entry->OurCopy);
    }

    // Cleanup unused data
    const double gc_threshold = current_time - MemoryCompactTimer;
    for (int n = 0; n < Cache.GetMapSize(); n++)
        if (ImDrawDataSnapshotEntry* entry = Cache.TryGetMapData(n))
        {
            if (entry->LastUsedTime > gc_threshold)
                continue;
            IM_DELETE(entry->OurCopy);
            Cache.Remove(GetDrawListID(entry->SrcCopy), entry);
        }
};

Test bed, press 1 to capture current output, hold 2 to display it:

        static ImDrawData snapshot;
        ImDrawData* draw_data_to_render = ImGui::GetDrawData();

        if (ImGui::IsKeyPressed(ImGuiKey_1))
        {
            snapshot.SnapUsingSwap(ImGui::GetDrawData(), ImGui::GetTime());
            draw_data_to_render = &snapshot.DrawData;
        }
        else if (ImGui::IsKeyDown(ImGuiKey_2))
        {
            draw_data_to_render = &snapshot.DrawData;
        }

        ImGui_ImplDX11_RenderDrawData(draw_data_to_render);

[pozemka]

Here's a suggested more optimal way to handle double buffering of ImDrawData. Maybe people have been using CloneOutput() but it's not ideally efficient. This implementation is a little more complex but allow avoiding a full copy of vertex data.

    // Internals
    ImGuiID                         GetDrawListID(ImDrawList* src_list) { return ImHashData(&src_list, sizeof(src_list)); }     // Hash pointer

Isn't sizeof supposed to be called on the structure instead of the pointer?

return ImHashData(&src_list, sizeof(*src_list));

[GamingMinds-DanielC]

Isn't sizeof supposed to be called on the structure instead of the pointer?

return ImHashData(&src_list, sizeof(*src_list));

I guess that's why that line has a comment "hash pointer", to make clear that it is indeed the pointer that is to be hashed. That also makes sense since you want an ID for the draw list, that shouldn't change if the contents of the draw list change. The pointer stays the same.

Btw, dangerous bug in your one-liner. You would be hashing the pointer and unspecified memory after where the pointer is stored, not the structure itself, but with the size of the structure. Could be an access violation, could be hashing the stack, could be hashing anything.