Do direct notification of backend nodeswip/refactor

Since aspect manager is now on main thread, we can directly update
backend nodes safely. Track nodes which have changed properties and
notify the backend nodes as part of the frame loop.
This avoid allocating and delivering many change messages.

To follow:
- implement on all nodes
- look at backend to frontend syncing
- figure out what to do with non property messages (components
  added/removed, commands, ...)

Change-Id: Ia0c442b0528e728c4324d168200bae021bc29266
Reviewed-by: Mike Krus <mike.krus@kdab.com>
Reviewed-by: Paul Lemire <paul.lemire@kdab.com>

6 files changed, 129 insertions, 16 deletions

diff --git a/src/core/aspects/qabstractaspect.cpp b/src/core/aspects/qabstractaspect.cpp
index 7b08dec81..7636fd666 100644
--- a/src/core/aspects/qabstractaspect.cpp
+++ b/src/core/aspects/qabstractaspect.cpp
@@ -40,6 +40,9 @@
 #include "qabstractaspect.h"
 #include "qabstractaspect_p.h"
 
+#include <QMetaObject>
+#include <QMetaProperty>
+
 #include <Qt3DCore/qentity.h>
 #include <Qt3DCore/qpropertyupdatedchange.h>
 
@@ -48,6 +51,7 @@
 #include <Qt3DCore/private/qaspectmanager_p.h>
 #include <Qt3DCore/private/qchangearbiter_p.h>
 #include <Qt3DCore/private/qnodevisitor_p.h>
+#include <Qt3DCore/private/qnode_p.h>
 #include <Qt3DCore/private/qscene_p.h>
 
 QT_BEGIN_NAMESPACE
@@ -170,7 +174,14 @@ QNodeId QAbstractAspect::rootEntityId() const Q_DECL_NOEXCEPT
 void QAbstractAspect::registerBackendType(const QMetaObject &obj, const QBackendNodeMapperPtr &functor)
 {
     Q_D(QAbstractAspect);
-    d->m_backendCreatorFunctors.insert(&obj, functor);
+    d->m_backendCreatorFunctors.insert(&obj, {functor, QAbstractAspectPrivate::DefaultMapper});
+}
+
+void QAbstractAspect::registerBackendType(const QMetaObject &obj, const QBackendNodeMapperPtr &functor, bool supportsSyncing)
+{
+    Q_D(QAbstractAspect);
+    const auto f = supportsSyncing ? QAbstractAspectPrivate::SupportsSyncing : QAbstractAspectPrivate::DefaultMapper;
+    d->m_backendCreatorFunctors.insert(&obj, {functor, f});
 }
 
 void QAbstractAspect::unregisterBackendType(const QMetaObject &obj)
@@ -203,12 +214,96 @@ QVector<QAspectJobPtr> QAbstractAspect::jobsToExecute(qint64 time)
     return QVector<QAspectJobPtr>();
 }
 
+void QAbstractAspect::syncDirtyFrontEndNodes(const QVector<QNode *> &nodes)
+{
+    Q_D(QAbstractAspect);
+    d->syncDirtyFrontEndNodes(nodes);
+}
+
+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();
+        }
+
+        if (!backendNodeMapper)
+            continue;
+
+        QBackendNode *backend = backendNodeMapper->get(node->id());
+        if (!backend)
+            continue;
+        if (supportsSyncing)
+            syncDirtyFrontEndNode(node, backend, false);
+        else
+            sendPropertyMessages(node, backend);
+    }
+}
+
+void QAbstractAspectPrivate::syncDirtyFrontEndNode(QNode *node, QBackendNode *backend, bool firstTime) const
+{
+    Q_UNUSED(firstTime);
+    Q_ASSERT(false); // overload in derived class
+    sendPropertyMessages(node, backend);
+}
+
+void QAbstractAspectPrivate::sendPropertyMessages(QNode *node, QBackendNode *backend) const
+{
+    const int offset = QNode::staticMetaObject.propertyOffset();
+    const auto metaObj = node->metaObject();
+    const int count = metaObj->propertyCount();
+
+    const auto toBackendValue = [](const QVariant &data) -> QVariant
+    {
+        if (data.canConvert<QNode*>()) {
+            QNode *node = data.value<QNode*>();
+
+            // Ensure the node and all ancestors have issued their node creation changes.
+            // We can end up here if a newly created node with a parent is immediately set
+            // as a property on another node. In this case the deferred call to
+            // _q_postConstructorInit() will not have happened yet as the event
+            // loop will still be blocked. We need to do this for all ancestors,
+            // since the subtree of this node otherwise can end up on the backend
+            // with a reference to a non-existent parent.
+            if (node)
+                QNodePrivate::get(node)->_q_ensureBackendNodeCreated();
+
+            const QNodeId id = node ? node->id() : QNodeId();
+            return QVariant::fromValue(id);
+        }
+
+        return data;
+    };
+
+    QPropertyUpdatedChange change(node->id());
+    QPropertyUpdatedChangePtr pchange(&change, [](QPropertyUpdatedChange *) { });
+    for (int index = offset; index < count; index++) {
+        const QMetaProperty pro = metaObj->property(index);
+        change.setPropertyName(pro.name());
+        change.setValue(toBackendValue(pro.read(node)));
+        backend->sceneChangeEvent(pchange);
+    }
+
+    auto const dynamicProperties = node->dynamicPropertyNames();
+    for (const QByteArray &name: dynamicProperties) {
+        change.setPropertyName(name.data());
+        change.setValue(toBackendValue(node->property(name.data())));
+        backend->sceneChangeEvent(pchange);
+    }
+}
+
 QBackendNode *QAbstractAspectPrivate::createBackendNode(const QNodeCreatedChangeBasePtr &change) const
 {
     const QMetaObject *metaObj = change->metaObject();
     QBackendNodeMapperPtr backendNodeMapper;
     while (metaObj != nullptr && backendNodeMapper.isNull()) {
-        backendNodeMapper = m_backendCreatorFunctors.value(metaObj);
+        backendNodeMapper = m_backendCreatorFunctors.value(metaObj).first;
         metaObj = metaObj->superClass();
     }
 
@@ -248,6 +343,7 @@ QBackendNode *QAbstractAspectPrivate::createBackendNode(const QNodeCreatedChange
     return backend;
 }
 
+
 void QAbstractAspectPrivate::clearBackendNode(const QNodeDestroyedChangePtr &change) const
 {
     // Each QNodeDestroyedChange may contain info about a whole sub-tree of nodes that
@@ -259,7 +355,7 @@ void QAbstractAspectPrivate::clearBackendNode(const QNodeDestroyedChangePtr &cha
 
         // Find backend node mapper for this type
         while (metaObj != nullptr && backendNodeMapper.isNull()) {
-            backendNodeMapper = m_backendCreatorFunctors.value(metaObj);
+            backendNodeMapper = m_backendCreatorFunctors.value(metaObj).first;
             metaObj = metaObj->superClass();
         }
 
diff --git a/src/core/aspects/qabstractaspect.h b/src/core/aspects/qabstractaspect.h
index 1ae85e4e6..a9f4f03fc 100644
--- a/src/core/aspects/qabstractaspect.h
+++ b/src/core/aspects/qabstractaspect.h
@@ -77,12 +77,17 @@ protected:
 
     template<class Frontend>
     void registerBackendType(const QBackendNodeMapperPtr &functor);
-    void registerBackendType(const QMetaObject &, const QBackendNodeMapperPtr &functor);
+    template<class Frontend, bool supportsSyncing>
+    void registerBackendType(const QBackendNodeMapperPtr &functor);
+    void registerBackendType(const QMetaObject &obj, const QBackendNodeMapperPtr &functor);
     template<class Frontend>
     void unregisterBackendType();
     void unregisterBackendType(const QMetaObject &);
 
 private:
+    void syncDirtyFrontEndNodes(const QVector<QNode *> &nodes);
+    void registerBackendType(const QMetaObject &obj, const QBackendNodeMapperPtr &functor, bool supportsSyncing);
+
     virtual QVariant executeCommand(const QStringList &args);
 
     virtual QVector<QAspectJobPtr> jobsToExecute(qint64 time);
@@ -104,6 +109,12 @@ void QAbstractAspect::registerBackendType(const QBackendNodeMapperPtr &functor)
     registerBackendType(Frontend::staticMetaObject, functor);
 }
 
+template<class Frontend, bool supportsSyncing>
+void QAbstractAspect::registerBackendType(const QBackendNodeMapperPtr &functor)
+{
+    registerBackendType(Frontend::staticMetaObject, functor, supportsSyncing);
+}
+
 template<class Frontend>
 void QAbstractAspect::unregisterBackendType()
 {
diff --git a/src/core/aspects/qabstractaspect_p.h b/src/core/aspects/qabstractaspect_p.h
index e743dc63a..668b9670c 100644
--- a/src/core/aspects/qabstractaspect_p.h
+++ b/src/core/aspects/qabstractaspect_p.h
@@ -121,6 +121,9 @@ public:
 
     QBackendNode *createBackendNode(const QNodeCreatedChangeBasePtr &change) const override;
     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;
@@ -134,12 +137,17 @@ public:
 
     Q_DECLARE_PUBLIC(QAbstractAspect)
 
+    enum NodeMapperInfo {
+        DefaultMapper = 0,
+        SupportsSyncing = 1 << 0
+    };
+
     QEntity *m_root;
     QNodeId m_rootId;
     QAspectManager *m_aspectManager;
     QAbstractAspectJobManager *m_jobManager;
     QChangeArbiter *m_arbiter;
-    QHash<const QMetaObject*, QBackendNodeMapperPtr> m_backendCreatorFunctors;
+    QHash<const QMetaObject*, QPair<QBackendNodeMapperPtr, NodeMapperInfo>> m_backendCreatorFunctors;
     QMutex m_singleShotMutex;
     QVector<QAspectJobPtr> m_singleShotJobs;
 
diff --git a/src/core/aspects/qaspectengine.cpp b/src/core/aspects/qaspectengine.cpp
index 9c98e9613..c5e4b2b26 100644
--- a/src/core/aspects/qaspectengine.cpp
+++ b/src/core/aspects/qaspectengine.cpp
@@ -120,12 +120,6 @@ void QAspectEnginePrivate::initEntity(QEntity *entity)
     }
 }
 
-void QAspectEnginePrivate::generateCreationChanges(QNode *root)
-{
-    const QNodeCreatedChangeGenerator generator(root);
-    m_creationChanges = generator.creationChanges();
-}
-
 /*!
  * \class Qt3DCore::QAspectEngine
  * \inheaderfile Qt3DCore/QAspectEngine
@@ -444,7 +438,8 @@ void QAspectEngine::setRootEntity(QEntityPtr root)
     d->initNodeTree(root.data());
 
     // Traverse tree to generate a vector of creation changes
-    d->generateCreationChanges(root.data());
+    const QNodeCreatedChangeGenerator generator(root.data());
+    auto creationChanges = generator.creationChanges();
 
     // Specify if the AspectManager should be driving the simulation loop or not
     d->m_aspectManager->setRunMode(d->m_runMode);
@@ -456,7 +451,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(), d->m_creationChanges);
+    d->m_aspectManager->setRootEntity(root.data(), creationChanges);
     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 e1aaa9de9..7dad2a352 100644
--- a/src/core/aspects/qaspectengine_p.h
+++ b/src/core/aspects/qaspectengine_p.h
@@ -106,9 +106,6 @@ public:
     void initNode(QNode *node);
     void initEntity(QEntity *entity);
 
-    void generateCreationChanges(QNode *rootNode);
-    QVector<QNodeCreatedChangeBasePtr> m_creationChanges;
-
     static QAspectEnginePrivate *get(QAspectEngine *engine);
 };
 
diff --git a/src/core/aspects/qaspectmanager.cpp b/src/core/aspects/qaspectmanager.cpp
index 6502590d8..0c16de882 100644
--- a/src/core/aspects/qaspectmanager.cpp
+++ b/src/core/aspects/qaspectmanager.cpp
@@ -370,6 +370,10 @@ void QAspectManager::processFrame()
     changeArbiterStats.threadId = reinterpret_cast<quint64>(QThread::currentThreadId());
     changeArbiterStats.startTime = QThreadPooler::m_jobsStatTimer.nsecsElapsed();
 #endif
+    const auto dirtyFrontEndNodes = m_changeArbiter->takeDirtyFrontEndNodes();
+    if (dirtyFrontEndNodes.size())
+        for (QAbstractAspect *aspect : qAsConst(m_aspects))
+           aspect->syncDirtyFrontEndNodes(dirtyFrontEndNodes);
     // TO DO: Having this done in the main thread actually means aspects could just
     // as simply read info out of the Frontend classes without risk of introducing
     // races. This could therefore be removed for Qt 6.
@@ -390,6 +394,8 @@ void QAspectManager::processFrame()
 #if defined(QT3D_CORE_JOB_TIMING)
     qDebug() << "Jobs took" << timer.nsecsElapsed() / 1.0e6;
 #endif
+
+    // TODO sync backend changes to frontend
 }
 
 } // namespace Qt3DCore