Properly order texture jobs

1) Load all the texture data
2) Sync
3) Look for dirty textures

We want to wait for textures to have been loaded to look for the dirty
textures otherwise the texture may not be ready and we would render some
invalid texture.

Task-number: QTBUG-56466
Task-number: QTBUG-57509
Change-Id: I2326f81bfe51dcdc753ff4eba150ce4cfae1635f
Reviewed-by: Sean Harmer <sean.harmer@kdab.com>

5 files changed, 25 insertions, 1 deletions

diff --git a/src/render/backend/abstractrenderer_p.h b/src/render/backend/abstractrenderer_p.h
index cb8b7c621..6edebf854 100644
--- a/src/render/backend/abstractrenderer_p.h
+++ b/src/render/backend/abstractrenderer_p.h
@@ -135,6 +135,7 @@ public:
 
     virtual QVector<Qt3DCore::QAspectJobPtr> renderBinJobs() = 0;
     virtual Qt3DCore::QAspectJobPtr pickBoundingVolumeJob() = 0;
+    virtual Qt3DCore::QAspectJobPtr syncTextureLoadingJob() = 0;
 
     virtual void setSceneRoot(Qt3DCore::QBackendNodeFactory *factory, Entity *root) = 0;
 
diff --git a/src/render/backend/renderer.cpp b/src/render/backend/renderer.cpp
index a0195a09c..af87ecd86 100644
--- a/src/render/backend/renderer.cpp
+++ b/src/render/backend/renderer.cpp
@@ -167,6 +167,7 @@ Renderer::Renderer(QRenderAspect::RenderType type)
     , m_bufferGathererJob(Render::GenericLambdaJobPtr<std::function<void ()>>::create([this] { lookForDirtyBuffers(); }, JobTypes::DirtyBufferGathering))
     , m_textureGathererJob(Render::GenericLambdaJobPtr<std::function<void ()>>::create([this] { lookForDirtyTextures(); }, JobTypes::DirtyTextureGathering))
     , m_shaderGathererJob(Render::GenericLambdaJobPtr<std::function<void ()>>::create([this] { lookForDirtyShaders(); }, JobTypes::DirtyShaderGathering))
+    , m_syncTextureLoadingJob(Render::GenericLambdaJobPtr<std::function<void ()>>::create([] {}, JobTypes::SyncTextureLoading))
     , m_ownedContext(false)
     #ifdef QT3D_JOBS_RUN_STATS
     , m_commandExecuter(new Qt3DRender::Debug::CommandExecuter(this))
@@ -185,6 +186,10 @@ Renderer::Renderer(QRenderAspect::RenderType type)
     m_expandBoundingVolumeJob->addDependency(m_updateWorldBoundingVolumeJob);
     m_updateShaderDataTransformJob->addDependency(m_worldTransformJob);
 
+    // Dirty texture gathering depends on m_syncTextureLoadingJob
+    // m_syncTextureLoadingJob will depend on the texture loading jobs
+    m_textureGathererJob->addDependency(m_syncTextureLoadingJob);
+
     // All world stuff depends on the RenderEntity's localBoundingVolume
     m_pickBoundingVolumeJob->addDependency(m_updateMeshTriangleListJob);
 
@@ -1172,6 +1177,7 @@ QVector<Qt3DCore::QAspectJobPtr> Renderer::renderBinJobs()
     renderBinJobs.append(bufferJobs);
 
     // Jobs to prepare GL Resource upload
+    renderBinJobs.push_back(m_syncTextureLoadingJob);
     renderBinJobs.push_back(m_bufferGathererJob);
     renderBinJobs.push_back(m_textureGathererJob);
     renderBinJobs.push_back(m_shaderGathererJob);
@@ -1187,6 +1193,11 @@ QAspectJobPtr Renderer::pickBoundingVolumeJob()
     return m_pickBoundingVolumeJob;
 }
 
+QAspectJobPtr Renderer::syncTextureLoadingJob()
+{
+    return m_syncTextureLoadingJob;
+}
+
 QAbstractFrameAdvanceService *Renderer::frameAdvanceService() const
 {
     return static_cast<Qt3DCore::QAbstractFrameAdvanceService *>(m_vsyncFrameAdvanceService.data());
diff --git a/src/render/backend/renderer_p.h b/src/render/backend/renderer_p.h
index 965bccbcf..ad86fc3ce 100644
--- a/src/render/backend/renderer_p.h
+++ b/src/render/backend/renderer_p.h
@@ -136,6 +136,8 @@ class VSyncFrameAdvanceService;
 class PickEventFilter;
 class NodeManagers;
 
+using SynchronizerJobPtr = GenericLambdaJobPtr<std::function<void()>>;
+
 class QT3DRENDERSHARED_PRIVATE_EXPORT Renderer : public AbstractRenderer
 {
 public:
@@ -180,6 +182,8 @@ public:
 
     QVector<Qt3DCore::QAspectJobPtr> renderBinJobs() Q_DECL_OVERRIDE;
     Qt3DCore::QAspectJobPtr pickBoundingVolumeJob() Q_DECL_OVERRIDE;
+    Qt3DCore::QAspectJobPtr syncTextureLoadingJob() Q_DECL_OVERRIDE;
+
     QVector<Qt3DCore::QAspectJobPtr> createRenderBufferJobs() const;
 
     inline FrameCleanupJobPtr frameCleanupJob() const { return m_cleanupJob; }
@@ -189,6 +193,7 @@ public:
     inline UpdateWorldTransformJobPtr updateWorldTransformJob() const { return m_worldTransformJob; }
     inline UpdateWorldBoundingVolumeJobPtr updateWorldBoundingVolumeJob() const { return m_updateWorldBoundingVolumeJob; }
     inline UpdateMeshTriangleListJobPtr updateMeshTriangleListJob() const { return m_updateMeshTriangleListJob; }
+    inline SynchronizerJobPtr textureLoadSyncJob() const { return m_syncTextureLoadingJob; }
 
     Qt3DCore::QAbstractFrameAdvanceService *frameAdvanceService() const Q_DECL_OVERRIDE;
 
@@ -311,6 +316,8 @@ private:
     GenericLambdaJobPtr<std::function<void ()>> m_textureGathererJob;
     GenericLambdaJobPtr<std::function<void ()>> m_shaderGathererJob;
 
+    SynchronizerJobPtr m_syncTextureLoadingJob;
+
     void lookForDirtyBuffers();
     void lookForDirtyTextures();
     void lookForDirtyShaders();
diff --git a/src/render/frontend/qrenderaspect.cpp b/src/render/frontend/qrenderaspect.cpp
index 7e4367879..4ba5c208a 100644
--- a/src/render/frontend/qrenderaspect.cpp
+++ b/src/render/frontend/qrenderaspect.cpp
@@ -351,7 +351,7 @@ QVector<Qt3DCore::QAspectJobPtr> QRenderAspect::jobsToExecute(qint64 time)
     // Create jobs that will get exectued by the threadpool
     QVector<QAspectJobPtr> jobs;
 
-    // 1 LoadBufferJobs, GeometryJobs, SceneLoaderJobs
+    // 1 LoadBufferJobs, GeometryJobs, SceneLoaderJobs, LoadTextureJobs
     // 2 CalculateBoundingVolumeJob (depends on LoadBuffer)
     // 3 WorldTransformJob
     // 4 UpdateBoundingVolume, FramePreparationJob (depend on WorlTransformJob)
@@ -371,17 +371,21 @@ QVector<Qt3DCore::QAspectJobPtr> QRenderAspect::jobsToExecute(qint64 time)
         }
 
         Render::NodeManagers *manager = d->m_renderer->nodeManagers();
+        QAspectJobPtr textureLoadingSync = d->m_renderer->syncTextureLoadingJob();
+        textureLoadingSync->removeDependency(QWeakPointer<QAspectJob>());
 
         // Launch texture generator jobs
         const QVector<QTextureImageDataGeneratorPtr> pendingImgGen = manager->textureImageDataManager()->pendingGenerators();
         for (const QTextureImageDataGeneratorPtr &imgGen : pendingImgGen) {
             auto loadTextureJob = Render::LoadTextureDataJobPtr::create(imgGen);
+            textureLoadingSync->addDependency(loadTextureJob);
             loadTextureJob->setNodeManagers(manager);
             jobs.append(loadTextureJob);
         }
         const QVector<QTextureGeneratorPtr> pendingTexGen = manager->textureDataManager()->pendingGenerators();
         for (const QTextureGeneratorPtr &texGen : pendingTexGen) {
             auto loadTextureJob = Render::LoadTextureDataJobPtr::create(texGen);
+            textureLoadingSync->addDependency(loadTextureJob);
             loadTextureJob->setNodeManagers(manager);
             jobs.append(loadTextureJob);
         }
diff --git a/tests/auto/render/commons/testrenderer.h b/tests/auto/render/commons/testrenderer.h
index 48588ab99..05aafbbd8 100644
--- a/tests/auto/render/commons/testrenderer.h
+++ b/tests/auto/render/commons/testrenderer.h
@@ -59,6 +59,7 @@ public:
     void skipNextFrame() Q_DECL_OVERRIDE {}
     QVector<Qt3DCore::QAspectJobPtr> renderBinJobs() Q_DECL_OVERRIDE { return QVector<Qt3DCore::QAspectJobPtr>(); }
     Qt3DCore::QAspectJobPtr pickBoundingVolumeJob() Q_DECL_OVERRIDE { return Qt3DCore::QAspectJobPtr(); }
+    Qt3DCore::QAspectJobPtr syncTextureLoadingJob() Q_DECL_OVERRIDE { return Qt3DCore::QAspectJobPtr(); }
     void setSceneRoot(Qt3DCore::QBackendNodeFactory *factory, Qt3DRender::Render::Entity *root) Q_DECL_OVERRIDE { Q_UNUSED(factory);  Q_UNUSED(root); }
     Qt3DRender::Render::Entity *sceneRoot() const Q_DECL_OVERRIDE { return nullptr; }
     Qt3DRender::Render::FrameGraphNode *frameGraphRoot() const Q_DECL_OVERRIDE { return nullptr; }