fix GL context teardown regression from QOpenGLWidget to QOpenGLWindow

Commit aa07211 migrated MapCanvas from QOpenGLWidget to QOpenGLWindow.
Under QOpenGLWidget, the GL context is still current during destruction,
so calling cleanupOpenGL() from ~MapCanvas() was safe. Under
QOpenGLWindow, the native context is destroyed before C++ destructors
run, making any GL call from a destructor operate without a current
context.

On NVIDIA drivers, glGetError() returns GL_INVALID_OPERATION indefinitely
when called without a current context, causing the checkError() loop to
spin forever. Two destructor call paths triggered this:

1. MainWindow::~MainWindow() called forceNewFile() -> slot_dataLoaded()
   -> forceUpdateMeshes(), which created and destroyed GL meshes after
   the context was gone.

2. GLWeather was not included in cleanupOpenGL(), so its particle meshes
   were destroyed in ~MapCanvas() after the context was gone.

Fix:
- Connect QOpenGLContext::aboutToBeDestroyed -> cleanupOpenGL() in
  initializeGL(), so GL resources are freed while the context is still
  current. (The call in ~MapCanvas() is removed.)
- Add m_weather.cleanup() to cleanupOpenGL(), matching the existing
  cleanup of m_batches and m_glFont.
- Remove forceNewFile() from MainWindow::~MainWindow() to avoid
  triggering GL mesh creation/destruction during teardown.
- Guard checkError() to return early when no context is current, as a
  safety net against future oversights.
- Guard VBO::reset() and Program::reset() to skip GL deletion and log
  a qCritical when the Functions weak_ptr has expired, consistent with
  the existing behaviour in VAO::reset().

Author: nschimme

src/display/mapcanvas.cpp

@@ -94,8 +94,6 @@ MapCanvas::~MapCanvas()
     if (pmc == this) {
         pmc = nullptr;
     }
-
-    cleanupOpenGL();
 }
 
 MapCanvas *MapCanvas::getPrimary()

src/display/mapcanvas_gl.cpp

@@ -125,6 +125,7 @@ void MapCanvas::cleanupOpenGL()
     // note: m_batchedMeshes co-owns textures created by MapCanvasData,
     // and it also owns the lifetime of some OpenGL objects (e.g. VBOs).
     m_batches.resetExistingMeshesAndIgnorePendingRemesh();
+    m_weather.cleanup();
     m_textures.destroyAll();
     getGLFont().cleanup();
     getOpenGL().cleanup();
@@ -306,6 +307,14 @@ void MapCanvas::initializeGL()
         this->updateTextures();
         m_frameManager.requestUpdate();
     });
+
+    // Clean up GL resources while the context is still current.
+    // The destructor is too late — Qt destroys the context before ~MapCanvas() runs.
+    connect(context(),
+            &QOpenGLContext::aboutToBeDestroyed,
+            this,
+            &MapCanvas::cleanupOpenGL,
+            Qt::DirectConnection);
 }
 
 /* Direct means it is always called from the emitter's thread */

src/mainwindow/mainwindow.cpp

@@ -102,7 +102,6 @@ static void addApplicationFont()
 
 MainWindow::~MainWindow()
 {
-    forceNewFile();
     mmqt::rdisconnect(this);
     async_tasks::cleanup();
     delete m_listener;

src/opengl/OpenGLTypes.h

@@ -401,6 +401,7 @@ class NODISCARD UniqueMesh final
     ~UniqueMesh() = default;
     DEFAULT_MOVES_DELETE_COPIES(UniqueMesh);
 
+    void reset() { m_mesh.reset(); }
     void render(const GLRenderState &rs) const { deref(m_mesh).render(rs); }
     NODISCARD explicit operator bool() const { return m_mesh != nullptr; }
 };

src/opengl/Weather.cpp

@@ -202,6 +202,14 @@ GLWeather::~GLWeather()
     m_gl.getUboManager().unregisterRebuildFunction(Legacy::SharedVboEnum::WeatherBlock);
 }
 
+void GLWeather::cleanup()
+{
+    m_simulation.reset();
+    m_particles.reset();
+    m_atmosphere.reset();
+    m_timeOfDay.reset();
+}
+
 void GLWeather::updateFromGame()
 {
     switch (m_observer.getWeather()) {

src/opengl/Weather.h

@@ -86,7 +86,7 @@ class NODISCARD GLWeather final
                        GameObserver &observer,
                        FrameManager &frameManager);
     ~GLWeather();
-
+    void cleanup();
     DELETE_CTORS_AND_ASSIGN_OPS(GLWeather);
 
 public:

src/opengl/legacy/Legacy.cpp

@@ -34,6 +34,7 @@
 #include <QDebug>
 #include <QFile>
 #include <QMessageLogContext>
+#include <QOpenGLContext>
 #include <QOpenGLExtraFunctions>
 #include <QOpenGLTexture>
 
@@ -354,6 +355,12 @@ UboManager &Functions::getUboManager()
 /// This only exists so we can detect errors in contexts that don't support \c glDebugMessageCallback().
 void Functions::checkError()
 {
+    // glGetError() returns GL_INVALID_OPERATION indefinitely when called without a current context
+    // (e.g. on NVIDIA drivers), causing an infinite loop. Skip the check if no context is current.
+    if (QOpenGLContext::currentContext() == nullptr) {
+        return;
+    }
+
 #define CASE(x) \
     case (x): \
         qCritical() << "OpenGL error" << #x; \

src/opengl/legacy/VBO.cpp

@@ -3,6 +3,8 @@
 
 #include "VBO.h"
 
+#include <QDebug>
+
 namespace Legacy {
 bool LOG_VBO_ALLOCATIONS = false;
 bool LOG_VBO_STATIC_UPLOADS = false;
@@ -25,8 +27,11 @@ void VBO::reset()
         if (LOG_VBO_ALLOCATIONS) {
             qInfo() << this << "Freeing VBO" << vbo;
         }
-        auto sharedFunctions = std::exchange(m_weakFunctions, {}).lock();
-        deref(sharedFunctions).glDeleteBuffers(1, &vbo);
+        if (auto sharedFunctions = std::exchange(m_weakFunctions, {}).lock()) {
+            sharedFunctions->glDeleteBuffers(1, &vbo);
+        } else {
+            qCritical() << "Legacy::Functions is no longer valid, leaking VBO" << vbo;
+        }
     }
     assert(m_weakFunctions.lock() == nullptr);
 }
@@ -77,8 +82,12 @@ void Program::reset()
         if (LOG_VBO_ALLOCATIONS) {
             qInfo() << this << "Freeing Shader Program" << program;
         }
-        auto sharedFunctions = std::exchange(m_weakFunctions, {}).lock();
-        deref(sharedFunctions).glDeleteProgram(program);
+        if (auto sharedFunctions = std::exchange(m_weakFunctions, {}).lock()) {
+            sharedFunctions->glDeleteProgram(program);
+        } else {
+            qCritical() << "Legacy::Functions is no longer valid, leaking shader program"
+                        << program;
+        }
     }
     assert(m_weakFunctions.lock() == nullptr);
 }