From 294894610b02a2cd4682fafe139a60b2b96f3289 Mon Sep 17 00:00:00 2001 From: Paul Lemire Date: Thu, 13 Feb 2020 11:51:38 +0100 Subject: Destroy FBOs when RenderTarget node is destroyed It appears we never destroyed FBOs which lead to bugs when destroying and recreating a RenderTarget Change-Id: I99b3df95b821670aa3bbd63209ff9bcc21afbf79 Reviewed-by: Mike Krus --- src/render/backend/managers_p.h | 29 ++++++++++++++++++++++ src/render/backend/rendertarget.cpp | 28 +++++++++++++++++++++ src/render/backend/rendertarget_p.h | 15 +++++++++++ src/render/frontend/qrenderaspect.cpp | 2 +- .../opengl/graphicshelpers/submissioncontext.cpp | 8 ++++++ .../opengl/graphicshelpers/submissioncontext_p.h | 1 + src/render/renderers/opengl/renderer/renderer.cpp | 7 ++++++ 7 files changed, 89 insertions(+), 1 deletion(-) (limited to 'src/render') diff --git a/src/render/backend/managers_p.h b/src/render/backend/managers_p.h index 759c16f64..b2dec9cb5 100644 --- a/src/render/backend/managers_p.h +++ b/src/render/backend/managers_p.h @@ -282,6 +282,35 @@ class RenderTargetManager : public Qt3DCore::QResourceManager< { public: RenderTargetManager() {} + + // Called in AspectThread by RenderTarget node functor destroy + void addRenderTargetIdToCleanup(Qt3DCore::QNodeId id) + { + m_renderTargetIdsToCleanup.push_back(id); + } + + // Called in AspectThread by RenderTarget node functor create + void removeRenderTargetToCleanup(Qt3DCore::QNodeId id) + { + m_renderTargetIdsToCleanup.removeAll(id); + } + + // Called by RenderThread in updateGLResources (locked) + QVector takeRenderTargetIdsToCleanup() + { + return std::move(m_renderTargetIdsToCleanup); + } + +#ifdef QT_BUILD_INTERNAL + // For unit testing purposes only + QVector renderTargetIdsToCleanup() const + { + return m_renderTargetIdsToCleanup; + } +#endif + +private: + QVector m_renderTargetIdsToCleanup; }; class RenderPassManager : public Qt3DCore::QResourceManager< diff --git a/src/render/backend/rendertarget.cpp b/src/render/backend/rendertarget.cpp index 088818c94..f78d149db 100644 --- a/src/render/backend/rendertarget.cpp +++ b/src/render/backend/rendertarget.cpp @@ -41,6 +41,7 @@ #include #include #include +#include #include QT_BEGIN_NAMESPACE @@ -94,6 +95,33 @@ QVector RenderTarget::renderOutputs() const return m_renderOutputs; } +RenderTargetFunctor::RenderTargetFunctor(AbstractRenderer *renderer, RenderTargetManager *manager) + : m_renderer(renderer) + , m_renderTargetManager(manager) +{ +} + +QBackendNode *RenderTargetFunctor::create(const QNodeCreatedChangeBasePtr &change) const +{ + RenderTarget *backend = m_renderTargetManager->getOrCreateResource(change->subjectId()); + // Remove from the list of ids to destroy in case we were added to it + m_renderTargetManager->removeRenderTargetToCleanup(change->subjectId()); + backend->setRenderer(m_renderer); + return backend; +} + +QBackendNode *RenderTargetFunctor::get(QNodeId id) const +{ + return m_renderTargetManager->lookupResource(id); +} + +void RenderTargetFunctor::destroy(QNodeId id) const +{ + // We add ourselves to the dirty list to tell the renderer that this rendertarget has been destroyed + m_renderTargetManager->addRenderTargetIdToCleanup(id); + m_renderTargetManager->releaseResource(id); +} + } // namespace Render } // namespace Qt3DRender diff --git a/src/render/backend/rendertarget_p.h b/src/render/backend/rendertarget_p.h index 30769dcfc..e156754b7 100644 --- a/src/render/backend/rendertarget_p.h +++ b/src/render/backend/rendertarget_p.h @@ -82,6 +82,21 @@ private: QVector m_renderOutputs; }; +class Q_AUTOTEST_EXPORT RenderTargetFunctor : public Qt3DCore::QBackendNodeMapper +{ +public: + explicit RenderTargetFunctor(AbstractRenderer *renderer, + RenderTargetManager *manager); + Qt3DCore::QBackendNode *create(const Qt3DCore::QNodeCreatedChangeBasePtr &change) const final; + Qt3DCore::QBackendNode *get(Qt3DCore::QNodeId id) const final; + void destroy(Qt3DCore::QNodeId id) const final; + +private: + AbstractRenderer *m_renderer; + RenderTargetManager *m_renderTargetManager; +}; + + } // namespace Render } // namespace Qt3DRender diff --git a/src/render/frontend/qrenderaspect.cpp b/src/render/frontend/qrenderaspect.cpp index 5784b35f4..a42da4908 100644 --- a/src/render/frontend/qrenderaspect.cpp +++ b/src/render/frontend/qrenderaspect.cpp @@ -269,7 +269,7 @@ void QRenderAspectPrivate::registerBackendTypes() registerBackendType(QSharedPointer >::create(m_renderer)); registerBackendType(QSharedPointer >::create(m_renderer)); registerBackendType(QSharedPointer::create(m_renderer, m_nodeManagers->sceneManager())); - registerBackendType(QSharedPointer >::create(m_renderer)); + registerBackendType(QSharedPointer::create(m_renderer, m_nodeManagers->renderTargetManager())); registerBackendType(QSharedPointer >::create(m_renderer)); registerBackendType(QSharedPointer::create(m_renderer)); registerBackendType(QSharedPointer >::create(m_renderer)); diff --git a/src/render/renderers/opengl/graphicshelpers/submissioncontext.cpp b/src/render/renderers/opengl/graphicshelpers/submissioncontext.cpp index 47779dded..b8229dd9e 100644 --- a/src/render/renderers/opengl/graphicshelpers/submissioncontext.cpp +++ b/src/render/renderers/opengl/graphicshelpers/submissioncontext.cpp @@ -506,6 +506,14 @@ void SubmissionContext::activateRenderTarget(Qt3DCore::QNodeId renderTargetNodeI activateDrawBuffers(attachments); } +void SubmissionContext::releaseRenderTarget(const Qt3DCore::QNodeId id) +{ + if (m_renderTargets.contains(id)) { + const GLuint fboId = m_renderTargets.take(id); + m_glHelper->releaseFrameBufferObject(fboId); + } +} + GLuint SubmissionContext::createRenderTarget(Qt3DCore::QNodeId renderTargetNodeId, const AttachmentPack &attachments) { const GLuint fboId = m_glHelper->createFrameBufferObject(); diff --git a/src/render/renderers/opengl/graphicshelpers/submissioncontext_p.h b/src/render/renderers/opengl/graphicshelpers/submissioncontext_p.h index a8700dd3a..0f769fa1c 100644 --- a/src/render/renderers/opengl/graphicshelpers/submissioncontext_p.h +++ b/src/render/renderers/opengl/graphicshelpers/submissioncontext_p.h @@ -111,6 +111,7 @@ public: // FBO GLuint activeFBO() const { return m_activeFBO; } void activateRenderTarget(const Qt3DCore::QNodeId id, const AttachmentPack &attachments, GLuint defaultFboId); + void releaseRenderTarget(const Qt3DCore::QNodeId id); QSize renderTargetSize(const QSize &surfaceSize) const; QImage readFramebuffer(const QRect &rect); void blitFramebuffer(Qt3DCore::QNodeId outputRenderTargetId, Qt3DCore::QNodeId inputRenderTargetId, diff --git a/src/render/renderers/opengl/renderer/renderer.cpp b/src/render/renderers/opengl/renderer/renderer.cpp index d61d1d8ac..3e5bef3cd 100644 --- a/src/render/renderers/opengl/renderer/renderer.cpp +++ b/src/render/renderers/opengl/renderer/renderer.cpp @@ -1397,6 +1397,13 @@ void Renderer::updateGLResources() // Record list of buffer that might need uploading lookForDownloadableBuffers(); + + // Remove destroyed FBOs + { + const QNodeIdVector destroyedRenderTargetIds = m_nodesManager->renderTargetManager()->takeRenderTargetIdsToCleanup(); + for (const Qt3DCore::QNodeId &renderTargetId : destroyedRenderTargetIds) + m_submissionContext->releaseRenderTarget(renderTargetId); + } } // Render Thread -- cgit v1.2.3