Delete abandoned VAOs after each frame

Use a job to traverse all active VAO handles to check if the
geometry and shader objects used to identify the VAO do stil exist.
If not, let the renderer dispose the VAOs.

Make sure to synchronize access to VAO state and hide state changes
behind member functions.

Change-Id: Ib77be67d55daa61885cd914af8d9cfc622cae940
Reviewed-by: Paul Lemire <paul.lemire@kdab.com>

5 files changed, 79 insertions, 37 deletions

diff --git a/src/render/backend/openglvertexarrayobject.cpp b/src/render/backend/openglvertexarrayobject.cpp
index 30f79c466..b88ddfb7b 100644
--- a/src/render/backend/openglvertexarrayobject.cpp
+++ b/src/render/backend/openglvertexarrayobject.cpp
@@ -52,15 +52,8 @@ OpenGLVertexArrayObject::OpenGLVertexArrayObject()
     : m_ctx(nullptr)
     , m_specified(false)
     , m_supportsVao(false)
-    , m_createdEmulatedVAO(false)
 {}
 
-void OpenGLVertexArrayObject::setGraphicsContext(GraphicsContext *ctx)
-{
-    m_ctx = ctx;
-    m_supportsVao = m_ctx->supportsVAO();
-}
-
 void OpenGLVertexArrayObject::bind()
 {
     Q_ASSERT(m_ctx);
@@ -99,19 +92,25 @@ void OpenGLVertexArrayObject::release()
     }
 }
 
-void OpenGLVertexArrayObject::create()
+// called from Render thread
+void OpenGLVertexArrayObject::create(GraphicsContext *ctx, const VAOIdentifier &key)
 {
-    Q_ASSERT(m_ctx);
-    if (m_supportsVao) {
-        Q_ASSERT(!m_vao.isNull());
-        m_vao->create();
-    } else {
-        m_createdEmulatedVAO = true;
-    }
+    QMutexLocker lock(&m_mutex);
+
+    Q_ASSERT(!m_ctx && !m_vao);
+
+    m_ctx = ctx;
+    m_supportsVao = m_ctx->supportsVAO();
+    m_vao.reset(m_supportsVao ? new QOpenGLVertexArrayObject() : nullptr);
+    m_vao->create();
+    m_owners = key;
 }
 
+// called from Render thread
 void OpenGLVertexArrayObject::destroy()
 {
+    QMutexLocker locker(&m_mutex);
+
     Q_ASSERT(m_ctx);
     cleanup();
 }
@@ -122,20 +121,22 @@ void OpenGLVertexArrayObject::cleanup()
     m_ctx = nullptr;
     m_specified = false;
     m_supportsVao = false;
-    m_createdEmulatedVAO = false;
     m_indexAttribute = GraphicsContext::VAOIndexAttribute();
     m_vertexAttributes.clear();
 }
 
-
-bool OpenGLVertexArrayObject::isCreated() const
+// called from job
+bool OpenGLVertexArrayObject::isAbandoned(GeometryManager *geomMgr, ShaderManager *shaderMgr)
 {
-    if (m_supportsVao) {
-        Q_ASSERT(!m_vao.isNull());
-        return m_vao->isCreated();
-    } else {
-        return m_createdEmulatedVAO;
-    }
+    QMutexLocker lock(&m_mutex);
+
+    if (!m_ctx)
+        return false;
+
+    const bool geometryExists = (geomMgr->data(m_owners.first) != nullptr);
+    const bool shaderExists = (shaderMgr->data(m_owners.second) != nullptr);
+
+    return !geometryExists || !shaderExists;
 }
 
 void OpenGLVertexArrayObject::saveVertexAttribute(const GraphicsContext::VAOVertexAttribute &attr)
diff --git a/src/render/backend/openglvertexarrayobject_p.h b/src/render/backend/openglvertexarrayobject_p.h
index 6d8eecec8..a564175f6 100644
--- a/src/render/backend/openglvertexarrayobject_p.h
+++ b/src/render/backend/openglvertexarrayobject_p.h
@@ -59,6 +59,9 @@ QT_BEGIN_NAMESPACE
 namespace Qt3DRender {
 namespace Render {
 
+class GeometryManager;
+class ShaderManager;
+
 typedef QPair<HGeometry, HShader> VAOIdentifier;
 
 class OpenGLVertexArrayObject
@@ -66,27 +69,29 @@ class OpenGLVertexArrayObject
 public:
     OpenGLVertexArrayObject();
 
-    void setGraphicsContext(GraphicsContext *ctx);
     void bind();
     void release();
-    void create();
+
+    void create(GraphicsContext *ctx, const VAOIdentifier &key);
     void destroy();
     void cleanup();
-    bool isCreated() const;
+
+    bool isAbandoned(GeometryManager *geomMgr, ShaderManager *shaderMgr);
 
     QOpenGLVertexArrayObject *vao() { return m_vao.data(); }
     const QOpenGLVertexArrayObject *vao() const { return m_vao.data(); }
-    void setVao(QOpenGLVertexArrayObject *vao) { m_vao.reset(vao); }
 
     void setSpecified(bool b) { m_specified = b; }
     bool isSpecified() const { return m_specified; }
 
+
 private:
+    QMutex m_mutex;
     GraphicsContext *m_ctx;
     QScopedPointer<QOpenGLVertexArrayObject> m_vao;
     bool m_specified;
     bool m_supportsVao;
-    bool m_createdEmulatedVAO;
+    VAOIdentifier m_owners;
 
     friend class GraphicsContext;
 
diff --git a/src/render/backend/renderer.cpp b/src/render/backend/renderer.cpp
index d38df13e2..100c4a33d 100644
--- a/src/render/backend/renderer.cpp
+++ b/src/render/backend/renderer.cpp
@@ -170,6 +170,7 @@ Renderer::Renderer(QRenderAspect::RenderType type)
     , m_updateMeshTriangleListJob(Render::UpdateMeshTriangleListJobPtr::create())
     , m_filterCompatibleTechniqueJob(Render::FilterCompatibleTechniqueJobPtr::create())
     , m_bufferGathererJob(Render::GenericLambdaJobPtr<std::function<void ()>>::create([this] { lookForDirtyBuffers(); }, JobTypes::DirtyBufferGathering))
+    , m_vaoGathererJob(Render::GenericLambdaJobPtr<std::function<void ()>>::create([this] { lookForAbandonedVaos(); }, JobTypes::DirtyVaoGathering))
     , m_textureGathererJob(Render::GenericLambdaJobPtr<std::function<void ()>>::create([this] { lookForDirtyTextures(); }, JobTypes::DirtyTextureGathering))
     , m_shaderGathererJob(Render::GenericLambdaJobPtr<std::function<void ()>>::create([this] { lookForDirtyShaders(); }, JobTypes::DirtyShaderGathering))
     , m_syncTextureLoadingJob(Render::GenericLambdaJobPtr<std::function<void ()>>::create([] {}, JobTypes::SyncTextureLoading))
@@ -831,6 +832,23 @@ void Renderer::prepareCommandsSubmission(const QVector<RenderView *> &renderView
 }
 
 // Executed in a job
+void Renderer::lookForAbandonedVaos()
+{
+    const QVector<HVao> activeVaos = m_nodesManager->vaoManager()->activeHandles();
+    for (HVao handle : activeVaos) {
+        OpenGLVertexArrayObject *vao = m_nodesManager->vaoManager()->data(handle);
+
+        // Make sure to only mark VAOs for deletion that were already created
+        // (ignore those that might be currently under construction in the render thread)
+        if (vao->isAbandoned(m_nodesManager->geometryManager(), m_nodesManager->shaderManager())) {
+            m_abandonedVaosMutex.lock();
+            m_abandonedVaos.push_back(handle);
+            m_abandonedVaosMutex.unlock();
+        }
+    }
+}
+
+// Executed in a job
 void Renderer::lookForDirtyBuffers()
 {
     const QVector<HBuffer> activeBufferHandles = m_nodesManager->bufferManager()->activeHandles();
@@ -923,8 +941,6 @@ void Renderer::updateGLResources()
         // We can really release the texture at this point
         m_nodesManager->textureManager()->releaseResource(textureCleanedUpId);
     }
-
-
 }
 
 // Render Thread
@@ -1267,6 +1283,7 @@ QVector<Qt3DCore::QAspectJobPtr> Renderer::renderBinJobs()
 
     // Jobs to prepare GL Resource upload
     renderBinJobs.push_back(m_syncTextureLoadingJob);
+    renderBinJobs.push_back(m_vaoGathererJob);
     renderBinJobs.push_back(m_bufferGathererJob);
     renderBinJobs.push_back(m_textureGathererJob);
     renderBinJobs.push_back(m_shaderGathererJob);
@@ -1368,16 +1385,15 @@ void Renderer::createOrUpdateVAO(RenderCommand *command,
                                  HVao *previousVaoHandle,
                                  OpenGLVertexArrayObject **vao)
 {
+    const VAOIdentifier vaoKey(command->m_geometry, command->m_shader);
+
     VAOManager *vaoManager = m_nodesManager->vaoManager();
-    command->m_vao = vaoManager->lookupHandle(QPair<HGeometry, HShader>(command->m_geometry, command->m_shader));
+    command->m_vao = vaoManager->lookupHandle(vaoKey);
 
     if (command->m_vao.isNull()) {
         qCDebug(Rendering) << Q_FUNC_INFO << "Allocating new VAO";
-        command->m_vao = vaoManager->getOrAcquireHandle(QPair<HGeometry, HShader>(command->m_geometry, command->m_shader));
-        vaoManager->data(command->m_vao)->setGraphicsContext(m_graphicsContext.data());
-        if (m_graphicsContext->supportsVAO())
-            vaoManager->data(command->m_vao)->setVao(new QOpenGLVertexArrayObject());
-        vaoManager->data(command->m_vao)->create();
+        command->m_vao = vaoManager->getOrAcquireHandle(vaoKey);
+        vaoManager->data(command->m_vao)->create(m_graphicsContext.data(), vaoKey);
     }
 
     if (*previousVaoHandle != command->m_vao) {
@@ -1573,6 +1589,20 @@ void Renderer::cleanGraphicsResources()
         tex->destroyGLTexture();
         delete tex;
     }
+
+    // Delete abandoned VAOs
+    m_abandonedVaosMutex.lock();
+    const QVector<HVao> abandonedVaos = std::move(m_abandonedVaos);
+    m_abandonedVaosMutex.unlock();
+    for (HVao vaoHandle : abandonedVaos) {
+        // might have already been destroyed last frame, but added by the cleanup job before, so
+        // check if the VAO is really still existent
+        OpenGLVertexArrayObject *vao = m_nodesManager->vaoManager()->data(vaoHandle);
+        if (vao) {
+            vao->destroy();
+            m_nodesManager->vaoManager()->release(vaoHandle);
+        }
+    }
 }
 
 QList<QMouseEvent> Renderer::pendingPickingEvents() const
diff --git a/src/render/backend/renderer_p.h b/src/render/backend/renderer_p.h
index 9f6e8d857..bd12ecec6 100644
--- a/src/render/backend/renderer_p.h
+++ b/src/render/backend/renderer_p.h
@@ -322,15 +322,20 @@ private:
                            OpenGLVertexArrayObject **vao);
 
     GenericLambdaJobPtr<std::function<void ()>> m_bufferGathererJob;
+    GenericLambdaJobPtr<std::function<void ()>> m_vaoGathererJob;
     GenericLambdaJobPtr<std::function<void ()>> m_textureGathererJob;
     GenericLambdaJobPtr<std::function<void ()>> m_shaderGathererJob;
 
     SynchronizerJobPtr m_syncTextureLoadingJob;
 
+    void lookForAbandonedVaos();
     void lookForDirtyBuffers();
     void lookForDirtyTextures();
     void lookForDirtyShaders();
 
+    QMutex m_abandonedVaosMutex;
+    QVector<HVao> m_abandonedVaos;
+
     QVector<HBuffer> m_dirtyBuffers;
     QVector<HShader> m_dirtyShaders;
     QVector<HTexture> m_dirtyTextures;
diff --git a/src/render/jobs/job_common_p.h b/src/render/jobs/job_common_p.h
index f3be2e6b0..4c3a6a6c0 100644
--- a/src/render/jobs/job_common_p.h
+++ b/src/render/jobs/job_common_p.h
@@ -86,6 +86,7 @@ namespace JobTypes {
         UpdateWorldBoundingVolume,
         FrameSubmissionPart2,
         DirtyBufferGathering,
+        DirtyVaoGathering,
         DirtyTextureGathering,
         DirtyShaderGathering,
         SendRenderCapture,