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-17 07:08:42 +0100 |
commit | c465d4f28d1dd132901869131b3c231280c737d0 (patch) | |
tree | 21c07b5e53932a18cde7a679839baf8a87454bc6 | |
parent | b164cb09d4221c3b3655755f758b333ea76cce2b (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 fc2a0b479..d22bf43b7 100644 --- a/src/render/backend/managers_p.h +++ b/src/render/backend/managers_p.h @@ -280,6 +280,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 b0565a26b..33ba79cc2 100644 --- a/src/render/backend/rendertarget.cpp +++ b/src/render/backend/rendertarget.cpp @@ -44,6 +44,7 @@ #include <Qt3DCore/qpropertyupdatedchange.h> #include <Qt3DCore/qpropertynodeaddedchange.h> #include <Qt3DCore/qpropertynoderemovedchange.h> +#include <Qt3DRender/private/managers_p.h> #include <QVariant> QT_BEGIN_NAMESPACE @@ -114,6 +115,33 @@ void RenderTarget::sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e) BackendNode::sceneChangeEvent(e); } +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 5e3e63582..4a8fc851f 100644 --- a/src/render/backend/rendertarget_p.h +++ b/src/render/backend/rendertarget_p.h @@ -84,6 +84,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 ac0bd7361..22ff4e14e 100644 --- a/src/render/frontend/qrenderaspect.cpp +++ b/src/render/frontend/qrenderaspect.cpp @@ -244,7 +244,7 @@ void QRenderAspectPrivate::registerBackendTypes() q->registerBackendType<QLevelOfDetail>(QSharedPointer<Render::NodeFunctor<Render::LevelOfDetail, Render::LevelOfDetailManager> >::create(m_renderer)); q->registerBackendType<QLevelOfDetailSwitch>(QSharedPointer<Render::NodeFunctor<Render::LevelOfDetail, Render::LevelOfDetailManager> >::create(m_renderer)); q->registerBackendType<QSceneLoader>(QSharedPointer<Render::RenderSceneFunctor>::create(m_renderer, m_nodeManagers->sceneManager())); - q->registerBackendType<QRenderTarget>(QSharedPointer<Render::NodeFunctor<Render::RenderTarget, Render::RenderTargetManager> >::create(m_renderer)); + q->registerBackendType<QRenderTarget>(QSharedPointer<Render::RenderTargetFunctor>::create(m_renderer, m_nodeManagers->renderTargetManager())); q->registerBackendType<QRenderTargetOutput>(QSharedPointer<Render::NodeFunctor<Render::RenderTargetOutput, Render::AttachmentManager> >::create(m_renderer)); q->registerBackendType<QRenderSettings>(QSharedPointer<Render::RenderSettingsFunctor>::create(m_renderer)); q->registerBackendType<QRenderState>(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 624ddb56f..345f7a6d0 100644 --- a/src/render/renderers/opengl/graphicshelpers/submissioncontext.cpp +++ b/src/render/renderers/opengl/graphicshelpers/submissioncontext.cpp @@ -494,6 +494,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 9b9bd7fa8..74238b173 100644 --- a/src/render/renderers/opengl/graphicshelpers/submissioncontext_p.h +++ b/src/render/renderers/opengl/graphicshelpers/submissioncontext_p.h @@ -109,6 +109,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 f32862bfd..6018aa398 100644 --- a/src/render/renderers/opengl/renderer/renderer.cpp +++ b/src/render/renderers/opengl/renderer/renderer.cpp @@ -1307,6 +1307,13 @@ void Renderer::updateGLResources() const QVector<Qt3DCore::QNodeId> cleanedUpTextureIds = m_nodesManager->textureManager()->takeTexturesIdsToCleanup(); for (const Qt3DCore::QNodeId textureCleanedUpId: cleanedUpTextureIds) cleanupTexture(textureCleanedUpId); + + // 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 a5d8cad77..30de0dde8 100644 --- a/tests/auto/render/rendertarget/tst_rendertarget.cpp +++ b/tests/auto/render/rendertarget/tst_rendertarget.cpp @@ -35,6 +35,7 @@ #include <Qt3DCore/qpropertyupdatedchange.h> #include <Qt3DCore/qpropertynodeaddedchange.h> #include <Qt3DCore/qpropertynoderemovedchange.h> +#include <Qt3DRender/private/managers_p.h> #include "qbackendnodetester.h" #include "testrenderer.h" @@ -204,6 +205,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) |