From 1cdf47f02e2b32da4ac82561f625716642d2933a Mon Sep 17 00:00:00 2001 From: Jim Albamont Date: Tue, 17 Sep 2019 17:04:43 -0700 Subject: Process pending nodes needing _q_postConstructorInit at start of frame NodePostConstructorInit::processNodes and QAspectManager::processFrame are both triggered by the event loop which means the frame can happen before the processNodes call. We want to ensure processNodes is called first so those pending nodes can be created during the processFrame call otherwise they will get deferred until the next frame. Created a test to show this and removed the now unnecessary double calls to processEvents in several other tests. Change-Id: I7a3f7b34be2858b4acdb9275804b458f9366ec67 Reviewed-by: Paul Lemire --- src/core/aspects/qaspectengine.cpp | 1 + src/core/aspects/qaspectmanager.cpp | 9 ++++++ src/core/aspects/qaspectmanager_p.h | 4 +++ src/core/nodes/qnode_p.h | 2 +- tests/auto/core/nodes/tst_nodes.cpp | 61 ++++++++++++++++++++++++++++--------- 5 files changed, 62 insertions(+), 15 deletions(-) diff --git a/src/core/aspects/qaspectengine.cpp b/src/core/aspects/qaspectengine.cpp index 6d1a8ca30..426741a61 100644 --- a/src/core/aspects/qaspectengine.cpp +++ b/src/core/aspects/qaspectengine.cpp @@ -275,6 +275,7 @@ void QAspectEnginePrivate::initialize() arbiter->setPostman(m_postman); arbiter->setScene(m_scene); m_initialized = true; + m_aspectManager->setPostConstructorInit(m_scene->postConstructorInit()); #if QT_CONFIG(qt3d_profile_jobs) m_commandDebugger->setAspectEngine(q_func()); m_commandDebugger->initialize(); diff --git a/src/core/aspects/qaspectmanager.cpp b/src/core/aspects/qaspectmanager.cpp index 6bca77a9e..a33f771e9 100644 --- a/src/core/aspects/qaspectmanager.cpp +++ b/src/core/aspects/qaspectmanager.cpp @@ -374,6 +374,11 @@ QServiceLocator *QAspectManager::serviceLocator() const return m_serviceLocator.data(); } +void QAspectManager::setPostConstructorInit(NodePostConstructorInit *postConstructorInit) +{ + m_postConstructorInit = postConstructorInit; +} + /*! \internal \brief Drives the Qt3D simulation loop in the main thread @@ -429,6 +434,10 @@ void QAspectManager::processFrame() changeArbiterStats.startTime = QThreadPooler::m_jobsStatTimer.nsecsElapsed(); #endif + // Tell the NodePostConstructorInit to process any pending nodes which will add them to our list of + // tree changes + m_postConstructorInit->processNodes(); + // Add and Remove Nodes const QVector nodeTreeChanges = std::move(m_nodeTreeChanges); for (const NodeTreeChange &change : nodeTreeChanges) { diff --git a/src/core/aspects/qaspectmanager_p.h b/src/core/aspects/qaspectmanager_p.h index b39ad1f89..38ddbc55d 100644 --- a/src/core/aspects/qaspectmanager_p.h +++ b/src/core/aspects/qaspectmanager_p.h @@ -74,6 +74,7 @@ class QChangeArbiter; class QAbstractAspect; class QAbstractAspectJobManager; class QServiceLocator; +class NodePostConstructorInit; struct NodeTreeChange; class Q_3DCORE_PRIVATE_EXPORT QAspectManager : public QObject @@ -105,6 +106,7 @@ public: QAbstractAspectJobManager *jobManager() const; QChangeArbiter *changeArbiter() const; QServiceLocator *serviceLocator() const; + void setPostConstructorInit(NodePostConstructorInit *postConstructorInit); private: bool event(QEvent *event) override; @@ -121,6 +123,8 @@ private: bool m_simulationLoopRunning; QAspectEngine::RunMode m_driveMode; QVector m_nodeTreeChanges; + NodePostConstructorInit* m_postConstructorInit; + }; } // namespace Qt3DCore diff --git a/src/core/nodes/qnode_p.h b/src/core/nodes/qnode_p.h index 491059ff9..d8310731c 100644 --- a/src/core/nodes/qnode_p.h +++ b/src/core/nodes/qnode_p.h @@ -194,7 +194,7 @@ public: void removeNode(QNode *node); void addNode(QNode *node); -private Q_SLOTS: +public Q_SLOTS: void processNodes(); private: diff --git a/tests/auto/core/nodes/tst_nodes.cpp b/tests/auto/core/nodes/tst_nodes.cpp index 76c66604c..c5369ab3e 100644 --- a/tests/auto/core/nodes/tst_nodes.cpp +++ b/tests/auto/core/nodes/tst_nodes.cpp @@ -79,6 +79,7 @@ private slots: void checkParentChangeFromExistingBackendParentToNewlyCreatedParent(); void checkBackendNodesCreatedFromTopDown(); //QTBUG-74106 void checkBackendNodesCreatedFromTopDownWithReparenting(); + void checkAllBackendCreationDoneInSingleFrame(); void removingSingleChildNodeFromNode(); void removingMultipleChildNodesFromNode(); @@ -929,8 +930,6 @@ void tst_Nodes::checkParentChangeFromExistingBackendParentToNewlyCreatedParent() MyQNode *child(new MyQNode(root.data())); MyQNode *child2(new MyQNode(root.data())); - // processEvents 2x because the postConstructorInit happens after the frame advance. - QCoreApplication::processEvents(); QCoreApplication::processEvents(); // Due to the way we create root, it has a backend @@ -961,8 +960,6 @@ void tst_Nodes::checkParentChangeFromExistingBackendParentToNewlyCreatedParent() QVERIFY(Qt3DCore::QNodePrivate::get(child)->scene() != nullptr); // WHEN - // processEvents 2x because the postConstructorInit happens after the frame advance. - QCoreApplication::processEvents(); QCoreApplication::processEvents(); // THEN @@ -1039,8 +1036,6 @@ void tst_Nodes::checkParentChangeFromExistingBackendParentToNewlyCreatedParent() QVERIFY(Qt3DCore::QNodePrivate::get(child)->scene() != nullptr); // WHEN - // processEvents 2x because the postConstructorInit happens after the frame advance. - QCoreApplication::processEvents(); QCoreApplication::processEvents(); // THEN @@ -1099,8 +1094,6 @@ void tst_Nodes::checkBackendNodesCreatedFromTopDown() QCOMPARE(spy.events.count(), 0); // WHEN - create the backend nodes - // processEvents 2x because the postConstructorInit happens after the frame advance. - QCoreApplication::processEvents(); QCoreApplication::processEvents(); // THEN @@ -1159,8 +1152,6 @@ void tst_Nodes::checkBackendNodesCreatedFromTopDownWithReparenting() auto parent1 = new MyQNode(parentWithBackend.data()); node1->setParent(parent1); - // processEvents 2x because the postConstructorInit happens after the frame advance. - QCoreApplication::processEvents(); QCoreApplication::processEvents(); // THEN @@ -1175,8 +1166,6 @@ void tst_Nodes::checkBackendNodesCreatedFromTopDownWithReparenting() node2->setParent(parent2); parent2->setParent(parentWithBackend.get()); - // processEvents 2x because the postConstructorInit happens after the frame advance. - QCoreApplication::processEvents(); QCoreApplication::processEvents(); // THEN @@ -1221,6 +1210,52 @@ void tst_Nodes::removingSingleChildNodeFromNode() QCOMPARE(removalEvent->metaObject(), child->metaObject()); } +void tst_Nodes::checkAllBackendCreationDoneInSingleFrame() +{ + // GIVEN + ObserverSpy spy; + Qt3DCore::QAspectEngine engine; + auto aspect = new TestAspect; + engine.registerAspect(aspect); + + QScopedPointer root(new MyQEntity()); + root->setArbiterAndEngine(&spy, &engine); + + QCoreApplication::processEvents(); + + // THEN + // Due to the way we create root, it has a backend + QVERIFY(Qt3DCore::QNodePrivate::get(root.data())->m_hasBackendNode == true); + QCOMPARE(aspect->events.count(), 1); + QCOMPARE(aspect->events[0].type, TestAspect::Creation); + QCOMPARE(aspect->events[0].nodeId, root->id()); + + // WHEN -> create 2 children: + // 1. a child with parent with backend node + // 2. a child with no parent that is then reparented to a parent with backend node + aspect->clearNodes(); + auto child1 = new MyQNode(root.data()); + auto child2 = new MyQNode; + child2->setParent(root.data()); + + // THEN - reparented child should have a backend node, but other child should + // still be waiting + QCOMPARE(child1->parent(), root.data()); + QCOMPARE(child2->parent(), root.data()); + QVERIFY(Qt3DCore::QNodePrivate::get(child1)->m_hasBackendNode == false); + QVERIFY(Qt3DCore::QNodePrivate::get(child2)->m_hasBackendNode == true); + + // WHEN + QCoreApplication::processEvents(); + + // THEN - both children have their backend nodes actually created. + QCOMPARE(aspect->events.count(), 2); + QCOMPARE(aspect->events[0].type, TestAspect::Creation); + QCOMPARE(aspect->events[0].nodeId, child2->id()); + QCOMPARE(aspect->events[1].type, TestAspect::Creation); + QCOMPARE(aspect->events[1].nodeId, child1->id()); +} + void tst_Nodes::removingMultipleChildNodesFromNode() { // GIVEN @@ -1329,8 +1364,6 @@ void tst_Nodes::checkConstructionSetParentMix() } // THEN - // processEvents 2x because the postConstructorInit happens after the frame advance. - QCoreApplication::processEvents(); QCoreApplication::processEvents(); QCOMPARE(root->children().count(), 1); QCOMPARE(subTreeRoot->children().count(), 100); -- cgit v1.2.3