From 46cd75cff48037f662b2a533cc9a8e7201ca4b78 Mon Sep 17 00:00:00 2001
From: Dan Paulat <dpaulat@gmail.com>
Date: Tue, 13 May 2025 22:43:56 -0500
Subject: [PATCH] Fix GlContext destruction of OpenGLFunctions causing crash

---
 scwx-qt/source/scwx/qt/gl/gl_context.cpp    | 57 ++++++++++++---------
 scwx-qt/source/scwx/qt/main/main_window.cpp |  6 ++-
 2 files changed, 36 insertions(+), 27 deletions(-)

diff --git a/scwx-qt/source/scwx/qt/gl/gl_context.cpp b/scwx-qt/source/scwx/qt/gl/gl_context.cpp
index d927aef3..66296a70 100644
--- a/scwx-qt/source/scwx/qt/gl/gl_context.cpp
+++ b/scwx-qt/source/scwx/qt/gl/gl_context.cpp
@@ -16,50 +16,48 @@ static const std::string logPrefix_ = "scwx::qt::gl::gl_context";
 class GlContext::Impl
 {
 public:
-   explicit Impl() :
-       gl_ {},
-       shaderProgramMap_ {},
-       shaderProgramMutex_ {},
-       textureAtlas_ {GL_INVALID_INDEX},
-       textureMutex_ {}
-   {
-   }
-   ~Impl() {}
+   explicit Impl() = default;
+   ~Impl()         = default;
+
+   Impl(const Impl&)             = delete;
+   Impl& operator=(const Impl&)  = delete;
+   Impl(const Impl&&)            = delete;
+   Impl& operator=(const Impl&&) = delete;
 
    void InitializeGL();
 
    static std::size_t
    GetShaderKey(std::initializer_list<std::pair<GLenum, std::string>> shaders);
 
-   gl::OpenGLFunctions  gl_;
-   QOpenGLFunctions_3_0 gl30_;
+   gl::OpenGLFunctions*  gl_ {nullptr};
+   QOpenGLFunctions_3_0* gl30_ {nullptr};
 
    bool glInitialized_ {false};
 
    std::unordered_map<std::size_t, std::shared_ptr<gl::ShaderProgram>>
-              shaderProgramMap_;
-   std::mutex shaderProgramMutex_;
+              shaderProgramMap_ {};
+   std::mutex shaderProgramMutex_ {};
 
-   GLuint     textureAtlas_;
-   std::mutex textureMutex_;
+   GLuint     textureAtlas_ {GL_INVALID_INDEX};
+   std::mutex textureMutex_ {};
 
    std::uint64_t textureBufferCount_ {};
 };
 
 GlContext::GlContext() : p(std::make_unique<Impl>()) {}
-GlContext::~GlContext() = default;
+GlContext::~GlContext() {};
 
 GlContext::GlContext(GlContext&&) noexcept            = default;
 GlContext& GlContext::operator=(GlContext&&) noexcept = default;
 
 gl::OpenGLFunctions& GlContext::gl()
 {
-   return p->gl_;
+   return *p->gl_;
 }
 
 QOpenGLFunctions_3_0& GlContext::gl30()
 {
-   return p->gl30_;
+   return *p->gl30_;
 }
 
 std::uint64_t GlContext::texture_buffer_count() const
@@ -74,10 +72,19 @@ void GlContext::Impl::InitializeGL()
       return;
    }
 
-   gl_.initializeOpenGLFunctions();
-   gl30_.initializeOpenGLFunctions();
+   // QOpenGLFunctions objects will not be freed. Since "destruction" takes
+   // place at the end of program execution, it is OK to intentionally leak
+   // these.
 
-   gl_.glGenTextures(1, &textureAtlas_);
+   // NOLINTBEGIN(cppcoreguidelines-owning-memory)
+   gl_   = new gl::OpenGLFunctions();
+   gl30_ = new QOpenGLFunctions_3_0();
+   // NOLINTEND(cppcoreguidelines-owning-memory)
+
+   gl_->initializeOpenGLFunctions();
+   gl30_->initializeOpenGLFunctions();
+
+   gl_->glGenTextures(1, &textureAtlas_);
 
    glInitialized_ = true;
 }
@@ -102,7 +109,7 @@ std::shared_ptr<gl::ShaderProgram> GlContext::GetShaderProgram(
 
    if (it == p->shaderProgramMap_.end())
    {
-      shaderProgram = std::make_shared<gl::ShaderProgram>(p->gl_);
+      shaderProgram = std::make_shared<gl::ShaderProgram>(*p->gl_);
       shaderProgram->Load(shaders);
       p->shaderProgramMap_[key] = shaderProgram;
    }
@@ -125,7 +132,7 @@ GLuint GlContext::GetTextureAtlas()
    if (p->textureBufferCount_ != textureAtlas.BuildCount())
    {
       p->textureBufferCount_ = textureAtlas.BuildCount();
-      textureAtlas.BufferAtlas(p->gl_, p->textureAtlas_);
+      textureAtlas.BufferAtlas(*p->gl_, p->textureAtlas_);
    }
 
    return p->textureAtlas_;
@@ -140,8 +147,8 @@ void GlContext::StartFrame()
 {
    auto& gl = p->gl_;
 
-   gl.glClearColor(0.0f, 0.0f, 0.0f, 1.0f);
-   gl.glClear(GL_COLOR_BUFFER_BIT);
+   gl->glClearColor(0.0f, 0.0f, 0.0f, 1.0f);
+   gl->glClear(GL_COLOR_BUFFER_BIT);
 }
 
 std::size_t GlContext::Impl::GetShaderKey(
diff --git a/scwx-qt/source/scwx/qt/main/main_window.cpp b/scwx-qt/source/scwx/qt/main/main_window.cpp
index d85a5a84..dec713ed 100644
--- a/scwx-qt/source/scwx/qt/main/main_window.cpp
+++ b/scwx-qt/source/scwx/qt/main/main_window.cpp
@@ -190,6 +190,8 @@ public:
    map::MapProvider    mapProvider_;
    map::MapWidget*     activeMap_;
 
+   std::shared_ptr<gl::GlContext> glContext_ {nullptr};
+
    ui::CollapsibleGroup*     mapSettingsGroup_;
    ui::CollapsibleGroup*     level2ProductsGroup_;
    ui::CollapsibleGroup*     level2SettingsGroup_;
@@ -777,7 +779,7 @@ void MainWindowImpl::ConfigureMapLayout()
       }
    };
 
-   auto glContext = std::make_shared<gl::GlContext>();
+   glContext_ = std::make_shared<gl::GlContext>();
 
    for (int64_t y = 0; y < gridHeight; y++)
    {
@@ -790,7 +792,7 @@ void MainWindowImpl::ConfigureMapLayout()
          {
             // NOLINTNEXTLINE(cppcoreguidelines-owning-memory): Owned by parent
             maps_[mapIndex] =
-               new map::MapWidget(mapIndex, settings_, glContext);
+               new map::MapWidget(mapIndex, settings_, glContext_);
          }
 
          hs->addWidget(maps_[mapIndex]);