From 1a7ff8184cb8f3edd2efd069b5a84aa71e7063fb Mon Sep 17 00:00:00 2001
From: Tobias Knopff <t.knopff@fz-juelich.de>
Date: Thu, 1 Jul 2021 15:53:18 +0200
Subject: [PATCH] Make ItemWithMaterial::P_MATERIAL private
---
GUI/Models/GUIDomainSampleVisitor.cpp | 11 ++---
GUI/Models/ItemWithMaterial.cpp | 20 +++++++++
GUI/Models/ItemWithMaterial.h | 9 +++-
GUI/Models/LayerItem.cpp | 2 +-
GUI/Models/LayerItem.h | 2 +-
GUI/Models/MaterialItemUtils.cpp | 13 ------
GUI/Models/MaterialItemUtils.h | 1 -
GUI/Models/MaterialPropertyController.cpp | 42 +++++++++----------
GUI/Models/MaterialPropertyController.h | 3 +-
GUI/Models/ParticleItem.cpp | 2 +-
GUI/Models/TransformToDomain.cpp | 6 +--
GUI/Models/TransformToDomain.h | 3 +-
GUI/Views/RealSpaceWidgets/TransformTo3D.cpp | 2 +-
GUI/Views/SampleDesigner/ILayerView.cpp | 23 +++-------
Tests/UnitTests/GUI/TestComponentUtils.cpp | 4 +-
Tests/UnitTests/GUI/TestLayerItems.cpp | 3 +-
.../GUI/TestMaterialPropertyController.cpp | 14 +++----
17 files changed, 78 insertions(+), 82 deletions(-)
diff --git a/GUI/Models/GUIDomainSampleVisitor.cpp b/GUI/Models/GUIDomainSampleVisitor.cpp
index c3bbf452757..7f1c4e0b857 100644
--- a/GUI/Models/GUIDomainSampleVisitor.cpp
+++ b/GUI/Models/GUIDomainSampleVisitor.cpp
@@ -44,6 +44,9 @@
#include "Sample/Particle/ParticleCoreShell.h"
#include "Sample/SoftParticle/SoftParticles.h"
+#include <boost/polymorphic_cast.hpp>
+using boost::polymorphic_downcast;
+
namespace {
SessionItem* AddFormFactorItem(SessionItem* parent, const QString& model_type)
{
@@ -111,8 +114,7 @@ void GUIDomainSampleVisitor::visit(const Layer* sample)
layer_index == 0 ? nullptr : multilayer->layerInterface(layer_index - 1);
LayerItem* layer_item = m_sampleModel->insertItem<LayerItem>(parent);
- layer_item->setItemValue(LayerItem::P_MATERIAL,
- createMaterialFromDomain(sample->material()).variant());
+ layer_item->setMaterial(createMaterialFromDomain(sample->material()));
GUI::Transform::FromDomain::setLayerItem(layer_item, sample, top_interface);
@@ -132,9 +134,8 @@ void GUIDomainSampleVisitor::visit(const MultiLayer* sample)
void GUIDomainSampleVisitor::visit(const Particle* sample)
{
- auto particle_item = InsertIParticle(sample, "Particle");
- particle_item->setItemValue(ParticleItem::P_MATERIAL,
- createMaterialFromDomain(sample->material()).variant());
+ auto particle_item = polymorphic_downcast<ParticleItem*>(InsertIParticle(sample, "Particle"));
+ particle_item->setMaterial(createMaterialFromDomain(sample->material()));
}
void GUIDomainSampleVisitor::visit(const ParticleCoreShell* sample)
diff --git a/GUI/Models/ItemWithMaterial.cpp b/GUI/Models/ItemWithMaterial.cpp
index 1cbc1ef15e9..7da9728b87d 100644
--- a/GUI/Models/ItemWithMaterial.cpp
+++ b/GUI/Models/ItemWithMaterial.cpp
@@ -19,6 +19,26 @@ const QString ItemWithMaterial::P_MATERIAL = "Material";
ItemWithMaterial::~ItemWithMaterial() = default;
+ExternalProperty ItemWithMaterial::material() const
+{
+ return getItemValue(P_MATERIAL).value<ExternalProperty>();
+}
+
+void ItemWithMaterial::setMaterial(const ExternalProperty& material)
+{
+ setItemValue(P_MATERIAL, material.variant());
+}
+
+bool ItemWithMaterial::isMaterialPropertyName(const QString& name)
+{
+ return name == P_MATERIAL;
+}
+
+SessionItem* ItemWithMaterial::materialItem() const
+{
+ return getItem(P_MATERIAL);
+}
+
ItemWithMaterial::ItemWithMaterial(const QString& model_type) : SessionGraphicsItem(model_type)
{
addProperty(P_MATERIAL, GUI::Model::MaterialItemUtils::defaultMaterialProperty().variant())
diff --git a/GUI/Models/ItemWithMaterial.h b/GUI/Models/ItemWithMaterial.h
index 67ce71af883..797794d7701 100644
--- a/GUI/Models/ItemWithMaterial.h
+++ b/GUI/Models/ItemWithMaterial.h
@@ -16,13 +16,20 @@
#define BORNAGAIN_GUI_MODELS_ITEMWITHMATERIAL_H
#include "GUI/Models/SessionGraphicsItem.h"
+#include "GUI/Models/ExternalProperty.h"
class BA_CORE_API_ ItemWithMaterial : public SessionGraphicsItem {
-public:
+private:
static const QString P_MATERIAL;
+public:
virtual ~ItemWithMaterial() = 0;
+ ExternalProperty material() const;
+ void setMaterial(const ExternalProperty& material);
+ static bool isMaterialPropertyName(const QString& name);
+ SessionItem* materialItem() const;
+
protected:
ItemWithMaterial(const QString& model_type);
};
diff --git a/GUI/Models/LayerItem.cpp b/GUI/Models/LayerItem.cpp
index 9524c1d7db1..586b097228a 100644
--- a/GUI/Models/LayerItem.cpp
+++ b/GUI/Models/LayerItem.cpp
@@ -50,7 +50,7 @@ LayerItem::LayerItem() : ItemWithMaterial(M_TYPE)
QVector<SessionItem*> LayerItem::materialPropertyItems()
{
QVector<SessionItem*> result;
- if (auto property = getItem(LayerItem::P_MATERIAL))
+ if (auto property = materialItem())
result.push_back(property);
for (auto layout : getItems(LayerItem::T_LAYOUTS))
result.append(GUI::Model::MaterialItemUtils::materialPropertyItems(layout));
diff --git a/GUI/Models/LayerItem.h b/GUI/Models/LayerItem.h
index dfa34a6b90b..3b701d24283 100644
--- a/GUI/Models/LayerItem.h
+++ b/GUI/Models/LayerItem.h
@@ -43,7 +43,7 @@ public:
template <typename T> T* setRoughnessType();
SessionItem* roughness() const;
SessionItem* roughnessItem() const;
-
+
private:
void updateAppearance(SessionItem* new_parent);
};
diff --git a/GUI/Models/MaterialItemUtils.cpp b/GUI/Models/MaterialItemUtils.cpp
index 116f00d0895..3fcaee1ae54 100644
--- a/GUI/Models/MaterialItemUtils.cpp
+++ b/GUI/Models/MaterialItemUtils.cpp
@@ -109,19 +109,6 @@ MaterialItem* GUI::Model::MaterialItemUtils::findMaterial(const ExternalProperty
return material;
}
-//! Returns material tag for given item. Returns empty string, if item doesn't have materials.
-
-QString GUI::Model::MaterialItemUtils::materialTag(const SessionItem& item)
-{
- QString result;
- if (item.modelType() == "Particle") {
- result = ParticleItem::P_MATERIAL;
- } else if (item.hasModelType<LayerItem>()) {
- result = LayerItem::P_MATERIAL;
- }
- return result;
-}
-
//! Returns list of model types which contains registered MaterialProperty.
QStringList GUI::Model::MaterialItemUtils::materialRelatedModelTypes()
diff --git a/GUI/Models/MaterialItemUtils.h b/GUI/Models/MaterialItemUtils.h
index 70372926e5c..45510318bc2 100644
--- a/GUI/Models/MaterialItemUtils.h
+++ b/GUI/Models/MaterialItemUtils.h
@@ -36,7 +36,6 @@ std::unique_ptr<Material> createDomainMaterial(const ExternalProperty& material_
const MaterialItemContainer& container);
MaterialItem* findMaterial(const ExternalProperty& material_property);
-QString materialTag(const SessionItem& item);
QStringList materialRelatedModelTypes();
//! Constructs material property corresponding to given material.
diff --git a/GUI/Models/MaterialPropertyController.cpp b/GUI/Models/MaterialPropertyController.cpp
index a0b847af278..a497f2721e5 100644
--- a/GUI/Models/MaterialPropertyController.cpp
+++ b/GUI/Models/MaterialPropertyController.cpp
@@ -13,6 +13,7 @@
// ************************************************************************************************
#include "GUI/Models/MaterialPropertyController.h"
+#include "GUI/Models/ItemWithMaterial.h"
#include "GUI/Models/MaterialItemUtils.h"
#include "GUI/Models/MaterialModel.h"
#include "GUI/Models/ModelPath.h"
@@ -20,6 +21,9 @@
#include "GUI/Models/SampleModel.h"
#include <QVector>
+#include <boost/polymorphic_cast.hpp>
+using boost::polymorphic_downcast;
+
MaterialPropertyController::MaterialPropertyController(QObject* parent)
: QObject(parent), m_materialModel(nullptr), m_sampleModel(nullptr)
{
@@ -45,19 +49,16 @@ void MaterialPropertyController::setModels(MaterialModel* materialModel, SampleM
void MaterialPropertyController::onMaterialModelLoad()
{
- for (auto sampleItem : relatedSampleItems()) {
- QString tag = GUI::Model::MaterialItemUtils::materialTag(*sampleItem);
- ASSERT(!tag.isEmpty());
-
- ExternalProperty property = sampleItem->getItemValue(tag).value<ExternalProperty>();
+ for (ItemWithMaterial* sampleItem : relatedSampleItems()) {
+ ExternalProperty property = sampleItem->material();
if (MaterialItem* material =
m_materialModel->materialFromIdentifier(property.identifier())) {
ExternalProperty new_property =
GUI::Model::MaterialItemUtils::materialProperty(*material);
- sampleItem->setItemValue(tag, new_property.variant());
+ sampleItem->setMaterial(new_property);
} else {
ExternalProperty undefined;
- sampleItem->setItemValue(tag, undefined.variant());
+ sampleItem->setMaterial(undefined);
}
}
}
@@ -71,15 +72,12 @@ void MaterialPropertyController::onMaterialDataChanged(const QModelIndex& topLef
if (auto materialItem = dynamic_cast<const MaterialItem*>(
GUI::Model::Path::ancestor(changedItem, MaterialItem::M_TYPE))) {
- for (auto sampleItem : relatedSampleItems()) {
- QString tag = GUI::Model::MaterialItemUtils::materialTag(*sampleItem);
- ASSERT(!tag.isEmpty());
-
- ExternalProperty property = sampleItem->getItemValue(tag).value<ExternalProperty>();
+ for (ItemWithMaterial* sampleItem : relatedSampleItems()) {
+ ExternalProperty property = sampleItem->material();
if (property.identifier() == materialItem->identifier()) {
ExternalProperty new_property =
GUI::Model::MaterialItemUtils::materialProperty(*materialItem);
- sampleItem->setItemValue(tag, new_property.variant());
+ sampleItem->setMaterial(new_property);
}
}
}
@@ -103,32 +101,30 @@ void MaterialPropertyController::onMaterialRowsAboutToBeRemoved(const QModelInde
}
// rewriting MaterialProperty in corresponding sample items
- for (auto sampleItem : relatedSampleItems()) {
- QString tag = GUI::Model::MaterialItemUtils::materialTag(*sampleItem);
- ASSERT(!tag.isEmpty());
-
- ExternalProperty property = sampleItem->getItemValue(tag).value<ExternalProperty>();
+ for (ItemWithMaterial* sampleItem : relatedSampleItems()) {
+ ExternalProperty property = sampleItem->material();
if (identifiersToDelete.contains(property.identifier())) {
ExternalProperty undefined;
- sampleItem->setItemValue(tag, undefined.variant());
+ sampleItem->setMaterial(undefined);
}
}
}
//! Returns vector of SessionItems having MaterialProperty on board.
-QVector<SessionItem*> MaterialPropertyController::relatedSampleItems()
+QVector<ItemWithMaterial*> MaterialPropertyController::relatedSampleItems()
{
- static QStringList materialRelated = GUI::Model::MaterialItemUtils::materialRelatedModelTypes();
+ static const QStringList materialRelated =
+ GUI::Model::MaterialItemUtils::materialRelatedModelTypes();
- QVector<SessionItem*> result;
+ QVector<ItemWithMaterial*> result;
GUI::Model::ItemUtils::iterate(QModelIndex(), m_sampleModel, [&](const QModelIndex& index) {
if (index.column() != 0)
return;
if (SessionItem* item = m_sampleModel->itemForIndex(index))
if (materialRelated.contains(item->modelType()))
- result.push_back(item);
+ result.push_back(polymorphic_downcast<ItemWithMaterial*>(item));
});
return result;
diff --git a/GUI/Models/MaterialPropertyController.h b/GUI/Models/MaterialPropertyController.h
index 634b4e9f3e8..8f77ce2804a 100644
--- a/GUI/Models/MaterialPropertyController.h
+++ b/GUI/Models/MaterialPropertyController.h
@@ -17,6 +17,7 @@
#include <QObject>
+class ItemWithMaterial;
class MaterialModel;
class SampleModel;
class SessionItem;
@@ -39,7 +40,7 @@ private slots:
void onMaterialRowsAboutToBeRemoved(const QModelIndex& parent, int first, int last);
private:
- QVector<SessionItem*> relatedSampleItems();
+ QVector<ItemWithMaterial*> relatedSampleItems();
MaterialModel* m_materialModel;
SampleModel* m_sampleModel;
diff --git a/GUI/Models/ParticleItem.cpp b/GUI/Models/ParticleItem.cpp
index 0f8e21bd4ec..573c16eddea 100644
--- a/GUI/Models/ParticleItem.cpp
+++ b/GUI/Models/ParticleItem.cpp
@@ -74,7 +74,7 @@ std::unique_ptr<Particle> ParticleItem::createParticle() const
QVector<SessionItem*> ParticleItem::materialPropertyItems()
{
- auto item = getItem(P_MATERIAL);
+ auto item = materialItem();
if (!item)
return {};
return {item};
diff --git a/GUI/Models/TransformToDomain.cpp b/GUI/Models/TransformToDomain.cpp
index 703c8ece2c5..9f7189099b7 100644
--- a/GUI/Models/TransformToDomain.cpp
+++ b/GUI/Models/TransformToDomain.cpp
@@ -48,13 +48,13 @@ void setParameterDistributionToSimulation(ParameterDistribution::WhichParameter
std::unique_ptr<ScanResolution> createScanResolution(const SessionItem* item);
} // namespace
-std::unique_ptr<Material> GUI::Transform::ToDomain::createDomainMaterial(const SessionItem& item)
+std::unique_ptr<Material> GUI::Transform::ToDomain::createDomainMaterial
+(const ItemWithMaterial& item)
{
auto parent_job = GUI::Model::JobFunctions::findJobItem(&item);
const MaterialItemContainer* container =
parent_job ? parent_job->materialContainerItem() : nullptr;
- QString tag = GUI::Model::MaterialItemUtils::materialTag(item);
- ExternalProperty property = item.getItemValue(tag).value<ExternalProperty>();
+ ExternalProperty property = item.material();
return container ? GUI::Model::MaterialItemUtils::createDomainMaterial(property, *container)
: GUI::Model::MaterialItemUtils::createDomainMaterial(property);
}
diff --git a/GUI/Models/TransformToDomain.h b/GUI/Models/TransformToDomain.h
index 77fb11b047b..d07bf70d0e4 100644
--- a/GUI/Models/TransformToDomain.h
+++ b/GUI/Models/TransformToDomain.h
@@ -30,6 +30,7 @@ class AngularSpecScan;
class BeamDistributionItem;
class BeamItem;
class GISASSimulation;
+class ItemWithMaterial;
class LayerItem;
class Material;
class MaterialItemContainer;
@@ -38,7 +39,7 @@ class ISimulation;
namespace GUI::Transform::ToDomain {
-std::unique_ptr<Material> createDomainMaterial(const SessionItem& item);
+std::unique_ptr<Material> createDomainMaterial(const ItemWithMaterial& item);
std::unique_ptr<IParticle> createIParticle(const SessionItem& item);
std::unique_ptr<Layer> createLayer(const LayerItem& item);
std::unique_ptr<LayerRoughness> createLayerRoughness(const SessionItem& item);
diff --git a/GUI/Views/RealSpaceWidgets/TransformTo3D.cpp b/GUI/Views/RealSpaceWidgets/TransformTo3D.cpp
index 3b6589357e6..1423a166237 100644
--- a/GUI/Views/RealSpaceWidgets/TransformTo3D.cpp
+++ b/GUI/Views/RealSpaceWidgets/TransformTo3D.cpp
@@ -66,7 +66,7 @@ GUI::View::TransformTo3D::createLayer(const LayerItem& layerItem,
GUI::RealSpace::Range(static_cast<float>(-s2), static_cast<float>(+s2)),
GUI::RealSpace::Range(static_cast<float>(ztop), static_cast<float>(zbottom))));
- QColor color = layerItem.getItemValue(LayerItem::P_MATERIAL).value<ExternalProperty>().color();
+ QColor color = layerItem.material().color();
color.setAlphaF(.3);
result->color = color;
diff --git a/GUI/Views/SampleDesigner/ILayerView.cpp b/GUI/Views/SampleDesigner/ILayerView.cpp
index fa3b4146ace..2e477056237 100644
--- a/GUI/Views/SampleDesigner/ILayerView.cpp
+++ b/GUI/Views/SampleDesigner/ILayerView.cpp
@@ -52,7 +52,7 @@ void ILayerView::onPropertyChange(const QString& propertyName)
{
if (LayerItem::isThicknessPropertyName(propertyName)) {
updateHeight();
- } else if (propertyName == LayerItem::P_MATERIAL) {
+ } else if (LayerItem::isMaterialPropertyName(propertyName)) {
updateColor();
updateLabel();
}
@@ -72,15 +72,9 @@ void ILayerView::updateHeight()
void ILayerView::updateColor()
{
- if (m_item->isTag(LayerItem::P_MATERIAL)) {
- QVariant v = m_item->getItemValue(LayerItem::P_MATERIAL);
- if (v.isValid()) {
- ExternalProperty mp = v.value<ExternalProperty>();
- setColor(mp.color());
- update();
- } else {
- ASSERT(0);
- }
+ if (LayerItem* item = dynamic_cast<LayerItem*>(m_item)) {
+ setColor(item->material().color());
+ update();
}
}
@@ -92,13 +86,8 @@ void ILayerView::updateLabel()
NodeEditorPort* port = getInputPorts()[0];
QString material = "";
- if (m_item->isTag(LayerItem::P_MATERIAL)) {
- QVariant v = m_item->getItemValue(LayerItem::P_MATERIAL);
- if (v.isValid()) {
- ExternalProperty mp = v.value<ExternalProperty>();
- material = mp.text();
- }
- }
+ if (LayerItem* item = dynamic_cast<LayerItem*>(m_item))
+ material = item->material().text();
/* Thickness and roughness can be added, but the length of the string
* becomes prohibitive.
diff --git a/Tests/UnitTests/GUI/TestComponentUtils.cpp b/Tests/UnitTests/GUI/TestComponentUtils.cpp
index 1cd6affef66..afa3d1de984 100644
--- a/Tests/UnitTests/GUI/TestComponentUtils.cpp
+++ b/Tests/UnitTests/GUI/TestComponentUtils.cpp
@@ -19,7 +19,7 @@ TEST_F(TestComponentUtils, test_componentItems)
SessionItem* ffItem = particle->getGroupItem(ParticleItem::P_FORM_FACTOR);
QList<const SessionItem*> expectedList = QList<const SessionItem*>()
- << particle->getItem(ParticleItem::P_MATERIAL)
+ << particle->materialItem()
<< group << ffItem->getItem(CylinderItem::P_RADIUS)
<< ffItem->getItem(CylinderItem::P_HEIGHT)
<< particle->getItem(ParticleItem::P_ABUNDANCE)
@@ -41,7 +41,7 @@ TEST_F(TestComponentUtils, test_componentItemsFFChange)
SessionItem* sphereItem = particle->getGroupItem(ParticleItem::P_FORM_FACTOR);
QList<const SessionItem*> expectedList =
- QList<const SessionItem*>() << particle->getItem(ParticleItem::P_MATERIAL)
+ QList<const SessionItem*>() << particle->materialItem()
<< group << sphereItem->getItem(FullSphereItem::P_RADIUS)
<< particle->getItem(ParticleItem::P_ABUNDANCE)
<< particle->getItem(ParticleItem::P_POSITION);
diff --git a/Tests/UnitTests/GUI/TestLayerItems.cpp b/Tests/UnitTests/GUI/TestLayerItems.cpp
index b115c40a580..ee24f4d36c6 100644
--- a/Tests/UnitTests/GUI/TestLayerItems.cpp
+++ b/Tests/UnitTests/GUI/TestLayerItems.cpp
@@ -18,8 +18,7 @@ TEST_F(TestLayerItems, test_LayerDefaultMaterial)
auto materials = models.materialModel()->topItems<MaterialItem>();
auto defMaterial = materials.front();
- ExternalProperty material =
- layer->getItemValue(LayerItem::P_MATERIAL).value<ExternalProperty>();
+ ExternalProperty material = layer->material();
EXPECT_EQ(material.text(), "Default");
EXPECT_EQ(material.identifier(), defMaterial->identifier());
}
diff --git a/Tests/UnitTests/GUI/TestMaterialPropertyController.cpp b/Tests/UnitTests/GUI/TestMaterialPropertyController.cpp
index 13a5f7c8f37..7e6a086c451 100644
--- a/Tests/UnitTests/GUI/TestMaterialPropertyController.cpp
+++ b/Tests/UnitTests/GUI/TestMaterialPropertyController.cpp
@@ -74,12 +74,9 @@ TEST_F(TestMaterialPropertyController, test_ControllerInEditorContext)
MaterialPropertyController controller;
controller.setModels(&materialModel, &sampleModel);
- layer1->setItemValue(LayerItem::P_MATERIAL,
- GUI::Model::MaterialItemUtils::materialProperty(*mat1).variant());
- layer2->setItemValue(LayerItem::P_MATERIAL,
- GUI::Model::MaterialItemUtils::materialProperty(*mat2).variant());
- layer3->setItemValue(LayerItem::P_MATERIAL,
- GUI::Model::MaterialItemUtils::materialProperty(*mat3).variant());
+ layer1->setMaterial(GUI::Model::MaterialItemUtils::materialProperty(*mat1));
+ layer2->setMaterial(GUI::Model::MaterialItemUtils::materialProperty(*mat2));
+ layer3->setMaterial(GUI::Model::MaterialItemUtils::materialProperty(*mat3));
// Making copy of material model
std::unique_ptr<MaterialModel> materialsCopy(materialModel.createCopy());
@@ -118,11 +115,10 @@ TEST_F(TestMaterialPropertyController, test_ControllerInEditorContext)
materialModel.modelLoaded();
// layer2 should have undefined material property
- ExternalProperty property =
- layer2->getItemValue(LayerItem::P_MATERIAL).value<ExternalProperty>();
+ ExternalProperty property = layer2->material();
EXPECT_FALSE(property.isValid());
// layer3 should have different MaterialProperty name
- property = layer3->getItemValue(LayerItem::P_MATERIAL).value<ExternalProperty>();
+ property = layer3->material();
EXPECT_EQ(property.text(), "name3changed");
}
--
GitLab