diff --git a/src/App/DocumentObject.cpp b/src/App/DocumentObject.cpp index 708b6ea3b9..8adcbc419b 100644 --- a/src/App/DocumentObject.cpp +++ b/src/App/DocumentObject.cpp @@ -723,6 +723,27 @@ void DocumentObject::onBeforeChange(const Property* prop) signalBeforeChange(*this,*prop); } +void DocumentObject::onEarlyChange(const Property *prop) +{ + if(GetApplication().isClosingAll()) + return; + + if(!GetApplication().isRestoring() && + !prop->testStatus(Property::PartialTrigger) && + getDocument() && + getDocument()->testStatus(Document::PartialDoc)) + { + static App::Document *warnedDoc; + if(warnedDoc != getDocument()) { + warnedDoc = getDocument(); + FC_WARN("Changes to partial loaded document will not be saved: " + << getFullName() << '.' << prop->getName()); + } + } + + signalEarlyChanged(*this, *prop); +} + /// get called by the container when a Property was changed void DocumentObject::onChanged(const Property* prop) { diff --git a/src/App/DocumentObject.h b/src/App/DocumentObject.h index b9b745ad54..f1271e1c1d 100644 --- a/src/App/DocumentObject.h +++ b/src/App/DocumentObject.h @@ -110,6 +110,8 @@ public: boost::signals2::signal signalBeforeChange; /// signal on changed property of this object boost::signals2::signal signalChanged; + /// signal on changed property of this object before document scoped signalChangedObject + boost::signals2::signal signalEarlyChanged; /// returns the type name of the ViewProvider virtual const char* getViewProviderName() const { @@ -607,6 +609,8 @@ protected: void onBeforeChange(const Property* prop) override; /// get called by the container when a property was changed void onChanged(const Property* prop) override; + /// get called by the container when a property was changed + void onEarlyChange(const Property* prop) override; /// get called after a document has been fully restored virtual void onDocumentRestored(); /// get called after an undo/redo transaction is finished diff --git a/src/App/Property.cpp b/src/App/Property.cpp index a8bb121723..fd02085d85 100644 --- a/src/App/Property.cpp +++ b/src/App/Property.cpp @@ -211,8 +211,10 @@ void Property::destroy(Property *p) { void Property::touch() { PropertyCleaner guard(this); - if (father) + if (father) { + father->onEarlyChange(this); father->onChanged(this); + } StatusBits.set(Touched); } diff --git a/src/App/PropertyContainer.h b/src/App/PropertyContainer.h index 17a2102efc..1bb7ecd7db 100644 --- a/src/App/PropertyContainer.h +++ b/src/App/PropertyContainer.h @@ -237,6 +237,11 @@ public: protected: + /** get called by the container when a property has changed + * + * This function is called before onChanged() + */ + virtual void onEarlyChange(const Property* /*prop*/){} /// get called by the container when a property has changed virtual void onChanged(const Property* /*prop*/){} /// get called before the value is changed diff --git a/src/Mod/Part/App/AppPart.cpp b/src/Mod/Part/App/AppPart.cpp index de78cddefe..4190208200 100644 --- a/src/Mod/Part/App/AppPart.cpp +++ b/src/Mod/Part/App/AppPart.cpp @@ -398,6 +398,7 @@ PyMOD_INIT_FUNC(Part) Part::PropertyGeometryList ::init(); Part::PropertyShapeHistory ::init(); Part::PropertyFilletEdges ::init(); + Part::PropertyShapeCache ::init(); Part::PropertyTopoShapeList ::init(); Part::FaceMaker ::init(); diff --git a/src/Mod/Part/App/Attacher.cpp b/src/Mod/Part/App/Attacher.cpp index cebff12d76..70e0e19df9 100644 --- a/src/Mod/Part/App/Attacher.cpp +++ b/src/Mod/Part/App/Attacher.cpp @@ -771,15 +771,15 @@ void AttachEngine::readLinks(const App::PropertyLinkSubList &references, } App::GeoFeature* geof = static_cast(objs[i]); geofs[i] = geof; - const Part::TopoShape* shape; + Part::TopoShape shape; if (geof->isDerivedFrom(Part::Feature::getClassTypeId())){ - shape = &(static_cast(geof)->Shape.getShape()); - if (shape->isNull()){ + shape = (static_cast(geof)->Shape.getShape()); + if (shape.isNull()){ throw AttachEngineException("AttachEngine3D: Part has null shape"); } if (sub[i].length()>0){ try{ - storage.push_back(shape->getSubShape(sub[i].c_str())); + storage.push_back(shape.getSubShape(sub[i].c_str())); } catch (Standard_Failure&){ throw AttachEngineException("AttachEngine3D: subshape not found"); } @@ -787,7 +787,7 @@ void AttachEngine::readLinks(const App::PropertyLinkSubList &references, throw AttachEngineException("AttachEngine3D: null subshape"); shapes[i] = &(storage[storage.size()-1]); } else { - shapes[i] = &(shape->getShape()); + shapes[i] = &(shape.getShape()); } } else if ( geof->isDerivedFrom(App::Plane::getClassTypeId()) ){ //obtain Z axis and origin of placement diff --git a/src/Mod/Part/App/PropertyTopoShape.cpp b/src/Mod/Part/App/PropertyTopoShape.cpp index c32889ab99..02d7d45426 100644 --- a/src/Mod/Part/App/PropertyTopoShape.cpp +++ b/src/Mod/Part/App/PropertyTopoShape.cpp @@ -103,8 +103,10 @@ TopoShape PropertyPartShape::getShape() const { _Shape.initCache(-1); auto res = _Shape; -// if (Feature::isElementMappingDisabled(getContainer())) - if ( false ) // TODO Implement + // March, 2024 Toponaming project: There was originally an unused feature to disable elementMapping + // that has not been kept: + // if (Feature::isElementMappingDisabled(getContainer())) + if ( false ) res.Tag = -1; else if (!res.Tag) { if (auto parent = Base::freecad_dynamic_cast(getContainer())) @@ -184,7 +186,9 @@ void PropertyPartShape::setPyObject(PyObject *value) shape = res; }else{ shape.Tag = owner->getID(); - shape.Hasher->clear(); // reset(); + if ( shape.Hasher ) { // TODO: This null guard added during TNP transition + shape.Hasher->clear(); + } } } setValue(shape); @@ -200,20 +204,15 @@ App::Property *PropertyPartShape::Copy() const { PropertyPartShape *prop = new PropertyPartShape(); + // March, 2024 Toponaming project: There was originally a feature to enable making an element + // copy ( new geometry and map ) that has not been kept: // if (PartParams::getShapePropertyCopy()) { - if ( false ) { // TODO: PartParams - // makeElementCopy() consume too much memory for complex geometry. - prop->_Shape = this->_Shape.makeElementCopy(); - } else - prop->_Shape = this->_Shape; - prop->_Ver = this->_Ver; - +// // makeElementCopy() consume too much memory for complex geometry. +// prop->_Shape = this->_Shape.makeElementCopy(); +// } else +// prop->_Shape = this->_Shape; prop->_Shape = this->_Shape; - if (!_Shape.getShape().IsNull()) { - BRepBuilderAPI_Copy copy(_Shape.getShape()); - prop->_Shape.setShape(copy.Shape()); - } - + prop->_Ver = this->_Ver; return prop; } @@ -348,6 +347,7 @@ void PropertyPartShape::Restore(Base::XMLReader &reader) reader.addFile(file.c_str(),this); } } + #ifdef NOT_YET_AND_MAYBE_NEVER void PropertyPartShape::Restore(Base::XMLReader &reader) { @@ -433,7 +433,6 @@ void PropertyPartShape::Restore(Base::XMLReader &reader) hasSetValue(); } } -#endif void PropertyPartShape::afterRestore() { @@ -447,6 +446,7 @@ void PropertyPartShape::afterRestore() _Shape.Hasher->clear(); //reset(); PropertyComplexGeoData::afterRestore(); } +#endif // The following function is copied from OCCT BRepTools.cxx and modified // to disable saving of triangulation @@ -912,6 +912,10 @@ void PropertyShapeCache::Restore(Base::XMLReader &) { } +/** + * Make a new python List with a tuple for each cache entry containing the key and the shape + * @return the python list + */ PyObject *PropertyShapeCache::getPyObject() { Py::List res; for(auto &v : cache) @@ -919,6 +923,10 @@ PyObject *PropertyShapeCache::getPyObject() { return Py::new_reference_to(res); } +/** + * Remove the cache entries for every element in the list + * @param value A python list of entry names + */ void PropertyShapeCache::setPyObject(PyObject *value) { if(!value) return; @@ -933,6 +941,12 @@ void PropertyShapeCache::setPyObject(PyObject *value) { } #define SHAPE_CACHE_NAME "_Part_ShapeCache" +/** + * Find or create the shape cache for a document object + * @param obj The document object + * @param create True if we should create the cache if it doesn't exist + * @return The shape cache, or null if we aren't creating and it doesn't exist + */ PropertyShapeCache *PropertyShapeCache::get(const App::DocumentObject *obj, bool create) { auto prop = Base::freecad_dynamic_cast( obj->getDynamicPropertyByName(SHAPE_CACHE_NAME)); @@ -953,9 +967,18 @@ PropertyShapeCache *PropertyShapeCache::get(const App::DocumentObject *obj, bool return prop; } +/** + * Look up and return a shape in the cache + * @param obj The document object to look in + * @param shape The found shape is returned here + * @param subname The key to look up + * @return True if the name was found + */ bool PropertyShapeCache::getShape(const App::DocumentObject *obj, TopoShape &shape, const char *subname) { +// March, 2024 Toponaming project: There was originally a feature to disable shape cache +// that has not been kept: // if (PartParams::getDisableShapeCache()) -// return false; //TODO PartParams +// return false; auto prop = get(obj,false); if(!prop) return false; @@ -968,11 +991,19 @@ bool PropertyShapeCache::getShape(const App::DocumentObject *obj, TopoShape &sha return false; } +/** + * Find or create the property shape cache in a document object and then add an entry + * @param obj The Object + * @param shape The shape to cache + * @param subname The key to point at that shape + */ void PropertyShapeCache::setShape( const App::DocumentObject *obj, const TopoShape &shape, const char *subname) { +// March, 2024 Toponaming project: There was originally a feature to disable shape cache +// that has not been kept: // if (PartParams::getDisableShapeCache()) -// return; // TODO: Part Params +// return; auto prop = get(obj,true); if(!prop) return; diff --git a/src/Mod/Part/App/PropertyTopoShape.h b/src/Mod/Part/App/PropertyTopoShape.h index 32df7f4ff3..fd9bb42570 100644 --- a/src/Mod/Part/App/PropertyTopoShape.h +++ b/src/Mod/Part/App/PropertyTopoShape.h @@ -102,7 +102,7 @@ public: virtual std::string getElementMapVersion(bool restored=false) const override; void resetElementMapVersion() {_Ver.clear();} - virtual void afterRestore() override; +// virtual void afterRestore() override; friend class Feature; diff --git a/tests/src/Mod/Part/App/CMakeLists.txt b/tests/src/Mod/Part/App/CMakeLists.txt index ceea6c5787..7b93f834d6 100644 --- a/tests/src/Mod/Part/App/CMakeLists.txt +++ b/tests/src/Mod/Part/App/CMakeLists.txt @@ -14,6 +14,7 @@ target_sources( ${CMAKE_CURRENT_SOURCE_DIR}/PartFeature.cpp ${CMAKE_CURRENT_SOURCE_DIR}/PartFeatures.cpp ${CMAKE_CURRENT_SOURCE_DIR}/PartTestHelpers.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/PropertyTopoShape.cpp ${CMAKE_CURRENT_SOURCE_DIR}/TopoShape.cpp ${CMAKE_CURRENT_SOURCE_DIR}/TopoShapeCache.cpp ${CMAKE_CURRENT_SOURCE_DIR}/TopoShapeExpansion.cpp diff --git a/tests/src/Mod/Part/App/FeaturePartCommon.cpp b/tests/src/Mod/Part/App/FeaturePartCommon.cpp index ab53c936d3..8f7b6bc4ec 100644 --- a/tests/src/Mod/Part/App/FeaturePartCommon.cpp +++ b/tests/src/Mod/Part/App/FeaturePartCommon.cpp @@ -209,8 +209,6 @@ TEST_F(FeaturePartCommonTest, testMapping) { // Arrange - _boxes[0]->Shape.getShape().Tag = 1L; - _boxes[1]->Shape.getShape().Tag = 2L; _common->Base.setValue(_boxes[0]); _common->Tool.setValue(_boxes[1]); // Act diff --git a/tests/src/Mod/Part/App/PartFeature.cpp b/tests/src/Mod/Part/App/PartFeature.cpp index f42d306245..6dc7465f07 100644 --- a/tests/src/Mod/Part/App/PartFeature.cpp +++ b/tests/src/Mod/Part/App/PartFeature.cpp @@ -34,8 +34,6 @@ protected: TEST_F(FeaturePartTest, testGetElementName) { // Arrange - _boxes[0]->Shape.getShape().Tag = 1L; - _boxes[1]->Shape.getShape().Tag = 2L; _common->Base.setValue(_boxes[0]); _common->Tool.setValue(_boxes[1]); diff --git a/tests/src/Mod/Part/App/PartFeatures.cpp b/tests/src/Mod/Part/App/PartFeatures.cpp index 2ec5c96bed..fab72f6378 100644 --- a/tests/src/Mod/Part/App/PartFeatures.cpp +++ b/tests/src/Mod/Part/App/PartFeatures.cpp @@ -38,14 +38,12 @@ TEST_F(PartFeaturesTest, testRuledSurface) _edge1->X2.setValue(2); _edge1->Y2.setValue(0); _edge1->Z2.setValue(0); - _edge1->Shape.getShape().Tag = 1L; // TODO: Can remove when TNP is on? _edge2->X1.setValue(0); _edge2->Y1.setValue(2); _edge2->Z1.setValue(0); _edge2->X2.setValue(2); _edge2->Y2.setValue(2); _edge2->Z2.setValue(0); - _edge2->Shape.getShape().Tag = 2L; // TODO: Can remove when TNP is on? auto _ruled = dynamic_cast(_doc->addObject("Part::RuledSurface")); _ruled->Curve1.setValue(_edge1); _ruled->Curve2.setValue(_edge2); diff --git a/tests/src/Mod/Part/App/PropertyTopoShape.cpp b/tests/src/Mod/Part/App/PropertyTopoShape.cpp new file mode 100644 index 0000000000..e1eb5851b0 --- /dev/null +++ b/tests/src/Mod/Part/App/PropertyTopoShape.cpp @@ -0,0 +1,188 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later + +#include "gtest/gtest.h" + +#include +#include "Mod/Part/App/FeaturePartCommon.h" +#include "Mod/Part/App/PropertyTopoShape.h" +#include +#include "PartTestHelpers.h" +#include "Mod/Part/App/TopoShapeCompoundPy.h" + +using namespace Part; +using namespace PartTestHelpers; + +class PropertyTopoShapeTest: public ::testing::Test, public PartTestHelperClass +{ +protected: + static void SetUpTestSuite() + { + tests::initApplication(); + } + + void SetUp() override + { + createTestDoc(); + _common = dynamic_cast(_doc->addObject("Part::Common")); + _common->Base.setValue(_boxes[0]); + _common->Tool.setValue(_boxes[1]); + _common->execute(); // We should now have an elementMap with 26 entries. + } + + void TearDown() override + {} + + Common* _common = nullptr; // NOLINT Can't be private in a test framework +}; + +TEST_F(PropertyTopoShapeTest, testPropertyPartShapeTopoShape) +{ + // Arrange + auto partShape = PropertyPartShape(); + auto topoShapeIn = _common->Shape.getShape(); + auto topoDsShapeIn = _common->Shape.getShape().getShape(); + // Act + partShape.setValue(topoShapeIn); + auto topoShapeOut = partShape.getShape(); + auto topoDsShapeOut = partShape.getValue(); + // Assert + EXPECT_TRUE(topoShapeOut.isSame(topoShapeIn)); + EXPECT_TRUE(topoDsShapeOut.IsSame(topoDsShapeIn)); + EXPECT_EQ(getVolume(topoDsShapeOut), 3); +#ifdef FC_USE_TNP_FIX + EXPECT_EQ(topoShapeOut.getElementMapSize(), 26); +#else + EXPECT_EQ(topoShapeOut.getElementMapSize(), 0); +#endif +} + +TEST_F(PropertyTopoShapeTest, testPropertyPartShapeTopoDSShape) +{ + // Arrange + auto property = _boxes[0]->addDynamicProperty("Part::PropertyPartShape", "test"); + auto partShape = dynamic_cast(property); + auto topoShapeIn = _common->Shape.getShape(); + auto topoDsShapeIn = _common->Shape.getShape().getShape(); + // Act + // The second parameter of setValue, whether to resetElementMap is never called and irrelevant. + // It appears to exist only to pass to the lower level setShape call. + partShape->setValue(topoDsShapeIn); + auto topoShapeOut = partShape->getShape(); + auto topoDsShapeOut = partShape->getValue(); + // Assert + EXPECT_FALSE(topoShapeOut.isSame(topoShapeIn)); + EXPECT_TRUE(topoDsShapeOut.IsSame(topoDsShapeIn)); + EXPECT_EQ(getVolume(topoDsShapeOut), 3); + EXPECT_EQ(topoShapeOut.getElementMapSize(), 0); // We passed in a TopoDS_Shape so lost the map +} + +TEST_F(PropertyTopoShapeTest, testPropertyPartShapeGetPyObject) +{ + // Arrange + Py_Initialize(); + Base::PyGILStateLocker lock; + auto partShape = PropertyPartShape(); + auto topoDsShapeIn = _common->Shape.getShape().getShape(); + auto topoDsShapeIn2 = _boxes[3]->Shape.getShape().getShape(); + // Act + partShape.setValue(topoDsShapeIn); + auto pyObj = partShape.getPyObject(); + // Assert + EXPECT_TRUE(PyObject_TypeCheck(pyObj, &TopoShapeCompoundPy::Type)); // _common is a compound. + // We can't build a TopoShapeCompoundPy directly ( protected destructor ), so we'll flip + // the compound out and in to prove setPyObject works as expected. + // Act + partShape.setValue(topoDsShapeIn2); + auto pyObj2 = partShape.getPyObject(); + // Assert the shape is no longer a compound + EXPECT_FALSE( + PyObject_TypeCheck(pyObj2, &TopoShapeCompoundPy::Type)); // _boxes[3] is not a compound. + Py_XDECREF(pyObj2); + // Act + partShape.setPyObject(pyObj); + auto pyObj3 = partShape.getPyObject(); + // Assert the shape returns to a compound if we setPyObject + EXPECT_TRUE(PyObject_TypeCheck(pyObj3, &TopoShapeCompoundPy::Type)); // _common is a compound. + Py_XDECREF(pyObj3); + Py_XDECREF(pyObj); +} + +// Possible future PropertyPartShape tests: +// Copy, Paste, getMemSize, beforeSave, Save. Restore + + +TEST_F(PropertyTopoShapeTest, testShapeHistory) +{ + // Arrange + TopoDS_Shape baseShape = _boxes[0]->Shape.getShape().getShape(); + BRepFilletAPI_MakeFillet mkFillet(baseShape); + auto edges = _boxes[0]->Shape.getShape().getSubTopoShapes(TopAbs_EDGE); + mkFillet.Add(0.1, 0.1, TopoDS::Edge(edges[0].getShape())); + TopoDS_Shape newShape = mkFillet.Shape(); + // Act to test the constructor + auto hist = ShapeHistory(mkFillet, TopAbs_EDGE, newShape, baseShape); + // Assert + EXPECT_EQ(hist.type, TopAbs_EDGE); + EXPECT_EQ(hist.shapeMap.size(), 11); // We filleted away one of the cubes 12 Edges + // Act to test the join operation + hist.join(hist); + // Assert + EXPECT_EQ(hist.type, TopAbs_EDGE); + EXPECT_EQ(hist.shapeMap.size(), 9); // TODO: Is this correct? + // Act to test the reset operation + hist.reset(mkFillet, TopAbs_VERTEX, newShape, baseShape); + // Assert + EXPECT_EQ(hist.type, TopAbs_VERTEX); + EXPECT_EQ(hist.shapeMap.size(), 8); // Vertexes on a cube. +} + +TEST_F(PropertyTopoShapeTest, testPropertyShapeHistory) +{ + // N/A nothing to really test +} + +TEST_F(PropertyTopoShapeTest, testPropertyShapeCache) +{ + // Arrange + PropertyShapeCache propertyShapeCache; + TopoShape topoShapeIn {_boxes[0]->Shape.getShape()}; // Any TopoShape to test with + TopoShape topoShapeOut; + char* subName = "Face1"; // Cache key + // Act + auto gotShapeNotYet = propertyShapeCache.getShape(_boxes[0], topoShapeOut, subName); + propertyShapeCache.setShape(_boxes[0], topoShapeIn, subName); + auto gotShapeGood = propertyShapeCache.getShape(_boxes[0], topoShapeOut, subName); + // Assert + ASSERT_FALSE(gotShapeNotYet); + ASSERT_TRUE(gotShapeGood); + EXPECT_EQ(getVolume(topoShapeIn.getShape()), 6); + EXPECT_EQ(getVolume(topoShapeOut.getShape()), 6); + EXPECT_TRUE(topoShapeIn.isSame(topoShapeOut)); +} + +TEST_F(PropertyTopoShapeTest, testPropertyShapeCachePyObj) +{ + // Arrange + Py_Initialize(); + Base::PyGILStateLocker lock; + PropertyShapeCache catalyst; + auto propertyShapeCache = catalyst.get(_boxes[1], true); + auto faces = _boxes[1]->Shape.getShape().getSubTopoShapes(TopAbs_FACE); + propertyShapeCache->setShape(_boxes[1], faces[0], "Face1"); + propertyShapeCache->setShape(_boxes[1], faces[1], "Face2"); + propertyShapeCache->setShape(_boxes[1], faces[2], "Face3"); + propertyShapeCache->setShape(_boxes[1], faces[3], "Face4"); + auto eraseList = PyList_New(2); + PyList_SetItem(eraseList, 0, PyUnicode_FromString("Face2")); + PyList_SetItem(eraseList, 1, PyUnicode_FromString("Face3")); + // Act + auto pyObjOut = propertyShapeCache->getPyObject(); + propertyShapeCache->setPyObject(eraseList); // pyObjInRepr); + auto pyObjOutErased = propertyShapeCache->getPyObject(); + // The faces that come back from python are in a random order compared to what was passed in + // so testing them individually is difficult. We'll just make sure the counts are right. + EXPECT_EQ(PyList_Size(pyObjOut), 4); // All four faces we added to the cache + EXPECT_EQ(PyList_Size(pyObjOutErased), 2); // setPyObject removed Face2 and Face3 + Py_XDECREF(pyObjOut); + Py_XDECREF(pyObjOutErased); +}