Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
brunopj1 opened this issue Jul 7, 2023 · 10 comments
Open

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

brunopj1 opened this issue Jul 7, 2023 · 10 comments

Comments

@brunopj1
Copy link

brunopj1 commented Jul 7, 2023

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
Copy link

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
Copy link

SC5Shout commented Jul 19, 2023

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
Copy link

lukaasm commented Aug 17, 2023

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
Copy link

lapinozz commented Jul 4, 2024

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

@thedmd
Copy link
Owner

thedmd commented Jul 5, 2024

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 added a commit to pthom/imgui that referenced this issue Oct 10, 2024
cf thedmd/imgui-node-editor#242 (comment)

This helps fixing issues in popup positionning inside the node editor
pthom added a commit to pthom/imgui that referenced this issue Oct 10, 2024
cf thedmd/imgui-node-editor#242 (comment)

This helps fixing issues in popup positionning inside the node editor
pthom added a commit to pthom/imgui-node-editor that referenced this issue Oct 10, 2024
cf thedmd#242 (comment)

This helps fixing issues in popup positionning inside the node editor
@pthom
Copy link

pthom commented Oct 10, 2024

@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();
  1. call those inside ImGuiEx::Canvas::EnterLocalSpace and ImGuiEx::Canvas::LeaveLocalSpace

pthom added a commit to pthom/imgui-node-editor that referenced this issue Oct 10, 2024
pthom added a commit to pthom/imgui that referenced this issue Oct 10, 2024
cf thedmd/imgui-node-editor#242 (comment)

This helps fixing issues in popup positionning inside the node editor
pthom added a commit to pthom/imgui-node-editor that referenced this issue Oct 10, 2024
@pthom
Copy link

pthom commented Oct 10, 2024

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 (#242 (comment)
and #242 (comment))

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

pthom added a commit to pthom/imgui that referenced this issue Oct 27, 2024
cf thedmd/imgui-node-editor#242 (comment)

This helps fixing issues in popup positionning inside the node editor
@Riztazz
Copy link

Riztazz commented Nov 18, 2024

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 ❤️

pthom added a commit to pthom/imgui that referenced this issue Nov 21, 2024
cf thedmd/imgui-node-editor#242 (comment)

This helps fixing issues in popup positionning inside the node editor
@GerradLong
Copy link

GerradLong commented Nov 21, 2024

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.

@joenot443
Copy link

@pthom Thanks for the patch and the detailed notes, it's been very enlightening.

Do we ever expect Child windows to function within the canvas? How far off are we?

In an ideal world I'd like to draw a scrollable area within a node which will override the scroll actions when the mouse is hovered over it, as one would expect a child window to behave. In the past I've drawn a window outside the canvas draw code using an ed::GetNodePosition and some offsets, but as others have noticed it isn't perfect.

pthom added a commit to pthom/imgui that referenced this issue Dec 18, 2024
cf thedmd/imgui-node-editor#242 (comment)

This helps fixing issues in popup positionning inside the node editor
pthom added a commit to pthom/imgui that referenced this issue Dec 22, 2024
cf thedmd/imgui-node-editor#242 (comment)

This helps fixing issues in popup positionning inside the node editor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants