Update LoadSkeletonJob to use direct sync

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

9 files changed, 66 insertions, 115 deletions

diff --git a/src/animation/backend/skeleton.cpp b/src/animation/backend/skeleton.cpp
index 2d395848c..60e436a71 100644
--- a/src/animation/backend/skeleton.cpp
+++ b/src/animation/backend/skeleton.cpp
@@ -36,6 +36,7 @@
 
 #include "skeleton_p.h"
 #include <Qt3DCore/qpropertyupdatedchange.h>
+#include <Qt3DCore/private/qabstractskeleton_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -62,31 +63,20 @@ void Skeleton::cleanup()
     m_jointLocalPoses.clear();
 }
 
-// TODOSYNC remove once backend > backend communication no longer requires messages
-void Skeleton::sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e)
+void Skeleton::syncFromFrontEnd(const Qt3DCore::QNode *frontEnd, bool firstTime)
 {
-    // Get the joint names and initial local poses from a change sent
-    // by the render aspect when the skeleton has been loaded.
-    switch (e->type()) {
-    case PropertyUpdated: {
-        const auto change = qSharedPointerCast<QPropertyUpdatedChange>(e);
-        if (change->propertyName() == QByteArrayLiteral("jointNamesAndLocalPoses")) {
-            const auto payload = change->value().value<JointNamesAndLocalPoses>();
-            m_jointNames = payload.names;
-            m_jointLocalPoses = payload.localPoses;
+    BackendNode::syncFromFrontEnd(frontEnd, firstTime);
 
-            // TODO: Mark joint info as dirty so we can rebuild any indexes used
-            // by the animators and channel mappings.
-        }
+    const Qt3DCore::QAbstractSkeleton *node = qobject_cast<const Qt3DCore::QAbstractSkeleton *>(frontEnd);
+    if (!node)
+        return;
 
-        break;
-    }
+    auto dnode = Qt3DCore::QAbstractSkeletonPrivate::get(node);
 
-    default:
-        break;
-    }
-
-    BackendNode::sceneChangeEvent(e);
+    // TODO: Mark joint info as dirty so we can rebuild any indexes used
+    // by the animators and channel mappings.
+    m_jointNames = dnode->m_jointNames;
+    m_jointLocalPoses = dnode->m_localPoses;
 }
 
 } // namespace Animation
diff --git a/src/animation/backend/skeleton_p.h b/src/animation/backend/skeleton_p.h
index 544eb08be..d44f8fc1c 100644
--- a/src/animation/backend/skeleton_p.h
+++ b/src/animation/backend/skeleton_p.h
@@ -62,7 +62,7 @@ public:
     Skeleton();
 
     void cleanup();
-    void sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e) Q_DECL_OVERRIDE;
+    void syncFromFrontEnd(const Qt3DCore::QNode *frontEnd, bool firstTime) override;
 
     QVector<Qt3DCore::Sqt> joints() const { return  m_jointLocalPoses; }
     int jointCount() const { return m_jointLocalPoses.size(); }
diff --git a/src/core/transforms/qabstractskeleton_p.h b/src/core/transforms/qabstractskeleton_p.h
index be50c24f5..35d0df755 100644
--- a/src/core/transforms/qabstractskeleton_p.h
+++ b/src/core/transforms/qabstractskeleton_p.h
@@ -74,6 +74,7 @@ public:
 
     int m_jointCount;
     QVector<Sqt> m_localPoses;
+    QVector<QString> m_jointNames;
 };
 
 } // namespace Qt3DCore
diff --git a/src/render/geometry/skeleton.cpp b/src/render/geometry/skeleton.cpp
index 14892922a..2fb74d1c1 100644
--- a/src/render/geometry/skeleton.cpp
+++ b/src/render/geometry/skeleton.cpp
@@ -119,9 +119,9 @@ void Skeleton::syncFromFrontEnd(const QNode *frontEnd, bool firstTime)
         }
         m_createJoints = loaderNode->isCreateJointsEnabled();
 
-        if ((loaderNode->rootJoint() && loaderNode->rootJoint()->id() != m_rootJointId) ||
-            (!loaderNode->rootJoint() && !m_rootJointId.isNull())) {
-            m_rootJointId = loaderNode->rootJoint() ? loaderNode->rootJoint()->id() : Qt3DCore::QNodeId{};
+        auto newJointId = Qt3DCore::qIdForNode(loaderNode->rootJoint());
+        if (newJointId != m_rootJointId) {
+            m_rootJointId = newJointId;
 
             // If using a QSkeletonLoader to create frontend QJoints, when those joints are
             // set on the skeleton, we end up here. In order to allow the subsequent call
@@ -156,25 +156,6 @@ void Skeleton::setStatus(QSkeletonLoader::Status status)
     }
 }
 
-void Skeleton::notifyJointCount()
-{
-    Qt3DCore::QPropertyUpdatedChangePtr e = Qt3DCore::QPropertyUpdatedChangePtr::create(peerId());
-    e->setDeliveryFlags(Qt3DCore::QSceneChange::DeliverToAll);
-    e->setPropertyName("jointCount");
-    e->setValue(jointCount());
-    notifyObservers(e);
-}
-
-void Skeleton::notifyJointNamesAndPoses()
-{
-    auto e = QPropertyUpdatedChangePtr::create(peerId());
-    JointNamesAndLocalPoses payload{m_skeletonData.jointNames, m_skeletonData.localPoses};
-    e->setDeliveryFlags(Qt3DCore::QSceneChange::BackendNodes);
-    e->setPropertyName("jointNamesAndLocalPoses");
-    e->setValue(QVariant::fromValue(payload));
-    notifyObservers(e);
-}
-
 void Skeleton::loadSkeleton()
 {
     qCDebug(Jobs) << Q_FUNC_INFO << m_source;
@@ -204,8 +185,6 @@ void Skeleton::loadSkeleton()
         else
             setStatus(QSkeletonLoader::Ready);
     }
-    notifyJointCount();
-    notifyJointNamesAndPoses();
 
     qCDebug(Jobs) << "Loaded skeleton data:" << *this;
 }
diff --git a/src/render/geometry/skeleton_p.h b/src/render/geometry/skeleton_p.h
index d1c6befe9..6bddbb93b 100644
--- a/src/render/geometry/skeleton_p.h
+++ b/src/render/geometry/skeleton_p.h
@@ -94,9 +94,9 @@ public:
     QString name() const { return m_name; }
 
     int jointCount() const { return m_skeletonData.joints.size(); }
-    void notifyJointCount();
-    void notifyJointNamesAndPoses();
     QVector<JointInfo> joints() const { return m_skeletonData.joints; }
+    QVector<QString> jointNames() const { return m_skeletonData.jointNames; }
+    QVector<Qt3DCore::Sqt> localPoses() const { return m_skeletonData.localPoses; }
 
     Qt3DCore::QNodeId rootJointId() const { return m_rootJointId; }
 
diff --git a/src/render/jobs/loadskeletonjob.cpp b/src/render/jobs/loadskeletonjob.cpp
index 413afc5b5..0f38538e9 100644
--- a/src/render/jobs/loadskeletonjob.cpp
+++ b/src/render/jobs/loadskeletonjob.cpp
@@ -35,6 +35,9 @@
 ****************************************************************************/
 
 #include "loadskeletonjob_p.h"
+#include <Qt3DCore/qabstractskeleton.h>
+#include <Qt3DCore/private/qaspectmanager_p.h>
+#include <Qt3DCore/private/qabstractskeleton_p.h>
 #include <Qt3DRender/private/nodemanagers_p.h>
 #include <Qt3DRender/private/managers_p.h>
 #include <Qt3DRender/private/job_common_p.h>
@@ -44,12 +47,23 @@ QT_BEGIN_NAMESPACE
 namespace Qt3DRender {
 namespace Render {
 
+class LoadSkeletonJobPrivate : public Qt3DCore::QAspectJobPrivate
+{
+public:
+    LoadSkeletonJobPrivate() : m_backendSkeleton(nullptr) { }
+    ~LoadSkeletonJobPrivate() override { }
+
+    void postFrame(Qt3DCore::QAspectManager *manager) override;
+
+    Skeleton *m_backendSkeleton;
+};
+
 LoadSkeletonJob::LoadSkeletonJob(const HSkeleton &handle)
     : QAspectJob()
     , m_handle(handle)
     , m_nodeManagers(nullptr)
 {
-    SET_JOB_RUN_STAT_TYPE(this, JobTypes::LoadSkeleton, 0);
+    SET_JOB_RUN_STAT_TYPE(this, JobTypes::LoadSkeleton, 0)
 }
 
 LoadSkeletonJob::~LoadSkeletonJob()
@@ -58,9 +72,31 @@ LoadSkeletonJob::~LoadSkeletonJob()
 
 void LoadSkeletonJob::run()
 {
+    Q_D(LoadSkeletonJob);
+    d->m_backendSkeleton = nullptr;
+
     Skeleton *skeleton = m_nodeManagers->skeletonManager()->data(m_handle);
-    if (skeleton != nullptr)
+    if (skeleton != nullptr) {
+        d->m_backendSkeleton = skeleton;
         skeleton->loadSkeleton();
+    }
+}
+
+void LoadSkeletonJobPrivate::postFrame(Qt3DCore::QAspectManager *manager)
+{
+    if (!m_backendSkeleton)
+        return;
+
+    using namespace Qt3DCore;
+    QAbstractSkeleton *node = qobject_cast<QAbstractSkeleton *>(manager->lookupNode(m_backendSkeleton->peerId()));
+    if (!node)
+        return;
+
+    QAbstractSkeletonPrivate *dnode = QAbstractSkeletonPrivate::get(node);
+    dnode->m_jointCount = m_backendSkeleton->jointCount();
+    dnode->m_jointNames = m_backendSkeleton->jointNames();
+    dnode->m_localPoses = m_backendSkeleton->localPoses();
+    dnode->update();
 }
 
 } // namespace Render
diff --git a/src/render/jobs/loadskeletonjob_p.h b/src/render/jobs/loadskeletonjob_p.h
index 2cd9fa8bf..e841e5e4f 100644
--- a/src/render/jobs/loadskeletonjob_p.h
+++ b/src/render/jobs/loadskeletonjob_p.h
@@ -60,6 +60,7 @@ namespace Qt3DRender {
 namespace Render {
 
 class NodeManagers;
+class LoadSkeletonJobPrivate;
 
 class LoadSkeletonJob : public Qt3DCore::QAspectJob
 {
@@ -73,6 +74,9 @@ protected:
     void run() override;
     HSkeleton m_handle;
     NodeManagers *m_nodeManagers;
+
+private:
+    Q_DECLARE_PRIVATE(LoadSkeletonJob)
 };
 
 typedef QSharedPointer<LoadSkeletonJob> LoadSkeletonJobPtr;
diff --git a/tests/auto/animation/skeleton/tst_skeleton.cpp b/tests/auto/animation/skeleton/tst_skeleton.cpp
index 9116be199..99acf0b43 100644
--- a/tests/auto/animation/skeleton/tst_skeleton.cpp
+++ b/tests/auto/animation/skeleton/tst_skeleton.cpp
@@ -63,7 +63,7 @@ private Q_SLOTS:
         skeleton.setRootJoint(rootJoint);
 
         // WHEN
-        simulateInitialization(&skeleton, &backendSkeleton);
+        simulateInitializationSync(&skeleton, &backendSkeleton);
 
         // THEN - nothing mirrored from frontend
         QCOMPARE(backendSkeleton.peerId(), skeleton.id());
@@ -127,37 +127,15 @@ private Q_SLOTS:
         Handler handler;
         Skeleton backendSkeleton;
         backendSkeleton.setHandler(&handler);
-        QPropertyUpdatedChangePtr updateChange;
-
-        // WHEN
-        updateChange = QPropertyUpdatedChangePtr::create(QNodeId());
-        updateChange->setPropertyName("enabled");
-        updateChange->setValue(true);
-        backendSkeleton.sceneChangeEvent(updateChange);
-
-        // THEN
-        QCOMPARE(backendSkeleton.isEnabled(), true);
-
-        // GIVEN
-        const QVector<QString> names = (QVector<QString>()
-                                        << QLatin1String("root")
-                                        << QLatin1String("child1")
-                                        << QLatin1String("child2"));
-        const QVector<Sqt> localPoses = (QVector<Sqt>() << Sqt() << Sqt() << Sqt());
+        QSkeleton skeleton;
+        simulateInitializationSync(&skeleton, &backendSkeleton);
 
         // WHEN
-        JointNamesAndLocalPoses namesAndPoses;
-        namesAndPoses.names = names;
-        namesAndPoses.localPoses = localPoses;
-
-        updateChange = QPropertyUpdatedChangePtr::create(QNodeId());
-        updateChange->setPropertyName("jointNamesAndLocalPoses");
-        updateChange->setValue(QVariant::fromValue(namesAndPoses));
-        backendSkeleton.sceneChangeEvent(updateChange);
+        skeleton.setEnabled(false);
+        backendSkeleton.syncFromFrontEnd(&skeleton, false);
 
         // THEN
-        QCOMPARE(backendSkeleton.jointNames(), names);
-        QCOMPARE(backendSkeleton.jointLocalPoses(), localPoses);
+        QCOMPARE(backendSkeleton.isEnabled(), false);
     }
 
     void checkJointTransforms_data()
diff --git a/tests/auto/render/skeleton/tst_skeleton.cpp b/tests/auto/render/skeleton/tst_skeleton.cpp
index 17996470c..c72f98a89 100644
--- a/tests/auto/render/skeleton/tst_skeleton.cpp
+++ b/tests/auto/render/skeleton/tst_skeleton.cpp
@@ -178,43 +178,6 @@ private Q_SLOTS:
         QCOMPARE(backendSkeleton.source(), newSource);
     }
 
-    void checkStatusPropertyBackendNotification()
-    {
-        // GIVEN
-        TestRenderer renderer;
-        NodeManagers nodeManagers;
-        renderer.setNodeManagers(&nodeManagers);
-        TestArbiter arbiter;
-        Skeleton backendSkeleton;
-        backendSkeleton.setRenderer(&renderer);
-        backendSkeleton.setSkeletonManager(nodeManagers.skeletonManager());
-        backendSkeleton.setEnabled(true);
-        Qt3DCore::QBackendNodePrivate::get(&backendSkeleton)->setArbiter(&arbiter);
-
-        // WHEN
-        backendSkeleton.setStatus(QSkeletonLoader::Error);
-
-        // THEN
-        QCOMPARE(backendSkeleton.status(), QSkeletonLoader::Error);
-        QCOMPARE(arbiter.events.count(), 1);
-        Qt3DCore::QPropertyUpdatedChangePtr change = arbiter.events.first().staticCast<Qt3DCore::QPropertyUpdatedChange>();
-        QCOMPARE(change->propertyName(), "status");
-        QCOMPARE(change->value().value<QSkeletonLoader::Status>(), backendSkeleton.status());
-        QCOMPARE(Qt3DCore::QPropertyUpdatedChangeBasePrivate::get(change.data())->m_isIntermediate,
-                 false);
-
-        arbiter.events.clear();
-
-        // WHEN
-        backendSkeleton.setStatus(QSkeletonLoader::Error);
-
-        // THEN
-        QCOMPARE(backendSkeleton.status(), QSkeletonLoader::Error);
-        QCOMPARE(arbiter.events.count(), 0);
-
-        arbiter.events.clear();
-    }
-
     void checkCreateFrontendJoint_data()
     {
         QTest::addColumn<QMatrix4x4>("inverseBindMatrix");