Add code path for destroying backend nodes when removed from the scene

...and not using cloning.

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

3 files changed, 43 insertions, 33 deletions

diff --git a/src/core/nodes/qdestructionidandtypecollector_p.h b/src/core/nodes/qdestructionidandtypecollector_p.h
index 3f64a409e..01efe4975 100644
--- a/src/core/nodes/qdestructionidandtypecollector_p.h
+++ b/src/core/nodes/qdestructionidandtypecollector_p.h
@@ -70,7 +70,7 @@ private:
 
         // Mark this node as having been handled for destruction so we don't
         // repeat it unnecessarily in an O(n^2) manner
-        QNodePrivate::get(node)->m_destroyedChangeCreated = true;
+        QNodePrivate::get(node)->m_hasBackendNode = true;
     }
 
     QVector<QNodeIdTypePair> m_subtreeIdsAndTypes;
diff --git a/src/core/nodes/qnode.cpp b/src/core/nodes/qnode.cpp
index fbbeef87e..11cd8fb3f 100644
--- a/src/core/nodes/qnode.cpp
+++ b/src/core/nodes/qnode.cpp
@@ -70,7 +70,7 @@ QNodePrivate::QNodePrivate()
     , m_id(QNodeId::createId())
     , m_blockNotifications(false)
     , m_wasCleanedUp(false)
-    , m_destroyedChangeCreated(false)
+    , m_hasBackendNode(false)
     , m_enabled(true)
     , m_propertyChangesSetup(false)
     , m_signals(this)
@@ -132,37 +132,47 @@ void QNodePrivate::_q_addChild(QNode *childNode)
 void QNodePrivate::_q_removeChild(QNode *childNode)
 {
     Q_ASSERT(childNode);
-    if (childNode->parent() != q_func())
-        qCWarning(Nodes) << Q_FUNC_INFO << "not a child of " << this;
 
-    // Notify only if child isn't a clone
-    if (m_changeArbiter != Q_NULLPTR) {
-        QScenePropertyChangePtr e(new QScenePropertyChange(NodeAboutToBeDeleted, QSceneChange::Node, m_id));
-        e->setPropertyName("node");
-        // We need to clone the parent of the childNode we send
-        //        QNode *parentClone = QNode::clone(childNode->parentNode());
-        //        QNode *childClone = Q_NULLPTR;
-        //        Q_FOREACH (QObject *c, parentClone->children()) {
-        //            QNode *clone = qobject_cast<QNode *>(c);
-        //            if (clone != Q_NULLPTR && clone->id() == childNode->id()) {
-        //                childClone = clone;
-        //                break;
-        //            }
-        //        }
-
-        // We cannot clone the parent as it seems that the childNode is already removed
-        // from the parent when the ChildRemoved event is triggered
-        // and that would therefore return us a childNode NULL (because not found in the parent's children list)
-        // and crash the backend
-
-        QNode *childClone = QNode::clone(childNode);
-        e->setValue(QVariant::fromValue(QNodePtr(childClone, &QNodePrivate::nodePtrDeleter)));
-        notifyObservers(e);
-    }
+    if (ms_useCloning) {
+        if (childNode->parent() != q_func())
+            qCWarning(Nodes) << Q_FUNC_INFO << "not a child of " << this;
+
+        // Notify only if child isn't a clone
+        if (m_changeArbiter != Q_NULLPTR) {
+            QScenePropertyChangePtr e(new QScenePropertyChange(NodeAboutToBeDeleted, QSceneChange::Node, m_id));
+            e->setPropertyName("node");
+            // We need to clone the parent of the childNode we send
+            //        QNode *parentClone = QNode::clone(childNode->parentNode());
+            //        QNode *childClone = Q_NULLPTR;
+            //        Q_FOREACH (QObject *c, parentClone->children()) {
+            //            QNode *clone = qobject_cast<QNode *>(c);
+            //            if (clone != Q_NULLPTR && clone->id() == childNode->id()) {
+            //                childClone = clone;
+            //                break;
+            //            }
+            //        }
+
+            // We cannot clone the parent as it seems that the childNode is already removed
+            // from the parent when the ChildRemoved event is triggered
+            // and that would therefore return us a childNode NULL (because not found in the parent's children list)
+            // and crash the backend
+
+            QNode *childClone = QNode::clone(childNode);
+            e->setValue(QVariant::fromValue(QNodePtr(childClone, &QNodePrivate::nodePtrDeleter)));
+            notifyObservers(e);
+        }
 
-    // Recursively unset the scene on all children
-    QNodeVisitor visitor;
-    visitor.traverse(childNode, this, &QNodePrivate::unsetSceneHelper);
+        // Recursively unset the scene on all children
+        QNodeVisitor visitor;
+        visitor.traverse(childNode, this, &QNodePrivate::unsetSceneHelper);
+    } else {
+        auto childNodePrivate = get(childNode);
+        if (!childNodePrivate->m_hasBackendNode) {
+            const QDestructionIdAndTypeCollector collector(childNode);
+            auto destroyedChange = QNodeDestroyedChangePtr::create(childNode, collector.subtreeIdsAndTypes());
+            notifyObservers(destroyedChange);
+        }
+    }
 }
 
 void QNodePrivate::registerNotifiedProperties()
@@ -487,7 +497,7 @@ QNode::~QNode()
     // Create a QNodeDestroyedChange for this node that informs the backend that
     // this node and all of its children are going away
     Q_D(QNode);
-    if (!d->m_destroyedChangeCreated) {
+    if (!d->m_hasBackendNode) {
         const QDestructionIdAndTypeCollector collector(this);
         const auto destroyedChange = QNodeDestroyedChangePtr::create(this, collector.subtreeIdsAndTypes());
         d->notifyObservers(destroyedChange);
diff --git a/src/core/nodes/qnode_p.h b/src/core/nodes/qnode_p.h
index 17c38ba53..8fab0817a 100644
--- a/src/core/nodes/qnode_p.h
+++ b/src/core/nodes/qnode_p.h
@@ -90,7 +90,7 @@ public:
     mutable QNodeId m_id;
     bool m_blockNotifications;
     bool m_wasCleanedUp;
-    bool m_destroyedChangeCreated;
+    bool m_hasBackendNode;
     bool m_enabled;
 
     static QNodePrivate *get(QNode *q);