imgui icon indicating copy to clipboard operation
imgui copied to clipboard

Viewports for win32+opengl backend (fix #2600)

Open theOtherMichael opened this issue 2 years ago • 1 comments

Adds support for Win32 + OpenGL backends. Previously, this combination didn't work correctly because spawned windows weren't given rendering contexts. This PR makes it so OpenGL contexts are created and destroyed correctly for each new window. Additionally, it links the contexts to the main one via wglShareLists().

The only API change is the addition of an optional parameter, hglrc, in ImGui_ImplWin32_Init(). Passing in the main window's render context handle here will engage the new code. If only the first parameter is passed, a non-OpenGL API will be inferred. This makes it so existing projects using the Win32 backend will be able to merge this change without issue.

When an OpenGL render context is specified, four WGL functions, wglCreateContext(), wglDeleteContext(), wglMakeCurrent(), and wglShareLists() will be loaded dynamically to enable the context management. The OpenGL library will not be loaded if no hglrc is specified.

theOtherMichael avatar Apr 06 '22 06:04 theOtherMichael

I've just committed a change that should resolve all of these issues. We no longer maintain the device context when it's not in use. To make this possible, I did have to add some changes to imgui.h, but I don't believe the change is invasive.

theOtherMichael avatar Apr 13 '22 19:04 theOtherMichael

Hello,

There is a Platform_RenderWindow hook which could be used instead of Platform_PushOglContext/Platform_PopOglContext:

The examples don't expect current context to be unchanged after rendering platform windows, see e.g. example_opengl3_glfw:

        // Update and Render additional Platform Windows
        // (Platform functions may change the current OpenGL context, so we save/restore it to make it easier to paste this code elsewhere.
        //  For this specific demo app we could also call glfwMakeContextCurrent(window) directly)
        if (io.ConfigFlags & ImGuiConfigFlags_ViewportsEnable)
        {
            GLFWwindow* backup_current_context = glfwGetCurrentContext();
            ImGui::UpdatePlatformWindows();
            ImGui::RenderPlatformWindowsDefault();
            glfwMakeContextCurrent(backup_current_context);
        }

Therefore I don't think a "pop" is needed here and it looks like this could be simplified.

While I have no timeframe for merging this, some other feedback:

  • Make the change to use Platform_RenderWindow(). Ditch the two new hooks and "stashedXXX" variables.
  • Adding an example_win32_opengl3/ would be good.
  • Minor corrections: OglDLL -> OpenGlDLL. Both init function routed through a static/internal ImGui_ImplWin32_InitEx() so it's not confusing for users to find OpenGL in their callstack. Combine all asserts after DLL loading into 1. Refer to GetProcAddress as ::GetProcAddress for consistency. Keep ImGui_ImplWin32_ViewportData() a one-liner.

Thanks!

ocornut avatar Jan 03 '23 18:01 ocornut

Could you post your example app for this? I'm trying to use #3218 one now but unsure how GL Context version affects code there.

ocornut avatar Apr 07 '23 16:04 ocornut

I rebased, added a (maybe) working example into: https://github.com/ocornut/imgui/commits/features/win32_opengl_example And now will try to dig into #5170 #3218 #2772 to find the best way to make this not affect Win32 backend so much.

The whole topic has been extremely confusing, even more so as if we 8 open issues/PR and none of them goes 100%.

#3218 seems particularly elegant, do you know if there's something that it doesn't do? I feel the change in #3218 could be reworded as a ImGui_ImplWin32_InitForOpenGL(). One thing I'm not sure about #3218 is it use the "GL2" backend and not the GL3 one.

ocornut avatar Apr 07 '23 17:04 ocornut

Closed/solved, see https://github.com/ocornut/imgui/pull/3218#issuecomment-1514886161

ocornut avatar Apr 19 '23 14:04 ocornut