Docking crash fix / Clear references when remove DockNode

[baisai]

clear references when remove DockNode

[ocornut]

Hello,
Thanks for the PR.
Please clarify what you are fixing along with a repro, it would help getting this merged.

[baisai]

    static bool dock = false;
    ImGuiID dockspace_id = ImGui::GetID("MyDockSpace");
    if (dock)
    {
        ImGui::DockSpace(dockspace_id, ImVec2(0.0f, 0.0f), dockspace_flags);
    }
    if (ImGui::Button(dock ? "remove dock node" : "add dock node"))
    {
        if (dock)
        {
            ImGui::DockBuilderRemoveNode(dockspace_id);
        }
        else
        {
            ImGui::DockBuilderAddNode(dockspace_id, 0);
            ImGui::DockBuilderSetNodeSize(dockspace_id, ImGui::GetWindowSize());
            ImGuiID rootId = dockspace_id;
            ImGuiID left1 = ImGui::DockBuilderSplitNode(rootId, ImGuiDir_Left, 0.2, NULL, &rootId);
            ImGuiID left2 = ImGui::DockBuilderSplitNode(rootId, ImGuiDir_Left, 0.3, NULL, &rootId);
            ImGui::DockBuilderDockWindow("left1", left1);
            ImGui::DockBuilderDockWindow("left2", left2);
            ImGui::DockBuilderDockWindow("root", rootId);
            ImGui::DockBuilderFinish(dockspace_id);
        }
        dock = !dock;
    }

    ImGui::Begin("left1");
    ImGui::Text("left1");
    ImGui::End();

    ImGui::Begin("left2");
    ImGui::Text("left2");
    ImGui::End();

    ImGui::Begin("root");
    ImGui::Text("root");
    ImGui::End();

this is the test code, sometimes crash, the window->DockNodeAsHost is already delete.

[ocornut]

Thanks!

• I cannot reproduce a crash with your code.
• What is your value for dockspace_flags ?
• Can you confirm exact steps?
• What is version number + can you confirm configuration options (obtained from About window)

I'm not denying there could be a dangling pointer somewhere, but proposed fix seems strange to me so I would first need a working repro.

[ocornut]

By messing around with the .ini data I managed to reproduce a (different) crash so I'll investigate from there..

[ocornut]

I have pushed cd4cc9f general fix for DockBuilderRemoveNode() which ended up being same #3179, and it fix the one issue I was seeing with your code.

However your specific callstack is the same as #3203 and I cannot seem to be able to repro it.
Could you undo your fix, update to latest with cd4cc9f and see if you still get an issue? If so some more details would be greatly useful. Thank you!

[ocornut]

OK managed to get a repro based on your code and understand this fully.
It happens in some instance when the a same node id moves from one window host to another during the frame.

FYI you are using the DockBuilderAPI incorrectly:

    ImGuiID dockspace_id = ImGui::GetID("MyDockSpace");
    if (dock)
    {
        ImGui::DockSpace(dockspace_id, ImVec2(0.0f, 0.0f), dockspace_flags);
    }
.....
ImGui::DockBuilderAddNode(dockspace_id, 0);

When you create the node with ImGui::DockBuilderAddNode(dockspace_id, 0); you don't set the ImGuiDockNodeFlags_Dockspace flag, so dear imgui will think your node is a floating node. In the next call to ImGui::NewFrame() we will create the floating window (this is why there is a flicker) then immediately you submit the DockSpace() which will take the node away from the floating window.

At this point the floating window has a pointer window->DockNodeAsHost it shouldn't have and this pointer becomes dangling as you delete the node, leading us to the crash.

So, two things:

• I should fix the code to not end up with an obscure crash in that situation (fix or at least nicely assert with an explanation).
• Your code should use ImGui::DockBuilderAddNode(dockspace_id, ImGuiDockNodeFlags_Dockspace) if it is called in this order, otherwise even with an upcoming fix there will be a flicker of a temporary floating window appearing.

It doesn't happen for most users who forget to ImGuiDockNodeFlags_Dockspace flag because most people do things in this order:

ImGui::NewFrame()
if (...)
   ImGui::DockBuilderAddNode(dockspace_id, 0)
ImGui::Begin(...)
ImGui::DockSpace(dockspace_id, ....)
ImGui::End()

So we can immediately promote the node to a dockspace during the frame.

In your situation you did:

(Frame N)
ImGui::NewFrame()
ImGui::Begin(...)
ImGui::DockSpace(dockspace_id, ....)
ImGui::End()
if (...)
  ImGui::DockBuilderAddNode(dockspace_id, 0)

(Frame N+1)
ImGui::NewFrame() // create a floating window for dockspace_id since it's not a dockspace
ImGui::Begin(...)
ImGui::DockSpace(dockspace_id, ....) // promote to dockspace (with a bug, we will fix)
ImGui::End()

I'll do the fix.
Please note there are many comments about this in the code:

// Make sure to use the ImGuiDockNodeFlags_DockSpace flag to create a dockspace node! Otherwise this will create a floating node!

    // - To create a DockSpace() node, make sure to set the ImGuiDockNodeFlags_DockSpace flag when calling DockBuilderAddNode().
    //   You can create dockspace nodes (attached to a window) _or_ floating nodes (carry its own window) with this API.

[ocornut]

Fixed with eb6f4b1 ! Yay!

[baisai]

yes, you are right.
it works now.
thanks!