diff --git a/src/Mod/Part/App/AppPart.cpp b/src/Mod/Part/App/AppPart.cpp index b66e3c60ca..6c99f8177f 100644 --- a/src/Mod/Part/App/AppPart.cpp +++ b/src/Mod/Part/App/AppPart.cpp @@ -425,6 +425,7 @@ PyMOD_INIT_FUNC(Part) Part::FaceMakerCheese ::init(); Part::FaceMakerExtrusion ::init(); Part::FaceMakerBullseye ::init(); + Part::FaceMakerRing ::init(); Attacher::AttachEngine ::init(); Attacher::AttachEngine3D ::init(); diff --git a/src/Mod/Part/App/FaceMakerBullseye.cpp b/src/Mod/Part/App/FaceMakerBullseye.cpp index 72ac281225..3c65816096 100644 --- a/src/Mod/Part/App/FaceMakerBullseye.cpp +++ b/src/Mod/Part/App/FaceMakerBullseye.cpp @@ -25,6 +25,7 @@ # include # include # include +# include # include # include # include @@ -36,18 +37,21 @@ # include # include # include +# include #endif #include "FaceMakerBullseye.h" #include "FaceMakerCheese.h" + #include "TopoShape.h" +#include "WireJoiner.h" using namespace Part; TYPESYSTEM_SOURCE(Part::FaceMakerBullseye, Part::FaceMakerPublic) -void FaceMakerBullseye::setPlane(const gp_Pln &plane) +void FaceMakerBullseye::setPlane(const gp_Pln& plane) { this->myPlane = gp_Pln(plane); this->planeSupplied = true; @@ -63,19 +67,26 @@ std::string FaceMakerBullseye::getBriefExplanation() const return {tr("Supports making planar faces with holes with islands.").toStdString()}; } +bool FaceMakerBullseye::WireInfo::operator<(const WireInfo& other) const +{ + return extent - other.extent > Precision::Confusion(); +} + void FaceMakerBullseye::Build_Essence() { - if (myWires.empty()) + if (myWires.empty()) { return; + } - //validity check + // validity check for (TopoDS_Wire& w : myWires) { - if (!BRep_Tool::IsClosed(w)) - throw Base::ValueError(QT_TRANSLATE_NOOP("Exception", "Wire is not closed.")); + if (!BRep_Tool::IsClosed(w)) { + throw Base::ValueError("Wire is not closed."); + } } - //find plane (at the same time, test that all wires are on the same plane) + // find plane (at the same time, test that all wires are on the same plane) gp_Pln plane; if (this->planeSupplied) { plane = this->myPlane; @@ -88,48 +99,85 @@ void FaceMakerBullseye::Build_Essence() builder.Add(comp, BRepBuilderAPI_Copy(w).Shape()); } BRepLib_FindSurface planeFinder(comp, -1, /*OnlyPlane=*/Standard_True); - if (!planeFinder.Found()) + if (!planeFinder.Found()) { throw Base::ValueError("Wires are not coplanar."); + } plane = GeomAdaptor_Surface(planeFinder.Surface()).Plane(); } - //sort wires by length of diagonal of bounding box. - std::vector wires = this->myWires; - std::stable_sort(wires.begin(), wires.end(), FaceMakerCheese::Wire_Compare()); + std::vector wireInfos; + for (const auto& w : this->myTopoWires) { + Bnd_Box box; + if (w.isNull()) { + continue; + } + BRepBndLib::AddOptimal(w.getShape(), box, Standard_False); + if (box.IsVoid()) { + continue; + } + wireInfos.emplace_back(w, box); + } - //add wires one by one to current set of faces. - //We go from last to first, to make it so that outer wires come before inner wires. - std::vector< std::unique_ptr > faces; - for (int i = static_cast(wires.size()) - 1; i >= 0; --i) { - TopoDS_Wire& w = wires[i]; + // Sort wires by length of diagonal of bounding box. + std::stable_sort(wireInfos.begin(), wireInfos.end()); - //test if this wire is on any of existing faces (if yes, it's a hole; - // if no, it's a beginning of a new face). - //Since we are assuming the wires do not intersect, testing if one vertex of wire is in a face is enough. - gp_Pnt p = BRep_Tool::Pnt(TopoDS::Vertex(TopExp_Explorer(w, TopAbs_VERTEX).Current())); - FaceDriller* foundFace = nullptr; - for (std::unique_ptr& ff : faces) { - if (ff->hitTest(p)) { - foundFace = &(*ff); - break; + for (int i = 0; i < (reuseInnerWire ? 2 : 1); ++i) { + // add wires one by one to current set of faces. + std::vector> faces; + for (auto it = wireInfos.begin(); it != wireInfos.end();) { + + // test if this wire is on any of existing faces (if yes, it's a hole; + // if no, it's a beginning of a new face). + FaceDriller* foundFace = nullptr; + bool hitted = false; + for (auto rit = faces.rbegin(); rit != faces.rend(); ++rit) { + switch ((*rit)->hitTest(it->wire)) { + case FaceDriller::HitTest::Hit: + foundFace = rit->get(); + hitted = true; + break; + case FaceDriller::HitTest::HitOuter: + // Shape in outer wire but not on face, which means it is + // within a hole. So it's a hit and we shall make a new face + // with the wire. + hitted = true; + break; + default: + break; + } + } + + TopoDS_Wire w = TopoDS::Wire(it->wire.getShape()); + + if (foundFace) { + // wire is on a face. + if (reuseInnerWire) { + foundFace->addHole(*it, mySourceShapes); + } + else { + foundFace->addHole(w); + } + } + else { + // wire is not on a face. Start a new face. + faces.push_back(std::make_unique(plane, w)); + } + + if (i == 0 && reuseInnerWire && !hitted) { + // If reuseInnerWire, then discard the outer-most wire, and + // retry so that the previous hole (and nested hole) wires can + // become outer wire for new faces. + it = wireInfos.erase(it); + } + else { + ++it; } } - if (foundFace) { - //wire is on a face. - foundFace->addHole(w); + // and we are done! + for (std::unique_ptr& ff : faces) { + this->myShapesToReturn.push_back(ff->Face()); } - else { - //wire is not on a face. Start a new face. - faces.push_back(std::make_unique( - plane, w - )); - } - } - - //and we are done! - for (std::unique_ptr& ff : faces) { - this->myShapesToReturn.push_back(ff->Face()); } } @@ -139,42 +187,154 @@ FaceMakerBullseye::FaceDriller::FaceDriller(const gp_Pln& plane, TopoDS_Wire out this->myPlane = plane; this->myFace = TopoDS_Face(); - //Ensure correct orientation of the wire. - if (getWireDirection(myPlane, outerWire) < 0) + // Ensure correct orientation of the wire. + if (getWireDirection(myPlane, outerWire) < 0) { outerWire.Reverse(); + } myHPlane = new Geom_Plane(this->myPlane); BRep_Builder builder; builder.MakeFace(this->myFace, myHPlane, Precision::Confusion()); builder.Add(this->myFace, outerWire); + this->myTopoFace = TopoShape(this->myFace); } -bool FaceMakerBullseye::FaceDriller::hitTest(const gp_Pnt& point) const +FaceMakerBullseye::FaceDriller::HitTest +FaceMakerBullseye::FaceDriller::hitTest(const TopoShape& shape) const { - double u, v; - GeomAPI_ProjectPointOnSurf(point, myHPlane).LowerDistanceParameters(u, v); - BRepClass_FaceClassifier cl(myFace, gp_Pnt2d(u, v), Precision::Confusion()); - TopAbs_State ret = cl.State(); - switch (ret) { - case TopAbs_UNKNOWN: - throw Base::ValueError("FaceMakerBullseye::FaceDriller::hitTest: result unknown."); - break; - default: - return ret == TopAbs_IN || ret == TopAbs_ON; + auto vertex = TopoDS::Vertex(shape.getSubShape(TopAbs_VERTEX, 1)); + if (!myFaceBound.IsNull()) { + if (myTopoFaceBound.findShape(vertex) > 0) { + return HitTest::HitNone; + } + for (const auto& info : myHoles) { + if (info.wire.findShape(vertex)) { + return HitTest::Hit; + } + } + } + else if (myTopoFace.findShape(vertex) > 0) { + return HitTest::HitNone; } + double tol = BRep_Tool::Tolerance(vertex); + auto point = BRep_Tool::Pnt(vertex); + double u, v; + GeomAPI_ProjectPointOnSurf(point, myHPlane).LowerDistanceParameters(u, v); + const char* err = "FaceMakerBullseye::FaceDriller::hitTest: result unknown."; + auto hit = HitTest::HitNone; + if (!myFaceBound.IsNull()) { + BRepClass_FaceClassifier cl(myFaceBound, gp_Pnt2d(u, v), tol); + switch (cl.State()) { + case TopAbs_OUT: + case TopAbs_ON: + return HitTest::HitNone; + case TopAbs_IN: + hit = HitTest::HitOuter; + break; + default: + throw Base::ValueError(err); + } + } + BRepClass_FaceClassifier cl(myFace, gp_Pnt2d(u, v), tol); + TopAbs_State ret = cl.State(); + switch (ret) { + case TopAbs_IN: + return HitTest::Hit; + case TopAbs_ON: + if (hit == HitTest::HitOuter) { + // the given point is within the outer wire, but on some other wire + // of the face, which must be a hole wire, which means that two hole + // wires have shared vertex (or edge). We can deal with this if + // reuseInnerWire is on by merging these holes. + return HitTest::Hit; + } + return HitTest::HitNone; + case TopAbs_OUT: + return hit; + default: + throw Base::ValueError(err); + } +} + +void FaceMakerBullseye::FaceDriller::copyFaceBound(TopoDS_Face& face, + TopoShape& topoFace, + const TopoShape& source) +{ + face = BRepBuilderAPI_MakeFace(myHPlane, TopoDS::Wire(source.getSubShape(TopAbs_WIRE, 1))); + topoFace = TopoShape(face); } void FaceMakerBullseye::FaceDriller::addHole(TopoDS_Wire w) { - //Ensure correct orientation of the wire. - if (getWireDirection(myPlane, w) > 0) //if wire is CCW.. - w.Reverse(); //.. we want CW! + // Ensure correct orientation of the wire. + if (getWireDirection(myPlane, w) > 0) { // if wire is CCW.. + w.Reverse(); //.. we want CW! + } + + if (this->myFaceBound.IsNull()) { + copyFaceBound(this->myFaceBound, this->myTopoFaceBound, this->myTopoFace); + } BRep_Builder builder; builder.Add(this->myFace, w); } +void FaceMakerBullseye::FaceDriller::addHole(const WireInfo& wireInfo, + std::vector& sources) +{ + if (this->myFaceBound.IsNull()) { + copyFaceBound(this->myFaceBound, this->myTopoFaceBound, this->myTopoFace); + } + + if (!myJoiner) { + myJoiner.reset(new WireJoiner); + myJoiner->setOutline(true); + } + myJoiner->addShape(wireInfo.wire); + + bool intersected = false; + for (const auto& info : myHoles) { + if (!info.bound.IsOut(wireInfo.bound) || !wireInfo.bound.IsOut(info.bound)) { + intersected = true; + break; + } + } + + myHoles.push_back(wireInfo); + TopoShape wire = wireInfo.wire; + + if (intersected) { + TopoShape hole; + // Join intersected wires and get their outline + myJoiner->getResultWires(hole); + // Check if the hole gets merged. + if (!hole.findShape(wireInfo.wire.getShape())) { + for (const auto& e : wireInfo.wire.getSubTopoShapes(TopAbs_EDGE)) { + if (hole.findShape(e.getShape()) > 0) { + continue; + } + for (const auto& e : hole.findSubShapesWithSharedVertex(e.getShape())) { + sources.push_back(e); + } + } + copyFaceBound(this->myFace, this->myTopoFace, this->myTopoFaceBound); + wire = hole; + } + } + + BRep_Builder builder; + for (const auto& w : wire.getSubShapes(TopAbs_WIRE)) { + // Ensure correct orientation of the wire. + if (getWireDirection(myPlane, TopoDS::Wire(w)) > 0) { // if wire is CCW.. + builder.Add(this->myFace, TopoDS::Wire(w.Reversed())); //.. we want CW! + } + else { + builder.Add(this->myFace, TopoDS::Wire(w)); + } + } +} + int FaceMakerBullseye::FaceDriller::getWireDirection(const gp_Pln& plane, const TopoDS_Wire& wire) { //make a test face @@ -194,3 +354,22 @@ int FaceMakerBullseye::FaceDriller::getWireDirection(const gp_Pln& plane, const return normal_co ? 1 : -1; } + +////////////////////////////////////////////////////////////////////////////////////////////////// + +TYPESYSTEM_SOURCE(Part::FaceMakerRing, Part::FaceMakerBullseye) + +FaceMakerRing::FaceMakerRing() +{ + reuseInnerWire = true; +} + +std::string FaceMakerRing::getUserFriendlyName() const +{ + return {tr("Ring facemaker").toStdString()}; +} + +std::string FaceMakerRing::getBriefExplanation() const +{ + return {tr("Supports making planar faces with holes and holes as faces.").toStdString()}; +} diff --git a/src/Mod/Part/App/FaceMakerBullseye.h b/src/Mod/Part/App/FaceMakerBullseye.h index 65d9c6c1b2..b7b9235243 100644 --- a/src/Mod/Part/App/FaceMakerBullseye.h +++ b/src/Mod/Part/App/FaceMakerBullseye.h @@ -27,11 +27,14 @@ #include #include +#include namespace Part { +class WireJoiner; + /** * @brief The FaceMakerBullseye class is a tool to make planar faces with holes, * where there can be additional faces inside holes and they can have holes too @@ -45,14 +48,15 @@ class PartExport FaceMakerBullseye: public FaceMakerPublic { TYPESYSTEM_HEADER_WITH_OVERRIDE(); public: - FaceMakerBullseye() = default; + FaceMakerBullseye(): + planeSupplied(false){} /** * @brief setPlane: sets the plane to use when making faces. This is * optional. If the plane was set, it is not tested that the wires are * planar or on the supplied plane, potentially speeding things up. * @param plane FIXME: the plane is not propagated if processing compounds. */ - void setPlane(const gp_Pln& plane) override; + virtual void setPlane(const gp_Pln& plane) override; std::string getUserFriendlyName() const override; std::string getBriefExplanation() const override; @@ -62,7 +66,22 @@ protected: protected: gp_Pln myPlane; //externally supplied plane (if any) - bool planeSupplied{false}; + bool planeSupplied {false}; + bool reuseInnerWire {false}; + + struct WireInfo + { + TopoShape wire; + Bnd_Box bound; + double extent; + WireInfo(const TopoShape& s, const Bnd_Box& b) + : wire(s) + , bound(b) + { + extent = bound.SquareExtent(); + } + bool operator<(const WireInfo& other) const; + }; /** * @brief The FaceDriller class is similar to BRepBuilderAPI_MakeFace, @@ -74,13 +93,26 @@ protected: public: FaceDriller(const gp_Pln& plane, TopoDS_Wire outerWire); + /// Hit test result + enum class HitTest + { + /// Not hitting + HitNone = 0, + /// Hit inside face + Hit = 1, + /// Hit inside the out wire bound + HitOuter = 2, + }; + /** - * @brief hitTest: returns True if point is on the face - * @param point + * @brief hitTest: returns True if a shape is on the face + * @param shape */ - bool hitTest(const gp_Pnt& point) const; + HitTest hitTest(const TopoShape& shape) const; void addHole(TopoDS_Wire w); + void addHole(const WireInfo& info, std::vector& sources); + void copyFaceBound(TopoDS_Face& f, TopoShape& tf, const TopoShape& source); const TopoDS_Face& Face() const {return myFace;} public: @@ -94,10 +126,35 @@ protected: private: gp_Pln myPlane; TopoDS_Face myFace; + TopoDS_Face myFaceBound; + TopoShape myTopoFace; + TopoShape myTopoFaceBound; + std::vector myHoles; Handle(Geom_Surface) myHPlane; + std::unique_ptr myJoiner; }; }; +/** + * The FaceMakerRing is a tool to make faces with holes. + * + * The tool assumes the wires are closed and do not intersect with each other. + * Each wire will be used to as an outer wire to make a face, with inner wire as + * holes. Only the first level inner wire will be used for holes. Nested inner + * wires are ignored. Note that each inner wire will also be used as outer wire + * for making face, with the second level inner wires as holes, so on and so + * forth. + */ +class PartExport FaceMakerRing: public FaceMakerBullseye +{ + TYPESYSTEM_HEADER_WITH_OVERRIDE(); + +public: + FaceMakerRing(); + + virtual std::string getUserFriendlyName() const override; + virtual std::string getBriefExplanation() const override; +}; }//namespace Part #endif // PART_FACEMAKER_BULLSEYE_H diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp index 422ba65fa1..192aae8a6c 100644 --- a/src/Mod/Sketcher/App/SketchObject.cpp +++ b/src/Mod/Sketcher/App/SketchObject.cpp @@ -474,16 +474,18 @@ Part::TopoShape SketchObject::buildInternals(const Part::TopoShape &edges) const joiner.getResultWires(result, "SKF"); result = result.makeElementFace(result.getSubTopoShapes(TopAbs_WIRE), /*op*/"", - /*maker*/"Part::FaceMakerBullseye", + /*maker*/"Part::FaceMakerRing", /*pln*/nullptr ); } Part::TopoShape openWires(getID(), getDocument()->getStringHasher()); joiner.getOpenWires(openWires, "SKF"); - if (openWires.isNull()) + if (openWires.isNull()) { return result; // No open wires, return either face or empty toposhape - if (result.isNull()) + } + if (result.isNull()) { return openWires; // No face, but we have open wires to return as a shape + } return result.makeElementCompound({result, openWires}); // Compound and return both } catch (Base::Exception &e) { FC_WARN("Failed to make face for sketch: " << e.what());