Add QNodePrivate::registerDestructionHelper for std::vector containers

Ideally should support generic containers but clang seems to fail to
generate the symbol on the mac.

Change-Id: Ic1d6d815ab9aff60a97b3b0047228f55d7efe158
Reviewed-by: Paul Lemire <paul.lemire@kdab.com>
(cherry picked from commit 5a33b94b348e5c65155b526e7746d9fe45560959)

2 files changed, 209 insertions, 9 deletions

diff --git a/src/core/nodes/qnode_p.h b/src/core/nodes/qnode_p.h
index 5aa4c5328..ddea912f0 100644
--- a/src/core/nodes/qnode_p.h
+++ b/src/core/nodes/qnode_p.h
@@ -54,6 +54,7 @@
 #include <Qt3DCore/qnode.h>
 
 #include <functional>
+#include <vector>
 
 #include <Qt3DCore/private/propertychangehandler_p.h>
 #include <Qt3DCore/private/qchangearbiter_p.h>
@@ -139,6 +140,24 @@ public:
         m_destructionConnections.push_back({node, QObject::connect(node, &QNode::nodeDestroyed, f)});
     }
 
+    template<typename Caller, typename NodeType>
+    void registerDestructionHelper(NodeType *node, DestructionFunctionPointer<Caller, NodeType> func, QList<NodeType*> &)
+    {
+        // If the node is destoyed, we make sure not to keep a dangling pointer to it
+        Q_Q(QNode);
+        auto f = [q, func, node]() { (static_cast<Caller *>(q)->*func)(node); };
+        m_destructionConnections.push_back({node, QObject::connect(node, &QNode::nodeDestroyed, f)});
+    }
+
+    template<typename Caller, typename NodeType>
+    void registerDestructionHelper(NodeType *node, DestructionFunctionPointer<Caller, NodeType> func, std::vector<NodeType*> &)
+    {
+        // If the node is destoyed, we make sure not to keep a dangling pointer to it
+        Q_Q(QNode);
+        auto f = [q, func, node]() { (static_cast<Caller *>(q)->*func)(node); };
+        m_destructionConnections.push_back({node, QObject::connect(node, &QNode::nodeDestroyed, f)});
+    }
+
     template<typename Caller, typename ValueType>
     using DestructionFunctionValue = void (Caller::*)(const ValueType&);
 
diff --git a/tests/auto/core/nodes/tst_nodes.cpp b/tests/auto/core/nodes/tst_nodes.cpp
index aac8638c1..00eaf53bf 100644
--- a/tests/auto/core/nodes/tst_nodes.cpp
+++ b/tests/auto/core/nodes/tst_nodes.cpp
@@ -53,6 +53,7 @@ QT_WARNING_DISABLE_DEPRECATED
 #include <Qt3DCore/private/qcomponent_p.h>
 #include <QSignalSpy>
 #include "testpostmanarbiter.h"
+#include <vector>
 
 class tst_Nodes : public QObject
 {
@@ -116,6 +117,10 @@ private slots:
     void checkTrackedPropertyNamesUpdate();
 
     void checkNodeRemovedFromDirtyListOnDestruction();
+
+    void checkBookkeepingSingleNode();
+    void checkBookkeeping_QVector_OfNode();
+    void checkBookkeeping_stdvector_OfNode();
 };
 
 class ObserverSpy;
@@ -216,7 +221,8 @@ class MyQNode : public Qt3DCore::QNode
 {
     Q_OBJECT
     Q_PROPERTY(QString customProperty READ customProperty WRITE setCustomProperty NOTIFY customPropertyChanged)
-    Q_PROPERTY(MyQNode *nodeProperty READ nodeProperty WRITE setNodeProperty NOTIFY nodePropertyChanged)
+    Q_PROPERTY(Qt3DCore::QNode *nodeProperty READ nodeProperty WRITE setNodeProperty NOTIFY nodePropertyChanged)
+
 public:
     explicit MyQNode(Qt3DCore::QNode *parent = nullptr)
         : QNode(parent)
@@ -256,10 +262,12 @@ public:
         Qt3DCore::QNodePrivate::get(this)->m_hasBackendNode = created;
     }
 
-    MyQNode *nodeProperty() const { return m_nodeProperty; }
+    Qt3DCore::QNode *nodeProperty() const { return m_nodeProperty; }
+
+    const QList<Qt3DCore::QNode *> &attributes() const { return m_attributes; }
 
 public slots:
-    void setNodeProperty(MyQNode *node)
+    void setNodeProperty(Qt3DCore::QNode *node)
     {
         Qt3DCore::QNodePrivate *d = Qt3DCore::QNodePrivate::get(this);
         if (m_nodeProperty == node)
@@ -280,7 +288,7 @@ public slots:
         emit nodePropertyChanged(node);
     }
 
-    void addAttribute(MyQNode *attribute)
+    void addAttribute(Qt3DCore::QNode *attribute)
     {
         Qt3DCore::QNodePrivate *d = Qt3DCore::QNodePrivate::get(this);
         if (!m_attributes.contains(attribute)) {
@@ -300,7 +308,7 @@ public slots:
         }
     }
 
-    void removeAttribute(MyQNode *attribute)
+    void removeAttribute(Qt3DCore::QNode *attribute)
     {
         Qt3DCore::QNodePrivate *d = Qt3DCore::QNodePrivate::get(this);
         d->updateNode(attribute, "attribute", Qt3DCore::PropertyValueRemoved);
@@ -312,12 +320,80 @@ public slots:
 
 signals:
     void customPropertyChanged();
-    void nodePropertyChanged(MyQNode *node);
+    void nodePropertyChanged(Qt3DCore::QNode *node);
 
 protected:
     QString m_customProperty;
-    MyQNode *m_nodeProperty;
-    QVector<MyQNode *> m_attributes;
+    Qt3DCore::QNode *m_nodeProperty;
+    QList<Qt3DCore::QNode *> m_attributes;
+};
+
+
+class MyQNodeVec : public Qt3DCore::QNode
+{
+    Q_OBJECT
+public:
+    explicit MyQNodeVec(Qt3DCore::QNode *parent = nullptr)
+        : QNode(parent)
+    {}
+
+    ~MyQNodeVec()
+    {
+    }
+
+    void setArbiterAndScene(Qt3DCore::QChangeArbiter *arbiter,
+                            Qt3DCore::QScene *scene = nullptr)
+    {
+        Q_ASSERT(arbiter);
+        if (scene)
+            scene->setArbiter(arbiter);
+        Qt3DCore::QNodePrivate::get(this)->setScene(scene);
+        Qt3DCore::QNodePrivate::get(this)->setArbiter(arbiter);
+    }
+
+    void setSimulateBackendCreated(bool created)
+    {
+        Qt3DCore::QNodePrivate::get(this)->m_hasBackendNode = created;
+    }
+
+    const std::vector<Qt3DCore::QNode *> &attributes() const { return m_attributes; }
+
+public slots:
+
+    void addAttribute(Qt3DCore::QNode *attribute)
+    {
+        Qt3DCore::QNodePrivate *d = Qt3DCore::QNodePrivate::get(this);
+        if (std::find(std::begin(m_attributes), std::end(m_attributes), attribute) == std::end(m_attributes)) {
+            m_attributes.push_back(attribute);
+
+            // Ensures proper bookkeeping
+            d->registerDestructionHelper(attribute, &MyQNodeVec::removeAttribute, m_attributes);
+
+            // We need to add it as a child of the current node if it has been declared inline
+            // Or not previously added as a child of the current node so that
+            // 1) The backend gets notified about it's creation
+            // 2) When the current node is destroyed, it gets destroyed as well
+            if (!attribute->parent())
+                attribute->setParent(this);
+
+            d->update();
+        }
+    }
+
+    void removeAttribute(Qt3DCore::QNode *attribute)
+    {
+        Qt3DCore::QNodePrivate *d = Qt3DCore::QNodePrivate::get(this);
+        d->update();
+
+        m_attributes.erase(std::remove(m_attributes.begin(),
+                                       m_attributes.end(),
+                                       attribute), m_attributes.end());
+        // Remove bookkeeping connection
+        d->unregisterDestructionHelper(attribute);
+    }
+
+protected:
+    std::vector<Qt3DCore::QNode *> m_attributes;
 };
 
 class MyQEntity : public Qt3DCore::QEntity
@@ -374,7 +450,7 @@ public:
             m_attributes.append(attribute);
 
             // Ensures proper bookkeeping
-            d->registerDestructionHelper(attribute, &MyQNode::removeAttribute, m_attributes);
+            d->registerDestructionHelper(attribute, &MyQEntity::removeAttribute, m_attributes);
 
             // We need to add it as a child of the current node if it has been declared inline
             // Or not previously added as a child of the current node so that
@@ -387,6 +463,16 @@ public:
         }
     }
 
+    void removeAttribute(MyQNode *attribute)
+    {
+        Qt3DCore::QNodePrivate *d = Qt3DCore::QNodePrivate::get(this);
+        d->update();
+
+        m_attributes.removeOne(attribute);
+        // Remove bookkeeping connection
+        d->unregisterDestructionHelper(attribute);
+    }
+
 public slots:
     void setNodeProperty(MyQNode *node)
     {
@@ -2209,6 +2295,101 @@ void tst_Nodes::checkNodeRemovedFromDirtyListOnDestruction()
     }
 }
 
+void tst_Nodes::checkBookkeepingSingleNode()
+{
+    // GIVEN
+    MyQNode root;
+
+    // THEN
+    QVERIFY(root.nodeProperty() == nullptr);
+
+    {
+        // WHEN
+        MyQNode node;
+        root.setNodeProperty(&node);
+
+        // THEN
+        QCOMPARE(root.nodeProperty(), &node);
+    }
+    // WHEN -> node destroyed
+
+    // THEN
+    QVERIFY(root.nodeProperty() == nullptr);
+}
+
+
+void tst_Nodes::checkBookkeeping_QVector_OfNode()
+{
+    // GIVEN
+    MyQNode root;
+
+    // THEN
+    QVERIFY(root.attributes().size() == 0);
+
+    {
+        // WHEN
+        MyQNode node1;
+        root.addAttribute(&node1);
+
+        // THEN
+        QVERIFY(root.attributes().size() == 1);
+
+        // WHEN
+        {
+            MyQNode node2;
+            root.addAttribute(&node2);
+
+            // THEN
+            QVERIFY(root.attributes().size() == 2);
+            QCOMPARE(root.attributes().first(), &node1);
+            QCOMPARE(root.attributes().last(), &node2);
+        }
+
+        // THEN
+        QVERIFY(root.attributes().size() == 1);
+        QCOMPARE(root.attributes().first(), &node1);
+    }
+
+    // THEN
+    QVERIFY(root.attributes().size() == 0);
+}
+
+void tst_Nodes::checkBookkeeping_stdvector_OfNode()
+{
+    // GIVEN
+    MyQNodeVec root;
+
+    // THEN
+    QVERIFY(root.attributes().size() == 0U);
+
+    {
+        // WHEN
+        MyQNode node1;
+        root.addAttribute(&node1);
+
+        // THEN
+        QVERIFY(root.attributes().size() == 1U);
+
+        // WHEN
+        {
+            MyQNode node2;
+            root.addAttribute(&node2);
+
+            // THEN
+            QVERIFY(root.attributes().size() == 2U);
+            QCOMPARE(root.attributes().front(), &node1);
+            QCOMPARE(root.attributes().back(), &node2);
+        }
+
+        // THEN
+        QVERIFY(root.attributes().size() == 1U);
+        QCOMPARE(root.attributes().front(), &node1);
+    }
+
+    // THEN
+    QVERIFY(root.attributes().size() == 0U);
+}
+
 
 QTEST_MAIN(tst_Nodes)