From b186d16ca7b4e1a94b790e334ab3600a86252443 Mon Sep 17 00:00:00 2001 From: CalligaroV Date: Thu, 8 Feb 2024 13:56:30 +0100 Subject: [PATCH] Part/Toponaming: makeElementWires * Renamed enum classes members to lowercaseCapword * Moved struct ShapeHasher back to TopoShapeMapper.h * Added test for MapperMaker::generated * Modifications for clang-tidy warnings * Formatting Signed-off-by: CalligaroV --- src/Mod/Part/App/TopoShape.cpp | 13 +- src/Mod/Part/App/TopoShape.h | 143 +++++------------- src/Mod/Part/App/TopoShapeExpansion.cpp | 85 ++++++----- src/Mod/Part/App/TopoShapeMapper.h | 71 ++++++++- tests/src/Mod/Part/App/TopoShapeExpansion.cpp | 4 +- 5 files changed, 168 insertions(+), 148 deletions(-) diff --git a/src/Mod/Part/App/TopoShape.cpp b/src/Mod/Part/App/TopoShape.cpp index 935a7eff6c..86664beb21 100644 --- a/src/Mod/Part/App/TopoShape.cpp +++ b/src/Mod/Part/App/TopoShape.cpp @@ -3148,8 +3148,9 @@ void TopoShape::sewShape(double tolerance) bool TopoShape::fix() { - if (this->_Shape.IsNull()) + if (this->_Shape.IsNull()) { return false; + } // First, we do fix regardless if the current shape is valid or not, // because not all problems that are handled by ShapeFix_Shape can be @@ -3164,12 +3165,14 @@ bool TopoShape::fix() ShapeFix_Shape fix(copy._Shape); fix.Perform(); - if (fix.Shape().IsSame(copy._Shape)) + if (fix.Shape().IsSame(copy._Shape)) { return false; + } BRepCheck_Analyzer aChecker(fix.Shape()); - if (!aChecker.IsValid()) + if (!aChecker.IsValid()) { return false; + } // If the above fix produces a valid shape, then we fix the original shape, // because BRepBuilderAPI_Copy has some undesired side effect (e.g. flatten @@ -3189,8 +3192,10 @@ bool TopoShape::fix() // remapping, there will be invalid index jumpping in reference in // Sketch002.ExternalEdge5. makeShapeWithElementMap(fixThis.Shape(), MapperHistory(fixThis), {*this}); - } else + } + else { makeShapeWithElementMap(fix.Shape(), MapperHistory(fix), {copy}); + } return true; } diff --git a/src/Mod/Part/App/TopoShape.h b/src/Mod/Part/App/TopoShape.h index 4350d313f7..5ba19be764 100644 --- a/src/Mod/Part/App/TopoShape.h +++ b/src/Mod/Part/App/TopoShape.h @@ -30,10 +30,6 @@ #include #include -#include -#include -#include -#include #include #include #include @@ -43,6 +39,9 @@ #include #include #include +#include +#include +#include class gp_Ax1; class gp_Ax2; @@ -695,7 +694,7 @@ public: bool canMapElement(const TopoShape &other) 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; + void mapSubElementsTo(std::vector& shapes, const char* op = nullptr) const; bool hasPendingElementMap() const; /** Helper class to return the generated and modified shape given an input shape @@ -741,8 +740,8 @@ public: * return the shape as given, or to force it to be placed in a Compound. */ enum class SingleShapeCompoundCreationPolicy { - RETURN_SHAPE, - FORCE_COMPOUND + returnShape, + forceCompound }; /** Make a compound shape @@ -759,12 +758,14 @@ public: */ TopoShape& makeElementCompound(const std::vector& shapes, const char* op = nullptr, - SingleShapeCompoundCreationPolicy policy = SingleShapeCompoundCreationPolicy::FORCE_COMPOUND); + SingleShapeCompoundCreationPolicy policy = + SingleShapeCompoundCreationPolicy::forceCompound); - enum class ConnectionPolicy { - REQUIRE_SHARED_VERTEX, - MERGE_WITH_TOLERANCE + enum class ConnectionPolicy + { + requireSharedVertex, + mergeWithTolerance }; /** Make a compound of wires by connecting input edges @@ -792,7 +793,7 @@ public: TopoShape& makeElementWires(const std::vector& shapes, const char* op = nullptr, double tol = 0.0, - ConnectionPolicy policy = ConnectionPolicy::MERGE_WITH_TOLERANCE, + ConnectionPolicy policy = ConnectionPolicy::mergeWithTolerance, TopoShapeMap* output = nullptr); @@ -805,8 +806,8 @@ public: * @param keepOrder: whether to respect the order of the input edges * @param tol: tolerance for checking the distance of two vertex to decide * if two edges are connected - * @param policy: if REQUIRE_SHARED_VERTEX, then only connect edges if they shared the same - * vertex. If MERGE_WITH_TOLERANCE use \c tol to check for connection. + * @param policy: if requireSharedVertex, then only connect edges if they shared the same + * vertex. If mergeWithTolerance use \c tol to check for connection. * @param output: optional output mapping from wire edges to input edge. * Note that edges may be modified after adding to the wire, * so the output edges may not be the same as the input @@ -821,7 +822,7 @@ public: TopoShape& makeElementWires(const TopoShape& shape, const char* op = nullptr, double tol = 0.0, - ConnectionPolicy policy = ConnectionPolicy::MERGE_WITH_TOLERANCE, + ConnectionPolicy policy = ConnectionPolicy::mergeWithTolerance, TopoShapeMap* output = nullptr); /** Make a compound of wires by connecting input edges in the given order @@ -869,14 +870,13 @@ public: */ TopoShape makeElementWires(const char* op = nullptr, double tol = 0.0, - ConnectionPolicy policy = ConnectionPolicy::MERGE_WITH_TOLERANCE, + ConnectionPolicy policy = ConnectionPolicy::mergeWithTolerance, TopoShapeMap* output = nullptr) const { return TopoShape(0, Hasher).makeElementWires(*this, op, tol, policy, output); } - /** Make a deep copy of the shape * * @param source: input shape @@ -890,7 +890,10 @@ public: * TopoShape itself as a self reference so that multiple operations * can be carried out for the same shape in the same line of code. */ - TopoShape &makeElementCopy(const TopoShape &source, const char *op=nullptr, bool copyGeom=true, bool copyMesh=false); + TopoShape& makeElementCopy(const TopoShape& source, + const char* op = nullptr, + bool copyGeom = true, + bool copyMesh = false); /** Make a deep copy of the shape * @@ -902,8 +905,10 @@ public: * @return Return a deep copy of the shape. The shape itself is not * modified */ - TopoShape makeElementCopy(const char *op=nullptr, bool copyGeom=true, bool copyMesh=false) const { - return TopoShape(Tag,Hasher).makeElementCopy(*this,op,copyGeom,copyMesh); + TopoShape + makeElementCopy(const char* op = nullptr, bool copyGeom = true, bool copyMesh = false) const + { + return TopoShape(Tag, Hasher).makeElementCopy(*this, op, copyGeom, copyMesh); } /* Make a shell using this shape @@ -1398,77 +1403,7 @@ private: */ static std::deque sortEdges(std::list& edges, bool keepOrder = false, double tol = 0.0); - static TopoShape reverseEdge (const TopoShape& edge); -}; - - -/// Shape hasher that ignore orientation -struct ShapeHasher -{ - inline size_t operator()(const TopoShape& s) const - { -#if OCC_VERSION_HEX >= 0x070800 - return std::hash {}(s.getShape()); -#else - return s.getShape().HashCode(INT_MAX); -#endif - } - inline size_t operator()(const TopoDS_Shape& s) const - { -#if OCC_VERSION_HEX >= 0x070800 - return std::hash {}(s); -#else - return s.HashCode(INT_MAX); -#endif - } - inline bool operator()(const TopoShape& a, const TopoShape& b) const - { - return a.getShape().IsSame(b.getShape()); - } - inline bool operator()(const TopoDS_Shape& a, const TopoDS_Shape& b) const - { - return a.IsSame(b); - } - template - static inline void hash_combine(std::size_t& seed, const T& v) - { - // copied from boost::hash_combine - std::hash hasher; - seed ^= hasher(v) + 0x9e3779b9 + (seed << 6) + (seed >> 2); - } - inline size_t operator()(const std::pair& s) const - { -#if OCC_VERSION_HEX >= 0x070800 - size_t res = std::hash {}(s.first.getShape()); - hash_combine(res, std::hash {}(s.second.getShape())); -#else - size_t res = s.first.getShape().HashCode(INT_MAX); - hash_combine(res, s.second.getShape().HashCode(INT_MAX)); -#endif - return res; - } - inline size_t operator()(const std::pair& s) const - { -#if OCC_VERSION_HEX >= 0x070800 - size_t res = std::hash {}(s.first); - hash_combine(res, std::hash {}(s.second)); -#else - size_t res = s.first.HashCode(INT_MAX); - hash_combine(res, s.second.HashCode(INT_MAX)); -#endif - return res; - } - inline bool operator()(const std::pair& a, - const std::pair& b) const - { - return a.first.getShape().IsSame(b.first.getShape()) - && a.second.getShape().IsSame(b.second.getShape()); - } - inline bool operator()(const std::pair& a, - const std::pair& b) const - { - return a.first.IsSame(b.first) && a.second.IsSame(b.second); - } + static TopoShape reverseEdge(const TopoShape& edge); }; /** Shape mapper for generic BRepBuilderAPI_MakeShape derived class @@ -1476,13 +1411,14 @@ struct ShapeHasher * Uses BRepBuilderAPI_MakeShape::Modified/Generated() function to extract * shape history for generating mapped element names */ -struct PartExport MapperMaker: TopoShape::Mapper { - BRepBuilderAPI_MakeShape &maker; - MapperMaker(BRepBuilderAPI_MakeShape &maker) - :maker(maker) +struct PartExport MapperMaker: TopoShape::Mapper +{ + BRepBuilderAPI_MakeShape& maker; + explicit MapperMaker(BRepBuilderAPI_MakeShape& maker) + : maker(maker) {} - virtual const std::vector &modified(const TopoDS_Shape &s) const override; - virtual const std::vector &generated(const TopoDS_Shape &s) const override; + const std::vector& modified(const TopoDS_Shape& s) const override; + const std::vector& generated(const TopoDS_Shape& s) const override; }; /** Shape mapper for BRepTools_History @@ -1490,13 +1426,14 @@ struct PartExport MapperMaker: TopoShape::Mapper { * Uses BRepTools_History::Modified/Generated() function to extract * shape history for generating mapped element names */ -struct PartExport MapperHistory: TopoShape::Mapper { +struct PartExport MapperHistory: TopoShape::Mapper +{ Handle(BRepTools_History) history; - MapperHistory(const Handle(BRepTools_History) &history); - MapperHistory(const Handle(BRepTools_ReShape) &reshape); - MapperHistory(ShapeFix_Root &fix); - virtual const std::vector &modified(const TopoDS_Shape &s) const override; - virtual const std::vector &generated(const TopoDS_Shape &s) const override; + explicit MapperHistory(const Handle(BRepTools_History) & history); + explicit MapperHistory(const Handle(BRepTools_ReShape) & reshape); + explicit MapperHistory(ShapeFix_Root& fix); + const std::vector& modified(const TopoDS_Shape& s) const override; + const std::vector& generated(const TopoDS_Shape& s) const override; }; } // namespace Part diff --git a/src/Mod/Part/App/TopoShapeExpansion.cpp b/src/Mod/Part/App/TopoShapeExpansion.cpp index 2f9e35be88..0aa0353e7f 100644 --- a/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -44,9 +44,7 @@ #include #include #include -#include #include -#include #include #endif @@ -1293,12 +1291,11 @@ void addShapesToBuilder(const std::vector& shapes, } } // namespace -TopoShape& -TopoShape::makeElementCompound(const std::vector& shapes, - const char* op, - SingleShapeCompoundCreationPolicy policy) +TopoShape& TopoShape::makeElementCompound(const std::vector& shapes, + const char* op, + SingleShapeCompoundCreationPolicy policy) { - if (policy == SingleShapeCompoundCreationPolicy::RETURN_SHAPE && shapes.size() == 1) { + if (policy == SingleShapeCompoundCreationPolicy::returnShape && shapes.size() == 1) { *this = shapes[0]; return *this; } @@ -1348,7 +1345,7 @@ TopoShape& TopoShape::makeElementWires(const TopoShape& shape, tol = Precision::Confusion(); } - if (policy == ConnectionPolicy::REQUIRE_SHARED_VERTEX) { + if (policy == ConnectionPolicy::requireSharedVertex) { // Can't use ShapeAnalysis_FreeBounds if not shared. It seems the output // edges are modified somehow, and it is not obvious how to map the // resulting edges. @@ -1371,7 +1368,7 @@ TopoShape& TopoShape::makeElementWires(const TopoShape& shape, wires.emplace_back(Tag, Hasher, wire); } shape.mapSubElementsTo(wires, op); - return makeElementCompound(wires, "", SingleShapeCompoundCreationPolicy::RETURN_SHAPE); + return makeElementCompound(wires, "", SingleShapeCompoundCreationPolicy::returnShape); } std::vector wires; @@ -1429,7 +1426,7 @@ TopoShape& TopoShape::makeElementWires(const TopoShape& shape, wires.back().mapSubElement(edges, op); wires.back().fix(); } - return makeElementCompound(wires, nullptr, SingleShapeCompoundCreationPolicy::RETURN_SHAPE); + return makeElementCompound(wires, nullptr, SingleShapeCompoundCreationPolicy::returnShape); } @@ -1438,7 +1435,7 @@ struct EdgePoints gp_Pnt v1, v2; std::list::iterator it; const TopoShape* edge; - bool closed{false}; + bool closed {false}; EdgePoints(std::list::iterator it, double tol) : it(it) @@ -1458,11 +1455,11 @@ struct EdgePoints } }; -TopoShape TopoShape::reverseEdge (const TopoShape& edge) { +TopoShape TopoShape::reverseEdge(const TopoShape& edge) +{ Standard_Real first = NAN; Standard_Real last = NAN; - const Handle(Geom_Curve)& curve = - BRep_Tool::Curve(TopoDS::Edge(edge.getShape()), first, last); + const Handle(Geom_Curve)& curve = BRep_Tool::Curve(TopoDS::Edge(edge.getShape()), first, last); first = curve->ReversedParameter(first); last = curve->ReversedParameter(last); TopoShape res(BRepBuilderAPI_MakeEdge(curve->Reversed(), last, first)); @@ -1596,7 +1593,8 @@ TopoShape& TopoShape::makeElementOrderedWires(const std::vector& shap std::vector wires; std::list edgeList; - auto shape = TopoShape().makeElementCompound(shapes, "", SingleShapeCompoundCreationPolicy::RETURN_SHAPE); + auto shape = + TopoShape().makeElementCompound(shapes, "", SingleShapeCompoundCreationPolicy::returnShape); for (auto& edge : shape.getSubTopoShapes(TopAbs_EDGE)) { edgeList.push_back(edge); } @@ -1618,29 +1616,34 @@ TopoShape& TopoShape::makeElementOrderedWires(const std::vector& shap wires.emplace_back(mkWire.Wire()); wires.back().mapSubElement(edges, op); } - return makeElementCompound(wires, nullptr, SingleShapeCompoundCreationPolicy::RETURN_SHAPE); + return makeElementCompound(wires, nullptr, SingleShapeCompoundCreationPolicy::returnShape); } -TopoShape &TopoShape::makeElementCopy(const TopoShape &shape, const char *op, bool copyGeom, bool copyMesh) +TopoShape& +TopoShape::makeElementCopy(const TopoShape& shape, const char* op, bool copyGeom, bool copyMesh) { - if(shape.isNull()) + if (shape.isNull()) { return *this; + } TopoShape tmp(shape); #if OCC_VERSION_HEX >= 0x070000 - tmp.setShape(BRepBuilderAPI_Copy(shape.getShape(),copyGeom,copyMesh).Shape(), false); + tmp.setShape(BRepBuilderAPI_Copy(shape.getShape(), copyGeom, copyMesh).Shape(), false); #else tmp.setShape(BRepBuilderAPI_Copy(shape.getShape()).Shape(), false); #endif - if(op || (shape.Tag && shape.Tag!=Tag)) { + if (op || (shape.Tag && shape.Tag != Tag)) { setShape(tmp._Shape); initCache(); - if (!Hasher) + if (!Hasher) { Hasher = tmp.Hasher; + } copyElementMap(tmp, op); - }else + } + else { *this = tmp; + } return *this; } @@ -2135,52 +2138,58 @@ const std::vector& MapperMaker::generated(const TopoDS_Shape& s) c return _res; } -MapperHistory::MapperHistory(const Handle(BRepTools_History) &history) - :history(history) +MapperHistory::MapperHistory(const Handle(BRepTools_History) & history) + : history(history) {} -MapperHistory::MapperHistory(const Handle(BRepTools_ReShape) &reshape) +MapperHistory::MapperHistory(const Handle(BRepTools_ReShape) & reshape) { - if (reshape) + if (reshape) { history = reshape->History(); + } } -MapperHistory::MapperHistory(ShapeFix_Root &fix) +MapperHistory::MapperHistory(ShapeFix_Root& fix) { - if (fix.Context()) + if (fix.Context()) { history = fix.Context()->History(); + } } -const std::vector & -MapperHistory::modified(const TopoDS_Shape &s) const +const std::vector& MapperHistory::modified(const TopoDS_Shape& s) const { _res.clear(); try { if (history) { TopTools_ListIteratorOfListOfShape it; - for (it.Initialize(history->Modified(s)); it.More(); it.Next()) + for (it.Initialize(history->Modified(s)); it.More(); it.Next()) { _res.push_back(it.Value()); + } } - } catch (const Standard_Failure & e) { - if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) + } + catch (const Standard_Failure& e) { + if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) { FC_WARN("Exception on shape mapper: " << e.GetMessageString()); + } } return _res; } -const std::vector & -MapperHistory::generated(const TopoDS_Shape &s) const +const std::vector& MapperHistory::generated(const TopoDS_Shape& s) const { _res.clear(); try { if (history) { TopTools_ListIteratorOfListOfShape it; - for (it.Initialize(history->Generated(s)); it.More(); it.Next()) + for (it.Initialize(history->Generated(s)); it.More(); it.Next()) { _res.push_back(it.Value()); + } } - } catch (const Standard_Failure & e) { - if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) + } + catch (const Standard_Failure& e) { + if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) { FC_WARN("Exception on shape mapper: " << e.GetMessageString()); + } } return _res; } diff --git a/src/Mod/Part/App/TopoShapeMapper.h b/src/Mod/Part/App/TopoShapeMapper.h index 9afc31b3b1..5ec165efab 100644 --- a/src/Mod/Part/App/TopoShapeMapper.h +++ b/src/Mod/Part/App/TopoShapeMapper.h @@ -25,7 +25,6 @@ #include #include -#include #include #include #include @@ -39,11 +38,81 @@ class ShapeFix_Root; namespace Part { +/// Shape hasher that ignore orientation +struct ShapeHasher +{ + inline size_t operator()(const TopoShape& s) const + { +#if OCC_VERSION_HEX >= 0x070800 + return std::hash {}(s.getShape()); +#else + return s.getShape().HashCode(INT_MAX); +#endif + } + inline size_t operator()(const TopoDS_Shape& s) const + { +#if OCC_VERSION_HEX >= 0x070800 + return std::hash {}(s); +#else + return s.HashCode(INT_MAX); +#endif + } + inline bool operator()(const TopoShape& a, const TopoShape& b) const + { + return a.getShape().IsSame(b.getShape()); + } + inline bool operator()(const TopoDS_Shape& a, const TopoDS_Shape& b) const + { + return a.IsSame(b); + } + template + static inline void hash_combine(std::size_t& seed, const T& v) + { + // copied from boost::hash_combine + std::hash hasher; + seed ^= hasher(v) + 0x9e3779b9 + (seed << 6) + (seed >> 2); + } + inline size_t operator()(const std::pair& s) const + { +#if OCC_VERSION_HEX >= 0x070800 + size_t res = std::hash {}(s.first.getShape()); + hash_combine(res, std::hash {}(s.second.getShape())); +#else + size_t res = s.first.getShape().HashCode(INT_MAX); + hash_combine(res, s.second.getShape().HashCode(INT_MAX)); +#endif + return res; + } + inline size_t operator()(const std::pair& s) const + { +#if OCC_VERSION_HEX >= 0x070800 + size_t res = std::hash {}(s.first); + hash_combine(res, std::hash {}(s.second)); +#else + size_t res = s.first.HashCode(INT_MAX); + hash_combine(res, s.second.HashCode(INT_MAX)); +#endif + return res; + } + inline bool operator()(const std::pair& a, + const std::pair& b) const + { + return a.first.getShape().IsSame(b.first.getShape()) + && a.second.getShape().IsSame(b.second.getShape()); + } + inline bool operator()(const std::pair& a, + const std::pair& b) const + { + return a.first.IsSame(b.first) && a.second.IsSame(b.second); + } +}; + enum class MappingStatus { Generated, Modified }; + /** Shape mapper for user defined shape mapping */ struct PartExport ShapeMapper: TopoShape::Mapper diff --git a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp index 44d9755ace..f92da3d9c0 100644 --- a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -60,7 +60,7 @@ TEST_F(TopoShapeExpansionTest, makeElementCompoundOneShapeReturnsShape) topoShape.makeElementCompound(shapes, "C", Part::TopoShape::SingleShapeCompoundCreationPolicy:: - RETURN_SHAPE /*Don't force the creation*/); + returnShape /*Don't force the creation*/); // Assert EXPECT_EQ(edge.ShapeType(), topoShape.getShape().ShapeType()); // NOT a Compound @@ -77,7 +77,7 @@ TEST_F(TopoShapeExpansionTest, makeElementCompoundOneShapeForceReturnsCompound) topoShape.makeElementCompound( shapes, "C", - Part::TopoShape::SingleShapeCompoundCreationPolicy::FORCE_COMPOUND /*Force the creation*/); + Part::TopoShape::SingleShapeCompoundCreationPolicy::forceCompound /*Force the creation*/); // Assert EXPECT_NE(edge.ShapeType(), topoShape.getShape().ShapeType()); // No longer the same thing