From 45c4aefaaec16b914678d0524ab1a97e093630c9 Mon Sep 17 00:00:00 2001 From: bgbsww Date: Fri, 10 May 2024 16:41:15 -0400 Subject: [PATCH] Rework makeElementChamfer to match current parms, and add PartDesign code for Chamfers --- src/Mod/Part/App/TopoShape.h | 17 +-- src/Mod/Part/App/TopoShapeExpansion.cpp | 23 ++-- src/Mod/Part/App/TopoShapePyImp.cpp | 4 +- src/Mod/PartDesign/App/FeatureChamfer.cpp | 59 +++++++++- src/Mod/PartDesign/App/FeatureDressUp.cpp | 102 +++++++++++++++- src/Mod/PartDesign/App/FeatureDressUp.h | 3 + src/Mod/PartDesign/App/FeaturePocket.cpp | 9 ++ .../TestTopologicalNamingProblem.py | 111 ++++++++++++++++-- tests/src/Mod/Part/App/TopoShapeExpansion.cpp | 2 +- 9 files changed, 298 insertions(+), 32 deletions(-) diff --git a/src/Mod/Part/App/TopoShape.h b/src/Mod/Part/App/TopoShape.h index a6894f9ab1..1bfab7ac14 100644 --- a/src/Mod/Part/App/TopoShape.h +++ b/src/Mod/Part/App/TopoShape.h @@ -208,10 +208,11 @@ enum class Flip flip }; -enum class AsAngle +enum class ChamferType { - no, - yes + equalDistance, + twoDistances, + distanceAngle }; enum class CheckScale @@ -2050,11 +2051,11 @@ public: */ TopoShape& makeElementChamfer(const TopoShape& source, const std::vector& edges, + ChamferType chamferType, double radius1, double radius2, const char* op = nullptr, - Flip flipDirection = Flip::none, - AsAngle asAngle = AsAngle::no); + Flip flipDirection = Flip::none); /* Make chamfer shape * * @param source: the source shape @@ -2067,14 +2068,14 @@ public: * @return Return the new shape. The TopoShape itself is not modified. */ TopoShape makeElementChamfer(const std::vector& edges, + ChamferType chamferType, double radius1, double radius2, const char* op = nullptr, - Flip flipDirection = Flip::none, - AsAngle asAngle = AsAngle::no) const + Flip flipDirection = Flip::none) const { return TopoShape(0, Hasher) - .makeElementChamfer(*this, edges, radius1, radius2, op, flipDirection, asAngle); + .makeElementChamfer(*this, edges, chamferType, radius1, radius2, op, flipDirection); } /** Make a new shape with transformation diff --git a/src/Mod/Part/App/TopoShapeExpansion.cpp b/src/Mod/Part/App/TopoShapeExpansion.cpp index 995d076198..e1bcb92168 100644 --- a/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -101,6 +101,7 @@ #include "FaceMaker.h" #include "Geometry.h" #include "BRepOffsetAPI_MakeOffsetFix.h" +#include "Base/Tools.h" #include #include @@ -4006,11 +4007,11 @@ TopoShape& TopoShape::makeElementFillet(const TopoShape& shape, TopoShape& TopoShape::makeElementChamfer(const TopoShape& shape, const std::vector& edges, + ChamferType chamferType, double radius1, double radius2, const char* op, - Flip flipDirection, - AsAngle asAngle) + Flip flipDirection) { if (!op) { op = Part::OpCodes::Chamfer; @@ -4038,11 +4039,19 @@ TopoShape& TopoShape::makeElementChamfer(const TopoShape& shape, else { face = shape.findAncestorShape(edge, TopAbs_FACE); } - if (asAngle == AsAngle::yes) { - mkChamfer.AddDA(radius1, radius2, TopoDS::Edge(edge), TopoDS::Face(face)); - } - else { - mkChamfer.Add(radius1, radius2, TopoDS::Edge(edge), TopoDS::Face(face)); + switch (chamferType) { + case ChamferType::equalDistance: // Equal distance + mkChamfer.Add(radius1, radius1, TopoDS::Edge(edge), TopoDS::Face(face)); + break; + case ChamferType::twoDistances: // Two distances + mkChamfer.Add(radius1, radius2, TopoDS::Edge(edge), TopoDS::Face(face)); + break; + case ChamferType::distanceAngle: // Distance and angle + mkChamfer.AddDA(radius1, + Base::toRadians(radius2), + TopoDS::Edge(edge), + TopoDS::Face(face)); + break; } } return makeElementShape(mkChamfer, shape, op); diff --git a/src/Mod/Part/App/TopoShapePyImp.cpp b/src/Mod/Part/App/TopoShapePyImp.cpp index bf6cd722ac..33e031e3d0 100644 --- a/src/Mod/Part/App/TopoShapePyImp.cpp +++ b/src/Mod/Part/App/TopoShapePyImp.cpp @@ -1856,6 +1856,7 @@ PyObject* TopoShapePy::makeFillet(PyObject *args) return nullptr; } +// TODO: Should this python interface support all three chamfer methods and not just two? PyObject* TopoShapePy::makeChamfer(PyObject *args) { // use two radii for all edges @@ -1876,7 +1877,7 @@ PyObject* TopoShapePy::makeChamfer(PyObject *args) PY_TRY { return Py::new_reference_to(shape2pyshape( - getTopoShapePtr()->makeElementChamfer(getPyShapes(obj), radius1, radius2))); + getTopoShapePtr()->makeElementChamfer(getPyShapes(obj), Part::ChamferType::twoDistances, radius1, radius2))); } PY_CATCH_OCC #else @@ -1910,6 +1911,7 @@ PyObject* TopoShapePy::makeChamfer(PyObject *args) #endif PyErr_Clear(); // use one radius for all edges + // TODO: Should this be using makeElementChamfer to support Toponaming fixes? double radius; if (PyArg_ParseTuple(args, "dO", &radius, &obj)) { try { diff --git a/src/Mod/PartDesign/App/FeatureChamfer.cpp b/src/Mod/PartDesign/App/FeatureChamfer.cpp index 34c3679be8..9f9a956099 100644 --- a/src/Mod/PartDesign/App/FeatureChamfer.cpp +++ b/src/Mod/PartDesign/App/FeatureChamfer.cpp @@ -114,6 +114,17 @@ App::DocumentObjectExecReturn *Chamfer::execute() return new App::DocumentObjectExecReturn(e.what()); } +#ifdef FC_USE_TNP_FIX + TopShape.setTransform(Base::Matrix4D()); + + auto edges = UseAllEdges.getValue() ? TopShape.getSubTopoShapes(TopAbs_EDGE) + : getContinuousEdges(TopShape); + + if (edges.empty()) { + return new App::DocumentObjectExecReturn( + QT_TRANSLATE_NOOP("Exception", "No edges specified")); + } +#else std::vector SubNames = std::vector(Base.getSubValues()); if (UseAllEdges.getValue()){ @@ -130,10 +141,10 @@ App::DocumentObjectExecReturn *Chamfer::execute() std::vector FaceNames; getContinuousEdges(TopShape, SubNames, FaceNames); - +#endif const int chamferType = ChamferType.getValue(); const double size = Size.getValue(); - const double size2 = Size2.getValue(); + double size2 = Size2.getValue(); const double angle = Angle.getValue(); const bool flipDirection = FlipDirection.getValue(); @@ -144,16 +155,57 @@ App::DocumentObjectExecReturn *Chamfer::execute() this->positionByBaseFeature(); +#ifdef FC_USE_TNP_FIX + if ( static_cast(chamferType) == Part::ChamferType::distanceAngle ) { + size2 = angle; + } +#else //If no element is selected, then we use a copy of previous feature. if (SubNames.empty()) { this->Shape.setValue(TopShape); return App::DocumentObject::StdReturn; } - // create an untransformed copy of the basefeature shape Part::TopoShape baseShape(TopShape); baseShape.setTransform(Base::Matrix4D()); +#endif try { +#ifdef FC_USE_TNP_FIX + TopoShape shape(0); + shape.makeElementChamfer(TopShape, + edges, + static_cast(chamferType), + size, + size2, + nullptr, + flipDirection ? Part::Flip::flip : Part::Flip::none); + if (shape.isNull()) { + return new App::DocumentObjectExecReturn( + QT_TRANSLATE_NOOP("Exception", "Failed to create chamfer")); + } + + TopTools_ListOfShape aLarg; + aLarg.Append(TopShape.getShape()); + bool failed = false; + if (!BRepAlgo::IsValid(aLarg, shape.getShape(), Standard_False, Standard_False)) { + ShapeFix_ShapeTolerance aSFT; + aSFT.LimitTolerance(shape.getShape(), + Precision::Confusion(), + Precision::Confusion(), + TopAbs_SHAPE); + } + if (!failed) { + shape = refineShapeIfActive(shape); + shape = getSolid(shape); + } + this->Shape.setValue(shape); + if (failed) { + return new App::DocumentObjectExecReturn( + QT_TRANSLATE_NOOP("Exception", "Resulting shape is invalid")); + } + return App::DocumentObject::StdReturn; + +#else BRepFilletAPI_MakeChamfer mkChamfer(baseShape.getShape()); TopTools_IndexedDataMapOfShapeListOfShape mapEdgeFace; @@ -225,6 +277,7 @@ App::DocumentObjectExecReturn *Chamfer::execute() shape = refineShapeIfActive(shape); this->Shape.setValue(getSolid(shape)); return App::DocumentObject::StdReturn; +#endif } catch (Standard_Failure& e) { return new App::DocumentObjectExecReturn(e.GetMessageString()); diff --git a/src/Mod/PartDesign/App/FeatureDressUp.cpp b/src/Mod/PartDesign/App/FeatureDressUp.cpp index 5e801194f7..e91e8eb249 100644 --- a/src/Mod/PartDesign/App/FeatureDressUp.cpp +++ b/src/Mod/PartDesign/App/FeatureDressUp.cpp @@ -29,13 +29,19 @@ #include #include #include +#include #endif -#include -#include + +#include #include "FeatureDressUp.h" +#include +#include +#include +#include "Mod/Part/App/TopoShapeMapper.h" +FC_LOG_LEVEL_INIT("PartDesign",true,true) using namespace PartDesign; @@ -169,14 +175,102 @@ void DressUp::getContinuousEdges(Part::TopoShape TopShape, std::vector< std::str } } +std::vector DressUp::getContinuousEdges(const TopoShape& shape) +{ + std::vector ret; + std::unordered_set shapeSet; + + auto addEdge = [&](const TopoDS_Shape& subshape, const std::string& ref) { + if (!shapeSet.insert(subshape).second) { + return; + } + + auto faces = shape.findAncestorsShapes(subshape, TopAbs_FACE); + if (faces.size() != 2) { + FC_WARN(getFullName() << ": skip edge " << ref << " with less two attaching faces"); + return; + } + const TopoDS_Shape& face1 = faces.front(); + const TopoDS_Shape& face2 = faces.back(); + GeomAbs_Shape cont = + BRep_Tool::Continuity(TopoDS::Edge(subshape), TopoDS::Face(face1), TopoDS::Face(face2)); + if (cont != GeomAbs_C0) { + FC_WARN(getFullName() << ": skip edge " << ref << " that is not C0 continuous"); + return; + } + ret.push_back(subshape); + }; + + for (const auto& v : Base.getShadowSubs()) { + TopoDS_Shape subshape; + const auto& ref = v.first.size() ? v.first : v.second; + subshape = shape.getSubShape(ref.c_str(), true); + if (subshape.IsNull()) { + FC_THROWM(Base::CADKernelError, "Invalid edge link: " << v.second); + } + + if (subshape.ShapeType() == TopAbs_EDGE) { + addEdge(subshape, ref); + } + else if (subshape.ShapeType() == TopAbs_FACE || subshape.ShapeType() == TopAbs_WIRE) { + for (TopExp_Explorer exp(subshape, TopAbs_EDGE); exp.More(); exp.Next()) { + addEdge(exp.Current(), std::string()); + } + } + else { + FC_WARN(getFullName() << ": skip invalid shape '" << ref << "' with type " + << TopoShape::shapeName(subshape.ShapeType())); + } + } + return ret; +} + +std::vector DressUp::getFaces(const TopoShape& shape) +{ + std::vector ret; + const auto& vals = Base.getSubValues(); + const auto& subs = Base.getShadowSubs(); + size_t i = 0; + for (auto& val : vals) { + if (!boost::starts_with(val, "Face")) { + continue; + } + auto& sub = subs[i++]; + auto& ref = sub.first.size() ? sub.first : val; + TopoShape subshape; + try { + subshape = shape.getSubTopoShape(ref.c_str()); + } + catch (...) { + } + + if (subshape.isNull()) { + FC_ERR(getFullName() << ": invalid face reference '" << ref << "'"); + throw Part::NullShapeException("Invalid Invalid face link"); + } + + if (subshape.shapeType() != TopAbs_FACE) { + FC_WARN(getFullName() << ": skip invalid shape '" << ref << "' with type " + << subshape.shapeName()); + continue; + } + ret.push_back(subshape); + } + return ret; +} void DressUp::onChanged(const App::Property* prop) { // the BaseFeature property should track the Base and vice-versa as long as // the feature is inside a body (aka BaseFeature is nonzero) if (prop == &BaseFeature) { - if (BaseFeature.getValue() && Base.getValue() != BaseFeature.getValue()) { - Base.setValue (BaseFeature.getValue()); + if (BaseFeature.getValue() + && Base.getValue() + && Base.getValue() != BaseFeature.getValue()) { + + auto subs = Base.getSubValues(false); + auto shadows = Base.getShadowSubs(); + Base.setValue (BaseFeature.getValue(),std::move(subs),std::move(shadows)); } } else if (prop == &Base) { // track the vice-versa changes diff --git a/src/Mod/PartDesign/App/FeatureDressUp.h b/src/Mod/PartDesign/App/FeatureDressUp.h index c01f1fadb2..a679b881ff 100644 --- a/src/Mod/PartDesign/App/FeatureDressUp.h +++ b/src/Mod/PartDesign/App/FeatureDressUp.h @@ -60,7 +60,10 @@ public: void getContinuousEdges(Part::TopoShape, std::vector< std::string >&); // add argument to return the selected face that edges were derived from void getContinuousEdges(Part::TopoShape, std::vector< std::string >&, std::vector< std::string >&); + // Todo: Post-TNP the above two versions should be able to be factored out. + std::vector getContinuousEdges(const TopoShape &shape); + std::vector getFaces(const TopoShape &shape); void getAddSubShape(Part::TopoShape &addShape, Part::TopoShape &subShape) override; protected: diff --git a/src/Mod/PartDesign/App/FeaturePocket.cpp b/src/Mod/PartDesign/App/FeaturePocket.cpp index 9cd2bf7c69..b772216189 100644 --- a/src/Mod/PartDesign/App/FeaturePocket.cpp +++ b/src/Mod/PartDesign/App/FeaturePocket.cpp @@ -70,6 +70,14 @@ Pocket::Pocket() App::DocumentObjectExecReturn *Pocket::execute() { +#ifdef FC_USE_TNP_FIX + // MakeFace|MakeFuse: because we want a solid. + // InverseDirection: to inverse the auto detected extrusion direction for + // backward compatibility to upstream + ExtrudeOptions options(ExtrudeOption::MakeFace | ExtrudeOption::MakeFuse + | ExtrudeOption::InverseDirection); + return buildExtrusion(options); +#else // Handle legacy features, these typically have Type set to 3 (previously NULL, now UpToFace), // empty FaceName (because it didn't exist) and a value for Length if (std::string(Type.getValueAsString()) == "UpToFace" && @@ -248,4 +256,5 @@ App::DocumentObjectExecReturn *Pocket::execute() catch (Base::Exception& e) { return new App::DocumentObjectExecReturn(e.what()); } +#endif } diff --git a/src/Mod/PartDesign/PartDesignTests/TestTopologicalNamingProblem.py b/src/Mod/PartDesign/PartDesignTests/TestTopologicalNamingProblem.py index 5324e60f5d..8ea9260656 100644 --- a/src/Mod/PartDesign/PartDesignTests/TestTopologicalNamingProblem.py +++ b/src/Mod/PartDesign/PartDesignTests/TestTopologicalNamingProblem.py @@ -23,6 +23,7 @@ """ Tests related to the Topological Naming Problem """ +import math import unittest import FreeCAD as App @@ -102,13 +103,10 @@ class TestTopologicalNamingProblem(unittest.TestCase): self.assertTrue(self.Pad.isValid()) self.assertTrue(self.Pad1.isValid()) - # Todo switch to actually asserting this and remove the printed lines as soon as - # the main branch is capable of passing this test. - # self.assertTrue(self.Pad2.isValid()) - if self.Pad2.isValid(): - print("Topological Naming Problem is not present.") + if self.Body.Shape.ElementMapVersion == "": # Should be '4' as of Mar 2023. + self.assertFalse(self.Pad2.isValid()) # TNP problem is present without ElementMaps else: - print("TOPOLOGICAL NAMING PROBLEM IS PRESENT.") + self.assertTrue(self.Pad2.isValid()) # TNP problem is not present with ElementMaps def testPartDesignElementMapPad(self): """ Test that padding a sketch results in a correct element map. Note that comprehensive testing @@ -502,7 +500,8 @@ class TestTopologicalNamingProblem(unittest.TestCase): self.assertEqual(body.Shape.ElementMapSize,30) self.assertEqual(sketch.Shape.ElementMapSize,12) self.assertEqual(pad.Shape.ElementMapSize,30) - # Todo: Assert that the names in the ElementMap are good; in particular that they are hashed with a # starting + # Todo: Assert that the names in the ElementMap are good; + # in particular that they are hashed with a # starting def testPartDesignElementMapRevolution(self): # Arrange @@ -1018,7 +1017,8 @@ class TestTopologicalNamingProblem(unittest.TestCase): doc.addObject('PartDesign::Body', 'TNP_Test_Body_Second') doc.getObject('TNP_Test_Body_Second').Label = 'TNP_Test_Body_Second' doc.recompute() - obj = doc.getObject('TNP_Test_Body_Second').newObject('PartDesign::ShapeBinder', 'ShapeBinder') + obj = doc.getObject('TNP_Test_Body_Second').newObject('PartDesign::ShapeBinder', + 'ShapeBinder') obj.Support = (doc.getObject("TNP_Test_Body_SubShape"), [u'Face6']) doc.recompute() doc.getObject('TNP_Test_Body_Second').newObject('Sketcher::SketchObject', 'Sketch001') @@ -1237,6 +1237,101 @@ class TestTopologicalNamingProblem(unittest.TestCase): self.assertEqual(self.Body.Shape.BoundBox.YMax, 25.2) self.assertEqual(self.Body.Shape.BoundBox.ZMax, 20) + def testPartDesignTNPChamfer(self): + """ Test Chamfer """ + # Arrange + doc = self.Doc + body = self.Doc.addObject('PartDesign::Body', 'Body') + box = self.Doc.addObject('PartDesign::AdditiveBox', 'Box') + body.addObject(box) + self.Doc.recompute() + volume1 = body.Shape.Volume + chamfer = self.Doc.addObject('PartDesign::Chamfer', 'Chamfer') + chamfer.Base = (box, ['Edge1', + 'Edge5', + 'Edge7', + ]) + chamfer.Size = 1 + body.addObject(chamfer) + self.Doc.recompute() + volume2 = body.Shape.Volume + + doc.Body.newObject('Sketcher::SketchObject', 'Sketch') + doc.Sketch.AttachmentSupport = (chamfer, "Face8") + # doc.Sketch.AttachmentOffset = App.Placement( + # App.Vector(0.0000000000, 2.0000000000, 0.0000000000), + # App.Rotation(0.0000000000, 0.0000000000, 0.0000000000)) + doc.Sketch.MapMode = 'FlatFace' + doc.recompute() + + x1, x2, y1, y2 = 10 / math.sqrt(2) - math.sqrt(2), 10 / math.sqrt(2) + math.sqrt(2), 6, 11 + geoList = [] + geoList.append( + Part.LineSegment(App.Vector(x1, y1, 0.0 ), + App.Vector(x1, y2, 0.0 ))) + geoList.append( + Part.LineSegment(App.Vector(x1, y2, 0.0), + App.Vector(x2, y2, 0.0))) + geoList.append( + Part.LineSegment(App.Vector(x2, y2, 0.0), + App.Vector(x2, y1, 0.0))) + geoList.append( + Part.LineSegment(App.Vector(x2, y1, 0.0), + App.Vector(x1, y1, 0.0))) + doc.Sketch.addGeometry(geoList, False) + del geoList + + constraintList = [] + constraintList.append(Sketcher.Constraint('Coincident', 0, 2, 1, 1)) + constraintList.append(Sketcher.Constraint('Coincident', 1, 2, 2, 1)) + constraintList.append(Sketcher.Constraint('Coincident', 2, 2, 3, 1)) + constraintList.append(Sketcher.Constraint('Coincident', 3, 2, 0, 1)) + constraintList.append(Sketcher.Constraint('Horizontal', 0)) + constraintList.append(Sketcher.Constraint('Horizontal', 2)) + constraintList.append(Sketcher.Constraint('Vertical', 1)) + constraintList.append(Sketcher.Constraint('Vertical', 3)) + doc.Sketch.addConstraint(constraintList) + del constraintList + body.addObject(doc.Sketch) + + pocket = self.Doc.addObject('PartDesign::Pocket', 'Pocket') + pocket.Type = "Length" + # pocket.Length2 = 2 + pocket.Length = 3 + pocket.Direction = App.Vector(-0.710000000,0.7100000000, 0.0000000000) + pocket.Profile = doc.Sketch + pocket.Reversed = True + body.addObject(pocket) + self.Doc.recompute() + volume3 = body.Shape.Volume + # Change the chamfered edges, potentially triggering TNP + chamfer.Base = (box, ['Edge5', + 'Edge7', + ]) + self.Doc.recompute() + volume4 = body.Shape.Volume + # Assert + if body.Shape.ElementMapVersion == "": # Skip without element maps. + return + reverseMap = body.Shape.childShapes()[0].ElementReverseMap + faces = [name for name in reverseMap.keys() if name.startswith("Face")] + edges = [name for name in reverseMap.keys() if name.startswith("Edge")] + vertexes = [name for name in reverseMap.keys() if name.startswith("Vertex")] + self.assertEqual(len(body.Shape.childShapes()), 1) + self.assertEqual(body.Shape.childShapes()[0].ElementMapSize, 62) + self.assertEqual(len(reverseMap),62) + self.assertEqual(len(faces),12) + self.assertEqual(len(edges),30) + self.assertEqual(len(vertexes),20) + boxVolume = 10 * 10 * 10 + chamferVolume = 1 * 1 * 0.5 * 10 + # cut area is rectangle with sqrt(2) as one side minus 2 isosceles right triangles + cutArea = (2 * math.sqrt(2)) * 3 - ((math.sqrt(2)/2 * math.sqrt(2)/2)/2)*2 + cutVolume = cutArea * 4 # height is 4 ( 11-6 with a limit of 10 from the box ) + self.assertAlmostEqual(volume1, boxVolume ) + self.assertAlmostEqual(volume2, boxVolume - 3 * chamferVolume) + self.assertAlmostEqual(volume3, boxVolume - 3 * chamferVolume - cutVolume, 4) + self.assertAlmostEqual(volume4, boxVolume - 2 * chamferVolume - cutVolume, 4) def create_t_sketch(self): self.Doc.getObject('Body').newObject('Sketcher::SketchObject', 'Sketch') diff --git a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp index 810bea6c70..20c9d652f9 100644 --- a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -2103,7 +2103,7 @@ TEST_F(TopoShapeExpansionTest, makeElementChamfer) TopoShape cube1TS {cube1, 1L}; auto edges = cube1TS.getSubTopoShapes(TopAbs_EDGE); // Act - cube1TS.makeElementChamfer({cube1TS}, edges, .05, .05); + cube1TS.makeElementChamfer({cube1TS}, edges, Part::ChamferType::equalDistance, .05, .05); auto elements = elementMap(cube1TS); // Assert shape is correct EXPECT_EQ(cube1TS.countSubElements("Wire"), 26);