From f6494bdf0596750ea57d313b0447c0811ee58138 Mon Sep 17 00:00:00 2001 From: "Zheng, Lei" Date: Thu, 18 Jul 2024 17:09:48 -0400 Subject: [PATCH 1/2] Toponaming: Bring in composite shapes for findSubshapesWithSharedVertex ( searchSubShape ) --- src/App/ComplexGeoData.h | 5 +- src/App/GeoFeature.h | 2 +- src/Base/Bitmask.h | 44 ++++++- src/Mod/Part/App/PartFeature.cpp | 2 +- src/Mod/Part/App/PartFeature.h | 2 +- src/Mod/Part/App/TopoShape.h | 2 +- src/Mod/Part/App/TopoShapeExpansion.cpp | 156 ++++++++++++++++++------ src/Mod/Part/App/TopoShapePyImp.cpp | 17 ++- 8 files changed, 181 insertions(+), 49 deletions(-) diff --git a/src/App/ComplexGeoData.h b/src/App/ComplexGeoData.h index 0eeb086f77..3f3c846ccf 100644 --- a/src/App/ComplexGeoData.h +++ b/src/App/ComplexGeoData.h @@ -54,11 +54,12 @@ namespace Data //struct MappedChildElements; /// Option for App::GeoFeature::searchElementCache() -enum class SearchOptions { +enum class SearchOption { /// Whether to compare shape geometry CheckGeometry = 1, SingleResult = 2, }; +typedef Base::Flags SearchOptions; /** Segments * Sub-element type of the ComplexGeoData type @@ -483,5 +484,5 @@ protected: } //namespace App - +ENABLE_BITMASK_OPERATORS(Data::SearchOption) #endif diff --git a/src/App/GeoFeature.h b/src/App/GeoFeature.h index 6e75cef0e8..3e0107fc0d 100644 --- a/src/App/GeoFeature.h +++ b/src/App/GeoFeature.h @@ -165,7 +165,7 @@ public: * reference to the same geometry of the old element. */ virtual const std::vector& searchElementCache(const std::string &element, - Data::SearchOptions options = Data::SearchOptions::CheckGeometry, + Data::SearchOptions options = Data::SearchOption::CheckGeometry, double tol = 1e-7, double atol = 1e-10) const; diff --git a/src/Base/Bitmask.h b/src/Base/Bitmask.h index cae10e6bee..133c470e2c 100644 --- a/src/Base/Bitmask.h +++ b/src/Base/Bitmask.h @@ -113,7 +113,7 @@ class Flags { Enum i; public: - constexpr inline Flags(Enum f) : i(f) {} + constexpr inline Flags(Enum f = Enum()) : i(f) {} constexpr bool testFlag(Enum f) const { using u = typename std::underlying_type::type; return (i & f) == f && (static_cast(f) != 0 || i == f); @@ -125,6 +125,48 @@ public: using u = typename std::underlying_type::type; return static_cast(i) == static_cast(f.i); } + constexpr Enum getFlags() const { + return i; + } + constexpr Flags &operator|=(const Flags &other) { + i |= other.i; + return *this; + } + constexpr Flags &operator|=(const Enum &f) { + i |= f; + return *this; + } + constexpr Flags operator|(const Flags &other) const { + return i | other.i; + } + constexpr Flags operator|(const Enum &f) const { + return i | f; + } + constexpr Flags &operator&=(const Flags &other) { + i &= other.i; + return *this; + } + constexpr Flags &operator&=(const Enum &f) { + i &= f; + return *this; + } + constexpr Flags operator&(const Flags &other) const { + return i & other.i; + } + constexpr Flags operator&(const Enum &f) const { + return i & f; + } + constexpr Flags operator~() const { + return ~i; + } + + constexpr bool operator!() const { + return !i; + } + + explicit operator bool() const { + return toUnderlyingType() != 0; + } typename std::underlying_type::type toUnderlyingType() const { return static_cast::type>(i); } diff --git a/src/Mod/Part/App/PartFeature.cpp b/src/Mod/Part/App/PartFeature.cpp index 9be1923ce2..5d1e7ec10f 100644 --- a/src/Mod/Part/App/PartFeature.cpp +++ b/src/Mod/Part/App/PartFeature.cpp @@ -1499,7 +1499,7 @@ const std::vector& Feature::searchElementCache(const std::string& e it->second.searched = true; propShape->getShape().findSubShapesWithSharedVertex(it->second.shape, &it->second.names, - static_cast(options), + options, tol, atol); if (prefix) { diff --git a/src/Mod/Part/App/PartFeature.h b/src/Mod/Part/App/PartFeature.h index a468eb6ef1..5c53b7f646 100644 --- a/src/Mod/Part/App/PartFeature.h +++ b/src/Mod/Part/App/PartFeature.h @@ -157,7 +157,7 @@ public: #ifdef FC_USE_TNP_FIX const std::vector& searchElementCache(const std::string &element, - Data::SearchOptions options = Data::SearchOptions::CheckGeometry, + Data::SearchOptions options = Data::SearchOption::CheckGeometry, double tol = 1e-7, double atol = 1e-10) const override; #endif diff --git a/src/Mod/Part/App/TopoShape.h b/src/Mod/Part/App/TopoShape.h index f67ce107e4..ffd4876cc2 100644 --- a/src/Mod/Part/App/TopoShape.h +++ b/src/Mod/Part/App/TopoShape.h @@ -1472,7 +1472,7 @@ public: */ std::vector findSubShapesWithSharedVertex(const TopoShape &subshape, std::vector *names=nullptr, - CheckGeometry checkGeometry=CheckGeometry::checkGeometry, + Data::SearchOptions = Data::SearchOption::CheckGeometry, double tol=1e-7, double atol=1e-12) const; //@} diff --git a/src/Mod/Part/App/TopoShapeExpansion.cpp b/src/Mod/Part/App/TopoShapeExpansion.cpp index f8e3c6b4c0..07ac4d7dd9 100644 --- a/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -319,12 +319,14 @@ TopoDS_Shape TopoShape::findShape(TopAbs_ShapeEnum type, int idx) const return _cache->findShape(_Shape, type, idx); } -std::vector TopoShape::findSubShapesWithSharedVertex(const TopoShape& subshape, - std::vector* names, - CheckGeometry checkGeometry, - double tol, - double atol) const -{ +std::vector +TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(misc-no-recursion) + std::vector *names, + Data::SearchOptions options, + double tol, + double atol) const { + bool checkGeometry = options.testFlag(Data::SearchOption::CheckGeometry); + bool singleSearch = options.testFlag(Data::SearchOption::SingleResult); std::vector res; if (subshape.isNull() || this->isNull()) { return res; @@ -332,19 +334,104 @@ std::vector TopoShape::findSubShapesWithSharedVertex(const TopoShape& double tol2 = tol * tol; int index = 0; TopAbs_ShapeEnum shapeType = subshape.shapeType(); + + // This is an intentionally recursive method, which will exit after looking through all ancestors. + auto searchCompositeShape = [&](TopAbs_ShapeEnum childType) { // NOLINT(misc-no-recursion) + unsigned long count = subshape.countSubShapes(childType); + if (!count) + return; + auto first = subshape.getSubTopoShape(childType, 1); + for (const auto &child: findSubShapesWithSharedVertex(first, nullptr, options, tol, atol)) { + for (int idx: findAncestors(child.getShape(), shapeType)) { + auto shape = getSubTopoShape(shapeType, idx); + if (shape.countSubShapes(childType) != count) + continue; + bool found = true; + for (unsigned long i = 2; i < count; ++i) { + if (shape.findSubShapesWithSharedVertex(subshape.getSubTopoShape(childType, i), nullptr, + options, tol, atol).empty()) { + found = false; + break; + } + } + if (found) { + res.push_back(shape); + if (names) + names->push_back(shapeName(shapeType) + std::to_string(idx)); + if (singleSearch) + return; + } + } + } + }; + switch (shapeType) { + case TopAbs_WIRE: + searchCompositeShape(TopAbs_EDGE); + break; + case TopAbs_SHELL: + searchCompositeShape(TopAbs_FACE); + break; + case TopAbs_SOLID: + searchCompositeShape(TopAbs_SHELL); + break; + case TopAbs_COMPSOLID: + searchCompositeShape(TopAbs_SOLID); + break; + case TopAbs_COMPOUND: + // special treatment of single sub-shape compound, that is, search + // its extracting the compound + if (countSubShapes(TopAbs_SHAPE) == 1) { + return findSubShapesWithSharedVertex(subshape.getSubTopoShape(TopAbs_SHAPE, 1), names, options, tol, + atol); + } else if (unsigned long count = countSubShapes(TopAbs_SHAPE)) { + // For multi-sub-shape compound, only search for compound with the same + // structure + int idx = 0; + for (const auto &compound: getSubTopoShapes(shapeType)) { + ++idx; + if (compound.countSubShapes(TopAbs_SHAPE) != count) + continue; + int i = 0; + bool found = true; + for (const auto &s: compound.getSubTopoShapes(TopAbs_SHAPE)) { + ++i; + auto ss = subshape.getSubTopoShape(TopAbs_SHAPE, i); + if (ss.isNull() && s.isNull()) + continue; + auto options2 = options; + options2.setFlag(Data::SearchOption::SingleResult); + if (ss.isNull() || s.isNull() + || ss.shapeType() != s.shapeType() + || ss.findSubShapesWithSharedVertex(s, nullptr, options2, tol, atol).empty()) { + found = false; + break; + } + } + if (found) { + if (names) + names->push_back(shapeName(shapeType) + std::to_string(idx)); + res.push_back(compound); + if (singleSearch) + return res; + } + } + } + break; case TopAbs_VERTEX: // Vertex search will do comparison with tolerance to account for // rounding error inccured through transformation. - for (auto& shape : getSubTopoShapes(TopAbs_VERTEX)) { + for (auto &shape: getSubTopoShapes(TopAbs_VERTEX)) { ++index; if (BRep_Tool::Pnt(TopoDS::Vertex(shape.getShape())) - .SquareDistance(BRep_Tool::Pnt(TopoDS::Vertex(subshape.getShape()))) + .SquareDistance(BRep_Tool::Pnt(TopoDS::Vertex(subshape.getShape()))) <= tol2) { if (names) { names->push_back(std::string("Vertex") + std::to_string(index)); } res.push_back(shape); + if (singleSearch) + return res; } } break; @@ -359,12 +446,11 @@ std::vector TopoShape::findSubShapesWithSharedVertex(const TopoShape& if (shapeType == TopAbs_FACE) { wire = subshape.splitWires(); vertices = wire.getSubShapes(TopAbs_VERTEX); - } - else { + } else { vertices = subshape.getSubShapes(TopAbs_VERTEX); } - if (vertices.empty() || checkGeometry == CheckGeometry::checkGeometry) { + if (vertices.empty() || checkGeometry) { geom = Geometry::fromShape(subshape.getShape()); if (!geom) { return res; @@ -372,13 +458,12 @@ std::vector TopoShape::findSubShapesWithSharedVertex(const TopoShape& if (shapeType == TopAbs_EDGE) { isLine = (geom->isDerivedFrom(GeomLine::getClassTypeId()) || geom->isDerivedFrom(GeomLineSegment::getClassTypeId())); - } - else { + } else { isPlane = geom->isDerivedFrom(GeomPlane::getClassTypeId()); } } - auto compareGeometry = [&](const TopoShape& s, bool strict) { + auto compareGeometry = [&](const TopoShape &s, bool strict) { std::unique_ptr g2(Geometry::fromShape(s.getShape())); if (!g2) { return false; @@ -391,16 +476,14 @@ std::vector TopoShape::findSubShapesWithSharedVertex(const TopoShape& && !g2->isDerivedFrom(GeomLineSegment::getClassTypeId())) { return false; } - } - else if (isPlane && !strict) { + } else if (isPlane && !strict) { // For planes, don't compare geometry either, so that // we don't need to worry about orientation and so on. // Just check the edges. if (!g2->isDerivedFrom(GeomPlane::getClassTypeId())) { return false; } - } - else if (!g2 || !g2->isSame(*geom, tol, atol)) { + } else if (!g2 || !g2->isSame(*geom, tol, atol)) { return false; } return true; @@ -409,13 +492,15 @@ std::vector TopoShape::findSubShapesWithSharedVertex(const TopoShape& if (vertices.empty()) { // Probably an infinite shape, so we have to search by geometry int idx = 0; - for (auto& shape : getSubTopoShapes(shapeType)) { + for (auto &shape: getSubTopoShapes(shapeType)) { ++idx; if (!shape.countSubShapes(TopAbs_VERTEX) && compareGeometry(shape, true)) { if (names) { names->push_back(shapeName(shapeType) + std::to_string(idx)); } res.push_back(shape); + if (singleSearch) + return res; } } break; @@ -428,9 +513,9 @@ std::vector TopoShape::findSubShapesWithSharedVertex(const TopoShape& // * Perform geometry comparison of the ancestor and input shape. // * For face, perform addition geometry comparison of each edge. std::unordered_set shapeSet; - for (auto& vert : - findSubShapesWithSharedVertex(vertices[0], nullptr, checkGeometry, tol, atol)) { - for (auto idx : findAncestors(vert.getShape(), shapeType)) { + for (auto &vert: + findSubShapesWithSharedVertex(vertices[0], nullptr, options, tol, atol)) { + for (auto idx: findAncestors(vert.getShape(), shapeType)) { auto shape = getSubTopoShape(shapeType, idx); if (!shapeSet.insert(shape).second) { continue; @@ -444,28 +529,27 @@ std::vector TopoShape::findSubShapesWithSharedVertex(const TopoShape& continue; } otherVertices = otherWire.getSubShapes(TopAbs_VERTEX); - } - else { + } else { otherVertices = shape.getSubShapes(TopAbs_VERTEX); } if (otherVertices.size() != vertices.size()) { continue; } - if (checkGeometry == CheckGeometry::checkGeometry + if (checkGeometry && !compareGeometry(shape, false)) { continue; } unsigned ind = 0; bool matched = true; - for (auto& vertex : vertices) { + for (auto &vertex: vertices) { bool found = false; for (unsigned inner = 0; inner < otherVertices.size(); ++inner) { - auto& vertex1 = otherVertices[ind]; + auto &vertex1 = otherVertices[ind]; if (++ind == otherVertices.size()) { ind = 0; } if (BRep_Tool::Pnt(TopoDS::Vertex(vertex)) - .SquareDistance(BRep_Tool::Pnt(TopoDS::Vertex(vertex1))) + .SquareDistance(BRep_Tool::Pnt(TopoDS::Vertex(vertex1))) <= tol2) { found = true; break; @@ -480,7 +564,7 @@ std::vector TopoShape::findSubShapesWithSharedVertex(const TopoShape& continue; } - if (shapeType == TopAbs_FACE && checkGeometry == CheckGeometry::checkGeometry) { + if (shapeType == TopAbs_FACE && checkGeometry) { // Is it really necessary to check geometries of each edge of a face? // Right now we only do outer wire check auto otherEdges = otherWire.getSubShapes(TopAbs_EDGE); @@ -489,8 +573,8 @@ std::vector TopoShape::findSubShapesWithSharedVertex(const TopoShape& bool matched2 = true; unsigned i = 0; auto edges = wire.getSubShapes(TopAbs_EDGE); - for (auto& edge : edges) { - std::unique_ptr geom2(Geometry::fromShape(edge)); + for (auto &edge: edges) { + std::unique_ptr geom2(Geometry::fromShape(edge, true)); if (!geom2) { matched2 = false; break; @@ -506,13 +590,13 @@ std::vector TopoShape::findSubShapesWithSharedVertex(const TopoShape& // We will tolerate on edge reordering bool found = false; for (unsigned j = 0; j < otherEdges.size(); j++) { - auto& e1 = otherEdges[i]; - auto& g1 = geos[i]; + auto &e1 = otherEdges[i]; + auto &g1 = geos[i]; if (++i >= otherEdges.size()) { i = 0; } if (!g1) { - g1 = Geometry::fromShape(e1); + g1 = Geometry::fromShape(e1, true); if (!g1) { break; } @@ -521,9 +605,9 @@ std::vector TopoShape::findSubShapesWithSharedVertex(const TopoShape& if (g1->isDerivedFrom(GeomLine::getClassTypeId()) || g1->isDerivedFrom(GeomLineSegment::getClassTypeId())) { auto p1 = - BRep_Tool::Pnt(TopExp::FirstVertex(TopoDS::Edge(e1))); + BRep_Tool::Pnt(TopExp::FirstVertex(TopoDS::Edge(e1))); auto p2 = - BRep_Tool::Pnt(TopExp::LastVertex(TopoDS::Edge(e1))); + BRep_Tool::Pnt(TopExp::LastVertex(TopoDS::Edge(e1))); if ((p1.SquareDistance(pt1) <= tol2 && p2.SquareDistance(pt2) <= tol2) || (p1.SquareDistance(pt2) <= tol2 diff --git a/src/Mod/Part/App/TopoShapePyImp.cpp b/src/Mod/Part/App/TopoShapePyImp.cpp index a6678b9d6a..66b74bd90b 100644 --- a/src/Mod/Part/App/TopoShapePyImp.cpp +++ b/src/Mod/Part/App/TopoShapePyImp.cpp @@ -3112,10 +3112,11 @@ PyObject* TopoShapePy::findSubShape(PyObject* args) PyObject* TopoShapePy::findSubShapesWithSharedVertex(PyObject* args, PyObject* keywds) { - static const std::array kwlist {"shape", "needName", "checkGeometry", "tol", "atol", nullptr}; + static const std::array kwlist {"shape", "needName", "checkGeometry", "tol", "atol", "singleResult", nullptr}; PyObject* pyobj; PyObject* needName = Py_False; PyObject* checkGeometry = Py_True; + PyObject* singleResult = Py_False; double tol = 1e-7; double atol = 1e-12; if (!Base::Wrapped_ParseTupleAndKeywords(args, @@ -3127,7 +3128,8 @@ PyObject* TopoShapePy::findSubShapesWithSharedVertex(PyObject* args, PyObject* k &needName, &checkGeometry, &tol, - &atol)) { + &atol, + &singleResult)) { return nullptr; } @@ -3135,13 +3137,17 @@ PyObject* TopoShapePy::findSubShapesWithSharedVertex(PyObject* args, PyObject* k { Py::List res; const TopoShape& shape = *static_cast(pyobj)->getTopoShapePtr(); + Data::SearchOptions options; + if (PyObject_IsTrue(checkGeometry)) + options.setFlag(Data::SearchOption::CheckGeometry); + if (PyObject_IsTrue(singleResult)) + options.setFlag(Data::SearchOption::SingleResult); if (PyObject_IsTrue(needName)) { std::vector names; auto shapes = getTopoShapePtr()->findSubShapesWithSharedVertex( shape, &names, - PyObject_IsTrue(checkGeometry) ? CheckGeometry::checkGeometry - : CheckGeometry::ignoreGeometry, + options, tol, atol); for (std::size_t i = 0; i < shapes.size(); ++i) { @@ -3152,8 +3158,7 @@ PyObject* TopoShapePy::findSubShapesWithSharedVertex(PyObject* args, PyObject* k for (auto& s : getTopoShapePtr()->findSubShapesWithSharedVertex( shape, nullptr, - PyObject_IsTrue(checkGeometry) ? CheckGeometry::checkGeometry - : CheckGeometry::ignoreGeometry, + options, tol, atol)) { res.append(shape2pyshape(s)); From 0178e848e4fde74bd752da3fc2b5bc2908f52a2f Mon Sep 17 00:00:00 2001 From: bgbsww Date: Thu, 18 Jul 2024 17:10:24 -0400 Subject: [PATCH 2/2] Toponaming: Clean code, apply remark recommendations and Tweak tests --- src/App/PropertyExpressionEngine.cpp | 28 ++-- src/App/PropertyLinks.cpp | 45 +++--- src/App/PropertyLinks.h | 2 +- src/Base/Bitmask.h | 3 +- src/Mod/Part/App/TopoShapeExpansion.cpp | 133 ++++++++++-------- src/Mod/Spreadsheet/App/PropertySheet.cpp | 29 ++-- tests/src/Mod/Part/App/TopoShapeExpansion.cpp | 33 +++-- 7 files changed, 149 insertions(+), 124 deletions(-) diff --git a/src/App/PropertyExpressionEngine.cpp b/src/App/PropertyExpressionEngine.cpp index 02f21654ec..63fd2c40fb 100644 --- a/src/App/PropertyExpressionEngine.cpp +++ b/src/App/PropertyExpressionEngine.cpp @@ -1105,22 +1105,18 @@ void PropertyExpressionEngine::getLinksTo(std::vector& id identifiers.push_back(expressionId); break; } - bool found = false; - for (const auto& path : paths) { - if (path.getSubObjectName() == subname) { - identifiers.push_back(expressionId); - found = true; - break; - } - App::SubObjectT sobjT(obj, path.getSubObjectName().c_str()); - if (sobjT.getSubObject() == sobj && sobjT.getOldElementName() == subElement) { - identifiers.push_back(expressionId); - found = true; - break; - } - } - if (found) { - break; + if (std::any_of(paths.begin(), + paths.end(), + [subname, obj, sobj, &subElement](const auto& path) { + if (path.getSubObjectName() == subname) { + return true; + } + + App::SubObjectT sobjT(obj, path.getSubObjectName().c_str()); + return (sobjT.getSubObject() == sobj + && sobjT.getOldElementName() == subElement); + })) { + identifiers.push_back(expressionId); } } } diff --git a/src/App/PropertyLinks.cpp b/src/App/PropertyLinks.cpp index b7f81199cc..848704b9ed 100644 --- a/src/App/PropertyLinks.cpp +++ b/src/App/PropertyLinks.cpp @@ -73,7 +73,7 @@ void PropertyLinkBase::setAllowExternal(bool allow) { } void PropertyLinkBase::setSilentRestore(bool allow) { - setFlag(LinkSilentRestore,allow); + setFlag(LinkSilentRestore, allow); } void PropertyLinkBase::setReturnNewElement(bool enable) { @@ -800,13 +800,15 @@ void PropertyLink::getLinks(std::vector &objs, objs.push_back(_pcLink); } -void PropertyLink::getLinksTo(std::vector& identifiers, - App::DocumentObject* obj, - const char* subname, - bool all) const -{ - (void)subname; - if ((all || _pcScope != LinkScope::Hidden) && obj && _pcLink == obj) { +void PropertyLink::getLinksTo(std::vector &identifiers, + App::DocumentObject *obj, + const char *subname, + bool all) const { + (void) subname; + if (!all && _pcScope == LinkScope::Hidden) { + return; // Don't get hidden links unless all is specified. + } + if (obj && _pcLink == obj) { identifiers.emplace_back(*this); } } @@ -1165,9 +1167,9 @@ void PropertyLinkList::getLinksTo(std::vector& identifier return; } int i = -1; - for (auto o : _lValueList) { + for (auto docObj : _lValueList) { ++i; - if (o == obj) { + if (docObj == obj) { identifiers.emplace_back(*this, i); break; } @@ -1658,13 +1660,13 @@ void PropertyLinkBase::_getLinksTo(std::vector& identifie if (!subObject) { continue; } - // There is a subobject and the subname doesn't match our current entry + // After above, there is a subobject and the subname doesn't match our current entry App::SubObjectT sobjT(obj, sub.c_str()); if (sobjT.getSubObject() == subObject && sobjT.getOldElementName() == subElement) { identifiers.emplace_back(*this); return; } - // The oldElementName ( short, I.E. "Edge5" ) doesn't match. + // And the oldElementName ( short, I.E. "Edge5" ) doesn't match. if (i < (int)shadows.size()) { const auto& [shadowNewName, shadowOldName] = shadows[i]; if (shadowNewName == subname || shadowOldName == subname) { @@ -2909,15 +2911,18 @@ void PropertyLinkSubList::getLinksTo(std::vector& identif auto subElement = objT.getOldElementName(); int i = -1; - for (const auto& o : _lValueList) { + for (const auto& docObj : _lValueList) { ++i; - if (o != obj) { + if (docObj != obj) { continue; } + // If we don't specify a subname we looking for all; or if the subname is in our + // property, add this entry to our result if (!subname || (i < (int)_lSubList.size() && subname == _lSubList[i])) { identifiers.emplace_back(*this, i); continue; } + // If we couldn't find any subobjects or this object's index is in our list, ignore it if (!subObject || i < (int)_lSubList.size()) { continue; } @@ -4917,12 +4922,12 @@ void PropertyXLinkSubList::getLinksTo(std::vector& identi const char* subname, bool all) const { - if (all || _pcScope != LinkScope::Hidden) { - for (auto& l : _Links) { - // This is the same algorithm as _getLinksTo, but returns lists, not single entries - if (obj && obj == l._pcLink) { - _getLinksToList(identifiers, obj, subname, l._SubList, l._ShadowSubList); - } + if ( ! all && _pcScope != LinkScope::Hidden) { + return; + } + for (auto& l : _Links) { + if (obj && obj == l._pcLink) { + _getLinksToList(identifiers, obj, subname, l._SubList, l._ShadowSubList); } } } diff --git a/src/App/PropertyLinks.h b/src/App/PropertyLinks.h index dd17cc3448..63f8a71dc8 100644 --- a/src/App/PropertyLinks.h +++ b/src/App/PropertyLinks.h @@ -183,7 +183,7 @@ public: virtual void getLinks(std::vector &objs, bool all=false, std::vector *subs=nullptr, bool newStyle=true) const = 0; - /** Obtain identifiers from this link property that link to a give object + /** Obtain identifiers from this link property that link to a given object * @param identifiers: holds the returned identifier to reference the given object * @param obj: the referenced object * @param subname: optional subname reference diff --git a/src/Base/Bitmask.h b/src/Base/Bitmask.h index 133c470e2c..a232df6820 100644 --- a/src/Base/Bitmask.h +++ b/src/Base/Bitmask.h @@ -113,7 +113,8 @@ class Flags { Enum i; public: - constexpr inline Flags(Enum f = Enum()) : i(f) {} + // Linter seems wrong on next line, don't want explicit here forcing downstream changes + constexpr inline Flags(Enum f = Enum()) : i(f) {} // NOLINT (runtime/explicit) constexpr bool testFlag(Enum f) const { using u = typename std::underlying_type::type; return (i & f) == f && (static_cast(f) != 0 || i == f); diff --git a/src/Mod/Part/App/TopoShapeExpansion.cpp b/src/Mod/Part/App/TopoShapeExpansion.cpp index 07ac4d7dd9..54e0143eaf 100644 --- a/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -41,7 +41,6 @@ #include #include #include -#include #include #include #include @@ -55,9 +54,6 @@ #include #include #include -#include -#include -#include #include #include #include @@ -81,7 +77,6 @@ #include #include #include -#include #include #include @@ -106,7 +101,6 @@ #include #include #include -#include #include FC_LOG_LEVEL_INIT("TopoShape", true, true) // NOLINT @@ -319,12 +313,12 @@ TopoDS_Shape TopoShape::findShape(TopAbs_ShapeEnum type, int idx) const return _cache->findShape(_Shape, type, idx); } -std::vector -TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(misc-no-recursion) - std::vector *names, - Data::SearchOptions options, - double tol, - double atol) const { +std::vector TopoShape::findSubShapesWithSharedVertex(const TopoShape& subshape, + std::vector* names, + Data::SearchOptions options, + double tol, + double atol) const +{ bool checkGeometry = options.testFlag(Data::SearchOption::CheckGeometry); bool singleSearch = options.testFlag(Data::SearchOption::SingleResult); std::vector res; @@ -335,31 +329,42 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m int index = 0; TopAbs_ShapeEnum shapeType = subshape.shapeType(); - // This is an intentionally recursive method, which will exit after looking through all ancestors. - auto searchCompositeShape = [&](TopAbs_ShapeEnum childType) { // NOLINT(misc-no-recursion) + // This is an intentionally recursive method, which will exit after looking through all + // ancestors. + auto searchCompositeShape = [&](TopAbs_ShapeEnum childType) { // NOLINT (misc-no-recu2rsive) unsigned long count = subshape.countSubShapes(childType); - if (!count) + if (!count) { return; + } auto first = subshape.getSubTopoShape(childType, 1); - for (const auto &child: findSubShapesWithSharedVertex(first, nullptr, options, tol, atol)) { - for (int idx: findAncestors(child.getShape(), shapeType)) { + for (const auto& child : + findSubShapesWithSharedVertex(first, nullptr, options, tol, atol)) { + for (int idx : findAncestors(child.getShape(), shapeType)) { auto shape = getSubTopoShape(shapeType, idx); - if (shape.countSubShapes(childType) != count) + if (shape.countSubShapes(childType) != count) { continue; + } bool found = true; for (unsigned long i = 2; i < count; ++i) { - if (shape.findSubShapesWithSharedVertex(subshape.getSubTopoShape(childType, i), nullptr, - options, tol, atol).empty()) { + if (shape + .findSubShapesWithSharedVertex(subshape.getSubTopoShape(childType, i), + nullptr, + options, + tol, + atol) + .empty()) { found = false; break; } } if (found) { res.push_back(shape); - if (names) + if (names) { names->push_back(shapeName(shapeType) + std::to_string(idx)); - if (singleSearch) + } + if (singleSearch) { return; + } } } } @@ -382,38 +387,46 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m // special treatment of single sub-shape compound, that is, search // its extracting the compound if (countSubShapes(TopAbs_SHAPE) == 1) { - return findSubShapesWithSharedVertex(subshape.getSubTopoShape(TopAbs_SHAPE, 1), names, options, tol, + return findSubShapesWithSharedVertex(subshape.getSubTopoShape(TopAbs_SHAPE, 1), + names, + options, + tol, atol); - } else if (unsigned long count = countSubShapes(TopAbs_SHAPE)) { + } + else if (unsigned long count = countSubShapes(TopAbs_SHAPE)) { // For multi-sub-shape compound, only search for compound with the same // structure int idx = 0; - for (const auto &compound: getSubTopoShapes(shapeType)) { + for (const auto& compound : getSubTopoShapes(shapeType)) { ++idx; - if (compound.countSubShapes(TopAbs_SHAPE) != count) + if (compound.countSubShapes(TopAbs_SHAPE) != count) { continue; + } int i = 0; bool found = true; - for (const auto &s: compound.getSubTopoShapes(TopAbs_SHAPE)) { + for (const auto& s : compound.getSubTopoShapes(TopAbs_SHAPE)) { ++i; auto ss = subshape.getSubTopoShape(TopAbs_SHAPE, i); - if (ss.isNull() && s.isNull()) + if (ss.isNull() && s.isNull()) { continue; + } auto options2 = options; options2.setFlag(Data::SearchOption::SingleResult); - if (ss.isNull() || s.isNull() - || ss.shapeType() != s.shapeType() - || ss.findSubShapesWithSharedVertex(s, nullptr, options2, tol, atol).empty()) { + if (ss.isNull() || s.isNull() || ss.shapeType() != s.shapeType() + || ss.findSubShapesWithSharedVertex(s, nullptr, options2, tol, atol) + .empty()) { found = false; break; } } if (found) { - if (names) + if (names) { names->push_back(shapeName(shapeType) + std::to_string(idx)); + } res.push_back(compound); - if (singleSearch) + if (singleSearch) { return res; + } } } } @@ -421,17 +434,18 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m case TopAbs_VERTEX: // Vertex search will do comparison with tolerance to account for // rounding error inccured through transformation. - for (auto &shape: getSubTopoShapes(TopAbs_VERTEX)) { + for (auto& shape : getSubTopoShapes(TopAbs_VERTEX)) { ++index; if (BRep_Tool::Pnt(TopoDS::Vertex(shape.getShape())) - .SquareDistance(BRep_Tool::Pnt(TopoDS::Vertex(subshape.getShape()))) + .SquareDistance(BRep_Tool::Pnt(TopoDS::Vertex(subshape.getShape()))) <= tol2) { if (names) { names->push_back(std::string("Vertex") + std::to_string(index)); } res.push_back(shape); - if (singleSearch) + if (singleSearch) { return res; + } } } break; @@ -446,7 +460,8 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m if (shapeType == TopAbs_FACE) { wire = subshape.splitWires(); vertices = wire.getSubShapes(TopAbs_VERTEX); - } else { + } + else { vertices = subshape.getSubShapes(TopAbs_VERTEX); } @@ -458,12 +473,13 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m if (shapeType == TopAbs_EDGE) { isLine = (geom->isDerivedFrom(GeomLine::getClassTypeId()) || geom->isDerivedFrom(GeomLineSegment::getClassTypeId())); - } else { + } + else { isPlane = geom->isDerivedFrom(GeomPlane::getClassTypeId()); } } - auto compareGeometry = [&](const TopoShape &s, bool strict) { + auto compareGeometry = [&](const TopoShape& s, bool strict) { std::unique_ptr g2(Geometry::fromShape(s.getShape())); if (!g2) { return false; @@ -476,14 +492,16 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m && !g2->isDerivedFrom(GeomLineSegment::getClassTypeId())) { return false; } - } else if (isPlane && !strict) { + } + else if (isPlane && !strict) { // For planes, don't compare geometry either, so that // we don't need to worry about orientation and so on. // Just check the edges. if (!g2->isDerivedFrom(GeomPlane::getClassTypeId())) { return false; } - } else if (!g2 || !g2->isSame(*geom, tol, atol)) { + } + else if (!g2 || !g2->isSame(*geom, tol, atol)) { return false; } return true; @@ -492,15 +510,16 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m if (vertices.empty()) { // Probably an infinite shape, so we have to search by geometry int idx = 0; - for (auto &shape: getSubTopoShapes(shapeType)) { + for (auto& shape : getSubTopoShapes(shapeType)) { ++idx; if (!shape.countSubShapes(TopAbs_VERTEX) && compareGeometry(shape, true)) { if (names) { names->push_back(shapeName(shapeType) + std::to_string(idx)); } res.push_back(shape); - if (singleSearch) + if (singleSearch) { return res; + } } } break; @@ -513,9 +532,9 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m // * Perform geometry comparison of the ancestor and input shape. // * For face, perform addition geometry comparison of each edge. std::unordered_set shapeSet; - for (auto &vert: - findSubShapesWithSharedVertex(vertices[0], nullptr, options, tol, atol)) { - for (auto idx: findAncestors(vert.getShape(), shapeType)) { + for (auto& vert : + findSubShapesWithSharedVertex(vertices[0], nullptr, options, tol, atol)) { + for (auto idx : findAncestors(vert.getShape(), shapeType)) { auto shape = getSubTopoShape(shapeType, idx); if (!shapeSet.insert(shape).second) { continue; @@ -529,27 +548,27 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m continue; } otherVertices = otherWire.getSubShapes(TopAbs_VERTEX); - } else { + } + else { otherVertices = shape.getSubShapes(TopAbs_VERTEX); } if (otherVertices.size() != vertices.size()) { continue; } - if (checkGeometry - && !compareGeometry(shape, false)) { + if (checkGeometry && !compareGeometry(shape, false)) { continue; } unsigned ind = 0; bool matched = true; - for (auto &vertex: vertices) { + for (auto& vertex : vertices) { bool found = false; for (unsigned inner = 0; inner < otherVertices.size(); ++inner) { - auto &vertex1 = otherVertices[ind]; + auto& vertex1 = otherVertices[ind]; if (++ind == otherVertices.size()) { ind = 0; } if (BRep_Tool::Pnt(TopoDS::Vertex(vertex)) - .SquareDistance(BRep_Tool::Pnt(TopoDS::Vertex(vertex1))) + .SquareDistance(BRep_Tool::Pnt(TopoDS::Vertex(vertex1))) <= tol2) { found = true; break; @@ -573,7 +592,7 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m bool matched2 = true; unsigned i = 0; auto edges = wire.getSubShapes(TopAbs_EDGE); - for (auto &edge: edges) { + for (auto& edge : edges) { std::unique_ptr geom2(Geometry::fromShape(edge, true)); if (!geom2) { matched2 = false; @@ -590,8 +609,8 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m // We will tolerate on edge reordering bool found = false; for (unsigned j = 0; j < otherEdges.size(); j++) { - auto &e1 = otherEdges[i]; - auto &g1 = geos[i]; + auto& e1 = otherEdges[i]; + auto& g1 = geos[i]; if (++i >= otherEdges.size()) { i = 0; } @@ -605,9 +624,9 @@ TopoShape::findSubShapesWithSharedVertex(const TopoShape &subshape, // NOLINT(m if (g1->isDerivedFrom(GeomLine::getClassTypeId()) || g1->isDerivedFrom(GeomLineSegment::getClassTypeId())) { auto p1 = - BRep_Tool::Pnt(TopExp::FirstVertex(TopoDS::Edge(e1))); + BRep_Tool::Pnt(TopExp::FirstVertex(TopoDS::Edge(e1))); auto p2 = - BRep_Tool::Pnt(TopExp::LastVertex(TopoDS::Edge(e1))); + BRep_Tool::Pnt(TopExp::LastVertex(TopoDS::Edge(e1))); if ((p1.SquareDistance(pt1) <= tol2 && p2.SquareDistance(pt2) <= tol2) || (p1.SquareDistance(pt2) <= tol2 diff --git a/src/Mod/Spreadsheet/App/PropertySheet.cpp b/src/Mod/Spreadsheet/App/PropertySheet.cpp index 4473de6e00..9c6bc8b045 100644 --- a/src/Mod/Spreadsheet/App/PropertySheet.cpp +++ b/src/Mod/Spreadsheet/App/PropertySheet.cpp @@ -2295,23 +2295,18 @@ void PropertySheet::getLinksTo(std::vector& identifiers, identifiers.emplace_back(owner, cellName.toString().c_str()); break; } - bool found = false; - for (const auto& path : paths) { - if (path.getSubObjectName() == subname) { - identifiers.emplace_back(owner, cellName.toString().c_str()); - found = true; - break; - } - App::SubObjectT sobjT(obj, path.getSubObjectName().c_str()); - if (sobjT.getSubObject() == subObject - && sobjT.getOldElementName() == subElement) { - identifiers.emplace_back(owner, cellName.toString().c_str()); - found = true; - break; - } - } - if (found) { - break; + if (std::any_of(paths.begin(), + paths.end(), + [subname, obj, subObject, &subElement](const auto& path) { + if (path.getSubObjectName() == subname) { + return true; + } + + App::SubObjectT sobjT(obj, path.getSubObjectName().c_str()); + return (sobjT.getSubObject() == subObject + && sobjT.getOldElementName() == subElement); + })) { + identifiers.emplace_back(owner, cellName.toString().c_str()); } } } diff --git a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp index 85855e0f22..55f0ac492c 100644 --- a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -1140,16 +1140,19 @@ TEST_F(TopoShapeExpansionTest, findSubShapesWithSharedVertexEverything) exp.Init(box1, TopAbs_VERTEX); auto vertex = exp.Current(); // Act - auto shapes = - box1TS.findSubShapesWithSharedVertex(face, &names, CheckGeometry::checkGeometry, tol, atol); + auto shapes = box1TS.findSubShapesWithSharedVertex(face, + &names, + Data::SearchOption::CheckGeometry, + tol, + atol); auto shapes1 = box1TS.findSubShapesWithSharedVertex(edge, &names1, - CheckGeometry::checkGeometry, + Data::SearchOption::CheckGeometry, tol, atol); auto shapes2 = box1TS.findSubShapesWithSharedVertex(vertex, &names2, - CheckGeometry::checkGeometry, + Data::SearchOption::CheckGeometry, tol, atol); // Assert @@ -1185,16 +1188,19 @@ TEST_F(TopoShapeExpansionTest, findSubShapesWithSharedVertexMid) exp.Init(box1, TopAbs_VERTEX); auto vertex = exp.Current(); // Act - auto shapes = - box1TS.findSubShapesWithSharedVertex(face, &names, CheckGeometry::checkGeometry, tol, atol); + auto shapes = box1TS.findSubShapesWithSharedVertex(face, + &names, + Data::SearchOption::CheckGeometry, + tol, + atol); auto shapes1 = box1TS.findSubShapesWithSharedVertex(edge, &names1, - CheckGeometry::checkGeometry, + Data::SearchOption::CheckGeometry, tol, atol); auto shapes2 = box1TS.findSubShapesWithSharedVertex(vertex, &names2, - CheckGeometry::checkGeometry, + Data::SearchOption::CheckGeometry, tol, atol); // Assert @@ -1224,16 +1230,19 @@ TEST_F(TopoShapeExpansionTest, findSubShapesWithSharedVertexClose) exp.Init(box1, TopAbs_VERTEX); auto vertex = exp.Current(); // Act - auto shapes = - box1TS.findSubShapesWithSharedVertex(face, &names, CheckGeometry::checkGeometry, tol, atol); + auto shapes = box1TS.findSubShapesWithSharedVertex(face, + &names, + Data::SearchOption::CheckGeometry, + tol, + atol); auto shapes1 = box1TS.findSubShapesWithSharedVertex(edge, &names1, - CheckGeometry::checkGeometry, + Data::SearchOption::CheckGeometry, tol, atol); auto shapes2 = box1TS.findSubShapesWithSharedVertex(vertex, &names2, - CheckGeometry::checkGeometry, + Data::SearchOption::CheckGeometry, tol, atol); // Assert