Clean QJoint message handling

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

4 files changed, 29 insertions, 102 deletions

diff --git a/src/core/transforms/qjoint.cpp b/src/core/transforms/qjoint.cpp
index c13b388e2..408d42f6d 100644
--- a/src/core/transforms/qjoint.cpp
+++ b/src/core/transforms/qjoint.cpp
@@ -350,7 +350,7 @@ void QJoint::addChildJoint(QJoint *joint)
         d->registerDestructionHelper(joint, &QJoint::removeChildJoint, d->m_childJoints);
 
         if (d->m_changeArbiter != nullptr)
-            d->update();
+            d->updateNode(joint, "childJoint", PropertyValueAdded);
     }
 }
 
@@ -362,9 +362,8 @@ void QJoint::removeChildJoint(QJoint *joint)
 {
     Q_D(QJoint);
     if (d->m_childJoints.contains(joint)) {
-
         if (d->m_changeArbiter != nullptr)
-            d->update();
+            d->updateNode(joint, "childJoint", PropertyValueRemoved);
 
         d->m_childJoints.removeOne(joint);
 
diff --git a/src/render/geometry/joint.cpp b/src/render/geometry/joint.cpp
index 86d583f51..9791f6c52 100644
--- a/src/render/geometry/joint.cpp
+++ b/src/render/geometry/joint.cpp
@@ -41,9 +41,6 @@
 #include <Qt3DRender/private/managers_p.h>
 #include <Qt3DCore/QJoint>
 #include <Qt3DCore/private/qjoint_p.h>
-#include <Qt3DCore/qpropertyupdatedchange.h>
-#include <Qt3DCore/qpropertynodeaddedchange.h>
-#include <Qt3DCore/qpropertynoderemovedchange.h>
 
 #include <algorithm>
 
@@ -72,69 +69,24 @@ void Joint::cleanup()
     setEnabled(false);
 }
 
-// TODOSYNC remove once animation changes don't use messages anymore
-void Joint::sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e)
-{
-    if (e->type() == PropertyUpdated) {
-        const QPropertyUpdatedChangePtr &propertyChange = qSharedPointerCast<QPropertyUpdatedChange>(e);
-        if (propertyChange->propertyName() == QByteArrayLiteral("scale")) {
-            m_localPose.scale = propertyChange->value().value<QVector3D>();
-            markDirty(AbstractRenderer::JointDirty);
-            m_jointManager->addDirtyJoint(peerId());
-        } else if (propertyChange->propertyName() == QByteArrayLiteral("rotation")) {
-            m_localPose.rotation = propertyChange->value().value<QQuaternion>();
-            markDirty(AbstractRenderer::JointDirty);
-            m_jointManager->addDirtyJoint(peerId());
-        } else if (propertyChange->propertyName() == QByteArrayLiteral("translation")) {
-            m_localPose.translation = propertyChange->value().value<QVector3D>();
-            markDirty(AbstractRenderer::JointDirty);
-            m_jointManager->addDirtyJoint(peerId());
-        } else if (propertyChange->propertyName() == QByteArrayLiteral("inverseBindMatrix")) {
-            // Setting the inverse bind matrix should be a rare operation. Usually it is
-            // set once and then remains constant for the duration of the skeleton. So just
-            // trigger a rebuild of the skeleton's SkeletonData which will include obtaining
-            // the inverse bind matrix.
-            m_inverseBindMatrix = propertyChange->value().value<QMatrix4x4>();
-            m_skeletonManager->addDirtySkeleton(SkeletonManager::SkeletonDataDirty, m_owningSkeleton);
-        } else if (propertyChange->propertyName() == QByteArrayLiteral("name")) {
-            // Joint name doesn't affect anything in the render aspect so no need
-            // to mark anything as dirty.
-            m_name = propertyChange->value().toString();
-
-            // TODO: Notify other aspects (animation) about the name change.
-        }
-    } else if (e->type() == PropertyValueAdded) {
-        const auto addedChange = qSharedPointerCast<QPropertyNodeAddedChange>(e);
-        if (addedChange->propertyName() == QByteArrayLiteral("childJoint"))
-            m_childJointIds.push_back(addedChange->addedNodeId());
-    } else if (e->type() == PropertyValueRemoved) {
-        const auto removedChange = qSharedPointerCast<QPropertyNodeRemovedChange>(e);
-        if (removedChange->propertyName() == QByteArrayLiteral("childJoint"))
-            m_childJointIds.removeOne(removedChange->removedNodeId());
-    }
-    BackendNode::sceneChangeEvent(e);
-}
-
 void Joint::syncFromFrontEnd(const QNode *frontEnd, bool firstTime)
 {
     const Qt3DCore::QJoint *joint = qobject_cast<const Qt3DCore::QJoint *>(frontEnd);
     if (!joint)
         return;
 
+    bool jointDirty = firstTime;
     if (m_localPose.scale != joint->scale()) {
         m_localPose.scale = joint->scale();
-        markDirty(AbstractRenderer::JointDirty);
-        m_jointManager->addDirtyJoint(peerId());
+        jointDirty = true;
     }
     if (m_localPose.rotation != joint->rotation()) {
         m_localPose.rotation = joint->rotation();
-        markDirty(AbstractRenderer::JointDirty);
-        m_jointManager->addDirtyJoint(peerId());
+        jointDirty = true;
     }
     if (m_localPose.translation != joint->translation()) {
         m_localPose.translation = joint->translation();
-        markDirty(AbstractRenderer::JointDirty);
-        m_jointManager->addDirtyJoint(peerId());
+        jointDirty = true;
     }
     if (m_inverseBindMatrix != joint->inverseBindMatrix()) {
         // Setting the inverse bind matrix should be a rare operation. Usually it is
@@ -154,11 +106,10 @@ void Joint::syncFromFrontEnd(const QNode *frontEnd, bool firstTime)
 
     Qt3DCore::QNodeIdVector childIds = qIdsForNodes(joint->childJoints());
     std::sort(std::begin(childIds), std::end(childIds));
-    if (m_childJointIds != childIds) {
+    if (m_childJointIds != childIds)
         m_childJointIds = childIds;
-    }
 
-    if (firstTime) {
+    if (jointDirty) {
         markDirty(AbstractRenderer::JointDirty);
         m_jointManager->addDirtyJoint(peerId());
     }
diff --git a/src/render/geometry/joint_p.h b/src/render/geometry/joint_p.h
index d0530ac04..de875459e 100644
--- a/src/render/geometry/joint_p.h
+++ b/src/render/geometry/joint_p.h
@@ -69,7 +69,6 @@ public:
     Joint();
 
     void cleanup();
-    void sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e) override;
     void syncFromFrontEnd(const Qt3DCore::QNode *frontEnd, bool firstTime) override;
 
     Qt3DCore::Sqt localPose() const { return m_localPose; }
diff --git a/tests/auto/render/joint/tst_joint.cpp b/tests/auto/render/joint/tst_joint.cpp
index c08d36725..0388f4a80 100644
--- a/tests/auto/render/joint/tst_joint.cpp
+++ b/tests/auto/render/joint/tst_joint.cpp
@@ -32,10 +32,6 @@
 #include <Qt3DCore/qjoint.h>
 #include <Qt3DCore/private/qnode_p.h>
 #include <Qt3DCore/private/qscene_p.h>
-#include <Qt3DCore/qpropertyupdatedchange.h>
-#include <Qt3DCore/private/qpropertyupdatedchangebase_p.h>
-#include <Qt3DCore/qpropertynodeaddedchange.h>
-#include <Qt3DCore/qpropertynoderemovedchange.h>
 #include <QtGui/qmatrix4x4.h>
 #include <QtGui/qvector3d.h>
 #include <qbackendnodetester.h>
@@ -153,43 +149,36 @@ private Q_SLOTS:
         backendJoint.setRenderer(&renderer);
         backendJoint.setJointManager(nodeManagers.jointManager());
         backendJoint.setSkeletonManager(nodeManagers.skeletonManager());
-        Qt3DCore::QPropertyUpdatedChangePtr updateChange;
+        QJoint joint;
+        simulateInitializationSync(&joint, &backendJoint);
 
         // WHEN
-        updateChange.reset(new Qt3DCore::QPropertyUpdatedChange(Qt3DCore::QNodeId()));
-        updateChange->setPropertyName("enabled");
-        updateChange->setValue(true);
-        backendJoint.sceneChangeEvent(updateChange);
+        joint.setEnabled(false);
+        backendJoint.syncFromFrontEnd(&joint, false);
 
         // THEN
-        QCOMPARE(backendJoint.isEnabled(), true);
+        QCOMPARE(backendJoint.isEnabled(), false);
 
         // WHEN
         const QVector3D newTranslation = QVector3D(1.0f, 2.0f, 3.0f);
-        updateChange.reset(new Qt3DCore::QPropertyUpdatedChange(Qt3DCore::QNodeId()));
-        updateChange->setPropertyName("translation");
-        updateChange->setValue(newTranslation);
-        backendJoint.sceneChangeEvent(updateChange);
+        joint.setTranslation(newTranslation);
+        backendJoint.syncFromFrontEnd(&joint, false);
 
         // THEN
         QCOMPARE(backendJoint.translation(), newTranslation);
 
         // WHEN
         const QQuaternion newRotation = QQuaternion::fromAxisAndAngle(1.0f, 0.0f, 0.0f, 45.0f);
-        updateChange.reset(new Qt3DCore::QPropertyUpdatedChange(Qt3DCore::QNodeId()));
-        updateChange->setPropertyName("rotation");
-        updateChange->setValue(newRotation);
-        backendJoint.sceneChangeEvent(updateChange);
+        joint.setRotation(newRotation);
+        backendJoint.syncFromFrontEnd(&joint, false);
 
         // THEN
         QCOMPARE(backendJoint.rotation(), newRotation);
 
         // WHEN
         const QVector3D newScale = QVector3D(1.5f, 2.5f, 3.5f);
-        updateChange.reset(new Qt3DCore::QPropertyUpdatedChange(Qt3DCore::QNodeId()));
-        updateChange->setPropertyName("scale");
-        updateChange->setValue(newScale);
-        backendJoint.sceneChangeEvent(updateChange);
+        joint.setScale(newScale);
+        backendJoint.syncFromFrontEnd(&joint, false);
 
         // THEN
         QCOMPARE(backendJoint.scale(), newScale);
@@ -197,44 +186,34 @@ private Q_SLOTS:
         // WHEN
         QMatrix4x4 newInverseBind;
         newInverseBind.scale(5.4f);
-        updateChange.reset(new Qt3DCore::QPropertyUpdatedChange(Qt3DCore::QNodeId()));
-        updateChange->setPropertyName("inverseBindMatrix");
-        updateChange->setValue(newInverseBind);
-        backendJoint.sceneChangeEvent(updateChange);
+        joint.setInverseBindMatrix(newInverseBind);
+        backendJoint.syncFromFrontEnd(&joint, false);
 
         // THEN
         QCOMPARE(backendJoint.inverseBindMatrix(), newInverseBind);
 
         // WHEN
         QVector<QJoint *> childJoints;
-        QPropertyNodeAddedChangePtr nodeAddedChange;
         for (int i = 0; i < 10; ++i) {
             const auto childJoint = new QJoint();
             childJoints.push_back(childJoint);
-
-            nodeAddedChange.reset(new QPropertyNodeAddedChange(QNodeId(), childJoint));
-            nodeAddedChange->setPropertyName("childJoint");
-            backendJoint.sceneChangeEvent(nodeAddedChange);
+            joint.addChildJoint(childJoint);
         }
+        backendJoint.syncFromFrontEnd(&joint, false);
 
         // THEN
-        for (int i = 0; i < childJoints.size(); ++i) {
+        for (int i = 0; i < childJoints.size(); ++i)
             QCOMPARE(backendJoint.childJointIds()[i], childJoints[i]->id());
-        }
 
-        QPropertyNodeRemovedChangePtr nodeRemovedChange;
         for (int i = 0; i < 10; ++i) {
             // WHEN
             const auto childJoint = childJoints.takeLast();
-
-            nodeRemovedChange.reset(new QPropertyNodeRemovedChange(QNodeId(), childJoint));
-            nodeRemovedChange->setPropertyName("childJoint");
-            backendJoint.sceneChangeEvent(nodeAddedChange);
+            joint.removeChildJoint(childJoint);
+            backendJoint.syncFromFrontEnd(&joint, false);
 
             // THEN
-            for (int i = 0; i < childJoints.size(); ++i) {
+            for (int i = 0; i < childJoints.size(); ++i)
                 QCOMPARE(backendJoint.childJointIds()[i], childJoints[i]->id());
-            }
         }
     }
 
@@ -314,9 +293,8 @@ private Q_SLOTS:
             backendJoint.syncFromFrontEnd(&joint, false);
 
             // THEN
-            for (int i = 0; i < childJoints.size(); ++i) {
-                QCOMPARE(backendJoint.childJointIds()[i], childJoints[i]->id());
-            }
+            for (int j = 0; j < childJoints.size(); ++j)
+                QCOMPARE(backendJoint.childJointIds()[j], childJoints[j]->id());
         }
     }
 };