From 12ef38e66ffbcb8e8827e012d196b1f4ce84571b Mon Sep 17 00:00:00 2001 From: wmayer Date: Wed, 10 Apr 2024 13:32:58 +0200 Subject: [PATCH] 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