From 66fc1e35db664ecaf0b37c855eea7391c0576773 Mon Sep 17 00:00:00 2001 From: Paul Lemire Date: Wed, 25 Feb 2015 15:38:42 +0100 Subject: ChangeArbiter/SceneChange: use id as change subject Instead of having a QNode* or QObservable as the subject of a change, we use it's unique NodeId instead. That will prevent the ChangeArbiter from trying to distribute changes by looking at a QNode/QObservable id when the QNode/QObservable might have been destroyed in the meantime. Change-Id: Ia419d5b841434fd65522c8c65de552089cfe97cf Task-number: QTBUG-44628 Reviewed-by: Laszlo Agocs Reviewed-by: Sean Harmer --- src/core/nodes/qcomponent.cpp | 4 ++-- src/core/nodes/qentity.cpp | 4 ++-- src/core/nodes/qnode.cpp | 9 +++----- src/core/qbackendscenepropertychange.cpp | 9 ++++---- src/core/qbackendscenepropertychange.h | 5 ++--- src/core/qchangearbiter.cpp | 9 ++++---- src/core/qscenechange.cpp | 38 +++++++------------------------- src/core/qscenechange.h | 14 ++++-------- src/core/qscenechange_p.h | 3 ++- src/core/qscenepropertychange.cpp | 18 ++++----------- src/core/qscenepropertychange.h | 6 ++--- 11 files changed, 38 insertions(+), 81 deletions(-) (limited to 'src/core') diff --git a/src/core/nodes/qcomponent.cpp b/src/core/nodes/qcomponent.cpp index a05b9980b..2827db9f9 100644 --- a/src/core/nodes/qcomponent.cpp +++ b/src/core/nodes/qcomponent.cpp @@ -59,7 +59,7 @@ void QComponentPrivate::addEntity(QEntity *entity) // We notify only if we have a QChangeArbiter if (m_changeArbiter != Q_NULLPTR) { Q_Q(QComponent); - QScenePropertyChangePtr e(new QScenePropertyChange(ComponentAdded, q)); + QScenePropertyChangePtr e(new QScenePropertyChange(ComponentAdded, QSceneChange::Node, q->id())); e->setPropertyName("entity"); e->setValue(QVariant::fromValue(entity->id())); notifyObservers(e); @@ -71,7 +71,7 @@ void QComponentPrivate::removeEntity(QEntity *entity) // We notify only if we have a QChangeArbiter if (m_changeArbiter != Q_NULLPTR) { Q_Q(QComponent); - QScenePropertyChangePtr e(new QScenePropertyChange(ComponentRemoved, q)); + QScenePropertyChangePtr e(new QScenePropertyChange(ComponentRemoved, QSceneChange::Node, q->id())); e->setPropertyName("entity"); e->setValue(QVariant::fromValue(entity->id())); notifyObservers(e); diff --git a/src/core/nodes/qentity.cpp b/src/core/nodes/qentity.cpp index a0f980b5a..0e8c596d0 100644 --- a/src/core/nodes/qentity.cpp +++ b/src/core/nodes/qentity.cpp @@ -117,7 +117,7 @@ void QEntity::addComponent(QComponent *comp) // Sending a full fledged component in the notification as we'll need // to know which type of component it was and its properties to create // the backend object - QScenePropertyChangePtr propertyChange(new QScenePropertyChange(ComponentAdded, this)); + QScenePropertyChangePtr propertyChange(new QScenePropertyChange(ComponentAdded, QSceneChange::Node, id())); propertyChange->setPropertyName("component"); propertyChange->setValue(QVariant::fromValue(QNodePtr(QNode::clone(comp), &QNodePrivate::nodePtrDeleter))); d->notifyObservers(propertyChange); @@ -137,7 +137,7 @@ void QEntity::removeComponent(QComponent *comp) // Sending just the component id as it is the only part needed to // cleanup the backend object. This way we avoid a clone which might // fail in the case of large scenes. - QScenePropertyChangePtr propertyChange(new QScenePropertyChange(ComponentRemoved, this)); + QScenePropertyChangePtr propertyChange(new QScenePropertyChange(ComponentRemoved, QSceneChange::Node, id())); propertyChange->setValue(QVariant::fromValue(comp->id())); propertyChange->setPropertyName("componentId"); d->notifyObservers(propertyChange); diff --git a/src/core/nodes/qnode.cpp b/src/core/nodes/qnode.cpp index c4b6088e6..4de2f85a5 100644 --- a/src/core/nodes/qnode.cpp +++ b/src/core/nodes/qnode.cpp @@ -82,8 +82,7 @@ void QNodePrivate::addChild(QNode *childNode) // We notify only if we have a QChangeArbiter if (m_changeArbiter != Q_NULLPTR) { - Q_Q(QNode); - QScenePropertyChangePtr e(new QScenePropertyChange(NodeCreated, q)); + QScenePropertyChangePtr e(new QScenePropertyChange(NodeCreated, QSceneChange::Node, m_id)); e->setPropertyName("node"); // We need to clone the parent of the childNode we send QNode *parentClone = QNode::clone(q_func()); @@ -109,8 +108,7 @@ void QNodePrivate::removeChild(QNode *childNode) // Notify only if child isn't a clone if (m_changeArbiter != Q_NULLPTR) { - Q_Q(QNode); - QScenePropertyChangePtr e(new QScenePropertyChange(NodeAboutToBeDeleted, q)); + 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()); @@ -229,8 +227,7 @@ void QNodePrivate::notifyPropertyChange(const char *name, const QVariant &value) if (m_blockNotifications) return; - Q_Q(QNode); - QScenePropertyChangePtr e(new QScenePropertyChange(NodeUpdated, q)); + QScenePropertyChangePtr e(new QScenePropertyChange(NodeUpdated, QSceneChange::Node, m_id)); e->setPropertyName(name); e->setValue(value); notifyObservers(e); diff --git a/src/core/qbackendscenepropertychange.cpp b/src/core/qbackendscenepropertychange.cpp index cdbce641a..7d33414f7 100644 --- a/src/core/qbackendscenepropertychange.cpp +++ b/src/core/qbackendscenepropertychange.cpp @@ -52,8 +52,8 @@ QBackendScenePropertyChangePrivate::~QBackendScenePropertyChangePrivate() { } -QBackendScenePropertyChange::QBackendScenePropertyChange(ChangeFlag type, QBackendNode *subject, QSceneChange::Priority priority) - : QScenePropertyChange(*new QBackendScenePropertyChangePrivate(this), type, subject->d_func(), priority) +QBackendScenePropertyChange::QBackendScenePropertyChange(ChangeFlag type, const QNodeId &subjectId, QSceneChange::Priority priority) + : QScenePropertyChange(*new QBackendScenePropertyChangePrivate(this), type, Observable, subjectId, priority) { } @@ -61,6 +61,7 @@ QBackendScenePropertyChange::~QBackendScenePropertyChange() { } +// TO DO get rid off setTargetNode, use the subject instead ?? void QBackendScenePropertyChange::setTargetNode(const QNodeId &id) { Q_D(QBackendScenePropertyChange); @@ -78,8 +79,8 @@ QBackendScenePropertyChange::QBackendScenePropertyChange(QBackendScenePropertyCh { } -QBackendScenePropertyChange::QBackendScenePropertyChange(QBackendScenePropertyChangePrivate &dd, ChangeFlag type, QBackendNode *subject, QSceneChange::Priority priority) - : QScenePropertyChange(dd, type, subject->d_func(), priority) +QBackendScenePropertyChange::QBackendScenePropertyChange(QBackendScenePropertyChangePrivate &dd, ChangeFlag type, const QNodeId &subjectId, QSceneChange::Priority priority) + : QScenePropertyChange(dd, type, Observable, subjectId, priority) { } diff --git a/src/core/qbackendscenepropertychange.h b/src/core/qbackendscenepropertychange.h index 8f26f9627..382418500 100644 --- a/src/core/qbackendscenepropertychange.h +++ b/src/core/qbackendscenepropertychange.h @@ -50,7 +50,7 @@ class QBackendNode; class QT3DCORESHARED_EXPORT QBackendScenePropertyChange : public QScenePropertyChange { public: - QBackendScenePropertyChange(ChangeFlag type, QBackendNode *subject, + QBackendScenePropertyChange(ChangeFlag type, const QNodeId &subjectId, Priority priority = Standard); virtual ~QBackendScenePropertyChange(); @@ -60,8 +60,7 @@ public: protected: Q_DECLARE_PRIVATE(QBackendScenePropertyChange) QBackendScenePropertyChange(QBackendScenePropertyChangePrivate &dd); - QBackendScenePropertyChange(QBackendScenePropertyChangePrivate &dd, ChangeFlag type, - QBackendNode *subject, Priority priority = Standard); + QBackendScenePropertyChange(QBackendScenePropertyChangePrivate &dd, ChangeFlag type, const QNodeId &subjectId, Priority priority = Standard); }; typedef QSharedPointer QBackendScenePropertyChangePtr; diff --git a/src/core/qchangearbiter.cpp b/src/core/qchangearbiter.cpp index 86ddc7d72..1d111d642 100644 --- a/src/core/qchangearbiter.cpp +++ b/src/core/qchangearbiter.cpp @@ -124,8 +124,7 @@ void QChangeArbiter::distributeQueueChanges(QChangeQueue *changeQueue) switch (change->observableType()) { case QSceneChange::Observable: { - QObservableInterface *subject = change->subject().m_observable; - QNodeId nodeId = m_scene->nodeIdFromObservable(subject); + const QNodeId nodeId = change->subjectId(); if (m_nodeObservations.contains(nodeId)) { QObserverList &observers = m_nodeObservations[nodeId]; Q_FOREACH (const QObserverPair&observer, observers) { @@ -139,9 +138,9 @@ void QChangeArbiter::distributeQueueChanges(QChangeQueue *changeQueue) } case QSceneChange::Node: { - QNode *subject = change->subject().m_node; - if (m_nodeObservations.contains(subject->id())) { - QObserverList &observers = m_nodeObservations[subject->id()]; + const QNodeId nodeId = change->subjectId(); + if (m_nodeObservations.contains(nodeId)) { + QObserverList &observers = m_nodeObservations[nodeId]; Q_FOREACH (const QObserverPair&observer, observers) { if ((change->type() & observer.first)) observer.second->sceneChangeEvent(change); diff --git a/src/core/qscenechange.cpp b/src/core/qscenechange.cpp index c5bb1622d..20218f04c 100644 --- a/src/core/qscenechange.cpp +++ b/src/core/qscenechange.cpp @@ -67,26 +67,15 @@ QSceneChangePrivate::~QSceneChangePrivate() { } -QSceneChange::QSceneChange(ChangeFlag type, QObservableInterface *observable, QSceneChange::Priority priority) +QSceneChange::QSceneChange(ChangeFlag type, ObservableType observableType, const QNodeId &subjectId, QSceneChange::Priority priority) : d_ptr(new QSceneChangePrivate(this)) { Q_D(QSceneChange); d->m_type = type; d->m_priority = priority; d->m_timestamp = QDateTime::currentMSecsSinceEpoch(); - d->m_subject.m_observable = observable; - d->m_subjectType = Observable; -} - -QSceneChange::QSceneChange(ChangeFlag type, QNode *node, QSceneChange::Priority priority) - : d_ptr(new QSceneChangePrivate(this)) -{ - Q_D(QSceneChange); - d->m_type = type; - d->m_priority = priority; - d->m_timestamp = QDateTime::currentMSecsSinceEpoch(); - d->m_subject.m_node = node; - d->m_subjectType = Node; + d->m_subjectId = subjectId; + d->m_subjectType = observableType; } QSceneChange::~QSceneChange() @@ -99,26 +88,15 @@ QSceneChange::QSceneChange(QSceneChangePrivate &dd) { } -QSceneChange::QSceneChange(QSceneChangePrivate &dd, ChangeFlag type, QObservableInterface *observable, QSceneChange::Priority priority) - : d_ptr(&dd) -{ - Q_D(QSceneChange); - d->m_type = type; - d->m_priority = priority; - d->m_timestamp = QDateTime::currentMSecsSinceEpoch(); - d->m_subject.m_observable = observable; - d->m_subjectType = Observable; -} - -QSceneChange::QSceneChange(QSceneChangePrivate &dd, ChangeFlag type, QNode *node, QSceneChange::Priority priority) +QSceneChange::QSceneChange(QSceneChangePrivate &dd, ChangeFlag type, ObservableType observableType, const QNodeId &subjectId, QSceneChange::Priority priority) : d_ptr(&dd) { Q_D(QSceneChange); d->m_type = type; d->m_priority = priority; d->m_timestamp = QDateTime::currentMSecsSinceEpoch(); - d->m_subject.m_node = node; - d->m_subjectType = Node; + d->m_subjectId = subjectId; + d->m_subjectType = observableType; } ChangeFlag QSceneChange::type() const @@ -145,10 +123,10 @@ QSceneChange::ObservableType QSceneChange::observableType() const return d->m_subjectType; } -QSceneChange::SubjectUnion QSceneChange::subject() const +QNodeId QSceneChange::subjectId() const { Q_D(const QSceneChange); - return d->m_subject; + return d->m_subjectId; } } // Qt3D diff --git a/src/core/qscenechange.h b/src/core/qscenechange.h index f52871b67..3c22638d3 100644 --- a/src/core/qscenechange.h +++ b/src/core/qscenechange.h @@ -39,6 +39,7 @@ #include #include +#include QT_BEGIN_NAMESPACE @@ -76,27 +77,20 @@ public: Node }; - union SubjectUnion { - QObservableInterface *m_observable; - QNode *m_node; - }; - - QSceneChange(ChangeFlag type, QObservableInterface *observable, Priority priority = Standard); - QSceneChange(ChangeFlag type, QNode *node, Priority priority = Standard); + QSceneChange(ChangeFlag type, ObservableType observableType, const QNodeId &subjectId, Priority priority = Standard); virtual ~QSceneChange(); ChangeFlag type() const; qint64 timestamp() const; QSceneChange::Priority priority() const; QSceneChange::ObservableType observableType() const; - QSceneChange::SubjectUnion subject() const; + QNodeId subjectId() const; protected: Q_DECLARE_PRIVATE(QSceneChange) QSceneChangePrivate *d_ptr; QSceneChange(QSceneChangePrivate &dd); - QSceneChange(QSceneChangePrivate &dd, ChangeFlag type, QObservableInterface *observable, Priority priority = Standard); - QSceneChange(QSceneChangePrivate &dd, ChangeFlag type, QNode *node, Priority priority = Standard); + QSceneChange(QSceneChangePrivate &dd, ChangeFlag type, ObservableType observableType, const QNodeId &subjectId, Priority priority = Standard); // TODO: add timestamp from central clock and priority level // These can be used to resolve any conflicts between events diff --git a/src/core/qscenechange_p.h b/src/core/qscenechange_p.h index 40748e444..b903b1035 100644 --- a/src/core/qscenechange_p.h +++ b/src/core/qscenechange_p.h @@ -44,6 +44,7 @@ QT_BEGIN_NAMESPACE namespace Qt3D { class QSceneChange; +class QNodeId; class QSceneChangePrivate { @@ -55,7 +56,7 @@ public : QSceneChange *q_ptr; - QSceneChange::SubjectUnion m_subject; + QNodeId m_subjectId; QSceneChange::ObservableType m_subjectType; ChangeFlag m_type; QSceneChange::Priority m_priority; diff --git a/src/core/qscenepropertychange.cpp b/src/core/qscenepropertychange.cpp index 14144b157..9daac1bd0 100644 --- a/src/core/qscenepropertychange.cpp +++ b/src/core/qscenepropertychange.cpp @@ -67,8 +67,8 @@ void QScenePropertyChangePrivate::operator delete(void *ptr, size_t size) QScenePropertyChangePrivate::m_allocator->deallocateRawMemory(ptr, size); } -QScenePropertyChange::QScenePropertyChange(ChangeFlag type, QObservableInterface *subject, QSceneChange::Priority priority) - : QSceneChange(*new QScenePropertyChangePrivate(this), type, subject, priority) +QScenePropertyChange::QScenePropertyChange(ChangeFlag type, ObservableType observableType, const QNodeId &subjectId, QSceneChange::Priority priority) + : QSceneChange(*new QScenePropertyChangePrivate(this), type, observableType, subjectId, priority) { } @@ -77,8 +77,8 @@ QScenePropertyChange::QScenePropertyChange(QScenePropertyChangePrivate &dd) { } -QScenePropertyChange::QScenePropertyChange(ChangeFlag type, QNode *node, QSceneChange::Priority priority) - : QSceneChange(*new QScenePropertyChangePrivate(this), type, node, priority) +QScenePropertyChange::QScenePropertyChange(QScenePropertyChangePrivate &dd, ChangeFlag type, ObservableType observableType, const QNodeId &subjectId, QSceneChange::Priority priority) + : QSceneChange(dd, type, observableType, subjectId, priority) { } @@ -86,16 +86,6 @@ QScenePropertyChange::~QScenePropertyChange() { } -QScenePropertyChange::QScenePropertyChange(QScenePropertyChangePrivate &dd, ChangeFlag type, QObservableInterface *subject, QSceneChange::Priority priority) - : QSceneChange(dd, type, subject, priority) -{ -} - -QScenePropertyChange::QScenePropertyChange(QScenePropertyChangePrivate &dd, ChangeFlag type, QNode *node, QSceneChange::Priority priority) - : QSceneChange(dd, type, node, priority) -{ -} - const char *QScenePropertyChange::propertyName() const { Q_D(const QScenePropertyChange); diff --git a/src/core/qscenepropertychange.h b/src/core/qscenepropertychange.h index 8e1363b9c..dcedf20a5 100644 --- a/src/core/qscenepropertychange.h +++ b/src/core/qscenepropertychange.h @@ -48,7 +48,7 @@ class QScenePropertyChangePrivate; class QT3DCORESHARED_EXPORT QScenePropertyChange : public QSceneChange { public: - QScenePropertyChange(ChangeFlag type, QNode *node, Priority priority = Standard); + QScenePropertyChange(ChangeFlag type, ObservableType observableType, const QNodeId &subjectId, Priority priority = Standard); virtual ~QScenePropertyChange(); const char *propertyName() const; @@ -62,10 +62,8 @@ public: protected: Q_DECLARE_PRIVATE(QScenePropertyChange) - QScenePropertyChange(ChangeFlag type, QObservableInterface *subject, Priority priority = Standard); QScenePropertyChange(QScenePropertyChangePrivate &dd); - QScenePropertyChange(QScenePropertyChangePrivate &dd, ChangeFlag type, QObservableInterface *subject, Priority priority = Standard); - QScenePropertyChange(QScenePropertyChangePrivate &dd, ChangeFlag type, QNode *node, Priority priority = Standard); + QScenePropertyChange(QScenePropertyChangePrivate &dd, ChangeFlag type, ObservableType observableType, const QNodeId &subjectId, Priority priority = Standard); }; typedef QSharedPointer QScenePropertyChangePtr; -- cgit v1.2.3