summaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorSean Harmer <sean.harmer@kdab.com>2018-01-29 11:04:45 +0000
committerJani Heikkinen <jani.heikkinen@qt.io>2018-02-08 07:24:42 +0000
commitb923dae74e9baf75aae99f87b19568ec1dc39f82 (patch)
tree52e8673b60b1f4aeecab4e31e424e055a1141e7d /src
parent89b7f69886da5de98a7ff27d44ca34cd2c1e45e1 (diff)
Ensure node creation changes are sent before using in list properties
This completes the fix for out of order event delivery related to creation changes. We now ensure that QNodes used as values in singular and list properties are fully constructed on the backend before they are referenced in properties of other nodes. Also added a check to not recurse into sending too many changes when adding a child node. Written with Svenn-Arne Dragly. Task-number: Task-number: QTBUG-65956 Change-Id: I1470e0f685c81d1277ac04ad985ec1b76f1c27c0 Reviewed-by: Paul Lemire <paul.lemire@kdab.com> (cherry picked from commit 8fa23602cff47de6d19d05a8428a8e753bf73d61) Reviewed-by: Antti Määttä <antti.maatta@qt.io>
Diffstat (limited to 'src')
-rw-r--r--src/core/changes/qpropertynodeaddedchange.cpp10
-rw-r--r--src/core/nodes/qnode.cpp17
-rw-r--r--src/core/nodes/qnode_p.h4
3 files changed, 29 insertions, 2 deletions
diff --git a/src/core/changes/qpropertynodeaddedchange.cpp b/src/core/changes/qpropertynodeaddedchange.cpp
index 390a170b6..9f8e50872 100644
--- a/src/core/changes/qpropertynodeaddedchange.cpp
+++ b/src/core/changes/qpropertynodeaddedchange.cpp
@@ -76,6 +76,16 @@ QPropertyNodeAddedChange::QPropertyNodeAddedChange(QNodeId subjectId, QNode *nod
{
Q_D(QPropertyNodeAddedChange);
d->m_addedNodeIdTypePair = QNodeIdTypePair(node->id(), QNodePrivate::findStaticMetaObject(node->metaObject()));
+
+ // Ensure the node has issued a node creation change. We can end
+ // up here if a newly created node with a parent is immediately set
+ // as a property on another node. In this case the deferred call to
+ // _q_postConstructorInit() will not have happened yet as the event
+ // loop will still be blocked. So force it here and we catch this
+ // eventuality in the _q_postConstructorInit() function so that we
+ // do not repeat the creation and new child scene change events.
+ if (node)
+ QNodePrivate::get(node)->_q_postConstructorInit();
}
/*! \internal */
diff --git a/src/core/nodes/qnode.cpp b/src/core/nodes/qnode.cpp
index dbe3fd102..e5a3aca71 100644
--- a/src/core/nodes/qnode.cpp
+++ b/src/core/nodes/qnode.cpp
@@ -74,6 +74,7 @@ QNodePrivate::QNodePrivate()
, m_blockNotifications(false)
, m_hasBackendNode(false)
, m_enabled(true)
+ , m_notifiedParent(false)
, m_defaultPropertyTrackMode(QNode::TrackFinalValues)
, m_propertyChangesSetup(false)
, m_signals(this)
@@ -210,13 +211,19 @@ void QNodePrivate::_q_addChild(QNode *childNode)
Q_ASSERT(childNode);
Q_ASSERT_X(childNode->parent() == q_func(), Q_FUNC_INFO, "not a child of this node");
+ // Have we already notified the parent about its new child? If so, bail out
+ // early so that we do not send more than one new child event to the backend
+ QNodePrivate *childD = QNodePrivate::get(childNode);
+ if (childD->m_notifiedParent == true)
+ return;
+
// Store our id as the parentId in the child so that even if the child gets
// removed from the scene as part of the destruction of the parent, when the
// parent's children are deleted in the QObject dtor, we still have access to
// the parentId. If we didn't store this, we wouldn't have access at that time
// because the parent would then only be a QObject, the QNode part would have
// been destroyed already.
- QNodePrivate::get(childNode)->m_parentId = m_id;
+ childD->m_parentId = m_id;
if (!m_scene)
return;
@@ -224,6 +231,11 @@ void QNodePrivate::_q_addChild(QNode *childNode)
// We need to send a QPropertyNodeAddedChange to the backend
// to notify the backend that we have a new child
if (m_changeArbiter != nullptr) {
+ // Flag that we have notified the parent. We do this immediately before
+ // creating the change because that recurses back into this function and
+ // we need to catch that to avoid sending more than one new child event
+ // to the backend.
+ childD->m_notifiedParent = true;
const auto change = QPropertyNodeAddedChangePtr::create(m_id, childNode);
change->setPropertyName("children");
notifyObservers(change);
@@ -299,6 +311,9 @@ void QNodePrivate::_q_setParentHelper(QNode *parent)
notifyDestructionChangesAndRemoveFromScene();
}
+ // Flag that we need to notify any new parent
+ m_notifiedParent = false;
+
// Basically QObject::setParent but for QObjectPrivate
QObjectPrivate::setParent_helper(parent);
QNode *newParentNode = q->parentNode();
diff --git a/src/core/nodes/qnode_p.h b/src/core/nodes/qnode_p.h
index ad9d2376e..87a0226f1 100644
--- a/src/core/nodes/qnode_p.h
+++ b/src/core/nodes/qnode_p.h
@@ -100,6 +100,7 @@ public:
bool m_blockNotifications;
bool m_hasBackendNode;
bool m_enabled;
+ bool m_notifiedParent;
QNode::PropertyTrackingMode m_defaultPropertyTrackMode;
QHash<QString, QNode::PropertyTrackingMode> m_trackedPropertiesOverrides;
@@ -137,10 +138,11 @@ public:
static const QMetaObject *findStaticMetaObject(const QMetaObject *metaObject);
+ void _q_postConstructorInit();
+
private:
void notifyCreationChange();
void notifyDestructionChangesAndRemoveFromScene();
- void _q_postConstructorInit();
void _q_addChild(QNode *childNode);
void _q_removeChild(QNode *childNode);
void _q_setParentHelper(QNode *parent);