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

Column problem #28

Closed
durswd opened this issue Jul 16, 2019 · 11 comments
Closed

Column problem #28

durswd opened this issue Jul 16, 2019 · 11 comments
Assignees
Labels

Comments

@durswd
Copy link

durswd commented Jul 16, 2019

I found a problem.

This code causes assert in Suspend.

ImGui::Begin("Begin");
ImGui::Columns(2);
ImGui::NextColumn();

ed::SetCurrentEditor(g_Context);
ed::Begin("My Editor", ImVec2(0.0, 0.0f));

// assert!
ed::Suspend();
ed::Resume();

ed::End();
ed::SetCurrentEditor(nullptr);

ImGui::Columns(1);
mGui::End();

But this codes runs with Begin and End child.

ImGui::Begin("Begin");
ImGui::Columns(2);
ImGui::NextColumn();

// add
ImGui::BeginChild("Child");
ed::SetCurrentEditor(g_Context);
ed::Begin("My Editor", ImVec2(0.0, 0.0f));

ed::Suspend();
ed::Resume();

ed::End();
ed::SetCurrentEditor(nullptr);

// add
ImGui::EndChild();

ImGui::Columns(1);
mGui::End();

Is it a bug?

Best regards.

@thedmd
Copy link
Owner

thedmd commented Jul 16, 2019

Hello @durswd,

I'm afraid there is a conflict here in the ImDrawList channel splitter. Columns API now is using it to separate column drawing. Node editor is also using channel splitter to manage node drawing.
Wrapping editor in child window avoid the problem, because child window has its own copy of ImDrawList.

Please use workaround with child window now, I will need to think how to address this problem.

@ocornut Is there a way to split single channel into sub-channels, to make channel splitter recursive? Or is this out of the scope of ImDrawList?

@thedmd thedmd added the bug label Jul 16, 2019
@ocornut
Copy link
Contributor

ocornut commented Jul 16, 2019

It should be possible now if you use the ImDrawListSplitter helper directly.
The upcoming table system uses that to split recursively.

(ImDrawList carry its own ImDrawListSplitter for legacy reason, but you can ignore that instance and use yours).

@thedmd
Copy link
Owner

thedmd commented Jul 16, 2019

Thank you for advice. This is almost exactly what I did.

Since my code relay on splitter inside ImDrawList I ended up swapping splitter while editor is drawing to avoid major rewrite.

@thedmd thedmd closed this as completed Jul 16, 2019
@thedmd
Copy link
Owner

thedmd commented Jul 16, 2019

@durswd Assert is solved. Editor should work in both cases now. Solution was simpler than I initially thought.

@durswd
Copy link
Author

durswd commented Jul 17, 2019

Thank you !

@meshula
Copy link

meshula commented Jul 21, 2019

I ran into a similar issue. I thought I might organize my node drawing using Columns, but can't due to the ImDrawList issue. Of course I can implement the node contents a different way, but thought I would leave another test case here for you.

    ed::BeginNode(nodeB_Id);
        ImGui::Text("Node B");
        ImGui::Columns(2);
            ed::BeginPin(nodeB_InputPinId1, ed::PinKind::Input);
                ImGui::Text("-> In1");
            ed::EndPin();
            ed::BeginPin(nodeB_InputPinId2, ed::PinKind::Input);
                ImGui::Text("-> In2");
            ed::EndPin();
        ImGui::NextColumn();
            ed::BeginPin(nodeB_OutputPinId, ed::PinKind::Output);
                ImGui::Text("Out ->");
            ed::EndPin();
        ImGui::Columns();
    ed::EndNode();

The assert occurs here:

void ImDrawListSplitter::Split(ImDrawList* draw_list, int channels_count)
{
    IM_ASSERT(_Current == 0 && _Count <= 1);

@thedmd
Copy link
Owner

thedmd commented Jul 21, 2019

I see the problem. I should be able to make similar fix to enable this use case.

@thedmd
Copy link
Owner

thedmd commented Jul 21, 2019

@meshula I fixed an assert. Now channel splitting can be used between Begin/End for nodes and pins.

However Columns API will not work inside node because it relay on windows size internally and always span to full width, which is incorrect for node. In node you want to minimize size, that's why I used stack layouts. There is a workaround for that issue:

void ImGuiEx_BeginColumn()
{
    ImGui::BeginGroup();
}

void ImGuiEx_NextColumn()
{
    ImGui::EndGroup();
    ImGui::SameLine();
    ImGui::BeginGroup();
}

void ImGuiEx_EndColumn()
{
    ImGui::EndGroup();
}

// and code:
    ed::BeginNode(nodeB_Id);
        ImGui::Text("Node B");
        ImGuiEx_BeginColumn();
            ed::BeginPin(nodeB_InputPinId1, ed::PinKind::Input);
                ImGui::Text("-> In1");
            ed::EndPin();
            ed::BeginPin(nodeB_InputPinId2, ed::PinKind::Input);
                ImGui::Text("-> In2");
            ed::EndPin();
        ImGuiEx_NextColumn();
            ed::BeginPin(nodeB_OutputPinId, ed::PinKind::Output);
                ImGui::Text("Out ->");
            ed::EndPin();
        ImGuiEx_EndColumn();
    ed::EndNode();

Does this works for you?

The way Column API work collide with how canvas work. Maybe it is a way to make work both but that will require sending PR's to patch Column API and as far as I know it is still in the flux, changing and probably evolve even more with tables @ocornut is working on. So until that's done I think above workaround is the next best thing we got.

@meshula
Copy link

meshula commented Jul 21, 2019

Yes, that's perfect. Your example is a nice "cookbook" reference :)

@mircolosi
Copy link

Hi everybody,
first of all I have to admit that the work behind imgui and imgui-node-editor is impressive and high-quality. So thank you all for that.
I had the same problem of assertion fail while trying to right-click the grid canvas in the Blueprint example.
Is there a fast workaround I can use to solve this problem?

This is the error I get

Blueprints:[...]/imgui-node-editor/ThirdParty/imgui/imgui_draw.cpp:1308: void ImDrawListSplitter::SetCurrentChannel(ImDrawList*, int): Assertion `idx < _Count' failed.
Aborted

thedmd added a commit that referenced this issue Jul 23, 2019
Introduction of ImDrawListSplitter changed how channels should
be managed internally. Now we do not need to worry about user
code colliding with ours.
@thedmd
Copy link
Owner

thedmd commented Jul 23, 2019

Thanks.
Assert is fixed now.

@thedmd thedmd closed this as completed Jul 23, 2019
fdfxalex pushed a commit to fdfxalex/imgui-node-editor that referenced this issue Dec 31, 2019
fdfxalex pushed a commit to fdfxalex/imgui-node-editor that referenced this issue Dec 31, 2019
fdfxalex pushed a commit to fdfxalex/imgui-node-editor that referenced this issue Dec 31, 2019
Introduction of ImDrawListSplitter changed how channels should
be managed internally. Now we do not need to worry about user
code colliding with ours.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants