Refactor ViewAll to not use commands

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

6 files changed, 73 insertions, 90 deletions

diff --git a/src/render/backend/cameralens.cpp b/src/render/backend/cameralens.cpp
index d902a08c7..e1d72efda 100644
--- a/src/render/backend/cameralens.cpp
+++ b/src/render/backend/cameralens.cpp
@@ -49,6 +49,7 @@
 #include <Qt3DRender/private/renderlogging_p.h>
 #include <Qt3DCore/qentity.h>
 #include <Qt3DCore/qtransform.h>
+#include <Qt3DCore/private/qaspectmanager_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -63,21 +64,21 @@ namespace {
 class GetBoundingVolumeWithoutCameraJob : public ComputeFilteredBoundingVolumeJob
 {
 public:
-    GetBoundingVolumeWithoutCameraJob(CameraLens *lens,
-                                      QNodeCommand::CommandId commandId)
-        : m_lens(lens), m_commandId(commandId)
+    GetBoundingVolumeWithoutCameraJob(CameraLens *lens, QNodeId commandId)
+        : m_lens(lens), m_requestId(commandId)
     {
     }
 
 protected:
-    void finished(const Sphere &sphere) override
+    // called in main thread
+    void finished(Qt3DCore::QAspectManager *aspectManager, const Sphere &sphere) override
     {
-        m_lens->notifySceneBoundingVolume(sphere, m_commandId);
+        m_lens->processViewAllResult(aspectManager, sphere, m_requestId);
     }
 
 private:
     CameraLens *m_lens;
-    QNodeCommand::CommandId m_commandId;
+    QNodeId m_requestId;
 };
 
 } // namespace
@@ -135,34 +136,23 @@ void CameraLens::syncFromFrontEnd(const Qt3DCore::QNode *frontEnd, bool firstTim
         markDirty(AbstractRenderer::AllDirty);
     }
 
-    if (node->exposure() != m_exposure) {
+    if (!qFuzzyCompare(node->exposure(), m_exposure)) {
         m_exposure = node->exposure();
         markDirty(AbstractRenderer::AllDirty);
     }
 
     const QCameraLensPrivate *d = static_cast<const QCameraLensPrivate *>(QNodePrivate::get(node));
-    if (d->m_pendingViewAllCommand != m_pendingViewAllCommand) {
-        m_pendingViewAllCommand = d->m_pendingViewAllCommand;
-
-        if (m_pendingViewAllCommand) {
-            const QVariant v = m_pendingViewAllCommand.data;
-            const QNodeCommand::CommandId commandId =  m_pendingViewAllCommand.commandId;
-
-            if (m_pendingViewAllCommand.name == QLatin1String("QueryRootBoundingVolume")) {
-                const QNodeId id = v.value<QNodeId>();
-                computeSceneBoundingVolume({}, id, commandId);
-            } else if (m_pendingViewAllCommand.name == QLatin1String("QueryEntityBoundingVolume")) {
-                const QVector<QNodeId> ids = v.value<QVector<QNodeId>>();
-                if (ids.size() == 2)
-                    computeSceneBoundingVolume(ids[0], ids[1], commandId);
-            }
-        }
+    if (d->m_pendingViewAllRequest != m_pendingViewAllRequest) {
+        m_pendingViewAllRequest = d->m_pendingViewAllRequest;
+
+        if (m_pendingViewAllRequest)
+            computeSceneBoundingVolume(m_pendingViewAllRequest.entityId, m_pendingViewAllRequest.cameraId, m_pendingViewAllRequest.requestId);
     }
 }
 
 void CameraLens::computeSceneBoundingVolume(QNodeId entityId,
                                             QNodeId cameraId,
-                                            QNodeCommand::CommandId commandId)
+                                            QNodeId requestId)
 {
     if (!m_renderer || !m_renderAspect)
         return;
@@ -175,7 +165,7 @@ void CameraLens::computeSceneBoundingVolume(QNodeId entityId,
         return;
 
     Entity *camNode = nodeManagers->renderNodesManager()->lookupResource(cameraId);
-    ComputeFilteredBoundingVolumeJobPtr job(new GetBoundingVolumeWithoutCameraJob(this, commandId));
+    ComputeFilteredBoundingVolumeJobPtr job(new GetBoundingVolumeWithoutCameraJob(this, requestId));
     job->addDependency(m_renderer->expandBoundingVolumeJob());
     job->setRoot(root);
     job->setManagers(nodeManagers);
@@ -183,18 +173,18 @@ void CameraLens::computeSceneBoundingVolume(QNodeId entityId,
     m_renderAspect->scheduleSingleShotJob(job);
 }
 
-void CameraLens::notifySceneBoundingVolume(const Sphere &sphere, QNodeCommand::CommandId commandId)
+void CameraLens::processViewAllResult(QAspectManager *aspectManager, const Sphere &sphere, QNodeId commandId)
 {
-    if (!m_pendingViewAllCommand || m_pendingViewAllCommand.commandId != commandId)
+    if (!m_pendingViewAllRequest || m_pendingViewAllRequest.requestId != commandId)
         return;
     if (sphere.radius() > 0.f) {
-        QVector<float> data = { sphere.center().x(), sphere.center().y(), sphere.center().z(),
-                                sphere.radius() };
-        QVariant v;
-        v.setValue(data);
-        sendCommand(QLatin1String("ViewAll"), v, m_pendingViewAllCommand.commandId);
+        QCameraLens *lens = qobject_cast<QCameraLens *>(aspectManager->lookupNode(peerId()));
+        if (lens) {
+            QCameraLensPrivate *dlens = static_cast<QCameraLensPrivate *>(QCameraLensPrivate::get(lens));
+            dlens->processViewAllResult(m_pendingViewAllRequest.requestId, { sphere.center().x(), sphere.center().y(), sphere.center().z() }, sphere.radius());
+        }
     }
-    m_pendingViewAllCommand = {};
+    m_pendingViewAllRequest = {};
 }
 
 void CameraLens::setProjection(const Matrix4x4 &projection)
diff --git a/src/render/backend/cameralens_p.h b/src/render/backend/cameralens_p.h
index bd721d5e9..4afa38620 100644
--- a/src/render/backend/cameralens_p.h
+++ b/src/render/backend/cameralens_p.h
@@ -52,7 +52,6 @@
 //
 
 #include <Qt3DRender/private/backendnode_p.h>
-#include <Qt3DCore/private/qnodecommand_p.h>
 #include <Qt3DCore/private/matrix4x4_p.h>
 #include <Qt3DRender/private/qcameralens_p.h>
 #include <QRectF>
@@ -99,7 +98,7 @@ public:
     inline float exposure() const { return m_exposure; }
     void syncFromFrontEnd(const Qt3DCore::QNode *frontEnd, bool firstTime) override;
 
-    void notifySceneBoundingVolume(const Sphere &sphere, Qt3DCore::QNodeCommand::CommandId commandId);
+    void processViewAllResult(Qt3DCore::QAspectManager *aspectManager, const Sphere &sphere, Qt3DCore::QNodeId commandId);
 
     static bool viewMatrixForCamera(EntityManager *manager, Qt3DCore::QNodeId cameraId,
                                     Matrix4x4 &viewMatrix, Matrix4x4 &projectionMatrix);
@@ -107,10 +106,10 @@ public:
 private:
     void computeSceneBoundingVolume(Qt3DCore::QNodeId entityId,
                                     Qt3DCore::QNodeId cameraId,
-                                    Qt3DCore::QNodeCommand::CommandId commandId);
+                                    Qt3DCore::QNodeId requestId);
 
     QRenderAspect *m_renderAspect;
-    CameraLensCommand m_pendingViewAllCommand;
+    CameraLensRequest m_pendingViewAllRequest;
     Matrix4x4 m_projection;
     float m_exposure;
 };
diff --git a/src/render/frontend/qcameralens.cpp b/src/render/frontend/qcameralens.cpp
index cf30b714a..1d8059f45 100644
--- a/src/render/frontend/qcameralens.cpp
+++ b/src/render/frontend/qcameralens.cpp
@@ -233,11 +233,7 @@ void QCameraLens::viewAll(Qt3DCore::QNodeId cameraId)
 {
     Q_D(QCameraLens);
     if (d->m_projectionType == PerspectiveProjection) {
-        QVariant v;
-        v.setValue(cameraId);
-        d->m_pendingViewAllCommand = {QLatin1String("QueryRootBoundingVolume"),
-                                      v,
-                                      id()};
+        d->m_pendingViewAllRequest = {Qt3DCore::QNodeId::createId(), cameraId, {}};
         d->update();
     }
 }
@@ -246,30 +242,19 @@ void QCameraLens::viewEntity(Qt3DCore::QNodeId entityId, Qt3DCore::QNodeId camer
 {
     Q_D(QCameraLens);
     if (d->m_projectionType == PerspectiveProjection) {
-        QVector<Qt3DCore::QNodeId> ids = {entityId, cameraId};
-        QVariant v;
-        v.setValue(ids);
-        d->m_pendingViewAllCommand = {QLatin1String("QueryEntityBoundingVolume"),
-                                      v,
-                                      id()};
+        d->m_pendingViewAllRequest = {Qt3DCore::QNodeId::createId(), cameraId, entityId};
         d->update();
     }
 }
 
-void QCameraLensPrivate::processViewAllCommand(Qt3DCore::QNodeCommand::CommandId commandId,
-                                               const QVariant &data)
+void QCameraLensPrivate::processViewAllResult(Qt3DCore::QNodeId requestId, const QVector3D &center, float radius)
 {
     Q_Q(QCameraLens);
-    if (!m_pendingViewAllCommand || m_pendingViewAllCommand.commandId != commandId)
+    if (!m_pendingViewAllRequest || m_pendingViewAllRequest.requestId != requestId)
         return;
 
-    QVector<float> boundingVolumeData = data.value< QVector<float> >();
-    if (boundingVolumeData.size() != 4)
-        return;
-    QVector3D center(boundingVolumeData[0], boundingVolumeData[1], boundingVolumeData[2]);
-    float radius = boundingVolumeData[3];
     Q_EMIT q->viewSphere(center, radius);
-    m_pendingViewAllCommand = {};
+    m_pendingViewAllRequest = {};
 }
 
 /*!
@@ -643,18 +628,7 @@ Qt3DCore::QNodeCreatedChangeBasePtr QCameraLens::createNodeCreationChange() cons
 
 void QCameraLens::sceneChangeEvent(const Qt3DCore::QSceneChangePtr &change)
 {
-    Q_D(QCameraLens);
-    switch (change->type()) {
-    case Qt3DCore::CommandRequested: {
-        Qt3DCore::QNodeCommandPtr command = qSharedPointerCast<Qt3DCore::QNodeCommand>(change);
-
-        if (command->name() == QLatin1String("ViewAll"))
-            d->processViewAllCommand(command->inReplyTo(), command->data());
-    }
-        break;
-    default:
-        break;
-    }
+    Q_UNUSED(change)
 }
 
 } // Qt3DRender
diff --git a/src/render/frontend/qcameralens_p.h b/src/render/frontend/qcameralens_p.h
index 5c5a6a42a..eca01b890 100644
--- a/src/render/frontend/qcameralens_p.h
+++ b/src/render/frontend/qcameralens_p.h
@@ -53,7 +53,6 @@
 
 #include <Qt3DRender/private/qt3drender_global_p.h>
 #include <Qt3DCore/private/qcomponent_p.h>
-#include <Qt3DCore/private/qnodecommand_p.h>
 
 #include "qcameralens.h"
 
@@ -63,21 +62,21 @@ QT_BEGIN_NAMESPACE
 
 namespace Qt3DRender {
 
-struct CameraLensCommand
+struct CameraLensRequest
 {
-    QString name;
-    QVariant data;
-    Qt3DCore::QNodeCommand::CommandId commandId;
+    Qt3DCore::QNodeId requestId;
+    Qt3DCore::QNodeId cameraId;
+    Qt3DCore::QNodeId entityId;
 
-    inline operator bool() const { return !name.isEmpty(); }
+    inline operator bool() const { return !requestId.isNull(); }
 };
 
-inline bool operator ==(const CameraLensCommand &a, const CameraLensCommand &b) noexcept
+inline bool operator ==(const CameraLensRequest &a, const CameraLensRequest &b) noexcept
 {
-    return a.name == b.name && a.data == b.data && a.commandId == b.commandId;
+    return a.cameraId == b.cameraId && a.entityId == b.entityId && a.requestId == b.requestId;
 }
 
-inline bool operator !=(const CameraLensCommand &a, const CameraLensCommand &b) noexcept
+inline bool operator !=(const CameraLensRequest &a, const CameraLensRequest &b) noexcept
 {
     return !(a == b);
 }
@@ -123,8 +122,8 @@ public:
 
     float m_exposure;
 
-    CameraLensCommand m_pendingViewAllCommand;
-    void processViewAllCommand(Qt3DCore::QNodeCommand::CommandId commandId, const QVariant &data);
+    CameraLensRequest m_pendingViewAllRequest;
+    void processViewAllResult(Qt3DCore::QNodeId requestId, const QVector3D &center, float radius);
 
 private:
     inline void updatePerpectiveProjection()
diff --git a/src/render/jobs/computefilteredboundingvolumejob.cpp b/src/render/jobs/computefilteredboundingvolumejob.cpp
index 02852685c..0cdbc8b6d 100644
--- a/src/render/jobs/computefilteredboundingvolumejob.cpp
+++ b/src/render/jobs/computefilteredboundingvolumejob.cpp
@@ -74,12 +74,25 @@ void expandWorldBoundingVolume(NodeManagers *manager,
 
 } // namespace
 
+class ComputeFilteredBoundingVolumeJobPrivate : public Qt3DCore::QAspectJobPrivate
+{
+public:
+    ComputeFilteredBoundingVolumeJobPrivate(ComputeFilteredBoundingVolumeJob *job) : Qt3DCore::QAspectJobPrivate(), m_job(job) {}
+    ~ComputeFilteredBoundingVolumeJobPrivate() {}
+
+    void postFrame(Qt3DCore::QAspectManager *aspectManager) override;
+
+    ComputeFilteredBoundingVolumeJob *m_job;
+    Qt3DRender::Render::Sphere m_sphere;
+};
+
 ComputeFilteredBoundingVolumeJob::ComputeFilteredBoundingVolumeJob()
-    : m_root(nullptr)
+    : Qt3DCore::QAspectJob(*new ComputeFilteredBoundingVolumeJobPrivate(this))
+    , m_root(nullptr)
     , m_ignoreSubTree(nullptr)
     , m_manager(nullptr)
 {
-    SET_JOB_RUN_STAT_TYPE(this, JobTypes::ExpandBoundingVolume, 0);
+    SET_JOB_RUN_STAT_TYPE(this, JobTypes::ExpandBoundingVolume, 0)
 }
 
 void ComputeFilteredBoundingVolumeJob::setRoot(Entity *root)
@@ -100,11 +113,13 @@ void ComputeFilteredBoundingVolumeJob::ignoreSubTree(Entity *node)
 void ComputeFilteredBoundingVolumeJob::run()
 {
     qCDebug(Jobs) << "Entering" << Q_FUNC_INFO << QThread::currentThread();
+    Q_D(ComputeFilteredBoundingVolumeJob);
+    d->m_sphere = {};
 
     if (!m_root)
         return;
     if (!m_ignoreSubTree) {
-        finished(*m_root->worldBoundingVolumeWithChildren());
+        d->m_sphere = *m_root->worldBoundingVolumeWithChildren();
         return;
     }
 
@@ -118,20 +133,24 @@ void ComputeFilteredBoundingVolumeJob::run()
         parent = parent->parent();
     }
     if (!isFilterChildOfRoot) {
-        finished(*m_root->worldBoundingVolumeWithChildren());
+        d->m_sphere = *m_root->worldBoundingVolumeWithChildren();
         return;
     }
 
-    Qt3DRender::Render::Sphere sphere;
-    expandWorldBoundingVolume(m_manager, &sphere, m_root, m_ignoreSubTree);
-    finished(sphere);
+    expandWorldBoundingVolume(m_manager, &d->m_sphere, m_root, m_ignoreSubTree);
 
     qCDebug(Jobs) << "Exiting" << Q_FUNC_INFO << QThread::currentThread();
 }
 
-void ComputeFilteredBoundingVolumeJob::finished(const Qt3DRender::Render::Sphere &sphere)
+void ComputeFilteredBoundingVolumeJob::finished(Qt3DCore::QAspectManager *aspectManager, const Qt3DRender::Render::Sphere &sphere)
+{
+    Q_UNUSED(aspectManager)
+    Q_UNUSED(sphere)
+}
+
+void ComputeFilteredBoundingVolumeJobPrivate::postFrame(Qt3DCore::QAspectManager *aspectManager)
 {
-    Q_UNUSED(sphere);
+    m_job->finished(aspectManager, m_sphere);
 }
 
 } // namespace Render
diff --git a/src/render/jobs/computefilteredboundingvolumejob_p.h b/src/render/jobs/computefilteredboundingvolumejob_p.h
index d2aca575c..0f804e647 100644
--- a/src/render/jobs/computefilteredboundingvolumejob_p.h
+++ b/src/render/jobs/computefilteredboundingvolumejob_p.h
@@ -64,6 +64,7 @@ namespace Render {
 class Entity;
 class NodeManagers;
 class Sphere;
+class ComputeFilteredBoundingVolumeJobPrivate;
 
 class Q_3DRENDERSHARED_PRIVATE_EXPORT ComputeFilteredBoundingVolumeJob : public Qt3DCore::QAspectJob
 {
@@ -76,9 +77,10 @@ public:
     void run() override;
 
 protected:
-    virtual void finished(const Qt3DRender::Render::Sphere &sphere);
+    virtual void finished(Qt3DCore::QAspectManager *aspectManager, const Qt3DRender::Render::Sphere &sphere);    // called in main thread
 
 private:
+    Q_DECLARE_PRIVATE(ComputeFilteredBoundingVolumeJob)
     Entity *m_root;
     Entity *m_ignoreSubTree;
     NodeManagers *m_manager;