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

Nesting multiple imgui contexts (glfw+opengl3) #2004

Closed
jdumas opened this issue Aug 4, 2018 · 49 comments
Closed

Nesting multiple imgui contexts (glfw+opengl3) #2004

jdumas opened this issue Aug 4, 2018 · 49 comments

Comments

@jdumas
Copy link
Contributor

jdumas commented Aug 4, 2018

Hi,

I'm struggling a bit to create nested GLFW windows with ImGui menus, and I was wondering what would be the best way to approach the problem. In my case, I have a single thread, and I just want to be able to spawn a subviewer from anywhere for debugging purposes. Once the user closes the subviewer, life should go on in the parent window. I am not really interested in sharing OpenGL context between the windows, that sounds a bit messy.

In the current implementation of imgui_impl_opengl3.cpp, we have a bunch of global variables holding data for the current context. What I'm doing right now is, before spawning the new viewer, I clear the current object/context/whatever, spawn the viewer, and then basically reinitialize ImGui for the parent window once the subviewer is closed.

This kind of works, but only if the subviewer is created outside a ImGui::NewFrame()/ImGui::Render() block. Otherwise I'm getting the error "Cannot modify a locked ImFontAtlas between NewFrame() and EndFrame/Render()!"

Lmk if you have any suggestion on how to achieve this.

@ocornut
Copy link
Owner

ocornut commented Aug 4, 2018 via email

@jdumas
Copy link
Contributor Author

jdumas commented Aug 4, 2018

Thanks I will take a look. Maybe I'm making it more complicated than it needs to be. But you're sure that I can spawn a new window and enter a new rendering loop while between the NewFrame()/EndFrame() of the parent window? Do you suggest to also share the ImGui context between them? Wouldn't that pose an issue with the ImFontAtlas being locked by the parent window?

@ocornut
Copy link
Owner

ocornut commented Aug 4, 2018 via email

@jdumas
Copy link
Contributor Author

jdumas commented Aug 4, 2018

But I do want to enter a separate render loop. My viewer is a class (this is for libigl), and I want to easily be able to spawn a new viewer at any point for debugging purposes.

@ocornut
Copy link
Owner

ocornut commented Aug 4, 2018

I fail to understand the benefit of entering a separate render loop (do you mean you want to freeze the underlying application completely to create a sort of debugger inspecting the frozen contents?).

Anyway:

, I clear the current object/context/whatever, spawn the viewer, and then basically reinitialize ImGui for the parent window once the subviewer is closed.

You can create multiple imgui context so there's no need to "clear" or "destroy" the first context.

This kind of works, but only if the subviewer is created outside a ImGui::NewFrame()/ImGui::Render() block. Otherwise I'm getting the error "Cannot modify a locked ImFontAtlas between NewFrame() and EndFrame/Render()!"

What is the atlas modification causing the error? What's the callstack.

@jdumas
Copy link
Contributor Author

jdumas commented Aug 5, 2018

I fail to understand the benefit of entering a separate render loop (do you mean you want to freeze the underlying application completely to create a sort of debugger inspecting the frozen contents?).

Yes that's exactly it. Right now in libigl the code you need to launch a viewer and display a mesh is just 3 lines:

igl::opengl::glfw::Viewer viewer;
viewer.data().set_mesh(V, F);
viewer.launch();

Where (V,F) is the triangle mesh you want to inspect (points and triangles). There's a one-liner to plot points, and one for lines as well. The last line initializes the glfw viewer and launches the main rendering loop. It is convenient to throw that inside a function that is doing some computation, as a way to debug the 3D data you are manipulating. Since this computation may very well be done within a ImGui::NewFrame() (e.g. the user presses a button "Optimize" or whatever), this case may arise more often than not.

You can create multiple imgui context so there's no need to "clear" or "destroy" the first context.

Yes, but the problem lies in the imgui_impl_opengl3.cpp file, which contains globals such as g_FontTexture,g_ShaderHandle, etc. So if I create a window/imgui menu with a different context, it is not aware of it.

What is the atlas modification causing the error? What's the callstack.

Sorry I'll do more testing tomorrow and provide that information =)

@ocornut
Copy link
Owner

ocornut commented Aug 5, 2018

Each imgui context created can share the same ImFontAtlas (it is a parameter to ImGui::CreateContext).
You are indeed not allowed to modify a font atlas while it is bound to an ImGui frame, but that's not needed at all for what you are doing.

ImFontAtlas* atlas = new ImFontAtlas();
ImGuiContext* main_context = ImGui::CreateContext(atlas);
ImGuiContext* another_context = ImGui::CreateContext(atlas);

etc.

Yes, but the problem lies in the imgui_impl_opengl3.cpp file, which contains globals such as g_FontTexture,g_ShaderHandle, etc. So if I create a window/imgui menu with a different context, it is not aware of it.

As for the opengl context you need shared context to use the imgui_impl_opengl3.cpp code as is (g_FontTexture/g_ShaderHandle are the same for all shared context). It is setup in the last parameter of glfwCreateWindow() so I'm not sure why resisting this.

@ocornut ocornut added the opengl label Aug 5, 2018
@ocornut ocornut changed the title Nested viewer windows with ImGui menus (glfw+opengl3) Nest/multiple imgui contexts (glfw+opengl3) Aug 5, 2018
@ocornut ocornut changed the title Nest/multiple imgui contexts (glfw+opengl3) Nesting multiple imgui contexts (glfw+opengl3) Aug 5, 2018
@jdumas
Copy link
Contributor Author

jdumas commented Aug 5, 2018

You are indeed not allowed to modify a font atlas while it is bound to an ImGui frame, but that's not needed at all for what you are doing.

Well, it kind of is. Here is an example of the kind of workflow I am looking for:

// #include headers
int main(void) {
    Eigen::MatrixXd V;
    Eigen::MatrixXi F;

    // Load a triangle mesh
    igl::readOFF("bunny.off", V, F);

    igl::opengl::glfw::Viewer viewer;
    viewer.data().set_mesh(V, F);

    menu.callback_draw_custom_window = [&]() {
        ImGui::Begin("New Window");
        if (ImGui::Button("Do Stuff")) {
            igl::opengl::glfw::Viewer subviewer;
            subviewer.data().set_mesh(V, F);
            subviewer.launch(); // enter subviewer render loop
        }
        ImGui::End();
    };

    viewer.launch(); // main viewer render loop

    return 0;
}

The callback_draw_custom_window() is called between a ImGui::NewFrame(); and a ImGui::Render() in the rendering loop of the viewer.

Again, I don't mind sharing the OpenGL context, but this wouldn't help in the kind of scenario I am looking to have (since the subviewer here cannot be launched inside a ImGui frame).

@ocornut
Copy link
Owner

ocornut commented Aug 5, 2018

But nothing in that code will MODIFY the ImFontAtlas used by the main context.
The assert you got is related to modifying a font atlas.

@jdumas
Copy link
Contributor Author

jdumas commented Aug 5, 2018

Ah, got it. That's probably because the initialization code for the Viewer is reloading the font and that could be avoided. Let's see if I can refactor my code to avoid that. There's one case where I need to reload the font though: when moving the window between screens with different DPI. In this case moving the subviewer to a different DPI screen will lead to an error. Granted that's a pretty rare use case, and I know better hidpi support is something on your roadmap so I think that's ok.

@ocornut
Copy link
Owner

ocornut commented Aug 5, 2018

It's also ok to reload the font if you do it in a new ImFontAtlas instance.

There's one case where I need to reload the font though:

It's also perfectly fine (and frequent for many users) do add/remove/reload fonts but you need to do it before NewFrame(). Vertices have already been submitted for rendering and altering the font atlas would lead to corrupted visuals.
So if you detect a DPI change just process it before calling NewFrame().

@jdumas
Copy link
Contributor Author

jdumas commented Aug 5, 2018

I do it before the NewFrame() of the nested viewer ofc, but for the parent viewer, when the subviewer exits, the code will resume between the parent NewFrame() and Render() function. I guess the only sensible solution would be to provide an easy way to end the parent frame before spawning the subviewer.

@ocornut
Copy link
Owner

ocornut commented Aug 5, 2018

I see, sorry I didn't connect the DPI change issue with the nested viewers.
Well in this case you need 1 ImFontAtlas for each ImGuiContext, so the font atlas won't be shared between the parent and the nested viewer. Then the nested viewer can do whatever it wants.

It's almost as you are spawning a new process here (and in fact, you could perhaps even fork() and make the parent wait for the child exit as well.).

@jdumas
Copy link
Contributor Author

jdumas commented Aug 5, 2018

Does fork() work on Windows? =)
Yes it's almost like spawning a new process. The thing with using 1 ImFontAtlas for each context is that I don't know how to pass that information to the opengl3 implementation without destroying the current g_FontTexture and other (I haven't look too much into the details so far, just looking for suggestions).

@ocornut
Copy link
Owner

ocornut commented Aug 5, 2018

Surrounding the nested viewer code with an outer ImGui_ImplOpenGL3_DestroyDeviceObjects() .. ImGui_ImplOpenGL3_CreateDeviceObjects() (or even ImGui_ImplOpenGL3_Shutdown() .. ImGui_ImplOpenGL3_Init() just as well) that runs on the main imgui context should be enough.

Neither will affect the ImFontAtlas of the main imgui context, so ImFontAtlas::Build() won't be called as it it's been already built.

Essentially this issue needs the call-stack for how you got this error in the first place (notice the Issue Template/Contribute documents "If you are discussing an assert or a crash, please provide a debugger callstack.", there's a reason for that!).

Make sure you also surround your viewer with two ImGui::SetCurrentContext() calls (one to apply the nested context, one to restore the main one).

@jdumas
Copy link
Contributor Author

jdumas commented Aug 5, 2018

Ok I will try that and provide the call stack if I run into the assert again!

@jdumas
Copy link
Contributor Author

jdumas commented Aug 5, 2018

Alright. You're gonna hate me, but the application actually crashes without a callstack this time:

invalid value
X Error of failed request:  BadWindow (invalid Window parameter)
  Major opcode of failed request:  3 (X_GetWindowAttributes)
  Resource id in failed request:  0x0
  Serial number of failed request:  1307
  Current serial number in output stream:  1308
[Thread 0x7ffff1701700 (LWP 10371) exited]
[Inferior 1 (process 10365) exited with code 01]
(gdb) bt
No stack.

I tried with the latest tip of the GLFW github repo, doesn't help. Here is the code have in my viewer class:

void ImGuiMenu::init() {
  ImGui_ImplOpenGL3_Shutdown(); // destroy parent GL objects
  context_ = ImGui::CreateContext();
  ImGui::SetCurrentContext(context_);
  ImGui_ImplGlfw_InitForOpenGL(viewer->window, false);
  ImGui_ImplOpenGL3_Init("#version 150");
  reload_font();
}

void ImGuiMenu::reload_font() {
  ImGuiIO& io = ImGui::GetIO();
  io.Fonts->Clear();
  io.Fonts->AddFontFromMemoryCompressedTTF(...);
}

void ImGuiMenu::shutdown() {
  // Cleanup
  ImGui_ImplOpenGL3_Shutdown();
  ImGui_ImplGlfw_Shutdown();
  ImGui::DestroyContext(context_);
  context_ = nullptr;
}

void ImGuiMenu::restore() {
  ImGui::SetCurrentContext(context_);
  ImGui_ImplOpenGL3_Init(); // recreate GL objects on parent viewer
}

init()/shutdown() are called when the viewer is created/destroyed, and restore() is called on the parent viewer after the subviewer is deleted. And the code I use to create the subviewer looks like this:

void draw() {
  ImGui_ImplOpenGL3_NewFrame();
  ImGui_ImplGlfw_NewFrame();
  ImGui::NewFrame();
  draw_menu();
  ImGui::Render();
  ImGui_ImplOpenGL3_RenderDrawData(ImGui::GetDrawData());
}

void draw_menu() {
  ImGui::Begin("Menu");
  if (ImGui::Button("New Viewer")) {
    igl::opengl::glfw::Viewer viewer;
    igl::opengl::glfw::imgui::ImGuiMenu menu;
    viewer.plugins.push_back(&menu);
    viewer.data().set_mesh(V, F);
    viewer.launch();
    ImGui::End();
    return;
  }
  ImGui::End();
}

@jdumas
Copy link
Contributor Author

jdumas commented Aug 5, 2018

Ok turns out the imgui_impl_glfw implementation also has some global variables g_Window that were responsible for this crash. Now If I change my init/restore function to also clear reset the glfw bindings:

void ImGuiMenu::init() {
  ImGui_ImplOpenGL3_Shutdown(); // destroy parent GL objects
  ImGui_ImplGlfw_Shutdown();
  context_ = ImGui::CreateContext();
  ImGui::SetCurrentContext(context_);
  ImGui_ImplGlfw_InitForOpenGL(viewer->window, false);
  ImGui_ImplOpenGL3_Init("#version 150");
  reload_font();
}

void ImGuiMenu::restore() {
  ImGui::SetCurrentContext(context_);
  ImGui_ImplOpenGL3_Init("#version 150");
  ImGui_ImplOpenGL3_Init(); // recreate GL objects on parent viewer
}

Then I get a crash when closing the subviewer with the following call stack:

Thread 1 "106_ViewerMenu_" received signal SIGSEGV, Segmentation fault.
0x00007ffff664cbdf in __memmove_avx_unaligned_erms () from /usr/lib/libc.so.6
(gdb) bt
#0  0x00007ffff664cbdf in __memmove_avx_unaligned_erms () from /usr/lib/libc.so.6
#1  0x00007ffff264cb28 in ?? () from /usr/lib/dri/i965_dri.so
#2  0x00007ffff265a158 in ?? () from /usr/lib/dri/i965_dri.so
#3  0x00007ffff264bfea in ?? () from /usr/lib/dri/i965_dri.so
#4  0x00007ffff28e16f9 in ?? () from /usr/lib/dri/i965_dri.so
#5  0x00007ffff28e20df in ?? () from /usr/lib/dri/i965_dri.so
#6  0x0000555555e907c9 in ImGui_ImplOpenGL3_RenderDrawData (draw_data=0x555556679b80) at /home/jdumas/workspace/github/libigl/external/imgui/imgui_impl_opengl3.cpp:217
#7  0x0000555555c1f10f in igl::opengl::glfw::imgui::ImGuiMenu::post_draw (this=0x7fffffffda60) at /home/jdumas/workspace/github/libigl/include/igl/opengl/glfw/imgui/ImGuiMenu.cpp:110
#8  0x0000555555c07ba5 in igl::opengl::glfw::Viewer::draw (this=0x7fffffffdb00) at /home/jdumas/workspace/github/libigl/include/igl/opengl/glfw/Viewer.cpp:870
#9  0x0000555555c04ac3 in igl::opengl::glfw::Viewer::launch_rendering (this=0x7fffffffdb00, loop=true) at /home/jdumas/workspace/github/libigl/include/igl/opengl/glfw/Viewer.cpp:225
#10 0x0000555555c0467b in igl::opengl::glfw::Viewer::launch (this=0x7fffffffdb00, resizable=true, fullscreen=false) at /home/jdumas/workspace/github/libigl/include/igl/opengl/glfw/Viewer.cpp:132
#11 0x0000555555bd3886 in main (argc=1, argv=0x7fffffffdfc8) at /home/jdumas/workspace/github/libigl/tutorial/106_ViewerMenu/main.cpp:130

So I guess the problem is that I need to call the parent's ImGui::Render() and ImGui_ImplOpenGL3_RenderDrawData(...) before spawning the new viewer inside the if (ImGui::Button("New Viewer")) { ... }. That's fine for this use case, but to spawn a sub-viewer from a more deeply nested piece of call I'd have to unwind the whole stack. I figure the best way to achieve this at this point would be to use a custom C++ exception and have a try/catch in my draw() function.

@jdumas
Copy link
Contributor Author

jdumas commented Aug 5, 2018

Ok I can get something kinda working with an exception, like

if (ImGui::Button("Sub Viewer")) {
  ImGui::End();
  throw AbortFrame([&]() {
    igl::opengl::glfw::Viewer viewer;
    igl::opengl::glfw::imgui::ImGuiMenu menu;
    viewer.plugins.push_back(&menu);
    viewer.data().set_mesh(V, F);
    viewer.launch();
  });
}

And changing the drawing call to the following:

struct AbortFrame : public std::exception {
  AbortFrame(std::function<void(void)> func) : deferred_callback_(func) { }
  std::function<void(void)> deferred_callback_;
};

void draw() {
  ImGui_ImplOpenGL3_NewFrame();
  ImGui_ImplGlfw_NewFrame();
  ImGui::NewFrame();
  try {
    draw_menu();
    ImGui::Render();
    ImGui_ImplOpenGL3_RenderDrawData(ImGui::GetDrawData());
  } catch (const AbortFrame &e) {
    ImGui::EndFrame();
    e.deferred_callback_();
  }
}

The problem is that I still have to call ImGui::End() from within the "Sub Viewer" button. Is there a way to close all open Begin() right now in ImGui?

@ocornut
Copy link
Owner

ocornut commented Aug 5, 2018

Is there a way to close all open Begin() right now in ImGui?

You may read g.CurrentWindowStack.Size and call End() the appropriate of time. But then it looks like you are looking for weird workaround to a problem that probably have a simple solution.

void ImGuiMenu::restore() {
  ImGui::SetCurrentContext(context_);

Is that a typo or missing some code, or are you restoring the child imgui context you just created? Also where is the destroy call for that child imgui context?

You may just call Shutdown() on the imgui context and recreate one, the problem will always be that the code following the closure of the viewer may be doing ImGui:: calls that are invalid.

You may also just create your own OpenGL3 renderer if that's relevant to you. Those files are in an examples/ folder for the reason that you can freely rework them.

The code that interface with your OS and/or OpenGL is small and under your control, you have the code and debugger, there's no magic, you should be able to figure it out without weird exception hacks, but I can't be debugging your project/code for you.

@jdumas
Copy link
Contributor Author

jdumas commented Aug 5, 2018

Is that a typo or missing some code, or are you restoring the child imgui context you just created? Also where is the destroy call for that child imgui context?

No it's not a typo. I haven't included the whole code to not pollute the thread but basically the shutdown sequence is just:

subviewer.shutdown();
parent.restore();

So the restore() is reinstating its own context_. The child context is destroyed when calling subviewer.shutdown().

You may just call Shutdown() on the imgui context and recreate one, the problem will always be that the code following the closure of the viewer may be doing ImGui:: calls that are invalid.

That's exactly what I'm struggling with. I don't see a way around that right now without the ugly exception hacks, or without rewriting parts of the glfw/opengl binding code (to have a stash/pop functionality for the global variables). I'd like to avoid doing the latter so I can keep up with the updates more easily, but I can give it a try.

@ocornut
Copy link
Owner

ocornut commented Aug 5, 2018

OK.

The saner solution we devised above should work, so the OpenGL crash you mention in:
#2004 (comment)

Should probably be investigated and clarified. There's an error somewhere waiting to be found, either in your code, either something that make a Shutdown/Init sequence in imgui_impl_opengl3.cpp not totally without side-effect?

@jdumas
Copy link
Contributor Author

jdumas commented Aug 5, 2018

The error I mentioned in #2004 (comment) was caused by the line

glDrawElements(GL_TRIANGLES, (GLsizei)pcmd->ElemCount, sizeof(ImDrawIdx) == 2 ? GL_UNSIGNED_SHORT : GL_UNSIGNED_INT, idx_buffer_offset);

So probably it is due to the ImGui draw list referring to OpenGL objects that were destroyed during the initialization of the child viewer.

For the record I did a quick test implementing a state stack for the OpenGL/GLFW backends:

static GLFWwindow*      g_Window = NULL;
static GlfwClientApi    g_ClientApi = GlfwClientApi_Unknown;
static double           g_Time = 0.0;
static std::array<bool, 5>             g_MouseJustPressed = { false, false, false, false, false };
static std::array<GLFWcursor*, ImGuiMouseCursor_COUNT>      g_MouseCursors = { 0 };

struct ImGui_ImplGlfw_Data {
    GLFWwindow*      g_Window;
    GlfwClientApi    g_ClientApi;
    double           g_Time;
    std::array<bool, 5>             g_MouseJustPressed;
    std::array<GLFWcursor*, ImGuiMouseCursor_COUNT>      g_MouseCursors;
};
static std::vector<ImGui_ImplGlfw_Data> g_DataStack;

void ImGui_ImplGlfw_PushState() {
    ImGui_ImplGlfw_Data state;
    state.g_Window = g_Window;
    state.g_ClientApi = g_ClientApi;
    state.g_Time = g_Time;
    state.g_MouseJustPressed = g_MouseJustPressed;
    state.g_MouseCursors = g_MouseCursors;
    g_DataStack.push_back(state);
    g_Window = NULL;
    g_ClientApi = GlfwClientApi_Unknown;
    g_Time = 0.0;
    g_MouseJustPressed = { false, false, false, false, false };
    g_MouseCursors = { 0 };
}

void ImGui_ImplGlfw_PopState() {
    if (g_DataStack.empty()) {
        return;
    }
    const ImGui_ImplGlfw_Data & state = g_DataStack.back();
    g_Window = state.g_Window;
    g_ClientApi = state.g_ClientApi;
    g_Time = state.g_Time;
    g_MouseJustPressed = state.g_MouseJustPressed;
    g_MouseCursors = state.g_MouseCursors;
    g_DataStack.pop_back();
}

And for OpenGL:

// OpenGL Data
static std::array<char, 32> g_GlslVersionString { '\0' };
static GLuint       g_FontTexture = 0;
static GLuint       g_ShaderHandle = 0, g_VertHandle = 0, g_FragHandle = 0;
static int          g_AttribLocationTex = 0, g_AttribLocationProjMtx = 0;
static int          g_AttribLocationPosition = 0, g_AttribLocationUV = 0, g_AttribLocationColor = 0;
static unsigned int g_VboHandle = 0, g_ElementsHandle = 0;

// OpenGL Data
struct ImGui_ImplOpenGL3_Data {
    std::array<char, 32>         g_GlslVersionString { '\0' };
    GLuint       g_FontTexture = 0;
    GLuint       g_ShaderHandle = 0, g_VertHandle = 0, g_FragHandle = 0;
    int          g_AttribLocationTex = 0, g_AttribLocationProjMtx = 0;
    int          g_AttribLocationPosition = 0, g_AttribLocationUV = 0, g_AttribLocationColor = 0;
    unsigned int g_VboHandle = 0, g_ElementsHandle = 0;
};
std::vector<ImGui_ImplOpenGL3_Data> g_DataStack;

// Nested states
void     ImGui_ImplOpenGL3_PushState() {
    ImGui_ImplOpenGL3_Data state;
    state.g_GlslVersionString = g_GlslVersionString;
    state.g_FontTexture = g_FontTexture;
    state.g_ShaderHandle = g_ShaderHandle;
    state.g_VertHandle = g_VertHandle;
    state.g_FragHandle = g_FragHandle;
    state.g_AttribLocationTex = g_AttribLocationTex;
    state.g_AttribLocationProjMtx = g_AttribLocationProjMtx;
    state.g_AttribLocationPosition = g_AttribLocationPosition;
    state.g_AttribLocationUV = g_AttribLocationUV;
    state.g_AttribLocationColor = g_AttribLocationColor;
    state.g_VboHandle = g_VboHandle;
    state.g_ElementsHandle = g_ElementsHandle;
    g_DataStack.push_back(state);
    g_GlslVersionString = { '\0' };
    g_FontTexture = 0;
    g_ShaderHandle = 0;
    g_VertHandle = 0;
    g_FragHandle = 0;
    g_AttribLocationTex = 0;
    g_AttribLocationProjMtx = 0;
    g_AttribLocationPosition = 0;
    g_AttribLocationUV = 0;
    g_AttribLocationColor = 0;
    g_VboHandle = 0;
    g_ElementsHandle = 0;
}

void     ImGui_ImplOpenGL3_PopState() {
    if (g_DataStack.empty()) {
        return;
    }
    const ImGui_ImplOpenGL3_Data & state = g_DataStack.back();
    g_GlslVersionString = state.g_GlslVersionString;
    g_FontTexture = state.g_FontTexture;
    g_ShaderHandle = state.g_ShaderHandle;
    g_VertHandle = state.g_VertHandle;
    g_FragHandle = state.g_FragHandle;
    g_AttribLocationTex = state.g_AttribLocationTex;
    g_AttribLocationProjMtx = state.g_AttribLocationProjMtx;
    g_AttribLocationPosition = state.g_AttribLocationPosition;
    g_AttribLocationUV = state.g_AttribLocationUV;
    g_AttribLocationColor = state.g_AttribLocationColor;
    g_VboHandle = state.g_VboHandle;
    g_ElementsHandle = state.g_ElementsHandle;
}

Which I then call in my init()/restore() functions as

void ImGuiMenu::init() {
  ImGui_ImplOpenGL3_PushState();
  ImGui_ImplGlfw_PushState();
  context_ = ImGui::CreateContext();
  ImGui::SetCurrentContext(context_);
  ImGui_ImplGlfw_InitForOpenGL(viewer->window, false);
  ImGui_ImplOpenGL3_Init("#version 150");
  reload_font();
}

void ImGuiMenu::restore() {
  ImGui::SetCurrentContext(context_);
  ImGui_ImplOpenGL3_PopState();
  ImGui_ImplGlfw_PopState();
}

And it works great, without exception hacks or anything! Idk if you'd accept a PR for that kind of stuff though, so let me know.

@ocornut
Copy link
Owner

ocornut commented Aug 5, 2018

So probably it is due to the ImGui draw list referring to OpenGL objects that were destroyed during the initialization of the child viewer.

OK I understand the issue now: in this back-end we're using raw GLuint texture identifier for ImTextureId and that value is stored in the ImDrawList contents. If your GPU driver doesn't give you back the same texture id on a glDeleteTextures/glGenTextures sequence then you are toast (of course OpenGL doesn't guarantee that at all).

Understanding this, we can devise a solution.

One hacky workaround would be: to record Fonts->TexID before calling ImGui_ImplOpenGL3_Shutdown(), then at the end of the frame iterate all the ImDrawList in every ImDrawCmd and replace the old TexId value with the new value of Fonts->TexID before calling the renderer. Bit silly but it'll work!

There are probably other solution you can find based on the above understand of what's going wrong.

I'd prefer to avoid a sort of PR in imgui_imp_opengl3.cpp. From my point of view, every new features like that I have to maintain forever and I am already largely drained and overwhelmed with the maintenance of this examples/ folder. So any solution to not make them more featured than they already are is better for me. Considering you are doing something extremely unusual there, I think it would fail in your camp to maintain an OpenGL3 renderer (it's little amount of code and it has its individual ChangeLog).

@ocornut
Copy link
Owner

ocornut commented Aug 5, 2018

One hacky workaround would be: to record Fonts->TexID before calling ImGui_ImplOpenGL3_Shutdown(), then at the end of the frame iterate all the ImDrawList in every ImDrawCmd and replace the old TexId value with the new value of Fonts->TexID before calling the renderer. Bit silly but it'll work!

Just came over a simpler workaround: just skip rendering for 1 frame after you exit the viewer loop.

EDIT
Another idea, if you only intend to use 1 level of nested viewer, is to instance imgui_impl_opengl3.cpp twice by including the .cpp file into a namespace. But skipping Render()+SwapBuffer once is simpler.

@ocornut
Copy link
Owner

ocornut commented Aug 6, 2018

is unfortunately not true =/

I don't know how/why the main instance's drawlist knows at all about the new texture ID and why this assert would trigger. We only fetch the texture atlas ID in ImGui::Begin():
window->DrawList->PushTextureID(g.Font->ContainerAtlas->TexID);
and ImGui::PushFont:
g.CurrentWindow->DrawList->PushTextureID(font->ContainerAtlas->TexID);

You'll have to debug this.

@jdumas
Copy link
Contributor Author

jdumas commented Aug 6, 2018

Well I think the reason here is pretty simple, it's because the subviewer is calling ImGui_ImplOpenGL3_Shutdown() to reset the global variables in imgui_impl_opengl3.cpp.

In the current implementation, ImGui_ImplOpenGL3_Shutdown() calls

void ImGui_ImplOpenGL3_DestroyFontsTexture()
{
    if (g_FontTexture)
    {
        ImGuiIO& io = ImGui::GetIO();
        glDeleteTextures(1, &g_FontTexture);
        io.Fonts->TexID = 0;
        g_FontTexture = 0;
    }
}

It looks like the drawlist doesn't know about the new texture ID yet, but the assert is triggered because the old one was destroyed and the TexID was set to 0.

@ocornut
Copy link
Owner

ocornut commented Aug 6, 2018

You may overwrite io.Fonts->TexID with the old texid for one frame then patch it afterward.
At this point every code path is going to be a small hackfest.

In the future I'd like to make it easier to override the texture binding function within an existing binding (from outside) so you could decide to have a special and non-changing identifier to signify "the font texture" with this scheme, but it's not possible at the moment.

Right I guess you'll need to choose your poison!

@jdumas
Copy link
Contributor Author

jdumas commented Aug 6, 2018

Ok I can get it to work by hacking around the TexID as you suggest! I'll take this since it doesn't require me to modify the implementation files, but still allows me to nest multiple viewers. I'll close this thread then, thanks for your help in figuring this out =)

@jdumas jdumas closed this as completed Aug 6, 2018
@ocornut
Copy link
Owner

ocornut commented Aug 6, 2018

Good to hear!
I will wrap the globals of some those impl_ file in a single structure at some point, will tag you when that happens.

@jdumas
Copy link
Contributor Author

jdumas commented Dec 4, 2018

Hi. Sorry to bring this issue back up, but I was wondering how you're handling multiple windows with the current GLFW bindings in imgui? It seems that GLFW sets a callback per window (glfwSetKeyCallback(window, key_callback);), but the backend implementation of imgui is still using a single static GLFWwindow *. How are the callbacks shared between the two windows?

@ocornut
Copy link
Owner

ocornut commented Dec 4, 2018

Check out the back-end in the viewport branch which handles multi-windows:
https://github.com/ocornut/imgui/blob/viewport/examples/imgui_impl_glfw.cpp

In each ImGuiViewport we store a ImGuiViewportDataGlfw* in the PlatformUserData field.

We install the callback on each new window, and at it matters for most of them when we receive them it doesn't matter for us which window we received them from. For example ImGui_ImplGlfw_KeyCallback, ImGui_ImplGlfw_MouseButtonCallback etc. they just pass the data to imgui and we have no use checking the window pointer for them.

EDIT

I will wrap the globals of some those impl_ file in a single structure at some point, will tag you when that happens.

Attached patch.zip not sure it is worth applying this and generally to all back-ends now.

@jdumas
Copy link
Contributor Author

jdumas commented Dec 4, 2018

Ah nice! So basically you are implementing your own version of glfwSetWindowUserPointer(), and let ImGui attach the GLFW callback itself when creating new windows. I need to think a bit but it should be possible to use this implementation directly when multiple windows are created outside of ImGui.

@jdumas
Copy link
Contributor Author

jdumas commented Dec 5, 2018

Argh, I tried to use the ./example_glfw_opengl3 on linux (arch) with a tiling windows manager (i3wm), but it seems it's not quite usable yet =(

@ocornut
Copy link
Owner

ocornut commented Dec 5, 2018 via email

@jdumas
Copy link
Contributor Author

jdumas commented Dec 5, 2018

I guess some of it has to do with how tiling windows managers work, so I'm not sure it there can be a workaround. The problems I've seen so far:

  • The menus start as separate windows, so they get tiles like this:
    2018-12-05-110318_1920x1080_scrot
    Seems that this could be fixed by setting the window hint GLFW_FLOATING?
  • Flickering when I move a menu that is in its own window.
  • Focus issues: when clicking on a viewport, my wm brings it to the front, so it hides any overlapping imgui menu that is in its own windows. I'm not sure if anything can be done about this one.

I'm also having keybinding issues when I put that into the libigl viewer (somehow the inputs are not recorded anymore). But that may be due to how I'm doing the setup on the libigl side (the viewer is installing its own callbacks).

@jdumas
Copy link
Contributor Author

jdumas commented Dec 6, 2018

Ok so back to the problem at hand (multiple/nested viewers with an ImGui menu), I'm still having troubles with the viewport branch when I hook it up to my viewer.

The problem is again the use of globals (g_Window) in the file impl_imgui_glfw.cpp. I'm wondering if the binding data can be stored as a pointer in the ImGui context/viewport data? I would love to get rid of all the globals in the official implementation, since it's really giving me trouble supporting multiple viewers (concurrent or nested).

@jdumas jdumas reopened this Dec 6, 2018
@jdumas
Copy link
Contributor Author

jdumas commented Dec 6, 2018

And just to not post an empty message, I've pushed the current state of my bindings (for the libigl viewer I'm working on) on a branch of my own fork. See in particular this file that hooks up our viewer to the OpenGL+GLFW bindings of ImGui, and this tutorial file that shows how to spawn a concurrent or nested viewer windows in this setting. In the current implementation, I am sharing the OpenGL context between windows as well as the ImGui texture atlas.

I know we've discussed this already but I would like to come up with a solution where I don't have to modify the upstream example bindings to achieve this behavior (e.g. something like storing the current binding data pointer in the ImGui context would be nice).

@jdumas
Copy link
Contributor Author

jdumas commented Dec 6, 2018

Ok how about this: without modifying existing ImGui code, how about applying the patch you previously posted (to wrap all globals in a single struct), and then have a ImGui_ImplGlfw_CreateBindingData(), ImGui_ImplGlfw_SetCurrentBindingData(), and ImGui_ImplGlfw_DestroyBindingData(), ImGui_ImplGlfw_GetCurrentBindingData(), and change g_BindingData to be a pointer instead. This would allow to update the binding data when hoping between two different windows. Kinda similar to how ImGuiContext is being handled, but for the binding globals. This should be fairly non-intrusive (just a few loc), and it wouldn't introduce any breaking change.

@ocornut
Copy link
Owner

ocornut commented Dec 6, 2018

The problem I have is that all those changes are adding confusion and cognitive load to what should be the simplest possible binding example provided to new users.

Perhaps adding the void* user pointers in ImGuiIO for use by the back-end would be a good idea.
However in my experience this also means that once we suggest that multiple instance of the bindings are possible, there are lots of subtle things to watch for and maintain and it gets more complex when we factor multi-viewports in the equation. The bindings will get more complex and error prone in dozens of way and that's both a cost for me (I am not eager to do that maintenance) and a cost for the new user
.
Realistically speaking if you have custom needs the burden of maintenance should fall on your side. Those folders are called examples/ for the reason that they are example and it is perfectly fine to maintain your own binding, dozens of game team are, if you look at what is in imgui_impl_glfw it is very little code (and it can be less if you remove the subtle features you don't need).

I'm not ruling out looking at this but I have hundreds of more important fishes to fry at the moment..

@jdumas
Copy link
Contributor Author

jdumas commented Dec 6, 2018

Yes, I agree that increasing the cognitive cost when you open the imgui_impl_glfw.h file is bad. In fact, if I could inject some code at the end of the imgui_impl_glfw.cpp and imgui_impl_opengl3.cpp to implement my own SetCurrentBindingData() that would be a way to achieve that without overloading the .h file. Maybe we could use a similar trick to the #ifndef GImGui in imgui.cpp to be able to inject a custom GetBindingData() function, that by default would just define a global g_BindingData and return a ref to it.

ocornut added a commit that referenced this issue Dec 20, 2018
…nguageUserData void* for storage use by back-ends. (#2004 + for cimgui)
@ocornut
Copy link
Owner

ocornut commented Dec 20, 2018

@jdumas I added io.BackendPlatformUserData and io.BackendRendererUserData, both void*, to eventually facilitate setting up this storage.

@jdumas
Copy link
Contributor Author

jdumas commented Dec 20, 2018

Thanks! I've been using the modified backend bindings for libigl for now since it was easier to set up. I'll try to get back to this whenever I have some free time =)

ocornut added a commit that referenced this issue Jun 29, 2021
…multi-contexts. (#586, #1851, #2004, #3012, #3934, #4141)

This is NOT enable multi-contexts for any backends
- in order to make this commit as harmless as possible, while containing all the cruft/renaming
-
ocornut added a commit that referenced this issue Jun 29, 2021
#1851, #2004, #3012, #3934, #4141)

I believe more renderer backends should work. GLFW/Win32/SDL/Vulkan probably have many issues.
@ocornut
Copy link
Owner

ocornut commented Jun 29, 2021

Hello,

@jdumas This should now be solved by 70c6038 + 4cec3a0.

I confirmed that both OpenGL3 backends worked my test bed.
Platform backends however are not confirmed to fully work but for your specific use case (nested, one blocking the other) it should work with no noticeable issue.

Details (posted in a few threads)

With 70c6038 i have rearranged all backends to pull their data from structures.
This commit is rather large and noisy so I tried to make it as harmless a possible and therefore the commit as-is tries to NOT change anything in the backends, which include NOT enabling support for multiple imgui-context.
It was then enabled with a small patch in each backend 4cec3a0

PS: Current change being generaly healthy I don't mind it but I'm not very excited with promoting and encouraging support of multiple-imgui-context-each-in-their-own-backend-context. I think that feature is likely to be incompatible with multi-viewports (single imgui context + multiple backend windows) and I think instead we should aim at reworking this to let user register any existing window.

@ocornut ocornut closed this as completed Jun 29, 2021
@jdumas
Copy link
Contributor Author

jdumas commented Jun 29, 2021

Thanks @ocornut, this looks great! I agree if multi-viewport support allows us to have multiple viewer windows running in nested mode maybe we can use that. It's been a while since I looked at this feature though, but I'll let you know once I get back to it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants