From 5c582b7b2d8cd260176dfed8d0f06b968601f3c7 Mon Sep 17 00:00:00 2001 From: bgbsww Date: Wed, 28 Feb 2024 15:56:45 -0500 Subject: [PATCH] Toposhape/Part:: fix, relocate and test element methods in ComplexGeoData and TopoShape --- src/App/ComplexGeoData.cpp | 77 ------------ src/App/ComplexGeoData.h | 24 ++-- src/App/ElementMap.cpp | 91 ++++++++++++++ src/App/ElementMap.h | 24 ++++ src/Mod/Part/App/FeaturePartCommon.cpp | 2 +- src/Mod/Part/App/TopoShape.h | 8 ++ src/Mod/Part/App/TopoShapeExpansion.cpp | 15 ++- tests/src/App/ComplexGeoData.cpp | 18 +++ tests/src/App/ElementMap.cpp | 1 - tests/src/Mod/Part/App/FeaturePartCommon.cpp | 2 +- tests/src/Mod/Part/App/TopoShapeExpansion.cpp | 116 ++++++++++++++++++ 11 files changed, 273 insertions(+), 105 deletions(-) diff --git a/src/App/ComplexGeoData.cpp b/src/App/ComplexGeoData.cpp index 89941ea3f3..cf99b2a22c 100644 --- a/src/App/ComplexGeoData.cpp +++ b/src/App/ComplexGeoData.cpp @@ -637,83 +637,6 @@ unsigned int ComplexGeoData::getMemSize() const return 0; } -void ComplexGeoData::traceElement(const MappedName &name, TraceCallback cb) const -{ - long tag = this->Tag, encodedTag = 0; - int len = 0; - - auto pos = findTagInElementName(name,&encodedTag,&len,nullptr,nullptr,true); - if(cb(name, len, encodedTag, tag) || pos < 0) - return; - - if (name.startsWith(externalTagPostfix(), len)) - return; - - std::set tagSet; - - std::vector names; - if (tag) - tagSet.insert(std::abs(tag)); - if (encodedTag) - tagSet.insert(std::abs(encodedTag)); - names.push_back(name); - - tag = encodedTag; - MappedName tmp; - bool first = true; - - // TODO: element tracing without object is inheriently unsafe, because of - // possible external linking object which means the element may be encoded - // using external string table. Looking up the wrong table may accidentally - // cause circular mapping, and is actually quite easy to reproduce. See - // - // https://github.com/realthunder/FreeCAD_assembly3/issues/968 - // - // A random depth limit is set here to not waste time. 'tagSet' above is - // also used for early detection of 'recursive' mapping. - - for (int i=0; i<50; ++i) { - if(!len || len>pos) - return; - if(first) { - first = false; - size_t offset = 0; - if(name.startsWith(elementMapPrefix())) - offset = elementMapPrefix().size(); - tmp = MappedName(name, offset, len); - }else - tmp = MappedName(tmp, 0, len); - tmp = dehashElementName(tmp); - names.push_back(tmp); - encodedTag = 0; - pos = findTagInElementName(tmp,&encodedTag,&len,nullptr,nullptr,true); - if (pos >= 0 && tmp.startsWith(externalTagPostfix(), len)) - break; - - if (encodedTag && tag != std::abs(encodedTag) - && !tagSet.insert(std::abs(encodedTag)).second) { - if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) { - FC_WARN("circular element mapping"); - if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_TRACE)) { - auto doc = App::GetApplication().getActiveDocument(); - if (doc) { - auto obj = doc->getObjectByID(this->Tag); - if (obj) - FC_LOG("\t" << obj->getFullName() << obj->getFullName() << "." << getIndexedName(name)); - } - for (auto &name : names) - FC_ERR("\t" << name); - } - } - break; - } - - if(cb(tmp, len, encodedTag, tag) || pos < 0) - return; - tag = encodedTag; - } -} - void ComplexGeoData::setMappedChildElements(const std::vector & children) { // DO NOT reset element map if there is one. Because we allow mixing child diff --git a/src/App/ComplexGeoData.h b/src/App/ComplexGeoData.h index 7d9699178b..ab91d9acac 100644 --- a/src/App/ComplexGeoData.h +++ b/src/App/ComplexGeoData.h @@ -298,23 +298,10 @@ public: virtual bool checkElementMapVersion(const char * ver) const; /// Check if the given sub-name only contains an element name - static bool isElementName(const char *subName) { - return (subName != nullptr) && (*subName != 0) && findElementName(subName)==subName; + static bool isElementName(const char* subName) + { + return (subName != nullptr) && (*subName != 0) && findElementName(subName) == subName; } - /** Element trace callback - * - * The callback has the following call signature - * (const std::string &name, size_t offset, long encodedTag, long tag) -> bool - * - * @param name: the current element name. - * @param offset: the offset skipping the encoded element name for the next iteration. - * @param encodedTag: the tag encoded inside the current element, which is usually the tag - * of the previous step in the shape history. - * @param tag: the tag of the current shape element. - * - * @sa traceElement() - */ - typedef std::function TraceCallback; /** Iterate through the history of the give element name with a given callback * @@ -322,7 +309,10 @@ public: * @param cb: trace callback with call signature. * @sa TraceCallback */ - void traceElement(const MappedName& name, TraceCallback cb) const; + void traceElement(const MappedName& name, TraceCallback cb) const + { + _elementMap->traceElement(name, Tag, cb); + } /** Flush an internal buffering for element mapping */ virtual void flushElementMap() const; diff --git a/src/App/ElementMap.cpp b/src/App/ElementMap.cpp index 11ee75439c..d149b5fb4f 100644 --- a/src/App/ElementMap.cpp +++ b/src/App/ElementMap.cpp @@ -11,6 +11,8 @@ #include "App/Application.h" #include "Base/Console.h" +#include "Document.h" +#include "DocumentObject.h" #include #include @@ -1348,5 +1350,94 @@ long ElementMap::getElementHistory(const MappedName& name, long masterTag, Mappe } } +void ElementMap::traceElement(const MappedName& name, long masterTag, TraceCallback cb) const +{ + long encodedTag = 0; + int len = 0; + + auto pos = name.findTagInElementName(&encodedTag, &len, nullptr, nullptr, true); + if (cb(name, len, encodedTag, masterTag) || pos < 0) { + return; + } + + if (name.startsWith(POSTFIX_EXTERNAL_TAG, len)) { + return; + } + + std::set tagSet; + + std::vector names; + if (masterTag) { + tagSet.insert(std::abs(masterTag)); + } + if (encodedTag) { + tagSet.insert(std::abs(encodedTag)); + } + names.push_back(name); + + masterTag = encodedTag; + MappedName tmp; + bool first = true; + + // TODO: element tracing without object is inheriently unsafe, because of + // possible external linking object which means the element may be encoded + // using external string table. Looking up the wrong table may accidentally + // cause circular mapping, and is actually quite easy to reproduce. See + // + // https://github.com/realthunder/FreeCAD_assembly3/issues/968 + // + // A random depth limit is set here to not waste time. 'tagSet' above is + // also used for early detection of 'recursive' mapping. + + for (int index = 0; index < 50; ++index) { + if (!len || len > pos) { + return; + } + if (first) { + first = false; + size_t offset = 0; + if (name.startsWith(ELEMENT_MAP_PREFIX)) { + offset = ELEMENT_MAP_PREFIX_SIZE; + } + tmp = MappedName(name, offset, len); + } + else { + tmp = MappedName(tmp, 0, len); + } + tmp = dehashElementName(tmp); + names.push_back(tmp); + encodedTag = 0; + pos = tmp.findTagInElementName(&encodedTag, &len, nullptr, nullptr, true); + if (pos >= 0 && tmp.startsWith(POSTFIX_EXTERNAL_TAG, len)) { + break; + } + + if (encodedTag && masterTag != std::abs(encodedTag) + && !tagSet.insert(std::abs(encodedTag)).second) { + if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) { + FC_WARN("circular element mapping"); + if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_TRACE)) { + auto doc = App::GetApplication().getActiveDocument(); + if (doc) { + auto obj = doc->getObjectByID(masterTag); + if (obj) { + FC_LOG("\t" << obj->getFullName() << obj->getFullName() << "." << name); + } + } + for (auto& errname : names) { + FC_ERR("\t" << errname); + } + } + } + break; + } + + if (cb(tmp, len, encodedTag, masterTag) || pos < 0) { + return; + } + masterTag = encodedTag; + } +} + }// Namespace Data diff --git a/src/App/ElementMap.h b/src/App/ElementMap.h index 195b6020dd..1902210ef9 100644 --- a/src/App/ElementMap.h +++ b/src/App/ElementMap.h @@ -45,6 +45,21 @@ namespace Data class ElementMap; using ElementMapPtr = std::shared_ptr; +/** Element trace callback + * + * The callback has the following call signature + * (const std::string &name, size_t offset, long encodedTag, long tag) -> bool + * + * @param name: the current element name. + * @param offset: the offset skipping the encoded element name for the next iteration. + * @param encodedTag: the tag encoded inside the current element, which is usually the tag + * of the previous step in the shape history. + * @param tag: the tag of the current shape element. + * + * @sa traceElement() + */ +typedef std::function TraceCallback; + /* This class provides for ComplexGeoData's ability to provide proper naming. * Specifically, ComplexGeoData uses this class for it's `_id` property. * Most of the operations work with the `indexedNames` and `mappedNames` maps. @@ -185,6 +200,15 @@ public: long masterTag, MappedName *original=nullptr, std::vector *history=nullptr) const; + /** Iterate through the history of the give element name with a given callback + * + * @param name: the input element name + * @param cb: trace callback with call signature. + * @sa TraceCallback + */ + void traceElement(const MappedName& name, long masterTag, TraceCallback cb) const; + + private: /** Serialize this map * @param stream: serialized stream diff --git a/src/Mod/Part/App/FeaturePartCommon.cpp b/src/Mod/Part/App/FeaturePartCommon.cpp index 3d7632ccb8..b2363050e5 100644 --- a/src/Mod/Part/App/FeaturePartCommon.cpp +++ b/src/Mod/Part/App/FeaturePartCommon.cpp @@ -211,7 +211,7 @@ App::DocumentObjectExecReturn *MultiCommon::execute() shapes.push_back(sh); } - TopoShape res {}; + TopoShape res {0}; res.makeElementBoolean(Part::OpCodes::Common, shapes); if (res.isNull()) { throw Base::RuntimeError("Resulting shape is null"); diff --git a/src/Mod/Part/App/TopoShape.h b/src/Mod/Part/App/TopoShape.h index 389284d3b0..a705b15742 100644 --- a/src/Mod/Part/App/TopoShape.h +++ b/src/Mod/Part/App/TopoShape.h @@ -1240,6 +1240,14 @@ public: void copyElementMap(const TopoShape & topoShape, const char *op=nullptr); bool canMapElement(const TopoShape &other) const; + void cacheRelatedElements(const Data::MappedName & name, + HistoryTraceType sameType, + const QVector & names) const; + + bool getRelatedElementsCached(const Data::MappedName & name, + HistoryTraceType sameType, + QVector &names) const; + void mapSubElement(const TopoShape &other,const char *op=nullptr, bool forceHasher=false); void mapSubElement(const std::vector &shapes, const char *op=nullptr); void mapSubElementsTo(std::vector& shapes, const char* op = nullptr) const; diff --git a/src/Mod/Part/App/TopoShapeExpansion.cpp b/src/Mod/Part/App/TopoShapeExpansion.cpp index ff46e6596b..90a1f7e030 100644 --- a/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -5002,23 +5002,22 @@ bool TopoShape::isSame(const Data::ComplexGeoData &_other) const && Hasher == other.Hasher && _Shape.IsEqual(other._Shape); } - void TopoShape::cacheRelatedElements(const Data::MappedName &name, - bool sameType, + HistoryTraceType sameType, const QVector & names) const { - INIT_SHAPE_CACHE(); - _Cache->insertRelation(ShapeRelationKey(name,sameType), names); + initCache(); + _cache->insertRelation(ShapeRelationKey(name,sameType), names); } bool TopoShape::getRelatedElementsCached(const Data::MappedName &name, - bool sameType, + HistoryTraceType sameType, QVector &names) const { - if(!_Cache) + if(!_cache) return false; - auto it = _Cache->relations.find(ShapeRelationKey(name,sameType)); - if(it == _Cache->relations.end()) + auto it = _cache->relations.find(ShapeRelationKey(name,sameType)); + if(it == _cache->relations.end()) return false; names = it->second; return true; diff --git a/tests/src/App/ComplexGeoData.cpp b/tests/src/App/ComplexGeoData.cpp index fecf3e318b..fe007957c7 100644 --- a/tests/src/App/ComplexGeoData.cpp +++ b/tests/src/App/ComplexGeoData.cpp @@ -469,4 +469,22 @@ TEST_F(ComplexGeoDataTest, saveDocFileWithElementMap) TEST_F(ComplexGeoDataTest, restoreStream) {} +TEST_F(ComplexGeoDataTest, traceElement) +{ // This test barely scratches the surface; see the ToposhapeExtension traceElement test for more + Data::MappedName mappedName; + Data::IndexedName indexedName; + std::string name(Data::ELEMENT_MAP_PREFIX); + name.append("TestMappedElement:;"); + std::tie(indexedName, mappedName) = createMappedName(name); + + // Arrange + Data::TraceCallback cb = + [name](const Data::MappedName& elementname, int offset, long encodedTag, long tag) { + EXPECT_STREQ(elementname.toString().c_str(), name.substr(1).c_str()); + return false; + }; + // Act + cgd().traceElement(mappedName, cb); +} + // NOLINTEND(readability-magic-numbers) diff --git a/tests/src/App/ElementMap.cpp b/tests/src/App/ElementMap.cpp index 3e63bb1a7e..dd425ed099 100644 --- a/tests/src/App/ElementMap.cpp +++ b/tests/src/App/ElementMap.cpp @@ -553,5 +553,4 @@ TEST_F(ElementMapTest, addAndGetChildElementsTest) return e.indexedName.toString() == "Pong2"; })); } - // NOLINTEND(readability-magic-numbers) diff --git a/tests/src/Mod/Part/App/FeaturePartCommon.cpp b/tests/src/Mod/Part/App/FeaturePartCommon.cpp index 8f7b6bc4ec..8a9f5e02aa 100644 --- a/tests/src/Mod/Part/App/FeaturePartCommon.cpp +++ b/tests/src/Mod/Part/App/FeaturePartCommon.cpp @@ -218,6 +218,6 @@ TEST_F(FeaturePartCommonTest, testMapping) #ifndef FC_USE_TNP_FIX EXPECT_EQ(ts1.getElementMap().size(), 0); #else - EXPECT_EQ(ts1.getElementMap().size(), 26); // Value and code TBD + EXPECT_EQ(ts1.getElementMap().size(), 26); #endif } diff --git a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp index 01a0392f34..cd0f570643 100644 --- a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -508,6 +508,40 @@ TEST_F(TopoShapeExpansionTest, flushElementMapTest) EXPECT_NE(childshapeWithMapFlushed.getElementMap()[0].name.find("Vertex1"), -1); } +TEST_F(TopoShapeExpansionTest, cacheRelatedElements) +{ + // Arrange + TopoShape topoShape {3L}; + QVector names { + {MappedName {"Test1"}, IndexedName {"Test", 1}}, + {MappedName {"Test2"}, IndexedName {"Test", 2}}, + {MappedName {"OtherTest1"}, IndexedName {"OtherTest", 1}}, + }; + QVector names2 { + {MappedName {"Test3"}, IndexedName {"Test", 3}}, + }; + HistoryTraceType traceType = HistoryTraceType::followTypeChange; + MappedName keyName {"Key1"}; + MappedName keyName2 {"Key2"}; + QVector returnedNames; + QVector returnedNames2; + QVector returnedNames3; + // Act + topoShape.cacheRelatedElements(keyName, traceType, names); + topoShape.cacheRelatedElements(keyName2, HistoryTraceType::stopOnTypeChange, names2); + topoShape.getRelatedElementsCached(keyName, traceType, returnedNames); + topoShape.getRelatedElementsCached(keyName, HistoryTraceType::stopOnTypeChange, returnedNames3); + topoShape.getRelatedElementsCached(keyName2, + HistoryTraceType::stopOnTypeChange, + returnedNames2); + // Assert + EXPECT_EQ(returnedNames.size(), 3); + EXPECT_STREQ(returnedNames[0].name.toString().c_str(), "Test1"); + EXPECT_EQ(returnedNames2.size(), 1); + EXPECT_STREQ(returnedNames2[0].name.toString().c_str(), "Test3"); + EXPECT_EQ(returnedNames3.size(), 0); // No entries of this type. +} + TEST_F(TopoShapeExpansionTest, makeElementWiresCombinesAdjacent) { // Arrange @@ -2517,4 +2551,86 @@ TEST_F(TopoShapeExpansionTest, makeElementEvolve) EXPECT_EQ(spine.getElementMap().size(), 0); } +TEST_F(TopoShapeExpansionTest, traceElement) +{ + // Arrange + auto [cube1, cube2] = CreateTwoCubes(); + auto tr {gp_Trsf()}; + tr.SetTranslation(gp_Vec(gp_XYZ(-0.5, -0.5, 0))); + cube2.Move(TopLoc_Location(tr)); + TopoShape topoShape1 {cube1, 1L}; + TopoShape topoShape2 {cube2, 2L}; + // Act + TopoShape& result = topoShape1.makeElementCut( + {topoShape1, topoShape2}); //, const char* op = nullptr, double tol = 0); + std::string name {"Face2;:M;CUT;:H1:7,F"}; + // auto faces = result.getSubTopoShapes(TopAbs_FACE); + Data::MappedName mappedName(name); + // Arrange + Data::TraceCallback cb = + [name](const Data::MappedName& elementname, int offset, long encodedTag, long tag) { + // TODO: This is likely a flawed way to address testing a callback. + // Also, it isn't clear exactly what the correct results are, although as soon as + // we start addressing History, we will quickly discover that, and likely the right + // things to test. + if (encodedTag == 1) { + EXPECT_STREQ(elementname.toString().c_str(), name.c_str()); + } + else { + EXPECT_STREQ(elementname.toString().c_str(), name.substr(0, 5).c_str()); + } + return false; + }; + // Act + result.traceElement(mappedName, cb); + // Assert we have the element map we think we do. + EXPECT_TRUE(allElementsMatch( + result, + { + "Edge10;:G(Edge2;K-1;:H2:4,E);CUT;:H1:1a,V", + "Edge10;:M;CUT;:H1:7,E", + "Edge10;:M;CUT;:H1:7,E;:U;CUT;:H1:7,V", + "Edge11;:M;CUT;:H2:7,E", + "Edge12;:M;CUT;:H2:7,E", + "Edge2;:M;CUT;:H2:7,E", + "Edge2;:M;CUT;:H2:7,E;:U;CUT;:H2:7,V", + "Edge4;:M;CUT;:H2:7,E", + "Edge4;:M;CUT;:H2:7,E;:U;CUT;:H2:7,V", + "Edge6;:G(Edge12;K-1;:H2:4,E);CUT;:H1:1b,V", + "Edge6;:M;CUT;:H1:7,E", + "Edge6;:M;CUT;:H1:7,E;:U;CUT;:H1:7,V", + "Edge8;:G(Edge11;K-1;:H2:4,E);CUT;:H1:1b,V", + "Edge8;:M;CUT;:H1:7,E", + "Edge8;:M;CUT;:H1:7,E;:U;CUT;:H1:7,V", + "Edge9;:G(Edge4;K-1;:H2:4,E);CUT;:H1:1a,V", + "Edge9;:M;CUT;:H1:7,E", + "Edge9;:M;CUT;:H1:7,E;:U;CUT;:H1:7,V", + "Face1;:M;CUT;:H2:7,F", + "Face1;:M;CUT;:H2:7,F;:U;CUT;:H2:7,E", + "Face2;:G(Face4;K-1;:H2:4,F);CUT;:H1:1a,E", + "Face2;:M;CUT;:H1:7,F", + "Face2;:M;CUT;:H1:7,F;:U;CUT;:H1:7,E", + "Face2;:M;CUT;:H1:7,F;:U;CUT;:H1:7,E;:L(Face5;:M;CUT;:H1:7,F;:U2;CUT;:H1:8,E;:U;CUT;:" + "H1:7,V;:L(Face6;:M;CUT;:H1:7,F;:U2;CUT;:H1:8,E;:U;CUT;:H1:7,V);CUT;:H1:3c,E|Face5;:M;" + "CUT;:H1:7,F;:U;CUT;:H1:7,E|Face6;:M;CUT;:H1:7,F;:U;CUT;:H1:7,E);CUT;:H1:c9,F", + "Face3;:G(Face1;K-1;:H2:4,F);CUT;:H1:1a,E", + "Face3;:M;CUT;:H1:7,F", + "Face3;:M;CUT;:H1:7,F;:U;CUT;:H1:7,E", + "Face3;:M;CUT;:H1:7,F;:U;CUT;:H1:7,E;:L(Face5;:M;CUT;:H1:7,F;:U2;CUT;:H1:8,E|Face5;:M;" + "CUT;:H1:7,F;:U2;CUT;:H1:8,E;:U;CUT;:H1:7,V;:L(Face6;:M;CUT;:H1:7,F;:U2;CUT;:H1:8,E;:U;" + "CUT;:H1:7,V);CUT;:H1:3c,E|Face6;:M;CUT;:H1:7,F;:U2;CUT;:H1:8,E);CUT;:H1:cb,F", + "Face4;:M;CUT;:H2:7,F", + "Face5;:M;CUT;:H1:7,F", + "Face5;:M;CUT;:H1:7,F;:U2;CUT;:H1:8,E", + "Face5;:M;CUT;:H1:7,F;:U2;CUT;:H1:8,E;:U;CUT;:H1:7,V", + "Face5;:M;CUT;:H1:7,F;:U2;CUT;:H1:8,E;:U;CUT;:H1:7,V;:L(Face6;:M;CUT;:H1:7,F;:U2;CUT;:" + "H1:8,E;:U;CUT;:H1:7,V);CUT;:H1:3c,E", + "Face5;:M;CUT;:H1:7,F;:U;CUT;:H1:7,E", + "Face6;:M;CUT;:H1:7,F", + "Face6;:M;CUT;:H1:7,F;:U2;CUT;:H1:8,E", + "Face6;:M;CUT;:H1:7,F;:U2;CUT;:H1:8,E;:U;CUT;:H1:7,V", + "Face6;:M;CUT;:H1:7,F;:U;CUT;:H1:7,E", + })); +} + // NOLINTEND(readability-magic-numbers,cppcoreguidelines-avoid-magic-numbers)