Clean up tracing code

- Removed some old optional timing code.
- Separated jobsDone into 2 parts since, on single threaded rendering
  mode (ie macOS), the render aspect blocks until the next vsync which
  confuses tracing

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

12 files changed, 33 insertions, 27 deletions

diff --git a/src/core/aspects/aspects.pri b/src/core/aspects/aspects.pri
index 068f74389..268a2fc37 100644
--- a/src/core/aspects/aspects.pri
+++ b/src/core/aspects/aspects.pri
@@ -1,5 +1,3 @@
-#DEFINES += QT3D_CORE_JOB_TIMING
-
 SOURCES += \
         $$PWD/qabstractaspect.cpp \
         $$PWD/qaspectengine.cpp \
diff --git a/src/core/aspects/qabstractaspect.cpp b/src/core/aspects/qabstractaspect.cpp
index a2aed732b..824434ce6 100644
--- a/src/core/aspects/qabstractaspect.cpp
+++ b/src/core/aspects/qabstractaspect.cpp
@@ -508,9 +508,12 @@ QVector<QAspectJobPtr> QAbstractAspectPrivate::jobsToExecute(qint64 time)
     return res;
 }
 
-void QAbstractAspectPrivate::jobsDone(QAspectManager *manager)
+void QAbstractAspectPrivate::jobsDone()
+{
+}
+
+void QAbstractAspectPrivate::frameDone()
 {
-    Q_UNUSED(manager)
 }
 
 /*!
diff --git a/src/core/aspects/qabstractaspect_p.h b/src/core/aspects/qabstractaspect_p.h
index 9435f2bc6..dd13ac5bf 100644
--- a/src/core/aspects/qabstractaspect_p.h
+++ b/src/core/aspects/qabstractaspect_p.h
@@ -127,7 +127,8 @@ public:
     QAbstractAspectJobManager *jobManager() const;
 
     QVector<QAspectJobPtr> jobsToExecute(qint64 time) override;
-    void jobsDone(Qt3DCore::QAspectManager *manager) override;
+    void jobsDone() override;      // called when all the jobs are completed
+    void frameDone() override;     // called when frame is completed (after the jobs), safe to wait until next frame here
 
     QBackendNode *createBackendNode(const NodeTreeChange &change) const;
     void clearBackendNode(const NodeTreeChange &change) const;
diff --git a/src/core/aspects/qaspectmanager.cpp b/src/core/aspects/qaspectmanager.cpp
index 662860db2..f0dc366b2 100644
--- a/src/core/aspects/qaspectmanager.cpp
+++ b/src/core/aspects/qaspectmanager.cpp
@@ -68,10 +68,6 @@
 #include <QtCore/QCoreApplication>
 #include <QtCore/QAbstractAnimation>
 
-#if defined(QT3D_CORE_JOB_TIMING)
-#include <QElapsedTimer>
-#endif
-
 QT_BEGIN_NAMESPACE
 
 namespace Qt3DCore {
@@ -496,7 +492,7 @@ void QAspectManager::processFrame()
     // without any such data race.
     {
         // scope for QTaskLogger
-        QTaskLogger logger(m_serviceLocator->systemInformation(), 4096, 0);
+        QTaskLogger logger(m_serviceLocator->systemInformation(), 4096, 0, QTaskLogger::AspectJob);
 
         // Tell the NodePostConstructorInit to process any pending nodes which will add them to our list of
         // tree changes
@@ -541,14 +537,11 @@ void QAspectManager::processFrame()
     // For each Aspect
     // Ask them to launch set of jobs for the current frame
     // Updates matrices, bounding volumes, render bins ...
-#if defined(QT3D_CORE_JOB_TIMING)
-    QElapsedTimer timer;
-    timer.start();
-#endif
     m_jobsInLastFrame = m_scheduler->scheduleAndWaitForFrameAspectJobs(t);
-#if defined(QT3D_CORE_JOB_TIMING)
-    qDebug() << "Jobs took" << timer.nsecsElapsed() / 1.0e6;
-#endif
+
+    // Tell the aspect the frame is complete (except rendering)
+    for (QAbstractAspect *aspect : qAsConst(m_aspects))
+        QAbstractAspectPrivate::get(aspect)->frameDone();
 }
 
 } // namespace Qt3DCore
diff --git a/src/core/jobs/qaspectjobproviderinterface_p.h b/src/core/jobs/qaspectjobproviderinterface_p.h
index b521fea68..bb2c5ed88 100644
--- a/src/core/jobs/qaspectjobproviderinterface_p.h
+++ b/src/core/jobs/qaspectjobproviderinterface_p.h
@@ -61,7 +61,6 @@ QT_BEGIN_NAMESPACE
 namespace Qt3DCore {
 
 class QScheduler;
-class QAspectManager;
 
 class QAspectJobProviderInterface
 {
@@ -70,7 +69,8 @@ public:
 
 private:
     virtual QVector<QAspectJobPtr> jobsToExecute(qint64 time) = 0;
-    virtual void jobsDone(QAspectManager *manager) = 0;
+    virtual void jobsDone() = 0;
+    virtual void frameDone() = 0;
 
     friend class QScheduler;
 };
diff --git a/src/core/qscheduler.cpp b/src/core/qscheduler.cpp
index 2294976ca..b371a5fb1 100644
--- a/src/core/qscheduler.cpp
+++ b/src/core/qscheduler.cpp
@@ -94,13 +94,10 @@ int QScheduler::scheduleAndWaitForFrameAspectJobs(qint64 time)
     m_aspectManager->jobManager()->waitForAllJobs();
 
     {
-        QTaskLogger logger(m_aspectManager->serviceLocator()->systemInformation(), 4097, 0);
+        QTaskLogger logger(m_aspectManager->serviceLocator()->systemInformation(), 4097, 0, QTaskLogger::AspectJob);
 
         for (auto &job : qAsConst(jobQueue))
             QAspectJobPrivate::get(job.data())->postFrame(m_aspectManager);
-
-        for (QAbstractAspect *aspect : aspects)
-            QAbstractAspectPrivate::get(aspect)->jobsDone(m_aspectManager);
     }
 
     return jobQueue.size();
diff --git a/src/render/backend/abstractrenderer_p.h b/src/render/backend/abstractrenderer_p.h
index 8bbfcd548..74457e79f 100644
--- a/src/render/backend/abstractrenderer_p.h
+++ b/src/render/backend/abstractrenderer_p.h
@@ -126,6 +126,7 @@ public:
 
     virtual qint64 time() const = 0;
     virtual void setTime(qint64 time) = 0;
+    virtual void setJobsInLastFrame(int jobsInLastFrame) = 0;
 
     virtual void setNodeManagers(NodeManagers *managers) = 0;
     virtual void setServices(Qt3DCore::QServiceLocator *services) = 0;
diff --git a/src/render/frontend/qrenderaspect.cpp b/src/render/frontend/qrenderaspect.cpp
index a65f66fd7..1ce9f172f 100644
--- a/src/render/frontend/qrenderaspect.cpp
+++ b/src/render/frontend/qrenderaspect.cpp
@@ -173,6 +173,7 @@
 #include <Qt3DCore/private/qservicelocator_p.h>
 #include <Qt3DCore/private/qscene_p.h>
 #include <Qt3DCore/private/qentity_p.h>
+#include <Qt3DCore/private/qaspectmanager_p.h>
 
 #include <QThread>
 #include <QOpenGLContext>
@@ -246,9 +247,14 @@ void QRenderAspectPrivate::syncDirtyFrontEndNode(QNode *node, QBackendNode *back
     renderBackend->syncFromFrontEnd(node, firstTime);
 }
 
-void QRenderAspectPrivate::jobsDone(QAspectManager *manager)
+void QRenderAspectPrivate::jobsDone()
 {
-    m_renderer->jobsDone(manager);
+    m_renderer->jobsDone(m_aspectManager);
+}
+
+void QRenderAspectPrivate::frameDone()
+{
+    m_renderer->setJobsInLastFrame(m_aspectManager->jobsInLastFrame());
     if (m_renderAfterJobs)
         m_renderer->doRender(true);
 }
diff --git a/src/render/frontend/qrenderaspect_p.h b/src/render/frontend/qrenderaspect_p.h
index 8723180ab..0a9f4ec46 100644
--- a/src/render/frontend/qrenderaspect_p.h
+++ b/src/render/frontend/qrenderaspect_p.h
@@ -87,7 +87,8 @@ public:
     static QRenderAspectPrivate* findPrivate(Qt3DCore::QAspectEngine *engine);
 
     void syncDirtyFrontEndNode(Qt3DCore::QNode *node, Qt3DCore::QBackendNode *backend, bool firstTime) const override;
-    void jobsDone(Qt3DCore::QAspectManager *manager) override;
+    void jobsDone() override;
+    void frameDone() override;
 
     void registerBackendTypes();
     void unregisterBackendTypes();
diff --git a/src/render/renderers/opengl/renderer/renderer.cpp b/src/render/renderers/opengl/renderer/renderer.cpp
index a65e3c7ff..c0549a8fd 100644
--- a/src/render/renderers/opengl/renderer/renderer.cpp
+++ b/src/render/renderers/opengl/renderer/renderer.cpp
@@ -368,6 +368,11 @@ void Renderer::setTime(qint64 time)
     m_time = time;
 }
 
+void Renderer::setJobsInLastFrame(int jobsInLastFrame)
+{
+    m_jobsInLastFrame = jobsInLastFrame;
+}
+
 void Renderer::setNodeManagers(NodeManagers *managers)
 {
     m_nodesManager = managers;
@@ -1771,7 +1776,6 @@ void Renderer::skipNextFrame()
 void Renderer::jobsDone(Qt3DCore::QAspectManager *manager)
 {
     // called in main thread once all jobs are done running
-    m_jobsInLastFrame = manager->jobsInLastFrame();
 
     // sync captured renders to frontend
     const QVector<Qt3DCore::QNodeId> pendingCaptureIds = std::move(m_pendingRenderCaptureSendRequests);
diff --git a/src/render/renderers/opengl/renderer/renderer_p.h b/src/render/renderers/opengl/renderer/renderer_p.h
index 48e4a5e3d..4b8574252 100644
--- a/src/render/renderers/opengl/renderer/renderer_p.h
+++ b/src/render/renderers/opengl/renderer/renderer_p.h
@@ -182,6 +182,7 @@ public:
 
     qint64 time() const override;
     void setTime(qint64 time) override;
+    void setJobsInLastFrame(int jobsInLastFrame) override;
 
     void setNodeManagers(NodeManagers *managers) override;
     void setServices(Qt3DCore::QServiceLocator *services) override;
diff --git a/tests/auto/render/commons/testrenderer.h b/tests/auto/render/commons/testrenderer.h
index 05ef4c130..1241bb0e0 100644
--- a/tests/auto/render/commons/testrenderer.h
+++ b/tests/auto/render/commons/testrenderer.h
@@ -43,6 +43,7 @@ public:
     API api() const override { return AbstractRenderer::OpenGL; }
     qint64 time() const override { return 0; }
     void setTime(qint64 time) override { Q_UNUSED(time) }
+    void setJobsInLastFrame(int jobsInLastFrame) override { Q_UNUSED(jobsInLastFrame) }
     void setNodeManagers(Qt3DRender::Render::NodeManagers *m) override { m_managers = m; }
     void setServices(Qt3DCore::QServiceLocator *services) override { Q_UNUSED(services) }
     void setSurfaceExposed(bool exposed) override { Q_UNUSED(exposed) }