From c6960cddbad50a6888ca9981a9183ce11d2439cb Mon Sep 17 00:00:00 2001 From: Pascal Thomet Date: Wed, 20 Mar 2024 09:34:18 +0100 Subject: [PATCH] LeaveLocalSpace(): Search for ImDrawCallback_ImCanvas after m_DrawListFirstCommandIndex (proposed fix for #282) 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} 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); --- imgui_canvas.cpp | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/imgui_canvas.cpp b/imgui_canvas.cpp index 2010ee39..5ca03473 100644 --- a/imgui_canvas.cpp +++ b/imgui_canvas.cpp @@ -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} 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);