From 24bb499c1f363edbe3661477760a3eed4294ff7f Mon Sep 17 00:00:00 2001 From: bgbsww <120601209+bgbsww@users.noreply.github.com> Date: Mon, 2 Sep 2024 11:59:44 -0400 Subject: [PATCH] Cleanup element map in Revolution and add test (#15959) --- src/Mod/Part/App/TopoShape.h | 6 +- src/Mod/Part/App/TopoShapeExpansion.cpp | 13 ++-- src/Mod/PartDesign/App/FeatureRevolution.cpp | 59 +++++++--------- src/Mod/PartDesign/App/FeatureRevolution.h | 6 +- .../TestTopologicalNamingProblem.py | 70 ++++++++++++++----- 5 files changed, 92 insertions(+), 62 deletions(-) diff --git a/src/Mod/Part/App/TopoShape.h b/src/Mod/Part/App/TopoShape.h index 89dadd5f9e..9154de03d1 100644 --- a/src/Mod/Part/App/TopoShape.h +++ b/src/Mod/Part/App/TopoShape.h @@ -1106,7 +1106,8 @@ public: /** Make revolved shell around a basis shape * - * @param base: the basis shape + * @param base: the basis shape (solid) + * @param profile: the shape to be revolved * @param axis: the revolving axis * @param face_maker: optional type name of the the maker used to make a * face from basis shape @@ -1120,6 +1121,7 @@ public: * @return Return the generated new shape. The TopoShape itself is not modified. */ TopoShape& makeElementRevolution(const TopoShape& _base, + const TopoDS_Shape& profile, const gp_Ax1& axis, const TopoDS_Face& supportface, const TopoDS_Face& uptoface, @@ -1143,6 +1145,7 @@ public: * @return Return the generated new shape. The TopoShape itself is not modified. */ TopoShape& makeElementRevolution(const gp_Ax1& axis, + const TopoDS_Shape& profile, const TopoDS_Face& supportface, const TopoDS_Face& uptoface, const char* face_maker = nullptr, @@ -1151,6 +1154,7 @@ public: const char* op = nullptr) const { return TopoShape(0, Hasher).makeElementRevolution(*this, + profile, axis, supportface, uptoface, diff --git a/src/Mod/Part/App/TopoShapeExpansion.cpp b/src/Mod/Part/App/TopoShapeExpansion.cpp index d54448e2f3..c7a8f7cf6c 100644 --- a/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -97,6 +97,7 @@ #include "Geometry.h" #include "BRepOffsetAPI_MakeOffsetFix.h" #include "Base/Tools.h" +#include "Base/BoundBox.h" #include #include @@ -4436,6 +4437,7 @@ TopoShape& TopoShape::makeElementRevolve(const TopoShape& _base, } TopoShape& TopoShape::makeElementRevolution(const TopoShape& _base, + const TopoDS_Shape& profile, const gp_Ax1& axis, const TopoDS_Face& supportface, const TopoDS_Face& uptoface, @@ -4447,7 +4449,9 @@ TopoShape& TopoShape::makeElementRevolution(const TopoShape& _base, if (!op) { op = Part::OpCodes::Revolve; } - + if (Mode == RevolMode::None) { + Mode = RevolMode::FuseWithBase; + } TopoShape base(_base); if (base.isNull()) { FC_THROWM(NullShapeException, "Null shape"); @@ -4460,8 +4464,8 @@ TopoShape& TopoShape::makeElementRevolution(const TopoShape& _base, } BRepFeat_MakeRevol mkRevol; - for (TopExp_Explorer xp(base.getShape(), TopAbs_FACE); xp.More(); xp.Next()) { - mkRevol.Init(_base.getShape(), + for (TopExp_Explorer xp(profile, TopAbs_FACE); xp.More(); xp.Next()) { + mkRevol.Init(base.getShape(), xp.Current(), supportface, axis, @@ -4472,9 +4476,6 @@ TopoShape& TopoShape::makeElementRevolution(const TopoShape& _base, throw Base::RuntimeError("Revolution: Up to face: Could not revolve the sketch!"); } base = mkRevol.Shape(); - if (Mode == RevolMode::None) { - Mode = RevolMode::FuseWithBase; - } } return makeElementShape(mkRevol, base, op); } diff --git a/src/Mod/PartDesign/App/FeatureRevolution.cpp b/src/Mod/PartDesign/App/FeatureRevolution.cpp index 6e9f153ded..b2a5ef6bf7 100644 --- a/src/Mod/PartDesign/App/FeatureRevolution.cpp +++ b/src/Mod/PartDesign/App/FeatureRevolution.cpp @@ -161,38 +161,40 @@ App::DocumentObjectExecReturn* Revolution::execute() // Create a fresh support even when base exists so that it can be used for patterns TopoShape result(0); - TopoDS_Face supportface = getSupportFace(); - supportface.Move(invObjLoc); + TopoShape supportface = getSupportFace(); + supportface.move(invObjLoc); if (method == RevolMethod::ToFace || method == RevolMethod::ToFirst || method == RevolMethod::ToLast) { - TopoDS_Face upToFace; + TopoShape upToFace; if (method == RevolMethod::ToFace) { - getFaceFromLinkSub(upToFace, UpToFace); - upToFace.Move(invObjLoc); + getUpToFaceFromLinkSub(upToFace, UpToFace); + upToFace.move(invObjLoc); } else { throw Base::RuntimeError( "ProfileBased: Revolution up to first/last is not yet supported"); } - // TODO: This method is designed for extrusions. needs to be adapted for revolutions. - // getUpToFace(upToFace, base, supportface, sketchshape, method, dir); - - // TopoDS_Face supportface = getSupportFace(); - supportface.Move(invObjLoc); - if (Reversed.getValue()) { dir.Reverse(); } - TopExp_Explorer Ex(supportface, TopAbs_WIRE); + TopExp_Explorer Ex(supportface.getShape(), TopAbs_WIRE); if (!Ex.More()) { supportface = TopoDS_Face(); } - + RevolMode mode = RevolMode::None; + // revolve the face to a solid try { - result = base.makeElementRevolution(gp_Ax1(pnt, dir), supportface, upToFace); + result = base.makeElementRevolution(base, + TopoDS::Face(sketchshape.getShape()), + gp_Ax1(pnt, dir), + TopoDS::Face(supportface.getShape()), + TopoDS::Face(upToFace.getShape()), + nullptr, + Part::RevolMode::None, + Standard_True); } catch (Standard_Failure&) { return new App::DocumentObjectExecReturn("Could not revolve the sketch!"); @@ -292,7 +294,7 @@ Revolution::RevolMethod Revolution::methodFromString(const std::string& methodSt } void Revolution::generateRevolution(TopoShape& revol, - const TopoDS_Shape& sketchshape, + const TopoShape& sketchshape, const gp_Ax1& axis, const double angle, const double angle2, @@ -322,15 +324,16 @@ void Revolution::generateRevolution(TopoShape& revol, revolAx.Reverse(); } - TopoDS_Shape from = sketchshape; + TopoShape from = sketchshape; if (method == RevolMethod::TwoDimensions || midplane) { gp_Trsf mov; mov.SetRotation(revolAx, angleOffset); TopLoc_Location loc(mov); - from.Move(loc); + from.move(loc); } - revol = TopoShape(from).makeElementRevolve(revolAx,angleTotal); + revol = from; + revol = revol.makeElementRevolve(revolAx,angleTotal); revol.Tag = -getID(); } else { std::stringstream str; @@ -339,8 +342,8 @@ void Revolution::generateRevolution(TopoShape& revol, } } -void Revolution::generateRevolution(TopoDS_Shape& revol, - const TopoDS_Shape& baseshape, +void Revolution::generateRevolution(TopoShape& revol, + const TopoShape& baseshape, const TopoDS_Shape& profileshape, const TopoDS_Face& supportface, const TopoDS_Face& uptoface, @@ -350,20 +353,8 @@ void Revolution::generateRevolution(TopoDS_Shape& revol, Standard_Boolean Modify) { if (method == RevolMethod::ToFirst || method == RevolMethod::ToFace || method == RevolMethod::ToLast) { - BRepFeat_MakeRevol RevolMaker; - TopoDS_Shape base = baseshape; - for (TopExp_Explorer xp(profileshape, TopAbs_FACE); xp.More(); xp.Next()) { - RevolMaker.Init(base, xp.Current(), supportface, axis, Mode, Modify); - RevolMaker.Perform(uptoface); - if (!RevolMaker.IsDone()) - throw Base::RuntimeError("ProfileBased: Up to face: Could not revolve the sketch!"); - - base = RevolMaker.Shape(); - if (Mode == RevolMode::None) - Mode = RevolMode::FuseWithBase; - } - - revol = base; + revol = revol.makeElementRevolution(baseshape, profileshape, axis, supportface, uptoface, nullptr, + static_cast(Mode), Modify, nullptr); } else { std::stringstream str; diff --git a/src/Mod/PartDesign/App/FeatureRevolution.h b/src/Mod/PartDesign/App/FeatureRevolution.h index c28fb6e8a1..c6ef8336b8 100644 --- a/src/Mod/PartDesign/App/FeatureRevolution.h +++ b/src/Mod/PartDesign/App/FeatureRevolution.h @@ -96,7 +96,7 @@ protected: * Generates a revolution of the input sketchshape and stores it in the given \a revol. */ void generateRevolution(TopoShape& revol, - const TopoDS_Shape& sketchshape, + const TopoShape& sketchshape, const gp_Ax1& ax1, const double angle, const double angle2, @@ -108,8 +108,8 @@ protected: * Generates a revolution of the input \a profileshape. * It will be a stand-alone solid created with BRepFeat_MakeRevol. */ - void generateRevolution(TopoDS_Shape& revol, - const TopoDS_Shape& baseshape, + void generateRevolution(TopoShape& revol, + const TopoShape& baseshape, const TopoDS_Shape& profileshape, const TopoDS_Face& supportface, const TopoDS_Face& uptoface, diff --git a/src/Mod/PartDesign/PartDesignTests/TestTopologicalNamingProblem.py b/src/Mod/PartDesign/PartDesignTests/TestTopologicalNamingProblem.py index c6a4228b5a..8705e07290 100644 --- a/src/Mod/PartDesign/PartDesignTests/TestTopologicalNamingProblem.py +++ b/src/Mod/PartDesign/PartDesignTests/TestTopologicalNamingProblem.py @@ -580,28 +580,62 @@ class TestTopologicalNamingProblem(unittest.TestCase): # See if we can turn those off, or try them on the other types? def testPartDesignElementMapRevolution(self): + # App.KeepTestDoc = True # Uncomment this if you want to keep the test document to examine + self.Doc.UseHasher = False # Arrange - body = self.Doc.addObject("PartDesign::Body", "Body") - sketch = self.Doc.addObject("Sketcher::SketchObject", "Sketch") - TestSketcherApp.CreateRectangleSketch(sketch, (1, 1), (2, 2)) # (pt), (w,l) - if body.Shape.ElementMapVersion == "": # Should be '4' as of Mar 2023. - return - # Act - revolution = self.Doc.addObject("PartDesign::Revolution", "Revolution") - revolution.ReferenceAxis = (self.Doc.getObject("Sketch"), ["V_Axis"]) - revolution.Profile = sketch + body = self.Doc.addObject('PartDesign::Body', 'Body') + sketch = self.Doc.addObject('Sketcher::SketchObject', 'Sketch') + TestSketcherApp.CreateRectangleSketch(sketch, (0, 1), (3, 2)) # (pt), (w,l) body.addObject(sketch) - body.addObject(revolution) self.Doc.recompute() - # Assert + pad = self.Doc.addObject('PartDesign::Pad', 'Pad') + pad.Profile = sketch + pad.Length = 3 + body.addObject(pad) + self.Doc.recompute() + + sketch2 = self.Doc.addObject('Sketcher::SketchObject', 'Sketch001') + TestSketcherApp.CreateRectangleSketch(sketch2, (2, -3), (1, 2)) # (pt), (w,l) + sketch2.AttachmentSupport = (pad, ["Face5"]) + sketch2.MapMode = 'FlatFace' + body.addObject(sketch2) + self.Doc.recompute() + revolution = self.Doc.addObject('PartDesign::Revolution', 'Revolution') + revolution.ReferenceAxis = (sketch2, ['V_Axis']) + revolution.Reversed = 1 + revolution.Profile = sketch2 + revolution.Angle=180 + revolution.Refine = True + body.addObject(revolution) + volume = (math.pi * 3 * 3 - math.pi * 2 * 2) * 2 / 2 + padVolume = 3 * 3 * 2 # 50.26548245743668 + # Act + self.Doc.recompute() + # Assert the Shape is correct self.assertEqual(len(body.Shape.childShapes()), 1) - self.assertEqual(body.Shape.childShapes()[0].ElementMapSize, 14) - self.assertEqual(revolution.Shape.ElementMapSize, 14) - self.assertEqual( - self.countFacesEdgesVertexes(revolution.Shape.ElementReverseMap), (4, 6, 4) - ) - volume = (math.pi * 3 * 3 - math.pi * 1 * 1) * 2 # 50.26548245743668 - self.assertAlmostEqual(revolution.Shape.Volume, volume) + self.assertAlmostEqual(pad.Shape.Volume, padVolume) + self.assertAlmostEqual(revolution.Shape.Volume, volume + padVolume) + # Assert the element map is correct + self.assertEqual(body.Shape.childShapes()[0].ElementMapSize, 46) + self.assertEqual(revolution.Shape.ElementMapSize, 46) + self.assertEqual(self.countFacesEdgesVertexes(revolution.Shape.ElementReverseMap), + (9, 21, 14)) + self.assertEqual( revolution.Shape.ElementReverseMap["Vertex9"][1].count(";"), 3) + self.assertEqual( revolution.Shape.ElementReverseMap["Face9"].count(";"), 16) + # Arrange for an UpToFace mode test + revolution.Type = 3 + revolution.UpToFace = (pad, ("Face4")) + revolution.Reversed = 1 + revolution.Midplane = 0 + volume = (math.pi * 3 * 3 - math.pi * 2 * 2) * 2 / 4 * 3 + # Act + self.Doc.recompute() + # Assert UpToFace shape is correct + self.assertAlmostEqual(revolution.Shape.Volume, volume + padVolume) + # Assert UpToFace element map is correct + self.assertEqual(self.countFacesEdgesVertexes(revolution.Shape.ElementReverseMap), + (8, 18, 12)) + self.assertEqual( revolution.Shape.ElementReverseMap["Face8"].count(";"), 7) def testPartDesignElementMapLoft(self): # Arrange