diff options
author | Paul Lemire <paul.lemire@kdab.com> | 2015-10-12 16:59:18 +0200 |
---|---|---|
committer | Paul Lemire <paul.lemire@kdab.com> | 2015-10-14 18:53:10 +0000 |
commit | 52bcbd19273842c7b46d353a2cc52d3d0229c00d (patch) | |
tree | 7eb094f79bdc645215071b46fd1ea12bba0a32d7 | |
parent | 6cf4712bda7d3453ddf728f5903374abd2f4efc9 (diff) |
QMaterial: remove NodeAdded/Remove notifications on Effect
Rely on the parent being set for inline declaration and the emit effectChanged
to automatically send notifications.
Updated unit tests accordingly.
Change-Id: I7304309ea248da5e15db3dea4d556162af5e940e
Reviewed-by: Andy Nichols <andy.nichols@theqtcompany.com>
-rw-r--r-- | src/render/materialsystem/material.cpp | 2 | ||||
-rw-r--r-- | src/render/materialsystem/qmaterial.cpp | 20 | ||||
-rw-r--r-- | tests/auto/render/material/tst_material.cpp | 18 | ||||
-rw-r--r-- | tests/auto/render/qmaterial/tst_qmaterial.cpp | 18 |
4 files changed, 31 insertions, 27 deletions
diff --git a/src/render/materialsystem/material.cpp b/src/render/materialsystem/material.cpp index faf934d3e..ad7aa0eae 100644 --- a/src/render/materialsystem/material.cpp +++ b/src/render/materialsystem/material.cpp @@ -88,6 +88,8 @@ void Material::sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e) case NodeUpdated: { if (propertyChange->propertyName() == QByteArrayLiteral("enabled")) m_enabled = propertyChange->value().toBool(); + else if (propertyChange->propertyName() == QByteArrayLiteral("effect")) + m_effectUuid = propertyChange->value().value<QNodeId>(); break; } // Check for shader parameter diff --git a/src/render/materialsystem/qmaterial.cpp b/src/render/materialsystem/qmaterial.cpp index c220b1841..3610c4e3b 100644 --- a/src/render/materialsystem/qmaterial.cpp +++ b/src/render/materialsystem/qmaterial.cpp @@ -113,30 +113,14 @@ void QMaterial::setEffect(QEffect *effect) Q_D(QMaterial); if (effect != d->m_effect) { - if (d->m_effect && d->m_changeArbiter) { - QScenePropertyChangePtr change(new QScenePropertyChange(NodeRemoved, QSceneChange::Node, id())); - change->setPropertyName("effect"); - change->setValue(QVariant::fromValue(d->m_effect->id())); - d->notifyObservers(change); - } - - d->m_effect = effect; - const bool blocked = blockNotifications(true); - emit effectChanged(); - blockNotifications(blocked); // We need to add it as a child of the current node if it has been declared inline // Or not previously added as a child of the current node so that // 1) The backend gets notified about it's creation // 2) When the current node is destroyed, it gets destroyed as well if (effect && !effect->parent()) effect->setParent(this); - - if (d->m_effect && d->m_changeArbiter) { - QScenePropertyChangePtr change(new QScenePropertyChange(NodeAdded, QSceneChange::Node, id())); - change->setPropertyName("effect"); - change->setValue(QVariant::fromValue(effect->id())); - d->notifyObservers(change); - } + d->m_effect = effect; + emit effectChanged(); } } diff --git a/tests/auto/render/material/tst_material.cpp b/tests/auto/render/material/tst_material.cpp index abf74c823..48733d03b 100644 --- a/tests/auto/render/material/tst_material.cpp +++ b/tests/auto/render/material/tst_material.cpp @@ -59,6 +59,7 @@ private slots: void shouldHavePropertiesMirroringFromItsPeer(); void shouldHandleParametersPropertyChange(); void shouldHandleEnablePropertyChange(); + void shouldHandleEffectPropertyChange(); }; @@ -181,6 +182,23 @@ void tst_RenderMaterial::shouldHandleEnablePropertyChange() // THEN QVERIFY(backend.isEnabled()); + +} + +void tst_RenderMaterial::shouldHandleEffectPropertyChange() +{ + // GIVEN + Material backend; + + // WHEN + QScenePropertyChangePtr updateChange(new Qt3DCore::QScenePropertyChange(Qt3DCore::NodeUpdated, Qt3DCore::QSceneChange::Node, Qt3DCore::QNodeId())); + Qt3DCore::QNodeId effectId = Qt3DCore::QNodeId::createId(); + updateChange->setValue(QVariant::fromValue(effectId)); + updateChange->setPropertyName("effect"); + backend.sceneChangeEvent(updateChange); + + // THEN + QCOMPARE(backend.effect(), effectId); } QTEST_APPLESS_MAIN(tst_RenderMaterial) diff --git a/tests/auto/render/qmaterial/tst_qmaterial.cpp b/tests/auto/render/qmaterial/tst_qmaterial.cpp index b12420a5d..3edeb33e1 100644 --- a/tests/auto/render/qmaterial/tst_qmaterial.cpp +++ b/tests/auto/render/qmaterial/tst_qmaterial.cpp @@ -87,6 +87,12 @@ class tst_QMaterial : public Qt3DCore::QNode { Q_OBJECT public: + tst_QMaterial() + : Qt3DCore::QNode() + { + qRegisterMetaType<Qt3DRender::QEffect*>("Qt3DRender::QEffect*"); + } + ~tst_QMaterial() { QNode::cleanup(); @@ -267,7 +273,7 @@ private Q_SLOTS: Qt3DCore::QScenePropertyChangePtr change = arbiter.events.first().staticCast<Qt3DCore::QScenePropertyChange>(); QCOMPARE(change->propertyName(), "effect"); QCOMPARE(change->value().value<Qt3DCore::QNodeId>(), effect.id()); - QCOMPARE(change->type(), Qt3DCore::NodeAdded); + QCOMPARE(change->type(), Qt3DCore::NodeUpdated); arbiter.events.clear(); @@ -276,21 +282,15 @@ private Q_SLOTS: TestArbiter arbiter2(material2.data()); // WHEN - Qt3DRender::QEffect *oldEffect = material2->effect(); material2->setEffect(&effect); QCoreApplication::processEvents(); // THEN - QCOMPARE(arbiter2.events.size(), 2); + QCOMPARE(arbiter2.events.size(), 1); change = arbiter2.events.first().staticCast<Qt3DCore::QScenePropertyChange>(); QCOMPARE(change->propertyName(), "effect"); - QCOMPARE(change->value().value<Qt3DCore::QNodeId>(), oldEffect->id()); - QCOMPARE(change->type(), Qt3DCore::NodeRemoved); - - change = arbiter2.events.last().staticCast<Qt3DCore::QScenePropertyChange>(); - QCOMPARE(change->propertyName(), "effect"); QCOMPARE(change->value().value<Qt3DCore::QNodeId>(), effect.id()); - QCOMPARE(change->type(), Qt3DCore::NodeAdded); + QCOMPARE(change->type(), Qt3DCore::NodeUpdated); } void checkDynamicParametersAddedUpdates() |