From bcbd538d0a95900bbb90b3877996a7d6b7390bd0 Mon Sep 17 00:00:00 2001
From: Andrew Tribick <ajtribick@googlemail.com>
Date: Sun, 28 Nov 2021 16:25:13 +0100
Subject: [PATCH] Remove const_cast in cmodview by implementing
 Model::setMaterial

---
 src/celmodel/model.cpp                      | 19 ++++++++++++++++++-
 src/celmodel/model.h                        |  2 +-
 src/tools/cmod/cmodview/materialwidget.cpp  | 15 +--------------
 src/tools/cmod/cmodview/modelviewwidget.cpp | 14 +-------------
 4 files changed, 21 insertions(+), 29 deletions(-)

diff --git a/src/celmodel/model.cpp b/src/celmodel/model.cpp
index 35b4f2d50..ce3f77b66 100644
--- a/src/celmodel/model.cpp
+++ b/src/celmodel/model.cpp
@@ -69,7 +69,7 @@ Model::addMaterial(Material&& m)
     // the model, we could potentially end up with false positives--this
     // won't cause any rendering troubles, but could hurt performance
     // if it forces multipass rendering when it's not required.
-    for (int i = 0; i < static_cast<int>(TextureSemantic::TextureSemanticMax); i++)
+    for (int i = 0; i < static_cast<int>(TextureSemantic::TextureSemanticMax); ++i)
     {
         if (m.maps[i] != InvalidResource)
         {
@@ -82,6 +82,23 @@ Model::addMaterial(Material&& m)
 }
 
 
+bool
+Model::setMaterial(unsigned int index, Material&& m)
+{
+    if (index >= materials.size()) { return false; }
+    materials[index] = std::move(m);
+
+    // Regenerate the texture map usage for the model by rescanning all the meshes.
+    for (int i = 0; i < static_cast<int>(TextureSemantic::TextureSemanticMax); ++i)
+    {
+        textureUsage[i] = std::any_of(materials.cbegin(), materials.cend(),
+                                      [&](const Material& mat) { return mat.maps[i] != InvalidResource; });
+    }
+
+    return true;
+}
+
+
 unsigned int
 Model::getMaterialCount() const
 {
diff --git a/src/celmodel/model.h b/src/celmodel/model.h
index 40a986756..f5f2190eb 100644
--- a/src/celmodel/model.h
+++ b/src/celmodel/model.h
@@ -39,7 +39,7 @@ class Model
     ~Model() = default;
 
     const Material* getMaterial(unsigned int index) const;
-    void setMaterial(unsigned int index, const Material* material);
+    bool setMaterial(unsigned int index, Material&& material);
     unsigned int addMaterial(Material&& material);
 
     /*! Return the number of materials in the model
diff --git a/src/tools/cmod/cmodview/materialwidget.cpp b/src/tools/cmod/cmodview/materialwidget.cpp
index 687e512ef..c0f67ca93 100644
--- a/src/tools/cmod/cmodview/materialwidget.cpp
+++ b/src/tools/cmod/cmodview/materialwidget.cpp
@@ -35,19 +35,6 @@ cmod::Color fromQtColor(const QColor& color)
 }
 
 
-// TODO: implement a copy constructor and assignment operator for materials
-void copyMaterial(cmod::Material& dest, const cmod::Material& src)
-{
-    dest.diffuse       = src.diffuse;
-    dest.specular      = src.specular;
-    dest.emissive      = src.emissive;
-    dest.opacity       = src.opacity;
-    dest.specularPower = src.specularPower;
-    dest.blend         = src.blend;
-    dest.maps          = src.maps;
-}
-
-
 static void setWidgetColor(QLabel* widget, const cmod::Color& color)
 {
     widget->setPalette(QPalette(toQtColor(color)));
@@ -193,7 +180,7 @@ MaterialWidget::MaterialWidget(QWidget* parent) :
 void
 MaterialWidget::setMaterial(const cmod::Material& material)
 {
-    copyMaterial(m_material, material);
+    m_material = material.clone();
 
     setWidgetColor(m_diffuseColor, m_material.diffuse);
     setWidgetColor(m_specularColor, m_material.specular);
diff --git a/src/tools/cmod/cmodview/modelviewwidget.cpp b/src/tools/cmod/cmodview/modelviewwidget.cpp
index fa0deeeca..d0125b56b 100644
--- a/src/tools/cmod/cmodview/modelviewwidget.cpp
+++ b/src/tools/cmod/cmodview/modelviewwidget.cpp
@@ -519,19 +519,7 @@ ModelViewWidget::setMaterial(unsigned int index, const cmod::Material& material)
     }
 
     // Copy material parameters
-    // TODO: eliminate const cast when Model::setMaterial() is implemented
-    cmod::Material* modelMaterial = const_cast<cmod::Material*>(m_model->getMaterial(index));
-    modelMaterial->diffuse = material.diffuse;
-    modelMaterial->specular = material.specular;
-    modelMaterial->emissive = material.emissive;
-    modelMaterial->opacity = material.opacity;
-    modelMaterial->specularPower = material.specularPower;
-
-    modelMaterial->setMap(cmod::TextureSemantic::DiffuseMap, material.getMap(cmod::TextureSemantic::DiffuseMap));
-    modelMaterial->setMap(cmod::TextureSemantic::SpecularMap, material.getMap(cmod::TextureSemantic::SpecularMap));
-    modelMaterial->setMap(cmod::TextureSemantic::NormalMap, material.getMap(cmod::TextureSemantic::NormalMap));
-    modelMaterial->setMap(cmod::TextureSemantic::EmissiveMap, material.getMap(cmod::TextureSemantic::EmissiveMap));
-
+    m_model->setMaterial(index, material.clone());
     update();
 }