From 9a16a7108fe50a05e3b490a3c92f0605d3b42c6b Mon Sep 17 00:00:00 2001 From: wmayer Date: Fri, 5 Apr 2024 10:56:51 +0200 Subject: [PATCH 01/16] App: Fix linter warnings * fix readability-uppercase-literal-suffix * fix readability-avoid-const-params-in-decls * fix cppcoreguidelines-special-member-functions * fix cppcoreguidelines-pro-type-member-init * fix modernize-use-equals-default --- src/App/Material.cpp | 309 ++++++++++++++++++++----------------------- src/App/Material.h | 25 +++- 2 files changed, 160 insertions(+), 174 deletions(-) diff --git a/src/App/Material.cpp b/src/App/Material.cpp index 3a34d208d2..8962dc33d7 100644 --- a/src/App/Material.cpp +++ b/src/App/Material.cpp @@ -32,61 +32,35 @@ using namespace App; +// NOLINTBEGIN(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) //=========================================================================== // Material //=========================================================================== Material::Material() - : shininess {0.9000f} + : shininess {0.9000F} , transparency {} + , _matType {USER_DEFINED} { setType(STEEL); setType(USER_DEFINED); } -Material::Material(const Material& other) - : ambientColor(other.ambientColor) - , diffuseColor(other.diffuseColor) - , specularColor(other.specularColor) - , emissiveColor(other.emissiveColor) - , shininess(other.shininess) - , transparency(other.transparency) - , uuid(other.uuid) - , _matType(other._matType) -{ -} - Material::Material(const char* MatName) - : shininess {0.9000f} + : shininess {0.9000F} , transparency {} + , _matType {USER_DEFINED} { set(MatName); } -Material::Material(const MaterialType MatType) - : shininess {0.9000f} +Material::Material(MaterialType MatType) + : shininess {0.9000F} , transparency {} + , _matType {USER_DEFINED} { setType(MatType); } -Material& Material::operator=(const Material& other) -{ - if (this == &other) { - return *this; - } - - _matType = other._matType; - ambientColor = other.ambientColor; - diffuseColor = other.diffuseColor; - specularColor = other.specularColor; - emissiveColor = other.emissiveColor; - shininess = other.shininess; - transparency = other.transparency; - uuid = other.uuid; - - return *this; -} - void Material::set(const char* MatName) { if (strcmp("Brass", MatName) == 0) { @@ -160,187 +134,188 @@ void Material::set(const char* MatName) } } -void Material::setType(const MaterialType MatType) +void Material::setType(MaterialType MatType) { _matType = MatType; switch (MatType) { case BRASS: - ambientColor.set(0.3294f, 0.2235f, 0.0275f); - diffuseColor.set(0.7804f, 0.5686f, 0.1137f); - specularColor.set(0.9922f, 0.9412f, 0.8078f); - emissiveColor.set(0.0000f, 0.0000f, 0.0000f); - shininess = 0.2179f; - transparency = 0.0000f; + ambientColor.set(0.3294F, 0.2235F, 0.0275F); + diffuseColor.set(0.7804F, 0.5686F, 0.1137F); + specularColor.set(0.9922F, 0.9412F, 0.8078F); + emissiveColor.set(0.0000F, 0.0000F, 0.0000F); + shininess = 0.2179F; + transparency = 0.0000F; break; case BRONZE: - ambientColor.set(0.2125f, 0.1275f, 0.0540f); - diffuseColor.set(0.7140f, 0.4284f, 0.1814f); - specularColor.set(0.3935f, 0.2719f, 0.1667f); - emissiveColor.set(0.0000f, 0.0000f, 0.0000f); - shininess = 0.2000f; - transparency = 0.0000f; + ambientColor.set(0.2125F, 0.1275F, 0.0540F); + diffuseColor.set(0.7140F, 0.4284F, 0.1814F); + specularColor.set(0.3935F, 0.2719F, 0.1667F); + emissiveColor.set(0.0000F, 0.0000F, 0.0000F); + shininess = 0.2000F; + transparency = 0.0000F; break; case COPPER: - ambientColor.set(0.3300f, 0.2600f, 0.2300f); - diffuseColor.set(0.5000f, 0.1100f, 0.0000f); - specularColor.set(0.9500f, 0.7300f, 0.0000f); - emissiveColor.set(0.0000f, 0.0000f, 0.0000f); - shininess = 0.9300f; - transparency = 0.0000f; + ambientColor.set(0.3300F, 0.2600F, 0.2300F); + diffuseColor.set(0.5000F, 0.1100F, 0.0000F); + specularColor.set(0.9500F, 0.7300F, 0.0000F); + emissiveColor.set(0.0000F, 0.0000F, 0.0000F); + shininess = 0.9300F; + transparency = 0.0000F; break; case GOLD: - ambientColor.set(0.3000f, 0.2306f, 0.0953f); - diffuseColor.set(0.4000f, 0.2760f, 0.0000f); - specularColor.set(0.9000f, 0.8820f, 0.7020f); - emissiveColor.set(0.0000f, 0.0000f, 0.0000f); - shininess = 0.0625f; - transparency = 0.0000f; + ambientColor.set(0.3000F, 0.2306F, 0.0953F); + diffuseColor.set(0.4000F, 0.2760F, 0.0000F); + specularColor.set(0.9000F, 0.8820F, 0.7020F); + emissiveColor.set(0.0000F, 0.0000F, 0.0000F); + shininess = 0.0625F; + transparency = 0.0000F; break; case PEWTER: - ambientColor.set(0.1059f, 0.0588f, 0.1137f); - diffuseColor.set(0.4275f, 0.4706f, 0.5412f); - specularColor.set(0.3333f, 0.3333f, 0.5216f); - emissiveColor.set(0.0000f, 0.0000f, 0.0000f); - shininess = 0.0769f; - transparency = 0.0000f; + ambientColor.set(0.1059F, 0.0588F, 0.1137F); + diffuseColor.set(0.4275F, 0.4706F, 0.5412F); + specularColor.set(0.3333F, 0.3333F, 0.5216F); + emissiveColor.set(0.0000F, 0.0000F, 0.0000F); + shininess = 0.0769F; + transparency = 0.0000F; break; case PLASTER: - ambientColor.set(0.0500f, 0.0500f, 0.0500f); - diffuseColor.set(0.1167f, 0.1167f, 0.1167f); - specularColor.set(0.0305f, 0.0305f, 0.0305f); - emissiveColor.set(0.0000f, 0.0000f, 0.0000f); - shininess = 0.0078f; - transparency = 0.0000f; + ambientColor.set(0.0500F, 0.0500F, 0.0500F); + diffuseColor.set(0.1167F, 0.1167F, 0.1167F); + specularColor.set(0.0305F, 0.0305F, 0.0305F); + emissiveColor.set(0.0000F, 0.0000F, 0.0000F); + shininess = 0.0078F; + transparency = 0.0000F; break; case PLASTIC: - ambientColor.set(0.1000f, 0.1000f, 0.1000f); - diffuseColor.set(0.0000f, 0.0000f, 0.0000f); - specularColor.set(0.0600f, 0.0600f, 0.0600f); - emissiveColor.set(0.0000f, 0.0000f, 0.0000f); - shininess = 0.0078f; - transparency = 0.0000f; + ambientColor.set(0.1000F, 0.1000F, 0.1000F); + diffuseColor.set(0.0000F, 0.0000F, 0.0000F); + specularColor.set(0.0600F, 0.0600F, 0.0600F); + emissiveColor.set(0.0000F, 0.0000F, 0.0000F); + shininess = 0.0078F; + transparency = 0.0000F; break; case SILVER: - ambientColor.set(0.1922f, 0.1922f, 0.1922f); - diffuseColor.set(0.5075f, 0.5075f, 0.5075f); - specularColor.set(0.5083f, 0.5083f, 0.5083f); - emissiveColor.set(0.0000f, 0.0000f, 0.0000f); - shininess = 0.2000f; - transparency = 0.0000f; + ambientColor.set(0.1922F, 0.1922F, 0.1922F); + diffuseColor.set(0.5075F, 0.5075F, 0.5075F); + specularColor.set(0.5083F, 0.5083F, 0.5083F); + emissiveColor.set(0.0000F, 0.0000F, 0.0000F); + shininess = 0.2000F; + transparency = 0.0000F; break; case STEEL: - ambientColor.set(0.0020f, 0.0020f, 0.0020f); - diffuseColor.set(0.0000f, 0.0000f, 0.0000f); - specularColor.set(0.9800f, 0.9800f, 0.9800f); - emissiveColor.set(0.0000f, 0.0000f, 0.0000f); - shininess = 0.0600f; - transparency = 0.0000f; + ambientColor.set(0.0020F, 0.0020F, 0.0020F); + diffuseColor.set(0.0000F, 0.0000F, 0.0000F); + specularColor.set(0.9800F, 0.9800F, 0.9800F); + emissiveColor.set(0.0000F, 0.0000F, 0.0000F); + shininess = 0.0600F; + transparency = 0.0000F; break; case STONE: - ambientColor.set(0.1900f, 0.1520f, 0.1178f); - diffuseColor.set(0.7500f, 0.6000f, 0.4650f); - specularColor.set(0.0784f, 0.0800f, 0.0480f); - emissiveColor.set(0.0000f, 0.0000f, 0.0000f); - shininess = 0.1700f; - transparency = 0.0000f; + ambientColor.set(0.1900F, 0.1520F, 0.1178F); + diffuseColor.set(0.7500F, 0.6000F, 0.4650F); + specularColor.set(0.0784F, 0.0800F, 0.0480F); + emissiveColor.set(0.0000F, 0.0000F, 0.0000F); + shininess = 0.1700F; + transparency = 0.0000F; break; case SHINY_PLASTIC: - ambientColor.set(0.0880f, 0.0880f, 0.0880f); - diffuseColor.set(0.0000f, 0.0000f, 0.0000f); - specularColor.set(1.0000f, 1.0000f, 1.0000f); - emissiveColor.set(0.0000f, 0.0000f, 0.0000f); - shininess = 1.0000f; - transparency = 0.0000f; + ambientColor.set(0.0880F, 0.0880F, 0.0880F); + diffuseColor.set(0.0000F, 0.0000F, 0.0000F); + specularColor.set(1.0000F, 1.0000F, 1.0000F); + emissiveColor.set(0.0000F, 0.0000F, 0.0000F); + shininess = 1.0000F; + transparency = 0.0000F; break; case SATIN: - ambientColor.set(0.0660f, 0.0660f, 0.0660f); - diffuseColor.set(0.0000f, 0.0000f, 0.0000f); - specularColor.set(0.4400f, 0.4400f, 0.4400f); - emissiveColor.set(0.0000f, 0.0000f, 0.0000f); - shininess = 0.0938f; - transparency = 0.0000f; + ambientColor.set(0.0660F, 0.0660F, 0.0660F); + diffuseColor.set(0.0000F, 0.0000F, 0.0000F); + specularColor.set(0.4400F, 0.4400F, 0.4400F); + emissiveColor.set(0.0000F, 0.0000F, 0.0000F); + shininess = 0.0938F; + transparency = 0.0000F; break; case METALIZED: - ambientColor.set(0.1800f, 0.1800f, 0.1800f); - diffuseColor.set(0.0000f, 0.0000f, 0.0000f); - specularColor.set(0.4500f, 0.4500f, 0.4500f); - emissiveColor.set(0.0000f, 0.0000f, 0.0000f); - shininess = 0.1300f; - transparency = 0.0000f; + ambientColor.set(0.1800F, 0.1800F, 0.1800F); + diffuseColor.set(0.0000F, 0.0000F, 0.0000F); + specularColor.set(0.4500F, 0.4500F, 0.4500F); + emissiveColor.set(0.0000F, 0.0000F, 0.0000F); + shininess = 0.1300F; + transparency = 0.0000F; break; case NEON_GNC: - ambientColor.set(0.2000f, 0.2000f, 0.2000f); - diffuseColor.set(0.0000f, 0.0000f, 0.0000f); - specularColor.set(0.6200f, 0.6200f, 0.6200f); - emissiveColor.set(1.0000f, 1.0000f, 0.0000f); - shininess = 0.0500f; - transparency = 0.0000f; + ambientColor.set(0.2000F, 0.2000F, 0.2000F); + diffuseColor.set(0.0000F, 0.0000F, 0.0000F); + specularColor.set(0.6200F, 0.6200F, 0.6200F); + emissiveColor.set(1.0000F, 1.0000F, 0.0000F); + shininess = 0.0500F; + transparency = 0.0000F; break; case CHROME: - ambientColor.set(0.3500f, 0.3500f, 0.3500f); - diffuseColor.set(0.9176f, 0.9176f, 0.9176f); - specularColor.set(0.9746f, 0.9746f, 0.9746f); - emissiveColor.set(0.0000f, 0.0000f, 0.0000f); - shininess = 0.1000f; - transparency = 0.0000f; + ambientColor.set(0.3500F, 0.3500F, 0.3500F); + diffuseColor.set(0.9176F, 0.9176F, 0.9176F); + specularColor.set(0.9746F, 0.9746F, 0.9746F); + emissiveColor.set(0.0000F, 0.0000F, 0.0000F); + shininess = 0.1000F; + transparency = 0.0000F; break; case ALUMINIUM: - ambientColor.set(0.3000f, 0.3000f, 0.3000f); - diffuseColor.set(0.3000f, 0.3000f, 0.3000f); - specularColor.set(0.7000f, 0.7000f, 0.8000f); - emissiveColor.set(0.0000f, 0.0000f, 0.0000f); - shininess = 0.0900f; - transparency = 0.0000f; + ambientColor.set(0.3000F, 0.3000F, 0.3000F); + diffuseColor.set(0.3000F, 0.3000F, 0.3000F); + specularColor.set(0.7000F, 0.7000F, 0.8000F); + emissiveColor.set(0.0000F, 0.0000F, 0.0000F); + shininess = 0.0900F; + transparency = 0.0000F; break; case OBSIDIAN: - ambientColor.set(0.0538f, 0.0500f, 0.0662f); - diffuseColor.set(0.1828f, 0.1700f, 0.2253f); - specularColor.set(0.3327f, 0.3286f, 0.3464f); - emissiveColor.set(0.0000f, 0.0000f, 0.0000f); - shininess = 0.3000f; - transparency = 0.0000f; + ambientColor.set(0.0538F, 0.0500F, 0.0662F); + diffuseColor.set(0.1828F, 0.1700F, 0.2253F); + specularColor.set(0.3327F, 0.3286F, 0.3464F); + emissiveColor.set(0.0000F, 0.0000F, 0.0000F); + shininess = 0.3000F; + transparency = 0.0000F; break; case NEON_PHC: - ambientColor.set(1.0000f, 1.0000f, 1.0000f); - diffuseColor.set(1.0000f, 1.0000f, 1.0000f); - specularColor.set(0.6200f, 0.6200f, 0.6200f); - emissiveColor.set(0.0000f, 0.9000f, 0.4140f); - shininess = 0.0500f; - transparency = 0.0000f; + ambientColor.set(1.0000F, 1.0000F, 1.0000F); + diffuseColor.set(1.0000F, 1.0000F, 1.0000F); + specularColor.set(0.6200F, 0.6200F, 0.6200F); + emissiveColor.set(0.0000F, 0.9000F, 0.4140F); + shininess = 0.0500F; + transparency = 0.0000F; break; case JADE: - ambientColor.set(0.1350f, 0.2225f, 0.1575f); - diffuseColor.set(0.5400f, 0.8900f, 0.6300f); - specularColor.set(0.3162f, 0.3162f, 0.3162f); - emissiveColor.set(0.0000f, 0.0000f, 0.0000f); - shininess = 0.1000f; - transparency = 0.0000f; + ambientColor.set(0.1350F, 0.2225F, 0.1575F); + diffuseColor.set(0.5400F, 0.8900F, 0.6300F); + specularColor.set(0.3162F, 0.3162F, 0.3162F); + emissiveColor.set(0.0000F, 0.0000F, 0.0000F); + shininess = 0.1000F; + transparency = 0.0000F; break; case RUBY: - ambientColor.set(0.1745f, 0.0118f, 0.0118f); - diffuseColor.set(0.6142f, 0.0414f, 0.0414f); - specularColor.set(0.7278f, 0.6279f, 0.6267f); - emissiveColor.set(0.0000f, 0.0000f, 0.0000f); - shininess = 0.6000f; - transparency = 0.0000f; + ambientColor.set(0.1745F, 0.0118F, 0.0118F); + diffuseColor.set(0.6142F, 0.0414F, 0.0414F); + specularColor.set(0.7278F, 0.6279F, 0.6267F); + emissiveColor.set(0.0000F, 0.0000F, 0.0000F); + shininess = 0.6000F; + transparency = 0.0000F; break; case EMERALD: - ambientColor.set(0.0215f, 0.1745f, 0.0215f); - diffuseColor.set(0.0757f, 0.6142f, 0.0757f); - specularColor.set(0.6330f, 0.7278f, 0.6330f); - emissiveColor.set(0.0000f, 0.0000f, 0.0000f); - shininess = 0.6000f; - transparency = 0.0000f; + ambientColor.set(0.0215F, 0.1745F, 0.0215F); + diffuseColor.set(0.0757F, 0.6142F, 0.0757F); + specularColor.set(0.6330F, 0.7278F, 0.6330F); + emissiveColor.set(0.0000F, 0.0000F, 0.0000F); + shininess = 0.6000F; + transparency = 0.0000F; break; case USER_DEFINED: break; default: - ambientColor.set(0.3333f, 0.3333f, 0.3333f); - diffuseColor.set(0.8000f, 0.8000f, 0.9000f); - specularColor.set(0.5333f, 0.5333f, 0.5333f); - emissiveColor.set(0.0000f, 0.0000f, 0.0000f); - shininess = 0.9000f; - transparency = 0.0000f; + ambientColor.set(0.3333F, 0.3333F, 0.3333F); + diffuseColor.set(0.8000F, 0.8000F, 0.9000F); + specularColor.set(0.5333F, 0.5333F, 0.5333F); + emissiveColor.set(0.0000F, 0.0000F, 0.0000F); + shininess = 0.9000F; + transparency = 0.0000F; break; } } +// NOLINTEND(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) diff --git a/src/App/Material.h b/src/App/Material.h index 02e32f5490..ba22b510e5 100644 --- a/src/App/Material.h +++ b/src/App/Material.h @@ -67,15 +67,17 @@ public: //@{ /** Sets the USER_DEFINED material type. The user must set the colors afterwards. */ Material(); + ~Material() = default; /** Copy constructor. */ - Material(const Material& other); + Material(const Material& other) = default; + Material(Material&& other) = default; /** Defines the colors and shininess for the material \a MatName. If \a MatName isn't defined * then USER_DEFINED is set and the user must define the colors itself. */ explicit Material(const char* MatName); /** Does basically the same as the constructor above unless that it accepts a MaterialType as * argument. */ - explicit Material(const MaterialType MatType); + explicit Material(MaterialType MatType); //@} /** Set a material by name @@ -111,7 +113,7 @@ public: * This method is provided for convenience which does basically the same as the method above * unless that it accepts a MaterialType as argument. */ - void setType(const MaterialType MatType); + void setType(MaterialType MatType); /** * Returns the currently set material type. */ @@ -122,6 +124,7 @@ public: /** @name Properties */ //@{ + // NOLINTBEGIN Color ambientColor; /**< Defines the ambient color. */ Color diffuseColor; /**< Defines the diffuse color. */ Color specularColor; /**< Defines the specular color. */ @@ -129,20 +132,28 @@ public: float shininess; float transparency; std::string uuid; + // NOLINTEND //@} bool operator==(const Material& m) const { - return _matType == m._matType && shininess == m.shininess && transparency == m.transparency - && ambientColor == m.ambientColor && diffuseColor == m.diffuseColor - && specularColor == m.specularColor && emissiveColor == m.emissiveColor + // clang-format off + return _matType == m._matType + && shininess == m.shininess + && transparency == m.transparency + && ambientColor == m.ambientColor + && diffuseColor == m.diffuseColor + && specularColor == m.specularColor + && emissiveColor == m.emissiveColor && uuid == m.uuid; + // clang-format on } bool operator!=(const Material& m) const { return !operator==(m); } - Material& operator=(const Material& other); + Material& operator=(const Material& other) = default; + Material& operator=(Material&& other) = default; private: MaterialType _matType; From 4fe1192e518a4b984013beef9ce1c64c2dcbc189 Mon Sep 17 00:00:00 2001 From: wmayer Date: Fri, 5 Apr 2024 13:48:54 +0200 Subject: [PATCH 02/16] App: refactor MaterialPy to avoid code duplication --- src/App/MaterialPy.xml | 3 ++ src/App/MaterialPyImp.cpp | 22 +++++------ src/App/PropertyStandard.cpp | 75 +----------------------------------- 3 files changed, 15 insertions(+), 85 deletions(-) diff --git a/src/App/MaterialPy.xml b/src/App/MaterialPy.xml index eff0db8c59..60b2567f9d 100644 --- a/src/App/MaterialPy.xml +++ b/src/App/MaterialPy.xml @@ -63,5 +63,8 @@ Satin, Metalized, Neon GNC, Chrome, Aluminium, Obsidian, Neon PHC, Jade, Ruby or + public: + static App::Color toColor(PyObject* value); + diff --git a/src/App/MaterialPyImp.cpp b/src/App/MaterialPyImp.cpp index dc0c5bbebf..bf7979106d 100644 --- a/src/App/MaterialPyImp.cpp +++ b/src/App/MaterialPyImp.cpp @@ -32,11 +32,11 @@ using namespace App; -Color parseColor(PyObject* value) +Color MaterialPy::toColor(PyObject* value) { Color cCol; if (PyTuple_Check(value) && (PyTuple_Size(value) == 3 || PyTuple_Size(value) == 4)) { - PyObject* item; + PyObject* item {}; item = PyTuple_GetItem(value, 0); if (PyFloat_Check(item)) { cCol.r = (float)PyFloat_AsDouble(item); @@ -65,17 +65,17 @@ Color parseColor(PyObject* value) } } else if (PyLong_Check(item)) { - cCol.r = PyLong_AsLong(item) / 255.0; + cCol.r = static_cast(PyLong_AsLong(item)) / 255.0F; item = PyTuple_GetItem(value, 1); if (PyLong_Check(item)) { - cCol.g = PyLong_AsLong(item) / 255.0; + cCol.g = static_cast(PyLong_AsLong(item)) / 255.0F; } else { throw Base::TypeError("Type in tuple must be consistent (integer)"); } item = PyTuple_GetItem(value, 2); if (PyLong_Check(item)) { - cCol.b = PyLong_AsLong(item) / 255.0; + cCol.b = static_cast(PyLong_AsLong(item)) / 255.0F; } else { throw Base::TypeError("Type in tuple must be consistent (integer)"); @@ -83,7 +83,7 @@ Color parseColor(PyObject* value) if (PyTuple_Size(value) == 4) { item = PyTuple_GetItem(value, 3); if (PyLong_Check(item)) { - cCol.a = PyLong_AsLong(item) / 255.0; + cCol.a = static_cast(PyLong_AsLong(item)) / 255.0F; } else { throw Base::TypeError("Type in tuple must be consistent (integer)"); @@ -178,7 +178,7 @@ std::string MaterialPy::representation() const PyObject* MaterialPy::set(PyObject* args) { - char* pstr; + char* pstr {}; if (!PyArg_ParseTuple(args, "s", &pstr)) { return nullptr; } @@ -200,7 +200,7 @@ Py::Object MaterialPy::getAmbientColor() const void MaterialPy::setAmbientColor(Py::Object arg) { - getMaterialPtr()->ambientColor = parseColor(*arg); + getMaterialPtr()->ambientColor = toColor(*arg); } Py::Object MaterialPy::getDiffuseColor() const @@ -215,7 +215,7 @@ Py::Object MaterialPy::getDiffuseColor() const void MaterialPy::setDiffuseColor(Py::Object arg) { - getMaterialPtr()->diffuseColor = parseColor(*arg); + getMaterialPtr()->diffuseColor = toColor(*arg); } Py::Object MaterialPy::getEmissiveColor() const @@ -230,7 +230,7 @@ Py::Object MaterialPy::getEmissiveColor() const void MaterialPy::setEmissiveColor(Py::Object arg) { - getMaterialPtr()->emissiveColor = parseColor(*arg); + getMaterialPtr()->emissiveColor = toColor(*arg); } Py::Object MaterialPy::getSpecularColor() const @@ -245,7 +245,7 @@ Py::Object MaterialPy::getSpecularColor() const void MaterialPy::setSpecularColor(Py::Object arg) { - getMaterialPtr()->specularColor = parseColor(*arg); + getMaterialPtr()->specularColor = toColor(*arg); } Py::Float MaterialPy::getShininess() const diff --git a/src/App/PropertyStandard.cpp b/src/App/PropertyStandard.cpp index ef5b1915d6..6319d289af 100644 --- a/src/App/PropertyStandard.cpp +++ b/src/App/PropertyStandard.cpp @@ -2560,83 +2560,11 @@ PyObject* PropertyMaterial::getPyObject() void PropertyMaterial::setPyObject(PyObject* value) { - App::Color cCol; if (PyObject_TypeCheck(value, &(MaterialPy::Type))) { setValue(*static_cast(value)->getMaterialPtr()); } - else if (PyTuple_Check(value) && (PyTuple_Size(value) == 3 || PyTuple_Size(value) == 4)) { - PyObject* item; - item = PyTuple_GetItem(value, 0); - if (PyFloat_Check(item)) { - cCol.r = (float)PyFloat_AsDouble(item); - item = PyTuple_GetItem(value, 1); - if (PyFloat_Check(item)) { - cCol.g = (float)PyFloat_AsDouble(item); - } - else { - throw Base::TypeError("Type in tuple must be consistent (float)"); - } - item = PyTuple_GetItem(value, 2); - if (PyFloat_Check(item)) { - cCol.b = (float)PyFloat_AsDouble(item); - } - else { - throw Base::TypeError("Type in tuple must be consistent (float)"); - } - if (PyTuple_Size(value) == 4) { - item = PyTuple_GetItem(value, 3); - if (PyFloat_Check(item)) { - cCol.a = (float)PyFloat_AsDouble(item); - } - else { - throw Base::TypeError("Type in tuple must be consistent (float)"); - } - } - - setValue(cCol); - } - else if (PyLong_Check(item)) { - cCol.r = PyLong_AsLong(item) / 255.0; - item = PyTuple_GetItem(value, 1); - if (PyLong_Check(item)) { - cCol.g = PyLong_AsLong(item) / 255.0; - } - else { - throw Base::TypeError("Type in tuple must be consistent (integer)"); - } - item = PyTuple_GetItem(value, 2); - if (PyLong_Check(item)) { - cCol.b = PyLong_AsLong(item) / 255.0; - } - else { - throw Base::TypeError("Type in tuple must be consistent (integer)"); - } - if (PyTuple_Size(value) == 4) { - item = PyTuple_GetItem(value, 3); - if (PyLong_Check(item)) { - cCol.a = PyLong_AsLong(item) / 255.0; - } - else { - throw Base::TypeError("Type in tuple must be consistent (integer)"); - } - } - - setValue(cCol); - } - else { - throw Base::TypeError("Type in tuple must be float or integer"); - } - } - else if (PyLong_Check(value)) { - cCol.setPackedValue(PyLong_AsUnsignedLong(value)); - - setValue(cCol); - } else { - std::string error = std::string( - "type must be 'Material', integer, tuple of float, or tuple of integer, not "); - error += value->ob_type->tp_name; - throw Base::TypeError(error); + setValue(MaterialPy::toColor(value)); } } @@ -3269,7 +3197,6 @@ void PropertyMaterialList::RestoreDocFileV1(Base::Reader& reader) std::vector values(count); uint32_t value; // must be 32 bit long float valueF; - char valueS[37]; // UUID length is 36 including '-'s for (auto& it : values) { str >> value; it.ambientColor.setPackedValue(value); From 8f649a8aa45c8b463ebc35d30c532bd8a048b40d Mon Sep 17 00:00:00 2001 From: wmayer Date: Fri, 5 Apr 2024 13:57:08 +0200 Subject: [PATCH 03/16] App: fix PropertyMaterial::Save to create valid XML output --- src/App/PropertyStandard.cpp | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/App/PropertyStandard.cpp b/src/App/PropertyStandard.cpp index 6319d289af..e7f335f882 100644 --- a/src/App/PropertyStandard.cpp +++ b/src/App/PropertyStandard.cpp @@ -2570,13 +2570,17 @@ void PropertyMaterial::setPyObject(PyObject* value) void PropertyMaterial::Save(Base::Writer& writer) const { - writer.Stream() << writer.ind() << "" - << "\" uuid=\"" << _cMat.uuid << "\"/>" << endl; + // clang-format off + writer.Stream() << writer.ind() + << "" << std::endl; + // clang-format on } void PropertyMaterial::Restore(Base::XMLReader& reader) From c161871689ee9fb950c684f384b57d40859234f6 Mon Sep 17 00:00:00 2001 From: wmayer Date: Fri, 5 Apr 2024 14:50:04 +0200 Subject: [PATCH 04/16] App: fix several linter warnings --- src/App/PropertyStandard.h | 95 +++++++++++++++++++++----------------- 1 file changed, 53 insertions(+), 42 deletions(-) diff --git a/src/App/PropertyStandard.h b/src/App/PropertyStandard.h index c59e3571a6..8e981e7b27 100644 --- a/src/App/PropertyStandard.h +++ b/src/App/PropertyStandard.h @@ -66,7 +66,7 @@ public: const char* getEditorName() const override { return "Gui::PropertyEditor::PropertyIntegerItem"; } PyObject *getPyObject() override; - void setPyObject(PyObject *) override; + void setPyObject(PyObject* py) override; void Save (Base::Writer &writer) const override; void Restore(Base::XMLReader &reader) override; @@ -80,8 +80,9 @@ public: const boost::any getPathValue(const App::ObjectIdentifier & /*path*/) const override { return _lValue; } bool isSame(const Property &other) const override { - if (&other == this) + if (&other == this) { return true; + } return getTypeId() == other.getTypeId() && getValue() == static_cast(&other)->getValue(); } @@ -117,7 +118,7 @@ public: const char* getEditorName() const override { return "Gui::PropertyEditor::PropertyPathItem"; } PyObject *getPyObject() override; - void setPyObject(PyObject *) override; + void setPyObject(PyObject* py) override; void Save (Base::Writer &writer) const override; void Restore(Base::XMLReader &reader) override; @@ -128,8 +129,9 @@ public: unsigned int getMemSize () const override; bool isSame(const Property &other) const override { - if (&other == this) + if (&other == this) { return true; + } return getTypeId() == other.getTypeId() && getValue() == static_cast(&other)->getValue(); } @@ -216,7 +218,7 @@ public: void setEditorName(const char* name) { _editorTypeName = name; } PyObject * getPyObject() override; - void setPyObject(PyObject *) override; + void setPyObject(PyObject* py) override; void Save(Base::Writer &writer) const override; void Restore(Base::XMLReader &reader) override; @@ -230,8 +232,9 @@ public: bool getPyPathValue(const ObjectIdentifier &path, Py::Object &r) const override; bool isSame(const Property &other) const override { - if (&other == this) + if (&other == this) { return true; + } return getTypeId() == other.getTypeId() && getEnum() == static_cast(&other)->getEnum(); } @@ -307,7 +310,7 @@ public: long getStepSize() const; const char* getEditorName() const override { return "Gui::PropertyEditor::PropertyIntegerConstraintItem"; } - void setPyObject(PyObject *) override; + void setPyObject(PyObject* py) override; protected: const Constraints* _ConstStruct{nullptr}; @@ -399,7 +402,7 @@ public: const std::set &getValues() const{return _lValueSet;} PyObject *getPyObject() override; - void setPyObject(PyObject *) override; + void setPyObject(PyObject* py) override; void Save (Base::Writer &writer) const override; void Restore(Base::XMLReader &reader) override; @@ -409,8 +412,9 @@ public: unsigned int getMemSize () const override; bool isSame(const Property &other) const override { - if (&other == this) + if (&other == this) { return true; + } return getTypeId() == other.getTypeId() && getValues() == static_cast(&other)->getValues(); } @@ -458,7 +462,7 @@ public: //virtual const char* getEditorName(void) const { return "Gui::PropertyEditor::PropertyStringListItem"; } PyObject *getPyObject() override; - void setPyObject(PyObject *) override; + void setPyObject(PyObject* py) override; void Save (Base::Writer &writer) const override; void Restore(Base::XMLReader &reader) override; @@ -469,8 +473,9 @@ public: unsigned int getMemSize () const override; bool isSame(const Property &other) const override { - if (&other == this) + if (&other == this) { return true; + } return getTypeId() == other.getTypeId() && getValues() == static_cast(&other)->getValues(); } @@ -511,7 +516,7 @@ public: const char* getEditorName() const override { return "Gui::PropertyEditor::PropertyFloatItem"; } PyObject *getPyObject() override; - void setPyObject(PyObject *) override; + void setPyObject(PyObject* py) override; void Save (Base::Writer &writer) const override; void Restore(Base::XMLReader &reader) override; @@ -525,8 +530,9 @@ public: const boost::any getPathValue(const App::ObjectIdentifier &path) const override; bool isSame(const Property &other) const override { - if (&other == this) + if (&other == this) { return true; + } return getTypeId() == other.getTypeId() && getValue() == static_cast(&other)->getValue(); } @@ -610,7 +616,7 @@ public: const char* getEditorName() const override { return "Gui::PropertyEditor::PropertyFloatConstraintItem"; } - void setPyObject(PyObject *) override; + void setPyObject(PyObject* py) override; protected: const Constraints* _ConstStruct{nullptr}; @@ -701,7 +707,7 @@ public: const char* getEditorName() const override { return "Gui::PropertyEditor::PropertyStringItem"; } PyObject *getPyObject() override; - void setPyObject(PyObject *) override; + void setPyObject(PyObject* py) override; void Save (Base::Writer &writer) const override; void Restore(Base::XMLReader &reader) override; @@ -714,8 +720,9 @@ public: const boost::any getPathValue(const App::ObjectIdentifier &path) const override; bool isSame(const Property &other) const override { - if (&other == this) + if (&other == this) { return true; + } return getTypeId() == other.getTypeId() && getStrValue() == static_cast(&other)->getStrValue(); } @@ -754,7 +761,7 @@ public: //virtual const char* getEditorName(void) const { return "Gui::PropertyEditor::PropertyStringItem"; } PyObject *getPyObject() override; - void setPyObject(PyObject *) override; + void setPyObject(PyObject* py) override; void Save (Base::Writer &writer) const override; void Restore(Base::XMLReader &reader) override; @@ -764,8 +771,9 @@ public: unsigned int getMemSize () const override; bool isSame(const Property &other) const override { - if (&other == this) + if (&other == this) { return true; + } return getTypeId() == other.getTypeId() && _uuid.getValue() == static_cast(&other)->_uuid.getValue(); } @@ -788,8 +796,9 @@ public: { return "Gui::PropertyEditor::PropertyFontItem"; } bool isSame(const Property &other) const override { - if (&other == this) + if (&other == this) { return true; + } return getTypeId() == other.getTypeId() && getValue() == static_cast(&other)->getValue(); } @@ -861,7 +870,7 @@ public: const char* getEditorName() const override { return "Gui::PropertyEditor::PropertyBoolItem"; } PyObject *getPyObject() override; - void setPyObject(PyObject *) override; + void setPyObject(PyObject* py) override; void Save (Base::Writer &writer) const override; void Restore(Base::XMLReader &reader) override; @@ -875,8 +884,9 @@ public: const boost::any getPathValue(const App::ObjectIdentifier &path) const override; bool isSame(const Property &other) const override { - if (&other == this) + if (&other == this) { return true; + } return getTypeId() == other.getTypeId() && getValue() == static_cast(&other)->getValue(); } @@ -898,7 +908,7 @@ public: ~PropertyBoolList() override; PyObject *getPyObject() override; - void setPyObject(PyObject *) override; + void setPyObject(PyObject* py) override; void Save (Base::Writer &writer) const override; void Restore(Base::XMLReader &reader) override; @@ -908,7 +918,7 @@ public: unsigned int getMemSize () const override; protected: - bool getPyValue(PyObject *) const override; + bool getPyValue(PyObject* py) const override; }; @@ -935,7 +945,7 @@ public: /** Sets the property */ void setValue(const Color &col); - void setValue(float r, float g, float b, float a=0.0f); + void setValue(float r, float g, float b, float a=0.0F); void setValue(uint32_t rgba); /** This method returns a string representation of the property @@ -945,7 +955,7 @@ public: const char* getEditorName() const override { return "Gui::PropertyEditor::PropertyColorItem"; } PyObject *getPyObject() override; - void setPyObject(PyObject *) override; + void setPyObject(PyObject* py) override; void Save (Base::Writer &writer) const override; void Restore(Base::XMLReader &reader) override; @@ -956,8 +966,9 @@ public: unsigned int getMemSize () const override{return sizeof(Color);} bool isSame(const Property &other) const override { - if (&other == this) + if (&other == this) { return true; + } return getTypeId() == other.getTypeId() && getValue() == static_cast(&other)->getValue(); } @@ -997,7 +1008,7 @@ public: unsigned int getMemSize () const override; protected: - Color getPyValue(PyObject *) const override; + Color getPyValue(PyObject* py) const override; }; @@ -1025,19 +1036,19 @@ public: */ void setValue(const Material& mat); void setValue(const Color& col); - void setValue(float r, float g, float b, float a = 0.0f); + void setValue(float r, float g, float b, float a = 0.0F); void setValue(uint32_t rgba); void setAmbientColor(const Color& col); - void setAmbientColor(float r, float g, float b, float a = 0.0f); + void setAmbientColor(float r, float g, float b, float a = 0.0F); void setAmbientColor(uint32_t rgba); void setDiffuseColor(const Color& col); - void setDiffuseColor(float r, float g, float b, float a = 0.0f); + void setDiffuseColor(float r, float g, float b, float a = 0.0F); void setDiffuseColor(uint32_t rgba); void setSpecularColor(const Color& col); - void setSpecularColor(float r, float g, float b, float a = 0.0f); + void setSpecularColor(float r, float g, float b, float a = 0.0F); void setSpecularColor(uint32_t rgba); void setEmissiveColor(const Color& col); - void setEmissiveColor(float r, float g, float b, float a = 0.0f); + void setEmissiveColor(float r, float g, float b, float a = 0.0F); void setEmissiveColor(uint32_t rgba); void setShininess(float); void setTransparency(float); @@ -1053,7 +1064,7 @@ public: double getTransparency() const; PyObject* getPyObject() override; - void setPyObject(PyObject*) override; + void setPyObject(PyObject* py) override; void Save(Base::Writer& writer) const override; void Restore(Base::XMLReader& reader) override; @@ -1109,31 +1120,31 @@ public: void setValue(int index, const Material& mat); void setAmbientColor(const Color& col); - void setAmbientColor(float r, float g, float b, float a = 0.0f); + void setAmbientColor(float r, float g, float b, float a = 0.0F); void setAmbientColor(uint32_t rgba); void setAmbientColor(int index, const Color& col); - void setAmbientColor(int index, float r, float g, float b, float a = 0.0f); + void setAmbientColor(int index, float r, float g, float b, float a = 0.0F); void setAmbientColor(int index, uint32_t rgba); void setDiffuseColor(const Color& col); - void setDiffuseColor(float r, float g, float b, float a = 0.0f); + void setDiffuseColor(float r, float g, float b, float a = 0.0F); void setDiffuseColor(uint32_t rgba); void setDiffuseColor(int index, const Color& col); - void setDiffuseColor(int index, float r, float g, float b, float a = 0.0f); + void setDiffuseColor(int index, float r, float g, float b, float a = 0.0F); void setDiffuseColor(int index, uint32_t rgba); void setSpecularColor(const Color& col); - void setSpecularColor(float r, float g, float b, float a = 0.0f); + void setSpecularColor(float r, float g, float b, float a = 0.0F); void setSpecularColor(uint32_t rgba); void setSpecularColor(int index, const Color& col); - void setSpecularColor(int index, float r, float g, float b, float a = 0.0f); + void setSpecularColor(int index, float r, float g, float b, float a = 0.0F); void setSpecularColor(int index, uint32_t rgba); void setEmissiveColor(const Color& col); - void setEmissiveColor(float r, float g, float b, float a = 0.0f); + void setEmissiveColor(float r, float g, float b, float a = 0.0F); void setEmissiveColor(uint32_t rgba); void setEmissiveColor(int index, const Color& col); - void setEmissiveColor(int index, float r, float g, float b, float a = 0.0f); + void setEmissiveColor(int index, float r, float g, float b, float a = 0.0F); void setEmissiveColor(int index, uint32_t rgba); void setShininess(float); @@ -1176,7 +1187,7 @@ public: unsigned int getMemSize() const override; protected: - Material getPyValue(PyObject*) const override; + Material getPyValue(PyObject* py) const override; void verifyIndex(int index) const; void setSizeOne(); From 5853314833bf7970e408278643bdc86c721db5eb Mon Sep 17 00:00:00 2001 From: wmayer Date: Fri, 5 Apr 2024 15:07:46 +0200 Subject: [PATCH 05/16] App: Add PropertyMaterialList::resizeByOneIfNeeded to avoid code duplication --- src/App/PropertyStandard.cpp | 101 +++++++++-------------------------- src/App/PropertyStandard.h | 7 ++- 2 files changed, 31 insertions(+), 77 deletions(-) diff --git a/src/App/PropertyStandard.cpp b/src/App/PropertyStandard.cpp index e7f335f882..c46f62ee4b 100644 --- a/src/App/PropertyStandard.cpp +++ b/src/App/PropertyStandard.cpp @@ -2666,6 +2666,17 @@ void PropertyMaterialList::setSizeOne() } } +int PropertyMaterialList::resizeByOneIfNeeded(int index) +{ + int size = getSize(); + if (index == -1 || index == size) { + index = size; + setSize(size + 1); + } + + return index; +} + void PropertyMaterialList::setValue() { Material empty; @@ -2687,11 +2698,7 @@ void PropertyMaterialList::setValue(int index, const Material& mat) verifyIndex(index); aboutToSetValue(); - int size = getSize(); - if (index == -1 || index == size) { - index = size; - setSize(index + 1); - } + index = resizeByOneIfNeeded(index); _lValueList[index] = mat; hasSetValue(); } @@ -2731,11 +2738,7 @@ void PropertyMaterialList::setAmbientColor(int index, const Color& col) verifyIndex(index); aboutToSetValue(); - int size = getSize(); - if (index == -1 || index == size) { - index = size; - setSize(index + 1); - } + index = resizeByOneIfNeeded(index); _lValueList[index].ambientColor = col; hasSetValue(); } @@ -2745,11 +2748,7 @@ void PropertyMaterialList::setAmbientColor(int index, float r, float g, float b, verifyIndex(index); aboutToSetValue(); - int size = getSize(); - if (index == -1 || index == size) { - index = size; - setSize(index + 1); - } + index = resizeByOneIfNeeded(index); _lValueList[index].ambientColor.set(r, g, b, a); hasSetValue(); } @@ -2759,11 +2758,7 @@ void PropertyMaterialList::setAmbientColor(int index, uint32_t rgba) verifyIndex(index); aboutToSetValue(); - int size = getSize(); - if (index == -1 || index == size) { - index = size; - setSize(index + 1); - } + index = resizeByOneIfNeeded(index); _lValueList[index].ambientColor.setPackedValue(rgba); hasSetValue(); } @@ -2803,11 +2798,7 @@ void PropertyMaterialList::setDiffuseColor(int index, const Color& col) verifyIndex(index); aboutToSetValue(); - int size = getSize(); - if (index == -1 || index == size) { - index = size; - setSize(index + 1); - } + index = resizeByOneIfNeeded(index); _lValueList[index].diffuseColor = col; hasSetValue(); } @@ -2817,11 +2808,7 @@ void PropertyMaterialList::setDiffuseColor(int index, float r, float g, float b, verifyIndex(index); aboutToSetValue(); - int size = getSize(); - if (index == -1 || index == size) { - index = size; - setSize(index + 1); - } + index = resizeByOneIfNeeded(index); _lValueList[index].diffuseColor.set(r, g, b, a); hasSetValue(); } @@ -2831,11 +2818,7 @@ void PropertyMaterialList::setDiffuseColor(int index, uint32_t rgba) verifyIndex(index); aboutToSetValue(); - int size = getSize(); - if (index == -1 || index == size) { - index = size; - setSize(index + 1); - } + index = resizeByOneIfNeeded(index); _lValueList[index].diffuseColor.setPackedValue(rgba); hasSetValue(); } @@ -2875,11 +2858,7 @@ void PropertyMaterialList::setSpecularColor(int index, const Color& col) verifyIndex(index); aboutToSetValue(); - int size = getSize(); - if (index == -1 || index == size) { - index = size; - setSize(index + 1); - } + index = resizeByOneIfNeeded(index); _lValueList[index].specularColor = col; hasSetValue(); } @@ -2889,11 +2868,7 @@ void PropertyMaterialList::setSpecularColor(int index, float r, float g, float b verifyIndex(index); aboutToSetValue(); - int size = getSize(); - if (index == -1 || index == size) { - index = size; - setSize(index + 1); - } + index = resizeByOneIfNeeded(index); _lValueList[index].specularColor.set(r, g, b, a); hasSetValue(); } @@ -2903,11 +2878,7 @@ void PropertyMaterialList::setSpecularColor(int index, uint32_t rgba) verifyIndex(index); aboutToSetValue(); - int size = getSize(); - if (index == -1 || index == size) { - index = size; - setSize(index + 1); - } + index = resizeByOneIfNeeded(index); _lValueList[index].specularColor.setPackedValue(rgba); hasSetValue(); } @@ -2947,11 +2918,7 @@ void PropertyMaterialList::setEmissiveColor(int index, const Color& col) verifyIndex(index); aboutToSetValue(); - int size = getSize(); - if (index == -1 || index == size) { - index = size; - setSize(index + 1); - } + index = resizeByOneIfNeeded(index); _lValueList[index].emissiveColor = col; hasSetValue(); } @@ -2961,11 +2928,7 @@ void PropertyMaterialList::setEmissiveColor(int index, float r, float g, float b verifyIndex(index); aboutToSetValue(); - int size = getSize(); - if (index == -1 || index == size) { - index = size; - setSize(index + 1); - } + index = resizeByOneIfNeeded(index); _lValueList[index].emissiveColor.set(r, g, b, a); hasSetValue(); } @@ -2975,11 +2938,7 @@ void PropertyMaterialList::setEmissiveColor(int index, uint32_t rgba) verifyIndex(index); aboutToSetValue(); - int size = getSize(); - if (index == -1 || index == size) { - index = size; - setSize(index + 1); - } + index = resizeByOneIfNeeded(index); _lValueList[index].emissiveColor.setPackedValue(rgba); hasSetValue(); } @@ -2999,11 +2958,7 @@ void PropertyMaterialList::setShininess(int index, float val) verifyIndex(index); aboutToSetValue(); - int size = getSize(); - if (index == -1 || index == size) { - index = size; - setSize(index + 1); - } + index = resizeByOneIfNeeded(index); _lValueList[index].shininess = val; hasSetValue(); } @@ -3023,11 +2978,7 @@ void PropertyMaterialList::setTransparency(int index, float val) verifyIndex(index); aboutToSetValue(); - int size = getSize(); - if (index == -1 || index == size) { - index = size; - setSize(index + 1); - } + index = resizeByOneIfNeeded(index); _lValueList[index].transparency = val; hasSetValue(); } diff --git a/src/App/PropertyStandard.h b/src/App/PropertyStandard.h index 8e981e7b27..a66e482f9c 100644 --- a/src/App/PropertyStandard.h +++ b/src/App/PropertyStandard.h @@ -1188,11 +1188,14 @@ public: protected: Material getPyValue(PyObject* py) const override; - void verifyIndex(int index) const; - void setSizeOne(); void RestoreDocFileV0(uint32_t count, Base::Reader& reader); void RestoreDocFileV1(Base::Reader& reader); + +private: + void verifyIndex(int index) const; + void setSizeOne(); + int resizeByOneIfNeeded(int index); }; From a19afb0f7d565cc8816b4789c8218b49dbe0a3de Mon Sep 17 00:00:00 2001 From: wmayer Date: Fri, 5 Apr 2024 17:29:10 +0200 Subject: [PATCH 06/16] Gui: fix ViewProviderGeometryObject * fix several linter warnings * remove code that cannot be executed: inside the constructor it cannot ever happen that getObject() returns a valid object * in ViewProviderGeometryObject::handleChangedPropertyName call the method of the direct base class as otherwise this may break the mechanism in the future * Shape is a property of an extension module -> move its handling to ViewProviderPartExt --- src/Gui/ViewProviderGeometryObject.cpp | 60 ++++++++++++-------------- src/Mod/Part/Gui/ViewProviderExt.cpp | 5 +++ 2 files changed, 32 insertions(+), 33 deletions(-) diff --git a/src/Gui/ViewProviderGeometryObject.cpp b/src/Gui/ViewProviderGeometryObject.cpp index 06bd1b9882..82d4295e47 100644 --- a/src/Gui/ViewProviderGeometryObject.cpp +++ b/src/Gui/ViewProviderGeometryObject.cpp @@ -74,28 +74,26 @@ ViewProviderGeometryObject::ViewProviderGeometryObject() Transparency.setConstraints(&intPercent); App::Material mat(App::Material::DEFAULT); - auto geometry = dynamic_cast(getObject()); - if (geometry) { - mat = geometry->getMaterialAppearance(); - } else { - // This is handled in the material code when using the object appearance - bool randomColor = hGrp->GetBool("RandomColor", false); - float r, g, b; + // This is handled in the material code when using the object appearance + bool randomColor = hGrp->GetBool("RandomColor", false); + float red {}; + float green {}; + float blue {}; - if (randomColor) { - auto fMax = (float)RAND_MAX; - r = (float)rand() / fMax; - g = (float)rand() / fMax; - b = (float)rand() / fMax; - } - else { - unsigned long shcol = hGrp->GetUnsigned("DefaultShapeColor", 3435980543UL); - r = ((shcol >> 24) & 0xff) / 255.0; - g = ((shcol >> 16) & 0xff) / 255.0; - b = ((shcol >> 8) & 0xff) / 255.0; - } - mat.diffuseColor = App::Color(r,g,b); + if (randomColor) { + auto fMax = (float)RAND_MAX; + red = (float)rand() / fMax; + green = (float)rand() / fMax; + blue = (float)rand() / fMax; } + else { + // Color = (204, 204, 230) + unsigned long shcol = hGrp->GetUnsigned("DefaultShapeColor", 3435980543UL); + red = ((shcol >> 24) & 0xff) / 255.0F; + green = ((shcol >> 16) & 0xff) / 255.0F; + blue = ((shcol >> 8) & 0xff) / 255.0F; + } + mat.diffuseColor = App::Color(red, green, blue); ADD_PROPERTY_TYPE(ShapeAppearance, (mat), osgroup, App::Prop_None, "Shape appearrance"); ADD_PROPERTY_TYPE(BoundingBox, (false), dogroup, App::Prop_None, "Display object bounding box"); @@ -171,12 +169,6 @@ void ViewProviderGeometryObject::attach(App::DocumentObject* pcObj) void ViewProviderGeometryObject::updateData(const App::Property* prop) { - std::string propName = prop->getName(); - if (propName == "Shape") { - // Reapply the appearance - const App::Material& Mat = ShapeAppearance[0]; - setSoMaterial(Mat); - } if (prop->isDerivedFrom(App::PropertyComplexGeoData::getClassTypeId())) { Base::BoundBox3d box = static_cast(prop)->getBoundingBox(); @@ -289,18 +281,20 @@ void ViewProviderGeometryObject::showBoundingBox(bool show) { if (!pcBoundSwitch && show) { unsigned long bbcol = getBoundColor(); - float r, g, b; - r = ((bbcol >> 24) & 0xff) / 255.0; - g = ((bbcol >> 16) & 0xff) / 255.0; - b = ((bbcol >> 8) & 0xff) / 255.0; + float red {}; + float green {}; + float blue {}; + red = ((bbcol >> 24) & 0xff) / 255.0F; + green = ((bbcol >> 16) & 0xff) / 255.0F; + blue = ((bbcol >> 8) & 0xff) / 255.0F; pcBoundSwitch = new SoSwitch(); auto pBoundingSep = new SoSeparator(); auto lineStyle = new SoDrawStyle; - lineStyle->lineWidth = 2.0f; + lineStyle->lineWidth = 2.0F; pBoundingSep->addChild(lineStyle); - pcBoundColor->rgb.setValue(r, g, b); + pcBoundColor->rgb.setValue(red, green, blue); pBoundingSep->addChild(pcBoundColor); auto font = new SoFont(); font->size.setValue(getBoundBoxFontSize()); @@ -375,6 +369,6 @@ void ViewProviderGeometryObject::handleChangedPropertyName(Base::XMLReader& read ShapeAppearance.setValue(prop.getValue()); } else { - App::PropertyContainer::handleChangedPropertyName(reader, TypeName, PropName); + ViewProviderDragger::handleChangedPropertyName(reader, TypeName, PropName); } } diff --git a/src/Mod/Part/Gui/ViewProviderExt.cpp b/src/Mod/Part/Gui/ViewProviderExt.cpp index e246d7900f..4321e86aa1 100644 --- a/src/Mod/Part/Gui/ViewProviderExt.cpp +++ b/src/Mod/Part/Gui/ViewProviderExt.cpp @@ -844,6 +844,11 @@ void ViewProviderPartExt::updateData(const App::Property* prop) } } } + if (propName && strcmp(propName, "Shape") == 0) { + // Reapply the appearance + const App::Material& Mat = ShapeAppearance[0]; + setSoMaterial(Mat); + } Gui::ViewProviderGeometryObject::updateData(prop); } From 4975da5a280bd9871133b65ca01966cd8d61c380 Mon Sep 17 00:00:00 2001 From: wmayer Date: Fri, 5 Apr 2024 17:59:22 +0200 Subject: [PATCH 07/16] Mod: Adjust parent class for Python wrappers --- src/Mod/Fem/Gui/ViewProviderFemConstraintPy.xml | 4 ++-- src/Mod/Fem/Gui/ViewProviderFemMeshPy.xml | 4 ++-- src/Mod/Mesh/Gui/ViewProviderMeshPy.xml | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Mod/Fem/Gui/ViewProviderFemConstraintPy.xml b/src/Mod/Fem/Gui/ViewProviderFemConstraintPy.xml index 9051a61062..3192cf5853 100644 --- a/src/Mod/Fem/Gui/ViewProviderFemConstraintPy.xml +++ b/src/Mod/Fem/Gui/ViewProviderFemConstraintPy.xml @@ -1,13 +1,13 @@ diff --git a/src/Mod/Fem/Gui/ViewProviderFemMeshPy.xml b/src/Mod/Fem/Gui/ViewProviderFemMeshPy.xml index e25e3c6a57..e887c882bb 100644 --- a/src/Mod/Fem/Gui/ViewProviderFemMeshPy.xml +++ b/src/Mod/Fem/Gui/ViewProviderFemMeshPy.xml @@ -1,13 +1,13 @@ diff --git a/src/Mod/Mesh/Gui/ViewProviderMeshPy.xml b/src/Mod/Mesh/Gui/ViewProviderMeshPy.xml index c4af258e95..aee4a45e9c 100644 --- a/src/Mod/Mesh/Gui/ViewProviderMeshPy.xml +++ b/src/Mod/Mesh/Gui/ViewProviderMeshPy.xml @@ -1,13 +1,13 @@ From 9c368d89162bc8bcbad3e969065bccb38a5aeda9 Mon Sep 17 00:00:00 2001 From: wmayer Date: Fri, 5 Apr 2024 19:21:31 +0200 Subject: [PATCH 08/16] Gui: fix Std_RandomColor --- src/Gui/CommandFeat.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/Gui/CommandFeat.cpp b/src/Gui/CommandFeat.cpp index 30b65ee3a7..56df7d1821 100644 --- a/src/Gui/CommandFeat.cpp +++ b/src/Gui/CommandFeat.cpp @@ -104,12 +104,15 @@ void StdCmdRandomColor::activated(int iMsg) vpLink->ShapeMaterial.setDiffuseColor(objColor); } else if (view) { - auto appearance = - dynamic_cast(view->getPropertyByName("ShapeAppearance")); - if (appearance) { - // get the view provider of the selected object and set the shape color - appearance->setDiffuseColor(objColor); + // clang-format off + // get the view provider of the selected object and set the shape color + if (auto prop = dynamic_cast(view->getPropertyByName("ShapeAppearance"))) { + prop->setDiffuseColor(objColor); } + else if (auto prop = dynamic_cast(view->getPropertyByName("ShapeAppearance"))) { + prop->setDiffuseColor(objColor); + } + // clang-format on } }; From 36d043cbbdc1d91c79ae105f3aa0a8773f73e830 Mon Sep 17 00:00:00 2001 From: wmayer Date: Sat, 6 Apr 2024 02:08:27 +0200 Subject: [PATCH 09/16] Mod: code cleanup --- src/Mod/Mesh/Gui/ViewProvider.cpp | 4 --- src/Mod/Part/Gui/DlgProjectionOnSurface.cpp | 30 ++++++++++----------- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/src/Mod/Mesh/Gui/ViewProvider.cpp b/src/Mod/Mesh/Gui/ViewProvider.cpp index 2a40a287d1..da3a52bda1 100644 --- a/src/Mod/Mesh/Gui/ViewProvider.cpp +++ b/src/Mod/Mesh/Gui/ViewProvider.cpp @@ -353,7 +353,6 @@ ViewProviderMesh::~ViewProviderMesh() void ViewProviderMesh::onChanged(const App::Property* prop) { // we're going to change the number of colors to one - // if (prop == &ShapeAppearance || prop == &ShapeMaterial) { if (prop == &ShapeAppearance) { pcMatBinding->value = SoMaterialBinding::OVERALL; } @@ -397,9 +396,6 @@ void ViewProviderMesh::onChanged(const App::Property* prop) if (prop == &ShapeAppearance) { setOpenEdgeColorFrom(ShapeAppearance.getDiffuseColor()); } - // else if (prop == &ShapeMaterial) { - // setOpenEdgeColorFrom(ShapeMaterial.getValue().diffuseColor); - // } } ViewProviderGeometryObject::onChanged(prop); diff --git a/src/Mod/Part/Gui/DlgProjectionOnSurface.cpp b/src/Mod/Part/Gui/DlgProjectionOnSurface.cpp index a84f890a33..9f99c21468 100644 --- a/src/Mod/Part/Gui/DlgProjectionOnSurface.cpp +++ b/src/Mod/Part/Gui/DlgProjectionOnSurface.cpp @@ -716,22 +716,20 @@ void PartGui::DlgProjectionOnSurface::higlight_object(Part::Feature* iCurrentObj } auto index = anIndices.FindIndex(currentShape); - //set color - PartGui::ViewProviderPartExt* vp = dynamic_cast(Gui::Application::Instance->getViewProvider(iCurrentObject)); - if (vp) - { - std::vector colors; - App::Color defaultColor; - if (currentShapeType == TopAbs_FACE) - { - colors = vp->DiffuseColor.getValues(); - defaultColor = vp->ShapeAppearance.getDiffuseColor(); - } - else if ( currentShapeType == TopAbs_EDGE ) - { - colors = vp->LineColorArray.getValues(); - defaultColor = vp->LineColor.getValue(); - } + // set color + auto vp = dynamic_cast( + Gui::Application::Instance->getViewProvider(iCurrentObject)); + if (vp) { + std::vector colors; + App::Color defaultColor; + if (currentShapeType == TopAbs_FACE) { + colors = vp->DiffuseColor.getValues(); + defaultColor = vp->ShapeAppearance.getDiffuseColor(); + } + else if (currentShapeType == TopAbs_EDGE) { + colors = vp->LineColorArray.getValues(); + defaultColor = vp->LineColor.getValue(); + } if (static_cast(colors.size()) != anIndices.Extent()) { colors.resize(anIndices.Extent(), defaultColor); From 9b7a7b9756eaa95ec38410aa97e35b840621d05c Mon Sep 17 00:00:00 2001 From: wmayer Date: Sat, 6 Apr 2024 02:09:20 +0200 Subject: [PATCH 10/16] Part: fix Part_RefineShape and Part_Section --- src/Mod/Part/Gui/Command.cpp | 2 +- src/Mod/Part/Gui/CommandSimple.cpp | 27 ++++++++++++++------------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/Mod/Part/Gui/Command.cpp b/src/Mod/Part/Gui/Command.cpp index 4ef844b170..0821411a01 100644 --- a/src/Mod/Part/Gui/Command.cpp +++ b/src/Mod/Part/Gui/Command.cpp @@ -925,7 +925,7 @@ void CmdPartSection::activated(int iMsg) doCommand(Doc,"App.activeDocument().%s.Tool = App.activeDocument().%s",FeatName.c_str(),ToolName.c_str()); doCommand(Gui,"Gui.activeDocument().hide('%s')",BaseName.c_str()); doCommand(Gui,"Gui.activeDocument().hide('%s')",ToolName.c_str()); - doCommand(Gui,"Gui.activeDocument().%s.LineColor = Gui.activeDocument().%s.ShapeAppearance.DiffuseColor",FeatName.c_str(),BaseName.c_str()); + doCommand(Gui,"Gui.activeDocument().%s.LineMaterial = Gui.activeDocument().%s.ShapeAppearance[0]",FeatName.c_str(),BaseName.c_str()); updateActive(); commitCommand(); } diff --git a/src/Mod/Part/Gui/CommandSimple.cpp b/src/Mod/Part/Gui/CommandSimple.cpp index b53392b85e..16e9936fe0 100644 --- a/src/Mod/Part/Gui/CommandSimple.cpp +++ b/src/Mod/Part/Gui/CommandSimple.cpp @@ -28,10 +28,11 @@ #include #include +#include #include #include #include -#include +#include #include #include #include @@ -368,19 +369,19 @@ void CmdPartRefineShape::activated(int iMsg) openCommand(QT_TRANSLATE_NOOP("Command", "Refine shape")); std::for_each(objs.begin(), objs.end(), [](App::DocumentObject* obj) { try { - doCommand(Doc,"App.ActiveDocument.addObject('Part::Refine','%s').Source=" - "App.ActiveDocument.%s\n" - "App.ActiveDocument.ActiveObject.Label=" - "App.ActiveDocument.%s.Label\n" - "Gui.ActiveDocument.%s.hide()\n", - obj->getNameInDocument(), - obj->getNameInDocument(), - obj->getNameInDocument(), - obj->getNameInDocument()); + App::DocumentObjectT objT(obj); + Gui::cmdAppDocumentArgs(obj->getDocument(), "addObject('Part::Refine','%s')", + obj->getNameInDocument()); + Gui::cmdAppDocumentArgs(obj->getDocument(), "ActiveObject.Source = %s", + objT.getObjectPython()); + Gui::cmdAppDocumentArgs(obj->getDocument(), "ActiveObject.Label = %s.Label", + objT.getObjectPython()); + Gui::cmdAppObjectHide(obj); - copyVisual("ActiveObject", "ShapeAppearance", obj->getNameInDocument()); - copyVisual("ActiveObject", "LineColor", obj->getNameInDocument()); - copyVisual("ActiveObject", "PointColor", obj->getNameInDocument()); + auto newObj = App::GetApplication().getActiveDocument()->getActiveObject(); + Gui::copyVisualT(newObj->getNameInDocument(), "ShapeAppearance", obj->getNameInDocument()); + Gui::copyVisualT(newObj->getNameInDocument(), "LineColor", obj->getNameInDocument()); + Gui::copyVisualT(newObj->getNameInDocument(), "PointColor", obj->getNameInDocument()); } catch (const Base::Exception& e) { Base::Console().Warning("%s: %s\n", obj->Label.getValue(), e.what()); From b7bc6ad1e686b091d32d1e247284f3279185b8cd Mon Sep 17 00:00:00 2001 From: wmayer Date: Sat, 6 Apr 2024 02:12:04 +0200 Subject: [PATCH 11/16] Gui: add security checks to view provider extensions because they can be accessed if a document objects is not yet attached --- .../ViewProviderGeoFeatureGroupExtension.cpp | 17 ++++++++++++----- src/Gui/ViewProviderGroupExtension.cpp | 7 ++++++- src/Gui/ViewProviderOriginGroupExtension.cpp | 12 ++++++++---- src/Gui/ViewProviderSuppressibleExtension.cpp | 2 +- .../Part/Gui/ViewProviderAttachExtension.cpp | 5 +++-- 5 files changed, 30 insertions(+), 13 deletions(-) diff --git a/src/Gui/ViewProviderGeoFeatureGroupExtension.cpp b/src/Gui/ViewProviderGeoFeatureGroupExtension.cpp index c6ab8a900e..e441220f4b 100644 --- a/src/Gui/ViewProviderGeoFeatureGroupExtension.cpp +++ b/src/Gui/ViewProviderGeoFeatureGroupExtension.cpp @@ -67,7 +67,8 @@ ViewProviderGeoFeatureGroupExtension::~ViewProviderGeoFeatureGroupExtension() std::vector ViewProviderGeoFeatureGroupExtension::extensionClaimChildren3D() const { //all object in the group must be claimed in 3D, as we are a coordinate system for all of them - auto* ext = getExtendedViewProvider()->getObject()->getExtensionByType(); + auto* obj = getExtendedViewProvider()->getObject(); + auto* ext = obj ? obj->getExtensionByType() : nullptr; if (ext) { auto objs = ext->Group.getValues(); return objs; @@ -77,7 +78,12 @@ std::vector ViewProviderGeoFeatureGroupExtension::extensio std::vector ViewProviderGeoFeatureGroupExtension::extensionClaimChildren() const { - auto* group = getExtendedViewProvider()->getObject()->getExtensionByType(); + auto* obj = getExtendedViewProvider()->getObject(); + if (!obj) { + return {}; + } + + auto* group = obj->getExtensionByType(); const std::vector &model = group->Group.getValues (); std::set outSet; //< set of objects not to claim (childrens of childrens) @@ -142,9 +148,10 @@ std::vector ViewProviderGeoFeatureGroupExtension::extensionGetDispl void ViewProviderGeoFeatureGroupExtension::extensionUpdateData(const App::Property* prop) { - auto obj = getExtendedViewProvider()->getObject()->getExtensionByType(); - if (obj && prop == &obj->placement()) { - getExtendedViewProvider()->setTransformation ( obj->placement().getValue().toMatrix() ); + auto obj = getExtendedViewProvider()->getObject(); + auto grp = obj ? obj->getExtensionByType() : nullptr; + if (grp && prop == &grp->placement()) { + getExtendedViewProvider()->setTransformation ( grp->placement().getValue().toMatrix() ); } else { ViewProviderGroupExtension::extensionUpdateData ( prop ); diff --git a/src/Gui/ViewProviderGroupExtension.cpp b/src/Gui/ViewProviderGroupExtension.cpp index 4bfbabbdbb..310997929e 100644 --- a/src/Gui/ViewProviderGroupExtension.cpp +++ b/src/Gui/ViewProviderGroupExtension.cpp @@ -108,7 +108,12 @@ void ViewProviderGroupExtension::extensionDropObject(App::DocumentObject* obj) { std::vector< App::DocumentObject* > ViewProviderGroupExtension::extensionClaimChildren() const { - auto* group = getExtendedViewProvider()->getObject()->getExtensionByType(); + auto* obj = getExtendedViewProvider()->getObject(); + if (!obj) { + return {}; + } + + auto* group = obj->getExtensionByType(); return group->Group.getValues(); } diff --git a/src/Gui/ViewProviderOriginGroupExtension.cpp b/src/Gui/ViewProviderOriginGroupExtension.cpp index 1eb7f3e526..2d08457968 100644 --- a/src/Gui/ViewProviderOriginGroupExtension.cpp +++ b/src/Gui/ViewProviderOriginGroupExtension.cpp @@ -62,7 +62,8 @@ ViewProviderOriginGroupExtension::~ViewProviderOriginGroupExtension() std::vector ViewProviderOriginGroupExtension::constructChildren ( const std::vector &children ) const { - auto* group = getExtendedViewProvider()->getObject()->getExtensionByType(); + auto* obj = getExtendedViewProvider()->getObject(); + auto* group = obj ? obj->getExtensionByType() : nullptr; if(!group) return children; @@ -108,7 +109,8 @@ void ViewProviderOriginGroupExtension::extensionAttach(App::DocumentObject *pcOb void ViewProviderOriginGroupExtension::extensionUpdateData( const App::Property* prop ) { - auto* group = getExtendedViewProvider()->getObject()->getExtensionByType(); + auto* obj = getExtendedViewProvider()->getObject(); + auto* group = obj ? obj->getExtensionByType() : nullptr; if ( group && prop == &group->Group ) { updateOriginSize(); } @@ -117,7 +119,8 @@ void ViewProviderOriginGroupExtension::extensionUpdateData( const App::Property* } void ViewProviderOriginGroupExtension::slotChangedObjectApp ( const App::DocumentObject& obj) { - auto* group = getExtendedViewProvider()->getObject()->getExtensionByType(); + auto* ext = getExtendedViewProvider()->getObject(); + auto* group = ext ? ext->getExtensionByType() : nullptr; if ( group && group->hasObject (&obj, /*recursive=*/ true ) ) { updateOriginSize (); } @@ -127,7 +130,8 @@ void ViewProviderOriginGroupExtension::slotChangedObjectGui ( const Gui::ViewPro if ( !vp.isDerivedFrom ( Gui::ViewProviderOriginFeature::getClassTypeId () )) { // Ignore origins to avoid infinite recursion (not likely in a well-formed document, // but may happen in documents designed in old versions of assembly branch ) - auto* group = getExtendedViewProvider()->getObject()->getExtensionByType(); + auto* ext = getExtendedViewProvider()->getObject(); + auto* group = ext ? ext->getExtensionByType() : nullptr; App::DocumentObject *obj = vp.getObject (); if ( group && obj && group->hasObject (obj, /*recursive=*/ true ) ) { diff --git a/src/Gui/ViewProviderSuppressibleExtension.cpp b/src/Gui/ViewProviderSuppressibleExtension.cpp index 17d48c4df0..4ebd3c5d64 100644 --- a/src/Gui/ViewProviderSuppressibleExtension.cpp +++ b/src/Gui/ViewProviderSuppressibleExtension.cpp @@ -53,7 +53,7 @@ void ViewProviderSuppressibleExtension::extensionUpdateData(const App::Property* { auto vp = getExtendedViewProvider(); auto owner = vp->getObject(); - if(!owner->isValid()) + if(!owner || !owner->isValid()) return; auto ext = owner->getExtensionByType(); diff --git a/src/Mod/Part/Gui/ViewProviderAttachExtension.cpp b/src/Mod/Part/Gui/ViewProviderAttachExtension.cpp index 550c6936e1..06a2dd377a 100644 --- a/src/Mod/Part/Gui/ViewProviderAttachExtension.cpp +++ b/src/Mod/Part/Gui/ViewProviderAttachExtension.cpp @@ -77,8 +77,9 @@ QIcon ViewProviderAttachExtension::extensionMergeColorfullOverlayIcons (const QI void ViewProviderAttachExtension::extensionUpdateData(const App::Property* prop) { - if (getExtendedViewProvider()->getObject()->hasExtension(Part::AttachExtension::getExtensionClassTypeId())) { - auto* attach = getExtendedViewProvider()->getObject()->getExtensionByType(); + auto obj = getExtendedViewProvider()->getObject(); + if (obj && obj->hasExtension(Part::AttachExtension::getExtensionClassTypeId())) { + auto* attach = obj->getExtensionByType(); if(attach) { if( prop == &(attach->AttachmentSupport) || From 780481f6ed913bdccb6fec82e997ee9ee42a3ab7 Mon Sep 17 00:00:00 2001 From: wmayer Date: Sat, 6 Apr 2024 02:20:52 +0200 Subject: [PATCH 12/16] PD: fix shape appearance of datum features, shape binder and sub-shape binder --- src/Mod/PartDesign/Gui/ViewProviderDatum.cpp | 4 ++++ src/Mod/PartDesign/Gui/ViewProviderShapeBinder.cpp | 11 +++++++++++ src/Mod/PartDesign/Gui/ViewProviderShapeBinder.h | 1 + 3 files changed, 16 insertions(+) diff --git a/src/Mod/PartDesign/Gui/ViewProviderDatum.cpp b/src/Mod/PartDesign/Gui/ViewProviderDatum.cpp index ae5c93b2d3..f5449bfcd8 100644 --- a/src/Mod/PartDesign/Gui/ViewProviderDatum.cpp +++ b/src/Mod/PartDesign/Gui/ViewProviderDatum.cpp @@ -101,6 +101,10 @@ ViewProviderDatum::~ViewProviderDatum() void ViewProviderDatum::attach(App::DocumentObject *obj) { + if (auto geo = dynamic_cast(obj)) { + geo->setMaterialAppearance(ShapeAppearance[0]); + } + ViewProviderGeometryObject::attach ( obj ); // TODO remove this field (2015-09-08, Fat-Zer) diff --git a/src/Mod/PartDesign/Gui/ViewProviderShapeBinder.cpp b/src/Mod/PartDesign/Gui/ViewProviderShapeBinder.cpp index 7dbb522c7a..8c4da52827 100644 --- a/src/Mod/PartDesign/Gui/ViewProviderShapeBinder.cpp +++ b/src/Mod/PartDesign/Gui/ViewProviderShapeBinder.cpp @@ -125,6 +125,14 @@ void ViewProviderShapeBinder::unsetEdit(int ModNum) { PartGui::ViewProviderPart::unsetEdit(ModNum); } +void ViewProviderShapeBinder::attach(App::DocumentObject *obj) +{ + if (auto geo = dynamic_cast(obj)) { + geo->setMaterialAppearance(ShapeAppearance[0]); + } + ViewProviderPart::attach(obj); +} + void ViewProviderShapeBinder::highlightReferences(bool on) { App::GeoFeature* obj = nullptr; @@ -221,6 +229,9 @@ ViewProviderSubShapeBinder::ViewProviderSubShapeBinder() { void ViewProviderSubShapeBinder::attach(App::DocumentObject* obj) { UseBinderStyle.setValue(boost::istarts_with(obj->getNameInDocument(), "binder")); + if (auto geo = dynamic_cast(obj)) { + geo->setMaterialAppearance(ShapeAppearance[0]); + } ViewProviderPart::attach(obj); } diff --git a/src/Mod/PartDesign/Gui/ViewProviderShapeBinder.h b/src/Mod/PartDesign/Gui/ViewProviderShapeBinder.h index aef4562150..3037b3e862 100644 --- a/src/Mod/PartDesign/Gui/ViewProviderShapeBinder.h +++ b/src/Mod/PartDesign/Gui/ViewProviderShapeBinder.h @@ -44,6 +44,7 @@ public: protected: bool setEdit(int ModNum) override; void unsetEdit(int ModNum) override; + void attach(App::DocumentObject *obj) override; private: std::vector originalLineColors; From 8cba167241cf0c9609af71846816db940b65f447 Mon Sep 17 00:00:00 2001 From: wmayer Date: Mon, 8 Apr 2024 13:57:59 +0200 Subject: [PATCH 13/16] Part: fix some further regressions: * Correctly load a file if colors are set per face * Result of boolean operation is correctly colored if source objects have different colors * Result of compound is correctly colored if source objects have different colors --- src/Mod/Part/Gui/ViewProviderExt.cpp | 42 ++++++++++++++-------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/Mod/Part/Gui/ViewProviderExt.cpp b/src/Mod/Part/Gui/ViewProviderExt.cpp index 4321e86aa1..9393a1371f 100644 --- a/src/Mod/Part/Gui/ViewProviderExt.cpp +++ b/src/Mod/Part/Gui/ViewProviderExt.cpp @@ -318,28 +318,28 @@ void ViewProviderPartExt::onChanged(const App::Property* prop) else if (prop == &DiffuseColor) { setHighlightedFaces(DiffuseColor.getValues()); } -// else if (prop == &ShapeMaterial) { -// pcFaceBind->value = SoMaterialBinding::OVERALL; -// ViewProviderGeometryObject::onChanged(prop); -// App::Color c = ShapeAppearance.getDiffuseColor(); -// c.a = Transparency.getValue()/100.0f; -// DiffuseColor.setValue(c); -// } + else if (prop == &ShapeAppearance) { + pcFaceBind->value = SoMaterialBinding::OVERALL; + ViewProviderGeometryObject::onChanged(prop); + App::Color c = ShapeAppearance.getDiffuseColor(); + c.a = Transparency.getValue()/100.0f; + DiffuseColor.setValue(c); + } else if (prop == &Transparency) { -// const App::Material& Mat = ShapeMaterial.getValue(); -// long value = (long)(100*Mat.transparency); -// if (value != Transparency.getValue()) { -// float trans = Transparency.getValue()/100.0f; -// auto colors = DiffuseColor.getValues(); -// for (auto &c : colors) -// c.a = trans; -// DiffuseColor.setValues(colors); -// -// App::PropertyContainer* parent = ShapeMaterial.getContainer(); -// ShapeMaterial.setContainer(nullptr); -// ShapeMaterial.setTransparency(trans); -// ShapeMaterial.setContainer(parent); -// } + const App::Material& Mat = ShapeAppearance[0]; + long value = (long)(100*Mat.transparency); + if (value != Transparency.getValue()) { + float trans = Transparency.getValue()/100.0f; + auto colors = DiffuseColor.getValues(); + for (auto &c : colors) + c.a = trans; + DiffuseColor.setValues(colors); + + App::PropertyContainer* parent = ShapeAppearance.getContainer(); + ShapeAppearance.setContainer(nullptr); + ShapeAppearance.setTransparency(trans); + ShapeAppearance.setContainer(parent); + } } else if (prop == &Lighting) { if (Lighting.getValue() == 0) From af317be718e02fb4b2d9f7d5c45eb0f6755ccc6a Mon Sep 17 00:00:00 2001 From: wmayer Date: Mon, 8 Apr 2024 18:04:53 +0200 Subject: [PATCH 14/16] Core: avoid conversion from float to double or vice-versa When synchronizing the Transparency property with the transparency value of the ShapeAppearance property then do not convert between float and double as otherwise some strange rounding issues can occur. Example: Set the Transparency property of an object to 35 in the Property Editor. After leaving the editor the value may switch to 34. --- src/App/PropertyStandard.cpp | 8 ++++---- src/App/PropertyStandard.h | 8 ++++---- src/Gui/ViewProviderGeometryObject.cpp | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/App/PropertyStandard.cpp b/src/App/PropertyStandard.cpp index c46f62ee4b..07452a4de1 100644 --- a/src/App/PropertyStandard.cpp +++ b/src/App/PropertyStandard.cpp @@ -3033,22 +3033,22 @@ const Color& PropertyMaterialList::getEmissiveColor(int index) const return _lValueList[index].emissiveColor; } -double PropertyMaterialList::getShininess() const +float PropertyMaterialList::getShininess() const { return _lValueList[0].transparency; } -double PropertyMaterialList::getShininess(int index) const +float PropertyMaterialList::getShininess(int index) const { return _lValueList[index].transparency; } -double PropertyMaterialList::getTransparency() const +float PropertyMaterialList::getTransparency() const { return _lValueList[0].transparency; } -double PropertyMaterialList::getTransparency(int index) const +float PropertyMaterialList::getTransparency(int index) const { return _lValueList[index].transparency; } diff --git a/src/App/PropertyStandard.h b/src/App/PropertyStandard.h index a66e482f9c..ed617cc821 100644 --- a/src/App/PropertyStandard.h +++ b/src/App/PropertyStandard.h @@ -1166,11 +1166,11 @@ public: const Color& getEmissiveColor() const; const Color& getEmissiveColor(int index) const; - double getShininess() const; - double getShininess(int index) const; + float getShininess() const; + float getShininess(int index) const; - double getTransparency() const; - double getTransparency(int index) const; + float getTransparency() const; + float getTransparency(int index) const; PyObject* getPyObject() override; diff --git a/src/Gui/ViewProviderGeometryObject.cpp b/src/Gui/ViewProviderGeometryObject.cpp index 82d4295e47..34170b4f49 100644 --- a/src/Gui/ViewProviderGeometryObject.cpp +++ b/src/Gui/ViewProviderGeometryObject.cpp @@ -149,7 +149,7 @@ void ViewProviderGeometryObject::onChanged(const App::Property* prop) getObject()->touch(true); } const App::Material& Mat = ShapeAppearance[0]; - long value = (long)(100.0 * ShapeAppearance.getTransparency() + 0.5); + long value = (long)(100.0 * ShapeAppearance.getTransparency()); if (value != Transparency.getValue()) { Transparency.setValue(value); } From 0a5a1a5c32c9a028e90fe341ead4105129b78d3a Mon Sep 17 00:00:00 2001 From: wmayer Date: Mon, 8 Apr 2024 20:12:16 +0200 Subject: [PATCH 15/16] Part: fix import of STEP files with colors per face --- src/Mod/Part/Gui/ViewProviderExt.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/Mod/Part/Gui/ViewProviderExt.cpp b/src/Mod/Part/Gui/ViewProviderExt.cpp index 9393a1371f..eb2c3c44fe 100644 --- a/src/Mod/Part/Gui/ViewProviderExt.cpp +++ b/src/Mod/Part/Gui/ViewProviderExt.cpp @@ -844,11 +844,6 @@ void ViewProviderPartExt::updateData(const App::Property* prop) } } } - if (propName && strcmp(propName, "Shape") == 0) { - // Reapply the appearance - const App::Material& Mat = ShapeAppearance[0]; - setSoMaterial(Mat); - } Gui::ViewProviderGeometryObject::updateData(prop); } From 74f614a93d591cbb26c2f88de19fecec35773435 Mon Sep 17 00:00:00 2001 From: wmayer Date: Wed, 10 Apr 2024 13:32:58 +0200 Subject: [PATCH 16/16] Fix handling of transparency / Restore colour per face The Materials module does a conversion from float to double when saving the transparency and again a conversion from double to float to double when restoring it. This causes a considerable loss of accuracy so that the representation in percent leads to different numbers. Using consistently some helper functions to do a proper conversion from float to long and back fixes the problem. The new property ShapeAppearance is a PropertyMaterialList and always read after the DiffuseColor property when restoring a document. Thus, the method onChanged() doesn't override DiffuseColor when restoring a document. Additionally, the method finishRestoring() is re-implemented to set the colours per face in case DiffuseColor has defined multiple colors. --- src/Gui/ViewProviderGeometryObject.cpp | 19 ++++++++++-- src/Mod/Part/Gui/ViewProviderExt.cpp | 42 +++++++++++++++++++++++--- src/Mod/Part/Gui/ViewProviderExt.h | 6 ++++ 3 files changed, 59 insertions(+), 8 deletions(-) diff --git a/src/Gui/ViewProviderGeometryObject.cpp b/src/Gui/ViewProviderGeometryObject.cpp index 34170b4f49..6aa19ea99f 100644 --- a/src/Gui/ViewProviderGeometryObject.cpp +++ b/src/Gui/ViewProviderGeometryObject.cpp @@ -51,6 +51,19 @@ using namespace Gui; +// Helper functions to consistently convert between float and long +namespace { +float fromPercent(long value) +{ + return static_cast(value) / 100.0F; +} + +long toPercent(float value) +{ + return static_cast(100.0 * value + 0.5); +} +} + PROPERTY_SOURCE(Gui::ViewProviderGeometryObject, Gui::ViewProviderDragger) const App::PropertyIntegerConstraint::Constraints intPercent = {0, 100, 5}; @@ -137,9 +150,9 @@ void ViewProviderGeometryObject::onChanged(const App::Property* prop) setSelectable(Sel); } else if (prop == &Transparency) { - long value = (long)(100 * ShapeAppearance.getTransparency()); + long value = toPercent(ShapeAppearance.getTransparency()); if (value != Transparency.getValue()) { - float trans = (float)Transparency.getValue() / 100.0f; + float trans = fromPercent(Transparency.getValue()); pcShapeMaterial->transparency = trans; ShapeAppearance.setTransparency(trans); } @@ -149,7 +162,7 @@ void ViewProviderGeometryObject::onChanged(const App::Property* prop) getObject()->touch(true); } const App::Material& Mat = ShapeAppearance[0]; - long value = (long)(100.0 * ShapeAppearance.getTransparency()); + long value = toPercent(ShapeAppearance.getTransparency()); if (value != Transparency.getValue()) { Transparency.setValue(value); } diff --git a/src/Mod/Part/Gui/ViewProviderExt.cpp b/src/Mod/Part/Gui/ViewProviderExt.cpp index eb2c3c44fe..36eb1d65e2 100644 --- a/src/Mod/Part/Gui/ViewProviderExt.cpp +++ b/src/Mod/Part/Gui/ViewProviderExt.cpp @@ -96,6 +96,19 @@ FC_LOG_LEVEL_INIT("Part", true, true) using namespace PartGui; +// Helper functions to consistently convert between float and long +namespace { +float fromPercent(long value) +{ + return static_cast(value) / 100.0F; +} + +long toPercent(float value) +{ + return static_cast(100.0 * value + 0.5); +} +} + PROPERTY_SOURCE(PartGui::ViewProviderPartExt, Gui::ViewProviderGeometryObject) @@ -321,15 +334,19 @@ void ViewProviderPartExt::onChanged(const App::Property* prop) else if (prop == &ShapeAppearance) { pcFaceBind->value = SoMaterialBinding::OVERALL; ViewProviderGeometryObject::onChanged(prop); - App::Color c = ShapeAppearance.getDiffuseColor(); - c.a = Transparency.getValue()/100.0f; - DiffuseColor.setValue(c); + // While restoring a document do not override the + // DiffuseColor that has already been restored + if (!isRestoring()) { + App::Color c = ShapeAppearance.getDiffuseColor(); + c.a = fromPercent(Transparency.getValue()); + DiffuseColor.setValue(c); + } } else if (prop == &Transparency) { const App::Material& Mat = ShapeAppearance[0]; - long value = (long)(100*Mat.transparency); + long value = toPercent(Mat.transparency); if (value != Transparency.getValue()) { - float trans = Transparency.getValue()/100.0f; + float trans = fromPercent(Transparency.getValue()); auto colors = DiffuseColor.getValues(); for (auto &c : colors) c.a = trans; @@ -847,6 +864,21 @@ void ViewProviderPartExt::updateData(const App::Property* prop) Gui::ViewProviderGeometryObject::updateData(prop); } +void ViewProviderPartExt::startRestoring() +{ + Gui::ViewProviderGeometryObject::startRestoring(); +} + +void ViewProviderPartExt::finishRestoring() +{ + // The ShapeAppearance property is restored after DiffuseColor + // and currently sets a single color. + // In case DiffuseColor has defined multiple colors they will + // be passed to the scene graph now. + DiffuseColor.touch(); + Gui::ViewProviderGeometryObject::finishRestoring(); +} + void ViewProviderPartExt::setupContextMenu(QMenu* menu, QObject* receiver, const char* member) { QIcon iconObject = mergeGreyableOverlayIcons(Gui::BitmapFactory().pixmap("Part_ColorFace.svg")); diff --git a/src/Mod/Part/Gui/ViewProviderExt.h b/src/Mod/Part/Gui/ViewProviderExt.h index 1e5ac28648..85ccf105ef 100644 --- a/src/Mod/Part/Gui/ViewProviderExt.h +++ b/src/Mod/Part/Gui/ViewProviderExt.h @@ -99,6 +99,12 @@ public: void updateData(const App::Property*) override; + /** @name Restoring view provider from document load */ + //@{ + void startRestoring() override; + void finishRestoring() override; + //@} + /** @name Selection handling * This group of methods do the selection handling. * Here you can define how the selection for your ViewProfider