From 70f28694e44e8c04b7e3773ba4c98dc579370611 Mon Sep 17 00:00:00 2001 From: Nicholas Sharp Date: Sat, 4 Jan 2025 12:54:09 -0800 Subject: [PATCH] track buffers which must be preserved for imgui render (#319) * track buffers which must be preserved for imgui render * fix comment typo --- include/polyscope/render/engine.h | 17 +++++++++++++++-- src/color_image_quantity.cpp | 1 + src/histogram.cpp | 1 + src/render/engine.cpp | 6 ++++++ src/render/mock_opengl/mock_gl_engine.cpp | 5 ++++- src/render/opengl/gl_engine_egl.cpp | 5 ++++- src/render/opengl/gl_engine_glfw.cpp | 1 + src/scalar_image_quantity.cpp | 1 + 8 files changed, 33 insertions(+), 4 deletions(-) diff --git a/include/polyscope/render/engine.h b/include/polyscope/render/engine.h index 0caf7754..c37d3f86 100644 --- a/include/polyscope/render/engine.h +++ b/include/polyscope/render/engine.h @@ -445,9 +445,9 @@ class Engine { virtual void shutdown() {}; virtual void checkError(bool fatal = false) = 0; void buildEngineGui(); - + // 'headless' means there is no physical display to actually render to, e.g. when running on a remote server - virtual bool isHeadless() { return false; } + virtual bool isHeadless() { return false; } virtual void clearDisplay(); virtual void bindDisplay(); @@ -519,6 +519,10 @@ class Engine { void setImGuiStyle(); ImFontAtlas* getImGuiGlobalFontAtlas(); + + // Display an ImGui window showing a texture + // WARNING: you must ensure that the texture buffer pointer stays valid until after the ImGui frame is rendered, which + // is not until the end of a main loop iteration. virtual void showTextureInImGuiWindow(std::string windowName, TextureBuffer* buffer); @@ -630,6 +634,11 @@ class Engine { ImFont* monoFont = nullptr; FrameBuffer* currRenderFramebuffer = nullptr; + // Manage some resources that we need to preserve because ImGui will use them to render at the end of the frame + // This matters if we delete something mid-frame but have already passed a pointer to a texture for imgui to render, + // which happens at the end of the frame. + void preserveResourceUntilImguiFrameCompletes(std::shared_ptr texture); + protected: // TODO Manage a cache of compiled shaders? @@ -664,6 +673,10 @@ class Engine { std::vector defaultRules_sceneObject{"GLSL_VERSION", "GLOBAL_FRAGMENT_FILTER"}; std::vector defaultRules_pick{"GLSL_VERSION", "GLOBAL_FRAGMENT_FILTER", "SHADE_COLOR", "LIGHT_PASSTHRU"}; std::vector defaultRules_process{"GLSL_VERSION"}; + + // Lists of points to support preserving resources until the end of an ImGUI frame (see note above) + void clearResourcesPreservedForImguiFrame(); + std::vector> resourcesPreservedForImGuiFrame; }; diff --git a/src/color_image_quantity.cpp b/src/color_image_quantity.cpp index d66b0f76..99089473 100644 --- a/src/color_image_quantity.cpp +++ b/src/color_image_quantity.cpp @@ -111,6 +111,7 @@ void ColorImageQuantity::showInImGuiWindow() { } else if (imageOrigin == ImageOrigin::UpperLeft) { ImGui::Image(colors.getRenderTextureBuffer()->getNativeHandle(), ImVec2(w, h)); } + render::engine->preserveResourceUntilImguiFrameCompletes(colors.getRenderTextureBuffer()); ImGui::End(); } diff --git a/src/histogram.cpp b/src/histogram.cpp index b8d898ef..42e2bdb1 100644 --- a/src/histogram.cpp +++ b/src/histogram.cpp @@ -202,6 +202,7 @@ void Histogram::buildUI(float width) { // Render image ImGui::Image(texture->getNativeHandle(), ImVec2(w, h), ImVec2(0, 1), ImVec2(1, 0)); + render::engine->preserveResourceUntilImguiFrameCompletes(texture); // Helpful info for drawing annotations below ImU32 annoColor = ImGui::ColorConvertFloat4ToU32(ImVec4(254 / 255., 221 / 255., 66 / 255., 1.0)); diff --git a/src/render/engine.cpp b/src/render/engine.cpp index e23998b7..819eb959 100644 --- a/src/render/engine.cpp +++ b/src/render/engine.cpp @@ -1183,5 +1183,11 @@ void Engine::showTextureInImGuiWindow(std::string windowName, TextureBuffer* buf ImFontAtlas* Engine::getImGuiGlobalFontAtlas() { return globalFontAtlas; } +void Engine::preserveResourceUntilImguiFrameCompletes(std::shared_ptr texture) { + resourcesPreservedForImGuiFrame.push_back(texture); +} + +void Engine::clearResourcesPreservedForImguiFrame() { resourcesPreservedForImGuiFrame.clear(); } + } // namespace render } // namespace polyscope diff --git a/src/render/mock_opengl/mock_gl_engine.cpp b/src/render/mock_opengl/mock_gl_engine.cpp index 7170c579..5a52407b 100644 --- a/src/render/mock_opengl/mock_gl_engine.cpp +++ b/src/render/mock_opengl/mock_gl_engine.cpp @@ -1659,7 +1659,10 @@ void MockGLEngine::ImGuiNewFrame() { ImGui::NewFrame(); } -void MockGLEngine::ImGuiRender() { ImGui::Render(); } +void MockGLEngine::ImGuiRender() { + ImGui::Render(); + clearResourcesPreservedForImguiFrame(); +} void MockGLEngine::setDepthMode(DepthMode newMode) {} diff --git a/src/render/opengl/gl_engine_egl.cpp b/src/render/opengl/gl_engine_egl.cpp index 4e57a9d1..0f95bbcb 100644 --- a/src/render/opengl/gl_engine_egl.cpp +++ b/src/render/opengl/gl_engine_egl.cpp @@ -396,7 +396,10 @@ void GLEngineEGL::ImGuiNewFrame() { ImGui::NewFrame(); } -void GLEngineEGL::ImGuiRender() { ImGui::Render(); } +void GLEngineEGL::ImGuiRender() { + ImGui::Render(); + clearResourcesPreservedForImguiFrame(); +} void GLEngineEGL::swapDisplayBuffers() { diff --git a/src/render/opengl/gl_engine_glfw.cpp b/src/render/opengl/gl_engine_glfw.cpp index bbcff2a2..d483c238 100644 --- a/src/render/opengl/gl_engine_glfw.cpp +++ b/src/render/opengl/gl_engine_glfw.cpp @@ -140,6 +140,7 @@ void GLEngineGLFW::ImGuiNewFrame() { void GLEngineGLFW::ImGuiRender() { ImGui::Render(); ImGui_ImplOpenGL3_RenderDrawData(ImGui::GetDrawData()); + clearResourcesPreservedForImguiFrame(); } diff --git a/src/scalar_image_quantity.cpp b/src/scalar_image_quantity.cpp index ad9e0f4b..eb5641bf 100644 --- a/src/scalar_image_quantity.cpp +++ b/src/scalar_image_quantity.cpp @@ -115,6 +115,7 @@ void ScalarImageQuantity::showInImGuiWindow() { // here we always use the same ImVec2 UV coords below, because the texture order is always openGL convention after the // intermediate render pass ImGui::Image(textureIntermediateRendered->getNativeHandle(), ImVec2(w, h), ImVec2(0, 1), ImVec2(1, 0)); + render::engine->preserveResourceUntilImguiFrameCompletes(textureIntermediateRendered); ImGui::End();