Fix: rebuild material and layer caches when FrameGraph tree is dirty

This could otherwise result in black screens when the FrameGraph
tree is updated and no other change is sent. Screen would remain
black until some other element with dirty changes triggers a cache
rebuild.

Change-Id: Iac43965f06d1d644de14e6a6c8768e035bed69a6
Reviewed-by: Mike Krus <mike.krus@kdab.com>

4 files changed, 92 insertions, 38 deletions

diff --git a/src/render/renderers/opengl/renderer/renderer.cpp b/src/render/renderers/opengl/renderer/renderer.cpp
index 7ed766f8b..4b70710f2 100644
--- a/src/render/renderers/opengl/renderer/renderer.cpp
+++ b/src/render/renderers/opengl/renderer/renderer.cpp
@@ -1596,7 +1596,6 @@ Renderer::ViewSubmissionResultData Renderer::submitRenderViews(const QVector<Ren
     ViewSubmissionResultData resultData;
     resultData.lastBoundFBOId = lastBoundFBOId;
     resultData.surface = lastUsedSurface;
-
     return resultData;
 }
 
@@ -1720,11 +1719,13 @@ QVector<Qt3DCore::QAspectJobPtr> Renderer::renderBinJobs()
     }
 
 
-    // Layer cache is dependent on layers, layer filters and the enabled flag
-    // on entities
+    // Layer cache is dependent on layers, layer filters (hence FG structure
+    // changes) and the enabled flag on entities
+    const bool frameGraphDirty = dirtyBitsForFrame & AbstractRenderer::FrameGraphDirty;
     const bool layersDirty = dirtyBitsForFrame & AbstractRenderer::LayersDirty;
-    const bool layersCacheNeedsToBeRebuilt = layersDirty || entitiesEnabledDirty;
+    const bool layersCacheNeedsToBeRebuilt = layersDirty || entitiesEnabledDirty || frameGraphDirty;
     const bool materialDirty = dirtyBitsForFrame & AbstractRenderer::MaterialDirty;
+    const bool materialCacheNeedsToBeRebuilt = materialDirty || frameGraphDirty;
 
     // Rebuild Entity Layers list if layers are dirty
     if (layersDirty)
@@ -1751,7 +1752,7 @@ QVector<Qt3DCore::QAspectJobPtr> Renderer::renderBinJobs()
         for (int i = 0; i < fgBranchCount; ++i) {
             RenderViewBuilder builder(fgLeaves.at(i), i, this);
             builder.setLayerCacheNeedsToBeRebuilt(layersCacheNeedsToBeRebuilt);
-            builder.setMaterialGathererCacheNeedsToBeRebuilt(materialDirty);
+            builder.setMaterialGathererCacheNeedsToBeRebuilt(materialCacheNeedsToBeRebuilt);
             builder.prepareJobs();
             renderBinJobs.append(builder.buildJobHierachy());
         }
@@ -1762,6 +1763,7 @@ QVector<Qt3DCore::QAspectJobPtr> Renderer::renderBinJobs()
         // FilterLayerEntityJob is part of the RenderViewBuilder jobs and must be run later
         // if none of those jobs are started this frame
         notCleared |= AbstractRenderer::EntityEnabledDirty;
+        notCleared |= AbstractRenderer::FrameGraphDirty;
         notCleared |= AbstractRenderer::LayersDirty;
     }
 
diff --git a/src/render/renderers/opengl/renderer/renderer_p.h b/src/render/renderers/opengl/renderer/renderer_p.h
index 23708e3e8..b89c1e7c0 100644
--- a/src/render/renderers/opengl/renderer/renderer_p.h
+++ b/src/render/renderers/opengl/renderer/renderer_p.h
@@ -185,6 +185,7 @@ public:
     Entity *sceneRoot() const override { return m_renderSceneRoot; }
 
     FrameGraphNode *frameGraphRoot() const override;
+    RenderQueue *renderQueue() const { return m_renderQueue; }
 
     void markDirty(BackendNodeDirtySet changes, BackendNode *node) override;
     BackendNodeDirtySet dirtyBits() override;
diff --git a/tests/auto/render/renderer/tst_renderer.cpp b/tests/auto/render/renderer/tst_renderer.cpp
index 98bf6cc07..9321f5303 100644
--- a/tests/auto/render/renderer/tst_renderer.cpp
+++ b/tests/auto/render/renderer/tst_renderer.cpp
@@ -35,6 +35,7 @@
 #include <Qt3DRender/private/renderview_p.h>
 #include <Qt3DRender/private/renderviewbuilder_p.h>
 #include <Qt3DRender/private/offscreensurfacehelper_p.h>
+#include <Qt3DRender/private/renderqueue_p.h>
 
 class tst_Renderer : public QObject
 {
@@ -49,6 +50,7 @@ private Q_SLOTS:
         // GIVEN
         Qt3DRender::Render::NodeManagers nodeManagers;
         Qt3DRender::Render::Renderer renderer(Qt3DRender::QRenderAspect::Synchronous);
+        Qt3DRender::Render::RenderQueue *renderQueue = renderer.renderQueue();
         Qt3DRender::Render::OffscreenSurfaceHelper offscreenHelper(&renderer);
         Qt3DRender::Render::RenderSettings settings;
         // owned by FG manager
@@ -69,7 +71,13 @@ private Q_SLOTS:
         // NOTE: FilterCompatibleTechniqueJob and ShaderGathererJob cannot run because the context
         // is not initialized in this test
 
-        const int singleRenderViewJobCount = 11 + 1 * Qt3DRender::Render::RenderViewBuilder::optimalJobCount();
+        const int renderViewBuilderMaterialCacheJobCount = 1 + Qt3DRender::Render::RenderViewBuilder::optimalJobCount();
+        // syncMaterialGathererJob
+        // n * materialGathererJob
+        const int layerCacheJobCount = 2;
+        // filterEntityByLayerJob,
+        // syncFilterEntityByLayerJob
+        const int singleRenderViewJobCount = 11 + Qt3DRender::Render::RenderViewBuilder::optimalJobCount();
         // RenderViewBuilder renderViewJob,
         //                   renderableEntityFilterJob,
         //                   lightGatherJob,
@@ -96,6 +104,7 @@ private Q_SLOTS:
                  singleRenderViewJobCount); // Only valid for the first call to renderBinJobs(), since subsequent calls won't have the renderqueue reset
 
         renderer.clearDirtyBits(Qt3DRender::Render::AbstractRenderer::AllDirty);
+        renderQueue->reset();
 
         // WHEN
         renderer.addRenderCaptureSendRequest(Qt3DCore::QNodeId::createId());
@@ -108,7 +117,11 @@ private Q_SLOTS:
                  1 + // sendBufferCaptureJob
                  1 + // sendRenderCaptureJob
                  1 + // VAOGatherer
-                 1); // updateSkinningPaletteJob
+                 1 + // updateSkinningPaletteJob
+                 singleRenderViewJobCount);
+
+        renderer.clearDirtyBits(Qt3DRender::Render::AbstractRenderer::AllDirty);
+        renderQueue->reset();
 
         // WHEN
         renderer.markDirty(Qt3DRender::Render::AbstractRenderer::EntityEnabledDirty, nullptr);
@@ -121,9 +134,12 @@ private Q_SLOTS:
                  1 + // sendBufferCaptureJob
                  1 + // VAOGatherer
                  1 + // updateSkinningPaletteJob
-                 1); // EntityEnabledDirty
+                 1 + // EntityEnabledDirty
+                 singleRenderViewJobCount +
+                 layerCacheJobCount);
 
         renderer.clearDirtyBits(Qt3DRender::Render::AbstractRenderer::AllDirty);
+        renderQueue->reset();
 
         // WHEN
         renderer.markDirty(Qt3DRender::Render::AbstractRenderer::TransformDirty, nullptr);
@@ -139,9 +155,28 @@ private Q_SLOTS:
                  1 + // UpdateWorldBoundingVolume
                  1 + // UpdateShaderDataTransform
                  1 + // updateSkinningPaletteJob
-                 1); // ExpandBoundingVolumeJob
+                 1 + // ExpandBoundingVolumeJob
+                 singleRenderViewJobCount);
+
+        renderer.clearDirtyBits(Qt3DRender::Render::AbstractRenderer::AllDirty);
+        renderQueue->reset();
+
+        // WHEN
+        renderer.markDirty(Qt3DRender::Render::AbstractRenderer::MaterialDirty, nullptr);
+        jobs = renderer.renderBinJobs();
+
+        // THEN (level
+        QCOMPARE(jobs.size(),
+                 1 + // updateLevelOfDetailJob
+                 1 + // cleanupJob
+                 1 + // VAOGatherer
+                 1 + // updateSkinningPaletteJob
+                 1 + // sendBufferCaptureJob
+                 singleRenderViewJobCount +
+                 renderViewBuilderMaterialCacheJobCount);
 
         renderer.clearDirtyBits(Qt3DRender::Render::AbstractRenderer::AllDirty);
+        renderQueue->reset();
 
         // WHEN
         renderer.markDirty(Qt3DRender::Render::AbstractRenderer::GeometryDirty, nullptr);
@@ -156,9 +191,11 @@ private Q_SLOTS:
                  1 + // CalculateBoundingVolumeJob
                  1 + // UpdateMeshTriangleListJob
                  1 + // updateSkinningPaletteJob
-                 1); // ExpandBoundingVolumeJob
+                 1 + // ExpandBoundingVolumeJob
+                 singleRenderViewJobCount);
 
         renderer.clearDirtyBits(Qt3DRender::Render::AbstractRenderer::AllDirty);
+        renderQueue->reset();
 
         // WHEN
         renderer.markDirty(Qt3DRender::Render::AbstractRenderer::BuffersDirty, nullptr);
@@ -173,9 +210,11 @@ private Q_SLOTS:
                  1 + // updateSkinningPaletteJob
                  1 + // CalculateBoundingVolumeJob
                  1 + // UpdateMeshTriangleListJob
-                 1); // BufferGathererJob
+                 1 + // BufferGathererJob
+                 singleRenderViewJobCount);
 
         renderer.clearDirtyBits(Qt3DRender::Render::AbstractRenderer::AllDirty);
+        renderQueue->reset();
 
         // WHEN
         renderer.markDirty(Qt3DRender::Render::AbstractRenderer::TexturesDirty, nullptr);
@@ -189,9 +228,28 @@ private Q_SLOTS:
                  1 + // VAOGatherer
                  1 + // TexturesGathererJob
                  1 + // updateSkinningPaletteJob
-                 1); // SyncTexturesGathererJob
+                 1 + // SyncTexturesGathererJob
+                 singleRenderViewJobCount);
+
+        renderer.clearDirtyBits(Qt3DRender::Render::AbstractRenderer::AllDirty);
+        renderQueue->reset();
+
+        // WHEN
+        renderer.markDirty(Qt3DRender::Render::AbstractRenderer::FrameGraphDirty, nullptr);
+        jobs = renderer.renderBinJobs();
+
+        QCOMPARE(jobs.size(),
+                 1 + // updateLevelOfDetailJob
+                 1 + // cleanupJob
+                 1 + // VAOGatherer
+                 1 + // sendBufferCaptureJob
+                 1 + // updateSkinningPaletteJob
+                 singleRenderViewJobCount +
+                 layerCacheJobCount +
+                 renderViewBuilderMaterialCacheJobCount);
 
         renderer.clearDirtyBits(Qt3DRender::Render::AbstractRenderer::AllDirty);
+        renderQueue->reset();
 
         // WHEN
         renderer.markDirty(Qt3DRender::Render::AbstractRenderer::AllDirty, nullptr);
@@ -215,9 +273,13 @@ private Q_SLOTS:
                  1 + // BufferGathererJob
                  1 + // TexturesGathererJob
                  1 + // SyncTextureLoadingJob
-                 1); // UpdateEntityLayersJob
+                 1 + // UpdateEntityLayersJob
+                 singleRenderViewJobCount +
+                 layerCacheJobCount +
+                 renderViewBuilderMaterialCacheJobCount);
 
         renderer.clearDirtyBits(Qt3DRender::Render::AbstractRenderer::AllDirty);
+        renderQueue->reset();
 
         // Properly shutdown command thread
         renderer.shutdown();
diff --git a/tests/auto/render/renderviewbuilder/tst_renderviewbuilder.cpp b/tests/auto/render/renderviewbuilder/tst_renderviewbuilder.cpp
index 81bca0158..24f56f25b 100644
--- a/tests/auto/render/renderviewbuilder/tst_renderviewbuilder.cpp
+++ b/tests/auto/render/renderviewbuilder/tst_renderviewbuilder.cpp
@@ -294,20 +294,12 @@ private Q_SLOTS:
             QVERIFY(renderViewBuilder.syncFrustumCullingJob()->dependencies().contains(testAspect.renderer()->updateWorldTransformJob()));
             QVERIFY(renderViewBuilder.syncFrustumCullingJob()->dependencies().contains(testAspect.renderer()->updateShaderDataTransformJob()));
 
-            for (const auto materialGatherer : renderViewBuilder.materialGathererJobs()) {
-                QCOMPARE(materialGatherer->dependencies().size(), 3);
-                QVERIFY(materialGatherer->dependencies().contains(testAspect.renderer()->introspectShadersJob()));
-                QVERIFY(materialGatherer->dependencies().contains(renderViewBuilder.syncRenderViewInitializationJob()));
-                QVERIFY(materialGatherer->dependencies().contains(testAspect.renderer()->filterCompatibleTechniqueJob()));
-            }
-
             // Step 4
             QCOMPARE(renderViewBuilder.frustumCullingJob()->dependencies().size(), 2);
             QVERIFY(renderViewBuilder.frustumCullingJob()->dependencies().contains(renderViewBuilder.syncFrustumCullingJob()));
             QVERIFY(renderViewBuilder.frustumCullingJob()->dependencies().contains(testAspect.renderer()->expandBoundingVolumeJob()));
 
 
-            QCOMPARE(renderViewBuilder.syncRenderCommandBuildingJob()->dependencies().size(), renderViewBuilder.materialGathererJobs().size() + 9);
             QVERIFY(renderViewBuilder.syncRenderCommandBuildingJob()->dependencies().contains(renderViewBuilder.syncRenderViewInitializationJob()));
             QVERIFY(renderViewBuilder.syncRenderCommandBuildingJob()->dependencies().contains(renderViewBuilder.renderableEntityFilterJob()));
             QVERIFY(renderViewBuilder.syncRenderCommandBuildingJob()->dependencies().contains(renderViewBuilder.computableEntityFilterJob()));
@@ -317,19 +309,16 @@ private Q_SLOTS:
             QVERIFY(renderViewBuilder.syncRenderCommandBuildingJob()->dependencies().contains(testAspect.renderer()->introspectShadersJob()));
             QVERIFY(renderViewBuilder.syncRenderCommandBuildingJob()->dependencies().contains(testAspect.renderer()->bufferGathererJob()));
             QVERIFY(renderViewBuilder.syncRenderCommandBuildingJob()->dependencies().contains(testAspect.renderer()->textureGathererJob()));
-            for (const auto materialGatherer : renderViewBuilder.materialGathererJobs()) {
-                QVERIFY(renderViewBuilder.syncRenderCommandBuildingJob()->dependencies().contains(materialGatherer));
-            }
 
             // Step 5
-            for (const auto renderViewBuilderJob : renderViewBuilder.renderViewBuilderJobs()) {
+            for (const auto &renderViewBuilderJob : renderViewBuilder.renderViewBuilderJobs()) {
                 QCOMPARE(renderViewBuilderJob->dependencies().size(), 1);
                 QCOMPARE(renderViewBuilderJob->dependencies().first().data(), renderViewBuilder.syncRenderCommandBuildingJob().data());
             }
 
             // Step 6
             QCOMPARE(renderViewBuilder.syncRenderViewCommandBuildersJob()->dependencies().size(), renderViewBuilder.renderViewBuilderJobs().size());
-            for (const auto renderViewBuilderJob : renderViewBuilder.renderViewBuilderJobs()) {
+            for (const auto &renderViewBuilderJob : renderViewBuilder.renderViewBuilderJobs()) {
                 QVERIFY(renderViewBuilder.syncRenderViewCommandBuildersJob()->dependencies().contains(renderViewBuilderJob));
             }
         }
@@ -337,6 +326,7 @@ private Q_SLOTS:
             // WHEN
             Qt3DRender::Render::RenderViewBuilder renderViewBuilder(leafNode, 0, testAspect.renderer());
             renderViewBuilder.setLayerCacheNeedsToBeRebuilt(true);
+            renderViewBuilder.setMaterialGathererCacheNeedsToBeRebuilt(true);
             renderViewBuilder.prepareJobs();
             renderViewBuilder.buildJobHierachy();
 
@@ -372,9 +362,10 @@ private Q_SLOTS:
             QVERIFY(renderViewBuilder.syncFrustumCullingJob()->dependencies().contains(testAspect.renderer()->updateWorldTransformJob()));
             QVERIFY(renderViewBuilder.syncFrustumCullingJob()->dependencies().contains(testAspect.renderer()->updateShaderDataTransformJob()));
 
-            for (const auto materialGatherer : renderViewBuilder.materialGathererJobs()) {
-                QCOMPARE(materialGatherer->dependencies().size(), 2);
+            for (const auto &materialGatherer : renderViewBuilder.materialGathererJobs()) {
+                QCOMPARE(materialGatherer->dependencies().size(), 3);
                 QVERIFY(materialGatherer->dependencies().contains(renderViewBuilder.syncRenderViewInitializationJob()));
+                QVERIFY(materialGatherer->dependencies().contains(testAspect.renderer()->introspectShadersJob()));
                 QVERIFY(materialGatherer->dependencies().contains(testAspect.renderer()->filterCompatibleTechniqueJob()));
             }
 
@@ -383,7 +374,8 @@ private Q_SLOTS:
             QVERIFY(renderViewBuilder.frustumCullingJob()->dependencies().contains(renderViewBuilder.syncFrustumCullingJob()));
             QVERIFY(renderViewBuilder.frustumCullingJob()->dependencies().contains(testAspect.renderer()->expandBoundingVolumeJob()));
 
-            QCOMPARE(renderViewBuilder.syncRenderCommandBuildingJob()->dependencies().size(), renderViewBuilder.materialGathererJobs().size() + 10);
+            QCOMPARE(renderViewBuilder.syncMaterialGathererJob()->dependencies().size(), renderViewBuilder.materialGathererJobs().size());
+            QVERIFY(renderViewBuilder.syncRenderCommandBuildingJob()->dependencies().contains(renderViewBuilder.syncMaterialGathererJob()));
             QVERIFY(renderViewBuilder.syncRenderCommandBuildingJob()->dependencies().contains(renderViewBuilder.syncRenderViewInitializationJob()));
             QVERIFY(renderViewBuilder.syncRenderCommandBuildingJob()->dependencies().contains(renderViewBuilder.renderableEntityFilterJob()));
             QVERIFY(renderViewBuilder.syncRenderCommandBuildingJob()->dependencies().contains(renderViewBuilder.computableEntityFilterJob()));
@@ -394,19 +386,16 @@ private Q_SLOTS:
             QVERIFY(renderViewBuilder.syncRenderCommandBuildingJob()->dependencies().contains(testAspect.renderer()->introspectShadersJob()));
             QVERIFY(renderViewBuilder.syncRenderCommandBuildingJob()->dependencies().contains(testAspect.renderer()->bufferGathererJob()));
             QVERIFY(renderViewBuilder.syncRenderCommandBuildingJob()->dependencies().contains(testAspect.renderer()->textureGathererJob()));
-            for (const auto materialGatherer : renderViewBuilder.materialGathererJobs()) {
-                QVERIFY(renderViewBuilder.syncRenderCommandBuildingJob()->dependencies().contains(materialGatherer));
-            }
 
             // Step 5
-            for (const auto renderViewBuilderJob : renderViewBuilder.renderViewBuilderJobs()) {
+            for (const auto &renderViewBuilderJob : renderViewBuilder.renderViewBuilderJobs()) {
                 QCOMPARE(renderViewBuilderJob->dependencies().size(), 1);
                 QCOMPARE(renderViewBuilderJob->dependencies().first().data(), renderViewBuilder.syncRenderCommandBuildingJob().data());
             }
 
             // Step 6
             QCOMPARE(renderViewBuilder.syncRenderViewCommandBuildersJob()->dependencies().size(), renderViewBuilder.renderViewBuilderJobs().size());
-            for (const auto renderViewBuilderJob : renderViewBuilder.renderViewBuilderJobs()) {
+            for (const auto &renderViewBuilderJob : renderViewBuilder.renderViewBuilderJobs()) {
                 QVERIFY(renderViewBuilder.syncRenderViewCommandBuildersJob()->dependencies().contains(renderViewBuilderJob));
             }
         }
@@ -501,7 +490,7 @@ private Q_SLOTS:
 
             // THEN
             QCOMPARE(renderViewBuilder.frustumCullingJob()->isActive(), false);
-            for (const auto materialGatherer : renderViewBuilder.materialGathererJobs()) {
+            for (const auto &materialGatherer : renderViewBuilder.materialGathererJobs()) {
                 QVERIFY(materialGatherer->techniqueFilter() == nullptr);
                 QVERIFY(materialGatherer->renderPassFilter() == nullptr);
             }
@@ -512,7 +501,7 @@ private Q_SLOTS:
 
             // THEN
             QCOMPARE(renderViewBuilder.frustumCullingJob()->isActive(), true);
-            for (const auto materialGatherer : renderViewBuilder.materialGathererJobs()) {
+            for (const auto &materialGatherer : renderViewBuilder.materialGathererJobs()) {
                 QVERIFY(materialGatherer->techniqueFilter() != nullptr);
                 QVERIFY(materialGatherer->renderPassFilter() != nullptr);
             }
@@ -528,7 +517,7 @@ private Q_SLOTS:
             QCOMPARE(renderViewBuilder.frustumCullingJob()->isActive(), false);
             QCOMPARE(renderViewBuilder.filterEntityByLayerJob()->hasLayerFilter(), false);
             QCOMPARE(renderViewBuilder.filterEntityByLayerJob()->layerFilters().size(), 0);
-            for (const auto materialGatherer : renderViewBuilder.materialGathererJobs()) {
+            for (const auto &materialGatherer : renderViewBuilder.materialGathererJobs()) {
                 QVERIFY(materialGatherer->techniqueFilter() == nullptr);
                 QVERIFY(materialGatherer->renderPassFilter() == nullptr);
             }
@@ -541,7 +530,7 @@ private Q_SLOTS:
             QCOMPARE(renderViewBuilder.frustumCullingJob()->isActive(), true);
             QCOMPARE(renderViewBuilder.filterEntityByLayerJob()->hasLayerFilter(), true);
             QCOMPARE(renderViewBuilder.filterEntityByLayerJob()->layerFilters().size(), 1);
-            for (const auto materialGatherer : renderViewBuilder.materialGathererJobs()) {
+            for (const auto &materialGatherer : renderViewBuilder.materialGathererJobs()) {
                 QVERIFY(materialGatherer->techniqueFilter() != nullptr);
                 QVERIFY(materialGatherer->renderPassFilter() != nullptr);
             }