diff --git a/src/App/GeoFeature.cpp b/src/App/GeoFeature.cpp index 344aea1a58..0f7175e3c6 100644 --- a/src/App/GeoFeature.cpp +++ b/src/App/GeoFeature.cpp @@ -48,6 +48,11 @@ PROPERTY_SOURCE(App::GeoFeature, App::DocumentObject) GeoFeature::GeoFeature() { ADD_PROPERTY_TYPE(Placement, (Base::Placement()), nullptr, Prop_NoRecompute, nullptr); + ADD_PROPERTY_TYPE(_ElementMapVersion, + (""), + "Base", + (App::PropertyType)(Prop_Output | Prop_Hidden | Prop_Transient), + ""); } GeoFeature::~GeoFeature() = default; @@ -239,6 +244,16 @@ void GeoFeature::updateElementReference() return; } bool reset = false; + + auto version = getElementMapVersion(prop); + if (_ElementMapVersion.getStrValue().empty()) { + _ElementMapVersion.setValue(version); + } + else if (_ElementMapVersion.getStrValue() != version) { + reset = true; + _ElementMapVersion.setValue(version); + } + PropertyLinkBase::updateElementReferences(this, reset); } @@ -253,6 +268,14 @@ void GeoFeature::onChanged(const Property* prop) DocumentObject::onChanged(prop); } +void GeoFeature::onDocumentRestored() +{ + if (!getDocument()->testStatus(Document::Status::Importing)) { + _ElementMapVersion.setValue(getElementMapVersion(getPropertyOfGeometry(), true)); + } + DocumentObject::onDocumentRestored(); +} + const std::vector& GeoFeature::searchElementCache(const std::string& element, Data::SearchOptions options, double tol, diff --git a/src/App/GeoFeature.h b/src/App/GeoFeature.h index abc36912c5..67f43a3bd6 100644 --- a/src/App/GeoFeature.h +++ b/src/App/GeoFeature.h @@ -42,6 +42,7 @@ class AppExport GeoFeature: public App::DocumentObject public: PropertyPlacement Placement; + PropertyString _ElementMapVersion; /// Constructor GeoFeature(); @@ -199,7 +200,7 @@ public: protected: void onChanged(const Property* prop) override; - // void onDocumentRestored() override; + void onDocumentRestored() override; void updateElementReference(); protected: diff --git a/src/Mod/Part/App/PartFeature.cpp b/src/Mod/Part/App/PartFeature.cpp index fdb2c3fbd4..fe805c9c84 100644 --- a/src/Mod/Part/App/PartFeature.cpp +++ b/src/Mod/Part/App/PartFeature.cpp @@ -1581,6 +1581,12 @@ Feature* Feature::create(const TopoShape& shape, const char* name, App::Document return res; } +void Feature::onDocumentRestored() +{ + // expandShapeContents(); + App::GeoFeature::onDocumentRestored(); +} + ShapeHistory Feature::buildHistory(BRepBuilderAPI_MakeShape& mkShape, TopAbs_ShapeEnum type, const TopoDS_Shape& newS, const TopoDS_Shape& oldS) { diff --git a/src/Mod/Part/App/PartFeature.h b/src/Mod/Part/App/PartFeature.h index 50abe520cd..6c1a63437b 100644 --- a/src/Mod/Part/App/PartFeature.h +++ b/src/Mod/Part/App/PartFeature.h @@ -168,6 +168,7 @@ protected: App::DocumentObjectExecReturn *execute() override; void onBeforeChange(const App::Property* prop) override; void onChanged(const App::Property* prop) override; + void onDocumentRestored() override; void copyMaterial(Feature* feature); void copyMaterial(App::DocumentObject* link); diff --git a/src/Mod/Part/App/PropertyTopoShape.cpp b/src/Mod/Part/App/PropertyTopoShape.cpp index 39d9283627..1a71419533 100644 --- a/src/Mod/Part/App/PropertyTopoShape.cpp +++ b/src/Mod/Part/App/PropertyTopoShape.cpp @@ -587,11 +587,25 @@ void PropertyPartShape::SaveDocFile (Base::Writer &writer) const void PropertyPartShape::RestoreDocFile(Base::Reader &reader) { + + // save the element map + auto elementMap = _Shape.resetElementMap(); + auto hasher = _Shape.Hasher; + Base::FileInfo brep(reader.getFileName()); + TopoShape shape; + + // In LS3 the following statement is executed right before shape.Hasher = hasher; + // https://github.com/realthunder/FreeCAD/blob/a9810d509a6f112b5ac03d4d4831b67e6bffd5b7/src/Mod/Part/App/PropertyTopoShape.cpp#L639 + // Now it's not possible anymore because both PropertyPartShape::loadFromFile() and + // PropertyPartShape::loadFromStream() calls PropertyPartShape::setValue() which clears the + // value of _Ver. + // Therefor we're storing the value of _Ver here so that we don't lose it. + + std::string ver = _Ver; + if (brep.hasExtension("bin")) { - TopoShape shape; shape.importBinary(reader); - setValue(shape); } else { bool direct = App::GetApplication().GetParameterGroupByPath @@ -604,7 +618,14 @@ void PropertyPartShape::RestoreDocFile(Base::Reader &reader) loadFromStream(reader); reader.exceptions(iostate); } + shape = getValue(); } + + // restore the element map + shape.Hasher = hasher; + shape.resetElementMap(elementMap); + setValue(shape); + _Ver = ver; } // ------------------------------------------------------------------------- diff --git a/src/Mod/Part/App/TopoShape.h b/src/Mod/Part/App/TopoShape.h index 878333b36e..ae94e6cea8 100644 --- a/src/Mod/Part/App/TopoShape.h +++ b/src/Mod/Part/App/TopoShape.h @@ -1502,6 +1502,8 @@ public: void mapSubElementsTo(std::vector& shapes, const char* op = nullptr) const; bool hasPendingElementMap() const; + std::string getElementMapVersion() const override; + void flushElementMap() const override; Data::ElementMapPtr resetElementMap( diff --git a/src/Mod/Part/App/TopoShapeExpansion.cpp b/src/Mod/Part/App/TopoShapeExpansion.cpp index 710950afa9..b3ddff0db1 100644 --- a/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -2057,6 +2057,27 @@ TopoShape TopoShape::getSubTopoShape(TopAbs_ShapeEnum type, int idx, bool silent return shapeMap.getTopoShape(*this, idx); } +static const std::string& _getElementMapVersion() +{ + static std::string _ver; + if (_ver.empty()) { + std::ostringstream ss; + // Stabilize the reported OCCT version: report 7.2.0 as the version so that we aren't + // constantly inadvertently reporting differing versions. This is retained for + // cross-compatibility with LinkStage3 (which retains supporting code for OCCT 6.x, + // removed here). + unsigned occ_ver {0x070200}; + ss << OpCodes::Version << '.' << std::hex << occ_ver << '.'; + _ver = ss.str(); + } + return _ver; +} + +std::string TopoShape::getElementMapVersion() const +{ + return _getElementMapVersion() + Data::ComplexGeoData::getElementMapVersion(); +} + TopoShape& TopoShape::makeElementEvolve(const TopoShape& spine, const TopoShape& profile, JoinType join, diff --git a/tests/src/Mod/PartDesign/App/BackwardCompatibility.cpp b/tests/src/Mod/PartDesign/App/BackwardCompatibility.cpp new file mode 100644 index 0000000000..49e593bae0 --- /dev/null +++ b/tests/src/Mod/PartDesign/App/BackwardCompatibility.cpp @@ -0,0 +1,123 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later + +#include +#include +#include +#include +#include "src/App/InitApplication.h" + +#include +#include +#include + +// NOLINTBEGIN(readability-magic-numbers,cppcoreguidelines-avoid-magic-numbers) + +class BackwardCompatibilityTest: public ::testing::Test +{ +protected: + static void SetUpTestSuite() + { + tests::initApplication(); + } + + void SetUp() override + { + _testPath = App::Application::getHomePath() + "tests/TestModels/"; + } + + void TearDown() override + { + App::GetApplication().closeDocument(_doc->getName()); + } + + const App::Document* getDocument() const + { + return _doc; + } + + void setDocument(App::Document* doc) + { + _doc = doc; + } + + std::string getTestPath() const + { + return _testPath; + } + +private: + App::Document* _doc = nullptr; + std::string _testPath; +}; + +TEST_F(BackwardCompatibilityTest, TestOpenV021Model) +{ + + // arrange + + auto doc = App::GetApplication().openDocument( + std::string(getTestPath() + "ModelFromV021.FCStd").c_str()); + setDocument(doc); + + auto chamfer = dynamic_cast(doc->getObject("Chamfer")); + auto chamferEdgesNames = chamfer->Base.getSubValues(); + + std::vector chamferOriginalEdges {}; + for (const auto& chamferEdgesName : chamferEdgesNames) { + chamferOriginalEdges.push_back( + chamfer->getBaseTopoShape().getSubTopoShape(chamferEdgesName.c_str()).getShape()); + } + + // act + + doc->recompute(); + chamfer = dynamic_cast(doc->getObject("Chamfer")); + chamferEdgesNames = chamfer->Base.getSubValues(); + + // assert + + auto checkSameVertexes = [](const TopoDS_Shape& e1, const TopoDS_Shape& e2) { + TopExp_Explorer e1Vertexes(e1, TopAbs_VERTEX); + TopExp_Explorer e2Vertexes(e2, TopAbs_VERTEX); + + bool sameCoords {true}; + bool moreToCheck {e1Vertexes.More() && e2Vertexes.More()}; + + // If one of the vertexes doesn't have the same coordinates of the other one then it'll be + // useless to continue + while (moreToCheck && sameCoords) { + auto p1 = BRep_Tool::Pnt(TopoDS::Vertex(e1Vertexes.Current())); + auto p2 = BRep_Tool::Pnt(TopoDS::Vertex(e2Vertexes.Current())); + + sameCoords &= (p1.X() == p2.X() && p1.Y() == p2.Y() && p1.Z() == p2.Z()); + e1Vertexes.Next(); + e2Vertexes.Next(); + + moreToCheck = (e1Vertexes.More() && e2Vertexes.More()); + + // Extra check: both edges should have the same number of vertexes (e*Vertexes.More() + // should be true or false at the same time for both the edges). + // If this doesn't happen then one of the edges won't have enough vertexes to perform a + // full coordinates comparison. + // This shouldn't happen, so it's here just in case + sameCoords &= (e1Vertexes.More() == e2Vertexes.More()); + } + + return sameCoords; + }; + + EXPECT_TRUE(checkSameVertexes( + chamfer->getBaseTopoShape().getSubTopoShape(chamferEdgesNames[0].c_str()).getShape(), + chamferOriginalEdges[0])); + EXPECT_TRUE(checkSameVertexes( + chamfer->getBaseTopoShape().getSubTopoShape(chamferEdgesNames[1].c_str()).getShape(), + chamferOriginalEdges[1])); + EXPECT_TRUE(checkSameVertexes( + chamfer->getBaseTopoShape().getSubTopoShape(chamferEdgesNames[2].c_str()).getShape(), + chamferOriginalEdges[2])); + EXPECT_TRUE(checkSameVertexes( + chamfer->getBaseTopoShape().getSubTopoShape(chamferEdgesNames[3].c_str()).getShape(), + chamferOriginalEdges[3])); +} + +// NOLINTEND(readability-magic-numbers,cppcoreguidelines-avoid-magic-numbers) diff --git a/tests/src/Mod/PartDesign/App/CMakeLists.txt b/tests/src/Mod/PartDesign/App/CMakeLists.txt index 007fd820de..48583700bc 100644 --- a/tests/src/Mod/PartDesign/App/CMakeLists.txt +++ b/tests/src/Mod/PartDesign/App/CMakeLists.txt @@ -2,7 +2,22 @@ target_sources( PartDesign_tests_run PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR}/BackwardCompatibility.cpp ${CMAKE_CURRENT_SOURCE_DIR}/DatumPlane.cpp ${CMAKE_CURRENT_SOURCE_DIR}/ShapeBinder.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Pad.cpp ) + +set(PartDesignTestData_Files + TestModels/ModelFromV021.FCStd +) + +ADD_CUSTOM_TARGET(PartDesignTestData ALL + SOURCES ${PartDesignTestData_Files} +) + +fc_target_copy_resource(PartDesignTestData + ${CMAKE_CURRENT_SOURCE_DIR} + ${CMAKE_BINARY_DIR}/tests + ${PartDesignTestData_Files} +) diff --git a/tests/src/Mod/PartDesign/App/TestModels/ModelFromV021.FCStd b/tests/src/Mod/PartDesign/App/TestModels/ModelFromV021.FCStd new file mode 100644 index 0000000000..2a062d8361 Binary files /dev/null and b/tests/src/Mod/PartDesign/App/TestModels/ModelFromV021.FCStd differ