Skip to content

Commit

Permalink
LeaveLocalSpace(): Search for ImDrawCallback_ImCanvas after m_DrawLis…
Browse files Browse the repository at this point in the history
…tFirstCommandIndex (proposed fix for thedmd#282)

        Analysis of the bug thedmd#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);
  • Loading branch information
pthom committed May 12, 2024
1 parent dd3aa4d commit dc1d661
Showing 1 changed file with 43 additions and 0 deletions.
43 changes: 43 additions & 0 deletions imgui_canvas.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -557,10 +557,53 @@ void ImGuiEx::Canvas::LeaveLocalSpace()
// Remove sentinel draw command if present
if (m_DrawListCommadBufferSize > 0)
{
/*
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);
*/

// Original tests; this test will fail because it tests 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);
}

auto& fringeScale = ImFringeScaleRef(m_DrawList);
Expand Down

0 comments on commit dc1d661

Please sign in to comment.