FrameGraphNode: properly handle dynamic tree changes

Change-Id: Ifa2c7f3c33cb410938e484f023e965d5ca5220cb
Task-number: QTBUG-55908
Reviewed-by: Sean Harmer <sean.harmer@kdab.com>

3 files changed, 157 insertions, 3 deletions

diff --git a/src/render/framegraph/framegraphnode.cpp b/src/render/framegraph/framegraphnode.cpp
index cfdf9834f..9bd069832 100644
--- a/src/render/framegraph/framegraphnode.cpp
+++ b/src/render/framegraph/framegraphnode.cpp
@@ -40,6 +40,8 @@
 #include "framegraphnode_p.h"
 #include <Qt3DRender/private/renderer_p.h>
 #include <Qt3DRender/private/nodemanagers_p.h>
+#include <Qt3DCore/qpropertynoderemovedchange.h>
+#include <Qt3DCore/qpropertynodeaddedchange.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -83,6 +85,12 @@ FrameGraphManager *FrameGraphNode::manager() const
 void FrameGraphNode::setParentId(Qt3DCore::QNodeId parentId)
 {
     if (m_parentId != parentId) {
+        // We already had a parent, tell it to abandon us
+        if (!m_parentId.isNull()) {
+            FrameGraphNode *parent = m_manager->lookupNode(m_parentId);
+            if (parent != nullptr)
+                parent->m_childrenIds.removeAll(peerId());
+        }
         m_parentId = parentId;
         FrameGraphNode *parent = m_manager->lookupNode(m_parentId);
         if (parent != nullptr && !parent->m_childrenIds.contains(peerId()))
@@ -140,6 +148,31 @@ QVector<FrameGraphNode *> FrameGraphNode::children() const
     return children;
 }
 
+void FrameGraphNode::sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e)
+{
+    switch (e->type()) {
+
+    case Qt3DCore::PropertyValueAdded: {
+       Qt3DCore::QPropertyNodeAddedChangePtr change = qSharedPointerCast<Qt3DCore::QPropertyNodeAddedChange>(e);
+        if (change->metaObject()->inherits(&QFrameGraphNode::staticMetaObject))
+            appendChildId(change->addedNodeId());
+        break;
+    }
+
+    case Qt3DCore::PropertyValueRemoved: {
+        Qt3DCore::QPropertyNodeRemovedChangePtr change = qSharedPointerCast<Qt3DCore::QPropertyNodeRemovedChange>(e);
+        if (change->metaObject()->inherits(&QFrameGraphNode::staticMetaObject))
+            removeChildId(change->removedNodeId());
+        break;
+    }
+    default:
+        break;
+    }
+
+    markDirty(AbstractRenderer::AllDirty);
+    BackendNode::sceneChangeEvent(e);
+}
+
 } // namespace Render
 } // namespace Qt3DRender
 
diff --git a/src/render/framegraph/framegraphnode_p.h b/src/render/framegraph/framegraphnode_p.h
index 62f8d645d..1de955abc 100644
--- a/src/render/framegraph/framegraphnode_p.h
+++ b/src/render/framegraph/framegraphnode_p.h
@@ -106,6 +106,8 @@ public:
     FrameGraphNode *parent() const;
     QVector<FrameGraphNode *> children() const;
 
+    void sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e) Q_DECL_OVERRIDE;
+
 protected:
     FrameGraphNode(FrameGraphNodeType nodeType);
     void initializeFromPeer(const Qt3DCore::QNodeCreatedChangeBasePtr &change) Q_DECL_OVERRIDE;
diff --git a/tests/auto/render/framegraphnode/tst_framegraphnode.cpp b/tests/auto/render/framegraphnode/tst_framegraphnode.cpp
index f3f3dbff1..07ff4c0d9 100644
--- a/tests/auto/render/framegraphnode/tst_framegraphnode.cpp
+++ b/tests/auto/render/framegraphnode/tst_framegraphnode.cpp
@@ -29,6 +29,11 @@
 #include <Qt3DRender/private/framegraphnode_p.h>
 #include <QtTest/QTest>
 #include <Qt3DRender/private/managers_p.h>
+#include <Qt3DRender/private/nodemanagers_p.h>
+#include <Qt3DCore/qpropertynodeaddedchange.h>
+#include <Qt3DCore/qpropertynoderemovedchange.h>
+#include <Qt3DCore/private/qnodecreatedchangegenerator_p.h>
+#include "testrenderer.h"
 
 class MyFrameGraphNode : public Qt3DRender::Render::FrameGraphNode
 {
@@ -37,9 +42,15 @@ public:
     {
         FrameGraphNode::setEnabled(enabled);
     }
+};
 
-protected:
-    void sceneChangeEvent(const Qt3DCore::QSceneChangePtr &) Q_DECL_FINAL {}
+class MyQFrameGraphNode : public Qt3DRender::QFrameGraphNode
+{
+    Q_OBJECT
+public:
+    MyQFrameGraphNode(Qt3DCore::QNode *parent = nullptr)
+        : Qt3DRender::QFrameGraphNode(parent)
+    {}
 };
 
 class tst_FrameGraphNode : public QObject
@@ -167,8 +178,116 @@ private Q_SLOTS:
         QCOMPARE(parent1->childrenIds().count(), 0);
         QCOMPARE(parent1->children().count(), parent1->childrenIds().count());
     }
+
+    void checkSetParent()
+    {
+        // GIVEN
+        QScopedPointer<Qt3DRender::Render::FrameGraphManager> manager(new Qt3DRender::Render::FrameGraphManager());
+        const Qt3DCore::QNodeId parent1Id = Qt3DCore::QNodeId::createId();
+        const Qt3DCore::QNodeId parent2Id = Qt3DCore::QNodeId::createId();
+        const Qt3DCore::QNodeId childId = Qt3DCore::QNodeId::createId();
+
+        Qt3DRender::Render::FrameGraphNode *parent1 = new MyFrameGraphNode();
+        Qt3DRender::Render::FrameGraphNode *parent2 = new MyFrameGraphNode();
+        Qt3DRender::Render::FrameGraphNode *child = new MyFrameGraphNode();
+
+        setIdInternal(parent1, parent1Id);
+        setIdInternal(parent2, parent2Id);
+        setIdInternal(child, childId);
+
+        manager->appendNode(parent1Id, parent1);
+        manager->appendNode(parent2Id, parent2);
+        manager->appendNode(childId, child);
+
+        parent1->setFrameGraphManager(manager.data());
+        parent2->setFrameGraphManager(manager.data());
+        child->setFrameGraphManager(manager.data());
+
+        // THEN
+        QCOMPARE(parent1->peerId(), parent1Id);
+        QCOMPARE(parent2->peerId(), parent2Id);
+        QCOMPARE(child->peerId(), childId);
+
+        QVERIFY(child->parentId().isNull());
+        QCOMPARE(parent1->childrenIds().size(), 0);
+        QCOMPARE(parent2->childrenIds().size(), 0);
+
+        // WHEN
+        child->setParentId(parent1Id);
+
+        // THEN
+        QCOMPARE(child->parentId(), parent1Id);
+        QCOMPARE(parent1->childrenIds().size(), 1);
+        QCOMPARE(parent2->childrenIds().size(), 0);
+
+        // WHEN
+        child->setParentId(parent2Id);
+
+        // THEN
+        QCOMPARE(child->parentId(), parent2Id);
+        QCOMPARE(parent1->childrenIds().size(), 0);
+        QCOMPARE(parent2->childrenIds().size(), 1);
+
+        // WHEN
+        child->setParentId(Qt3DCore::QNodeId());
+
+        // THEN
+        QVERIFY(child->parentId().isNull());
+        QCOMPARE(parent1->childrenIds().size(), 0);
+        QCOMPARE(parent2->childrenIds().size(), 0);
+    }
+
+    void checkSceneChangeEvents()
+    {
+        // GIVEN
+        const Qt3DCore::QNodeId fgNode1Id = Qt3DCore::QNodeId::createId();
+
+        Qt3DRender::Render::FrameGraphNode *backendFGNode = new MyFrameGraphNode();
+        Qt3DRender::QFrameGraphNode *frontendFGChild = new MyQFrameGraphNode();
+        Qt3DRender::Render::FrameGraphNode *backendFGChild = new MyFrameGraphNode();
+
+        QScopedPointer<Qt3DRender::Render::FrameGraphManager> manager(new Qt3DRender::Render::FrameGraphManager());
+
+        TestRenderer renderer;
+
+        backendFGNode->setRenderer(&renderer);
+
+        setIdInternal(backendFGNode, fgNode1Id);
+        setIdInternal(backendFGChild, frontendFGChild->id());
+
+        manager->appendNode(fgNode1Id, backendFGNode);
+        manager->appendNode(frontendFGChild->id(), backendFGChild);
+
+        backendFGNode->setFrameGraphManager(manager.data());
+        backendFGChild->setFrameGraphManager(manager.data());
+
+
+        // To geneate the type_info in the QNodePrivate of frontendFGChild
+        Qt3DCore::QNodeCreatedChangeGenerator generator(frontendFGChild);
+
+        QCOMPARE(backendFGNode->childrenIds().size(), 0);
+
+        {
+            // WHEN
+            const auto change = Qt3DCore::QPropertyNodeAddedChangePtr::create(Qt3DCore::QNodeId(), frontendFGChild);
+            backendFGNode->sceneChangeEvent(change);
+
+            // THEN
+            QCOMPARE(backendFGNode->childrenIds().size(), 1);
+            QCOMPARE(backendFGNode->childrenIds().first(), frontendFGChild->id());
+        }
+        {
+            // WHEN
+            const auto change = Qt3DCore::QPropertyNodeRemovedChangePtr::create(Qt3DCore::QNodeId(), frontendFGChild);
+            backendFGNode->sceneChangeEvent(change);
+
+            // THEN
+            QCOMPARE(backendFGNode->childrenIds().size(), 0);
+        }
+    }
+
 };
 
-QTEST_APPLESS_MAIN(tst_FrameGraphNode)
+QTEST_MAIN(tst_FrameGraphNode)
 
 #include "tst_framegraphnode.moc"