Fix context cleanup

Context was not getting properly cleaned up in cases where
render thread stopped before the cleanup took place.
Now we ensure that required cleanup is done before context
thread terminates.

Change-Id: I1489914dafec928eebb69bac737f6f858ff49432
Reviewed-by: Tomi Korpipää <tomi.korpipaa@digia.com>

9 files changed, 111 insertions, 37 deletions

diff --git a/src/datavisualization/engine/abstract3dcontroller.cpp b/src/datavisualization/engine/abstract3dcontroller.cpp
index f8a1a813..37d7c08b 100644
--- a/src/datavisualization/engine/abstract3dcontroller.cpp
+++ b/src/datavisualization/engine/abstract3dcontroller.cpp
@@ -100,7 +100,7 @@ Abstract3DController::~Abstract3DController()
 void Abstract3DController::destroyRenderer()
 {
     // Renderer can be in another thread, don't delete it directly in that case
-    if (m_renderer && m_renderer->thread() != QThread::currentThread())
+    if (m_renderer && m_renderer->thread() && m_renderer->thread() != this->thread())
         m_renderer->deleteLater();
     else
         delete m_renderer;
@@ -114,6 +114,13 @@ void Abstract3DController::destroyRenderer()
 void Abstract3DController::setRenderer(Abstract3DRenderer *renderer)
 {
     m_renderer = renderer;
+
+    // If renderer is created in different thread than controller, make sure renderer gets
+    // destroyed before the render thread finishes.
+    if (renderer->thread() != this->thread()) {
+        QObject::connect(renderer->thread(), &QThread::finished, this,
+                         &Abstract3DController::destroyRenderer, Qt::DirectConnection);
+    }
 }
 
 void Abstract3DController::addSeries(QAbstract3DSeries *series)
diff --git a/src/datavisualization/engine/abstract3dcontroller_p.h b/src/datavisualization/engine/abstract3dcontroller_p.h
index 4d7b36a3..5bb3d863 100644
--- a/src/datavisualization/engine/abstract3dcontroller_p.h
+++ b/src/datavisualization/engine/abstract3dcontroller_p.h
@@ -218,7 +218,6 @@ public:
     virtual ~Abstract3DController();
 
     inline bool isInitialized() { return (m_renderer != 0); }
-    virtual void destroyRenderer();
     virtual void synchDataToRenderer();
     virtual void render(const GLuint defaultFboHandle = 0);
     virtual void initializeOpenGL() = 0;
@@ -348,6 +347,8 @@ public:
     void markSeriesItemLabelsDirty();
 
 public slots:
+    void destroyRenderer();
+
     void handleAxisTitleChanged(const QString &title);
     void handleAxisLabelsChanged();
     void handleAxisRangeChanged(float min, float max);
diff --git a/src/datavisualization/engine/abstract3drenderer.cpp b/src/datavisualization/engine/abstract3drenderer.cpp
index c47e2b29..d7bad300 100644
--- a/src/datavisualization/engine/abstract3drenderer.cpp
+++ b/src/datavisualization/engine/abstract3drenderer.cpp
@@ -28,6 +28,8 @@
 #include "scatter3drenderer_p.h"
 
 #include <QtCore/qmath.h>
+#include <QtGui/QWindow>
+#include <QtCore/QThread>
 
 QT_BEGIN_NAMESPACE_DATAVISUALIZATION
 
@@ -101,7 +103,11 @@ Abstract3DRenderer::Abstract3DRenderer(Abstract3DController *controller)
       m_scaleZWithBackground(0.0f),
       m_oldCameraTarget(QVector3D(2000.0f, 2000.0f, 2000.0f)), // Just random invalid target
       m_reflectionEnabled(false),
-      m_reflectivity(0.5)
+      m_reflectivity(0.5),
+      m_context(0),
+      m_currentContextAtDelete(0),
+      m_currentSurfaceAtDelete(0),
+      m_dummySurfaceAtDelete(0)
 {
     QObject::connect(m_drawer, &Drawer::drawerChanged, this, &Abstract3DRenderer::updateTextures);
     QObject::connect(this, &Abstract3DRenderer::needRender, controller,
@@ -151,10 +157,14 @@ Abstract3DRenderer::~Abstract3DRenderer()
 
         delete m_textureHelper;
     }
+
+    restoreContextAfterDelete();
 }
 
 void Abstract3DRenderer::initializeOpenGL()
 {
+    m_context = QOpenGLContext::currentContext();
+
     // Set OpenGL features
     glEnable(GL_DEPTH_TEST);
     glDepthFunc(GL_LESS);
@@ -1772,6 +1782,36 @@ void Abstract3DRenderer::queriedGraphPosition(const QMatrix4x4 &projectionViewMa
     m_graphPositionQueryPending = false;
 }
 
+void Abstract3DRenderer::fixContextBeforeDelete()
+{
+    m_currentContextAtDelete = QOpenGLContext::currentContext();
+    if (m_currentContextAtDelete)
+        m_currentSurfaceAtDelete = m_currentContextAtDelete->surface();
+    if (!m_context.isNull() && m_context.data() != m_currentContextAtDelete
+            && QThread::currentThread() == this->thread()) {
+        m_dummySurfaceAtDelete = new QWindow();
+        m_dummySurfaceAtDelete->setSurfaceType(QWindow::OpenGLSurface);
+        m_dummySurfaceAtDelete->setFormat(m_context->format());
+        m_dummySurfaceAtDelete->create();
+
+        m_context->makeCurrent(m_dummySurfaceAtDelete);
+    }
+}
+
+void Abstract3DRenderer::restoreContextAfterDelete()
+{
+    if (m_currentContextAtDelete && m_currentSurfaceAtDelete
+            && m_context.data() != m_currentContextAtDelete) {
+        m_currentContextAtDelete->makeCurrent(m_currentSurfaceAtDelete);
+    } else if (m_dummySurfaceAtDelete) {
+        m_context->doneCurrent();
+    }
+    delete m_dummySurfaceAtDelete;
+    m_currentContextAtDelete = 0;
+    m_currentSurfaceAtDelete = 0;
+    m_dummySurfaceAtDelete = 0;
+}
+
 void Abstract3DRenderer::calculatePolarXZ(const QVector3D &dataPos, float &x, float &z) const
 {
     // x is angular, z is radial
diff --git a/src/datavisualization/engine/abstract3drenderer_p.h b/src/datavisualization/engine/abstract3drenderer_p.h
index d8ca7696..1857278c 100644
--- a/src/datavisualization/engine/abstract3drenderer_p.h
+++ b/src/datavisualization/engine/abstract3drenderer_p.h
@@ -37,6 +37,8 @@
 #include "seriesrendercache_p.h"
 #include "customrenderitem_p.h"
 
+class QSurface;
+
 QT_BEGIN_NAMESPACE_DATAVISUALIZATION
 
 class TextureHelper;
@@ -229,6 +231,9 @@ protected:
     void queriedGraphPosition(const QMatrix4x4 &projectionViewMatrix, const QVector3D &scaling,
                               GLuint defaultFboHandle);
 
+    void fixContextBeforeDelete();
+    void restoreContextAfterDelete();
+
     bool m_hasNegativeValues;
     Q3DTheme *m_cachedTheme;
     Drawer *m_drawer;
@@ -318,6 +323,10 @@ protected:
     qreal m_reflectivity;
 
     QLocale m_locale;
+    QPointer<QOpenGLContext> m_context; // Not owned
+    QOpenGLContext *m_currentContextAtDelete; // Not owned
+    QSurface *m_currentSurfaceAtDelete; // Not owned
+    QWindow *m_dummySurfaceAtDelete;
 
 private:
     friend class Abstract3DController;
diff --git a/src/datavisualization/engine/bars3drenderer.cpp b/src/datavisualization/engine/bars3drenderer.cpp
index e18e4fa5..07bb5f9a 100644
--- a/src/datavisualization/engine/bars3drenderer.cpp
+++ b/src/datavisualization/engine/bars3drenderer.cpp
@@ -90,6 +90,8 @@ Bars3DRenderer::Bars3DRenderer(Bars3DController *controller)
 
 Bars3DRenderer::~Bars3DRenderer()
 {
+    fixContextBeforeDelete();
+
     if (QOpenGLContext::currentContext()) {
         m_textureHelper->glDeleteFramebuffers(1, &m_selectionFrameBuffer);
         m_textureHelper->glDeleteRenderbuffers(1, &m_selectionDepthBuffer);
diff --git a/src/datavisualization/engine/scatter3drenderer.cpp b/src/datavisualization/engine/scatter3drenderer.cpp
index f5d0793f..8ccd0824 100644
--- a/src/datavisualization/engine/scatter3drenderer.cpp
+++ b/src/datavisualization/engine/scatter3drenderer.cpp
@@ -79,6 +79,8 @@ Scatter3DRenderer::Scatter3DRenderer(Scatter3DController *controller)
 
 Scatter3DRenderer::~Scatter3DRenderer()
 {
+    fixContextBeforeDelete();
+
     if (QOpenGLContext::currentContext()) {
         m_textureHelper->glDeleteFramebuffers(1, &m_selectionFrameBuffer);
         m_textureHelper->glDeleteRenderbuffers(1, &m_selectionDepthBuffer);
diff --git a/src/datavisualization/engine/surface3drenderer.cpp b/src/datavisualization/engine/surface3drenderer.cpp
index 5daded8f..efecb0e1 100644
--- a/src/datavisualization/engine/surface3drenderer.cpp
+++ b/src/datavisualization/engine/surface3drenderer.cpp
@@ -85,6 +85,8 @@ Surface3DRenderer::Surface3DRenderer(Surface3DController *controller)
 
 Surface3DRenderer::~Surface3DRenderer()
 {
+    fixContextBeforeDelete();
+
     if (QOpenGLContext::currentContext()) {
         m_textureHelper->glDeleteFramebuffers(1, &m_depthFrameBuffer);
         m_textureHelper->glDeleteRenderbuffers(1, &m_selectionDepthBuffer);
@@ -2879,19 +2881,12 @@ void Surface3DRenderer::initShaders(const QString &vertexShader, const QString &
     Q_UNUSED(vertexShader);
     Q_UNUSED(fragmentShader);
 
-    // draw the shader for the surface according to smooth status, shadow and uniform color
-    if (m_surfaceFlatShader)
-        delete m_surfaceFlatShader;
-    if (m_surfaceSmoothShader)
-        delete m_surfaceSmoothShader;
-    if (m_surfaceTexturedSmoothShader)
-        delete m_surfaceTexturedSmoothShader;
-    if (m_surfaceTexturedFlatShader)
-        delete m_surfaceTexturedFlatShader;
-    if (m_surfaceSliceFlatShader)
-        delete m_surfaceSliceFlatShader;
-    if (m_surfaceSliceSmoothShader)
-        delete m_surfaceSliceSmoothShader;
+    delete m_surfaceFlatShader;
+    delete m_surfaceSmoothShader;
+    delete m_surfaceTexturedSmoothShader;
+    delete m_surfaceTexturedFlatShader;
+    delete m_surfaceSliceFlatShader;
+    delete m_surfaceSliceSmoothShader;
 
 #if !defined(QT_OPENGL_ES_2)
     if (m_cachedShadowQuality > QAbstract3DGraph::ShadowQualityNone) {
@@ -2924,6 +2919,7 @@ void Surface3DRenderer::initShaders(const QString &vertexShader, const QString &
     } else {
         m_surfaceFlatShader = 0;
         m_surfaceSliceFlatShader = 0;
+        m_surfaceTexturedFlatShader = 0;
     }
 #else
     m_surfaceSmoothShader = new ShaderHelper(this, QStringLiteral(":/shaders/vertex"),
@@ -2983,8 +2979,7 @@ void Surface3DRenderer::initSurfaceShaders()
 #if !defined(QT_OPENGL_ES_2)
 void Surface3DRenderer::initDepthShader()
 {
-    if (m_depthShader)
-        delete m_depthShader;
+    delete m_depthShader;
     m_depthShader = new ShaderHelper(this, QStringLiteral(":/shaders/vertexDepth"),
                                      QStringLiteral(":/shaders/fragmentDepth"));
     m_depthShader->initialize();
diff --git a/src/datavisualizationqml2/abstractdeclarative.cpp b/src/datavisualizationqml2/abstractdeclarative.cpp
index 6c6cdb90..7e8de95c 100644
--- a/src/datavisualizationqml2/abstractdeclarative.cpp
+++ b/src/datavisualizationqml2/abstractdeclarative.cpp
@@ -62,24 +62,7 @@ AbstractDeclarative::AbstractDeclarative(QQuickItem *parent) :
 
 AbstractDeclarative::~AbstractDeclarative()
 {
-#ifdef USE_SHARED_CONTEXT
-    // Context can be in another thread, don't delete it directly in that case
-    if (m_contextThread && m_contextThread != m_mainThread) {
-        if (m_context)
-            m_context->deleteLater();
-        m_context = 0;
-    } else {
-        delete m_context;
-    }
-#else
-    if (m_contextThread && m_contextThread != m_mainThread) {
-        if (m_stateStore)
-            m_stateStore->deleteLater();
-        m_stateStore = 0;
-    } else {
-        delete m_stateStore;
-    }
-#endif
+    destroyContext();
 
     disconnect(this, 0, this, 0);
     checkWindowList(0);
@@ -363,7 +346,6 @@ void AbstractDeclarative::activateOpenGLContext(QQuickWindow *window)
         m_contextThread = QThread::currentThread();
         m_contextWindow = window;
         m_qtContext = currentContext;
-
         m_context = new QOpenGLContext();
         m_context->setFormat(m_qtContext->format());
         m_context->setShareContext(m_qtContext);
@@ -371,6 +353,10 @@ void AbstractDeclarative::activateOpenGLContext(QQuickWindow *window)
 
         m_context->makeCurrent(window);
         m_controller->initializeOpenGL();
+
+        // Make sure context gets deleted.
+        QObject::connect(m_contextThread, &QThread::finished, this,
+                         &AbstractDeclarative::destroyContext, Qt::DirectConnection);
     } else {
         m_context->makeCurrent(window);
     }
@@ -392,6 +378,10 @@ void AbstractDeclarative::activateOpenGLContext(QQuickWindow *window)
 
         m_stateStore->storeGLState();
         m_controller->initializeOpenGL();
+
+        // Make sure state store gets deleted.
+        QObject::connect(m_contextThread, &QThread::finished, this,
+                         &AbstractDeclarative::destroyContext, Qt::DirectConnection);
     } else {
         m_stateStore->storeGLState();
     }
@@ -827,4 +817,31 @@ void AbstractDeclarative::windowDestroyed(QObject *obj)
     windowClearList.remove(win);
 }
 
+void AbstractDeclarative::destroyContext()
+{
+#ifdef USE_SHARED_CONTEXT
+    // Context can be in another thread, don't delete it directly in that case
+    if (m_contextThread && m_contextThread != m_mainThread) {
+        if (m_context)
+            m_context->deleteLater();
+    } else {
+        delete m_context;
+    }
+    m_context = 0;
+#else
+    if (m_contextThread && m_contextThread != m_mainThread) {
+        if (m_stateStore)
+            m_stateStore->deleteLater();
+    } else {
+        delete m_stateStore;
+    }
+    m_stateStore = 0;
+#endif
+    if (m_contextThread) {
+        QObject::disconnect(m_contextThread, &QThread::finished, this,
+                            &AbstractDeclarative::destroyContext);
+        m_contextThread = 0;
+    }
+}
+
 QT_END_NAMESPACE_DATAVISUALIZATION
diff --git a/src/datavisualizationqml2/abstractdeclarative_p.h b/src/datavisualizationqml2/abstractdeclarative_p.h
index 04db21f4..0c32a4a5 100644
--- a/src/datavisualizationqml2/abstractdeclarative_p.h
+++ b/src/datavisualizationqml2/abstractdeclarative_p.h
@@ -229,6 +229,7 @@ public slots:
     virtual void handleAxisYChanged(QAbstract3DAxis *axis) = 0;
     virtual void handleAxisZChanged(QAbstract3DAxis *axis) = 0;
     void windowDestroyed(QObject *obj);
+    void destroyContext();
 
 protected:
     virtual void mouseDoubleClickEvent(QMouseEvent *event);