Skip to content

Commit

Permalink
Material: Compatibility with older FCMat files
Browse files Browse the repository at this point in the history
Provides compatibility loading older files outside the context of
a library.

Older material files were loaded by specifying a path. The new
material system used the path to associated the material with a
library, which may not be appropriate for legacy files. This change
allows the use of materials outside of a library.

Additionally, legacy files often have name/value pairs not part of the
standard list of properties. Since these were unable to be mapped to
a model property they were ignored. Materials now maintain a legacy
map to hold properties not associated with a property model. These
properties are considered transient and will not be saved. It is not
intended for this feature to be used as a generic container for
properties not mapped to an appropriate model.

Fixes #13302
  • Loading branch information
davesrocketshop authored and yorikvanhavre committed Apr 15, 2024
1 parent 0056038 commit f950a0c
Show file tree
Hide file tree
Showing 8 changed files with 135 additions and 0 deletions.
17 changes: 17 additions & 0 deletions src/Mod/Material/App/MaterialConfigLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1017,6 +1017,22 @@ void MaterialConfigLoader::addMechanical(const QMap<QString, QString>& fcmat,
setPhysicalValue(finalModel, "Stiffness", stiffness);
}

void MaterialConfigLoader::addLegacy(const QMap<QString, QString>& fcmat,
const std::shared_ptr<Material>& finalModel)
{
for (auto const& legacy : fcmat.keys()) {

Check warning on line 1023 in src/Mod/Material/App/MaterialConfigLoader.cpp

View workflow job for this annotation

GitHub Actions / Lint / Lint

allocating an unneeded temporary container [-Wclazy-container-anti-pattern]

Check warning on line 1023 in src/Mod/Material/App/MaterialConfigLoader.cpp

View workflow job for this annotation

GitHub Actions / Lint / Lint

c++11 range-loop might detach Qt container (QList) [-Wclazy-range-loop-detach]
auto name = legacy;
int last = name.lastIndexOf(QLatin1String("/"));
if (last > 0) {
name = name.mid(last + 1);
}

if (!finalModel->hasNonLegacyProperty(name)) {
setLegacyValue(finalModel, name.toStdString(), fcmat[legacy]);
}
}
}

std::shared_ptr<Material>
MaterialConfigLoader::getMaterialFromPath(const std::shared_ptr<MaterialLibrary>& library,
const QString& path)
Expand Down Expand Up @@ -1081,6 +1097,7 @@ MaterialConfigLoader::getMaterialFromPath(const std::shared_ptr<MaterialLibrary>
addRendering(fcmat, finalModel);
addVectorRendering(fcmat, finalModel);
addRenderWB(fcmat, finalModel);
addLegacy(fcmat, finalModel);

return finalModel;
}
10 changes: 10 additions & 0 deletions src/Mod/Material/App/MaterialConfigLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,14 @@ class MaterialConfigLoader
finalModel->setAppearanceValue(QString::fromStdString(name), value);
}
}
static void setLegacyValue(const std::shared_ptr<Material>& finalModel,
const std::string& name,
const QString& value)
{
if (!value.isEmpty()) {
finalModel->setLegacyValue(QString::fromStdString(name), value);
}
}

static bool isTexture(const QString& value)
{
Expand Down Expand Up @@ -147,6 +155,8 @@ class MaterialConfigLoader
const std::shared_ptr<Material>& finalModel);
static void addRenderWB(QMap<QString, QString>& fcmat,
const std::shared_ptr<Material>& finalModel);
static void addLegacy(const QMap<QString, QString>& fcmat,
const std::shared_ptr<Material>& finalModel);
};

} // namespace Materials
Expand Down
11 changes: 11 additions & 0 deletions src/Mod/Material/App/MaterialManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,17 @@ std::shared_ptr<Material> MaterialManager::getMaterialByPath(const QString& path
}
}

// Older workbenches may try files outside the context of a library
{
QMutexLocker locker(&_mutex);

if (MaterialConfigLoader::isConfigStyle(path)) {
auto material = MaterialConfigLoader::getMaterialFromPath(nullptr, path);

return material;
}
}

throw MaterialNotFound();
}

Expand Down
11 changes: 11 additions & 0 deletions src/Mod/Material/App/MaterialPy.xml
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,11 @@
<UserDocu>Check if the material implements the appearance property with the given name</UserDocu>
</Documentation>
</Methode>
<Methode Name="hasLegacyProperties" ReadOnly="true">
<Documentation>
<UserDocu>Returns true of there are legacy properties</UserDocu>
</Documentation>
</Methode>
<Attribute Name="Properties" ReadOnly="true">
<Documentation>
<UserDocu>deprecated -- Dictionary of all material properties.</UserDocu>
Expand All @@ -159,6 +164,12 @@
</Documentation>
<Parameter Name="AppearanceProperties" Type="Dict"/>
</Attribute>
<Attribute Name="LegacyProperties" ReadOnly="true">
<Documentation>
<UserDocu>deprecated -- Dictionary of material legacy properties.</UserDocu>
</Documentation>
<Parameter Name="LegacyProperties" Type="Dict"/>
</Attribute>
<Methode Name="getPhysicalValue" ReadOnly="true">
<Documentation>
<UserDocu>Get the value associated with the property</UserDocu>
Expand Down
37 changes: 37 additions & 0 deletions src/Mod/Material/App/MaterialPyImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,16 @@ PyObject* MaterialPy::hasAppearanceProperty(PyObject* args)
return PyBool_FromLong(hasProperty ? 1 : 0);
}

PyObject* MaterialPy::hasLegacyProperties(PyObject* args)
{
if (!PyArg_ParseTuple(args, "")) {
return nullptr;
}

bool hasProperty = getMaterialPtr()->hasLegacyProperties();
return PyBool_FromLong(hasProperty ? 1 : 0);
}

Py::Dict MaterialPy::getProperties() const
{
Py::Dict dict;
Expand Down Expand Up @@ -319,6 +329,16 @@ Py::Dict MaterialPy::getProperties() const
}
}

auto legacy = getMaterialPtr()->getLegacyProperties();
for (auto& it : legacy) {
auto key = it.first;
auto value = it.second;

if (!value.isEmpty()) {
dict.setItem(Py::String(key.toStdString()), Py::String(value.toStdString()));
}
}

return dict;
}

Expand Down Expand Up @@ -358,6 +378,23 @@ Py::Dict MaterialPy::getAppearanceProperties() const
return dict;
}

Py::Dict MaterialPy::getLegacyProperties() const
{
Py::Dict dict;

auto legacy = getMaterialPtr()->getLegacyProperties();
for (auto& it : legacy) {
auto key = it.first;
auto value = it.second;

if (!value.isEmpty()) {
dict.setItem(Py::String(key.toStdString()), Py::String(value.toStdString()));
}
}

return dict;
}

static Py::List getList(const QVariant& value)
{
auto listValue = value.value<QList<QVariant>>();
Expand Down
27 changes: 27 additions & 0 deletions src/Mod/Material/App/Materials.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,9 @@ Material::Material(const Material& other)
MaterialProperty prop(it.second);
_appearance[it.first] = std::make_shared<MaterialProperty>(prop);
}
for (auto& it : other._legacy) {
_legacy[it.first] = it.second;
}
}

QString Material::getAuthorAndLicense() const
Expand Down Expand Up @@ -890,6 +893,13 @@ void Material::setAppearanceValue(const QString& name,
}
}

void Material::setLegacyValue(const QString& name, const QString& value)
{
setEditStateAlter();

_legacy[name] = value;
}

std::shared_ptr<MaterialProperty> Material::getPhysicalProperty(const QString& name)
{
try {
Expand Down Expand Up @@ -1047,6 +1057,19 @@ bool Material::hasAppearanceProperty(const QString& name) const
return true;
}

bool Material::hasNonLegacyProperty(const QString& name) const
{
if (hasPhysicalProperty(name) || hasAppearanceProperty(name)) {
return true;
}
return false;
}

bool Material::hasLegacyProperties() const
{
return !_legacy.empty();
}

bool Material::isInherited(const QString& uuid) const
{
if (_physicalUuids.contains(uuid)) {
Expand Down Expand Up @@ -1464,6 +1487,10 @@ Material& Material::operator=(const Material& other)
MaterialProperty prop(it.second);
_appearance[it.first] = std::make_shared<MaterialProperty>(prop);
}
_legacy.clear();
for (auto& it : other._legacy) {
_legacy[it.first] = it.second;
}

return *this;
}
Expand Down
17 changes: 17 additions & 0 deletions src/Mod/Material/App/Materials.h
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,15 @@ class MaterialsExport Material: public Base::BaseClass
void setAppearanceValue(const QString& name, const std::shared_ptr<MaterialValue>& value);
void setAppearanceValue(const QString& name, const std::shared_ptr<QList<QVariant>>& value);

/*
* Legacy values are thosed contained in old format files that don't fit in the new
* property format. It should not be used as a catch all for defining a property with
* no model.
*

Check warning on line 306 in src/Mod/Material/App/Materials.h

View workflow job for this annotation

GitHub Actions / Lint / Lint

* <-- trailing whitespace
* These values are transient and will not be saved.
*/
void setLegacyValue(const QString& name, const QString& value);

std::shared_ptr<MaterialProperty> getPhysicalProperty(const QString& name);
std::shared_ptr<MaterialProperty> getPhysicalProperty(const QString& name) const;
std::shared_ptr<MaterialProperty> getAppearanceProperty(const QString& name);
Expand All @@ -313,6 +322,9 @@ class MaterialsExport Material: public Base::BaseClass
QString getAppearanceValueString(const QString& name) const;
bool hasPhysicalProperty(const QString& name) const;
bool hasAppearanceProperty(const QString& name) const;
bool hasNonLegacyProperty(const QString& name) const;
bool hasLegacyProperty(const QString& name) const;
bool hasLegacyProperties() const;

// Test if the model is defined, and if values are provided for all properties
bool hasModel(const QString& uuid) const;
Expand All @@ -334,6 +346,10 @@ class MaterialsExport Material: public Base::BaseClass
{
return _appearance;
}
std::map<QString, QString>& getLegacyProperties()
{
return _legacy;
}

QString getModelByName(const QString& name) const;

Expand Down Expand Up @@ -438,6 +454,7 @@ class MaterialsExport Material: public Base::BaseClass
QSet<QString> _allUuids; // Includes inherited models
std::map<QString, std::shared_ptr<MaterialProperty>> _physical;
std::map<QString, std::shared_ptr<MaterialProperty>> _appearance;
std::map<QString, QString> _legacy;
bool _dereferenced;
bool _oldFormat;
ModelEdit _editState;
Expand Down
5 changes: 5 additions & 0 deletions src/Mod/Material/materialtests/TestMaterials.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ def testCalculiXSteel(self):
self.assertFalse(steel.isPhysicalModelComplete(self.uuids.LinearElastic))
self.assertTrue(steel.isAppearanceModelComplete(self.uuids.BasicRendering))

self.assertFalse(steel.hasLegacyProperties())

self.assertTrue(steel.hasPhysicalProperty("Density"))
self.assertTrue(steel.hasPhysicalProperty("BulkModulus"))
self.assertTrue(steel.hasPhysicalProperty("PoissonRatio"))
Expand Down Expand Up @@ -118,6 +120,9 @@ def testCalculiXSteel(self):
self.assertIn("SpecularColor", properties)
self.assertIn("Transparency", properties)

properties = steel.LegacyProperties
self.assertEqual(len(properties), 0)

properties = steel.Properties
self.assertIn("Density", properties)
self.assertNotIn("BulkModulus", properties)
Expand Down

0 comments on commit f950a0c

Please sign in to comment.