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>

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;

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;

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

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;
 };

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())

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();
 

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();
-    }
 };
 
 

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
+        QVERIFY(!rayCaster->isEnabled());
-        Qt3DCore::QPropertyUpdatedChangePtr change = arbiter.events.first().staticCast<Qt3DCore::QPropertyUpdatedChange>();
+        auto dirtyNodes = test->arbiter()->takeDirtyFrontEndNodes();
-        QCOMPARE(change->propertyName(), "hits");
+        QCOMPARE(dirtyNodes.count(), 1); // hits & disable
-        Qt3DRender::QRayCaster::Hits hits = change->value().value<Qt3DRender::QRayCaster::Hits>();
+        QCOMPARE(rayCaster->hits().size(), numIntersections);
-        QCOMPARE(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
+        QVERIFY(!rayCaster->isEnabled());
-        Qt3DCore::QPropertyUpdatedChangePtr change = arbiter.events.first().staticCast<Qt3DCore::QPropertyUpdatedChange>();
+        auto dirtyNodes = test->arbiter()->takeDirtyFrontEndNodes();
-        QCOMPARE(change->propertyName(), "hits");
+        QCOMPARE(dirtyNodes.count(), 1); // hits & disable
-        Qt3DRender::QScreenRayCaster::Hits hits = change->value().value<Qt3DRender::QScreenRayCaster::Hits>();
+        QCOMPARE(rayCaster->hits().size(), numIntersections);
-        QCOMPARE(hits.size(), numIntersections);
 
         if (numIntersections)
-            QVERIFY(hits.first().entityId());
+            QVERIFY(rayCaster->hits().first().entityId());
     }
 
 };