GLFW3 & multiple OS windows

[meshula]

Hi there, I'm using this for docking -

https://github.com/edin-p/ImGuiDock

but it doesn't work with GLFW3 as written. In order to "tear off" a panel to become a free floating OS window, it's necessary to adapt imgui_impl_glfw_gl3.cpp to be smart enough to work with multiple GL contexts.

Unfortunately imgui_impl_glfw_gl3.cpp uses a global vertex array object, which can't be shared between multiple GL contexts because it isn't shareable GL data, like say a texture.

As a result, in order for ImGui to work with multiple OS windows in the same application, a patch like the following is necessary. Note that I realize std::map is not idiomatic for ImGui, I'm using it here as a quick implementation to illustrate a non-disruptive fix for the problem.

diff --git a/src/interface/imgui/imgui_impl_glfw_gl3.cpp b/src/interface/imgui/imgui_impl_glfw_gl3.cpp
index c775f62..21649c2 100644
--- a/src/interface/imgui/imgui_impl_glfw_gl3.cpp
+++ b/src/interface/imgui/imgui_impl_glfw_gl3.cpp
@@ -10,6 +10,7 @@

 #include <imgui.h>
 #include "imgui_impl_glfw_gl3.h"
+#include <map>

 // GL3W/GLFW
 #include <GLFW/glfw3.h>
@@ -29,7 +30,9 @@ static GLuint       g_FontTexture = 0;
 static int          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_VaoHandle = 0, g_ElementsHandle = 0;
+static unsigned int g_VboHandle = 0, g_ElementsHandle = 0;
+static std::map<ImGuiContext*, unsigned int> g_VaoHandles;

 // This is the main rendering function that you have to implement and provide to ImGui (via setting up 'RenderDrawListsFn' in the ImGuiIO structure)
 // If text or lines are blurry when integrating ImGui in your engine:
@@ -81,9 +84,32 @@ void ImGui_ImplGlfwGL3_RenderDrawLists(ImDrawData* draw_data)
         {-1.0f,                  1.0f,                   0.0f, 1.0f },
     };
     glUseProgram(g_ShaderHandle);
-    glUniform1i(g_AttribLocationTex, 0);
-    glUniformMatrix4fv(g_AttribLocationProjMtx, 1, GL_FALSE, &ortho_projection[0][0]);
-    glBindVertexArray(g_VaoHandle);
+   glUniform1i(g_AttribLocationTex, 0);
+   glUniformMatrix4fv(g_AttribLocationProjMtx, 1, GL_FALSE, &ortho_projection[0][0]);
+
+   ImGuiContext* imgui_ctx = ImGui::GetCurrentContext();
+   auto vao = g_VaoHandles.find(imgui_ctx);
+   if (vao == g_VaoHandles.end())
+   {
+       unsigned int new_vao;
+       glGenVertexArrays(1, &new_vao);
+       g_VaoHandles[imgui_ctx] = new_vao;
+       glBindVertexArray(new_vao);
+       glBindBuffer(GL_ARRAY_BUFFER, g_VboHandle);
+       glEnableVertexAttribArray(g_AttribLocationPosition);
+       glEnableVertexAttribArray(g_AttribLocationUV);
+       glEnableVertexAttribArray(g_AttribLocationColor);
+
+#define OFFSETOF(TYPE, ELEMENT) ((size_t)&(((TYPE *)0)->ELEMENT))
+       glVertexAttribPointer(g_AttribLocationPosition, 2, GL_FLOAT, GL_FALSE, sizeof(ImDrawVert), (GLvoid*)OFFSETOF(ImDrawVert, pos));
+       glVertexAttribPointer(g_AttribLocationUV, 2, GL_FLOAT, GL_FALSE, sizeof(ImDrawVert), (GLvoid*)OFFSETOF(ImDrawVert, uv));
+       glVertexAttribPointer(g_AttribLocationColor, 4, GL_UNSIGNED_BYTE, GL_TRUE, sizeof(ImDrawVert), (GLvoid*)OFFSETOF(ImDrawVert, col));
+#undef OFFSETOF
+   }
+   else
+   {
+       glBindVertexArray(vao->second);
+   }

     for (int n = 0; n < draw_data->CmdListsCount; n++)
     {
@@ -253,8 +279,6 @@ bool ImGui_ImplGlfwGL3_CreateDeviceObjects()
     glGenBuffers(1, &g_VboHandle);
     glGenBuffers(1, &g_ElementsHandle);

-    glGenVertexArrays(1, &g_VaoHandle);
-    glBindVertexArray(g_VaoHandle);
     glBindBuffer(GL_ARRAY_BUFFER, g_VboHandle);
     glEnableVertexAttribArray(g_AttribLocationPosition);
     glEnableVertexAttribArray(g_AttribLocationUV);
@@ -278,10 +302,16 @@ bool ImGui_ImplGlfwGL3_CreateDeviceObjects()

 void    ImGui_ImplGlfwGL3_InvalidateDeviceObjects()
 {
-    if (g_VaoHandle) glDeleteVertexArrays(1, &g_VaoHandle);
-    if (g_VboHandle) glDeleteBuffers(1, &g_VboHandle);
+   for (auto i : g_VaoHandles)
+   {
+       glDeleteVertexArrays(1, &i.second);
+   }
+   g_VaoHandles.clear();
+
+   if (g_VboHandle) glDeleteBuffers(1, &g_VboHandle);
     if (g_ElementsHandle) glDeleteBuffers(1, &g_ElementsHandle);
-    g_VaoHandle = g_VboHandle = g_ElementsHandle = 0;
+    g_VboHandle = g_ElementsHandle = 0;

     if (g_ShaderHandle && g_VertHandle) glDetachShader(g_ShaderHandle, g_VertHandle);
     if (g_VertHandle) glDeleteShader(g_VertHandle);

[ocornut]

Thanks Nick! I have just stumbled on the same problem today and after poking around remembered your post. Currently trying to improve the situation toward moving the main example backends to support multiple windows better.

The only issue I can find in this code is the fact that it maps 1 imgui context to 1 opengl context, whereas right now what I am doing will break exactly this assumption. I will rework the examples accordingly before patching this in.

[meshula]

Yes, I was able to enforce 1:1 at a higher level in my app where I manage GL context sharing. edin-p's docking code does a bunch of work to create and manage multiple ImGui contexts through copying around a bunch of state. I look forward to a cleaner solution!

[ocornut]

You can see how it's doing there: https://github.com/ocornut/imgui/issues/351#issuecomment-354348675 Basically I'm working on a system where 1 imgui contexts will be able to output to multiple virtual viewports, and it'll all dynamic.

For the GLFW/GL VAO part, the problem is that I cannot use the GLFWWindow* as a key because that function has no knowledge of a window being destroyed. For now to keep things simple I am recreating the VAO every frame, which I suspect is totally fine. As I will refactor the code/api in examples/ to support inputs/rendering for multiple windows I'll see if I can provide enough data to the render function to use a cache that we can clean.

(One problem I will be facing ahead is making those main backends (DX11, GLFW etc.) support multiple windows while still making them simple and clear enough so that a first time user who want to use their engine won't be overwhelmed by the too many details.)

[meshula]

Yes, my code leaks the VAO in the case a window is closed.

[ocornut]

The leak isn’t too bad but if you destroy and recreate an imgui context (along with associated gl context) and end up with same pointer the rendering will fail.

(It’s an easy problem to solve but I want to reorganize those functions a bit and i’ll only know the proper design once I have all features in place.)

[meshula]

At this point, I have backed my docking code out to a simple splitter system. I still have multiple GL contexts to manage because I do offscreen rendering and what not, but I don't have multiple glfw windows. I'll re-migrate to docks when official support lands :)

[ocornut]

Merged change where the VAO is created in the draw function, so that fixes the issue described here. Hopefully it's not a perf issue.

[ocornut]

@meshula Saw you posting a message and deleting it. Thanks - they was indeed a leak in my commit earlier today. Handling multiple branches (since I'm refactoring the example) made me make those mistakes :( Should be fixed now. I couldn't see a perf difference but it could depend on drivers.

OpenGL context is dissociated from ImGuiContext because the code is heading toward a single imgui context managing multiple viewport that can be rendered with multiple graphics context. Eventually the target is to make the example architecture store per-viewport information, this may be coming next month hopefully.

[meshula]

I didn't mean to delete, must have fumble-clicked :)

Thanks for addressing my questions!