From c31ebeeee68ab39ed436a60cf9924723bc304348 Mon Sep 17 00:00:00 2001 From: bgbsww Date: Mon, 18 Mar 2024 09:08:34 -0400 Subject: [PATCH] Toponaming/Part: cleanup FeatureExtrusion --- src/Mod/Part/App/ExtrusionHelper.cpp | 232 ++++++-------------- src/Mod/Part/App/ExtrusionHelper.h | 23 +- src/Mod/Part/App/FeatureExtrusion.cpp | 67 +++--- src/Mod/Part/App/FeatureExtrusion.h | 22 +- tests/src/Mod/Part/App/FeatureExtrusion.cpp | 27 +++ tests/src/Mod/Part/App/PartTestHelpers.cpp | 2 + 6 files changed, 155 insertions(+), 218 deletions(-) diff --git a/src/Mod/Part/App/ExtrusionHelper.cpp b/src/Mod/Part/App/ExtrusionHelper.cpp index 2b40a7a637..191d260dc5 100644 --- a/src/Mod/Part/App/ExtrusionHelper.cpp +++ b/src/Mod/Part/App/ExtrusionHelper.cpp @@ -41,10 +41,14 @@ #include #include +#include +#include +// #include #include "ExtrusionHelper.h" +#include "TopoShape.h" #include "BRepOffsetAPI_MakeOffsetFix.h" - +#include "Geometry.h" using namespace Part; @@ -478,138 +482,58 @@ void ExtrusionHelper::createTaperedPrismOffset(TopoDS_Wire sourceWire, } -static TopoShape makEDraftUsingPipe(const std::vector &_wires, - App::StringHasherRef hasher) +void ExtrusionHelper::makeElementDraft(const ExtrusionParameters& params, + const TopoShape& _shape, + std::vector& drafts) { - std::vector shells; - std::vector frontwires, backwires; + double distanceFwd = tan(params.taperAngleFwd) * params.lengthFwd; + double distanceRev = tan(params.taperAngleRev) * params.lengthRev; - if (_wires.size() < 2) - throw Base::CADKernelError("Not enough wire section"); - - std::vector wires; - wires.reserve(_wires.size()); - for (auto &wire : _wires) { - // Make a copy to work around OCCT bug on offset circular shapes - wires.push_back(wire.makECopy()); - } - GeomLineSegment line; - Base::Vector3d pstart, pend; - wires.front().getCenterOfGravity(pstart); - gp_Pln pln; - if (wires.back().findPlane(pln)) { - auto dir = pln.Position().Direction(); - auto base = pln.Location(); - pend = pstart; - pend.ProjectToPlane(Base::Vector3d(base.X(), base.Y(), base.Z()), - Base::Vector3d(dir.X(), dir.Y(), dir.Z())); - } else - wires.back().getCenterOfGravity(pend); - line.setPoints(pstart, pend); - - BRepBuilderAPI_MakeWire mkWire(TopoDS::Edge(line.toShape())); - BRepOffsetAPI_MakePipeShell mkPS(mkWire.Wire()); - mkPS.SetTolerance(Precision::Confusion()); - mkPS.SetTransitionMode(BRepBuilderAPI_Transformed); - mkPS.SetMode(false); - - for (auto &wire : wires) - mkPS.Add(TopoDS::Wire(wire.getShape())); - - if (!mkPS.IsReady()) - throw Base::CADKernelError("Shape could not be built"); - - TopoShape result(0,hasher); - result.makEShape(mkPS,wires); - - if (!mkPS.Shape().Closed()) { - // shell is not closed - use simulate to get the end wires - TopTools_ListOfShape sim; - mkPS.Simulate(2, sim); - - TopoShape front(sim.First()); - if(front.countSubShapes(TopAbs_EDGE)==wires.front().countSubShapes(TopAbs_EDGE)) { - front = wires.front(); - front.setShape(sim.First(),false); - }else - front.Tag = -wires.front().Tag; - TopoShape back(sim.Last()); - if(back.countSubShapes(TopAbs_EDGE)==wires.back().countSubShapes(TopAbs_EDGE)) { - back = wires.back(); - back.setShape(sim.Last(),false); - }else - back.Tag = -wires.back().Tag; - - // build the end faces, sew the shell and build the final solid - front = front.makEFace(); - back = back.makEFace(); - - BRepBuilderAPI_Sewing sewer; - sewer.SetTolerance(Precision::Confusion()); - sewer.Add(front.getShape()); - sewer.Add(back.getShape()); - sewer.Add(result.getShape()); - - sewer.Perform(); - result = result.makEShape(sewer); - } - - result = result.makESolid(); - - BRepClass3d_SolidClassifier SC(result.getShape()); - SC.PerformInfinitePoint(Precision::Confusion()); - if (SC.State() == TopAbs_IN) { - result.setShape(result.getShape().Reversed(),false); - } - return result; -} - -void ExtrusionHelper::makEDraft(const ExtrusionParameters& params, - const TopoShape& _shape, - std::vector& drafts, - App::StringHasherRef hasher) -{ - double distanceFwd = tan(params.taperAngleFwd)*params.lengthFwd; - double distanceRev = tan(params.taperAngleRev)*params.lengthRev; - - gp_Vec vecFwd = gp_Vec(params.dir)*params.lengthFwd; - gp_Vec vecRev = gp_Vec(params.dir.Reversed())*params.lengthRev; + gp_Vec vecFwd = gp_Vec(params.dir) * params.lengthFwd; + gp_Vec vecRev = gp_Vec(params.dir.Reversed()) * params.lengthRev; bool bFwd = fabs(params.lengthFwd) > Precision::Confusion(); bool bRev = fabs(params.lengthRev) > Precision::Confusion(); - bool bMid = !bFwd || !bRev || params.lengthFwd*params.lengthRev > 0.0; //include the source shape as loft section? + bool bMid = !bFwd || !bRev + || params.lengthFwd * params.lengthRev > 0.0; // include the source shape as loft section? TopoShape shape = _shape; TopoShape sourceWire; - if (shape.isNull()) + if (shape.isNull()) { Standard_Failure::Raise("Not a valid shape"); + } - if (params.solid && !shape.hasSubShape(TopAbs_FACE)) - shape = shape.makEFace(nullptr, params.faceMakerClass.c_str()); + if (params.solid && !shape.hasSubShape(TopAbs_FACE)) { + shape = shape.makeElementFace(nullptr, params.faceMakerClass.c_str()); + } if (shape.shapeType() == TopAbs_FACE) { std::vector wires; TopoShape outerWire = shape.splitWires(&wires, TopoShape::ReorientForward); - if (outerWire.isNull()) + if (outerWire.isNull()) { Standard_Failure::Raise("Missing outer wire"); - if (wires.empty()) + } + if (wires.empty()) { shape = outerWire; + } else { unsigned pos = drafts.size(); - makEDraft(params, outerWire, drafts, hasher); - if (drafts.size() != pos+1) + makeElementDraft(params, outerWire, drafts); + if (drafts.size() != pos + 1) { Standard_Failure::Raise("Failed to make drafted extrusion"); + } std::vector inner; - TopoShape innerWires(0, hasher); - innerWires.makECompound(wires,"",false); - ExtrusionParameters copy = params; - copy.taperAngleFwd = params.innerTaperAngleFwd; - copy.taperAngleRev = params.innerTaperAngleRev; - makEDraft(copy, innerWires, inner, hasher); - if (inner.empty()) + TopoShape innerWires(0); + innerWires.makeElementCompound( + wires, + "", + TopoShape::SingleShapeCompoundCreationPolicy::returnShape); + makeElementDraft(params, innerWires, inner); + if (inner.empty()) { Standard_Failure::Raise("Failed to make drafted extrusion with inner hole"); + } inner.insert(inner.begin(), drafts.back()); - drafts.back().makECut(inner); + drafts.back().makeElementCut(inner); return; } } @@ -625,8 +549,9 @@ void ExtrusionHelper::makEDraft(const ExtrusionParameters& params, sourceWire.mapSubElement(shape); } else if (shape.shapeType() == TopAbs_COMPOUND) { - for(auto &s : shape.getSubTopoShapes()) - makEDraft(params, s, drafts, hasher); + for (auto& s : shape.getSubTopoShapes()) { + makeElementDraft(params, s, drafts); + } } else { Standard_Failure::Raise("Only a wire or a face is supported"); @@ -634,73 +559,48 @@ void ExtrusionHelper::makEDraft(const ExtrusionParameters& params, if (!sourceWire.isNull()) { std::vector list_of_sections; - auto makeOffset = [&sourceWire](const gp_Vec& translation, double offset) -> TopoShape { - gp_Trsf mat; - mat.SetTranslation(translation); - TopoShape offsetShape(sourceWire.makETransform(mat,"RV")); - if (fabs(offset)>Precision::Confusion()) - offsetShape = offsetShape.makEOffset2D(offset, TopoShape::JoinType::Intersection); - return offsetShape; - }; - - //first. add wire for reversed part of extrusion - if (bRev){ - auto offsetShape = makeOffset(vecRev, distanceRev); - if (offsetShape.isNull()) - Standard_Failure::Raise("Tapered shape is empty"); - TopAbs_ShapeEnum type = offsetShape.getShape().ShapeType(); - if (type == TopAbs_WIRE) { - list_of_sections.push_back(offsetShape); - } - else if (type == TopAbs_EDGE) { - list_of_sections.push_back(offsetShape.makEWires()); - } - else { - Standard_Failure::Raise("Tapered shape type is not supported"); - } + if (bRev) { + TopoDS_Wire offsetWire; + createTaperedPrismOffset(TopoDS::Wire(sourceWire.getShape()), + vecRev, + distanceRev, + false, + offsetWire); + list_of_sections.push_back(TopoShape(offsetWire, sourceWire.Tag)); } - //next. Add source wire as middle section. Order is important. - if (bMid){ + // next. Add source wire as middle section. Order is important. + if (bMid) { list_of_sections.push_back(sourceWire); } - //finally. Forward extrusion offset wire. - if (bFwd){ - auto offsetShape = makeOffset(vecFwd, distanceFwd); - if (offsetShape.isNull()) - Standard_Failure::Raise("Tapered shape is empty"); - TopAbs_ShapeEnum type = offsetShape.getShape().ShapeType(); - if (type == TopAbs_WIRE) { - list_of_sections.push_back(offsetShape); - } - else if (type == TopAbs_EDGE) { - list_of_sections.push_back(offsetShape.makEWires()); - } - else { - Standard_Failure::Raise("Tapered shape type is not supported"); - } + // finally. Forward extrusion offset wire. + if (bFwd) { + TopoDS_Wire offsetWire; + createTaperedPrismOffset(TopoDS::Wire(sourceWire.getShape()), + vecFwd, + distanceFwd, + false, + offsetWire); + list_of_sections.push_back(TopoShape(offsetWire, sourceWire.Tag)); } try { -#if defined(__GNUC__) && defined (FC_OS_LINUX) +#if defined(__GNUC__) && defined(FC_OS_LINUX) Base::SignalException se; #endif - if (params.usepipe) { - drafts.push_back(makEDraftUsingPipe(list_of_sections, hasher)); - return; + + // make loft + BRepOffsetAPI_ThruSections mkGenerator(params.solid ? Standard_True : Standard_False, + /*ruled=*/Standard_True); + for (auto& s : list_of_sections) { + mkGenerator.AddWire(TopoDS::Wire(s.getShape())); } - //make loft - BRepOffsetAPI_ThruSections mkGenerator( - params.solid ? Standard_True : Standard_False, /*ruled=*/Standard_True); - for(auto &s : list_of_sections) - mkGenerator.AddWire(TopoDS::Wire(s.getShape())); - mkGenerator.Build(); - drafts.push_back(TopoShape(0,hasher).makEShape(mkGenerator,list_of_sections)); + drafts.push_back(TopoShape(0).makeElementShape(mkGenerator, list_of_sections)); } - catch (Standard_Failure &){ + catch (Standard_Failure&) { throw; } catch (...) { diff --git a/src/Mod/Part/App/ExtrusionHelper.h b/src/Mod/Part/App/ExtrusionHelper.h index f87474502e..bff010a94d 100644 --- a/src/Mod/Part/App/ExtrusionHelper.h +++ b/src/Mod/Part/App/ExtrusionHelper.h @@ -34,6 +34,24 @@ namespace Part { +class TopoShape; + +/** + * @brief The ExtrusionParameters struct is supposed to be filled with final + * extrusion parameters, after resolving links, applying mode logic, + * reversing, etc., and be passed to extrudeShape. + */ +struct ExtrusionParameters +{ + gp_Dir dir; + double lengthFwd {0}; + double lengthRev {0}; + bool solid {false}; + double taperAngleFwd {0}; // in radians + double taperAngleRev {0}; + std::string faceMakerClass; +}; + class PartExport ExtrusionHelper { public: @@ -70,9 +88,8 @@ public: TopoDS_Wire& result); /** Same as makeDraft() with support of element mapping */ - static void makEDraft(const ExtrusionParameters& params, const TopoShape&, - std::vector&, App::StringHasherRef hasher); - + static void + makeElementDraft(const ExtrusionParameters& params, const TopoShape&, std::vector&); }; } //namespace Part diff --git a/src/Mod/Part/App/FeatureExtrusion.cpp b/src/Mod/Part/App/FeatureExtrusion.cpp index ef38272aec..ae12b30f2c 100644 --- a/src/Mod/Part/App/FeatureExtrusion.cpp +++ b/src/Mod/Part/App/FeatureExtrusion.cpp @@ -127,9 +127,9 @@ bool Extrusion::fetchAxisLink(const App::PropertyLinkSub& axisLink, Base::Vector return true; } -Extrusion::ExtrusionParameters Extrusion::computeFinalParameters() +ExtrusionParameters Extrusion::computeFinalParameters() { - Extrusion::ExtrusionParameters result; + ExtrusionParameters result; Base::Vector3d dir; switch (this->DirMode.getValue()) { case dmCustom: @@ -229,11 +229,10 @@ Base::Vector3d Extrusion::calculateShapeNormal(const App::PropertyLink& shapeLin return Base::Vector3d(normal.X(), normal.Y(), normal.Z()); } -TopoShape Extrusion::extrudeShape(const TopoShape& source, const Extrusion::ExtrusionParameters& params) +void Extrusion::extrudeShape(TopoShape &result, const TopoShape &source, const ExtrusionParameters& params) { gp_Vec vec = gp_Vec(params.dir).Multiplied(params.lengthFwd + params.lengthRev);//total vector of extrusion #ifndef FC_USE_TNP_FIX - TopoDS_Shape result; if (std::fabs(params.taperAngleFwd) >= Precision::Angular() || std::fabs(params.taperAngleRev) >= Precision::Angular()) { //Tapered extrusion! @@ -306,51 +305,57 @@ TopoShape Extrusion::extrudeShape(const TopoShape& source, const Extrusion::Extr result = mkPrism.Shape(); } - if (result.IsNull()) + if (result.isNull()) throw NullShapeException("Result of extrusion is null shape."); - return TopoShape(result); -#else // FC_NO_ELEMENT_MAP +// return TopoShape(result); +#else // #0000910: Circles Extrude Only Surfaces, thus use BRepBuilderAPI_Copy - TopoShape myShape(source.makECopy()); + TopoShape myShape(source.makeElementCopy()); - if (std::fabs(params.taperAngleFwd) >= Precision::Angular() || - std::fabs(params.taperAngleRev) >= Precision::Angular()) { - //Tapered extrusion! -# if defined(__GNUC__) && defined (FC_OS_LINUX) + if (std::fabs(params.taperAngleFwd) >= Precision::Angular() + || std::fabs(params.taperAngleRev) >= Precision::Angular()) { + // Tapered extrusion! +#if defined(__GNUC__) && defined(FC_OS_LINUX) Base::SignalException se; -# endif +#endif std::vector drafts; - ExtrusionHelper::makEDraft(params, myShape, drafts, result.Hasher); + ExtrusionHelper::makeElementDraft(params, myShape, drafts); if (drafts.empty()) { Standard_Failure::Raise("Drafting shape failed"); - }else - result.makECompound(drafts,0,false); + } + else { + result.makeElementCompound(drafts, + 0, + TopoShape::SingleShapeCompoundCreationPolicy::returnShape); + } } else { - //Regular (non-tapered) extrusion! - if (source.isNull()) + // Regular (non-tapered) extrusion! + if (source.isNull()) { Standard_Failure::Raise("Cannot extrude empty shape"); + } - //apply reverse part of extrusion by shifting the source shape + // apply reverse part of extrusion by shifting the source shape if (fabs(params.lengthRev) > Precision::Confusion()) { gp_Trsf mov; mov.SetTranslation(gp_Vec(params.dir) * (-params.lengthRev)); - myShape = myShape.makETransform(mov); + myShape = myShape.makeElementTransform(mov); } - //make faces from wires + // make faces from wires if (params.solid) { - //test if we need to make faces from wires. If there are faces - we don't. - if(!myShape.hasSubShape(TopAbs_FACE)) { - if(!myShape.Hasher) + // test if we need to make faces from wires. If there are faces - we don't. + if (!myShape.hasSubShape(TopAbs_FACE)) { + if (!myShape.Hasher) { myShape.Hasher = result.Hasher; - myShape = myShape.makEFace(0,params.faceMakerClass.c_str()); + } + myShape = myShape.makeElementFace(nullptr, params.faceMakerClass.c_str()); } } - //extrude! - result.makEPrism(myShape,vec); + // extrude! + result.makeElementPrism(myShape, vec); } #endif } @@ -358,12 +363,14 @@ TopoShape Extrusion::extrudeShape(const TopoShape& source, const Extrusion::Extr App::DocumentObjectExecReturn* Extrusion::execute() { App::DocumentObject* link = Base.getValue(); - if (!link) + if (!link) { return new App::DocumentObjectExecReturn("No object linked"); + } try { - Extrusion::ExtrusionParameters params = computeFinalParameters(); - TopoShape result = extrudeShape(Feature::getShape(link), params); + ExtrusionParameters params = computeFinalParameters(); + TopoShape result(0); + extrudeShape(result, Feature::getTopoShape(link), params); this->Shape.setValue(result); return App::DocumentObject::StdReturn; } diff --git a/src/Mod/Part/App/FeatureExtrusion.h b/src/Mod/Part/App/FeatureExtrusion.h index 0101972b60..eb1c976f7a 100644 --- a/src/Mod/Part/App/FeatureExtrusion.h +++ b/src/Mod/Part/App/FeatureExtrusion.h @@ -28,7 +28,7 @@ #include "FaceMakerCheese.h" #include "PartFeature.h" - +#include "ExtrusionHelper.h" namespace Part { @@ -53,22 +53,6 @@ public: App::PropertyAngle TaperAngleRev; App::PropertyString FaceMakerClass; - - /** - * @brief The ExtrusionParameters struct is supposed to be filled with final - * extrusion parameters, after resolving links, applying mode logic, - * reversing, etc., and be passed to extrudeShape. - */ - struct ExtrusionParameters { - gp_Dir dir; - double lengthFwd{0}; - double lengthRev{0}; - bool solid{false}; - double taperAngleFwd{0}; //in radians - double taperAngleRev{0}; - std::string faceMakerClass; - }; - /** @name methods override feature */ //@{ /// recalculate the feature @@ -82,11 +66,11 @@ public: /** * @brief extrudeShape powers the extrusion feature. + * @param result: result of extrusion * @param source: the shape to be extruded * @param params: extrusion parameters - * @return result of extrusion */ - static TopoShape extrudeShape(const TopoShape& source, const ExtrusionParameters& params); + static void extrudeShape(TopoShape &result, const TopoShape &source, const ExtrusionParameters& params); /** * @brief fetchAxisLink: read AxisLink to obtain the direction and diff --git a/tests/src/Mod/Part/App/FeatureExtrusion.cpp b/tests/src/Mod/Part/App/FeatureExtrusion.cpp index 92fcb4909d..7374ed729b 100644 --- a/tests/src/Mod/Part/App/FeatureExtrusion.cpp +++ b/tests/src/Mod/Part/App/FeatureExtrusion.cpp @@ -7,6 +7,7 @@ #include "BRepBuilderAPI_MakeEdge.hxx" #include "PartTestHelpers.h" +#include "Mod/Sketcher/App/SketchObject.h" class FeatureExtrusionTest: public ::testing::Test, public PartTestHelpers::PartTestHelperClass { @@ -298,3 +299,29 @@ TEST_F(FeatureExtrusionTest, testExecuteFaceMaker) EXPECT_FLOAT_EQ(volume, len * wid * ext1); EXPECT_TRUE(PartTestHelpers::boxesMatch(bb, Base::BoundBox3d(0, 0, 0, len, wid, ext1))); } + +TEST_F(FeatureExtrusionTest, testFaceWithHoles) +{ + // Arrange + float radius = 0.75; + auto [face1, wire1, wire2] = PartTestHelpers::CreateFaceWithRoundHole(len, wid, radius); + // face1 is the sum of the outside (wire1) and the internal hole (wire2). + Part::TopoShape newFace = Part::TopoShape(face1).makeElementFace(nullptr); + // newFace cleans that up and is the outside minus the internal hole. + auto face2 = newFace.getShape(); + + auto partFeature = dynamic_cast(_doc->addObject("Part::Feature")); + partFeature->Shape.setValue(face2); + _extrusion->Base.setValue(_doc->getObjects().back()); + _extrusion->FaceMakerClass.setValue("Part::FaceMakerCheese"); + // Act + _extrusion->execute(); + Part::TopoShape ts = _extrusion->Shape.getValue(); + double volume = PartTestHelpers::getVolume(ts.getShape()); + Base::BoundBox3d bb = ts.getBoundBox(); + // Assert + EXPECT_FLOAT_EQ(volume, len * wid * ext1 - radius * radius * M_PI * ext1); + EXPECT_TRUE(PartTestHelpers::boxesMatch(bb, Base::BoundBox3d(0, 0, 0, len, wid, ext1))); + EXPECT_FLOAT_EQ(PartTestHelpers::getArea(face1), len * wid + radius * radius * M_PI); + EXPECT_FLOAT_EQ(PartTestHelpers::getArea(face2), len * wid - radius * radius * M_PI); +} diff --git a/tests/src/Mod/Part/App/PartTestHelpers.cpp b/tests/src/Mod/Part/App/PartTestHelpers.cpp index 76a8d6284a..7e34bde2b0 100644 --- a/tests/src/Mod/Part/App/PartTestHelpers.cpp +++ b/tests/src/Mod/Part/App/PartTestHelpers.cpp @@ -111,6 +111,8 @@ CreateFaceWithRoundHole(float len, float wid, float radius) auto edge5 = BRepBuilderAPI_MakeEdge(circ1).Edge(); auto wire2 = BRepBuilderAPI_MakeWire(edge5).Wire(); auto face2 = BRepBuilderAPI_MakeFace(face1, wire2).Face(); + // Beware: somewhat counterintuitively, face2 is the sum of face1 and the area inside wire2, + // not the difference. return {face2, wire1, wire2}; }