QMesh: do not rely on QAspectEngine to create QGeometryFactory

- This would prevent QMesh created without parent/scene to have a proper
geometry factory.

- This avoid passing the engine around

Change-Id: I5091970f96e87ab8b129475a1113ef84ce170388
Task-number: QTBUG-65506
Reviewed-by: Mike Krus <mike.krus@kdab.com>

5 files changed, 164 insertions, 34 deletions

diff --git a/src/render/geometry/geometryrenderer.cpp b/src/render/geometry/geometryrenderer.cpp
index 270380e14..00b53fd8d 100644
--- a/src/render/geometry/geometryrenderer.cpp
+++ b/src/render/geometry/geometryrenderer.cpp
@@ -41,11 +41,13 @@
 #include <Qt3DRender/private/geometryrenderermanager_p.h>
 #include <Qt3DRender/private/qboundingvolume_p.h>
 #include <Qt3DRender/private/qgeometryrenderer_p.h>
+#include <Qt3DRender/private/qmesh_p.h>
 #include <Qt3DCore/qpropertyupdatedchange.h>
 #include <Qt3DCore/qpropertynodeaddedchange.h>
 #include <Qt3DCore/qpropertynoderemovedchange.h>
 #include <Qt3DCore/private/qnode_p.h>
 #include <Qt3DCore/private/qtypedpropertyupdatechange_p.h>
+#include <Qt3DCore/private/qservicelocator_p.h>
 #include <QtCore/qcoreapplication.h>
 
 #include <memory>
@@ -197,7 +199,28 @@ void GeometryRenderer::sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e)
 void GeometryRenderer::executeFunctor()
 {
     Q_ASSERT(m_geometryFactory);
+
+    // What kind of functor are we dealing with?
+    const bool isQMeshFunctor = m_geometryFactory->id() == Qt3DRender::functorTypeId<MeshLoaderFunctor>();
+
+    if (isQMeshFunctor) {
+        QSharedPointer<MeshLoaderFunctor> meshLoader = qSharedPointerCast<MeshLoaderFunctor>(m_geometryFactory);
+
+        // Set the aspect engine to allow remote downloads
+        if (meshLoader->nodeManagers() == nullptr)
+            meshLoader->setNodeManagers(m_renderer->nodeManagers());
+
+        if (meshLoader->downloaderService() == nullptr) {
+            Qt3DCore::QServiceLocator *services = m_renderer->services();
+            meshLoader->setDownloaderService(services->service<Qt3DCore::QDownloadHelperService>(Qt3DCore::QServiceLocator::DownloadHelperService));
+        };
+    }
+
+    // Load geometry
     std::unique_ptr<QGeometry> geometry((*m_geometryFactory)());
+
+    // 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)
     if (!geometry)
         return;
 
diff --git a/src/render/geometry/qmesh.cpp b/src/render/geometry/qmesh.cpp
index 6df06d723..362706204 100644
--- a/src/render/geometry/qmesh.cpp
+++ b/src/render/geometry/qmesh.cpp
@@ -89,9 +89,7 @@ void QMeshPrivate::setScene(Qt3DCore::QScene *scene)
 void QMeshPrivate::updateFunctor()
 {
     Q_Q(QMesh);
-    Qt3DCore::QAspectEngine *engine = m_scene ? m_scene->engine() : nullptr;
-    if (engine)
-        q->setGeometryFactory(QGeometryFactoryPtr(new MeshLoaderFunctor(q, engine)));
+    q->setGeometryFactory(QGeometryFactoryPtr(new MeshLoaderFunctor(q)));
 }
 
 void QMeshPrivate::setStatus(QMesh::Status status)
@@ -226,6 +224,7 @@ void QMesh::sceneChangeEvent(const Qt3DCore::QSceneChangePtr &change)
         if (e->propertyName() == QByteArrayLiteral("status"))
             d->setStatus(e->value().value<QMesh::Status>());
     }
+    Qt3DRender::QGeometryRenderer::sceneChangeEvent(change);
 }
 
 void QMesh::setSource(const QUrl& source)
@@ -289,13 +288,14 @@ QMesh::Status QMesh::status() const
 /*!
  * \internal
  */
-MeshLoaderFunctor::MeshLoaderFunctor(QMesh *mesh, Qt3DCore::QAspectEngine *engine, const QByteArray &sourceData)
+MeshLoaderFunctor::MeshLoaderFunctor(QMesh *mesh, const QByteArray &sourceData)
     : QGeometryFactory()
     , m_mesh(mesh->id())
     , m_sourcePath(mesh->source())
     , m_meshName(mesh->meshName())
-    , m_engine(engine)
     , m_sourceData(sourceData)
+    , m_nodeManagers(nullptr)
+    , m_downloaderService(nullptr)
 {
 }
 
@@ -313,9 +313,14 @@ QGeometry *MeshLoaderFunctor::operator()()
     if (!Qt3DCore::QDownloadHelperService::isLocal(m_sourcePath)) {
         if (m_sourceData.isEmpty()) {
             if (m_mesh) {
-                auto downloadService = Qt3DCore::QDownloadHelperService::getService(m_engine);
-                Qt3DCore::QDownloadRequestPtr request(new MeshDownloadRequest(m_mesh, m_sourcePath, m_engine));
-                downloadService->submitRequest(request);
+                // Output a warning in the case a user is calling the functor directly
+                // in the frontend
+                if (m_nodeManagers == nullptr || m_downloaderService == nullptr) {
+                    qWarning() << "Mesh source points to a remote URL. Remotes meshes can only be loaded if the geometry is processed by the Qt3DRender backend";
+                    return nullptr;
+                }
+                Qt3DCore::QDownloadRequestPtr request(new MeshDownloadRequest(m_mesh, m_sourcePath, m_nodeManagers));
+                m_downloaderService->submitRequest(request);
             }
             return nullptr;
         }
@@ -386,39 +391,48 @@ bool MeshLoaderFunctor::operator ==(const QGeometryFactory &other) const
         return (otherFunctor->m_sourcePath == m_sourcePath &&
                 otherFunctor->m_sourceData.isEmpty() == m_sourceData.isEmpty() &&
                 otherFunctor->m_meshName == m_meshName &&
-                otherFunctor->m_engine == m_engine);
+                otherFunctor->m_downloaderService == m_downloaderService &&
+                otherFunctor->m_nodeManagers == m_nodeManagers);
     return false;
 }
 
 /*!
  * \internal
  */
-MeshDownloadRequest::MeshDownloadRequest(Qt3DCore::QNodeId mesh, QUrl source, Qt3DCore::QAspectEngine *engine)
+MeshDownloadRequest::MeshDownloadRequest(Qt3DCore::QNodeId mesh, QUrl source, Render::NodeManagers *managers)
     : Qt3DCore::QDownloadRequest(source)
     , m_mesh(mesh)
-    , m_engine(engine)
+    , m_nodeManagers(managers)
 {
-
 }
 
+// Called in Aspect Thread context (not a Qt3D AspectJob)
+// We are sure that when this is called, no AspectJob are running
 void MeshDownloadRequest::onCompleted()
 {
     if (cancelled() || !succeeded())
         return;
 
-    QRenderAspectPrivate* d_aspect = QRenderAspectPrivate::findPrivate(m_engine);
-    if (!d_aspect)
+    if (!m_nodeManagers)
         return;
 
-    Render::GeometryRenderer *renderer = d_aspect->m_nodeManagers->geometryRendererManager()->lookupResource(m_mesh);
+    Render::GeometryRenderer *renderer = m_nodeManagers->geometryRendererManager()->lookupResource(m_mesh);
     if (!renderer)
         return;
 
-    QSharedPointer<MeshLoaderFunctor> functor = qSharedPointerCast<MeshLoaderFunctor>(renderer->geometryFactory());
-    functor->m_sourceData = m_data;
+    QGeometryFactoryPtr geometryFactory = renderer->geometryFactory();
+    if (!geometryFactory.isNull() && geometryFactory->id() == Qt3DRender::functorTypeId<MeshLoaderFunctor>()) {
+        QSharedPointer<MeshLoaderFunctor> functor = qSharedPointerCast<MeshLoaderFunctor>(geometryFactory);
+
+        // We make sure we are setting the result for the right request
+        // (the functor for the mesh could have changed in the meantime)
+        if (m_url == functor->sourcePath()) {
+            functor->setSourceData(m_data);
 
-    // mark the component as dirty so that the functor runs again in the correct job
-    d_aspect->m_nodeManagers->geometryRendererManager()->addDirtyGeometryRenderer(m_mesh);
+            // mark the component as dirty so that the functor runs again in the correct job
+            m_nodeManagers->geometryRendererManager()->addDirtyGeometryRenderer(m_mesh);
+        }
+    }
 }
 
 } // namespace Qt3DRender
diff --git a/src/render/geometry/qmesh_p.h b/src/render/geometry/qmesh_p.h
index 76fc351a8..dfbd9016d 100644
--- a/src/render/geometry/qmesh_p.h
+++ b/src/render/geometry/qmesh_p.h
@@ -59,8 +59,16 @@
 
 QT_BEGIN_NAMESPACE
 
+namespace Qt3DCore {
+class QDownloadHelperService;
+}
+
 namespace Qt3DRender {
 
+namespace Render {
+class NodeManagers;
+}
+
 class QT3DRENDERSHARED_PRIVATE_EXPORT QMeshPrivate : public QGeometryRendererPrivate
 {
 public:
@@ -81,28 +89,44 @@ public:
 class Q_AUTOTEST_EXPORT MeshDownloadRequest : public Qt3DCore::QDownloadRequest
 {
 public:
-    MeshDownloadRequest(Qt3DCore::QNodeId mesh, QUrl source, Qt3DCore::QAspectEngine *engine);
+    MeshDownloadRequest(Qt3DCore::QNodeId mesh, QUrl source, Render::NodeManagers *managers);
 
     void onCompleted() override;
 
 private:
     Qt3DCore::QNodeId m_mesh;
-    Qt3DCore::QAspectEngine *m_engine;
+    Render::NodeManagers *m_nodeManagers;
 };
 
 class Q_AUTOTEST_EXPORT MeshLoaderFunctor : public QGeometryFactory
 {
 public :
-    MeshLoaderFunctor(QMesh *mesh, Qt3DCore::QAspectEngine *engine, const QByteArray &sourceData = QByteArray());
+    MeshLoaderFunctor(QMesh *mesh, const QByteArray &sourceData = QByteArray());
+
+    void setNodeManagers(Render::NodeManagers *managers) { m_nodeManagers = managers; }
+    Render::NodeManagers *nodeManagers() const { return m_nodeManagers; }
+
+    void setDownloaderService(Qt3DCore::QDownloadHelperService *service) { m_downloaderService = service; }
+    Qt3DCore::QDownloadHelperService *downloaderService() const { return m_downloaderService; }
+
+    void setSourceData(const QByteArray &data) { m_sourceData = data; }
+    QByteArray sourceData() const { return m_sourceData; }
+
+    QUrl sourcePath() const { return m_sourcePath; }
+    Qt3DCore::QNodeId mesh() const { return m_mesh; }
+    QString meshName() const { return m_meshName; }
+
     QGeometry *operator()() override;
     bool operator ==(const QGeometryFactory &other) const override;
     QT3D_FUNCTOR(MeshLoaderFunctor)
 
+private:
     Qt3DCore::QNodeId m_mesh;
     QUrl m_sourcePath;
     QString m_meshName;
-    Qt3DCore::QAspectEngine *m_engine;
     QByteArray m_sourceData;
+    Render::NodeManagers *m_nodeManagers;
+    Qt3DCore::QDownloadHelperService *m_downloaderService;
 };
 
 
diff --git a/tests/auto/render/meshfunctors/tst_meshfunctors.cpp b/tests/auto/render/meshfunctors/tst_meshfunctors.cpp
index 1904169fb..5d89767c7 100644
--- a/tests/auto/render/meshfunctors/tst_meshfunctors.cpp
+++ b/tests/auto/render/meshfunctors/tst_meshfunctors.cpp
@@ -100,6 +100,24 @@ class tst_MeshFunctors : public QObject
 {
     Q_OBJECT
 private Q_SLOTS:
+
+    void checkInitialState()
+    {
+        // GIVEN
+        Qt3DRender::QMesh mesh;
+        mesh.setSource(QUrl(QStringLiteral("./some_path.obj")));
+
+        // WHEN
+        const Qt3DRender::MeshLoaderFunctor functor(&mesh);
+
+        // THEN
+        QVERIFY(functor.nodeManagers() == nullptr);
+        QVERIFY(functor.downloaderService() == nullptr);
+        QVERIFY(functor.sourceData().isEmpty());
+        QCOMPARE(functor.mesh(), mesh.id());
+        QCOMPARE(functor.sourcePath(), mesh.source());
+    }
+
     void functorComparison()
     {
         // GIVEN
@@ -125,7 +143,6 @@ private Q_SLOTS:
     void checkMeshFunctorEquality()
     {
         // GIVEN
-        Qt3DCore::QAspectEngine engine;
         auto meshA = new Qt3DRender::QMesh();
         meshA->setSource(QUrl::fromLocalFile(QLatin1String("/foo")));
         meshA->setMeshName(QLatin1String("bar"));
@@ -142,10 +159,10 @@ private Q_SLOTS:
         meshD->setSource(QUrl::fromLocalFile(QLatin1String("/foo")));
         meshD->setMeshName(QLatin1String("bar"));
 
-        const Qt3DRender::MeshLoaderFunctor functorA(meshA, &engine);
-        const Qt3DRender::MeshLoaderFunctor functorB(meshB, &engine);
-        const Qt3DRender::MeshLoaderFunctor functorC(meshC, &engine);
-        const Qt3DRender::MeshLoaderFunctor functorD(meshD, &engine);
+        const Qt3DRender::MeshLoaderFunctor functorA(meshA);
+        const Qt3DRender::MeshLoaderFunctor functorB(meshB);
+        const Qt3DRender::MeshLoaderFunctor functorC(meshC);
+        const Qt3DRender::MeshLoaderFunctor functorD(meshD);
 
         // WHEN
         const bool selfEquality = (functorA == functorA);
@@ -159,6 +176,47 @@ private Q_SLOTS:
         QCOMPARE(sameMeshName, false);
         QCOMPARE(perfectMatch, true);
     }
+
+    void checkExecution()
+    {
+        {
+            // GIVEN
+            Qt3DRender::QMesh mesh;
+            Qt3DRender::MeshLoaderFunctor functor(&mesh);
+
+            // WHEN
+            const Qt3DRender::QGeometry *g = functor();
+
+            // THEN
+            QVERIFY(g == nullptr);
+        }
+
+        {
+            // GIVEN
+            Qt3DRender::QMesh mesh;
+            mesh.setSource(QUrl(QStringLiteral("./non_existing.obj")));
+            Qt3DRender::MeshLoaderFunctor functor(&mesh);
+
+            // WHEN
+            const Qt3DRender::QGeometry *g = functor();
+
+            // THEN
+            QVERIFY(g == nullptr);
+        }
+
+        {
+            // GIVEN
+            Qt3DRender::QMesh mesh;
+            mesh.setSource(QUrl(QStringLiteral("http://www.somedomain.org/non_exisiting.obj")));
+            Qt3DRender::MeshLoaderFunctor functor(&mesh);
+
+            // WHEN
+            const Qt3DRender::QGeometry *g = functor();
+
+            // THEN
+            QVERIFY(g == nullptr);
+        }
+    }
 };
 
 QTEST_MAIN(tst_MeshFunctors)
diff --git a/tests/auto/render/qmesh/tst_qmesh.cpp b/tests/auto/render/qmesh/tst_qmesh.cpp
index 67d938add..fbc566395 100644
--- a/tests/auto/render/qmesh/tst_qmesh.cpp
+++ b/tests/auto/render/qmesh/tst_qmesh.cpp
@@ -137,8 +137,8 @@ private Q_SLOTS:
             const auto creationChangeData = qSharedPointerCast<Qt3DCore::QNodeCreatedChange<Qt3DRender::QGeometryRendererData>>(creationChanges.first());
             const Qt3DRender::QGeometryRendererData cloneData = creationChangeData->data;
 
-            // Geometry factory is null until the engine becomes available
-            QVERIFY(cloneData.geometryFactory == nullptr);
+            // Geometry factory shouldn't be null
+            QVERIFY(cloneData.geometryFactory != nullptr);
             QCOMPARE(mesh.id(), creationChangeData->subjectId());
             QCOMPARE(mesh.isEnabled(), true);
             QCOMPARE(mesh.isEnabled(), creationChangeData->isNodeEnabled());
@@ -194,8 +194,8 @@ private Q_SLOTS:
             Qt3DRender::QGeometryFactoryPtr factory = change->value().value<Qt3DRender::QGeometryFactoryPtr>();
             QSharedPointer<Qt3DRender::MeshLoaderFunctor> meshFunctor = qSharedPointerCast<Qt3DRender::MeshLoaderFunctor>(factory);
             QVERIFY(meshFunctor != nullptr);
-            QCOMPARE(meshFunctor->m_mesh, mesh.id());
-            QCOMPARE(meshFunctor->m_sourcePath, mesh.source());
+            QCOMPARE(meshFunctor->mesh(), mesh.id());
+            QCOMPARE(meshFunctor->sourcePath(), mesh.source());
 
             arbiter.events.clear();
         }
@@ -239,8 +239,8 @@ private Q_SLOTS:
             Qt3DRender::QGeometryFactoryPtr factory = change->value().value<Qt3DRender::QGeometryFactoryPtr>();
             QSharedPointer<Qt3DRender::MeshLoaderFunctor> meshFunctor = qSharedPointerCast<Qt3DRender::MeshLoaderFunctor>(factory);
             QVERIFY(meshFunctor != nullptr);
-            QCOMPARE(meshFunctor->m_mesh, mesh.id());
-            QCOMPARE(meshFunctor->m_meshName, mesh.meshName());
+            QCOMPARE(meshFunctor->mesh(), mesh.id());
+            QCOMPARE(meshFunctor->meshName(), mesh.meshName());
 
             arbiter.events.clear();
         }
@@ -278,6 +278,17 @@ private Q_SLOTS:
         QCOMPARE(spy.count(), 1);
     }
 
+    void checkGeometryFactoryIsAccessibleEvenWithNoScene() // QTBUG-65506
+    {
+        // GIVEN
+        Qt3DRender::QMesh mesh;
+
+        // WHEN
+        mesh.setSource(QUrl(QStringLiteral("some_path")));
+
+        // THEN
+        QVERIFY(!mesh.geometryFactory().isNull());
+    }
 };
 
 QTEST_MAIN(tst_QMesh)