Merge gatherer and caching jobs

Avoid extra thread sync just for copying data around

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

7 files changed, 47 insertions, 82 deletions

diff --git a/src/plugins/renderers/opengl/renderer/renderer.cpp b/src/plugins/renderers/opengl/renderer/renderer.cpp
index 9dffba30b..8ea13fb82 100644
--- a/src/plugins/renderers/opengl/renderer/renderer.cpp
+++ b/src/plugins/renderers/opengl/renderer/renderer.cpp
@@ -81,7 +81,6 @@
 #include <Qt3DRender/private/subtreeenabler_p.h>
 #include <Qt3DRender/private/qshaderprogrambuilder_p.h>
 #include <Qt3DRender/private/qshaderprogram_p.h>
-#include <Qt3DRender/private/filterentitybycomponentjob_p.h>
 #include <Qt3DRender/private/qrenderaspect_p.h>
 #ifndef Q_OS_INTEGRITY
 #include <imguirenderer_p.h>
@@ -139,41 +138,41 @@ namespace OpenGL {
 
 namespace {
 
-class SyncLightsGatherer
-{
+class CachingLightGatherer : public LightGatherer {
 public:
-    explicit SyncLightsGatherer(LightGathererPtr gatherJob,
-                                RendererCache *cache)
-        : m_gatherJob(gatherJob)
+    CachingLightGatherer(RendererCache *cache)
+        : LightGatherer()
         , m_cache(cache)
     {
     }
 
-    void operator()()
+    void run() override
     {
+        LightGatherer::run();
+
         QMutexLocker lock(m_cache->mutex());
-        m_cache->gatheredLights = m_gatherJob->lights();
-        m_cache->environmentLight = m_gatherJob->takeEnvironmentLight();
+        m_cache->gatheredLights = lights();
+        m_cache->environmentLight = environmentLight();
     }
 
 private:
-    LightGathererPtr m_gatherJob;
     RendererCache *m_cache;
 };
 
-class SyncRenderableEntities
-{
+class CachingRenderableEntityFilter : public RenderableEntityFilter {
 public:
-    explicit SyncRenderableEntities(RenderableEntityFilterPtr gatherJob,
-                                    RendererCache *cache)
-        : m_gatherJob(gatherJob)
+    CachingRenderableEntityFilter(RendererCache *cache)
+        : RenderableEntityFilter()
         , m_cache(cache)
     {
+
     }
 
-    void operator()()
+    void run() override
     {
-        QVector<Entity *> selectedEntities = m_gatherJob->filteredEntities();
+        RenderableEntityFilter::run();
+
+        QVector<Entity *> selectedEntities = filteredEntities();
         std::sort(selectedEntities.begin(), selectedEntities.end());
 
         QMutexLocker lock(m_cache->mutex());
@@ -181,23 +180,23 @@ public:
     }
 
 private:
-    RenderableEntityFilterPtr m_gatherJob;
     RendererCache *m_cache;
 };
 
-class SyncComputableEntities
-{
+class CachingComputableEntityFilter : public ComputableEntityFilter {
 public:
-    explicit SyncComputableEntities(ComputableEntityFilterPtr gatherJob,
-                                    RendererCache *cache)
-        : m_gatherJob(gatherJob)
+    CachingComputableEntityFilter(RendererCache *cache)
+        : ComputableEntityFilter()
         , m_cache(cache)
     {
+
     }
 
-    void operator()()
+    void run() override
     {
-        QVector<Entity *> selectedEntities = m_gatherJob->filteredEntities();
+        ComputableEntityFilter::run();
+
+        QVector<Entity *> selectedEntities = filteredEntities();
         std::sort(selectedEntities.begin(), selectedEntities.end());
 
         QMutexLocker lock(m_cache->mutex());
@@ -205,7 +204,6 @@ public:
     }
 
 private:
-    ComputableEntityFilterPtr m_gatherJob;
     RendererCache *m_cache;
 };
 
@@ -253,9 +251,9 @@ Renderer::Renderer(QRenderAspect::RenderType type)
     , m_cleanupJob(Render::FrameCleanupJobPtr::create())
     , m_sendBufferCaptureJob(Render::SendBufferCaptureJobPtr::create())
     , m_filterCompatibleTechniqueJob(FilterCompatibleTechniqueJobPtr::create())
-    , m_lightGathererJob(Render::LightGathererPtr::create())
-    , m_renderableEntityFilterJob(Render::RenderableEntityFilterPtr::create())
-    , m_computableEntityFilterJob(Render::ComputableEntityFilterPtr::create())
+    , m_lightGathererJob(new CachingLightGatherer(&m_cache))
+    , m_renderableEntityFilterJob(new CachingRenderableEntityFilter(&m_cache))
+    , m_computableEntityFilterJob(new CachingComputableEntityFilter(&m_cache))
     , m_bufferGathererJob(CreateSynchronizerJobPtr([this] { lookForDirtyBuffers(); }, JobTypes::DirtyBufferGathering))
     , m_vaoGathererJob(CreateSynchronizerJobPtr([this] { lookForAbandonedVaos(); }, JobTypes::DirtyVaoGathering))
     , m_textureGathererJob(CreateSynchronizerJobPtr([this] { lookForDirtyTextures(); }, JobTypes::DirtyTextureGathering))
@@ -263,12 +261,6 @@ Renderer::Renderer(QRenderAspect::RenderType type)
     , m_introspectShaderJob(CreateSynchronizerPostFramePtr([this] { reloadDirtyShaders(); },
                                                            [this] (Qt3DCore::QAspectManager *m) { sendShaderChangesToFrontend(m); },
                                                            JobTypes::DirtyShaderGathering))
-    , m_cacheRenderableEntitiesJob(CreateSynchronizerJobPtr(SyncRenderableEntities(m_renderableEntityFilterJob, &m_cache),
-                                                            JobTypes::EntityComponentTypeFiltering))
-    , m_cacheComputableEntitiesJob(CreateSynchronizerJobPtr(SyncComputableEntities(m_computableEntityFilterJob, &m_cache),
-                                                            JobTypes::EntityComponentTypeFiltering))
-    , m_cacheLightsJob(CreateSynchronizerJobPtr(SyncLightsGatherer(m_lightGathererJob, &m_cache),
-                                                JobTypes::EntityComponentTypeFiltering))
     , m_ownedContext(false)
     , m_offscreenHelper(nullptr)
     , m_glResourceManagers(nullptr)
@@ -285,10 +277,6 @@ Renderer::Renderer(QRenderAspect::RenderType type)
 
     m_introspectShaderJob->addDependency(m_filterCompatibleTechniqueJob);
 
-    m_cacheLightsJob->addDependency(m_lightGathererJob);
-    m_cacheRenderableEntitiesJob->addDependency(m_renderableEntityFilterJob);
-    m_cacheComputableEntitiesJob->addDependency(m_computableEntityFilterJob);
-
     m_filterCompatibleTechniqueJob->setRenderer(this);
 
     m_defaultRenderStateSet = new RenderStateSet;
@@ -1901,20 +1889,14 @@ QVector<Qt3DCore::QAspectJobPtr> Renderer::renderBinJobs()
     const bool materialCacheNeedsToBeRebuilt = shadersDirty || materialDirty || frameGraphDirty;
     const bool renderCommandsDirty = materialCacheNeedsToBeRebuilt || renderableDirty || computeableDirty;
 
-    if (renderableDirty) {
+    if (renderableDirty)
         renderBinJobs.push_back(m_renderableEntityFilterJob);
-        renderBinJobs.push_back(m_cacheRenderableEntitiesJob);
-    }
 
-    if (computeableDirty) {
+    if (computeableDirty)
         renderBinJobs.push_back(m_computableEntityFilterJob);
-        renderBinJobs.push_back(m_cacheComputableEntitiesJob);
-    }
 
-    if (lightsDirty) {
+    if (lightsDirty)
         renderBinJobs.push_back(m_lightGathererJob);
-        renderBinJobs.push_back(m_cacheLightsJob);
-    }
 
     QMutexLocker lock(m_renderQueue->mutex());
     if (m_renderQueue->wasReset()) { // Have we rendered yet? (Scene3D case)
diff --git a/src/plugins/renderers/opengl/renderer/renderer_p.h b/src/plugins/renderers/opengl/renderer/renderer_p.h
index a285659b5..93faf982d 100644
--- a/src/plugins/renderers/opengl/renderer/renderer_p.h
+++ b/src/plugins/renderers/opengl/renderer/renderer_p.h
@@ -67,6 +67,7 @@
 #include <Qt3DRender/private/shaderbuilder_p.h>
 #include <Qt3DRender/private/lightgatherer_p.h>
 #include <Qt3DRender/private/texture_p.h>
+#include <Qt3DRender/private/filterentitybycomponentjob_p.h>
 #include <shaderparameterpack_p.h>
 #include <renderviewinitializerjob_p.h>
 #include <filtercompatibletechniquejob_p.h>
@@ -142,15 +143,10 @@ class VSyncFrameAdvanceService;
 class NodeManagers;
 class ResourceAccessor;
 
-class UpdateLevelOfDetailJob;
-typedef QSharedPointer<UpdateLevelOfDetailJob> UpdateLevelOfDetailJobPtr;
-
-template<typename T, typename ... Ts>
-class FilterEntityByComponentJob;
-template<typename T, typename ... Ts>
-using FilterEntityByComponentJobPtr = QSharedPointer<FilterEntityByComponentJob<T, Ts...>>;
-using ComputableEntityFilterPtr = FilterEntityByComponentJobPtr<Render::ComputeCommand, Render::Material>;
-using RenderableEntityFilterPtr = FilterEntityByComponentJobPtr<Render::GeometryRenderer, Render::Material>;
+using ComputableEntityFilter = FilterEntityByComponentJob<Render::ComputeCommand, Render::Material>;
+using ComputableEntityFilterPtr = QSharedPointer<ComputableEntityFilter>;
+using RenderableEntityFilter = FilterEntityByComponentJob<Render::GeometryRenderer, Render::Material>;
+using RenderableEntityFilterPtr = QSharedPointer<RenderableEntityFilter>;
 
 using SynchronizerJobPtr = GenericLambdaJobPtr<std::function<void()>>;
 using SynchronizerPostFramePtr = GenericLambdaJobAndPostFramePtr<std::function<void ()>, std::function<void (Qt3DCore::QAspectManager *)>>;
@@ -233,9 +229,6 @@ public:
     inline LightGathererPtr lightGathererJob() const { return m_lightGathererJob; }
     inline RenderableEntityFilterPtr renderableEntityFilterJob() const { return m_renderableEntityFilterJob; }
     inline ComputableEntityFilterPtr computableEntityFilterJob() const { return m_computableEntityFilterJob; }
-    inline SynchronizerJobPtr cacheLightJob() const { return m_cacheLightsJob; }
-    inline SynchronizerJobPtr cacheRenderableEntitiesJob() const { return m_cacheRenderableEntitiesJob; }
-    inline SynchronizerJobPtr cacheComputableEntitiesJob() const { return m_cacheComputableEntitiesJob; }
 
     Qt3DCore::QAbstractFrameAdvanceService *frameAdvanceService() const override;
 
@@ -384,9 +377,6 @@ private:
     SynchronizerJobPtr m_textureGathererJob;
     SynchronizerJobPtr m_sendSetFenceHandlesToFrontendJob;
     SynchronizerPostFramePtr m_introspectShaderJob;
-    SynchronizerJobPtr m_cacheRenderableEntitiesJob;
-    SynchronizerJobPtr m_cacheComputableEntitiesJob;
-    SynchronizerJobPtr m_cacheLightsJob;
 
     void lookForAbandonedVaos();
     void lookForDirtyBuffers();
diff --git a/src/plugins/renderers/opengl/renderer/renderviewbuilder.cpp b/src/plugins/renderers/opengl/renderer/renderviewbuilder.cpp
index 7264064f6..c1a7fcaf4 100644
--- a/src/plugins/renderers/opengl/renderer/renderviewbuilder.cpp
+++ b/src/plugins/renderers/opengl/renderer/renderviewbuilder.cpp
@@ -676,7 +676,7 @@ QVector<Qt3DCore::QAspectJobPtr> RenderViewBuilder::buildJobHierachy() const
     m_syncRenderViewPreCommandUpdateJob->addDependency(m_renderer->introspectShadersJob());
     m_syncRenderViewPreCommandUpdateJob->addDependency(m_renderer->bufferGathererJob());
     m_syncRenderViewPreCommandUpdateJob->addDependency(m_renderer->textureGathererJob());
-    m_syncRenderViewPreCommandUpdateJob->addDependency(m_renderer->cacheLightJob());
+    m_syncRenderViewPreCommandUpdateJob->addDependency(m_renderer->lightGathererJob());
 
     for (const auto &renderViewCommandUpdater : qAsConst(m_renderViewCommandUpdaterJobs)) {
         renderViewCommandUpdater->addDependency(m_syncRenderViewPreCommandUpdateJob);
@@ -692,8 +692,8 @@ QVector<Qt3DCore::QAspectJobPtr> RenderViewBuilder::buildJobHierachy() const
     jobs.push_back(m_syncRenderViewPostInitializationJob); // Step 2
 
     if (m_renderCommandCacheNeedsToBeRebuilt) { // Step 3
-        m_syncRenderViewPreCommandBuildingJob->addDependency(m_renderer->cacheComputableEntitiesJob());
-        m_syncRenderViewPreCommandBuildingJob->addDependency(m_renderer->cacheRenderableEntitiesJob());
+        m_syncRenderViewPreCommandBuildingJob->addDependency(m_renderer->computableEntityFilterJob());
+        m_syncRenderViewPreCommandBuildingJob->addDependency(m_renderer->renderableEntityFilterJob());
         m_syncRenderViewPreCommandBuildingJob->addDependency(m_syncRenderViewPostInitializationJob);
 
         if (m_materialGathererCacheNeedsToBeRebuilt)
diff --git a/src/render/jobs/filterentitybycomponentjob_p.h b/src/render/jobs/filterentitybycomponentjob_p.h
index cefcdd296..61b2e7ce4 100644
--- a/src/render/jobs/filterentitybycomponentjob_p.h
+++ b/src/render/jobs/filterentitybycomponentjob_p.h
@@ -80,7 +80,7 @@ public:
     inline void setManager(EntityManager *manager) Q_DECL_NOTHROW { m_manager = manager; }
     inline QVector<Entity *> &filteredEntities() Q_DECL_NOTHROW { return m_filteredEntities; }
 
-    void run() final
+    void run()
     {
         m_filteredEntities.clear();
         const QVector<HEntity> handles = m_manager->activeHandles();
@@ -97,6 +97,9 @@ private:
     QVector<Entity *> m_filteredEntities;
 };
 
+template<typename T, typename ... Ts>
+using FilterEntityByComponentJobPtr = QSharedPointer<FilterEntityByComponentJob<T, Ts...>>;
+
 } // Render
 
 } // Qt3DRender
diff --git a/src/render/jobs/lightgatherer_p.h b/src/render/jobs/lightgatherer_p.h
index 59e64ad74..cc3948f12 100644
--- a/src/render/jobs/lightgatherer_p.h
+++ b/src/render/jobs/lightgatherer_p.h
@@ -71,14 +71,9 @@ public:
 
     inline void setManager(EntityManager *manager) Q_DECL_NOTHROW { m_manager = manager; }
     inline QVector<LightSource> &lights() { return m_lights; }
-    inline EnvironmentLight *takeEnvironmentLight()
-    {
-        auto envLight = m_environmentLight;
-        m_environmentLight = nullptr;
-        return envLight;
-    }
-
-    void run() final;
+    inline EnvironmentLight *environmentLight() const { return m_environmentLight; }
+
+    void run() override;
 
 private:
     EntityManager *m_manager;
diff --git a/tests/auto/render/opengl/renderer/tst_renderer.cpp b/tests/auto/render/opengl/renderer/tst_renderer.cpp
index abb1aee86..3deb9dc6b 100644
--- a/tests/auto/render/opengl/renderer/tst_renderer.cpp
+++ b/tests/auto/render/opengl/renderer/tst_renderer.cpp
@@ -162,11 +162,8 @@ private Q_SLOTS:
                  1 + // BufferGathererJob
                  1 + // TexturesGathererJob
                  1 + // LightGathererJob
-                 1 + // CacheLightJob
                  1 + // RenderableEntityFilterJob
-                 1 + // CacheRenderableEntitiesJob
                  1 + // ComputableEntityFilterJob
-                 1 + // CacheComputableEntitiesJob
                  singleRenderViewJobCount +
                  singleRenderViewCommandRebuildJobCount +
                  layerCacheJobCount +
@@ -260,7 +257,6 @@ private Q_SLOTS:
                  1 + // cleanupJob
                  1 + // VAOGatherer
                  1 + // RenderableEntityFilterPtr
-                 1 + // SyncRenderableEntities
                  singleRenderViewCommandRebuildJobCount +
                  singleRenderViewJobCount);
 
diff --git a/tests/auto/render/opengl/renderviewbuilder/tst_renderviewbuilder.cpp b/tests/auto/render/opengl/renderviewbuilder/tst_renderviewbuilder.cpp
index f0d02d63a..19ab51c8c 100644
--- a/tests/auto/render/opengl/renderviewbuilder/tst_renderviewbuilder.cpp
+++ b/tests/auto/render/opengl/renderviewbuilder/tst_renderviewbuilder.cpp
@@ -344,7 +344,7 @@ private Q_SLOTS:
             QVERIFY(renderViewBuilder.syncRenderViewPreCommandUpdateJob()->dependencies().contains(testAspect.renderer()->introspectShadersJob()));
             QVERIFY(renderViewBuilder.syncRenderViewPreCommandUpdateJob()->dependencies().contains(testAspect.renderer()->bufferGathererJob()));
             QVERIFY(renderViewBuilder.syncRenderViewPreCommandUpdateJob()->dependencies().contains(testAspect.renderer()->textureGathererJob()));
-            QVERIFY(renderViewBuilder.syncRenderViewPreCommandUpdateJob()->dependencies().contains(testAspect.renderer()->cacheLightJob()));
+            QVERIFY(renderViewBuilder.syncRenderViewPreCommandUpdateJob()->dependencies().contains(testAspect.renderer()->lightGathererJob()));
 
             // Step 5
             for (const auto &renderViewBuilderJob : renderViewBuilder.renderViewCommandUpdaterJobs()) {
@@ -471,7 +471,7 @@ private Q_SLOTS:
 
         // THEN
         QCOMPARE(renderer->lightGathererJob()->lights().size(), 2);
-        QVERIFY(renderer->lightGathererJob()->takeEnvironmentLight() != nullptr);
+        QVERIFY(renderer->lightGathererJob()->environmentLight() != nullptr);
     }
 
     void checkRenderableEntitiesFilteringExecution()
@@ -642,7 +642,6 @@ private Q_SLOTS:
         renderViewBuilder.buildJobHierachy();
 
         renderer->renderableEntityFilterJob()->run();
-        renderer->cacheRenderableEntitiesJob()->run();
 
         renderViewBuilder.renderViewJob()->run();
         renderViewBuilder.syncRenderViewPostInitializationJob()->run();