diff options
author | Paul Lemire <paul.lemire@kdab.com> | 2017-05-09 14:23:24 +0200 |
---|---|---|
committer | Jani Heikkinen <jani.heikkinen@qt.io> | 2017-05-10 04:12:45 +0000 |
commit | 2eb53d735488cd2fb609e51c036c5ee3554557c7 (patch) | |
tree | b7c654fe29267515425e0830ada1174ea3dfea5e | |
parent | c0f12806a2851f481788f383fef569d441b131ed (diff) |
QNode: setParent create creation change only when needed
Two cases need to be handled by setParent:
1) Parent already has a backend node and receives a new child
-> in which case we need to send the creation event to the backend
2) Parent was created in the frontend, but has no backend (delayed
notification sending because a ctor can't call a virtual)
-> in that case, when adding a child and setting its parent we shouldn't be
sending the creation change. We rather let that be handled when the
creation change for the parent is requested
Change-Id: I434c7d4e6af785c0314ac6538dc689992d90ed0c
Task-number: QTBUG-60612
Reviewed-by: Sean Harmer <sean.harmer@kdab.com>
Reviewed-by: Oleg Evseev <ev.mipt@gmail.com>
-rw-r--r-- | src/core/nodes/qnode.cpp | 18 | ||||
-rw-r--r-- | tests/auto/core/nodes/tst_nodes.cpp | 63 | ||||
-rw-r--r-- | tests/auto/core/qchangearbiter/tst_qchangearbiter.cpp | 4 | ||||
-rw-r--r-- | tests/auto/core/qscene/tst_qscene.cpp | 3 |
4 files changed, 84 insertions, 4 deletions
diff --git a/src/core/nodes/qnode.cpp b/src/core/nodes/qnode.cpp index 42c8887ce..ce5e76a55 100644 --- a/src/core/nodes/qnode.cpp +++ b/src/core/nodes/qnode.cpp @@ -114,7 +114,7 @@ void QNodePrivate::notifyCreationChange() Q_Q(QNode); // Do nothing if we already have already sent a node creation change // and not a subsequent node destroyed change. - if (m_hasBackendNode) + if (m_hasBackendNode || !m_scene) return; QNodeCreatedChangeGenerator generator(q); const auto creationChanges = generator.creationChanges(); @@ -305,7 +305,21 @@ void QNodePrivate::_q_setParentHelper(QNode *parent) visitor.traverse(q, newParentNode->d_func(), &QNodePrivate::setSceneHelper); } - notifyCreationChange(); + // We want to make sure that subTreeRoot is always created before + // child. + // Given a case such as below + // QEntity *subTreeRoot = new QEntity(someGlobalExisitingRoot) + // QEntity *child = new QEntity(); + // child->setParent(subTreeRoot) + // We need to take into account that subTreeRoot needs to be + // created in the backend before the child. + // Therefore we only call notifyCreationChanges if the parent + // hasn't been created yet as we know that when the parent will be + // fully created, it will also send the changes for all of its + // children + + if (QNodePrivate::get(newParentNode)->m_hasBackendNode) + notifyCreationChange(); } // If we have a valid new parent, we let him know that we are its child diff --git a/tests/auto/core/nodes/tst_nodes.cpp b/tests/auto/core/nodes/tst_nodes.cpp index 89d396931..25c2a6dba 100644 --- a/tests/auto/core/nodes/tst_nodes.cpp +++ b/tests/auto/core/nodes/tst_nodes.cpp @@ -79,6 +79,8 @@ private slots: void appendingChildEntitiesToNode(); void removingChildEntitiesFromNode(); + void checkConstructionSetParentMix(); // QTBUG-60612 + void appendingComponentToEntity(); void appendingParentlessComponentToEntity(); void removingComponentFromEntity(); @@ -204,6 +206,11 @@ public: Qt3DCore::QNodePrivate::get(this)->setArbiter(arbiter); } + void setSimulateBackendCreated(bool created) + { + Qt3DCore::QNodePrivate::get(this)->m_hasBackendNode = created; + } + signals: void customPropertyChanged(); @@ -232,6 +239,11 @@ public: Qt3DCore::QNodePrivate::get(this)->setScene(scene); Qt3DCore::QNodePrivate::get(this)->setArbiter(arbiter); } + + void setSimulateBackendCreated(bool created) + { + Qt3DCore::QNodePrivate::get(this)->m_hasBackendNode = created; + } }; class MyQComponent : public Qt3DCore::QComponent @@ -389,6 +401,8 @@ void tst_Nodes::appendSingleChildNodeToNodeSceneExplicitParenting() QScopedPointer<MyQNode> node(new MyQNode()); // WHEN node->setArbiterAndScene(&spy, &scene); + node->setSimulateBackendCreated(true); + // THEN QVERIFY(Qt3DCore::QNodePrivate::get(node.data())->scene() != nullptr); @@ -405,7 +419,7 @@ void tst_Nodes::appendSingleChildNodeToNodeSceneExplicitParenting() // THEN QVERIFY(child->parent() == node.data()); QVERIFY(child->parentNode() == node.data()); - QCOMPARE(spy.events.size(), 2); + QCOMPARE(spy.events.size(), 2); // Created + Child Added QCOMPARE(node->children().count(), 1); QVERIFY(Qt3DCore::QNodePrivate::get(child.data())->scene() != nullptr); @@ -482,6 +496,7 @@ void tst_Nodes::appendMultipleChildNodesToNodeScene() // WHEN node->setArbiterAndScene(&spy, &scene); + node->setSimulateBackendCreated(true); // THEN QVERIFY(Qt3DCore::QNodePrivate::get(node.data())->scene() != nullptr); @@ -664,6 +679,7 @@ void tst_Nodes::removingSingleChildNodeFromNode() // WHEN root->setArbiterAndScene(&spy, &scene); + root->setSimulateBackendCreated(true); child->setParent(root.data()); // Clear any creation event @@ -790,6 +806,47 @@ void tst_Nodes::removingChildEntitiesFromNode() QVERIFY(childEntity->parent() == nullptr); } +void tst_Nodes::checkConstructionSetParentMix() +{ + // GIVEN + ObserverSpy spy; + Qt3DCore::QScene scene; + QScopedPointer<MyQNode> root(new MyQNode()); + + // WHEN + root->setArbiterAndScene(&spy, &scene); + root->setSimulateBackendCreated(true); + + // THEN + QVERIFY(Qt3DCore::QNodePrivate::get(root.data())->scene() != nullptr); + + // WHEN + Qt3DCore::QEntity *subTreeRoot = new Qt3DCore::QEntity(root.data()); + + for (int i = 0; i < 100; ++i) { + Qt3DCore::QEntity *child = new Qt3DCore::QEntity(); + child->setParent(subTreeRoot); + } + + // THEN + QCoreApplication::processEvents(); + QCOMPARE(root->children().count(), 1); + QCOMPARE(subTreeRoot->children().count(), 100); + QCOMPARE(spy.events.size(), 102); // 1 subTreeRoot creation change, 100 child creation, 1 child added (subTree to root) + + // Ensure first event is subTreeRoot + const Qt3DCore::QNodeCreatedChangeBasePtr firstEvent = spy.events.takeFirst().change().dynamicCast<Qt3DCore::QNodeCreatedChangeBase>(); + QVERIFY(!firstEvent.isNull()); + QCOMPARE(firstEvent->subjectId(), subTreeRoot->id()); + QCOMPARE(firstEvent->parentId(), root->id()); + + const Qt3DCore::QPropertyNodeAddedChangePtr lastEvent = spy.events.takeLast().change().dynamicCast<Qt3DCore::QPropertyNodeAddedChange>(); + QVERIFY(!lastEvent.isNull()); + QCOMPARE(lastEvent->subjectId(), root->id()); + QCOMPARE(lastEvent->propertyName(), "children"); + QCOMPARE(lastEvent->addedNodeId(), subTreeRoot->id()); +} + void tst_Nodes::appendingParentlessComponentToEntity() { // GIVEN @@ -798,6 +855,8 @@ void tst_Nodes::appendingParentlessComponentToEntity() { QScopedPointer<MyQEntity> entity(new MyQEntity()); entity->setArbiterAndScene(&entitySpy); + entity->setSimulateBackendCreated(true); + MyQComponent *comp = new MyQComponent(); comp->setArbiter(&componentSpy); @@ -816,7 +875,7 @@ void tst_Nodes::appendingParentlessComponentToEntity() QVERIFY(comp->parentNode() == entity.data()); QCOMPARE(entitySpy.events.size(), 1); QVERIFY(entitySpy.events.first().wasLocked()); - QCOMPARE(componentSpy.events.size(), 2); // first event is parent being set + QCOMPARE(componentSpy.events.size(), 1); // Note: since QEntity has no scene in this test case, we only have the // ComponentAdded event In theory we should also get the NodeCreated event diff --git a/tests/auto/core/qchangearbiter/tst_qchangearbiter.cpp b/tests/auto/core/qchangearbiter/tst_qchangearbiter.cpp index 061de06fa..faccb6d34 100644 --- a/tests/auto/core/qchangearbiter/tst_qchangearbiter.cpp +++ b/tests/auto/core/qchangearbiter/tst_qchangearbiter.cpp @@ -442,6 +442,7 @@ void tst_QChangeArbiter::registerSceneObserver() tst_Node *root = new tst_Node(); Qt3DCore::QNode *child = new tst_Node(); Qt3DCore::QNodePrivate::get(root)->setScene(scene.data()); + Qt3DCore::QNodePrivate::get(root)->m_hasBackendNode = true; scene->addObservable(root); QList<tst_SimpleObserver *> observers; @@ -573,6 +574,7 @@ void tst_QChangeArbiter::unregisterSceneObservers() tst_Node *root = new tst_Node(); Qt3DCore::QNode *child = new tst_Node(); Qt3DCore::QNodePrivate::get(root)->setScene(scene.data()); + Qt3DCore::QNodePrivate::get(root)->m_hasBackendNode = true; scene->addObservable(root); QList<tst_SimpleObserver *> observers; @@ -796,6 +798,7 @@ void tst_QChangeArbiter::distributePropertyChanges() // WHEN PropertyTestNode *root = new PropertyTestNode(); Qt3DCore::QNodePrivate::get(root)->setScene(scene.data()); + Qt3DCore::QNodePrivate::get(root)->m_hasBackendNode = true; scene->addObservable(root); tst_SimpleObserver *rootObserver = new tst_SimpleObserver(); @@ -890,6 +893,7 @@ void tst_QChangeArbiter::distributeBackendChanges() // WHEN tst_Node *root = new tst_Node(); Qt3DCore::QNodePrivate::get(root)->setScene(scene.data()); + Qt3DCore::QNodePrivate::get(root)->m_hasBackendNode = true; scene->addObservable(root); tst_BackendObserverObservable *backenObserverObservable = new tst_BackendObserverObservable(); diff --git a/tests/auto/core/qscene/tst_qscene.cpp b/tests/auto/core/qscene/tst_qscene.cpp index f4b04362d..78f17e8fa 100644 --- a/tests/auto/core/qscene/tst_qscene.cpp +++ b/tests/auto/core/qscene/tst_qscene.cpp @@ -297,6 +297,8 @@ void tst_QScene::deleteChildNode() Qt3DCore::QNode *root2 = new tst_Node(); Qt3DCore::QNodePrivate::get(root1)->setScene(scene); Qt3DCore::QNodePrivate::get(root2)->setScene(scene); + Qt3DCore::QNodePrivate::get(root1)->m_hasBackendNode = true; + Qt3DCore::QNodePrivate::get(root2)->m_hasBackendNode = true; // WHEN scene->addObservable(root1); @@ -358,6 +360,7 @@ void tst_QScene::removeChildNode() Qt3DCore::QNode *root = new tst_Node; Qt3DCore::QNodePrivate::get(root)->setScene(scene); + Qt3DCore::QNodePrivate::get(root)->m_hasBackendNode = true; scene->addObservable(root); // WHEN |