imgui-node-editor icon indicating copy to clipboard operation
imgui-node-editor copied to clipboard

Proposed fix for issue #282: search and remove ImDrawCallback_ImCanvas after m_DrawListFirstCommandIndex

Open pthom opened this issue 1 year ago • 1 comments

Hi,

This is a draft for a fix for #282, with lengthy explanations inside the code, because it needs to be further analyzed.

Analysis of the bug https://github.com/thedmd/imgui-node-editor/issues/282 when using this repro: https://github.com/pthom/node_window_clipping_issue/tree/suspend_resume_issue2

At the 4th call of LeaveLocalSpace(), we have:

this = {ImGuiEx::Canvas *}
     m_DrawList = {ImDrawList *}                      mDrawList is of size 4
         CmdBuffer = {ImVector<ImDrawCmd>}            its elements at index 2
             Size = {int} 4                           has UserCallback == {ImDrawCallback_ImCanvas}
             Capacity = {int} 8
             Data = {ImDrawCmd *}
     m_ExpectedChannel = {int} 0
     m_DrawListFirstCommandIndex = {int} 2
     m_DrawListCommadBufferSize = {int} 1
     m_DrawListStartVertexIndex = {int} 8

We have m_DrawListFirstCommandIndex = 2, however, the tests in the original code will test only at index 0 and 1:

if (m_DrawList->CmdBuffer.size() > m_DrawListCommadBufferSize && m_DrawList->CmdBuffer[m_DrawListCommadBufferSize].UserCallback == ImDrawCallback_ImCanvas).   
    m_DrawList->CmdBuffer.erase(m_DrawList->CmdBuffer.Data + m_DrawListCommadBufferSize);
else if (m_DrawList->CmdBuffer.size() >= m_DrawListCommadBufferSize && m_DrawList->CmdBuffer[m_DrawListCommadBufferSize - 1].UserCallback == ImDrawCallback_ImCanvas)
    m_DrawList->CmdBuffer.erase(m_DrawList->CmdBuffer.Data + m_DrawListCommadBufferSize - 1);

Proposed solution: test all commands from index >= m_DrawListFirstCommandIndex and remove the one with UserCallback == ImDrawCallback_ImCanvas (based on the original code, it seems there can be only one)

        int idxCommand_ImDrawCallback_ImCanvas = -1;
        for (int i = m_DrawListFirstCommandIndex; i < m_DrawList->CmdBuffer.size(); ++i)
        {
            auto & command = m_DrawList->CmdBuffer[i];
            if (command.UserCallback == ImDrawCallback_ImCanvas)
            {
                idxCommand_ImDrawCallback_ImCanvas = i;
                break;
            }
        }
        if (idxCommand_ImDrawCallback_ImCanvas >= 0)
            m_DrawList->CmdBuffer.erase(m_DrawList->CmdBuffer.Data + idxCommand_ImDrawCallback_ImCanvas);

pthom avatar Mar 20 '24 08:03 pthom

I had the same crash and the PR seems to fix it

IceLuna avatar May 05 '24 14:05 IceLuna