thedmd / imgui-node-editor

Node Editor built using Dear ImGui
MIT License
3.73k stars 551 forks source link

ImGui::ColorEdit4 popups don't work inside a node #242

Open brunopj1 opened 1 year ago

brunopj1 commented 1 year ago

I have a node that has an ImGui::ColorEdit4 inside its body.

The number input fields work normally, but the tooltip, color picker and right click option popups don't work. They get positioned in the wrong place and have weird interactions with the mouse.

I know that in order to make the popups work, I should suspend the editor before rendering them, but these popups are handled inside the ImGui::ColorEdit4 function, and I can't call this outside the node.

Is there any way to make these popups work?

SC5Shout commented 1 year ago

You can have bool openPopup and do something like this:

inside of the node's body

openPopup = ImGui::Button("OpenPopup");

outside of the node's body

ed::Suspend();
if(openPopup) {
    ImGui::OpenPopup("picker");
}
ed::Resume();

ed::Suspend();
if(ImGui::BeginPopup("picker")) {

//...

ImGui::EndPopup();
}
ed::Resume();
SC5Shout commented 1 year ago

The same thing has to be done for combo widgets. It starts being annoying. And for the combo popups, I don't think it works. The popup window position is weird, probably because I've done something wrong. @thedmd any idea how to bypass it?

lukaasm commented 1 year ago

To "fix" this properly ImGui modification is needed right now. I did it by adding 2 new ImGui hooks: ImGuiContextHookType_BeginWindow, ImGuiContextHookType_EndWindow that are called on start of ImGui::Begin and end of ImGui::End

bool ImGui::Begin(const char* name, bool* p_open, ImGuiWindowFlags flags)
{
...
    CallContextHooks(&g, ImGuiContextHookType_BeginWindow);
 ...
 }
bool ImGui::End()
{
...
    CallContextHooks(&g, ImGuiContextHookType_EndWindow);
...
}

and registering/unregistering them in ImGuiEx::Canvas::Begin/End

bool ImGuiEx::Canvas::Begin(ImGuiID id, const ImVec2& size)
{
...
    auto beginWindowHook = ImGuiContextHook{};
    beginWindowHook.UserData = this;
    beginWindowHook.Type = ImGuiContextHookType_BeginWindow;
    beginWindowHook.Callback = []( ImGuiContext * context, ImGuiContextHook * hook )
    {
        //ImGui::SetNextWindowViewport( ImGui::GetCurrentWindow()->Viewport->ID );

        auto canvas = reinterpret_cast< Canvas * >( hook->UserData );
        if ( canvas->m_SuspendCounter == 0 )
        {
            if ( ( context->NextWindowData.Flags & ImGuiNextWindowDataFlags_HasPos ) != 0 )
            {
                auto pos = canvas->FromLocal( context->NextWindowData.PosVal );
                ImGui::SetNextWindowPos( pos, context->NextWindowData.PosCond, context->NextWindowData.PosPivotVal );
            }

            if ( context->BeginPopupStack.size() )
            {
                auto & popup = context->BeginPopupStack.back();
                popup.OpenPopupPos = canvas->FromLocal( popup.OpenPopupPos );
                popup.OpenMousePos = canvas->FromLocal( popup.OpenMousePos );
            }

            if ( context->OpenPopupStack.size() )
            {
                auto & popup = context->OpenPopupStack.back();
                popup.OpenPopupPos = canvas->FromLocal( popup.OpenPopupPos );
                popup.OpenMousePos = canvas->FromLocal( popup.OpenMousePos );
            }

        }
        canvas->m_BeginWindowCursorBackup = ImGui::GetCursorScreenPos();
        canvas->Suspend();
    };

    m_beginWindowHook = ImGui::AddContextHook( ImGui::GetCurrentContext(), &beginWindowHook );

    auto endWindowHook = ImGuiContextHook{};
    endWindowHook.UserData = this;
    endWindowHook.Type = ImGuiContextHookType_EndWindow;
    endWindowHook.Callback = []( ImGuiContext * ctx, ImGuiContextHook * hook )
    {
        auto canvas = reinterpret_cast< Canvas * >( hook->UserData );
        canvas->Resume();
        ImGui::SetCursorScreenPos( canvas->m_BeginWindowCursorBackup );
        ImGui::GetCurrentWindow()->DC.IsSetPos = false;
    };

    m_endWindowHook = ImGui::AddContextHook( ImGui::GetCurrentContext(), &endWindowHook );

    return true;
}

void ImGuiEx::Canvas::End()
{
...
    m_InBeginEnd = false;

    ImGui::RemoveContextHook( ImGui::GetCurrentContext(), m_beginWindowHook );
    ImGui::RemoveContextHook( ImGui::GetCurrentContext(), m_endWindowHook );
}

Now tooltips and other spawned windows withing Canvas context work semi properly

image
lapinozz commented 4 months ago

I can confirm the above fixes the issue, would love to see this merged somehow

thedmd commented 4 months ago

What can I say. Idea of using hooks will do the trick. I will look into that. This might solve whole bunch of issues. And if child windows will be taken into account maybe all related issues could be resolved.

pthom commented 1 month ago

@lukaasm : your patch is brillant!

I confirm that it solves a lot of issues, whenever a popup is opened (e.g. by ImGui::Combo, ImGui::ColorEdit, etc.)


About child windows

As far as child windows are concerned, there is still an issue as @thedmd wrote. I made a an investigation into this, which I summarize below.

Which widgets are concerned

In order to have a more complete idea of the impact, I did a search through the whole ImGui codebase, and the only widgets that will use BeginChild/EndChild are:

    ImGui::InputTextMultiline()
    ImGui::BeginListbox() and ImGui::EndListbox()
    ImGui::BeginChild() and ImGui::EndChild()

How to inform users they should not use child windows

An additional patch can be added to inform users that they should not use ImGui::Begin/EndChild when in the canvas. I know that it is a band aid but some might need it.

It can be implemented as follows:

Somewhere at the beginning of imgui.cpp Add functions that will be called upon entering/exiting the canvas

static bool gIsInNodeEditorCanvas = false;
void Priv_ImGuiNodeEditor_EnterCanvas() { gIsInNodeEditorCanvas = true; }
void Priv_ImGuiNodeEditor_ExitCanvas() { gIsInNodeEditorCanvas = false; }
bool Priv_ImGuiNodeEditor_IsInCanvas() { return gIsInNodeEditorCanvas; }

At the beginning of ImGui::BeginChildEx Check if we are in a canvas, and assert with a useful message.

bool ImGui::BeginChildEx(const char* name, ImGuiID id, const ImVec2& size_arg, ImGuiChildFlags child_flags, ImGuiWindowFlags window_flags)
{
    if (gIsInNodeEditorCanvas)
    {
        const char* msg = R"(
    Sorry, some ImGui widgets are incompatible withing imgui-node-editor, and cannot be used while its canvas is active.
        Incompatible widgets are:
            ImGui::InputTextMultiline()
            ImGui::BeginListbox() and ImGui::EndListbox()
            ImGui::BeginChild() and ImGui::EndChild()

        )";
        if ((name != nullptr) && (strlen(name) > 0))
            fprintf(stderr, "%sImGui::BeginChildEx was called with name=%s\n", msg, name);
        else
            fprintf(stderr, "%s(no name available, please examine the call stack)\n", msg);
        IM_ASSERT(false && "ImGui::BeginChild should not be called inside a node editor canvas");
    }
   ...

inside imgui_canvas.cpp

  1. At the beginning, declare the functions from imgui.cpp

    // from imgui.cpp 
    void Priv_ImGuiNodeEditor_EnterCanvas();
    void Priv_ImGuiNodeEditor_ExitCanvas();
  2. call those inside ImGuiEx::Canvas::EnterLocalSpace and ImGuiEx::Canvas::LeaveLocalSpace

pthom commented 1 month ago

Edit: see this link for a more complete implementation.

An additional patch below, in order to handle ImGui::TextMultiline

This patch works only if the previous patches mentioned in this current thread are applied (https://github.com/thedmd/imgui-node-editor/issues/242#issuecomment-1681806764 and https://github.com/thedmd/imgui-node-editor/issues/242#issuecomment-2404714757)

patch ImGui::InputTextMultiline like this:

imgui_widgets.cpp

bool Priv_ImGuiNodeEditor_IsInCanvas();

bool ImGui::InputTextMultiline(const char* label, char* buf, size_t buf_size, const ImVec2& size, ImGuiInputTextFlags flags, ImGuiInputTextCallback callback, void* user_data)
{
    // [ADAPT_IMGUI_BUNDLE] cf
    // When inside imgui-node-editor canvas, we cannot open child windows
    // In this case, we present a one line version of the input text,
    // and offer the possibility to open a popup to edit the text in a multiline widget
    if (Priv_ImGuiNodeEditor_IsInCanvas())
    {
        PushID(label); // make sure to use unique ids
        bool changed = false;

        // A- One line version text, without the label
        if (size.x > 0.f)
            SetNextItemWidth(size.x);
        if (InputText("##hidden_label", buf, buf_size, flags, callback, user_data))
            changed = true;
        float input_text_actual_width = GetItemRectSize().x;  // we will reuse this width for the widget in the popup
        SameLine();

        // B- Add a button to open a popup to edit the text
        //   i. First, move the cursor to the left, so that the button appears right next to the input text
        ImVec2 pos = ImGui::GetCursorScreenPos();
        pos.x -= (ImGui::GetStyle().ItemSpacing.x);
        ImGui::SetCursorScreenPos(pos);
        //   ii. Then add the button
        if (SmallButton("..."))
            OpenPopup("InputTextMultilinePopup");

        // C. Finally, add the label (up until "##")
        const char* label_end = FindRenderedTextEnd(label);
        SameLine();
        TextUnformatted(label, label_end);

        // D. Handle the popup
        if (ImGui::BeginPopup("InputTextMultilinePopup"))
        {
            // Note: there is no infinite recursion here, since we are not inside the canvas anymore
            // (as soon as BeginPopup return true, we are outside the canvas)
            // (iif the patches https://github.com/thedmd/imgui-node-editor/issues/242#issuecomment-1681806764
            //  and https://github.com/thedmd/imgui-node-editor/issues/242#issuecomment-2404714757 are applied)
            ImVec2 size_multiline = size;
            size_multiline.x = input_text_actual_width;
            if (InputTextMultiline("##edit", buf, buf_size, size_multiline, flags, callback, user_data))
                changed = true;
            EndPopup();
        }
        PopID();
        return changed;
    }
    // [/ADAPT_IMGUI_BUNDLE]
    else
    {
        // Standard behavior outside of imgui-node-editor canvas
        return InputTextEx(label, NULL, buf, (int)buf_size, size, flags | ImGuiInputTextFlags_Multiline, callback, user_data);
    }
}

This gives:

image

And when opening the popup:

image
Riztazz commented 1 week ago

If you're interleaving draw channels, you need to cache and restore proper draw channels in the callbacks. Suspend expects us to be on Canvas::m_ExpectedChannel which is not always the case.

Thanks for the patch ❤️

GerradLong commented 5 days ago

Hello! :D I've integrated this hook hack for pickers/popups but something is not right.

I've created a testing node with a color picker: image

blueprintBuilder.Input(pinID);

...

static float colors[4] = { 1.0f, 1.0f, 1.0f, 1.0f };
ImGui::ColorEdit4("color", colors, ImGuiColorEditFlags_DefaultOptions_);

...

blueprintBuilder.EndInput();

this is what I get when I put the mouse on the preview:

image

The picker is called inside the "pin," but it has the same effect when it's called inside the node instead.