From 53efe4ea5a26e67b5a37a7a79744872af1ec34bb Mon Sep 17 00:00:00 2001 From: Mike Krus Date: Tue, 17 Sep 2019 15:47:59 +0100 Subject: [PATCH] Remove use of node/components added/removed messages Introduce mechanism to notify backend nodes of changes in relationship between nodes. If a component is added or removed from an entity, or if a node is added or removed from a property of another node, then just rely on the node being marked as dirty when syncing takes place. For nodes that do not support syncing, messages are delivered as before but allocated on the stack rather than the heap. Change-Id: I06affac77e42a9998d9c7f44e231c7724c52b320 Reviewed-by: Mike Krus --- src/core/aspects/qabstractaspect.cpp | 108 +++++++++++++ src/core/aspects/qabstractaspect.h | 2 + src/core/aspects/qabstractaspect_p.h | 1 + src/core/aspects/qaspectmanager.cpp | 7 + src/core/changes/qscenechange.h | 9 ++ src/core/nodes/qbackendnode.cpp | 20 +++ src/core/nodes/qbackendnode_p.h | 7 + src/core/nodes/qcomponent.cpp | 6 - src/core/nodes/qentity.cpp | 19 +-- src/core/nodes/qnode.cpp | 8 + src/core/nodes/qnode_p.h | 1 + src/core/qchangearbiter.cpp | 15 ++ src/core/qchangearbiter_p.h | 4 + src/render/backend/backendnode.cpp | 6 + src/render/backend/backendnode_p.h | 7 + src/render/backend/entity.cpp | 65 ++++---- src/render/backend/entity_p.h | 5 +- src/render/backend/entity_p_p.h | 87 +++++++++++ src/render/backend/render-backend.pri | 1 + tests/auto/core/common/testpostmanarbiter.cpp | 7 + tests/auto/core/common/testpostmanarbiter.h | 2 + tests/auto/core/nodes/tst_nodes.cpp | 142 ++++++------------ tests/auto/render/entity/tst_entity.cpp | 31 ++-- 23 files changed, 380 insertions(+), 180 deletions(-) create mode 100644 src/render/backend/entity_p_p.h diff --git a/src/core/aspects/qabstractaspect.cpp b/src/core/aspects/qabstractaspect.cpp index 866a0ce25..05afdd293 100644 --- a/src/core/aspects/qabstractaspect.cpp +++ b/src/core/aspects/qabstractaspect.cpp @@ -43,8 +43,13 @@ #include #include +#include #include #include +#include +#include +#include +#include #include #include @@ -209,6 +214,12 @@ void QAbstractAspect::syncDirtyFrontEndNodes(const QVector &nodes) d->syncDirtyFrontEndNodes(nodes); } +void QAbstractAspect::syncDirtyFrontEndSubNodes(const QVector &nodes) +{ + Q_D(QAbstractAspect); + d->syncDirtyFrontEndSubNodes(nodes); +} + QAbstractAspectPrivate::BackendNodeMapperAndInfo QAbstractAspectPrivate::mapperForNode(const QMetaObject *metaObj) const { Q_ASSERT(metaObj); @@ -243,6 +254,103 @@ void QAbstractAspectPrivate::syncDirtyFrontEndNodes(const QVector &node } } +void QAbstractAspectPrivate::syncDirtyFrontEndSubNodes(const QVector &nodes) +{ + for (const auto &nodeChange: qAsConst(nodes)) { + auto getBackend = [this](QNode *node) -> std::tuple { + const QMetaObject *metaObj = QNodePrivate::get(node)->m_typeInfo; + const BackendNodeMapperAndInfo backendNodeMapperInfo = mapperForNode(metaObj); + const QBackendNodeMapperPtr backendNodeMapper = backendNodeMapperInfo.first; + + if (!backendNodeMapper) + return {}; + + QBackendNode *backend = backendNodeMapper->get(node->id()); + if (!backend) + return {}; + + const bool supportsSyncing = backendNodeMapperInfo.second & SupportsSyncing; + + return std::tuple(backend, supportsSyncing); + }; + + auto nodeInfo = getBackend(nodeChange.node); + if (!std::get<0>(nodeInfo)) + continue; + + auto subNodeInfo = getBackend(nodeChange.subNode); + if (!std::get<0>(subNodeInfo)) + continue; + + switch (nodeChange.change) { + case PropertyValueAdded: { + if (std::get<1>(nodeInfo)) + break; // do nothing as the node will be dirty anyway + + QPropertyValueAddedChange change(nodeChange.node->id()); + change.setPropertyName(nodeChange.property); + change.setAddedValue(QVariant::fromValue(nodeChange.subNode->id())); + QPropertyValueAddedChangePtr pChange(&change, [](QPropertyValueAddedChange *) { }); + std::get<0>(nodeInfo)->sceneChangeEvent(pChange); + } + break; + case PropertyValueRemoved: { + if (std::get<1>(nodeInfo)) + break; // do nothing as the node will be dirty anyway + + QPropertyValueRemovedChange change(nodeChange.node->id()); + change.setPropertyName(nodeChange.property); + change.setRemovedValue(QVariant::fromValue(nodeChange.subNode->id())); + QPropertyValueRemovedChangePtr pChange(&change, [](QPropertyValueRemovedChange *) { }); + std::get<0>(nodeInfo)->sceneChangeEvent(pChange); + } + break; + case ComponentAdded: { + // let the entity know it has a new component + if (std::get<1>(nodeInfo)) { + QBackendNodePrivate::get(std::get<0>(nodeInfo))->componentAdded(nodeChange.subNode); + } else { + QComponentAddedChange change(qobject_cast(nodeChange.subNode), qobject_cast(nodeChange.node)); + QComponentAddedChangePtr pChange(&change, [](QComponentAddedChange *) { }); + std::get<0>(nodeInfo)->sceneChangeEvent(pChange); + } + + // let the component know it was added to an entity + if (std::get<1>(subNodeInfo)) { + QBackendNodePrivate::get(std::get<0>(subNodeInfo))->addedToEntity(nodeChange.node); + } else { + QComponentAddedChange change(qobject_cast(nodeChange.subNode), qobject_cast(nodeChange.node)); + QComponentAddedChangePtr pChange(&change, [](QComponentAddedChange *) { }); + std::get<0>(subNodeInfo)->sceneChangeEvent(pChange); + } + } + break; + case ComponentRemoved: { + // let the entity know a component was removed + if (std::get<1>(nodeInfo)) { + QBackendNodePrivate::get(std::get<0>(nodeInfo))->componentRemoved(nodeChange.subNode); + } else { + QComponentRemovedChange change(qobject_cast(nodeChange.subNode), qobject_cast(nodeChange.node)); + QComponentRemovedChangePtr pChange(&change, [](QComponentRemovedChange *) { }); + std::get<0>(nodeInfo)->sceneChangeEvent(pChange); + } + + // let the component know it was removed from an entity + if (std::get<1>(subNodeInfo)) { + QBackendNodePrivate::get(std::get<0>(subNodeInfo))->removedFromEntity(nodeChange.node); + } else { + QComponentRemovedChange change(qobject_cast(nodeChange.node), qobject_cast(nodeChange.subNode)); + QComponentRemovedChangePtr pChange(&change, [](QComponentRemovedChange *) { }); + std::get<0>(nodeInfo)->sceneChangeEvent(pChange); + } + } + break; + default: + break; + } + } +} + void QAbstractAspectPrivate::syncDirtyFrontEndNode(QNode *node, QBackendNode *backend, bool firstTime) const { Q_ASSERT(false); // overload in derived class diff --git a/src/core/aspects/qabstractaspect.h b/src/core/aspects/qabstractaspect.h index a9f4f03fc..8c191c1d8 100644 --- a/src/core/aspects/qabstractaspect.h +++ b/src/core/aspects/qabstractaspect.h @@ -42,6 +42,7 @@ #include #include +#include #include #include @@ -86,6 +87,7 @@ protected: private: void syncDirtyFrontEndNodes(const QVector &nodes); + void syncDirtyFrontEndSubNodes(const QVector &nodes); void registerBackendType(const QMetaObject &obj, const QBackendNodeMapperPtr &functor, bool supportsSyncing); virtual QVariant executeCommand(const QStringList &args); diff --git a/src/core/aspects/qabstractaspect_p.h b/src/core/aspects/qabstractaspect_p.h index 8bd26cf00..670302cfa 100644 --- a/src/core/aspects/qabstractaspect_p.h +++ b/src/core/aspects/qabstractaspect_p.h @@ -130,6 +130,7 @@ public: QBackendNode *createBackendNode(const NodeTreeChange &change) const; void clearBackendNode(const NodeTreeChange &change) const; void syncDirtyFrontEndNodes(const QVector &nodes); + void syncDirtyFrontEndSubNodes(const QVector &nodes); virtual void syncDirtyFrontEndNode(QNode *node, QBackendNode *backend, bool firstTime) const; void sendPropertyMessages(QNode *node, QBackendNode *backend) const; diff --git a/src/core/aspects/qaspectmanager.cpp b/src/core/aspects/qaspectmanager.cpp index 99f601cc9..71e05b34f 100644 --- a/src/core/aspects/qaspectmanager.cpp +++ b/src/core/aspects/qaspectmanager.cpp @@ -475,11 +475,18 @@ void QAspectManager::processFrame() } } + // Sync node / subnode relationship changes + const auto dirtySubNodes = m_changeArbiter->takeDirtyFrontEndSubNodes(); + if (dirtySubNodes.size()) + for (QAbstractAspect *aspect : qAsConst(m_aspects)) + aspect->syncDirtyFrontEndSubNodes(dirtySubNodes); + // Sync property updates const auto dirtyFrontEndNodes = m_changeArbiter->takeDirtyFrontEndNodes(); if (dirtyFrontEndNodes.size()) for (QAbstractAspect *aspect : qAsConst(m_aspects)) aspect->syncDirtyFrontEndNodes(dirtyFrontEndNodes); + // TO DO: Having this done in the main thread actually means aspects could just // as simply read info out of the Frontend classes without risk of introducing // races. This could therefore be removed for Qt 6. diff --git a/src/core/changes/qscenechange.h b/src/core/changes/qscenechange.h index 7d88d334f..e8c0ea748 100644 --- a/src/core/changes/qscenechange.h +++ b/src/core/changes/qscenechange.h @@ -63,6 +63,15 @@ enum ChangeFlag { Q_DECLARE_FLAGS(ChangeFlags, ChangeFlag) Q_DECLARE_OPERATORS_FOR_FLAGS(ChangeFlags) +class QNode; +//! internal +struct NodeRelationshipChange { + QNode *node; + QNode *subNode; + ChangeFlag change; + const char *property; +}; + class QSceneChangePrivate; class Q_3DCORESHARED_EXPORT QSceneChange diff --git a/src/core/nodes/qbackendnode.cpp b/src/core/nodes/qbackendnode.cpp index 3eb1cd9f7..e5f93e96f 100644 --- a/src/core/nodes/qbackendnode.cpp +++ b/src/core/nodes/qbackendnode.cpp @@ -95,6 +95,26 @@ QBackendNodePrivate *QBackendNodePrivate::get(QBackendNode *n) return n->d_func(); } +void QBackendNodePrivate::addedToEntity(QNode *frontend) +{ + Q_UNUSED(frontend) +} + +void QBackendNodePrivate::removedFromEntity(QNode *frontend) +{ + Q_UNUSED(frontend) +} + +void QBackendNodePrivate::componentAdded(QNode *frontend) +{ + Q_UNUSED(frontend) +} + +void QBackendNodePrivate::componentRemoved(QNode *frontend) +{ + Q_UNUSED(frontend) +} + /*! * \class Qt3DCore::QBackendNodeMapper * \inheaderfile Qt3DCore/QBackendNodeMapper diff --git a/src/core/nodes/qbackendnode_p.h b/src/core/nodes/qbackendnode_p.h index dde86fa48..260eef087 100644 --- a/src/core/nodes/qbackendnode_p.h +++ b/src/core/nodes/qbackendnode_p.h @@ -63,6 +63,8 @@ QT_BEGIN_NAMESPACE namespace Qt3DCore { +class QNode; + class Q_3DCORE_PRIVATE_EXPORT QBackendNodePrivate : public QObserverInterface , public QObservableInterface @@ -85,6 +87,11 @@ public: QNodeId m_peerId; bool m_enabled; + virtual void addedToEntity(QNode *frontend); + virtual void removedFromEntity(QNode *frontend); + virtual void componentAdded(QNode *frontend); + virtual void componentRemoved(QNode *frontend); + private: Q_DISABLE_COPY(QBackendNodePrivate) }; diff --git a/src/core/nodes/qcomponent.cpp b/src/core/nodes/qcomponent.cpp index 773d2df88..79bba229d 100644 --- a/src/core/nodes/qcomponent.cpp +++ b/src/core/nodes/qcomponent.cpp @@ -40,8 +40,6 @@ #include "qcomponent.h" #include "qcomponent_p.h" -#include -#include #include #include @@ -72,8 +70,6 @@ void QComponentPrivate::addEntity(QEntity *entity) m_scene->addEntityForComponent(m_id, entity->id()); } - const auto componentAddedChange = QComponentAddedChangePtr::create(q, entity); // TODOSYNC notify backend directly - notifyObservers(componentAddedChange); Q_EMIT q->addedToEntity(entity); } @@ -85,8 +81,6 @@ void QComponentPrivate::removeEntity(QEntity *entity) m_entities.removeAll(entity); - const auto componentRemovedChange = QComponentRemovedChangePtr::create(q, entity); // TODOSYNC notify backend directly - notifyObservers(componentRemovedChange); Q_EMIT q->removedFromEntity(entity); } diff --git a/src/core/nodes/qentity.cpp b/src/core/nodes/qentity.cpp index 58b5ef33e..024991387 100644 --- a/src/core/nodes/qentity.cpp +++ b/src/core/nodes/qentity.cpp @@ -41,8 +41,6 @@ #include "qentity_p.h" #include -#include -#include #include #include #include @@ -101,13 +99,8 @@ void QEntityPrivate::removeDestroyedComponent(QComponent *comp) Q_CHECK_PTR(comp); qCDebug(Nodes) << Q_FUNC_INFO << comp; - Q_Q(QEntity); - - if (m_changeArbiter) { - const auto componentRemovedChange = QComponentRemovedChangePtr::create(q, comp); // TODOSYNC notify backend directly - notifyObservers(componentRemovedChange); - } + updateNode(comp, nullptr, ComponentRemoved); m_components.removeOne(comp); // Remove bookkeeping connection @@ -186,10 +179,7 @@ void QEntity::addComponent(QComponent *comp) // Ensures proper bookkeeping d->registerPrivateDestructionHelper(comp, &QEntityPrivate::removeDestroyedComponent); - if (d->m_changeArbiter) { - const auto componentAddedChange = QComponentAddedChangePtr::create(this, comp); // TODOSYNC notify backend directly - d->notifyObservers(componentAddedChange); - } + d->updateNode(comp, nullptr, ComponentAdded); static_cast(QComponentPrivate::get(comp))->addEntity(this); } @@ -204,10 +194,7 @@ void QEntity::removeComponent(QComponent *comp) static_cast(QComponentPrivate::get(comp))->removeEntity(this); - if (d->m_changeArbiter) { - const auto componentRemovedChange = QComponentRemovedChangePtr::create(this, comp); - d->notifyObservers(componentRemovedChange); - } + d->updateNode(comp, nullptr, ComponentRemoved); d->m_components.removeOne(comp); diff --git a/src/core/nodes/qnode.cpp b/src/core/nodes/qnode.cpp index 4dda92ec6..cfe83f4db 100644 --- a/src/core/nodes/qnode.cpp +++ b/src/core/nodes/qnode.cpp @@ -680,6 +680,14 @@ void QNodePrivate::update() } } +void QNodePrivate::updateNode(QNode *node, const char *property, ChangeFlag change) +{ + if (m_changeArbiter) { + Q_Q(QNode); + m_changeArbiter->addDirtyFrontEndNode(q, node, property, change); + } +} + /*! \internal */ diff --git a/src/core/nodes/qnode_p.h b/src/core/nodes/qnode_p.h index 839751a5e..a7a300a5e 100644 --- a/src/core/nodes/qnode_p.h +++ b/src/core/nodes/qnode_p.h @@ -90,6 +90,7 @@ public: void updatePropertyTrackMode(); void update(); + void updateNode(QNode *node, const char* property, ChangeFlag change); Q_DECLARE_PUBLIC(QNode) diff --git a/src/core/qchangearbiter.cpp b/src/core/qchangearbiter.cpp index 60cdc3bd2..6c118341a 100644 --- a/src/core/qchangearbiter.cpp +++ b/src/core/qchangearbiter.cpp @@ -253,9 +253,19 @@ void QChangeArbiter::addDirtyFrontEndNode(QNode *node) m_dirtyFrontEndNodes += node; } +void QChangeArbiter::addDirtyFrontEndNode(QNode *node, QNode *subNode, const char *property, ChangeFlag change) +{ + if (!m_dirtyFrontEndNodes.contains(node)) + m_dirtyFrontEndNodes += node; + m_dirtySubNodeChanges.push_back({node, subNode, change, property}); +} + void QChangeArbiter::removeDirtyFrontEndNode(QNode *node) { m_dirtyFrontEndNodes.removeOne(node); + m_dirtySubNodeChanges.erase(std::remove_if(m_dirtySubNodeChanges.begin(), m_dirtySubNodeChanges.end(), [node](const NodeRelationshipChange &elt) { + return elt.node == node || elt.subNode == node; + }), m_dirtySubNodeChanges.end()); } QVector QChangeArbiter::takeDirtyFrontEndNodes() @@ -263,6 +273,11 @@ QVector QChangeArbiter::takeDirtyFrontEndNodes() return std::move(m_dirtyFrontEndNodes); } +QVector QChangeArbiter::takeDirtyFrontEndSubNodes() +{ + return std::move(m_dirtySubNodeChanges); +} + // Either we have the postman or we could make the QChangeArbiter agnostic to the postman // but that would require adding it to every QObserverList in m_aspectObservations. void QChangeArbiter::setPostman(QAbstractPostman *postman) diff --git a/src/core/qchangearbiter_p.h b/src/core/qchangearbiter_p.h index 0a6196756..f31480685 100644 --- a/src/core/qchangearbiter_p.h +++ b/src/core/qchangearbiter_p.h @@ -83,6 +83,7 @@ public: virtual QAbstractPostman *postman() const = 0; virtual void addDirtyFrontEndNode(QNode *node) = 0; virtual void removeDirtyFrontEndNode(QNode *node) = 0; + virtual void addDirtyFrontEndNode(QNode *node, QNode *subNode, const char *property, ChangeFlag change) = 0; }; class Q_3DCORE_PRIVATE_EXPORT QChangeArbiter final @@ -109,8 +110,10 @@ public: void sceneChangeEventWithLock(const QSceneChangeList &e) override; // QLockableObserverInterface impl void addDirtyFrontEndNode(QNode *node) override; + void addDirtyFrontEndNode(QNode *node, QNode *subNode, const char *property, ChangeFlag change) override; void removeDirtyFrontEndNode(QNode *node) override; QVector takeDirtyFrontEndNodes(); + QVector takeDirtyFrontEndSubNodes(); void setPostman(Qt3DCore::QAbstractPostman *postman); void setScene(Qt3DCore::QScene *scene); @@ -159,6 +162,7 @@ private: QScene *m_scene; QVector m_dirtyFrontEndNodes; + QVector m_dirtySubNodeChanges; }; } // namespace Qt3DCore diff --git a/src/render/backend/backendnode.cpp b/src/render/backend/backendnode.cpp index 188e017e6..642f6e44d 100644 --- a/src/render/backend/backendnode.cpp +++ b/src/render/backend/backendnode.cpp @@ -56,6 +56,12 @@ BackendNode::BackendNode(Mode mode) { } +BackendNode::BackendNode(Qt3DCore::QBackendNodePrivate &dd) + : QBackendNode(dd) + , m_renderer(nullptr) +{ +} + BackendNode::~BackendNode() { } diff --git a/src/render/backend/backendnode_p.h b/src/render/backend/backendnode_p.h index 167cb87f2..37f8305af 100644 --- a/src/render/backend/backendnode_p.h +++ b/src/render/backend/backendnode_p.h @@ -58,6 +58,12 @@ QT_BEGIN_NAMESPACE +namespace Qt3DCore { + +class QBackendNodePrivate; + +} + namespace Qt3DRender { namespace Render { @@ -78,6 +84,7 @@ public: QSharedPointer resourceAccessor(); protected: + explicit BackendNode(Qt3DCore::QBackendNodePrivate &dd); void markDirty(AbstractRenderer::BackendNodeDirtySet changes); AbstractRenderer *m_renderer; }; diff --git a/src/render/backend/entity.cpp b/src/render/backend/entity.cpp index a5d7ebe7e..682dc000e 100644 --- a/src/render/backend/entity.cpp +++ b/src/render/backend/entity.cpp @@ -38,6 +38,7 @@ ****************************************************************************/ #include "entity_p.h" +#include "entity_p_p.h" #include #include #include @@ -59,8 +60,6 @@ #include #include -#include -#include #include #include #include @@ -76,8 +75,33 @@ using namespace Qt3DCore; namespace Qt3DRender { namespace Render { + +EntityPrivate::EntityPrivate() + : Qt3DCore::QBackendNodePrivate(Entity::ReadOnly) +{ +} + +EntityPrivate *EntityPrivate::get(Entity *node) +{ + return node->d_func(); +} + +void EntityPrivate::componentAdded(Qt3DCore::QNode *frontend) +{ + Q_Q(Entity); + const auto componentIdAndType = QNodeIdTypePair(frontend->id(), QNodePrivate::findStaticMetaObject(frontend->metaObject())); + q->addComponent(componentIdAndType); +} + +void EntityPrivate::componentRemoved(Qt3DCore::QNode *frontend) +{ + Q_Q(Entity); + q->removeComponent(frontend->id()); +} + + Entity::Entity() - : BackendNode() + : BackendNode(*new EntityPrivate) , m_nodeManagers(nullptr) , m_boundingDirty(false) , m_treeEnabled(true) @@ -159,31 +183,6 @@ void Entity::setHandle(HEntity handle) m_handle = handle; } -void Entity::sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e) -{ - switch (e->type()) { - - case ComponentAdded: { - QComponentAddedChangePtr change = qSharedPointerCast(e); - const auto componentIdAndType = QNodeIdTypePair(change->componentId(), change->componentMetaObject()); - addComponent(componentIdAndType); - qCDebug(Render::RenderNodes) << Q_FUNC_INFO << "Component Added. Id =" << change->componentId(); - break; - } - - case ComponentRemoved: { - QComponentRemovedChangePtr change = qSharedPointerCast(e); - removeComponent(change->componentId()); - qCDebug(Render::RenderNodes) << Q_FUNC_INFO << "Component Removed. Id =" << change->componentId(); - break; - } - - default: - break; - } - BackendNode::sceneChangeEvent(e); -} - void Entity::syncFromFrontEnd(const QNode *frontEnd, bool firstTime) { const Qt3DCore::QEntity *node = qobject_cast(frontEnd); @@ -264,16 +263,6 @@ void Entity::removeFromParentChildHandles() p->removeChildHandle(m_handle); } -void Entity::appendChildHandle(HEntity childHandle) -{ - if (!m_childrenHandles.contains(childHandle)) { - m_childrenHandles.append(childHandle); - Entity *child = m_nodeManagers->renderNodesManager()->data(childHandle); - if (child != nullptr) - child->m_parentHandle = m_handle; - } -} - QVector Entity::children() const { QVector childrenVector; diff --git a/src/render/backend/entity_p.h b/src/render/backend/entity_p.h index 403f5568c..d13d96784 100644 --- a/src/render/backend/entity_p.h +++ b/src/render/backend/entity_p.h @@ -79,6 +79,7 @@ namespace Render { class Sphere; class Renderer; class NodeManagers; +class EntityPrivate; class Q_AUTOTEST_EXPORT Entity : public BackendNode { @@ -89,7 +90,6 @@ public: void setParentHandle(HEntity parentHandle); void setNodeManagers(NodeManagers *manager); - void sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e) override; void syncFromFrontEnd(const Qt3DCore::QNode *frontEnd, bool firstTime) override; void dump() const; @@ -176,6 +176,9 @@ public: return containsComponentsOfType() && containsComponentsOfType(); } +protected: + Q_DECLARE_PRIVATE(Entity) + private: NodeManagers *m_nodeManagers; HEntity m_handle; diff --git a/src/render/backend/entity_p_p.h b/src/render/backend/entity_p_p.h new file mode 100644 index 000000000..4ac6ab978 --- /dev/null +++ b/src/render/backend/entity_p_p.h @@ -0,0 +1,87 @@ +/**************************************************************************** +** +** Copyright (C) 2014 Klaralvdalens Datakonsult AB (KDAB). +** Copyright (C) 2016 The Qt Company Ltd and/or its subsidiary(-ies). +** Contact: https://www.qt.io/licensing/ +** +** This file is part of the Qt3D module of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:LGPL$ +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/contact-us. +** +** GNU Lesser General Public License Usage +** Alternatively, this file may be used under the terms of the GNU Lesser +** General Public License version 3 as published by the Free Software +** Foundation and appearing in the file LICENSE.LGPL3 included in the +** packaging of this file. Please review the following information to +** ensure the GNU Lesser General Public License version 3 requirements +** will be met: https://www.gnu.org/licenses/lgpl-3.0.html. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 2.0 or (at your option) the GNU General +** Public license version 3 or any later version approved by the KDE Free +** Qt Foundation. The licenses are as published by the Free Software +** Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3 +** included in the packaging of this file. Please review the following +** information to ensure the GNU General Public License requirements will +** be met: https://www.gnu.org/licenses/gpl-2.0.html and +** https://www.gnu.org/licenses/gpl-3.0.html. +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +#ifndef QT3DRENDER_RENDER_ENTITY_P_P_H +#define QT3DRENDER_RENDER_ENTITY_P_P_H + +// +// W A R N I N G +// ------------- +// +// This file is not part of the Qt API. It exists for the convenience +// of other Qt classes. This header file may change from version to +// version without notice, or even be removed. +// +// We mean it. +// + +#include +#include + +QT_BEGIN_NAMESPACE + +namespace Qt3DCore { +class QNode; +} + +namespace Qt3DRender { + +class QRenderAspect; + +namespace Render { + +class Q_AUTOTEST_EXPORT EntityPrivate : public Qt3DCore::QBackendNodePrivate { +public: + EntityPrivate(); + + Q_DECLARE_PUBLIC(Entity) + + static EntityPrivate *get(Entity *node); + + void componentAdded(Qt3DCore::QNode *frontend) override; + void componentRemoved(Qt3DCore::QNode *frontend) override; +}; + +} // namespace Render +} // namespace Qt3DRender + +QT_END_NAMESPACE + +#endif // QT3DRENDER_RENDER_ENTITY_P_P_H diff --git a/src/render/backend/render-backend.pri b/src/render/backend/render-backend.pri index 6b60dfcda..9510b9530 100644 --- a/src/render/backend/render-backend.pri +++ b/src/render/backend/render-backend.pri @@ -11,6 +11,7 @@ HEADERS += \ $$PWD/platformsurfacefilter_p.h \ $$PWD/cameralens_p.h \ $$PWD/entity_p.h \ + $$PWD/entity_p_p.h \ $$PWD/entityvisitor_p.h \ $$PWD/entityaccumulator_p.h \ $$PWD/layer_p.h \ diff --git a/tests/auto/core/common/testpostmanarbiter.cpp b/tests/auto/core/common/testpostmanarbiter.cpp index 3fd8c80d1..5869cbad3 100644 --- a/tests/auto/core/common/testpostmanarbiter.cpp +++ b/tests/auto/core/common/testpostmanarbiter.cpp @@ -94,6 +94,13 @@ void TestArbiter::addDirtyFrontEndNode(Qt3DCore::QNode *node) dirtyNodes << node; } +void TestArbiter::addDirtyFrontEndNode(Qt3DCore::QNode *node, Qt3DCore::QNode *subNode, const char *property, Qt3DCore::ChangeFlag change) +{ + if (!dirtyNodes.contains(node)) + dirtyNodes << node; + dirtySubNodes.push_back({node, subNode, change, property}); +} + void TestArbiter::removeDirtyFrontEndNode(Qt3DCore::QNode *node) { dirtyNodes.removeOne(node); diff --git a/tests/auto/core/common/testpostmanarbiter.h b/tests/auto/core/common/testpostmanarbiter.h index e79283763..cecc24f93 100644 --- a/tests/auto/core/common/testpostmanarbiter.h +++ b/tests/auto/core/common/testpostmanarbiter.h @@ -66,9 +66,11 @@ public: QVector events; QVector dirtyNodes; + QVector dirtySubNodes; void setArbiterOnNode(Qt3DCore::QNode *node); void addDirtyFrontEndNode(Qt3DCore::QNode *node) final; + void addDirtyFrontEndNode(Qt3DCore::QNode *node, Qt3DCore::QNode *subNode, const char *property, Qt3DCore::ChangeFlag change) final; void removeDirtyFrontEndNode(Qt3DCore::QNode *node) final; private: diff --git a/tests/auto/core/nodes/tst_nodes.cpp b/tests/auto/core/nodes/tst_nodes.cpp index 3da3e97e0..3b93e7715 100644 --- a/tests/auto/core/nodes/tst_nodes.cpp +++ b/tests/auto/core/nodes/tst_nodes.cpp @@ -179,11 +179,18 @@ public: dirtyNodes << node; } + void addDirtyFrontEndNode(Qt3DCore::QNode *node, Qt3DCore::QNode *subNode, const char *property, Qt3DCore::ChangeFlag change) final { + if (!dirtyNodes.contains(node)) + dirtyNodes << node; + dirtySubNodes.push_back({node, subNode, change, property}); + } + void removeDirtyFrontEndNode(Qt3DCore::QNode *node) final { dirtyNodes.removeOne(node); - }; + } QVector dirtyNodes; + QVector dirtySubNodes; QList events; QScopedPointer m_postman; }; @@ -1594,7 +1601,8 @@ void tst_Nodes::checkSceneIsSetOnConstructionWithParent() } // THEN - QCOMPARE(spy.events.size(), 10); // 5 QComponentAddedChange(entity, cmp) and 5 QComponentAddedChange(cmp, entity) + QCOMPARE(spy.events.size(), 0); + QCOMPARE(spy.dirtySubNodes.size(), 5); // 5 entities changed } void tst_Nodes::appendingParentlessComponentToEntityWithoutScene() @@ -1623,33 +1631,21 @@ void tst_Nodes::appendingParentlessComponentToEntityWithoutScene() QVERIFY(entity->components().count() == 1); QVERIFY(entity->components().first() == comp); QVERIFY(comp->parentNode() == entity.data()); - QCOMPARE(entitySpy.events.size(), 1); - QVERIFY(entitySpy.events.first().wasLocked()); - QCOMPARE(componentSpy.events.size(), 1); + QCOMPARE(entitySpy.events.size(), 0); + QCOMPARE(entitySpy.dirtyNodes.size(), 1); + QCOMPARE(entitySpy.dirtySubNodes.size(), 1); + + const auto event = entitySpy.dirtySubNodes.first(); + QCOMPARE(event.change, Qt3DCore::ComponentAdded); + QCOMPARE(event.node, entity.data()); + QCOMPARE(event.subNode, comp); + QCOMPARE(event.property, nullptr); // 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 // when setting the parent but that doesn't happen since no scene is // actually set on the entity and that QNodePrivate::_q_addChild will // return early in such a case. - - // Check that we received ComponentAdded - { - const auto event = entitySpy.events.takeFirst().change().dynamicCast(); - QCOMPARE(event->type(), Qt3DCore::ComponentAdded); - QCOMPARE(event->subjectId(), entity->id()); - QCOMPARE(event->entityId(), entity->id()); - QCOMPARE(event->componentId(), comp->id()); - QCOMPARE(event->componentMetaObject(), comp->metaObject()); - } - { - const auto event = componentSpy.events.takeFirst().change().dynamicCast(); - QCOMPARE(event->type(), Qt3DCore::ComponentAdded); - QCOMPARE(event->subjectId(), comp->id()); - QCOMPARE(event->entityId(), entity->id()); - QCOMPARE(event->componentId(), comp->id()); - QCOMPARE(event->componentMetaObject(), comp->metaObject()); - } } } @@ -1687,10 +1683,8 @@ void tst_Nodes::appendingParentlessComponentToNonRootEntity() QVERIFY(entity->components().first() == comp); QVERIFY(comp->parentNode() == entity.data()); - QCOMPARE(eventSpy.events.size(), 3); + QCOMPARE(eventSpy.events.size(), 1); // - entity added as child to root - // - component added for entity - // - component added for compontent QVERIFY(eventSpy.events.first().wasLocked()); QVERIFY(Qt3DCore::QNodePrivate::get(entity.data())->m_hasBackendNode); @@ -1704,24 +1698,6 @@ void tst_Nodes::appendingParentlessComponentToNonRootEntity() QCOMPARE(event->propertyName(), QByteArrayLiteral("children")); QCOMPARE(event->addedNodeId(), entity->id()); } - { - const auto event = eventSpy.events.takeFirst().change().dynamicCast(); - QVERIFY(!event.isNull()); - QCOMPARE(event->type(), Qt3DCore::ComponentAdded); - QCOMPARE(event->subjectId(), entity->id()); - QCOMPARE(event->entityId(), entity->id()); - QCOMPARE(event->componentId(), comp->id()); - QCOMPARE(event->componentMetaObject(), comp->metaObject()); - } - { - const auto event = eventSpy.events.takeFirst().change().dynamicCast(); - QVERIFY(!event.isNull()); - QCOMPARE(event->type(), Qt3DCore::ComponentAdded); - QCOMPARE(event->subjectId(), comp->id()); - QCOMPARE(event->entityId(), entity->id()); - QCOMPARE(event->componentId(), comp->id()); - QCOMPARE(event->componentMetaObject(), comp->metaObject()); - } } } @@ -1755,10 +1731,8 @@ void tst_Nodes::appendingParentlessComponentToEntityWithScene() QVERIFY(entity->components().first() == comp); QVERIFY(comp->parentNode() == entity.data()); - QCOMPARE(eventSpy.events.size(), 3); + QCOMPARE(eventSpy.events.size(), 1); // - child added - // - component added for entity - // - component added for compontent QVERIFY(eventSpy.events.first().wasLocked()); { @@ -1769,24 +1743,6 @@ void tst_Nodes::appendingParentlessComponentToEntityWithScene() QCOMPARE(event->propertyName(), QByteArrayLiteral("children")); QCOMPARE(event->addedNodeId(), comp->id()); } - { - const auto event = eventSpy.events.takeFirst().change().dynamicCast(); - QVERIFY(!event.isNull()); - QCOMPARE(event->type(), Qt3DCore::ComponentAdded); - QCOMPARE(event->subjectId(), entity->id()); - QCOMPARE(event->entityId(), entity->id()); - QCOMPARE(event->componentId(), comp->id()); - QCOMPARE(event->componentMetaObject(), comp->metaObject()); - } - { - const auto event = eventSpy.events.takeFirst().change().dynamicCast(); - QVERIFY(!event.isNull()); - QCOMPARE(event->type(), Qt3DCore::ComponentAdded); - QCOMPARE(event->subjectId(), comp->id()); - QCOMPARE(event->entityId(), entity->id()); - QCOMPARE(event->componentId(), comp->id()); - QCOMPARE(event->componentMetaObject(), comp->metaObject()); - } } } @@ -1816,24 +1772,15 @@ void tst_Nodes::appendingComponentToEntity() QVERIFY(entity->components().count() == 1); QVERIFY(entity->components().first() == comp); QVERIFY(comp->parentNode() == entity.data()); - QCOMPARE(entitySpy.events.size(), 1); - QVERIFY(entitySpy.events.first().wasLocked()); - { - const auto event = entitySpy.events.takeFirst().change().dynamicCast(); - QCOMPARE(event->type(), Qt3DCore::ComponentAdded); - QCOMPARE(event->subjectId(), entity->id()); - QCOMPARE(event->entityId(), entity->id()); - QCOMPARE(event->componentId(), comp->id()); - QCOMPARE(event->componentMetaObject(), comp->metaObject()); - } - { - const auto event = componentSpy.events.takeFirst().change().dynamicCast(); - QCOMPARE(event->type(), Qt3DCore::ComponentAdded); - QCOMPARE(event->subjectId(), comp->id()); - QCOMPARE(event->entityId(), entity->id()); - QCOMPARE(event->componentId(), comp->id()); - QCOMPARE(event->componentMetaObject(), comp->metaObject()); - } + QCOMPARE(entitySpy.events.size(), 0); + QCOMPARE(entitySpy.dirtyNodes.size(), 1); + QCOMPARE(entitySpy.dirtySubNodes.size(), 1); + QCOMPARE(entitySpy.dirtyNodes.first(), entity.data()); + const auto event = entitySpy.dirtySubNodes.takeFirst(); + QCOMPARE(event.node, entity.data()); + QCOMPARE(event.subNode, comp); + QCOMPARE(event.change, Qt3DCore::ComponentAdded); + QCOMPARE(event.property, nullptr); } } @@ -1858,6 +1805,8 @@ void tst_Nodes::removingComponentFromEntity() // WHEN entitySpy.events.clear(); + entitySpy.dirtyNodes.clear(); + entitySpy.dirtySubNodes.clear(); componentSpy.events.clear(); entity->removeComponent(comp); @@ -1865,24 +1814,17 @@ void tst_Nodes::removingComponentFromEntity() QVERIFY(entity->components().count() == 0); QVERIFY(comp->parent() == entity.data()); QVERIFY(entity->children().count() == 1); - QCOMPARE(entitySpy.events.size(), 1); - QVERIFY(entitySpy.events.first().wasLocked()); - QCOMPARE(componentSpy.events.size(), 1); + QCOMPARE(entitySpy.events.size(), 0); + QCOMPARE(entitySpy.dirtyNodes.size(), 1); + QCOMPARE(entitySpy.dirtySubNodes.size(), 1); + QCOMPARE(componentSpy.events.size(), 0); { - const auto event = entitySpy.events.takeFirst().change().dynamicCast(); - QCOMPARE(event->type(), Qt3DCore::ComponentRemoved); - QCOMPARE(event->subjectId(), entity->id()); - QCOMPARE(event->entityId(), entity->id()); - QCOMPARE(event->componentId(), comp->id()); - QCOMPARE(event->componentMetaObject(), comp->metaObject()); - } - { - const auto event = componentSpy.events.takeFirst().change().dynamicCast(); - QCOMPARE(event->type(), Qt3DCore::ComponentRemoved); - QCOMPARE(event->subjectId(), comp->id()); - QCOMPARE(event->entityId(), entity->id()); - QCOMPARE(event->componentId(), comp->id()); - QCOMPARE(event->componentMetaObject(), comp->metaObject()); + const auto event = entitySpy.dirtySubNodes.takeFirst(); + qDebug() << event.change; + QCOMPARE(event.change, Qt3DCore::ComponentRemoved); + QCOMPARE(event.node, entity.data()); + QCOMPARE(event.subNode, comp); + QCOMPARE(event.property, nullptr); } } } diff --git a/tests/auto/render/entity/tst_entity.cpp b/tests/auto/render/entity/tst_entity.cpp index 1f3ed4733..83d32fddb 100644 --- a/tests/auto/render/entity/tst_entity.cpp +++ b/tests/auto/render/entity/tst_entity.cpp @@ -28,6 +28,7 @@ #include #include +#include #include #include #include @@ -35,8 +36,6 @@ #include #include -#include -#include #include #include @@ -144,19 +143,18 @@ private slots: QVERIFY(entity.layerIds().isEmpty()); // WHEN - for (QComponent *component : components) { - const auto addChange = QComponentAddedChangePtr::create(&dummyFrontendEntity, component); - entity.sceneChangeEvent(addChange); - } + for (QComponent *component : components) + EntityPrivate::get(&entity)->componentAdded(component); Qt3DCore::QEntity dummyFrontendEntityChild; // Create nodes in the backend manager nodeManagers.renderNodesManager()->getOrCreateResource(dummyFrontendEntity.id()); nodeManagers.renderNodesManager()->getOrCreateResource(dummyFrontendEntityChild.id()); - // Send children added event to entity - const auto addEntityChange = QPropertyNodeAddedChangePtr::create(dummyFrontendEntity.id(), &dummyFrontendEntityChild); - entity.sceneChangeEvent(addEntityChange); +// TODOSYNC clean up +// // Send children added event to entity +// const auto addEntityChange = QPropertyNodeAddedChangePtr::create(dummyFrontendEntity.id(), &dummyFrontendEntityChild); +// entity.sceneChangeEvent(addEntityChange); // THEN QVERIFY(!entity.componentUuid().isNull()); @@ -382,8 +380,7 @@ private slots: QVERIFY(method(&entity).isNull()); // WHEN - const auto addChange = QComponentAddedChangePtr::create(&dummyFrontendEntity, component); - entity.sceneChangeEvent(addChange); + EntityPrivate::get(&entity)->componentAdded(component); // THEN QCOMPARE(method(&entity), component->id()); @@ -391,8 +388,7 @@ private slots: // WHEN renderer.resetDirty(); - const auto removeChange = QComponentRemovedChangePtr::create(&dummyFrontendEntity, component); - entity.sceneChangeEvent(removeChange); + EntityPrivate::get(&entity)->componentRemoved(component); // THEN QVERIFY(method(&entity).isNull()); @@ -437,10 +433,8 @@ private slots: QVERIFY(method(&entity).isEmpty()); // WHEN - for (QComponent *component : components) { - const auto addChange = QComponentAddedChangePtr::create(&dummyFrontendEntity, component); - entity.sceneChangeEvent(addChange); - } + for (QComponent *component : components) + EntityPrivate::get(&entity)->componentAdded(component); // THEN QCOMPARE(method(&entity).size(), components.size()); @@ -451,8 +445,7 @@ private slots: // WHEN renderer.resetDirty(); - const auto removeChange = QComponentRemovedChangePtr::create(&dummyFrontendEntity, components.first()); - entity.sceneChangeEvent(removeChange); + EntityPrivate::get(&entity)->componentRemoved(components.first()); // THEN QCOMPARE(method(&entity).size(), components.size() - 1);