imnodes icon indicating copy to clipboard operation
imnodes copied to clipboard

After deleting and re-creating nodes, attribute pins can become non-interactable.

Open francesco-cattoglio opened this issue 3 years ago • 1 comments

This is basically the counter-part of https://github.com/Nelarius/imnodes/issues/71 but for pins instead of nodes.

First and foremost: this is the code to reproduce the bug

    void RenderTestFrame() {
        ImGui::Begin("simple node editor");

        imnodes::BeginNodeEditor();

        static int i = 0;
        if (i == 0) { // frame 1
            imnodes::BeginNode(3);
                imnodes::BeginOutputAttribute(3);
                    ImGui::Text("out_3");
                imnodes::EndOutputAttribute();
            imnodes::EndNode();
            imnodes::BeginNode(1);
                imnodes::BeginOutputAttribute(1);
                    ImGui::Text("out_1");
                imnodes::EndOutputAttribute();
                imnodes::BeginInputAttribute(2);
                    ImGui::Text("in_2");
                imnodes::EndInputAttribute();
            imnodes::EndNode();
                i++;
        } else if (i == 1) { // frame 2
                i++;
        } else if (i >= 2) { // frame 3 onwards
            imnodes::BeginNode(3);
                imnodes::BeginNodeTitleBar();
                    ImGui::TextUnformatted("try using attribute pin");
                imnodes::EndNodeTitleBar();
                imnodes::BeginOutputAttribute(3);
                    ImGui::Text("out_3");
                imnodes::EndOutputAttribute();
            imnodes::EndNode();
            i++;
        }

        imnodes::EndNodeEditor();
        ImGui::End();
    }

From the third call of RenderTestFrame() onwards, the pin on the node will not be interactive anymore, because the state of the PinData object pool is messed up. I am currently using revision ee6d407 because I am stuck with an older version of imgui, but I am pretty sure you will be able to reproduce on master as well.

The culprit is the following block inside imnodes.cpp

template<typename T>
void object_pool_update(ObjectPool<T>& objects)
{
    objects.free_list.clear();
    for (int i = 0; i < objects.in_use.size(); ++i)
    {
        if (!objects.in_use[i])
        {
            objects.id_map.SetInt(objects.pool[i].id, -1);
            objects.free_list.push_back(i);
            (objects.pool.Data + i)->~T();
        }
    }
}

In this situation, a pin is not in use, and therefore the id_map at the related id gets reset to -1. However, since the destructor of a PinData does nothing, the value in memory stays there and that id will be reset over and over again, even if that id is currently used by another PinData in a different slot. My fix for that was to add a destructor to the PinData that resets the stored ID to INT_MIN and add an a relevant check to the code. However I have not yet investigated if this is something that should be taken care of for links as well. In my experiments everything was fine without any change to the LinkData, but I had very little time to test that.

francesco-cattoglio avatar Mar 23 '21 12:03 francesco-cattoglio

Thanks for bringing this up @francesco-cattoglio ! My goal is to fix all these issues at once when I refactor the internal object management: https://github.com/Nelarius/imnodes/issues/81

Nelarius avatar Mar 27 '21 16:03 Nelarius