Update LoadAnimationClipJob to use direct sync

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

9 files changed, 45 insertions, 144 deletions

diff --git a/src/animation/backend/animationclip.cpp b/src/animation/backend/animationclip.cpp
index ea1fd76d9..2dea81b52 100644
--- a/src/animation/backend/animationclip.cpp
+++ b/src/animation/backend/animationclip.cpp
@@ -43,7 +43,6 @@
 #include <Qt3DAnimation/private/managers_p.h>
 #include <Qt3DAnimation/private/gltfimporter_p.h>
 #include <Qt3DRender/private/qurlhelper_p.h>
-#include <Qt3DCore/qpropertyupdatedchange.h>
 
 #include <QtCore/qbytearray.h>
 #include <QtCore/qfile.h>
@@ -90,11 +89,6 @@ void AnimationClip::setStatus(QAnimationClipLoader::Status status)
 {
     if (status != m_status) {
         m_status = status;
-        Qt3DCore::QPropertyUpdatedChangePtr e = Qt3DCore::QPropertyUpdatedChangePtr::create(peerId());
-        e->setDeliveryFlags(Qt3DCore::QSceneChange::DeliverToAll);
-        e->setPropertyName("status");
-        e->setValue(QVariant::fromValue(m_status));
-        notifyObservers(e);
     }
 }
 
@@ -317,13 +311,6 @@ void AnimationClip::setDuration(float duration)
         return;
 
     m_duration = duration;
-
-    // Send a change to the frontend
-    auto e = Qt3DCore::QPropertyUpdatedChangePtr::create(peerId());
-    e->setDeliveryFlags(Qt3DCore::QSceneChange::DeliverToAll);
-    e->setPropertyName("duration");
-    e->setValue(m_duration);
-    notifyObservers(e);
 }
 
 int AnimationClip::channelIndex(const QString &channelName, int jointIndex) const
diff --git a/src/animation/backend/loadanimationclipjob.cpp b/src/animation/backend/loadanimationclipjob.cpp
index d3319f8e1..977564c7f 100644
--- a/src/animation/backend/loadanimationclipjob.cpp
+++ b/src/animation/backend/loadanimationclipjob.cpp
@@ -36,7 +36,11 @@
 
 #include "loadanimationclipjob_p.h"
 
+#include <Qt3DCore/private/qaspectmanager_p.h>
+#include <Qt3DAnimation/qanimationcliploader.h>
+#include <Qt3DAnimation/private/qanimationcliploader_p.h>
 #include <Qt3DAnimation/private/animationclip_p.h>
+#include <Qt3DAnimation/private/qabstractanimationclip_p.h>
 #include <Qt3DAnimation/private/handler_p.h>
 #include <Qt3DAnimation/private/managers_p.h>
 #include <Qt3DAnimation/private/job_common_p.h>
@@ -46,16 +50,27 @@ QT_BEGIN_NAMESPACE
 namespace Qt3DAnimation {
 namespace Animation {
 
+class LoadAnimationClipJobPrivate : public Qt3DCore::QAspectJobPrivate
+{
+public:
+    LoadAnimationClipJobPrivate() { }
+    ~LoadAnimationClipJobPrivate() override { }
+
+    void postFrame(Qt3DCore::QAspectManager *manager) override;
+
+    QVector<AnimationClip *> m_updatedNodes;
+};
+
 LoadAnimationClipJob::LoadAnimationClipJob()
-    : Qt3DCore::QAspectJob()
+    : Qt3DCore::QAspectJob(*new LoadAnimationClipJobPrivate)
     , m_animationClipHandles()
 {
-    SET_JOB_RUN_STAT_TYPE(this, JobTypes::LoadAnimationClip, 0);
+    SET_JOB_RUN_STAT_TYPE(this, JobTypes::LoadAnimationClip, 0)
 }
 
 void LoadAnimationClipJob::addDirtyAnimationClips(const QVector<HAnimationClip> &animationClipHandles)
 {
-    for (const auto handle : animationClipHandles) {
+    for (const auto &handle : animationClipHandles) {
         if (!m_animationClipHandles.contains(handle))
             m_animationClipHandles.push_back(handle);
     }
@@ -69,16 +84,40 @@ void LoadAnimationClipJob::clearDirtyAnimationClips()
 void LoadAnimationClipJob::run()
 {
     Q_ASSERT(m_handler);
+    Q_D(LoadAnimationClipJob);
+
+    d->m_updatedNodes.reserve(m_animationClipHandles.size());
     AnimationClipLoaderManager *animationClipManager = m_handler->animationClipLoaderManager();
     for (const auto &animationClipHandle : qAsConst(m_animationClipHandles)) {
         AnimationClip *animationClip = animationClipManager->data(animationClipHandle);
         Q_ASSERT(animationClip);
         animationClip->loadAnimation();
+        d->m_updatedNodes.push_back(animationClip);
     }
 
     clearDirtyAnimationClips();
 }
 
+void LoadAnimationClipJobPrivate::postFrame(Qt3DCore::QAspectManager *manager)
+{
+    for (AnimationClip *clip: qAsConst(m_updatedNodes)) {
+        QAbstractAnimationClip *node = qobject_cast<QAbstractAnimationClip *>(manager->lookupNode(clip->peerId()));
+        if (!node)
+            continue;
+
+        QAbstractAnimationClipPrivate *dnode = static_cast<QAbstractAnimationClipPrivate *>(Qt3DCore::QNodePrivate::get(node));
+        dnode->setDuration(clip->duration());
+
+        QAnimationClipLoader *loader = qobject_cast<QAnimationClipLoader *>(node);
+        if (loader) {
+            QAnimationClipLoaderPrivate *dloader = static_cast<QAnimationClipLoaderPrivate *>(dnode);
+            dloader->setStatus(clip->status());
+        }
+    }
+
+    m_updatedNodes.clear();
+}
+
 } // namespace Animation
 } // namespace Qt3DAnimation
 
diff --git a/src/animation/backend/loadanimationclipjob_p.h b/src/animation/backend/loadanimationclipjob_p.h
index 25de2c6b6..07d78d416 100644
--- a/src/animation/backend/loadanimationclipjob_p.h
+++ b/src/animation/backend/loadanimationclipjob_p.h
@@ -59,6 +59,7 @@ namespace Animation {
 
 class Handler;
 class FindGraphJob;
+class LoadAnimationClipJobPrivate;
 
 class LoadAnimationClipJob : public Qt3DCore::QAspectJob
 {
@@ -75,6 +76,8 @@ protected:
     void run() override;
 
 private:
+    Q_DECLARE_PRIVATE(LoadAnimationClipJob)
+
     QVector<HAnimationClip> m_animationClipHandles;
     Handler *m_handler;
 };
diff --git a/src/animation/frontend/qabstractanimationclip.cpp b/src/animation/frontend/qabstractanimationclip.cpp
index 7e6df4d70..e662a1408 100644
--- a/src/animation/frontend/qabstractanimationclip.cpp
+++ b/src/animation/frontend/qabstractanimationclip.cpp
@@ -153,19 +153,6 @@ float QAbstractAnimationClip::duration() const
     return d->m_duration;
 }
 
-/*!
-    \internal
-*/
-void QAbstractAnimationClip::sceneChangeEvent(const Qt3DCore::QSceneChangePtr &change)
-{
-    Q_D(QAbstractAnimationClip);
-    if (change->type() == Qt3DCore::PropertyUpdated) {
-        Qt3DCore::QPropertyUpdatedChangePtr e = qSharedPointerCast<Qt3DCore::QPropertyUpdatedChange>(change);
-        if (e->propertyName() == QByteArrayLiteral("duration"))
-            d->setDuration(e->value().toFloat());
-    }
-}
-
 } // namespace Qt3DAnimation
 
 QT_END_NAMESPACE
diff --git a/src/animation/frontend/qabstractanimationclip.h b/src/animation/frontend/qabstractanimationclip.h
index 79464b13d..f68950c4c 100644
--- a/src/animation/frontend/qabstractanimationclip.h
+++ b/src/animation/frontend/qabstractanimationclip.h
@@ -62,7 +62,6 @@ Q_SIGNALS:
 
 protected:
     QAbstractAnimationClip(QAbstractAnimationClipPrivate &dd, Qt3DCore::QNode *parent = nullptr);
-    void sceneChangeEvent(const Qt3DCore::QSceneChangePtr &change) override;
 
 private:
     Q_DECLARE_PRIVATE(QAbstractAnimationClip)
diff --git a/src/animation/frontend/qanimationcliploader.cpp b/src/animation/frontend/qanimationcliploader.cpp
index d31f9bd5d..446995b17 100644
--- a/src/animation/frontend/qanimationcliploader.cpp
+++ b/src/animation/frontend/qanimationcliploader.cpp
@@ -142,19 +142,6 @@ void QAnimationClipLoader::setSource(const QUrl &source)
     emit sourceChanged(source);
 }
 
-/*!
-    \internal
-*/
-void QAnimationClipLoader::sceneChangeEvent(const Qt3DCore::QSceneChangePtr &change)
-{
-    Q_D(QAnimationClipLoader);
-    if (change->type() == Qt3DCore::PropertyUpdated) {
-        const Qt3DCore::QPropertyUpdatedChangePtr e = qSharedPointerCast<Qt3DCore::QPropertyUpdatedChange>(change);
-        if (e->propertyName() == QByteArrayLiteral("status"))
-            d->setStatus(static_cast<QAnimationClipLoader::Status>(e->value().toInt()));
-    }
-}
-
 Qt3DCore::QNodeCreatedChangeBasePtr QAnimationClipLoader::createNodeCreationChange() const
 {
     auto creationChange = Qt3DCore::QNodeCreatedChangePtr<QAnimationClipLoaderData>::create(this);
diff --git a/src/animation/frontend/qanimationcliploader.h b/src/animation/frontend/qanimationcliploader.h
index bf82f742e..1ff388297 100644
--- a/src/animation/frontend/qanimationcliploader.h
+++ b/src/animation/frontend/qanimationcliploader.h
@@ -78,7 +78,6 @@ Q_SIGNALS:
 
 protected:
     explicit QAnimationClipLoader(QAnimationClipLoaderPrivate &dd, Qt3DCore::QNode *parent = nullptr);
-    void sceneChangeEvent(const Qt3DCore::QSceneChangePtr &change) override;
 
 private:
     Q_DECLARE_PRIVATE(QAnimationClipLoader)
diff --git a/tests/auto/animation/animationclip/tst_animationclip.cpp b/tests/auto/animation/animationclip/tst_animationclip.cpp
index a98fbe886..35df21a58 100644
--- a/tests/auto/animation/animationclip/tst_animationclip.cpp
+++ b/tests/auto/animation/animationclip/tst_animationclip.cpp
@@ -117,70 +117,6 @@ private Q_SLOTS:
         // THEN
         QCOMPARE(backendClip.source(), newSource);
     }
-
-    void checkDurationPropertyBackendNotification()
-    {
-        // GIVEN
-        TestArbiter arbiter;
-        AnimationClip backendClip;
-        backendClip.setEnabled(true);
-        Qt3DCore::QBackendNodePrivate::get(&backendClip)->setArbiter(&arbiter);
-
-        // WHEN
-        backendClip.setDuration(64.0f);
-
-        // THEN
-        QCOMPARE(backendClip.duration(), 64.0f);
-        QCOMPARE(arbiter.events.count(), 1);
-        Qt3DCore::QPropertyUpdatedChangePtr change = arbiter.events.first().staticCast<Qt3DCore::QPropertyUpdatedChange>();
-        QCOMPARE(change->propertyName(), "duration");
-        QCOMPARE(change->value().toFloat(), backendClip.duration());
-        QCOMPARE(Qt3DCore::QPropertyUpdatedChangeBasePrivate::get(change.data())->m_isIntermediate,
-                 false);
-
-        arbiter.events.clear();
-
-        // WHEN
-        backendClip.setDuration(64.0f);
-
-        // THEN
-        QCOMPARE(backendClip.duration(), 64.0f);
-        QCOMPARE(arbiter.events.count(), 0);
-
-        arbiter.events.clear();
-    }
-
-    void checkStatusPropertyBackendNotification()
-    {
-        // GIVEN
-        TestArbiter arbiter;
-        AnimationClip backendClip;
-        backendClip.setEnabled(true);
-        Qt3DCore::QBackendNodePrivate::get(&backendClip)->setArbiter(&arbiter);
-
-        // WHEN
-        backendClip.setStatus(Qt3DAnimation::QAnimationClipLoader::Error);
-
-        // THEN
-        QCOMPARE(backendClip.status(), Qt3DAnimation::QAnimationClipLoader::Error);
-        QCOMPARE(arbiter.events.count(), 1);
-        Qt3DCore::QPropertyUpdatedChangePtr change = arbiter.events.first().staticCast<Qt3DCore::QPropertyUpdatedChange>();
-        QCOMPARE(change->propertyName(), "status");
-        QCOMPARE(change->value().value<Qt3DAnimation::QAnimationClipLoader::Status>(), backendClip.status());
-        QCOMPARE(Qt3DCore::QPropertyUpdatedChangeBasePrivate::get(change.data())->m_isIntermediate,
-                 false);
-
-        arbiter.events.clear();
-
-        // WHEN
-        backendClip.setStatus(Qt3DAnimation::QAnimationClipLoader::Error);
-
-        // THEN
-        QCOMPARE(backendClip.status(), Qt3DAnimation::QAnimationClipLoader::Error);
-        QCOMPARE(arbiter.events.count(), 0);
-
-        arbiter.events.clear();
-    }
 };
 
 QTEST_APPLESS_MAIN(tst_AnimationClip)
diff --git a/tests/auto/animation/qanimationcliploader/tst_qanimationcliploader.cpp b/tests/auto/animation/qanimationcliploader/tst_qanimationcliploader.cpp
index c52986055..274144d3e 100644
--- a/tests/auto/animation/qanimationcliploader/tst_qanimationcliploader.cpp
+++ b/tests/auto/animation/qanimationcliploader/tst_qanimationcliploader.cpp
@@ -158,42 +158,6 @@ private Q_SLOTS:
         }
 
     }
-
-    void checkStatusPropertyUpdate()
-    {
-        // GIVEN
-        qRegisterMetaType<Qt3DAnimation::QAnimationClipLoader::Status>("Status");
-        TestArbiter arbiter;
-        arbiter.setArbiterOnNode(this);
-        QSignalSpy spy(this, SIGNAL(statusChanged(Status)));
-        const Qt3DAnimation::QAnimationClipLoader::Status newStatus = Qt3DAnimation::QAnimationClipLoader::Error;
-
-        // THEN
-        QVERIFY(spy.isValid());
-
-        // WHEN
-        Qt3DCore::QPropertyUpdatedChangePtr valueChange(new Qt3DCore::QPropertyUpdatedChange(Qt3DCore::QNodeId()));
-        valueChange->setPropertyName("status");
-        valueChange->setValue(QVariant::fromValue(newStatus));
-        sceneChangeEvent(valueChange);
-
-        // THEN
-        QCOMPARE(spy.count(), 1);
-        QCOMPARE(arbiter.events.size(), 0);
-        QCOMPARE(status(), newStatus);
-
-        // WHEN
-        spy.clear();
-        sceneChangeEvent(valueChange);
-
-        // THEN
-        QCOMPARE(spy.count(), 0);
-        QCOMPARE(arbiter.events.size(), 0);
-        QCOMPARE(status(), newStatus);
-
-        // Cleanup
-        Qt3DCore::QNodePrivate::get(this)->setArbiter(nullptr);
-    }
 };
 
 QTEST_MAIN(tst_QAnimationClipLoader)