From f4cc2df2bd7926db1e69575006ae56a5daf52cb3 Mon Sep 17 00:00:00 2001 From: "Zheng, Lei" Date: Wed, 17 Jan 2024 20:38:50 -0500 Subject: [PATCH 1/6] Toponaming move makEFace as makeElementFace and dependencies --- src/App/ComplexGeoData.h | 4 + src/App/MappedElement.cpp | 109 +++++++++++++ src/App/MappedElement.h | 17 ++ src/Mod/Part/App/FaceMaker.cpp | 108 ++++++++++++- src/Mod/Part/App/FaceMaker.h | 35 ++-- src/Mod/Part/App/FeatureExtrusion.cpp | 6 +- src/Mod/Part/App/TopoShape.h | 80 ++++++++- src/Mod/Part/App/TopoShapeExpansion.cpp | 206 ++++++++++++++++++++++++ 8 files changed, 543 insertions(+), 22 deletions(-) diff --git a/src/App/ComplexGeoData.h b/src/App/ComplexGeoData.h index 7bf68b3bea..d5630c3560 100644 --- a/src/App/ComplexGeoData.h +++ b/src/App/ComplexGeoData.h @@ -247,6 +247,10 @@ public: } // NOTE: getElementHistory is now in ElementMap + long getElementHistory(const MappedName & name, + MappedName *original=nullptr, std::vector *history=nullptr) const { + return _elementMap->getElementHistory(name, Tag, original, history); + }; void setMappedChildElements(const std::vector & children); std::vector getMappedChildElements() const; diff --git a/src/App/MappedElement.cpp b/src/App/MappedElement.cpp index 14eb6f99ba..41a99f2e02 100644 --- a/src/App/MappedElement.cpp +++ b/src/App/MappedElement.cpp @@ -20,5 +20,114 @@ * * **************************************************************************************************/ +// NOLINTNEXTLINE #include "PreCompiled.h" + +#ifndef _PreComp_ +# include +# include +#endif + #include "MappedElement.h" + +using namespace Data; + +bool ElementNameComp::operator()(const MappedName &a, const MappedName &b) const { + size_t size = std::min(a.size(),b.size()); + if(!size) + return a.size()bc) + res = 1; + } + }else if(std::isxdigit(ac)) + return false; + else + break; + } + if(res < 0) + return true; + else if(res > 0) + return false; + + for (; i bc) + return false; + } + return a.size()bc) + return false; + } else if(!std::isdigit(ac)) { + return false; + } else + break; + } + + // Then compare the following digits part by integer value + int res = 0; + for(;ibc) + res = 1; + } + }else if(std::isdigit(ac)) + return false; + else + break; + } + if(res < 0) + return true; + else if(res > 0) + return false; + + // Finally, compare the remaining tail using lexical order + for (; i bc) + return false; + } + return a.size() #include "FaceMaker.h" +#include #include "TopoShape.h" +#include "TopoShapeOpCode.h" TYPESYSTEM_SOURCE_ABSTRACT(Part::FaceMaker, Base::BaseClass) @@ -46,6 +48,11 @@ void Part::FaceMaker::addWire(const TopoDS_Wire& w) void Part::FaceMaker::addShape(const TopoDS_Shape& sh) { + addTopoShape(sh); +} + +void Part::FaceMaker::addTopoShape(const TopoShape& shape) { + const TopoDS_Shape &sh = shape.getShape(); if(sh.IsNull()) throw Base::ValueError("Input shape is null."); switch(sh.ShapeType()){ @@ -58,11 +65,14 @@ void Part::FaceMaker::addShape(const TopoDS_Shape& sh) case TopAbs_EDGE: this->myWires.push_back(BRepBuilderAPI_MakeWire(TopoDS::Edge(sh)).Wire()); break; + case TopAbs_FACE: + this->myInputFaces.push_back(sh); + break; default: throw Base::TypeError("Shape must be a wire, edge or compound. Something else was supplied."); break; } - this->mySourceShapes.push_back(sh); + this->mySourceShapes.push_back(shape); } void Part::FaceMaker::useCompound(const TopoDS_Compound& comp) @@ -73,14 +83,29 @@ void Part::FaceMaker::useCompound(const TopoDS_Compound& comp) } } +void Part::FaceMaker::useTopoCompound(const TopoShape& comp) +{ + for(auto &s : comp.getSubTopoShapes()) + this->addTopoShape(s); +} + const TopoDS_Face& Part::FaceMaker::Face() { - const TopoDS_Shape &sh = this->Shape(); - if(sh.IsNull()) + return TopoDS::Face(TopoFace().getShape()); +} + +const Part::TopoShape &Part::FaceMaker::TopoFace() const{ + if(this->myTopoShape.isNull()) throw NullShapeException("Part::FaceMaker: result shape is null."); - if (sh.ShapeType() != TopAbs_FACE) + if (this->myTopoShape.getShape().ShapeType() != TopAbs_FACE) throw Base::TypeError("Part::FaceMaker: return shape is not a single face."); - return TopoDS::Face(sh); + return this->myTopoShape; +} + +const Part::TopoShape &Part::FaceMaker::getTopoShape() const{ + if(this->myTopoShape.isNull()) + throw NullShapeException("Part::FaceMaker: result shape is null."); + return this->myTopoShape; } #if OCC_VERSION_HEX >= 0x070600 @@ -90,7 +115,7 @@ void Part::FaceMaker::Build() #endif { this->NotDone(); - this->myShapesToReturn.clear(); + this->myShapesToReturn = this->myInputFaces; this->myGenerated.Clear(); this->Build_Essence();//adds stuff to myShapesToReturn @@ -118,6 +143,7 @@ void Part::FaceMaker::Build() if(this->myShapesToReturn.empty()){ //nothing to do, null shape will be returned. + this->myShape = TopoDS_Shape(); } else if (this->myShapesToReturn.size() == 1){ this->myShape = this->myShapesToReturn[0]; } else { @@ -129,6 +155,72 @@ void Part::FaceMaker::Build() } this->myShape = cmp_res; } + + postBuild(); +} + +struct ElementName { + long tag; + Data::MappedName name; + Data::ElementIDRefs sids; + + ElementName(long t, const Data::MappedName &n, const Data::ElementIDRefs &sids) + :tag(t),name(n), sids(sids) + {} + + inline bool operator<(const ElementName &other) const { + if(tagother.tag) + return false; + return Data::ElementNameComp()(name,other.name); + } +}; + +void Part::FaceMaker::postBuild() { + this->myTopoShape.setShape(this->myShape); + this->myTopoShape.Hasher = this->MyHasher; + this->myTopoShape.mapSubElement(this->mySourceShapes); + int i = 0; + const char *op = this->MyOp; + if(!op) + op = Part::OpCodes::Face; + const auto &faces = this->myTopoShape.getSubTopoShapes(TopAbs_FACE); + // name the face using the edges of its outer wire + for(auto &face : faces) { + ++i; + TopoShape wire = face.splitWires(); + wire.mapSubElement(face); + std::set edgeNames; + int count = wire.countSubShapes(TopAbs_EDGE); + for(int i=1;i<=count;++i) { + Data::ElementIDRefs sids; + Data::MappedName name = face.getMappedName( + Data::IndexedName::fromConst("Edge",i), false, &sids); + if(!name) + continue; + edgeNames.emplace(wire.getElementHistory(name),name,sids); + } + if(edgeNames.empty()) + continue; + + std::vector names; + Data::ElementIDRefs sids; +#if 0 + for (auto &e : edgeNames) { + names.insert(e.name); + sids += e.sids; + } +#else + // We just use the first source element name to make the face name more + // stable + names.push_back(edgeNames.begin()->name); + sids = edgeNames.begin()->sids; +#endif + this->myTopoShape.setElementComboName( + Data::IndexedName::fromConst("Face",i),names,op,nullptr,&sids); + } + this->myTopoShape.initCache(true); this->Done(); } @@ -172,12 +264,12 @@ TYPESYSTEM_SOURCE(Part::FaceMakerSimple, Part::FaceMakerPublic) std::string Part::FaceMakerSimple::getUserFriendlyName() const { - return {QT_TRANSLATE_NOOP("Part_FaceMaker","Simple")}; + return std::string(QT_TRANSLATE_NOOP("Part_FaceMaker","Simple")); } std::string Part::FaceMakerSimple::getBriefExplanation() const { - return {QT_TRANSLATE_NOOP("Part_FaceMaker","Makes separate plane face from every wire independently. No support for holes; wires can be on different planes.")}; + return std::string(QT_TRANSLATE_NOOP("Part_FaceMaker","Makes separate plane face from every wire independently. No support for holes; wires can be on different planes.")); } void Part::FaceMakerSimple::Build_Essence() diff --git a/src/Mod/Part/App/FaceMaker.h b/src/Mod/Part/App/FaceMaker.h index aaf289c2a0..3f80639165 100644 --- a/src/Mod/Part/App/FaceMaker.h +++ b/src/Mod/Part/App/FaceMaker.h @@ -33,6 +33,8 @@ #include #include +#include +#include "TopoShape.h" namespace Part { @@ -47,11 +49,16 @@ namespace Part */ class PartExport FaceMaker: public BRepBuilderAPI_MakeShape, public Base::BaseClass { - TYPESYSTEM_HEADER_WITH_OVERRIDE(); + TYPESYSTEM_HEADER(); public: - FaceMaker() = default; - ~FaceMaker() override = default; + FaceMaker() {} + virtual ~FaceMaker() {} + + void addTopoShape(const TopoShape &s); + void useTopoCompound(const TopoShape &comp); + const TopoShape &getTopoShape() const; + const TopoShape &TopoFace() const; virtual void addWire(const TopoDS_Wire& w); /** @@ -69,6 +76,8 @@ public: */ virtual void useCompound(const TopoDS_Compound &comp); + virtual void setPlane(const gp_Pln &) {} + /** * @brief Face: returns the face (result). If result is not a single face, * throws Base::TypeError. (hint: use .Shape() instead) @@ -77,9 +86,9 @@ public: virtual const TopoDS_Face& Face(); #if OCC_VERSION_HEX >= 0x070600 - void Build(const Message_ProgressRange& theRange = Message_ProgressRange()) override; + virtual void Build(const Message_ProgressRange& theRange = Message_ProgressRange()); #else - void Build() override; + virtual void Build(); #endif //fails to compile, huh! @@ -90,11 +99,16 @@ public: static std::unique_ptr ConstructFromType(const char* className); static std::unique_ptr ConstructFromType(Base::Type type); + const char *MyOp = 0; + App::StringHasherRef MyHasher; + protected: - std::vector mySourceShapes; //wire or compound + std::vector mySourceShapes; //wire or compound std::vector myWires; //wires from mySourceShapes std::vector myCompounds; //compounds, for recursive processing std::vector myShapesToReturn; + std::vector myInputFaces; + TopoShape myTopoShape; /** * @brief Build_Essence: build routine that can assume there is no nesting. @@ -106,6 +120,7 @@ protected: * whole Build(). */ virtual void Build_Essence() = 0; + void postBuild(); static void throwNotImplemented(); }; @@ -115,7 +130,7 @@ protected: */ class PartExport FaceMakerPublic : public FaceMaker { - TYPESYSTEM_HEADER_WITH_OVERRIDE(); + TYPESYSTEM_HEADER(); public: virtual std::string getUserFriendlyName() const = 0; virtual std::string getBriefExplanation() const = 0; @@ -141,10 +156,10 @@ class PartExport FaceMakerSimple : public FaceMakerPublic { TYPESYSTEM_HEADER_WITH_OVERRIDE(); public: - std::string getUserFriendlyName() const override; - std::string getBriefExplanation() const override; + virtual std::string getUserFriendlyName() const override; + virtual std::string getBriefExplanation() const override; protected: - void Build_Essence() override; + virtual void Build_Essence() override; }; diff --git a/src/Mod/Part/App/FeatureExtrusion.cpp b/src/Mod/Part/App/FeatureExtrusion.cpp index c3fb5ba21f..bca055c84a 100644 --- a/src/Mod/Part/App/FeatureExtrusion.cpp +++ b/src/Mod/Part/App/FeatureExtrusion.cpp @@ -356,14 +356,14 @@ void FaceMakerExtrusion::Build() if (mySourceShapes.empty()) throw Base::ValueError("No input shapes!"); if (mySourceShapes.size() == 1) { - inputShape = mySourceShapes[0]; + inputShape = mySourceShapes[0].getShape(); } else { TopoDS_Builder builder; TopoDS_Compound cmp; builder.MakeCompound(cmp); - for (const TopoDS_Shape& sh : mySourceShapes) { - builder.Add(cmp, sh); + for (const auto &sh : mySourceShapes) { + builder.Add(cmp, sh.getShape()); } inputShape = cmp; } diff --git a/src/Mod/Part/App/TopoShape.h b/src/Mod/Part/App/TopoShape.h index ad4ef2a780..7a5faa1156 100644 --- a/src/Mod/Part/App/TopoShape.h +++ b/src/Mod/Part/App/TopoShape.h @@ -387,6 +387,27 @@ public: TopoDS_Shape defeaturing(const std::vector& s) const; TopoDS_Shape makeShell(const TopoDS_Shape&) const; //@} + + /// Wire re-orientation when calling splitWires() + enum SplitWireReorient { + /// Keep original reorientation + NoReorient, + /// Make outer wire forward, and inner wires reversed + Reorient, + /// Make both outer and inner wires forward + ReorientForward, + /// Make both outer and inner wires reversed + ReorientReversed, + }; + /** Return the outer and inner wires of a face + * + * @param inner: optional output of inner wires + * @param reorient: wire reorientation, see SplitWireReorient + * + * @return Return the outer wire + */ + TopoShape splitWires(std::vector *inner = nullptr, + SplitWireReorient reorient = Reorient) const; /** @name Element name mapping aware shape maker * @@ -571,6 +592,11 @@ public: const std::string& shapeName(bool silent = false) const; static std::pair shapeTypeAndIndex(const char* name); + Data::MappedName setElementComboName(const Data::IndexedName & element, + const std::vector &names, + const char *marker=nullptr, + const char *op=nullptr, + const Data::ElementIDRefs *sids=nullptr); /** @name sub shape cached functions * @@ -609,7 +635,7 @@ public: void copyElementMap(const TopoShape & topoShape, const char *op=nullptr); 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); + void mapSubElement(const std::vector &shapes, const char *op=nullptr); bool hasPendingElementMap() const; @@ -629,6 +655,58 @@ public: */ TopoShape &makeElementCompound(const std::vector &shapes, const char *op=nullptr, bool force=true); + TopoShape &makeElementFace(const std::vector &shapes, + const char *op = nullptr, + const char *maker = nullptr, + const gp_Pln *pln = nullptr); + /** Make a planar face with the input wire or edge + * + * @param shape: input shape. Can be either edge, wire, or compound of + * those two types + * @param op: optional string to be encoded into topo naming for indicating + * the operation + * @param maker: optional type name of the face maker. If not given, + * default to "Part::FaceMakerBullseye" + * @param pln: optional plane of the face. + * + * @return The function creates a planar face. The original content of this + * TopoShape is discarded and replaced with the new shape. The + * function returns the TopoShape itself as a reference so that + * multiple operations can be carried out for the same shape in the + * same line of code. + */ + TopoShape &makeElementFace(const TopoShape &shape, + const char *op = nullptr, + const char *maker = nullptr, + const gp_Pln *pln = nullptr); + /** Make a planar face using this shape + * + * @param op: optional string to be encoded into topo naming for indicating + * the operation + * @param maker: optional type name of the face maker. If not given, + * default to "Part::FaceMakerBullseye" + * @param pln: optional plane of the face. + * + * @return The function returns a new planar face made using the wire or edge + * inside this shape. The shape itself is not modified. + */ + TopoShape makeElementFace(const char *op = nullptr, + const char *maker = nullptr, + const gp_Pln *pln = nullptr) const { + return TopoShape(0,Hasher).makeElementFace(*this,op,maker,pln); + } + + /// Filling style when making a BSpline face + enum FillingStyle { + /// The style with the flattest patches + FillingStyle_Strech, + /// A rounded style of patch with less depth than those of Curved + FillingStyle_Coons, + /// The style with the most rounded patches + FillingStyle_Curved, + }; + + friend class TopoShapeCache; private: diff --git a/src/Mod/Part/App/TopoShapeExpansion.cpp b/src/Mod/Part/App/TopoShapeExpansion.cpp index d5b5b5227b..7bf252fdf0 100644 --- a/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -26,11 +26,19 @@ #include "PreCompiled.h" #ifndef _PreComp_ #include +# include #include +#include + +# include +# include + #endif #include "TopoShape.h" #include "TopoShapeCache.h" +#include "FaceMaker.h" + FC_LOG_LEVEL_INIT("TopoShape", true, true) // NOLINT @@ -540,4 +548,202 @@ TopoShape::makeElementCompound(const std::vector& shapes, const char* return *this; } + +TopoShape &TopoShape::makeElementFace(const TopoShape &shape, + const char *op, + const char *maker, + const gp_Pln *pln) +{ + std::vector shapes; + if (shape.isNull()) + FC_THROWM(NullShapeException, "Null shape"); + if(shape.getShape().ShapeType() == TopAbs_COMPOUND) + shapes = shape.getSubTopoShapes(); + else + shapes.push_back(shape); + return makeElementFace(shapes,op,maker,pln); +} + +TopoShape &TopoShape::makeElementFace(const std::vector &shapes, + const char *op, + const char *maker, + const gp_Pln *pln) +{ + if(!maker || !maker[0]) maker = "Part::FaceMakerBullseye"; + std::unique_ptr mkFace = FaceMaker::ConstructFromType(maker); + mkFace->MyHasher = Hasher; + mkFace->MyOp = op; + if (pln) + mkFace->setPlane(*pln); + + for(auto &s : shapes) { + if (s.getShape().ShapeType() == TopAbs_COMPOUND) + mkFace->useTopoCompound(s); + else + mkFace->addTopoShape(s); + } + mkFace->Build(); + + const auto &ret = mkFace->getTopoShape(); + setShape(ret._Shape); + Hasher = ret.Hasher; + resetElementMap(ret.elementMap()); + if (!isValid()) { + ShapeFix_ShapeTolerance aSFT; + aSFT.LimitTolerance(getShape(), + Precision::Confusion(), Precision::Confusion(), TopAbs_SHAPE); + + // In some cases, the OCC reports the returned shape having invalid + // tolerance. Not sure about the real cause. + // + // Update: one of the cause is related to OCC bug in + // BRepBuilder_FindPlane, A possible call sequence is, + // + // makEOffset2D() -> TopoShape::findPlane() -> BRepLib_FindSurface + // + // See code comments in findPlane() for the description of the bug and + // work around. + + ShapeFix_Shape fixer(getShape()); + fixer.Perform(); + setShape(fixer.Shape(), false); + + if (!isValid()) + FC_WARN("makeElementFace: resulting face is invalid"); + } + return *this; +} + +Data::MappedName TopoShape::setElementComboName(const Data::IndexedName & element, + const std::vector &names, + const char *marker, + const char *op, + const Data::ElementIDRefs *_sids) +{ + if(names.empty()) + return Data::MappedName(); + std::string _marker; + if(!marker) + marker = elementMapPrefix().c_str(); + else if(!boost::starts_with(marker,elementMapPrefix())){ + _marker = elementMapPrefix() + marker; + marker = _marker.c_str(); + } + auto it = names.begin(); + Data::MappedName newName = *it; + std::ostringstream ss; + Data::ElementIDRefs sids; + if (_sids) + sids = *_sids; + if(names.size() == 1) + ss << marker; + else { + bool first = true; + ss.str(""); + if(!Hasher) + ss << marker; + ss << '('; + for(++it;it!=names.end();++it) { + if(first) + first = false; + else + ss << '|'; + ss << *it; + } + ss << ')'; + if(Hasher) { + sids.push_back(Hasher->getID(ss.str().c_str())); + ss.str(""); + ss << marker << sids.back().toString(); + } + } + elementMap()->encodeElementName(element[0],newName,ss,&sids,Tag,op); + return elementMap()->setElementName(element,newName, Tag, &sids); +} + +TopoShape TopoShape::splitWires(std::vector *inner, + SplitWireReorient reorient) const +{ + // ShapeAnalysis::OuterWire() is un-reliable for some reason. OCC source + // code shows it works by creating face using each wire, and then test using + // BRepTopAdaptor_FClass2d::PerformInfinitePoint() to check if it is an out + // bound wire. And practice shows it sometimes returns the incorrect + // result. Need more investigation. Note that this may be related to + // unreliable solid face orientation + // (https://forum.freecadweb.org/viewtopic.php?p=446006#p445674) + // + // Use BrepTools::OuterWire() instead. OCC source code shows it is + // implemented using simple bound box checking. This should be a + // reliable method, especially so for a planar face. + + TopoDS_Shape tmp; + if (shapeType(true) == TopAbs_FACE) + tmp = BRepTools::OuterWire(TopoDS::Face(_Shape)); + else if (countSubShapes(TopAbs_FACE) == 1) + tmp = BRepTools::OuterWire( + TopoDS::Face(getSubShape(TopAbs_FACE, 1))); + if (tmp.IsNull()) + return TopoShape(); + const auto & wires = getSubTopoShapes(TopAbs_WIRE); + auto it = wires.begin(); + + TopAbs_Orientation orientOuter, orientInner; + switch(reorient) { + case ReorientReversed: + orientOuter = orientInner = TopAbs_REVERSED; + break; + case ReorientForward: + orientOuter = orientInner = TopAbs_FORWARD; + break; + default: + orientOuter = TopAbs_FORWARD; + orientInner = TopAbs_REVERSED; + break; + } + + auto doReorient = [](TopoShape &s, TopAbs_Orientation orient) { + // Special case of single edge wire. Make sure the edge is in the + // required orientation. This is necessary because BRepFill_OffsetWire + // has special handling of circular edge offset, which seem to only + // respect the edge orientation and disregard the wire orientation. The + // orientation is used to determine whether to shrink or expand. + if (s.countSubShapes(TopAbs_EDGE) == 1) { + TopoDS_Shape e = s.getSubShape(TopAbs_EDGE, 1); + if (e.Orientation() == orient) { + if (s._Shape.Orientation() == orient) + return; + } else + e = e.Oriented(orient); + BRepBuilderAPI_MakeWire mkWire(TopoDS::Edge(e)); + s.setShape(mkWire.Shape(), false); + } + else if (s._Shape.Orientation() != orient) + s.setShape(s._Shape.Oriented(orient), false); + }; + + for (; it != wires.end(); ++it) { + auto & wire = *it; + if (wire.getShape().IsSame(tmp)) { + if (inner) { + for (++it; it != wires.end(); ++it) { + inner->push_back(*it); + if (reorient) + doReorient(inner->back(), orientInner); + } + } + auto res = wire; + if (reorient) + doReorient(res, orientOuter); + return res; + } + if (inner) { + inner->push_back(wire); + if (reorient) + doReorient(inner->back(), orientInner); + } + } + return TopoShape(); +} + + } // namespace Part From d4c5906c5c8d8c1c88a417034c09732e5c518251 Mon Sep 17 00:00:00 2001 From: bgbsww Date: Sun, 21 Jan 2024 15:34:46 -0500 Subject: [PATCH 2/6] Add tests, reformat to modern C++, clean --- src/Mod/Part/App/FaceMaker.h | 6 +- src/Mod/Part/App/TopoShape.h | 2 +- src/Mod/Part/App/TopoShapeExpansion.cpp | 186 +++++++----- tests/src/Mod/Part/App/PartTestHelpers.cpp | 27 +- tests/src/Mod/Part/App/PartTestHelpers.h | 8 +- tests/src/Mod/Part/App/TopoShapeExpansion.cpp | 279 +++++++++++++++++- 6 files changed, 422 insertions(+), 86 deletions(-) diff --git a/src/Mod/Part/App/FaceMaker.h b/src/Mod/Part/App/FaceMaker.h index 3f80639165..fc40e243a1 100644 --- a/src/Mod/Part/App/FaceMaker.h +++ b/src/Mod/Part/App/FaceMaker.h @@ -156,10 +156,10 @@ class PartExport FaceMakerSimple : public FaceMakerPublic { TYPESYSTEM_HEADER_WITH_OVERRIDE(); public: - virtual std::string getUserFriendlyName() const override; - virtual std::string getBriefExplanation() const override; + std::string getUserFriendlyName() const override; + std::string getBriefExplanation() const override; protected: - virtual void Build_Essence() override; + void Build_Essence() override; }; diff --git a/src/Mod/Part/App/TopoShape.h b/src/Mod/Part/App/TopoShape.h index 7a5faa1156..b5bf372a93 100644 --- a/src/Mod/Part/App/TopoShape.h +++ b/src/Mod/Part/App/TopoShape.h @@ -81,7 +81,7 @@ class PartExport ShapeSegment: public Data::Segment TYPESYSTEM_HEADER_WITH_OVERRIDE(); public: - ShapeSegment(const TopoDS_Shape& ShapeIn) + explicit ShapeSegment(const TopoDS_Shape& ShapeIn) : Shape(ShapeIn) {} ShapeSegment() = default; diff --git a/src/Mod/Part/App/TopoShapeExpansion.cpp b/src/Mod/Part/App/TopoShapeExpansion.cpp index 7bf252fdf0..c59ada479b 100644 --- a/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -26,12 +26,12 @@ #include "PreCompiled.h" #ifndef _PreComp_ #include -# include +#include #include #include -# include -# include +#include +#include #endif @@ -480,7 +480,7 @@ void TopoShape::mapCompoundSubElements(const std::vector& shapes, con ++count; auto subshape = getSubShape(TopAbs_SHAPE, count, /*silent = */ true); if (!subshape.IsPartner(topoShape._Shape)) { - return; // Not a partner shape, don't do any mapping at all + return; // Not a partner shape, don't do any mapping at all } } auto children {createChildMap(count, shapes, op)}; @@ -549,49 +549,59 @@ TopoShape::makeElementCompound(const std::vector& shapes, const char* } -TopoShape &TopoShape::makeElementFace(const TopoShape &shape, - const char *op, - const char *maker, - const gp_Pln *pln) +TopoShape& TopoShape::makeElementFace(const TopoShape& shape, + const char* op, + const char* maker, + const gp_Pln* pln) { std::vector shapes; - if (shape.isNull()) - FC_THROWM(NullShapeException, "Null shape"); - if(shape.getShape().ShapeType() == TopAbs_COMPOUND) + if (shape.isNull()) { + FC_THROWM(NullShapeException, "Null shape"); + } + if (shape.getShape().ShapeType() == TopAbs_COMPOUND) { shapes = shape.getSubTopoShapes(); - else + } + else { shapes.push_back(shape); - return makeElementFace(shapes,op,maker,pln); + } + return makeElementFace(shapes, op, maker, pln); } -TopoShape &TopoShape::makeElementFace(const std::vector &shapes, - const char *op, - const char *maker, - const gp_Pln *pln) +TopoShape& TopoShape::makeElementFace(const std::vector& shapes, + const char* op, + const char* maker, + const gp_Pln* pln) { - if(!maker || !maker[0]) maker = "Part::FaceMakerBullseye"; + if (!maker || !maker[0]) { + maker = "Part::FaceMakerBullseye"; + } std::unique_ptr mkFace = FaceMaker::ConstructFromType(maker); mkFace->MyHasher = Hasher; mkFace->MyOp = op; - if (pln) + if (pln) { mkFace->setPlane(*pln); + } - for(auto &s : shapes) { - if (s.getShape().ShapeType() == TopAbs_COMPOUND) + for (auto& s : shapes) { + if (s.getShape().ShapeType() == TopAbs_COMPOUND) { mkFace->useTopoCompound(s); - else + } + else { mkFace->addTopoShape(s); + } } mkFace->Build(); - const auto &ret = mkFace->getTopoShape(); + const auto& ret = mkFace->getTopoShape(); setShape(ret._Shape); Hasher = ret.Hasher; resetElementMap(ret.elementMap()); if (!isValid()) { ShapeFix_ShapeTolerance aSFT; aSFT.LimitTolerance(getShape(), - Precision::Confusion(), Precision::Confusion(), TopAbs_SHAPE); + Precision::Confusion(), + Precision::Confusion(), + TopAbs_SHAPE); // In some cases, the OCC reports the returned shape having invalid // tolerance. Not sure about the real cause. @@ -608,24 +618,39 @@ TopoShape &TopoShape::makeElementFace(const std::vector &shapes, fixer.Perform(); setShape(fixer.Shape(), false); - if (!isValid()) + if (!isValid()) { FC_WARN("makeElementFace: resulting face is invalid"); + } } return *this; } -Data::MappedName TopoShape::setElementComboName(const Data::IndexedName & element, - const std::vector &names, - const char *marker, - const char *op, - const Data::ElementIDRefs *_sids) +/** + * Encode and set an element name in the elementMap. If a hasher is defined, apply it to the name. + * + * @param element The element name(type) that provides 1 one character suffix to the name IF . + * @param names The subnames to build the name from. If empty, return the TopoShape MappedName. + * @param marker The elementMap name or suffix to start the name with. If null, use the + * elementMapPrefix. + * @param op The op text passed to the element name encoder along with the TopoShape Tag + * @param _sids If defined, records the sub ids processed. + * + * @return The encoded, possibly hashed name. + */ +Data::MappedName TopoShape::setElementComboName(const Data::IndexedName& element, + const std::vector& names, + const char* marker, + const char* op, + const Data::ElementIDRefs* _sids) { - if(names.empty()) + if (names.empty()) { return Data::MappedName(); + } std::string _marker; - if(!marker) + if (!marker) { marker = elementMapPrefix().c_str(); - else if(!boost::starts_with(marker,elementMapPrefix())){ + } + else if (!boost::starts_with(marker, elementMapPrefix())) { _marker = elementMapPrefix() + marker; marker = _marker.c_str(); } @@ -633,36 +658,50 @@ Data::MappedName TopoShape::setElementComboName(const Data::IndexedName & elemen Data::MappedName newName = *it; std::ostringstream ss; Data::ElementIDRefs sids; - if (_sids) + if (_sids) { sids = *_sids; - if(names.size() == 1) + } + if (names.size() == 1) { ss << marker; + } else { bool first = true; ss.str(""); - if(!Hasher) + if (!Hasher) { ss << marker; + } ss << '('; - for(++it;it!=names.end();++it) { - if(first) + for (++it; it != names.end(); ++it) { + if (first) { first = false; - else + } + else { ss << '|'; + } ss << *it; } ss << ')'; - if(Hasher) { + if (Hasher) { sids.push_back(Hasher->getID(ss.str().c_str())); ss.str(""); ss << marker << sids.back().toString(); } } - elementMap()->encodeElementName(element[0],newName,ss,&sids,Tag,op); - return elementMap()->setElementName(element,newName, Tag, &sids); + elementMap()->encodeElementName(element[0], newName, ss, &sids, Tag, op); + return elementMap()->setElementName(element, newName, Tag, &sids); } -TopoShape TopoShape::splitWires(std::vector *inner, - SplitWireReorient reorient) const +/** + * Reorient the outer and inner wires of the TopoShape + * + * @param inner If this is not a nullptr, then any inner wires processed will be returned in this + * vector. + * @param reorient One of NoReorient, Reorient ( Outer forward, inner reversed ), + * ReorientForward ( all forward ), or ReorientReversed ( all reversed ) + * @return The outer wire, or an empty TopoShape if this isn't a Face, has no Face subShapes, or the + * outer wire isn't found. + */ +TopoShape TopoShape::splitWires(std::vector* inner, SplitWireReorient reorient) const { // ShapeAnalysis::OuterWire() is un-reliable for some reason. OCC source // code shows it works by creating face using each wire, and then test using @@ -677,31 +716,33 @@ TopoShape TopoShape::splitWires(std::vector *inner, // reliable method, especially so for a planar face. TopoDS_Shape tmp; - if (shapeType(true) == TopAbs_FACE) + if (shapeType(true) == TopAbs_FACE) { tmp = BRepTools::OuterWire(TopoDS::Face(_Shape)); - else if (countSubShapes(TopAbs_FACE) == 1) - tmp = BRepTools::OuterWire( - TopoDS::Face(getSubShape(TopAbs_FACE, 1))); - if (tmp.IsNull()) + } + else if (countSubShapes(TopAbs_FACE) == 1) { + tmp = BRepTools::OuterWire(TopoDS::Face(getSubShape(TopAbs_FACE, 1))); + } + if (tmp.IsNull()) { return TopoShape(); - const auto & wires = getSubTopoShapes(TopAbs_WIRE); + } + const auto& wires = getSubTopoShapes(TopAbs_WIRE); auto it = wires.begin(); TopAbs_Orientation orientOuter, orientInner; - switch(reorient) { - case ReorientReversed: - orientOuter = orientInner = TopAbs_REVERSED; - break; - case ReorientForward: - orientOuter = orientInner = TopAbs_FORWARD; - break; - default: - orientOuter = TopAbs_FORWARD; - orientInner = TopAbs_REVERSED; - break; + switch (reorient) { + case ReorientReversed: + orientOuter = orientInner = TopAbs_REVERSED; + break; + case ReorientForward: + orientOuter = orientInner = TopAbs_FORWARD; + break; + default: + orientOuter = TopAbs_FORWARD; + orientInner = TopAbs_REVERSED; + break; } - auto doReorient = [](TopoShape &s, TopAbs_Orientation orient) { + auto doReorient = [](TopoShape& s, TopAbs_Orientation orient) { // Special case of single edge wire. Make sure the edge is in the // required orientation. This is necessary because BRepFill_OffsetWire // has special handling of circular edge offset, which seem to only @@ -710,36 +751,43 @@ TopoShape TopoShape::splitWires(std::vector *inner, if (s.countSubShapes(TopAbs_EDGE) == 1) { TopoDS_Shape e = s.getSubShape(TopAbs_EDGE, 1); if (e.Orientation() == orient) { - if (s._Shape.Orientation() == orient) + if (s._Shape.Orientation() == orient) { return; - } else + } + } + else { e = e.Oriented(orient); + } BRepBuilderAPI_MakeWire mkWire(TopoDS::Edge(e)); s.setShape(mkWire.Shape(), false); } - else if (s._Shape.Orientation() != orient) + else if (s._Shape.Orientation() != orient) { s.setShape(s._Shape.Oriented(orient), false); + } }; for (; it != wires.end(); ++it) { - auto & wire = *it; + auto& wire = *it; if (wire.getShape().IsSame(tmp)) { if (inner) { for (++it; it != wires.end(); ++it) { inner->push_back(*it); - if (reorient) + if (reorient) { doReorient(inner->back(), orientInner); + } } } auto res = wire; - if (reorient) + if (reorient) { doReorient(res, orientOuter); + } return res; } if (inner) { inner->push_back(wire); - if (reorient) + if (reorient) { doReorient(inner->back(), orientInner); + } } } return TopoShape(); diff --git a/tests/src/Mod/Part/App/PartTestHelpers.cpp b/tests/src/Mod/Part/App/PartTestHelpers.cpp index 02a3d33d04..24d106f9f8 100644 --- a/tests/src/Mod/Part/App/PartTestHelpers.cpp +++ b/tests/src/Mod/Part/App/PartTestHelpers.cpp @@ -14,6 +14,21 @@ double getVolume(const TopoDS_Shape& shape) return prop.Mass(); } +double getArea(const TopoDS_Shape& shape) +{ + GProp_GProps prop; + BRepGProp::SurfaceProperties(shape, prop); + return prop.Mass(); +} + +double getLength(const TopoDS_Shape& shape) +{ + GProp_GProps prop; + BRepGProp::LinearProperties(shape, prop); + return prop.Mass(); +} + + void PartTestHelperClass::createTestDoc() { _docName = App::GetApplication().getUniqueDocumentName("test"); @@ -48,7 +63,8 @@ _getFilletEdges(const std::vector& edges, double startRadius, double endRad return filletElements; } -void executePython(const std::vector& python) + +void ExecutePython(const std::vector& python) { Base::InterpreterSingleton is = Base::InterpreterSingleton(); @@ -68,16 +84,9 @@ void rectangle(double height, double width, char* name) boost::str(boost::format("V4 = FreeCAD.Vector(0, %d, 0)") % width), "P1 = Part.makePolygon([V1, V2, V3, V4],True)", "F1 = Part.Face(P1)", // Make the face or the volume calc won't work right. - // "L1 = Part.LineSegment(V1, V2)", - // "L2 = Part.LineSegment(V2, V3)", - // "L3 = Part.LineSegment(V3, V4)", - // "L4 = Part.LineSegment(V4, V1)", - // "S1 = Part.Shape([L1,L2,L3,L4])", - // "W1 = Part.Wire(S1.Edges)", - // "F1 = Part.Face(W1)", // Make the face or the volume calc won't work right. boost::str(boost::format("Part.show(F1,'%s')") % name), }; - executePython(rectstring); + ExecutePython(rectstring); } testing::AssertionResult diff --git a/tests/src/Mod/Part/App/PartTestHelpers.h b/tests/src/Mod/Part/App/PartTestHelpers.h index d651951c2f..742eaf26bc 100644 --- a/tests/src/Mod/Part/App/PartTestHelpers.h +++ b/tests/src/Mod/Part/App/PartTestHelpers.h @@ -1,21 +1,25 @@ // SPDX-License-Identifier: LGPL-2.1-or-later #include "gtest/gtest.h" +#include #include #include +#include "Base/Interpreter.h" #include #include "Mod/Part/App/FeaturePartBox.h" #include "Mod/Part/App/FeaturePartFuse.h" #include "Mod/Part/App/FeatureFillet.h" #include -#include "Base/Interpreter.h" -#include namespace PartTestHelpers { double getVolume(const TopoDS_Shape& shape); +double getArea(const TopoDS_Shape& shape); + +double getLength(const TopoDS_Shape& shape); + std::vector _getFilletEdges(const std::vector& edges, double startRadius, double endRadius); diff --git a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp index 964f346f88..bb793a326b 100644 --- a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -3,12 +3,18 @@ #include "gtest/gtest.h" #include "src/App/InitApplication.h" #include +#include + +#include "PartTestHelpers.h" -#include #include +#include +#include #include +#include +#include +#include #include -#include // NOLINTBEGIN(readability-magic-numbers,cppcoreguidelines-avoid-magic-numbers) @@ -34,6 +40,7 @@ protected: App::GetApplication().closeDocument(_docName.c_str()); } + private: std::string _docName; Data::ElementIDRefs _sid; @@ -161,4 +168,272 @@ TEST_F(TopoShapeExpansionTest, makeElementCompoundTwoCubes) // 26 subshapes each } +std::tuple +CreateRectFace(float len = 2.0, float wid = 3.0) +{ + auto edge1 = BRepBuilderAPI_MakeEdge(gp_Pnt(0.0, 0.0, 0.0), gp_Pnt(len, 0.0, 0.0)).Edge(); + auto edge2 = BRepBuilderAPI_MakeEdge(gp_Pnt(len, 0.0, 0.0), gp_Pnt(len, wid, 0.0)).Edge(); + auto edge3 = BRepBuilderAPI_MakeEdge(gp_Pnt(len, wid, 0.0), gp_Pnt(0.0, wid, 0.0)).Edge(); + auto edge4 = BRepBuilderAPI_MakeEdge(gp_Pnt(0.0, wid, 0.0), gp_Pnt(0.0, 0.0, 0.0)).Edge(); + auto wire1 = BRepBuilderAPI_MakeWire({edge1, edge2, edge3, edge4}).Wire(); + auto face1 = BRepBuilderAPI_MakeFace(wire1).Face(); + return {face1, wire1, edge1, edge2, edge3, edge4}; +} + +std::tuple +CreateFaceWithRoundHole(float len = 2.0, float wid = 3.0, float radius = 1.0) +{ + auto [face1, wire1, edge1, edge2, edge3, edge4] = CreateRectFace(len, wid); + auto circ1 = + GC_MakeCircle(gp_Pnt(len / 2.0, wid / 2.0, 0), gp_Dir(0.0, 0.0, 1.0), radius).Value(); + auto edge5 = BRepBuilderAPI_MakeEdge(circ1).Edge(); + auto wire2 = BRepBuilderAPI_MakeWire(edge5).Wire(); + auto face2 = BRepBuilderAPI_MakeFace(face1, wire2).Face(); + return {face2, wire1, wire2}; +} + +TEST_F(TopoShapeExpansionTest, makeElementFaceNull) +{ + // Arrange + const double L = 3, W = 2, R = 1; + auto [face1, wire1, wire2] = CreateFaceWithRoundHole(L, W, R); + Part::TopoShape topoShape {face1}; + double area = PartTestHelpers::getArea(face1); + double area1 = PartTestHelpers::getArea(topoShape.getShape()); + // Act + Part::TopoShape newFace = topoShape.makeElementFace(nullptr); + double area2 = PartTestHelpers::getArea(newFace.getShape()); + double area3 = PartTestHelpers::getArea(topoShape.getShape()); + // Assert + EXPECT_FALSE(face1.IsEqual(newFace.getShape())); + EXPECT_FLOAT_EQ(area, L * W + M_PI * R * R); + EXPECT_FLOAT_EQ(area1, L * W + M_PI * R * R); + EXPECT_FLOAT_EQ(area2, L * W - M_PI * R * R); + EXPECT_FLOAT_EQ(area3, L * W + M_PI * R * R); + EXPECT_STREQ(newFace.shapeName().c_str(), "Face"); +} + +TEST_F(TopoShapeExpansionTest, makeElementFaceSimple) +{ + // Arrange + const double L = 3, W = 2, R = 1; + auto [face1, wire1, wire2] = CreateFaceWithRoundHole(L, W, R); + Part::TopoShape topoShape {face1}; + double area = PartTestHelpers::getArea(face1); + double area1 = PartTestHelpers::getArea(topoShape.getShape()); + // Act + Part::TopoShape newFace = topoShape.makeElementFace(wire1); + double area2 = PartTestHelpers::getArea(newFace.getShape()); + double area3 = PartTestHelpers::getArea(topoShape.getShape()); + // Assert + EXPECT_TRUE(newFace.getShape().IsEqual(topoShape.getShape())); // topoShape was altered + EXPECT_FALSE(face1.IsEqual(newFace.getShape())); + EXPECT_FLOAT_EQ(area, L * W + M_PI * R * R); + EXPECT_FLOAT_EQ(area1, L * W + M_PI * R * R); + EXPECT_FLOAT_EQ(area2, L * W); + EXPECT_FLOAT_EQ(area3, L * W); + EXPECT_STREQ(newFace.shapeName().c_str(), "Face"); +} + +TEST_F(TopoShapeExpansionTest, makeElementFaceParams) +{ + // Arrange + const double L = 3, W = 2, R = 1; + auto [face1, wire1, wire2] = CreateFaceWithRoundHole(L, W, R); + Part::TopoShape topoShape {face1, 1L}; + double area = PartTestHelpers::getArea(face1); + double area1 = PartTestHelpers::getArea(topoShape.getShape()); + // Act + Part::TopoShape newFace = + topoShape.makeElementFace(wire1, "Cut", "Part::FaceMakerBullseye", nullptr); + double area2 = PartTestHelpers::getArea(newFace.getShape()); + double area3 = PartTestHelpers::getArea(topoShape.getShape()); + // Assert + EXPECT_TRUE(newFace.getShape().IsEqual(topoShape.getShape())); // topoShape was altered + EXPECT_FALSE(face1.IsEqual(newFace.getShape())); + EXPECT_FLOAT_EQ(area, L * W + M_PI * R * R); + EXPECT_FLOAT_EQ(area1, L * W + M_PI * R * R); + EXPECT_FLOAT_EQ(area2, L * W); + EXPECT_FLOAT_EQ(area3, L * W); + EXPECT_STREQ(newFace.shapeName().c_str(), "Face"); +} + +TEST_F(TopoShapeExpansionTest, makeElementFaceFromFace) +{ + // Arrange + const double L = 3, W = 2, R = 1; + auto [face1, wire1, wire2] = CreateFaceWithRoundHole(L, W, R); + Part::TopoShape topoShape {face1, 1L}; + double area = PartTestHelpers::getArea(face1); + double area1 = PartTestHelpers::getArea(topoShape.getShape()); + // Act + Part::TopoShape newFace = + topoShape.makeElementFace(face1, "Cut", "Part::FaceMakerBullseye", nullptr); + double area2 = PartTestHelpers::getArea(newFace.getShape()); + double area3 = PartTestHelpers::getArea(topoShape.getShape()); + // Assert + EXPECT_TRUE(newFace.getShape().IsEqual(topoShape.getShape())); // topoShape was altered + EXPECT_FALSE(face1.IsEqual(newFace.getShape())); + EXPECT_FLOAT_EQ(area, L * W + M_PI * R * R); + EXPECT_FLOAT_EQ(area1, L * W + M_PI * R * R); + EXPECT_FLOAT_EQ(area2, L * W - M_PI * R * R); + EXPECT_FLOAT_EQ(area3, L * W - M_PI * R * R); + EXPECT_STREQ(newFace.shapeName().c_str(), "Face"); +} + + +TEST_F(TopoShapeExpansionTest, makeElementFaceOpenWire) +{ + // Arrange + const double L = 3, W = 2, R = 1; + auto [face1, wire1, wire2] = CreateFaceWithRoundHole(L, W, R); + Part::TopoShape topoShape {wire1, 1L}; + double area = PartTestHelpers::getArea(face1); + double area1 = PartTestHelpers::getArea(topoShape.getShape()); + // Act + Part::TopoShape newFace = topoShape.makeElementFace(wire1, "Cut", nullptr, nullptr); + double area2 = PartTestHelpers::getArea(newFace.getShape()); + double area3 = PartTestHelpers::getArea(topoShape.getShape()); + // Assert + EXPECT_TRUE(newFace.getShape().IsEqual(topoShape.getShape())); // topoShape was altered + EXPECT_FALSE(face1.IsEqual(newFace.getShape())); + EXPECT_FLOAT_EQ(area, L * W + M_PI * R * R); + EXPECT_FLOAT_EQ(area1, 0); // L * W - M_PI * R * R); + EXPECT_FLOAT_EQ(area2, L * W); + EXPECT_FLOAT_EQ(area3, L * W); + EXPECT_STREQ(newFace.shapeName().c_str(), "Face"); +} + + +TEST_F(TopoShapeExpansionTest, makeElementFaceClosedWire) +{ + // Arrange + const double L = 3, W = 2, R = 1; + auto [face1, wire1, wire2] = CreateFaceWithRoundHole(L, W, R); + Part::TopoShape topoShape {wire2, 1L}; + double area = PartTestHelpers::getArea(face1); + double area1 = PartTestHelpers::getArea(topoShape.getShape()); + // Act + Part::TopoShape newFace = + topoShape.makeElementFace(wire2, "Cut", "Part::FaceMakerBullseye", nullptr); + double area2 = PartTestHelpers::getArea(newFace.getShape()); + double area3 = PartTestHelpers::getArea(topoShape.getShape()); + // Assert + EXPECT_TRUE(newFace.getShape().IsEqual(topoShape.getShape())); // topoShape was altered + EXPECT_FALSE(face1.IsEqual(newFace.getShape())); + EXPECT_FLOAT_EQ(area, L * W + M_PI * R * R); + EXPECT_FLOAT_EQ(area1, 0); // L * W - M_PI * R * R); + EXPECT_FLOAT_EQ(area2, M_PI * R * R); + EXPECT_FLOAT_EQ(area3, M_PI * R * R); + EXPECT_STREQ(newFace.shapeName().c_str(), "Face"); +} + +// Possible future makeElementFace tests: +// Overlapping wire +// Compound of wires +// Compound of faces +// Compound of other shape types + + +TEST_F(TopoShapeExpansionTest, setElementComboNameNothing) +{ + // Arrange + Part::TopoShape topoShape(1L); + // Act + Data::MappedName result = topoShape.setElementComboName(Data::IndexedName(), {}); + // ASSERT + EXPECT_STREQ(result.toString().c_str(), ""); +} + + +TEST_F(TopoShapeExpansionTest, setElementComboNameSimple) +{ + // Arrange + auto edge1 = BRepBuilderAPI_MakeEdge(gp_Pnt(0.0, 0.0, 0.0), gp_Pnt(1.0, 0.0, 0.0)).Edge(); + Part::TopoShape topoShape(edge1, 1L); + topoShape.setElementMap({}); // Initialize the map to avoid a segfault. + // Also, maybe the end of TopoShape::mapSubElementTypeForShape should enforce that elementMap() + // isn't nullptr to eliminate the segfault. + Data::MappedName edgeName("testname"); + // Act + Data::MappedName result = + topoShape.setElementComboName(Data::IndexedName::fromConst("Edge", 1), {edgeName}); + // Assert + EXPECT_STREQ(result.toString().c_str(), "testname;"); +} + + +TEST_F(TopoShapeExpansionTest, setElementComboName) +{ + // Arrange + Part::TopoShape topoShape(2L); + topoShape.setElementMap({}); + Data::MappedName edgeName = + topoShape.getMappedName(Data::IndexedName::fromConst("Edge", 1), true); + Data::MappedName faceName = + topoShape.getMappedName(Data::IndexedName::fromConst("Face", 7), true); + Data::MappedName faceName2 = + topoShape.getMappedName(Data::IndexedName::fromConst("Face", 8), true); + char* op = "Copy"; + // Act + Data::MappedName result = topoShape.setElementComboName(Data::IndexedName::fromConst("Edge", 1), + {edgeName, faceName, faceName2}, + Part::OpCodes::Common, + op); + // Assert + EXPECT_STREQ(result.toString().c_str(), "Edge1;CMN(Face7|Face8);Copy"); + // The detailed forms of names are covered in encodeElementName tests +} + +TEST_F(TopoShapeExpansionTest, setElementComboNameCompound) +{ + // Arrange + auto edge1 = BRepBuilderAPI_MakeEdge(gp_Pnt(0.0, 0.0, 0.0), gp_Pnt(1.0, 0.0, 0.0)).Edge(); + auto wire1 = BRepBuilderAPI_MakeWire({edge1}).Wire(); + auto wire2 = BRepBuilderAPI_MakeWire({edge1}).Wire(); + Part::TopoShape topoShape(2L); + topoShape.makeElementCompound({wire1, wire2}); // Quality of shape doesn't matter + Data::MappedName edgeName = + topoShape.getMappedName(Data::IndexedName::fromConst("Edge", 1), true); + Data::MappedName faceName = + topoShape.getMappedName(Data::IndexedName::fromConst("Face", 7), true); + Data::MappedName faceName2 = + topoShape.getMappedName(Data::IndexedName::fromConst("Face", 8), true); + char* op = "Copy"; + // Act + Data::MappedName result = topoShape.setElementComboName(Data::IndexedName::fromConst("Edge", 1), + {edgeName, faceName, faceName2}, + Part::OpCodes::Common, + op); + // ASSERT + EXPECT_STREQ(result.toString().c_str(), "Edge1;:H,E;CMN(Face7|Face8);Copy"); + // The detailed forms of names are covered in encodeElementName tests +} + +TEST_F(TopoShapeExpansionTest, splitWires) +{ + // Arrange + const double L = 3, W = 2, R = 1; + auto [face1, wire1, wire2] = CreateFaceWithRoundHole(L, W, R); + Part::TopoShape topoShape {face1, 1L}; + std::vector inner; + // Act + EXPECT_EQ(topoShape.getShape().Orientation(), TopAbs_FORWARD); + Part::TopoShape wire = + topoShape.splitWires(&inner, Part::TopoShape::SplitWireReorient::ReorientReversed); + // Assert + EXPECT_EQ(inner.size(), 1); + EXPECT_FLOAT_EQ(PartTestHelpers::getLength(wire.getShape()), 2 + 2 + 3 + 3); + EXPECT_FLOAT_EQ(PartTestHelpers::getLength(inner.front().getShape()), M_PI * R * 2); + EXPECT_EQ(wire.getShape().Orientation(), TopAbs_REVERSED); + for (Part::TopoShape ts : inner) { + EXPECT_EQ(ts.getShape().Orientation(), TopAbs_FORWARD); + } +} + +// Possible future tests: +// splitWires without inner Wires +// splitWires with allfour reorientation values NoReorient, ReOrient, ReorientForward, +// ReorientRevesed + // NOLINTEND(readability-magic-numbers,cppcoreguidelines-avoid-magic-numbers) From 1490de0087c52b8300280c0a2ea932184db72879 Mon Sep 17 00:00:00 2001 From: bgbsww Date: Sun, 21 Jan 2024 16:06:32 -0500 Subject: [PATCH 3/6] apply Comp to Comparator name change --- src/Mod/Part/App/FaceMaker.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Mod/Part/App/FaceMaker.cpp b/src/Mod/Part/App/FaceMaker.cpp index fcdef3537f..21bd1d789d 100644 --- a/src/Mod/Part/App/FaceMaker.cpp +++ b/src/Mod/Part/App/FaceMaker.cpp @@ -173,7 +173,7 @@ struct ElementName { return true; if(tag>other.tag) return false; - return Data::ElementNameComp()(name,other.name); + return Data::ElementNameComparator()(name,other.name); } }; From 31a6eb5a4a00a12746ee16650d66c098661887ee Mon Sep 17 00:00:00 2001 From: bgbsww Date: Mon, 22 Jan 2024 11:01:19 -0500 Subject: [PATCH 4/6] lint / review cleanups --- src/Mod/Part/App/FaceMaker.cpp | 7 ---- src/Mod/Part/App/FaceMaker.h | 2 +- src/Mod/Part/App/TopoShape.h | 38 ++++++++++--------- src/Mod/Part/App/TopoShapeExpansion.cpp | 20 +++++----- tests/src/Mod/Part/App/TopoShapeExpansion.cpp | 4 +- 5 files changed, 33 insertions(+), 38 deletions(-) diff --git a/src/Mod/Part/App/FaceMaker.cpp b/src/Mod/Part/App/FaceMaker.cpp index 21bd1d789d..5db6f6c640 100644 --- a/src/Mod/Part/App/FaceMaker.cpp +++ b/src/Mod/Part/App/FaceMaker.cpp @@ -206,17 +206,10 @@ void Part::FaceMaker::postBuild() { std::vector names; Data::ElementIDRefs sids; -#if 0 - for (auto &e : edgeNames) { - names.insert(e.name); - sids += e.sids; - } -#else // We just use the first source element name to make the face name more // stable names.push_back(edgeNames.begin()->name); sids = edgeNames.begin()->sids; -#endif this->myTopoShape.setElementComboName( Data::IndexedName::fromConst("Face",i),names,op,nullptr,&sids); } diff --git a/src/Mod/Part/App/FaceMaker.h b/src/Mod/Part/App/FaceMaker.h index fc40e243a1..cd25ada21b 100644 --- a/src/Mod/Part/App/FaceMaker.h +++ b/src/Mod/Part/App/FaceMaker.h @@ -86,7 +86,7 @@ public: virtual const TopoDS_Face& Face(); #if OCC_VERSION_HEX >= 0x070600 - virtual void Build(const Message_ProgressRange& theRange = Message_ProgressRange()); + void Build(const Message_ProgressRange& theRange = Message_ProgressRange()) override; #else virtual void Build(); #endif diff --git a/src/Mod/Part/App/TopoShape.h b/src/Mod/Part/App/TopoShape.h index b5bf372a93..716237de47 100644 --- a/src/Mod/Part/App/TopoShape.h +++ b/src/Mod/Part/App/TopoShape.h @@ -655,11 +655,11 @@ public: */ TopoShape &makeElementCompound(const std::vector &shapes, const char *op=nullptr, bool force=true); - TopoShape &makeElementFace(const std::vector &shapes, - const char *op = nullptr, - const char *maker = nullptr, - const gp_Pln *pln = nullptr); - /** Make a planar face with the input wire or edge + TopoShape& makeElementFace(const std::vector& shapes, + const char* op = nullptr, + const char* maker = nullptr, + const gp_Pln* pln = nullptr); + /** Make a planar face with the input wire or edge * * @param shape: input shape. Can be either edge, wire, or compound of * those two types @@ -675,11 +675,11 @@ public: * multiple operations can be carried out for the same shape in the * same line of code. */ - TopoShape &makeElementFace(const TopoShape &shape, - const char *op = nullptr, - const char *maker = nullptr, - const gp_Pln *pln = nullptr); - /** Make a planar face using this shape + TopoShape& makeElementFace(const TopoShape& shape, + const char* op = nullptr, + const char* maker = nullptr, + const gp_Pln* pln = nullptr); + /** Make a planar face using this shape * * @param op: optional string to be encoded into topo naming for indicating * the operation @@ -690,20 +690,22 @@ public: * @return The function returns a new planar face made using the wire or edge * inside this shape. The shape itself is not modified. */ - TopoShape makeElementFace(const char *op = nullptr, - const char *maker = nullptr, - const gp_Pln *pln = nullptr) const { - return TopoShape(0,Hasher).makeElementFace(*this,op,maker,pln); + TopoShape makeElementFace(const char* op = nullptr, + const char* maker = nullptr, + const gp_Pln* pln = nullptr) const + { + return TopoShape(0, Hasher).makeElementFace(*this, op, maker, pln); } /// Filling style when making a BSpline face - enum FillingStyle { + enum class FillingStyle + { /// The style with the flattest patches - FillingStyle_Strech, + Stretch, /// A rounded style of patch with less depth than those of Curved - FillingStyle_Coons, + Coons, /// The style with the most rounded patches - FillingStyle_Curved, + Curved, }; diff --git a/src/Mod/Part/App/TopoShapeExpansion.cpp b/src/Mod/Part/App/TopoShapeExpansion.cpp index c59ada479b..d0109d4e72 100644 --- a/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -552,7 +552,7 @@ TopoShape::makeElementCompound(const std::vector& shapes, const char* TopoShape& TopoShape::makeElementFace(const TopoShape& shape, const char* op, const char* maker, - const gp_Pln* pln) + const gp_Pln* plane) { std::vector shapes; if (shape.isNull()) { @@ -564,13 +564,13 @@ TopoShape& TopoShape::makeElementFace(const TopoShape& shape, else { shapes.push_back(shape); } - return makeElementFace(shapes, op, maker, pln); + return makeElementFace(shapes, op, maker, plane); } TopoShape& TopoShape::makeElementFace(const std::vector& shapes, const char* op, const char* maker, - const gp_Pln* pln) + const gp_Pln* plane) { if (!maker || !maker[0]) { maker = "Part::FaceMakerBullseye"; @@ -578,16 +578,16 @@ TopoShape& TopoShape::makeElementFace(const std::vector& shapes, std::unique_ptr mkFace = FaceMaker::ConstructFromType(maker); mkFace->MyHasher = Hasher; mkFace->MyOp = op; - if (pln) { - mkFace->setPlane(*pln); + if (plane) { + mkFace->setPlane(*plane); } - for (auto& s : shapes) { - if (s.getShape().ShapeType() == TopAbs_COMPOUND) { - mkFace->useTopoCompound(s); + for (auto& shape : shapes) { + if (shape.getShape().ShapeType() == TopAbs_COMPOUND) { + mkFace->useTopoCompound(shape); } else { - mkFace->addTopoShape(s); + mkFace->addTopoShape(shape); } } mkFace->Build(); @@ -634,7 +634,7 @@ TopoShape& TopoShape::makeElementFace(const std::vector& shapes, * elementMapPrefix. * @param op The op text passed to the element name encoder along with the TopoShape Tag * @param _sids If defined, records the sub ids processed. - * + * * @return The encoded, possibly hashed name. */ Data::MappedName TopoShape::setElementComboName(const Data::IndexedName& element, diff --git a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp index bb793a326b..f534e4d02e 100644 --- a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -426,8 +426,8 @@ TEST_F(TopoShapeExpansionTest, splitWires) EXPECT_FLOAT_EQ(PartTestHelpers::getLength(wire.getShape()), 2 + 2 + 3 + 3); EXPECT_FLOAT_EQ(PartTestHelpers::getLength(inner.front().getShape()), M_PI * R * 2); EXPECT_EQ(wire.getShape().Orientation(), TopAbs_REVERSED); - for (Part::TopoShape ts : inner) { - EXPECT_EQ(ts.getShape().Orientation(), TopAbs_FORWARD); + for (Part::TopoShape& shape : inner) { + EXPECT_EQ(shape.getShape().Orientation(), TopAbs_FORWARD); } } From 96b4171c09ee6e612fc8491a1940237c565fa21a Mon Sep 17 00:00:00 2001 From: bgbsww Date: Mon, 22 Jan 2024 11:29:41 -0500 Subject: [PATCH 5/6] Single char constants --- tests/src/Mod/Part/App/TopoShapeExpansion.cpp | 78 +++++++++---------- 1 file changed, 39 insertions(+), 39 deletions(-) diff --git a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp index f534e4d02e..1255a91da6 100644 --- a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -195,8 +195,8 @@ CreateFaceWithRoundHole(float len = 2.0, float wid = 3.0, float radius = 1.0) TEST_F(TopoShapeExpansionTest, makeElementFaceNull) { // Arrange - const double L = 3, W = 2, R = 1; - auto [face1, wire1, wire2] = CreateFaceWithRoundHole(L, W, R); + const double Len = 3, Wid = 2, Rad = 1; + auto [face1, wire1, wire2] = CreateFaceWithRoundHole(Len, Wid, Rad); Part::TopoShape topoShape {face1}; double area = PartTestHelpers::getArea(face1); double area1 = PartTestHelpers::getArea(topoShape.getShape()); @@ -206,18 +206,18 @@ TEST_F(TopoShapeExpansionTest, makeElementFaceNull) double area3 = PartTestHelpers::getArea(topoShape.getShape()); // Assert EXPECT_FALSE(face1.IsEqual(newFace.getShape())); - EXPECT_FLOAT_EQ(area, L * W + M_PI * R * R); - EXPECT_FLOAT_EQ(area1, L * W + M_PI * R * R); - EXPECT_FLOAT_EQ(area2, L * W - M_PI * R * R); - EXPECT_FLOAT_EQ(area3, L * W + M_PI * R * R); + EXPECT_FLOAT_EQ(area, Len * Wid + M_PI * Rad * Rad); + EXPECT_FLOAT_EQ(area1, Len * Wid + M_PI * Rad * Rad); + EXPECT_FLOAT_EQ(area2, Len * Wid - M_PI * Rad * Rad); + EXPECT_FLOAT_EQ(area3, Len * Wid + M_PI * Rad * Rad); EXPECT_STREQ(newFace.shapeName().c_str(), "Face"); } TEST_F(TopoShapeExpansionTest, makeElementFaceSimple) { // Arrange - const double L = 3, W = 2, R = 1; - auto [face1, wire1, wire2] = CreateFaceWithRoundHole(L, W, R); + const double Len = 3, Wid = 2, Rad = 1; + auto [face1, wire1, wire2] = CreateFaceWithRoundHole(Len, Wid, Rad); Part::TopoShape topoShape {face1}; double area = PartTestHelpers::getArea(face1); double area1 = PartTestHelpers::getArea(topoShape.getShape()); @@ -228,18 +228,18 @@ TEST_F(TopoShapeExpansionTest, makeElementFaceSimple) // Assert EXPECT_TRUE(newFace.getShape().IsEqual(topoShape.getShape())); // topoShape was altered EXPECT_FALSE(face1.IsEqual(newFace.getShape())); - EXPECT_FLOAT_EQ(area, L * W + M_PI * R * R); - EXPECT_FLOAT_EQ(area1, L * W + M_PI * R * R); - EXPECT_FLOAT_EQ(area2, L * W); - EXPECT_FLOAT_EQ(area3, L * W); + EXPECT_FLOAT_EQ(area, Len * Wid + M_PI * Rad * Rad); + EXPECT_FLOAT_EQ(area1, Len * Wid + M_PI * Rad * Rad); + EXPECT_FLOAT_EQ(area2, Len * Wid); + EXPECT_FLOAT_EQ(area3, Len * Wid); EXPECT_STREQ(newFace.shapeName().c_str(), "Face"); } TEST_F(TopoShapeExpansionTest, makeElementFaceParams) { // Arrange - const double L = 3, W = 2, R = 1; - auto [face1, wire1, wire2] = CreateFaceWithRoundHole(L, W, R); + const double Len = 3, Wid = 2, Rad = 1; + auto [face1, wire1, wire2] = CreateFaceWithRoundHole(Len, Wid, Rad); Part::TopoShape topoShape {face1, 1L}; double area = PartTestHelpers::getArea(face1); double area1 = PartTestHelpers::getArea(topoShape.getShape()); @@ -251,18 +251,18 @@ TEST_F(TopoShapeExpansionTest, makeElementFaceParams) // Assert EXPECT_TRUE(newFace.getShape().IsEqual(topoShape.getShape())); // topoShape was altered EXPECT_FALSE(face1.IsEqual(newFace.getShape())); - EXPECT_FLOAT_EQ(area, L * W + M_PI * R * R); - EXPECT_FLOAT_EQ(area1, L * W + M_PI * R * R); - EXPECT_FLOAT_EQ(area2, L * W); - EXPECT_FLOAT_EQ(area3, L * W); + EXPECT_FLOAT_EQ(area, Len * Wid + M_PI * Rad * Rad); + EXPECT_FLOAT_EQ(area1, Len * Wid + M_PI * Rad * Rad); + EXPECT_FLOAT_EQ(area2, Len * Wid); + EXPECT_FLOAT_EQ(area3, Len * Wid); EXPECT_STREQ(newFace.shapeName().c_str(), "Face"); } TEST_F(TopoShapeExpansionTest, makeElementFaceFromFace) { // Arrange - const double L = 3, W = 2, R = 1; - auto [face1, wire1, wire2] = CreateFaceWithRoundHole(L, W, R); + const double Len = 3, Wid = 2, Rad = 1; + auto [face1, wire1, wire2] = CreateFaceWithRoundHole(Len, Wid, Rad); Part::TopoShape topoShape {face1, 1L}; double area = PartTestHelpers::getArea(face1); double area1 = PartTestHelpers::getArea(topoShape.getShape()); @@ -274,10 +274,10 @@ TEST_F(TopoShapeExpansionTest, makeElementFaceFromFace) // Assert EXPECT_TRUE(newFace.getShape().IsEqual(topoShape.getShape())); // topoShape was altered EXPECT_FALSE(face1.IsEqual(newFace.getShape())); - EXPECT_FLOAT_EQ(area, L * W + M_PI * R * R); - EXPECT_FLOAT_EQ(area1, L * W + M_PI * R * R); - EXPECT_FLOAT_EQ(area2, L * W - M_PI * R * R); - EXPECT_FLOAT_EQ(area3, L * W - M_PI * R * R); + EXPECT_FLOAT_EQ(area, Len * Wid + M_PI * Rad * Rad); + EXPECT_FLOAT_EQ(area1, Len * Wid + M_PI * Rad * Rad); + EXPECT_FLOAT_EQ(area2, Len * Wid - M_PI * Rad * Rad); + EXPECT_FLOAT_EQ(area3, Len * Wid - M_PI * Rad * Rad); EXPECT_STREQ(newFace.shapeName().c_str(), "Face"); } @@ -285,8 +285,8 @@ TEST_F(TopoShapeExpansionTest, makeElementFaceFromFace) TEST_F(TopoShapeExpansionTest, makeElementFaceOpenWire) { // Arrange - const double L = 3, W = 2, R = 1; - auto [face1, wire1, wire2] = CreateFaceWithRoundHole(L, W, R); + const double Len = 3, Wid = 2, Rad = 1; + auto [face1, wire1, wire2] = CreateFaceWithRoundHole(Len, Wid, Rad); Part::TopoShape topoShape {wire1, 1L}; double area = PartTestHelpers::getArea(face1); double area1 = PartTestHelpers::getArea(topoShape.getShape()); @@ -297,10 +297,10 @@ TEST_F(TopoShapeExpansionTest, makeElementFaceOpenWire) // Assert EXPECT_TRUE(newFace.getShape().IsEqual(topoShape.getShape())); // topoShape was altered EXPECT_FALSE(face1.IsEqual(newFace.getShape())); - EXPECT_FLOAT_EQ(area, L * W + M_PI * R * R); - EXPECT_FLOAT_EQ(area1, 0); // L * W - M_PI * R * R); - EXPECT_FLOAT_EQ(area2, L * W); - EXPECT_FLOAT_EQ(area3, L * W); + EXPECT_FLOAT_EQ(area, Len * Wid + M_PI * Rad * Rad); + EXPECT_FLOAT_EQ(area1, 0); // Len * Wid - M_PI * Rad * Rad); + EXPECT_FLOAT_EQ(area2, Len * Wid); + EXPECT_FLOAT_EQ(area3, Len * Wid); EXPECT_STREQ(newFace.shapeName().c_str(), "Face"); } @@ -308,8 +308,8 @@ TEST_F(TopoShapeExpansionTest, makeElementFaceOpenWire) TEST_F(TopoShapeExpansionTest, makeElementFaceClosedWire) { // Arrange - const double L = 3, W = 2, R = 1; - auto [face1, wire1, wire2] = CreateFaceWithRoundHole(L, W, R); + const double Len = 3, Wid = 2, Rad = 1; + auto [face1, wire1, wire2] = CreateFaceWithRoundHole(Len, Wid, Rad); Part::TopoShape topoShape {wire2, 1L}; double area = PartTestHelpers::getArea(face1); double area1 = PartTestHelpers::getArea(topoShape.getShape()); @@ -321,10 +321,10 @@ TEST_F(TopoShapeExpansionTest, makeElementFaceClosedWire) // Assert EXPECT_TRUE(newFace.getShape().IsEqual(topoShape.getShape())); // topoShape was altered EXPECT_FALSE(face1.IsEqual(newFace.getShape())); - EXPECT_FLOAT_EQ(area, L * W + M_PI * R * R); - EXPECT_FLOAT_EQ(area1, 0); // L * W - M_PI * R * R); - EXPECT_FLOAT_EQ(area2, M_PI * R * R); - EXPECT_FLOAT_EQ(area3, M_PI * R * R); + EXPECT_FLOAT_EQ(area, Len * Wid + M_PI * Rad * Rad); + EXPECT_FLOAT_EQ(area1, 0); // Len * Wid - M_PI * Rad * Rad); + EXPECT_FLOAT_EQ(area2, M_PI * Rad * Rad); + EXPECT_FLOAT_EQ(area3, M_PI * Rad * Rad); EXPECT_STREQ(newFace.shapeName().c_str(), "Face"); } @@ -413,8 +413,8 @@ TEST_F(TopoShapeExpansionTest, setElementComboNameCompound) TEST_F(TopoShapeExpansionTest, splitWires) { // Arrange - const double L = 3, W = 2, R = 1; - auto [face1, wire1, wire2] = CreateFaceWithRoundHole(L, W, R); + const double Len = 3, Wid = 2, Rad = 1; + auto [face1, wire1, wire2] = CreateFaceWithRoundHole(Len, Wid, Rad); Part::TopoShape topoShape {face1, 1L}; std::vector inner; // Act @@ -424,7 +424,7 @@ TEST_F(TopoShapeExpansionTest, splitWires) // Assert EXPECT_EQ(inner.size(), 1); EXPECT_FLOAT_EQ(PartTestHelpers::getLength(wire.getShape()), 2 + 2 + 3 + 3); - EXPECT_FLOAT_EQ(PartTestHelpers::getLength(inner.front().getShape()), M_PI * R * 2); + EXPECT_FLOAT_EQ(PartTestHelpers::getLength(inner.front().getShape()), M_PI * Rad * 2); EXPECT_EQ(wire.getShape().Orientation(), TopAbs_REVERSED); for (Part::TopoShape& shape : inner) { EXPECT_EQ(shape.getShape().Orientation(), TopAbs_FORWARD); From 6ec676799d28fc7dde3269d2fc47cf31439def68 Mon Sep 17 00:00:00 2001 From: bgbsww Date: Mon, 22 Jan 2024 16:13:53 -0500 Subject: [PATCH 6/6] Small cleanups --- src/Mod/Part/App/FaceMaker.cpp | 20 +++++++++++--------- src/Mod/Part/App/TopoShape.h | 14 +++++++------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/Mod/Part/App/FaceMaker.cpp b/src/Mod/Part/App/FaceMaker.cpp index 5db6f6c640..44fa2ce2cd 100644 --- a/src/Mod/Part/App/FaceMaker.cpp +++ b/src/Mod/Part/App/FaceMaker.cpp @@ -181,28 +181,30 @@ void Part::FaceMaker::postBuild() { this->myTopoShape.setShape(this->myShape); this->myTopoShape.Hasher = this->MyHasher; this->myTopoShape.mapSubElement(this->mySourceShapes); - int i = 0; + int index = 0; const char *op = this->MyOp; if(!op) op = Part::OpCodes::Face; const auto &faces = this->myTopoShape.getSubTopoShapes(TopAbs_FACE); // name the face using the edges of its outer wire for(auto &face : faces) { - ++i; + ++index; TopoShape wire = face.splitWires(); wire.mapSubElement(face); std::set edgeNames; int count = wire.countSubShapes(TopAbs_EDGE); - for(int i=1;i<=count;++i) { + for (int index2 = 1; index2 <= count; ++index2) { Data::ElementIDRefs sids; - Data::MappedName name = face.getMappedName( - Data::IndexedName::fromConst("Edge",i), false, &sids); - if(!name) + Data::MappedName name = + face.getMappedName(Data::IndexedName::fromConst("Edge", index2), false, &sids); + if (!name) { continue; - edgeNames.emplace(wire.getElementHistory(name),name,sids); + } + edgeNames.emplace(wire.getElementHistory(name), name, sids); } - if(edgeNames.empty()) + if (edgeNames.empty()) { continue; + } std::vector names; Data::ElementIDRefs sids; @@ -211,7 +213,7 @@ void Part::FaceMaker::postBuild() { names.push_back(edgeNames.begin()->name); sids = edgeNames.begin()->sids; this->myTopoShape.setElementComboName( - Data::IndexedName::fromConst("Face",i),names,op,nullptr,&sids); + Data::IndexedName::fromConst("Face",index),names,op,nullptr,&sids); } this->myTopoShape.initCache(true); this->Done(); diff --git a/src/Mod/Part/App/TopoShape.h b/src/Mod/Part/App/TopoShape.h index 28cc8d0e7f..06439746a9 100644 --- a/src/Mod/Part/App/TopoShape.h +++ b/src/Mod/Part/App/TopoShape.h @@ -257,7 +257,7 @@ public: bool analyze(bool runBopCheck, std::ostream&) const; bool isClosed() const; bool isCoplanar(const TopoShape& other, double tol = -1) const; - bool findPlane(gp_Pln& pln, double tol = -1) const; + bool findPlane(gp_Pln& plane, double tol = -1) const; /// Returns true if the expansion of the shape is infinite, false otherwise bool isInfinite() const; /// Checks whether the shape is a planar face @@ -679,7 +679,7 @@ public: TopoShape& makeElementFace(const std::vector& shapes, const char* op = nullptr, const char* maker = nullptr, - const gp_Pln* pln = nullptr); + const gp_Pln* plane = nullptr); /** Make a planar face with the input wire or edge * * @param shape: input shape. Can be either edge, wire, or compound of @@ -688,7 +688,7 @@ public: * the operation * @param maker: optional type name of the face maker. If not given, * default to "Part::FaceMakerBullseye" - * @param pln: optional plane of the face. + * @param plane: optional plane of the face. * * @return The function creates a planar face. The original content of this * TopoShape is discarded and replaced with the new shape. The @@ -699,23 +699,23 @@ public: TopoShape& makeElementFace(const TopoShape& shape, const char* op = nullptr, const char* maker = nullptr, - const gp_Pln* pln = nullptr); + const gp_Pln* plane = nullptr); /** Make a planar face using this shape * * @param op: optional string to be encoded into topo naming for indicating * the operation * @param maker: optional type name of the face maker. If not given, * default to "Part::FaceMakerBullseye" - * @param pln: optional plane of the face. + * @param plane: optional plane of the face. * * @return The function returns a new planar face made using the wire or edge * inside this shape. The shape itself is not modified. */ TopoShape makeElementFace(const char* op = nullptr, const char* maker = nullptr, - const gp_Pln* pln = nullptr) const + const gp_Pln* plane = nullptr) const { - return TopoShape(0, Hasher).makeElementFace(*this, op, maker, pln); + return TopoShape(0, Hasher).makeElementFace(*this, op, maker, plane); } /// Filling style when making a BSpline face