Update QMaterial to use direct sync

Change-Id: I3e2a687ab46196728083a91ce114066329e77278
Reviewed-by: Paul Lemire <paul.lemire@kdab.com>

6 files changed, 52 insertions, 86 deletions

diff --git a/src/render/frontend/qrenderaspect.cpp b/src/render/frontend/qrenderaspect.cpp
index 78922a7ba..b7db4c53e 100644
--- a/src/render/frontend/qrenderaspect.cpp
+++ b/src/render/frontend/qrenderaspect.cpp
@@ -285,7 +285,7 @@ void QRenderAspectPrivate::registerBackendTypes()
     q->registerBackendType<QFilterKey>(QSharedPointer<Render::NodeFunctor<Render::FilterKey, Render::FilterKeyManager> >::create(m_renderer));
     q->registerBackendType<QAbstractLight, true>(QSharedPointer<Render::RenderLightFunctor>::create(m_renderer, m_nodeManagers));
     q->registerBackendType<QEnvironmentLight, true>(QSharedPointer<Render::NodeFunctor<Render::EnvironmentLight, Render::EnvironmentLightManager> >::create(m_renderer));
-    q->registerBackendType<QMaterial>(QSharedPointer<Render::NodeFunctor<Render::Material, Render::MaterialManager> >::create(m_renderer));
+    q->registerBackendType<QMaterial, true>(QSharedPointer<Render::NodeFunctor<Render::Material, Render::MaterialManager> >::create(m_renderer));
     q->registerBackendType<QParameter>(QSharedPointer<Render::NodeFunctor<Render::Parameter, Render::ParameterManager> >::create(m_renderer));
     q->registerBackendType<QRenderPass>(QSharedPointer<Render::NodeFunctor<Render::RenderPass, Render::RenderPassManager> >::create(m_renderer));
     q->registerBackendType<QShaderData>(QSharedPointer<Render::RenderShaderDataFunctor>::create(m_renderer, m_nodeManagers));
diff --git a/src/render/materialsystem/material.cpp b/src/render/materialsystem/material.cpp
index a69ff8a13..07764c207 100644
--- a/src/render/materialsystem/material.cpp
+++ b/src/render/materialsystem/material.cpp
@@ -73,47 +73,26 @@ void Material::cleanup()
     m_parameterPack.clear();
 }
 
-void Material::initializeFromPeer(const Qt3DCore::QNodeCreatedChangeBasePtr &change)
+void Material::syncFromFrontEnd(const QNode *frontEnd, bool firstTime)
 {
-    const auto typedChange = qSharedPointerCast<Qt3DCore::QNodeCreatedChange<QMaterialData>>(change);
-    const auto &data = typedChange->data;
-    m_effectUuid = data.effectId;
-    m_parameterPack.setParameters(data.parameterIds);
-
-    markDirty(AbstractRenderer::MaterialDirty);
-}
-
-void Material::sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e)
-{
-
-    switch (e->type()) {
-    case PropertyUpdated: {
-        const auto change = qSharedPointerCast<QPropertyUpdatedChange>(e);
-        if (change->propertyName() == QByteArrayLiteral("effect"))
-            m_effectUuid = change->value().value<QNodeId>();
-        break;
-    }
-
-    case PropertyValueAdded: {
-        const auto change = qSharedPointerCast<QPropertyNodeAddedChange>(e);
-        if (change->propertyName() == QByteArrayLiteral("parameter"))
-            m_parameterPack.appendParameter(change->addedNodeId());
-        break;
-    }
-
-    case PropertyValueRemoved: {
-        const auto change = qSharedPointerCast<QPropertyNodeRemovedChange>(e);
-        if (change->propertyName() == QByteArrayLiteral("parameter"))
-            m_parameterPack.removeParameter(change->removedNodeId());
-        break;
-    }
-
-    default:
-        break;
-    }
-    markDirty(AbstractRenderer::AllDirty);
-
-    BackendNode::sceneChangeEvent(e);
+    BackendNode::syncFromFrontEnd(frontEnd, firstTime);
+    const QMaterial *node = qobject_cast<const QMaterial *>(frontEnd);
+    if (!node)
+        return;
+
+    auto parameters = qIdsForNodes(node->parameters());
+    std::sort(std::begin(parameters), std::end(parameters));
+    if (m_parameterPack.parameters() != parameters)
+        m_parameterPack.setParameters(parameters);
+
+    const auto effectId = node->effect() ? node->effect()->id() : QNodeId{};
+    if (effectId != m_effectUuid)
+        m_effectUuid = effectId;
+
+    if (firstTime)
+        markDirty(AbstractRenderer::MaterialDirty);
+    else
+        markDirty(AbstractRenderer::AllDirty);
 }
 
 QVector<Qt3DCore::QNodeId> Material::parameters() const
diff --git a/src/render/materialsystem/material_p.h b/src/render/materialsystem/material_p.h
index 3b55dc657..7a02c6691 100644
--- a/src/render/materialsystem/material_p.h
+++ b/src/render/materialsystem/material_p.h
@@ -79,14 +79,12 @@ public:
     ~Material();
     void cleanup();
 
-    void sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e) override;
+    void syncFromFrontEnd(const Qt3DCore::QNode *frontEnd, bool firstTime) override;
 
     QVector<Qt3DCore::QNodeId> parameters() const;
     Qt3DCore::QNodeId effect() const;
 
 private:
-    void initializeFromPeer(const Qt3DCore::QNodeCreatedChangeBasePtr &change) final;
-
     ParameterPack m_parameterPack;
     Qt3DCore::QNodeId m_effectUuid;
 };
diff --git a/src/render/materialsystem/qmaterial.cpp b/src/render/materialsystem/qmaterial.cpp
index c6913441e..db523a700 100644
--- a/src/render/materialsystem/qmaterial.cpp
+++ b/src/render/materialsystem/qmaterial.cpp
@@ -275,11 +275,7 @@ void QMaterial::addParameter(QParameter *parameter)
         if (!parameter->parent())
             parameter->setParent(this);
 
-        if (d->m_changeArbiter != nullptr) {
-            const auto change = QPropertyNodeAddedChangePtr::create(id(), parameter);
-            change->setPropertyName("parameter");
-            d->notifyObservers(change);
-        }
+        d->update();
     }
 }
 
@@ -290,11 +286,7 @@ void QMaterial::removeParameter(QParameter *parameter)
 {
     Q_ASSERT(parameter);
     Q_D(QMaterial);
-    if (d->m_changeArbiter != nullptr) {
-        const auto change = QPropertyNodeRemovedChangePtr::create(id(), parameter);
-        change->setPropertyName("parameter");
-        d->notifyObservers(change);
-    }
+    d->update();
     d->m_parameters.removeOne(parameter);
 }
 
diff --git a/tests/auto/render/material/tst_material.cpp b/tests/auto/render/material/tst_material.cpp
index 9dfa0c246..e47eaea34 100644
--- a/tests/auto/render/material/tst_material.cpp
+++ b/tests/auto/render/material/tst_material.cpp
@@ -45,6 +45,7 @@ using namespace Qt3DRender::Render;
 class tst_RenderMaterial : public Qt3DCore::QBackendNodeTester
 {
     Q_OBJECT
+
 public:
     tst_RenderMaterial() {}
 
@@ -117,7 +118,7 @@ void tst_RenderMaterial::shouldHavePropertiesMirroringFromItsPeer()
 
     // GIVEN
     backend.setRenderer(&renderer);
-    simulateInitialization(frontendMaterial, &backend);
+    simulateInitializationSync(frontendMaterial, &backend);
 
     // THEN
     QVERIFY(backend.isEnabled() == frontendMaterial->isEnabled());
@@ -136,15 +137,17 @@ void tst_RenderMaterial::shouldHavePropertiesMirroringFromItsPeer()
 void tst_RenderMaterial::shouldHandleParametersPropertyChange()
 {
     // GIVEN
-    QScopedPointer<QParameter> parameter(new QParameter());
+    QParameter *parameter = new QParameter();
     Material backend;
     TestRenderer renderer;
     backend.setRenderer(&renderer);
 
+    QMaterial material;
+    simulateInitializationSync(&material, &backend);
+
     // WHEN
-    const auto addChange = Qt3DCore::QPropertyNodeAddedChangePtr::create(Qt3DCore::QNodeId(), parameter.data());
-    addChange->setPropertyName("parameter");
-    backend.sceneChangeEvent(addChange);
+    material.addParameter(parameter);
+    backend.syncFromFrontEnd(&material, false);
 
     // THEN
     QCOMPARE(backend.parameters().count(), 1);
@@ -152,9 +155,8 @@ void tst_RenderMaterial::shouldHandleParametersPropertyChange()
     QVERIFY(renderer.dirtyBits() != 0);
 
     // WHEN
-    const auto removeChange = Qt3DCore::QPropertyNodeRemovedChangePtr::create(Qt3DCore::QNodeId(), parameter.data());
-    removeChange->setPropertyName("parameter");
-    backend.sceneChangeEvent(removeChange);
+    material.removeParameter(parameter);
+    backend.syncFromFrontEnd(&material, false);
 
     // THEN
     QVERIFY(backend.parameters().isEmpty());
@@ -167,24 +169,23 @@ void tst_RenderMaterial::shouldHandleEnablePropertyChange()
     TestRenderer renderer;
     backend.setRenderer(&renderer);
 
+    QMaterial material;
+    simulateInitializationSync(&material, &backend);
+
     // WHEN
-    auto updateChange = QPropertyUpdatedChangePtr::create(QNodeId());
-    updateChange->setValue(true);
-    updateChange->setPropertyName("enabled");
-    backend.sceneChangeEvent(updateChange);
+    material.setEnabled(false);
+    backend.syncFromFrontEnd(&material, false);
 
     // THEN
-    QVERIFY(backend.isEnabled());
+    QVERIFY(!backend.isEnabled());
     QVERIFY(renderer.dirtyBits() != 0);
 
     // WHEN
-    auto secondUpdateChange = QPropertyUpdatedChangePtr::create(QNodeId());
-    secondUpdateChange->setValue(false);
-    secondUpdateChange->setPropertyName("enabled");
-    backend.sceneChangeEvent(secondUpdateChange);
+    material.setEnabled(true);
+    backend.syncFromFrontEnd(&material, false);
 
     // THEN
-    QVERIFY(!backend.isEnabled());
+    QVERIFY(backend.isEnabled());
 
 }
 
@@ -195,15 +196,16 @@ void tst_RenderMaterial::shouldHandleEffectPropertyChange()
     TestRenderer renderer;
     backend.setRenderer(&renderer);
 
+    QMaterial material;
+    simulateInitializationSync(&material, &backend);
+
     // WHEN
-    QPropertyUpdatedChangePtr updateChange(new Qt3DCore::QPropertyUpdatedChange(Qt3DCore::QNodeId()));
-    Qt3DCore::QNodeId effectId = Qt3DCore::QNodeId::createId();
-    updateChange->setValue(QVariant::fromValue(effectId));
-    updateChange->setPropertyName("effect");
-    backend.sceneChangeEvent(updateChange);
+    QEffect effect;
+    material.setEffect(&effect);
+    backend.syncFromFrontEnd(&material, false);
 
     // THEN
-    QCOMPARE(backend.effect(), effectId);
+    QCOMPARE(backend.effect(), effect.id());
     QVERIFY(renderer.dirtyBits() != 0);
 }
 
diff --git a/tests/auto/render/qmaterial/tst_qmaterial.cpp b/tests/auto/render/qmaterial/tst_qmaterial.cpp
index fdf8c828a..c2263d71c 100644
--- a/tests/auto/render/qmaterial/tst_qmaterial.cpp
+++ b/tests/auto/render/qmaterial/tst_qmaterial.cpp
@@ -295,13 +295,8 @@ private Q_SLOTS:
         QCOMPARE(param->parent(), material);
 
         // THEN
-        QCOMPARE(arbiter.events.size(), 1);
-        Qt3DCore::QPropertyNodeAddedChangePtr change = arbiter.events.first().staticCast<Qt3DCore::QPropertyNodeAddedChange>();
-        QCOMPARE(change->propertyName(), "parameter");
-        QCOMPARE(change->addedNodeId(), param->id());
-        QCOMPARE(change->type(), Qt3DCore::PropertyValueAdded);
-
-        arbiter.events.clear();
+        QCOMPARE(arbiter.events.size(), 0);
+        QVERIFY(material->parameters().contains(param));
 
         // WHEN (add parameter to effect)
         param = new Qt3DRender::QParameter("testParamEffect", QVariant::fromValue(383.0f));
@@ -319,7 +314,7 @@ private Q_SLOTS:
 
         // THEN
         QCOMPARE(arbiter.events.size(), 1);
-        change = arbiter.events.first().staticCast<Qt3DCore::QPropertyNodeAddedChange>();
+        auto change = arbiter.events.first().staticCast<Qt3DCore::QPropertyNodeAddedChange>();
         QCOMPARE(change->propertyName(), "parameter");
         QCOMPARE(change->addedNodeId(), param->id());
         QCOMPARE(change->type(), Qt3DCore::PropertyValueAdded);