Update ray casting job to use direct sync

When the job is complete and we're back in the main thread,
the job can look up the frontend node and deliver the hits
directly. This saves allocating messages.

Unit test changed quite a bit as it needs an aspect engine &
manager to pass to the job for looking up nodes.

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

8 files changed, 106 insertions, 69 deletions

diff --git a/src/render/jobs/abstractpickingjob.cpp b/src/render/jobs/abstractpickingjob.cpp
index 74e6a7f80..35e658535 100644
--- a/src/render/jobs/abstractpickingjob.cpp
+++ b/src/render/jobs/abstractpickingjob.cpp
@@ -58,13 +58,24 @@ namespace Qt3DRender {
 namespace Render {
 
 AbstractPickingJob::AbstractPickingJob()
-    : m_manager(nullptr)
+    : Qt3DCore::QAspectJob()
+    , m_manager(nullptr)
     , m_node(nullptr)
     , m_frameGraphRoot(nullptr)
     , m_renderSettings(nullptr)
 {
 }
 
+AbstractPickingJob::AbstractPickingJob(Qt3DCore::QAspectJobPrivate &dd)
+    : Qt3DCore::QAspectJob(dd)
+    , m_manager(nullptr)
+    , m_node(nullptr)
+    , m_frameGraphRoot(nullptr)
+    , m_renderSettings(nullptr)
+{
+
+}
+
 void AbstractPickingJob::setRoot(Entity *root)
 {
     m_node = root;
diff --git a/src/render/jobs/abstractpickingjob_p.h b/src/render/jobs/abstractpickingjob_p.h
index 059c87aa7..c0c6ed7e9 100644
--- a/src/render/jobs/abstractpickingjob_p.h
+++ b/src/render/jobs/abstractpickingjob_p.h
@@ -90,6 +90,8 @@ public:
                                               const QRect &viewport);
 
 protected:
+    AbstractPickingJob(Qt3DCore::QAspectJobPrivate &dd);
+
     void run() final;
 
     NodeManagers *m_manager;
diff --git a/src/render/jobs/raycastingjob.cpp b/src/render/jobs/raycastingjob.cpp
index f3571c210..380447873 100644
--- a/src/render/jobs/raycastingjob.cpp
+++ b/src/render/jobs/raycastingjob.cpp
@@ -38,6 +38,7 @@
 ****************************************************************************/
 
 #include "raycastingjob_p.h"
+#include <Qt3DCore/private/qaspectmanager_p.h>
 #include <Qt3DRender/qgeometryrenderer.h>
 #include <Qt3DRender/private/entity_p.h>
 #include <Qt3DRender/private/geometryrenderer_p.h>
@@ -51,6 +52,7 @@
 #include <Qt3DRender/private/rendersettings_p.h>
 #include <Qt3DRender/private/trianglesvisitor_p.h>
 #include <Qt3DRender/private/entityvisitor_p.h>
+#include <Qt3DRender/private/qabstractraycaster_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -82,11 +84,43 @@ public:
 } // anonymous
 
 
+class Qt3DRender::Render::RayCastingJobPrivate : public Qt3DCore::QAspectJobPrivate
+{
+public:
+    RayCastingJobPrivate() { }
+    ~RayCastingJobPrivate() override { Q_ASSERT(dispatches.isEmpty()); }
+
+    void postFrame(Qt3DCore::QAspectManager *manager) override;
+
+    QVector<QPair<RayCaster *, QAbstractRayCaster::Hits>> dispatches;
+};
+
+
+void RayCastingJobPrivate::postFrame(Qt3DCore::QAspectManager *manager)
+{
+    for (auto res: qAsConst(dispatches)) {
+        QAbstractRayCaster *node = qobject_cast<QAbstractRayCaster *>(manager->lookupNode(res.first->peerId()));
+        if (!node)
+            continue;
+
+        QAbstractRayCasterPrivate *d = QAbstractRayCasterPrivate::get(node);
+        d->dispatchHits(res.second);
+
+        if (node->runMode() == QAbstractRayCaster::SingleShot) {
+            node->setEnabled(false);
+            res.first->setEnabled(false);
+        }
+    }
+
+    dispatches.clear();
+}
+
+
 RayCastingJob::RayCastingJob()
-    : AbstractPickingJob()
+    : AbstractPickingJob(*new RayCastingJobPrivate())
     , m_castersDirty(true)
 {
-    SET_JOB_RUN_STAT_TYPE(this, JobTypes::RayCasting, 0);
+    SET_JOB_RUN_STAT_TYPE(this, JobTypes::RayCasting, 0)
 }
 
 void RayCastingJob::markCastersDirty()
@@ -239,7 +273,8 @@ void RayCastingJob::dispatchHits(RayCaster *rayCaster, const PickingUtils::HitLi
         };
     }
 
-    rayCaster->dispatchHits(hits);
+    Q_D(RayCastingJob);
+    d->dispatches.push_back({rayCaster, hits});
 }
 
 QT_END_NAMESPACE
diff --git a/src/render/jobs/raycastingjob_p.h b/src/render/jobs/raycastingjob_p.h
index 0bd8d445a..4b8b91ad5 100644
--- a/src/render/jobs/raycastingjob_p.h
+++ b/src/render/jobs/raycastingjob_p.h
@@ -68,6 +68,8 @@ namespace PickingUtils {
 typedef QVector<RayCasting::QCollisionQueryResult::Hit> HitList;
 }
 
+class RayCastingJobPrivate;
+
 class Q_AUTOTEST_EXPORT RayCastingJob : public AbstractPickingJob
 {
 public:
@@ -80,6 +82,8 @@ protected:
     void dispatchHits(RayCaster *rayCaster, const PickingUtils::HitList &sphereHits);
 
 private:
+    Q_DECLARE_PRIVATE(RayCastingJob)
+
     bool m_castersDirty;
     bool m_oneEnabledAtLeast;
 };
diff --git a/src/render/picking/raycaster.cpp b/src/render/picking/raycaster.cpp
index fdf4eea26..3a79204c7 100644
--- a/src/render/picking/raycaster.cpp
+++ b/src/render/picking/raycaster.cpp
@@ -177,24 +177,6 @@ void RayCaster::syncFromFrontEnd(const Qt3DCore::QNode *frontEnd, bool firstTime
     }
 }
 
-void RayCaster::dispatchHits(const QAbstractRayCaster::Hits &hits)
-{
-    auto e = Qt3DCore::QPropertyUpdatedChangePtr::create(peerId());
-    e->setDeliveryFlags(Qt3DCore::QSceneChange::DeliverToAll);
-    e->setPropertyName("hits");
-    e->setValue(QVariant::fromValue(hits));
-    notifyObservers(e);
-
-    if (m_runMode == QAbstractRayCaster::SingleShot) {
-        setEnabled(false);
-        auto e = Qt3DCore::QPropertyUpdatedChangePtr::create(peerId());
-        e->setDeliveryFlags(Qt3DCore::QSceneChange::DeliverToAll);
-        e->setPropertyName("enabled");
-        e->setValue(false);
-        notifyObservers(e);
-    }
-}
-
 void RayCaster::notifyJob()
 {
     if (m_renderer && m_renderer->rayCastingJob())
diff --git a/src/render/picking/raycaster_p.h b/src/render/picking/raycaster_p.h
index 2be87ec3c..865d40365 100644
--- a/src/render/picking/raycaster_p.h
+++ b/src/render/picking/raycaster_p.h
@@ -83,8 +83,6 @@ public:
     void syncFromFrontEnd(const Qt3DCore::QNode *frontEnd, bool firstTime) override;
     void cleanup();
 
-    void dispatchHits(const QAbstractRayCaster::Hits &hits);
-
 private:
     void notifyJob();
 
diff --git a/tests/auto/render/raycaster/tst_raycaster.cpp b/tests/auto/render/raycaster/tst_raycaster.cpp
index 357517f0d..72ef396d7 100644
--- a/tests/auto/render/raycaster/tst_raycaster.cpp
+++ b/tests/auto/render/raycaster/tst_raycaster.cpp
@@ -106,26 +106,6 @@ private Q_SLOTS:
             QVERIFY(renderer.dirtyBits() != 0);
         }
     }
-
-    void checkBackendPropertyNotifications()
-    {
-        // GIVEN
-        TestArbiter arbiter;
-        Qt3DRender::Render::RayCaster rayCaster;
-        Qt3DCore::QBackendNodePrivate::get(&rayCaster)->setArbiter(&arbiter);
-        Qt3DRender::QAbstractRayCaster::Hits hits;
-
-        // WHEN
-        rayCaster.dispatchHits(hits);
-
-        // THEN
-        QCOMPARE(arbiter.events.count(), 2);
-        Qt3DCore::QPropertyUpdatedChangePtr change = arbiter.events.first().staticCast<Qt3DCore::QPropertyUpdatedChange>();
-        QCOMPARE(change->propertyName(), "hits");
-        QVERIFY(!rayCaster.isEnabled());
-
-        arbiter.events.clear();
-    }
 };
 
 
diff --git a/tests/auto/render/raycastingjob/tst_raycastingjob.cpp b/tests/auto/render/raycastingjob/tst_raycastingjob.cpp
index e45ec704e..1ff899936 100644
--- a/tests/auto/render/raycastingjob/tst_raycastingjob.cpp
+++ b/tests/auto/render/raycastingjob/tst_raycastingjob.cpp
@@ -35,6 +35,9 @@
 #include <Qt3DCore/private/qnodecreatedchangegenerator_p.h>
 #include <Qt3DCore/private/qaspectjobmanager_p.h>
 #include <Qt3DCore/private/qnodevisitor_p.h>
+#include <Qt3DCore/private/qaspectmanager_p.h>
+#include <Qt3DCore/private/qscene_p.h>
+#include <Qt3DCore/private/qaspectengine_p.h>
 #include <QtQuick/qquickwindow.h>
 
 #include <Qt3DRender/QCamera>
@@ -111,10 +114,18 @@ public:
         : Qt3DRender::QRenderAspect(Qt3DRender::QRenderAspect::Synchronous)
         , m_sceneRoot(nullptr)
     {
-        QRenderAspect::onRegistered();
-
+        m_engine = new Qt3DCore::QAspectEngine(this);
+        m_engine->registerAspect(this);
+        Q_ASSERT(d_func()->m_aspectManager);
+
+        // do what QAspectEngine::setRootEntity does since we don't want to enter the simulation loop
+        Qt3DCore::QEntityPtr proot(qobject_cast<Qt3DCore::QEntity *>(root), [](Qt3DCore::QEntity *) { });
+        Qt3DCore::QAspectEnginePrivate *aed = Qt3DCore::QAspectEnginePrivate::get(m_engine);
+        aed->m_root = proot;
+        aed->initialize();
+        aed->initNodeTree(root);
         const QVector<Qt3DCore::QNode *> nodes = getNodesForCreation(root);
-        d_func()->setRootAndCreateNodes(qobject_cast<Qt3DCore::QEntity *>(root), nodeTreeChangesForNodes(nodes));
+        aed->m_aspectManager->setRootEntity(proot.data(), nodes);
 
         Render::Entity *rootEntity = nodeManagers()->lookupResource<Render::Entity, Render::EntityManager>(rootEntityId());
         Q_ASSERT(rootEntity);
@@ -123,7 +134,17 @@ public:
 
     ~TestAspect()
     {
-        QRenderAspect::onUnregistered();
+        using namespace Qt3DCore;
+        QNodeVisitor visitor;
+        visitor.traverse(m_engine->rootEntity().data(), [](QNode *node) {
+            QNodePrivate *d = QNodePrivate::get(node);
+            d->m_scene = nullptr;
+            d->m_changeArbiter = nullptr;
+        });
+
+        m_engine->unregisterAspect(this);
+        delete m_engine;
+        m_engine = nullptr;
     }
 
     void onRegistered() { QRenderAspect::onRegistered(); }
@@ -133,8 +154,10 @@ public:
     Qt3DRender::Render::FrameGraphNode *frameGraphRoot() const { return d_func()->m_renderer->frameGraphRoot(); }
     Qt3DRender::Render::RenderSettings *renderSettings() const { return d_func()->m_renderer->settings(); }
     Qt3DRender::Render::Entity *sceneRoot() const { return m_sceneRoot; }
-
+    Qt3DCore::QAspectManager *aspectManager() const { return  d_func()->m_aspectManager; }
+    Qt3DCore::QChangeArbiter *arbiter() const { return d_func()->m_arbiter; }
 private:
+    Qt3DCore::QAspectEngine *m_engine;
     Render::Entity *m_sceneRoot;
 };
 
@@ -146,6 +169,10 @@ namespace {
 
 void runRequiredJobs(Qt3DRender::TestAspect *test)
 {
+    QCoreApplication::processEvents();
+    const auto dn = test->arbiter()->takeDirtyFrontEndNodes();
+    Qt3DCore::QAbstractAspectPrivate::get(test)->syncDirtyFrontEndNodes(dn);
+
     Qt3DRender::Render::UpdateWorldTransformJob updateWorldTransform;
     updateWorldTransform.setRoot(test->sceneRoot());
     updateWorldTransform.setManagers(test->nodeManagers());
@@ -233,6 +260,7 @@ private Q_SLOTS:
         QmlSceneReader sceneReader(source);
         QScopedPointer<Qt3DCore::QEntity> root(qobject_cast<Qt3DCore::QEntity *>(sceneReader.root()));
         QVERIFY(root);
+        QScopedPointer<Qt3DRender::TestAspect> test(new Qt3DRender::TestAspect(root.data()));
 
         Qt3DCore::QComponentVector rootComponents = root->components();
         Qt3DRender::QRayCaster *rayCaster = nullptr;
@@ -245,33 +273,31 @@ private Q_SLOTS:
 
         rayCaster->trigger(rayOrigin, rayDirection, rayLength);
 
-        QScopedPointer<Qt3DRender::TestAspect> test(new Qt3DRender::TestAspect(root.data()));
-        TestArbiter arbiter;
-
         // Runs Required jobs
         runRequiredJobs(test.data());
 
         Qt3DRender::Render::RayCaster *backendRayCaster = test->nodeManagers()->rayCasterManager()->lookupResource(rayCaster->id());
         QVERIFY(backendRayCaster);
-        Qt3DCore::QBackendNodePrivate::get(backendRayCaster)->setArbiter(&arbiter);
+        Qt3DCore::QBackendNodePrivate::get(backendRayCaster)->setArbiter(test->arbiter());
 
         // WHEN
         Qt3DRender::Render::RayCastingJob rayCastingJob;
         initializeJob(&rayCastingJob, test.data());
 
         bool earlyReturn = !rayCastingJob.runHelper();
+        rayCastingJob.postFrame(test->aspectManager());
+        QCoreApplication::processEvents();
 
         // THEN
         QVERIFY(!earlyReturn);
         QVERIFY(!backendRayCaster->isEnabled());
-        QCOMPARE(arbiter.events.count(), 2); // hits & disable
-        Qt3DCore::QPropertyUpdatedChangePtr change = arbiter.events.first().staticCast<Qt3DCore::QPropertyUpdatedChange>();
-        QCOMPARE(change->propertyName(), "hits");
-        Qt3DRender::QRayCaster::Hits hits = change->value().value<Qt3DRender::QRayCaster::Hits>();
-        QCOMPARE(hits.size(), numIntersections);
+        QVERIFY(!rayCaster->isEnabled());
+        auto dirtyNodes = test->arbiter()->takeDirtyFrontEndNodes();
+        QCOMPARE(dirtyNodes.count(), 1); // hits & disable
+        QCOMPARE(rayCaster->hits().size(), numIntersections);
 
         if (numIntersections)
-            QVERIFY(hits.first().entityId());
+            QVERIFY(rayCaster->hits().first().entityId());
     }
 
     void screenSpaceRayCaster_data()
@@ -294,6 +320,7 @@ private Q_SLOTS:
         QmlSceneReader sceneReader(source);
         QScopedPointer<Qt3DCore::QEntity> root(qobject_cast<Qt3DCore::QEntity *>(sceneReader.root()));
         QVERIFY(root);
+        QScopedPointer<Qt3DRender::TestAspect> test(new Qt3DRender::TestAspect(root.data()));
 
         Qt3DCore::QComponentVector rootComponents = root->components();
         Qt3DRender::QScreenRayCaster *rayCaster = nullptr;
@@ -306,33 +333,31 @@ private Q_SLOTS:
 
         rayCaster->trigger(rayPosition);
 
-        QScopedPointer<Qt3DRender::TestAspect> test(new Qt3DRender::TestAspect(root.data()));
-        TestArbiter arbiter;
-
         // Runs Required jobs
         runRequiredJobs(test.data());
 
         Qt3DRender::Render::RayCaster *backendRayCaster = test->nodeManagers()->rayCasterManager()->lookupResource(rayCaster->id());
         QVERIFY(backendRayCaster);
-        Qt3DCore::QBackendNodePrivate::get(backendRayCaster)->setArbiter(&arbiter);
+        Qt3DCore::QBackendNodePrivate::get(backendRayCaster)->setArbiter(test->arbiter());
 
         // WHEN
         Qt3DRender::Render::RayCastingJob rayCastingJob;
         initializeJob(&rayCastingJob, test.data());
 
         bool earlyReturn = !rayCastingJob.runHelper();
+        rayCastingJob.postFrame(test->aspectManager());
+        QCoreApplication::processEvents();
 
         // THEN
         QVERIFY(!earlyReturn);
         QVERIFY(!backendRayCaster->isEnabled());
-        QCOMPARE(arbiter.events.count(), 2); // hits & disable
-        Qt3DCore::QPropertyUpdatedChangePtr change = arbiter.events.first().staticCast<Qt3DCore::QPropertyUpdatedChange>();
-        QCOMPARE(change->propertyName(), "hits");
-        Qt3DRender::QScreenRayCaster::Hits hits = change->value().value<Qt3DRender::QScreenRayCaster::Hits>();
-        QCOMPARE(hits.size(), numIntersections);
+        QVERIFY(!rayCaster->isEnabled());
+        auto dirtyNodes = test->arbiter()->takeDirtyFrontEndNodes();
+        QCOMPARE(dirtyNodes.count(), 1); // hits & disable
+        QCOMPARE(rayCaster->hits().size(), numIntersections);
 
         if (numIntersections)
-            QVERIFY(hits.first().entityId());
+            QVERIFY(rayCaster->hits().first().entityId());
     }
 
 };