Convert LoadGeometryJob to use direct sync

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

9 files changed, 51 insertions, 66 deletions

diff --git a/src/render/geometry/geometryrenderer.cpp b/src/render/geometry/geometryrenderer.cpp
index d885c4192..3b460f48c 100644
--- a/src/render/geometry/geometryrenderer.cpp
+++ b/src/render/geometry/geometryrenderer.cpp
@@ -146,7 +146,7 @@ void GeometryRenderer::syncFromFrontEnd(const QNode *frontEnd, bool firstTime)
     markDirty(AbstractRenderer::GeometryDirty);
 }
 
-void GeometryRenderer::executeFunctor()
+GeometryFunctorResult GeometryRenderer::executeFunctor()
 {
     Q_ASSERT(m_geometryFactory);
 
@@ -167,7 +167,8 @@ void GeometryRenderer::executeFunctor()
     }
 
     // Load geometry
-    std::unique_ptr<QGeometry> geometry((*m_geometryFactory)());
+    QGeometry *geometry = (*m_geometryFactory)();
+    QMesh::Status meshLoaderStatus = QMesh::None;
 
     // If the geometry is null, then we were either unable to load it (Error)
     // or the mesh is located at a remote url and needs to be downloaded first (Loading)
@@ -176,24 +177,15 @@ void GeometryRenderer::executeFunctor()
         // corresponding QGeometryRenderer
         const auto appThread = QCoreApplication::instance()->thread();
         geometry->moveToThread(appThread);
-
-        auto e = QGeometryChangePtr::create(peerId());
-        e->setDeliveryFlags(Qt3DCore::QSceneChange::Nodes);
-        e->setPropertyName("geometry");
-        e->data = std::move(geometry);
-        notifyObservers(e);
     }
 
     // Send Status
     if (isQMeshFunctor) {
         QSharedPointer<MeshLoaderFunctor> meshLoader = qSharedPointerCast<MeshLoaderFunctor>(m_geometryFactory);
-
-        auto e = QPropertyUpdatedChangePtr::create(peerId());
-        e->setDeliveryFlags(Qt3DCore::QSceneChange::Nodes);
-        e->setPropertyName("status");
-        e->setValue(meshLoader->status());
-        notifyObservers(e);
+        meshLoaderStatus = meshLoader->status();
     }
+
+    return { geometry, meshLoaderStatus };
 }
 
 void GeometryRenderer::unsetDirty()
diff --git a/src/render/geometry/geometryrenderer_p.h b/src/render/geometry/geometryrenderer_p.h
index da538a00d..d2ddad4bb 100644
--- a/src/render/geometry/geometryrenderer_p.h
+++ b/src/render/geometry/geometryrenderer_p.h
@@ -54,6 +54,7 @@
 #include <Qt3DRender/private/backendnode_p.h>
 #include <Qt3DRender/qgeometryrenderer.h>
 #include <Qt3DRender/qgeometryfactory.h>
+#include <Qt3DRender/qmesh.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -66,6 +67,12 @@ namespace Render {
 
 class GeometryRendererManager;
 
+struct GeometryFunctorResult
+{
+    QGeometry *geometry;
+    QMesh::Status status;
+};
+
 class Q_AUTOTEST_EXPORT GeometryRenderer : public BackendNode
 {
 public:
@@ -75,7 +82,7 @@ public:
     void cleanup();
     void setManager(GeometryRendererManager *manager);
     void syncFromFrontEnd(const Qt3DCore::QNode *frontEnd, bool firstTime) override;
-    void executeFunctor();
+    GeometryFunctorResult executeFunctor();
 
     inline Qt3DCore::QNodeId geometryId() const { return m_geometryId; }
     inline int instanceCount() const { return m_instanceCount; }
diff --git a/src/render/geometry/qgeometryrenderer.cpp b/src/render/geometry/qgeometryrenderer.cpp
index 5ffbb5e6f..6e0a24f61 100644
--- a/src/render/geometry/qgeometryrenderer.cpp
+++ b/src/render/geometry/qgeometryrenderer.cpp
@@ -480,19 +480,6 @@ void QGeometryRenderer::setGeometryFactory(const QGeometryFactoryPtr &factory)
     d->update();
 }
 
-/*!
-    \internal
- */
-void QGeometryRenderer::sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e)
-{
-    auto change = qSharedPointerCast<QStaticPropertyUpdatedChangeBase>(e);
-    if (change->type() == PropertyUpdated && change->propertyName() == QByteArrayLiteral("geometry")) {
-        auto typedChange = qSharedPointerCast<QGeometryChange>(e);
-        auto geometry = std::move(typedChange->data);
-        setGeometry(geometry.release());
-    }
-}
-
 Qt3DCore::QNodeCreatedChangeBasePtr QGeometryRenderer::createNodeCreationChange() const
 {
     auto creationChange = Qt3DCore::QNodeCreatedChangePtr<QGeometryRendererData>::create(this);
diff --git a/src/render/geometry/qgeometryrenderer.h b/src/render/geometry/qgeometryrenderer.h
index 81bd1ff67..eceeb9173 100644
--- a/src/render/geometry/qgeometryrenderer.h
+++ b/src/render/geometry/qgeometryrenderer.h
@@ -134,7 +134,6 @@ Q_SIGNALS:
 
 protected:
     explicit QGeometryRenderer(QGeometryRendererPrivate &dd, Qt3DCore::QNode *parent = nullptr);
-    void sceneChangeEvent(const Qt3DCore::QSceneChangePtr &change) override;
 
 private:
     Q_DECLARE_PRIVATE(QGeometryRenderer)
diff --git a/src/render/geometry/qmesh.cpp b/src/render/geometry/qmesh.cpp
index 66c4a69bc..1d7d33f9a 100644
--- a/src/render/geometry/qmesh.cpp
+++ b/src/render/geometry/qmesh.cpp
@@ -215,18 +215,6 @@ QMesh::QMesh(QMeshPrivate &dd, QNode *parent)
 {
 }
 
-/*! \internal */
-void QMesh::sceneChangeEvent(const Qt3DCore::QSceneChangePtr &change)
-{
-    Q_D(QMesh);
-    if (change->type() == Qt3DCore::PropertyUpdated) {
-        const Qt3DCore::QPropertyUpdatedChangePtr e = qSharedPointerCast<Qt3DCore::QPropertyUpdatedChange>(change);
-        if (e->propertyName() == QByteArrayLiteral("status"))
-            d->setStatus(e->value().value<QMesh::Status>());
-    }
-    Qt3DRender::QGeometryRenderer::sceneChangeEvent(change);
-}
-
 void QMesh::setSource(const QUrl& source)
 {
     Q_D(QMesh);
diff --git a/src/render/geometry/qmesh.h b/src/render/geometry/qmesh.h
index 458a21fa4..04fdedc20 100644
--- a/src/render/geometry/qmesh.h
+++ b/src/render/geometry/qmesh.h
@@ -88,7 +88,6 @@ Q_SIGNALS:
 
 protected:
     explicit QMesh(QMeshPrivate &dd, Qt3DCore::QNode *parent = nullptr);
-    void sceneChangeEvent(const Qt3DCore::QSceneChangePtr &change) override;
 
 private:
     Q_DECLARE_PRIVATE(QMesh)
diff --git a/src/render/jobs/loadgeometryjob.cpp b/src/render/jobs/loadgeometryjob.cpp
index d28b15a7c..88930038a 100644
--- a/src/render/jobs/loadgeometryjob.cpp
+++ b/src/render/jobs/loadgeometryjob.cpp
@@ -41,6 +41,8 @@
 #include <Qt3DRender/private/nodemanagers_p.h>
 #include <Qt3DRender/private/geometryrenderermanager_p.h>
 #include <Qt3DRender/private/job_common_p.h>
+#include <Qt3DCore/private/qaspectmanager_p.h>
+#include <Qt3DRender/private/qmesh_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -48,8 +50,19 @@ namespace Qt3DRender {
 
 namespace Render {
 
+class LoadGeometryJobPrivate : public Qt3DCore::QAspectJobPrivate
+{
+public:
+    LoadGeometryJobPrivate() {}
+    ~LoadGeometryJobPrivate() {}
+
+    void postFrame(Qt3DCore::QAspectManager *manager) override;
+
+    QVector<std::pair<Qt3DCore::QNodeId, GeometryFunctorResult>> m_updates;
+};
+
 LoadGeometryJob::LoadGeometryJob(const HGeometryRenderer &handle)
-    : QAspectJob()
+    : QAspectJob(*new LoadGeometryJobPrivate)
     , m_handle(handle)
     , m_nodeManagers(nullptr)
 {
@@ -62,9 +75,27 @@ LoadGeometryJob::~LoadGeometryJob()
 
 void LoadGeometryJob::run()
 {
+    Q_D(LoadGeometryJob);
     GeometryRenderer *geometryRenderer = m_nodeManagers->geometryRendererManager()->data(m_handle);
     if (geometryRenderer != nullptr)
-        geometryRenderer->executeFunctor();
+        d->m_updates.push_back({ geometryRenderer->peerId(), geometryRenderer->executeFunctor() });
+}
+
+void LoadGeometryJobPrivate::postFrame(Qt3DCore::QAspectManager *manager)
+{
+    const auto updates = std::move(m_updates);
+    for (const auto &update : updates) {
+        QGeometryRenderer *gR = static_cast<decltype(gR)>(manager->lookupNode(update.first));
+        const GeometryFunctorResult &result = update.second;
+        gR->setGeometry(result.geometry);
+
+        // Set status if gR is a QMesh instance
+        QMesh *mesh = qobject_cast<QMesh *>(gR);
+        if (mesh) {
+            QMeshPrivate *dMesh = static_cast<decltype(dMesh)>(Qt3DCore::QNodePrivate::get(mesh));
+            dMesh->setStatus(result.status);
+        }
+    }
 }
 
 } // namespace Render
diff --git a/src/render/jobs/loadgeometryjob_p.h b/src/render/jobs/loadgeometryjob_p.h
index c02739647..998d543ac 100644
--- a/src/render/jobs/loadgeometryjob_p.h
+++ b/src/render/jobs/loadgeometryjob_p.h
@@ -62,6 +62,7 @@ namespace Qt3DRender {
 namespace Render {
 
 class NodeManagers;
+class LoadGeometryJobPrivate;
 
 class Q_AUTOTEST_EXPORT LoadGeometryJob : public Qt3DCore::QAspectJob
 {
@@ -75,6 +76,9 @@ protected:
     void run() override;
     HGeometryRenderer m_handle;
     NodeManagers *m_nodeManagers;
+
+private:
+    Q_DECLARE_PRIVATE(LoadGeometryJob)
 };
 
 typedef QSharedPointer<LoadGeometryJob> LoadGeometryJobPtr;
diff --git a/tests/auto/render/qmesh/tst_qmesh.cpp b/tests/auto/render/qmesh/tst_qmesh.cpp
index 49679e29e..0df96dd24 100644
--- a/tests/auto/render/qmesh/tst_qmesh.cpp
+++ b/tests/auto/render/qmesh/tst_qmesh.cpp
@@ -240,28 +240,6 @@ private Q_SLOTS:
 
     }
 
-    void checkStatusUpdate()
-    {
-        // GIVEN
-        qRegisterMetaType<Qt3DRender::QMesh::Status>("Status");
-        MyQMesh mesh;
-        QSignalSpy spy(&mesh, SIGNAL(statusChanged(Status)));
-
-        // THEN
-        QCOMPARE(mesh.status(), Qt3DRender::QMesh::None);
-
-        // WHEN
-        const Qt3DRender::QMesh::Status newStatus = Qt3DRender::QMesh::Error;
-        Qt3DCore::QPropertyUpdatedChangePtr e(new Qt3DCore::QPropertyUpdatedChange(mesh.id()));
-        e->setPropertyName("status");
-        e->setValue(QVariant::fromValue(newStatus));
-        mesh.sceneChangeEvent(e);
-
-        // THEN
-        QCOMPARE(mesh.status(), newStatus);
-        QCOMPARE(spy.count(), 1);
-    }
-
     void checkGeometryFactoryIsAccessibleEvenWithNoScene() // QTBUG-65506
     {
         // GIVEN