Use sync method when initializing new backend nodes

If the node type supports syncing, use that rather than the creation
message. The message is still needed since that is passed to the
instantiation functor (none of qt3d's classes appear to use anything
but the node id, but can't be sure no other classes do, and can't add
other virtual method without breaking BC).

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

6 files changed, 109 insertions, 44 deletions

diff --git a/src/core/aspects/qabstractaspect.cpp b/src/core/aspects/qabstractaspect.cpp
index 7636fd666..2ad3248e5 100644
--- a/src/core/aspects/qabstractaspect.cpp
+++ b/src/core/aspects/qabstractaspect.cpp
@@ -53,6 +53,7 @@
 #include <Qt3DCore/private/qnodevisitor_p.h>
 #include <Qt3DCore/private/qnode_p.h>
 #include <Qt3DCore/private/qscene_p.h>
+#include <Qt3DCore/private/qnode_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -190,12 +191,6 @@ void QAbstractAspect::unregisterBackendType(const QMetaObject &obj)
     d->m_backendCreatorFunctors.remove(&obj);
 }
 
-void QAbstractAspectPrivate::sceneNodeAdded(QSceneChangePtr &change)
-{
-    QNodeCreatedChangeBasePtr creationChange = qSharedPointerCast<QNodeCreatedChangeBase>(change);
-    createBackendNode(creationChange);
-}
-
 void QAbstractAspectPrivate::sceneNodeRemoved(QSceneChangePtr &change)
 {
     QNodeDestroyedChangePtr destructionChange = qSharedPointerCast<QNodeDestroyedChange>(change);
@@ -220,18 +215,23 @@ void QAbstractAspect::syncDirtyFrontEndNodes(const QVector<QNode *> &nodes)
     d->syncDirtyFrontEndNodes(nodes);
 }
 
+QAbstractAspectPrivate::BackendNodeMapperAndInfo QAbstractAspectPrivate::mapperForNode(QNode *n) const
+{
+    const QMetaObject *metaObj = n->metaObject();
+    BackendNodeMapperAndInfo info;
+
+    while (metaObj != nullptr && info.first.isNull()) {
+        info = m_backendCreatorFunctors.value(metaObj);
+        metaObj = metaObj->superClass();
+    }
+    return info;
+}
+
 void QAbstractAspectPrivate::syncDirtyFrontEndNodes(const QVector<QNode *> &nodes)
 {
     for (auto node: qAsConst(nodes)) {
-        const QMetaObject *metaObj = node->metaObject();
-        QBackendNodeMapperPtr backendNodeMapper;
-        bool supportsSyncing = false;
-        while (metaObj != nullptr && backendNodeMapper.isNull()) {
-            auto v = m_backendCreatorFunctors.value(metaObj);
-            backendNodeMapper = v.first;
-            supportsSyncing = v.second & SupportsSyncing;
-            metaObj = metaObj->superClass();
-        }
+        const BackendNodeMapperAndInfo backendNodeMapperInfo = mapperForNode(node);
+        const QBackendNodeMapperPtr backendNodeMapper = backendNodeMapperInfo.first;
 
         if (!backendNodeMapper)
             continue;
@@ -239,6 +239,8 @@ void QAbstractAspectPrivate::syncDirtyFrontEndNodes(const QVector<QNode *> &node
         QBackendNode *backend = backendNodeMapper->get(node->id());
         if (!backend)
             continue;
+
+        const bool supportsSyncing = backendNodeMapperInfo.second & SupportsSyncing;
         if (supportsSyncing)
             syncDirtyFrontEndNode(node, backend, false);
         else
@@ -248,9 +250,9 @@ void QAbstractAspectPrivate::syncDirtyFrontEndNodes(const QVector<QNode *> &node
 
 void QAbstractAspectPrivate::syncDirtyFrontEndNode(QNode *node, QBackendNode *backend, bool firstTime) const
 {
-    Q_UNUSED(firstTime);
     Q_ASSERT(false); // overload in derived class
-    sendPropertyMessages(node, backend);
+    if (!firstTime)
+        sendPropertyMessages(node, backend);
 }
 
 void QAbstractAspectPrivate::sendPropertyMessages(QNode *node, QBackendNode *backend) const
@@ -300,50 +302,66 @@ void QAbstractAspectPrivate::sendPropertyMessages(QNode *node, QBackendNode *bac
 
 QBackendNode *QAbstractAspectPrivate::createBackendNode(const QNodeCreatedChangeBasePtr &change) const
 {
-    const QMetaObject *metaObj = change->metaObject();
-    QBackendNodeMapperPtr backendNodeMapper;
-    while (metaObj != nullptr && backendNodeMapper.isNull()) {
-        backendNodeMapper = m_backendCreatorFunctors.value(metaObj).first;
-        metaObj = metaObj->superClass();
-    }
+    Q_UNUSED(change);
+    Q_ASSERT(false); // should not be here
+    return nullptr;
+}
+
+QBackendNode *QAbstractAspectPrivate::createBackendNode(QNode *node) const
+{
+    const BackendNodeMapperAndInfo backendNodeMapperInfo = mapperForNode(node);
+    const QBackendNodeMapperPtr backendNodeMapper = backendNodeMapperInfo.first;
 
     if (!backendNodeMapper)
         return nullptr;
 
-    QBackendNode *backend = backendNodeMapper->get(change->subjectId());
+    QBackendNode *backend = backendNodeMapper->get(node->id());
     if (backend != nullptr)
         return backend;
-    backend = backendNodeMapper->create(change);
+
+    QNodeCreatedChangeBasePtr change;
+    const bool supportsSyncing = backendNodeMapperInfo.second & SupportsSyncing;
+    if (supportsSyncing) {
+        // All objects modified to use syncing should only use the id in the creation functor
+        QNodeCreatedChangeBase changeObj(node);
+        change = QNodeCreatedChangeBasePtr(&changeObj, [](QNodeCreatedChangeBase *) {});
+        backend = backendNodeMapper->create(change);
+    } else {
+        change = node->createNodeCreationChange();
+        backend = backendNodeMapper->create(change);
+    }
 
     if (!backend)
         return nullptr;
 
     // TODO: Find some place else to do all of this function from the arbiter
-    backend->setPeerId(change->subjectId());
+    backend->setPeerId(node->id());
 
     // Backend could be null if the user decides that his functor should only
     // perform some action when encountering a given type of item but doesn't need to
     // return a QBackendNode pointer.
 
     QBackendNodePrivate *backendPriv = QBackendNodePrivate::get(backend);
-    backendPriv->setEnabled(change->isNodeEnabled());
+    backendPriv->setEnabled(node->isEnabled());
 
     // TO DO: Find a way to specify the changes to observe
     // Register backendNode with QChangeArbiter
     if (m_arbiter != nullptr) { // Unit tests may not have the arbiter registered
         qCDebug(Nodes) << q_func()->objectName() << "Creating backend node for node id"
-                       << change->subjectId() << "of type" << change->metaObject()->className();
+                       << node->id() << "of type" << node->metaObject()->className();
         m_arbiter->registerObserver(backendPriv, backend->peerId(), AllChanges);
         if (backend->mode() == QBackendNode::ReadWrite)
             m_arbiter->scene()->addObservable(backendPriv, backend->peerId());
     }
 
-    backend->initializeFromPeer(change);
+    if (supportsSyncing)
+        syncDirtyFrontEndNode(node, backend, true);
+    else
+        backend->initializeFromPeer(change);
 
     return backend;
 }
 
-
 void QAbstractAspectPrivate::clearBackendNode(const QNodeDestroyedChangePtr &change) const
 {
     // Each QNodeDestroyedChange may contain info about a whole sub-tree of nodes that
@@ -376,7 +394,7 @@ void QAbstractAspectPrivate::clearBackendNode(const QNodeDestroyedChangePtr &cha
     }
 }
 
-void QAbstractAspectPrivate::setRootAndCreateNodes(QEntity *rootObject, const QVector<QNodeCreatedChangeBasePtr> &changes)
+void QAbstractAspectPrivate::setRootAndCreateNodes(QEntity *rootObject, const QVector<QNode *> &nodes)
 {
     qCDebug(Aspects) << Q_FUNC_INFO << "rootObject =" << rootObject;
     if (rootObject == m_root)
@@ -385,8 +403,13 @@ void QAbstractAspectPrivate::setRootAndCreateNodes(QEntity *rootObject, const QV
     m_root = rootObject;
     m_rootId = rootObject->id();
 
-    for (const auto &change : changes)
-        createBackendNode(change);
+    createNodes(nodes);
+}
+
+void QAbstractAspectPrivate::createNodes(const QVector<QNode *> &nodes)
+{
+    for (const auto node : nodes)
+        createBackendNode(node);
 }
 
 QServiceLocator *QAbstractAspectPrivate::services() const
diff --git a/src/core/aspects/qabstractaspect_p.h b/src/core/aspects/qabstractaspect_p.h
index 668b9670c..ea2712fe5 100644
--- a/src/core/aspects/qabstractaspect_p.h
+++ b/src/core/aspects/qabstractaspect_p.h
@@ -112,7 +112,8 @@ public:
     QAbstractAspectPrivate();
     ~QAbstractAspectPrivate();
 
-    void setRootAndCreateNodes(QEntity *rootObject, const QVector<Qt3DCore::QNodeCreatedChangeBasePtr> &changes);
+    void setRootAndCreateNodes(QEntity *rootObject, const QVector<QNode *> &nodes);
+    void createNodes(const QVector<QNode *> &nodes);
 
     QServiceLocator *services() const;
     QAbstractAspectJobManager *jobManager() const;
@@ -120,12 +121,12 @@ public:
     QVector<QAspectJobPtr> jobsToExecute(qint64 time) override;
 
     QBackendNode *createBackendNode(const QNodeCreatedChangeBasePtr &change) const override;
+    QBackendNode *createBackendNode(QNode *node) const;
     void clearBackendNode(const QNodeDestroyedChangePtr &change) const;
     void syncDirtyFrontEndNodes(const QVector<QNode *> &nodes);
     virtual void syncDirtyFrontEndNode(QNode *node, QBackendNode *backend, bool firstTime) const;
     void sendPropertyMessages(QNode *node, QBackendNode *backend) const;
 
-    void sceneNodeAdded(Qt3DCore::QSceneChangePtr &e) override;
     void sceneNodeRemoved(Qt3DCore::QSceneChangePtr &e) override;
 
     virtual void onEngineAboutToShutdown();
@@ -141,13 +142,15 @@ public:
         DefaultMapper = 0,
         SupportsSyncing = 1 << 0
     };
+    using BackendNodeMapperAndInfo = QPair<QBackendNodeMapperPtr, NodeMapperInfo>;
+    BackendNodeMapperAndInfo mapperForNode(QNode *n) const;
 
     QEntity *m_root;
     QNodeId m_rootId;
     QAspectManager *m_aspectManager;
     QAbstractAspectJobManager *m_jobManager;
     QChangeArbiter *m_arbiter;
-    QHash<const QMetaObject*, QPair<QBackendNodeMapperPtr, NodeMapperInfo>> m_backendCreatorFunctors;
+    QHash<const QMetaObject*, BackendNodeMapperAndInfo> m_backendCreatorFunctors;
     QMutex m_singleShotMutex;
     QVector<QAspectJobPtr> m_singleShotJobs;
 
diff --git a/src/core/aspects/qaspectengine.cpp b/src/core/aspects/qaspectengine.cpp
index c5e4b2b26..41662e2a3 100644
--- a/src/core/aspects/qaspectengine.cpp
+++ b/src/core/aspects/qaspectengine.cpp
@@ -52,7 +52,6 @@
 #include <Qt3DCore/private/qeventfilterservice_p.h>
 #include <Qt3DCore/private/qnode_p.h>
 #include <Qt3DCore/private/qnodevisitor_p.h>
-#include <Qt3DCore/private/qnodecreatedchangegenerator_p.h>
 #include <Qt3DCore/private/qpostman_p.h>
 #include <Qt3DCore/private/qscene_p.h>
 #include <Qt3DCore/private/qservicelocator_p.h>
@@ -64,6 +63,34 @@
 
 QT_BEGIN_NAMESPACE
 
+namespace{
+
+QVector<Qt3DCore::QNode *> getNodesForCreation(Qt3DCore::QNode *root)
+{
+    using namespace Qt3DCore;
+
+    QVector<QNode *> nodes;
+    QNodeVisitor visitor;
+    visitor.traverse(root, [&nodes](QNode *node) {
+        nodes.append(node);
+
+        // Store the metaobject of the node in the QNode so that we have it available
+        // to us during destruction in the QNode destructor. This allows us to send
+        // the QNodeId and the metaobject as typeinfo to the backend aspects so they
+        // in turn can find the correct QBackendNodeMapper object to handle the destruction
+        // of the corresponding backend nodes.
+        QNodePrivate *d = QNodePrivate::get(node);
+        d->m_typeInfo = const_cast<QMetaObject*>(QNodePrivate::findStaticMetaObject(node->metaObject()));
+
+        // Mark this node as having been handled for creation so that it is picked up
+        d->m_hasBackendNode = true;
+    });
+
+    return nodes;
+}
+
+}
+
 namespace Qt3DCore {
 
 QAspectEnginePrivate *QAspectEnginePrivate::get(QAspectEngine *q)
@@ -120,6 +147,11 @@ void QAspectEnginePrivate::initEntity(QEntity *entity)
     }
 }
 
+void QAspectEnginePrivate::addNode(QNode *node)
+{
+    m_aspectManager->addNodes(getNodesForCreation(node));
+}
+
 /*!
  * \class Qt3DCore::QAspectEngine
  * \inheaderfile Qt3DCore/QAspectEngine
@@ -437,9 +469,7 @@ void QAspectEngine::setRootEntity(QEntityPtr root)
     // deregister the nodes from the scene
     d->initNodeTree(root.data());
 
-    // Traverse tree to generate a vector of creation changes
-    const QNodeCreatedChangeGenerator generator(root.data());
-    auto creationChanges = generator.creationChanges();
+    const QVector<QNode *> nodes = getNodesForCreation(root.data());
 
     // Specify if the AspectManager should be driving the simulation loop or not
     d->m_aspectManager->setRunMode(d->m_runMode);
@@ -451,7 +481,7 @@ void QAspectEngine::setRootEntity(QEntityPtr root)
     // TODO: Pass the creation changes via the arbiter rather than relying upon
     // an invokeMethod call.
     qCDebug(Aspects) << "Begin setting scene root on aspect manager";
-    d->m_aspectManager->setRootEntity(root.data(), creationChanges);
+    d->m_aspectManager->setRootEntity(root.data(), nodes);
     qCDebug(Aspects) << "Done setting scene root on aspect manager";
     d->m_aspectManager->enterSimulationLoop();
 }
diff --git a/src/core/aspects/qaspectengine_p.h b/src/core/aspects/qaspectengine_p.h
index 7dad2a352..3e68a2caa 100644
--- a/src/core/aspects/qaspectengine_p.h
+++ b/src/core/aspects/qaspectengine_p.h
@@ -105,6 +105,7 @@ public:
     void initNodeTree(QNode *node);
     void initNode(QNode *node);
     void initEntity(QEntity *entity);
+    void addNode(QNode *node);
 
     static QAspectEnginePrivate *get(QAspectEngine *engine);
 };
diff --git a/src/core/aspects/qaspectmanager.cpp b/src/core/aspects/qaspectmanager.cpp
index 0c16de882..b6e6122a1 100644
--- a/src/core/aspects/qaspectmanager.cpp
+++ b/src/core/aspects/qaspectmanager.cpp
@@ -60,6 +60,7 @@
 #include <Qt3DCore/private/qthreadpooler_p.h>
 #include <Qt3DCore/private/qtickclock_p.h>
 #include <Qt3DCore/private/qtickclockservice_p.h>
+#include <Qt3DCore/private/qnodevisitor_p.h>
 
 #include <QtCore/QCoreApplication>
 
@@ -230,7 +231,7 @@ void QAspectManager::shutdown()
 }
 
 // MainThread called by QAspectEngine::setRootEntity
-void QAspectManager::setRootEntity(Qt3DCore::QEntity *root, const QVector<Qt3DCore::QNodeCreatedChangeBasePtr> &changes)
+void QAspectManager::setRootEntity(Qt3DCore::QEntity *root, const QVector<QNode *> &nodes)
 {
     qCDebug(Aspects) << Q_FUNC_INFO;
 
@@ -246,10 +247,16 @@ void QAspectManager::setRootEntity(Qt3DCore::QEntity *root, const QVector<Qt3DCo
 
     if (m_root) {
         for (QAbstractAspect *aspect : qAsConst(m_aspects))
-            aspect->d_func()->setRootAndCreateNodes(m_root, changes);
+            aspect->d_func()->setRootAndCreateNodes(m_root, nodes);
     }
 }
 
+void QAspectManager::addNodes(const QVector<QNode *> &nodes)
+{
+    for (QAbstractAspect *aspect : qAsConst(m_aspects))
+        aspect->d_func()->createNodes(nodes);
+}
+
 /*!
  * \internal
  *
diff --git a/src/core/aspects/qaspectmanager_p.h b/src/core/aspects/qaspectmanager_p.h
index d654f7b5a..e984244ff 100644
--- a/src/core/aspects/qaspectmanager_p.h
+++ b/src/core/aspects/qaspectmanager_p.h
@@ -94,7 +94,8 @@ public Q_SLOTS:
     void shutdown();
     void processFrame();
 
-    void setRootEntity(Qt3DCore::QEntity *root, const QVector<Qt3DCore::QNodeCreatedChangeBasePtr> &changes);
+    void setRootEntity(Qt3DCore::QEntity *root, const QVector<QNode *> &nodes);
+    void addNodes(const QVector<QNode *> &nodes);
     void registerAspect(Qt3DCore::QAbstractAspect *aspect);
     void unregisterAspect(Qt3DCore::QAbstractAspect *aspect);