imgui 🚀 - Nesting multiple imgui contexts (glfw+opengl3)

Sharing OpenGL context is really easy and recommended, it’s only a single extra parameter to glfwCreateWindow().

Have you looked at what the Viewport branch does? It is meant to facilitate this kind of thing while also reusing the same imgui context.

ocornut on 4 Aug 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?

jdumas on 5 Aug 2018

None of those assumptions are valid. You don’t need to enter a separate newframe/render loop. Build and run the examples applications in the Viewport branch and find out what happens when you drag windows outside. You can essentially seamlessly create imgui windows within multiple os windows.

Note that GLFW backend has a few focus-related issues that are not solved on Linux yet, waiting for changes to be made on GLFW.

ocornut on 5 Aug 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.

jdumas on 5 Aug 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.

ocornut on 5 Aug 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 =)

jdumas on 5 Aug 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 on 5 Aug 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).

jdumas on 5 Aug 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.

ocornut on 5 Aug 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.

jdumas on 5 Aug 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().

ocornut on 5 Aug 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.

jdumas on 5 Aug 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.).

ocornut on 5 Aug 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).

jdumas on 5 Aug 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).

ocornut on 5 Aug 2018

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

jdumas on 5 Aug 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 on 5 Aug 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 on 5 Aug 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?

jdumas on 5 Aug 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.

ocornut on 5 Aug 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.

jdumas on 5 Aug 2018

OK.

The saner solution we devised above should work, so the OpenGL crash you mention in:
https://github.com/ocornut/imgui/issues/2004#issuecomment-410541291

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?

ocornut on 5 Aug 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.

jdumas on 6 Aug 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 on 6 Aug 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 on 6 Aug 2018

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).

I think this might be a side effect of the fact that my viewer also allocates textures and VBO. So when the subviewer is closed, but parent.restore() is called before the subviewer's OpenGL objects are destroyed, then we might not get the same slots exactly. I'll take a look at the hacky workaround you suggest, but otherwise I may stick with my "state stack" solution.

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

Well this is what I was trying to achieve with my "exception hack".
I'm also not too keen on limiting the number of nested viewers to two. In practice we likely never need more, but that's also a bit "hackish".

I admit that what I'm trying to achieve here may be a bit unconventional. However, I think I have a perfectly valid use-case for doing so =)

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.

I perfectly understand, no worries. Would you be open to a PR that puts the global variables into a single global/copyable struct though? I.e replace

// OpenGL Data
static char         g_GlslVersionString[32] = "";
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;

with

// OpenGL Data
struct ImGui_ImplOpenGL3_Data {
    char         GlslVersionString[32] = "";
    GLuint       FontTexture = 0;
    GLuint       ShaderHandle = 0, VertHandle = 0, FragHandle = 0;
    int          AttribLocationTex = 0, AttribLocationProjMtx = 0;
    int          AttribLocationPosition = 0, AttribLocationUV = 0, AttribLocationColor = 0;
    unsigned int VboHandle = 0, ElementsHandle = 0;
};
static ImGui_ImplOpenGL3_Data g_State;

It's technically not a new feature, but it would make my implementing of the stack on top of it more simple (and trivial to grow if new global variables are added).

jdumas on 6 Aug 2018

Well this is what I was trying to achieve with my "exception hack".

But you don't need that complexity, nor you need to worry about ImGui:: calls further down the frame. You only need to avoid calling ImGui_ImplOpenGL3_RenderDrawData() for one frame, as the ImDrawData for the frame will contains references two different texture id, one of which is invalid at this point.

I don't think VBO have anything to do it as you are destroying/recreating the impl_opengl3 data just fine.

Would you be open to a PR that puts the global variables into a single global/copyable struct though? I.e replace

Yes-ish. The problem I have here is that the code as-is (as you quoted it) is not valid C++03 anymore, we need a constructor and initialize all those members individually.

ocornut on 6 Aug 2018

But you don't need that complexity, nor you need to worry about ImGui:: calls further down the frame. You only need to avoid calling ImGui_ImplOpenGL3_RenderDrawData() for one frame, as the ImDrawData for the frame will contains references two different texture id, one of which is invalid at this point.

I know, but my options are either 1) set a flag in the viewer skip_one_frame = true, 2) return a boolean from the draw_menu() function to specify whether the frame should be skipped or not, or 3) use a continuation function in an exception. 1) and 2) would work well if the code spawning the viewer is called inside the draw_menu() function. But let's say I call another function further down the line, which has type void f() and is not inside the viewer, then I'd need to either 1) pass information about the parent viewer to set the flag skip_one_frame, or 2) modify the return type of the surrounding function f() to return a boolean (which might be problematic if f() was already returning something else). Both options are not great for debugging purpose, where you don't want to modify your whole pipeline just to spawn a subviewer. The C++ exception trick allows to "shortcut" this and forward a continuation function to the surrounding ImGui code, but it still has limitations (need to end any open ImGui::Begin(), etc.). I don't have any satisfactory solution to this problem at the moment.

EDIT: Thinking about it, I guess I could keep also global reference to the currently active viewer, and use that to set the flag skip_this_frame from anywhere in the code... That'd probably be simpler. It still puts the burden on the user to think about this step, unless I do that during the initialization of the subviewer...

Yes-ish. The problem I have here is that the code as-is (as you quoted it) is not valid C++03 anymore, we need a constructor and initialize all those members individually.

Ah right, in-class initialization of member variable is C++11 I think? I'm wondering if default initialization wouldn't guarantee that everyone is initialized to 0 though (but I think not, and it's better not to take any chances).

jdumas on 6 Aug 2018

Ok the last idea (with keeping a pointer to the currently active menu, to set the flag skip_this_frame = true;) almost works, but now I'm getting a different error due to a ImGui::Button() being called after the subviewer has closed.

Here is the code to draw the 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();
    }
    if (ImGui::Button("Deferred Viewer")) {
      launch_nested_viewer = true;
    }

And the callstack with the error being generated:

106_ViewerMenu_bin: /home/jdumas/workspace/github/libigl/external/imgui/imgui/imgui_draw.cpp:1144: void ImDrawList::AddText(const ImFont*, float, const ImVec2&, ImU32, const char*, const char*, float, const ImVec4*): Assertion `font->ContainerAtlas->TexID == _TextureIdStack.back()' failed.

Thread 1 "106_ViewerMenu_" received signal SIGABRT, Aborted.
0x00007ffff652886b in raise () from /usr/lib/libc.so.6
(gdb) bt
#0  0x00007ffff652886b in raise () from /usr/lib/libc.so.6
#1  0x00007ffff651340e in abort () from /usr/lib/libc.so.6
#2  0x00007ffff65132e0 in __assert_fail_base.cold.0 () from /usr/lib/libc.so.6
#3  0x00007ffff6521112 in __assert_fail () from /usr/lib/libc.so.6
#4  0x0000555555e894a8 in ImDrawList::AddText (this=0x555556666ef0, font=0x555556685360, font_size=21.666666, pos=..., col=4294967295, text_begin=0x555555eb5da6 "Deferred Viewer", text_end=0x555555eb5db5 "", 
    wrap_width=0, cpu_fine_clip_rect=0x0) at /home/jdumas/workspace/github/libigl/external/imgui/imgui/imgui_draw.cpp:1144
#5  0x0000555555e36d44 in ImGui::RenderTextClipped (pos_min=..., pos_max=..., text=0x555555eb5da6 "Deferred Viewer", text_end=0x0, text_size_if_known=0x7fffffffd318, align=..., clip_rect=0x7fffffffd340)
    at /home/jdumas/workspace/github/libigl/external/imgui/imgui/imgui.cpp:4749
#6  0x0000555555e45ca6 in ImGui::ButtonEx (label=0x555555eb5da6 "Deferred Viewer", size_arg=..., flags=0) at /home/jdumas/workspace/github/libigl/external/imgui/imgui/imgui.cpp:8296
#7  0x0000555555e45cf0 in ImGui::Button (label=0x555555eb5da6 "Deferred Viewer", size_arg=...) at /home/jdumas/workspace/github/libigl/external/imgui/imgui/imgui.cpp:8307
#8  0x0000555555bd7051 in <lambda()>::operator()(void) const (__closure=0x5555563475c0) at /home/jdumas/workspace/github/libigl/tutorial/106_ViewerMenu/main.cpp:110
#9  0x0000555555bd7a99 in std::_Function_handler<void(), main(int, char**)::<lambda()> >::_M_invoke(const std::_Any_data &) (__functor=...) at /usr/include/c++/8.1.1/bits/std_function.h:297
#10 0x0000555555c245a2 in std::function<void ()>::operator()() const (this=0x7fffffffdad8) at /usr/include/c++/8.1.1/bits/std_function.h:687
#11 0x0000555555c22fad in igl::opengl::glfw::imgui::ImGuiMenu::draw_menu (this=0x7fffffffda50) at /home/jdumas/workspace/github/libigl/include/igl/opengl/glfw/imgui/ImGuiMenu.cpp:199
#12 0x0000555555c22d3d in igl::opengl::glfw::imgui::ImGuiMenu::post_draw (this=0x7fffffffda50) at /home/jdumas/workspace/github/libigl/include/igl/opengl/glfw/imgui/ImGuiMenu.cpp:122
#13 0x0000555555c0b7ab in igl::opengl::glfw::Viewer::draw (this=0x7fffffffdb00) at /home/jdumas/workspace/github/libigl/include/igl/opengl/glfw/Viewer.cpp:870
#14 0x0000555555c086c9 in igl::opengl::glfw::Viewer::launch_rendering (this=0x7fffffffdb00, loop=true) at /home/jdumas/workspace/github/libigl/include/igl/opengl/glfw/Viewer.cpp:225
#15 0x0000555555c08281 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
#16 0x0000555555bd7481 in main (argc=1, argv=0x7fffffffdfc8) at /home/jdumas/workspace/github/libigl/tutorial/106_ViewerMenu/main.cpp:131

So it looks like your statement

nor you need to worry about ImGui:: calls further down the frame. You only need to avoid calling ImGui_ImplOpenGL3_RenderDrawData() for one frame

is unfortunately not true =/

(Note that if I remove the button "Deferred Viewer" in my example above the nested viewer works great, and without modification to the glfw+opengl3 implementation!)

jdumas on 6 Aug 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.

ocornut on 6 Aug 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.

jdumas on 6 Aug 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!

ocornut on 6 Aug 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 on 6 Aug 2018

👍1

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.

ocornut on 6 Aug 2018

👍1

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?

jdumas on 4 Dec 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.

ocornut on 4 Dec 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 on 4 Dec 2018

👍1

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 =(

jdumas on 5 Dec 2018

Could you clarify why?

ocornut on 5 Dec 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:

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 on 5 Dec 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 on 6 Dec 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 on 6 Dec 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.

jdumas on 6 Dec 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..

ocornut on 6 Dec 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.

jdumas on 7 Dec 2018

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

ocornut on 20 Dec 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 =)

jdumas on 20 Dec 2018

Imgui: Nesting multiple imgui contexts (glfw+opengl3)

All 47 comments

Related issues