diff options
author | Paul Lemire <paul.lemire@kdab.com> | 2020-02-13 11:51:38 +0100 |
---|---|---|
committer | Paul Lemire <paul.lemire@kdab.com> | 2020-02-13 15:59:51 +0100 |
commit | 294894610b02a2cd4682fafe139a60b2b96f3289 (patch) | |
tree | 70dc752d85d8b10df0f75be56c1fa2b5c12822c4 | |
parent | d046e7cc3e362e6cb45afd5ecae464d796c21079 (diff) |
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 <mike.krus@kdab.com>
-rw-r--r-- | src/render/backend/managers_p.h | 29 | ||||
-rw-r--r-- | src/render/backend/rendertarget.cpp | 28 | ||||
-rw-r--r-- | src/render/backend/rendertarget_p.h | 15 | ||||
-rw-r--r-- | src/render/frontend/qrenderaspect.cpp | 2 | ||||
-rw-r--r-- | src/render/renderers/opengl/graphicshelpers/submissioncontext.cpp | 8 | ||||
-rw-r--r-- | src/render/renderers/opengl/graphicshelpers/submissioncontext_p.h | 1 | ||||
-rw-r--r-- | src/render/renderers/opengl/renderer/renderer.cpp | 7 | ||||
-rw-r--r-- | tests/auto/render/rendertarget/tst_rendertarget.cpp | 42 |
8 files changed, 131 insertions, 1 deletions
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<Qt3DCore::QNodeId> takeRenderTargetIdsToCleanup() + { + return std::move(m_renderTargetIdsToCleanup); + } + +#ifdef QT_BUILD_INTERNAL + // For unit testing purposes only + QVector<Qt3DCore::QNodeId> renderTargetIdsToCleanup() const + { + return m_renderTargetIdsToCleanup; + } +#endif + +private: + QVector<Qt3DCore::QNodeId> 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 <Qt3DRender/qrendertarget.h> #include <Qt3DRender/private/qrendertarget_p.h> #include <Qt3DRender/qrendertargetoutput.h> +#include <Qt3DRender/private/managers_p.h> #include <QVariant> QT_BEGIN_NAMESPACE @@ -94,6 +95,33 @@ QVector<Qt3DCore::QNodeId> 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<Qt3DCore::QNodeId> 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<QLevelOfDetail, true>(QSharedPointer<Render::NodeFunctor<Render::LevelOfDetail, Render::LevelOfDetailManager> >::create(m_renderer)); registerBackendType<QLevelOfDetailSwitch, true>(QSharedPointer<Render::NodeFunctor<Render::LevelOfDetail, Render::LevelOfDetailManager> >::create(m_renderer)); registerBackendType<QSceneLoader, true>(QSharedPointer<Render::RenderSceneFunctor>::create(m_renderer, m_nodeManagers->sceneManager())); - registerBackendType<QRenderTarget, true>(QSharedPointer<Render::NodeFunctor<Render::RenderTarget, Render::RenderTargetManager> >::create(m_renderer)); + registerBackendType<QRenderTarget, true>(QSharedPointer<Render::RenderTargetFunctor>::create(m_renderer, m_nodeManagers->renderTargetManager())); registerBackendType<QRenderTargetOutput, true>(QSharedPointer<Render::NodeFunctor<Render::RenderTargetOutput, Render::AttachmentManager> >::create(m_renderer)); registerBackendType<QRenderSettings, true>(QSharedPointer<Render::RenderSettingsFunctor>::create(m_renderer)); registerBackendType<QRenderState, true>(QSharedPointer<Render::NodeFunctor<Render::RenderStateNode, Render::RenderStateManager> >::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 diff --git a/tests/auto/render/rendertarget/tst_rendertarget.cpp b/tests/auto/render/rendertarget/tst_rendertarget.cpp index 31eee6ec5..9b436aa5b 100644 --- a/tests/auto/render/rendertarget/tst_rendertarget.cpp +++ b/tests/auto/render/rendertarget/tst_rendertarget.cpp @@ -32,6 +32,7 @@ #include <Qt3DRender/qrendertargetoutput.h> #include <Qt3DRender/private/qrendertarget_p.h> #include <Qt3DRender/private/rendertarget_p.h> +#include <Qt3DRender/private/managers_p.h> #include "qbackendnodetester.h" #include "testrenderer.h" @@ -205,6 +206,47 @@ private Q_SLOTS: } } + void checkRenderTargetManager() + { + // GIVEN + Qt3DRender::QRenderTarget renderTarget; + TestRenderer renderer; + Qt3DRender::Render::RenderTargetManager manager; + Qt3DRender::Render::RenderTargetFunctor creationFunctor(&renderer, &manager); + + // THEN + QVERIFY(manager.renderTargetIdsToCleanup().isEmpty()); + + // WHEN + Qt3DCore::QNodeCreatedChangeBase changeObj(&renderTarget); + Qt3DCore::QNodeCreatedChangeBasePtr changePtr(&changeObj, [](Qt3DCore::QNodeCreatedChangeBase *) {}); + auto backend = creationFunctor.create(changePtr); + + // THEN + QVERIFY(backend != nullptr); + QVERIFY(manager.renderTargetIdsToCleanup().isEmpty()); + + { + // WHEN + auto sameBackend = creationFunctor.get(renderTarget.id()); + // THEN + QCOMPARE(backend, sameBackend); + } + + // WHEN + creationFunctor.destroy(renderTarget.id()); + + // THEN -> Should be in list of ids to remove and return null on get + QVERIFY(manager.renderTargetIdsToCleanup().contains(renderTarget.id())); + QVERIFY(creationFunctor.get(renderTarget.id()) == nullptr); + + // WHEN -> Should be removed from list of ids to remove + creationFunctor.create(changePtr); + + // THEN + QVERIFY(manager.renderTargetIdsToCleanup().isEmpty()); + QVERIFY(creationFunctor.get(renderTarget.id()) != nullptr); + } }; QTEST_MAIN(tst_RenderTarget) |