From 92b1e777448bb939c2139f28a90e0bbdef748321 Mon Sep 17 00:00:00 2001
From: wmayer <wmayer@users.sourceforge.net>
Date: Fri, 5 Apr 2024 17:29:10 +0200
Subject: [PATCH] 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<App::GeoFeature*>(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<const App::PropertyComplexGeoData*>(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);
 }