From 9e2a8343a0f1c6099a6d944fbf42e7a3a99da23d Mon Sep 17 00:00:00 2001 From: bgbsww Date: Mon, 20 May 2024 16:17:38 -0400 Subject: [PATCH] Toponaming: Cleanup verified face calls --- src/Mod/PartDesign/App/FeatureGroove.cpp | 2 +- src/Mod/PartDesign/App/FeatureHelix.cpp | 2 +- src/Mod/PartDesign/App/FeatureHole.cpp | 2 +- src/Mod/PartDesign/App/FeatureRevolution.cpp | 2 +- src/Mod/PartDesign/App/FeatureSketchBased.cpp | 264 +++++++++--------- .../TestTopologicalNamingProblem.py | 22 ++ 6 files changed, 158 insertions(+), 136 deletions(-) diff --git a/src/Mod/PartDesign/App/FeatureGroove.cpp b/src/Mod/PartDesign/App/FeatureGroove.cpp index b1db6d5d8d..f77e6a84b7 100644 --- a/src/Mod/PartDesign/App/FeatureGroove.cpp +++ b/src/Mod/PartDesign/App/FeatureGroove.cpp @@ -260,7 +260,7 @@ App::DocumentObjectExecReturn *Groove::execute() TopoShape sketchshape; try { - sketchshape = getVerifiedFace(); + sketchshape = getTopoShapeVerifiedFace(); } catch (const Base::Exception& e) { return new App::DocumentObjectExecReturn(e.what()); } diff --git a/src/Mod/PartDesign/App/FeatureHelix.cpp b/src/Mod/PartDesign/App/FeatureHelix.cpp index a50e2cdf31..55e53d8e8d 100644 --- a/src/Mod/PartDesign/App/FeatureHelix.cpp +++ b/src/Mod/PartDesign/App/FeatureHelix.cpp @@ -170,7 +170,7 @@ App::DocumentObjectExecReturn* Helix::execute() return new App::DocumentObjectExecReturn(QT_TRANSLATE_NOOP("Exception", "Error: unsupported mode")); } - TopoDS_Shape sketchshape; + TopoDS_Shape sketchshape; // Fixme: Should this be TopoShape here and below? try { sketchshape = getVerifiedFace(); } diff --git a/src/Mod/PartDesign/App/FeatureHole.cpp b/src/Mod/PartDesign/App/FeatureHole.cpp index dd5b6ba2a1..440761fa4d 100644 --- a/src/Mod/PartDesign/App/FeatureHole.cpp +++ b/src/Mod/PartDesign/App/FeatureHole.cpp @@ -1654,7 +1654,7 @@ App::DocumentObjectExecReturn* Hole::execute() { TopoShape profileshape; try { - profileshape = getVerifiedFace(); + profileshape = getTopoShapeVerifiedFace(); } catch (const Base::Exception& e) { return new App::DocumentObjectExecReturn(e.what()); diff --git a/src/Mod/PartDesign/App/FeatureRevolution.cpp b/src/Mod/PartDesign/App/FeatureRevolution.cpp index 9c9903b441..306a04fe93 100644 --- a/src/Mod/PartDesign/App/FeatureRevolution.cpp +++ b/src/Mod/PartDesign/App/FeatureRevolution.cpp @@ -99,7 +99,7 @@ App::DocumentObjectExecReturn* Revolution::execute() TopoShape sketchshape; try { - sketchshape = getVerifiedFace(); + sketchshape = getTopoShapeVerifiedFace(); } catch (const Base::Exception& e) { return new App::DocumentObjectExecReturn(e.what()); diff --git a/src/Mod/PartDesign/App/FeatureSketchBased.cpp b/src/Mod/PartDesign/App/FeatureSketchBased.cpp index 08cebf0dcf..cf5b699b39 100644 --- a/src/Mod/PartDesign/App/FeatureSketchBased.cpp +++ b/src/Mod/PartDesign/App/FeatureSketchBased.cpp @@ -169,121 +169,6 @@ Part::Feature* ProfileBased::getVerifiedObject(bool silent) const { return static_cast(result); } - -#ifdef FC_USE_TNP_FIX -TopoShape ProfileBased::getProfileShape() const -{ - TopoShape shape; - const auto& subs = Profile.getSubValues(); - auto profile = Profile.getValue(); - if (subs.empty()) { - shape = Part::Feature::getTopoShape(profile); - } - else { - std::vector shapes; - for (auto& sub : subs) { - shapes.push_back( - Part::Feature::getTopoShape(profile, sub.c_str(), /* needSubElement */ true)); - } - shape = TopoShape(shape.Tag).makeElementCompound(shapes); - } - if (shape.isNull()) { - throw Part::NullShapeException("Linked shape object is empty"); - } - return shape; -} -#else -Part::TopoShape ProfileBased::getProfileShape() const -{ - auto shape = getTopoShape(Profile.getValue()); - if (!shape.isNull() && !Profile.getSubValues().empty()) { - std::vector shapes; - for (auto& sub : Profile.getSubValues(true)) - shapes.emplace_back(shape.getSubShape(sub.c_str())); - shape = Part::TopoShape().makeCompound(shapes); - } - return shape; -} -#endif -// TODO: Toponaming April 2024 Deprecated in favor of TopoShape method. Remove when possible. -TopoDS_Shape ProfileBased::getVerifiedFace(bool silent) const { - - App::DocumentObject* result = Profile.getValue(); - const char* err = nullptr; - std::string _err; - - if (!result) { - err = "No profile linked"; - } - else if (AllowMultiFace.getValue()) { - try { - auto shape = getProfileShape(); - if (shape.isNull()) - err = "Linked shape object is empty"; - else { - auto faces = shape.getSubTopoShapes(TopAbs_FACE); - if (faces.empty()) { - if (!shape.hasSubShape(TopAbs_WIRE)) - shape = shape.makeWires(); - if (shape.hasSubShape(TopAbs_WIRE)) - shape = shape.makeFace(nullptr, "Part::FaceMakerBullseye"); - else - err = "Cannot make face from profile"; - } - else if (faces.size() == 1) - shape = faces.front(); - else - shape = TopoShape().makeCompound(faces); - } - if (!err) - return shape.getShape(); - } - catch (Standard_Failure& e) { - _err = e.GetMessageString(); - err = _err.c_str(); - } - } - else { - if (result->isDerivedFrom()) { - - auto wires = getProfileWires(); - return Part::FaceMakerCheese::makeFace(wires); - } - else if (result->isDerivedFrom()) { - if (Profile.getSubValues().empty()) - err = "Linked object has no subshape specified"; - else { - - const Part::TopoShape& shape = Profile.getValue()->Shape.getShape(); - TopoDS_Shape sub = shape.getSubShape(Profile.getSubValues()[0].c_str()); - if (sub.ShapeType() == TopAbs_FACE) - return TopoDS::Face(sub); - else if (sub.ShapeType() == TopAbs_WIRE) { - - auto wire = TopoDS::Wire(sub); - if (!wire.Closed()) - err = "Linked wire is not closed"; - else { - BRepBuilderAPI_MakeFace mk(wire); - mk.Build(); - return TopoDS::Face(mk.Shape()); - } - } - else - err = "Linked Subshape cannot be used"; - } - } - else - err = "Linked object is neither Sketch, Part2DObject or Part::Feature"; - } - - if (!silent && err) { - throw Base::RuntimeError(err); - } - - return TopoDS_Face(); -} - TopoShape ProfileBased::getTopoShapeVerifiedFace(bool silent, [[maybe_unused]]bool doFit, // TODO: Remove parameter bool allowOpen, @@ -400,17 +285,17 @@ TopoShape ProfileBased::getTopoShapeVerifiedFace(bool silent, } // Toponaming April 2024: This appears to be new feature, not TNP: - // if (doFit && (std::abs(Fit.getValue()) > Precision::Confusion() - // || std::abs(InnerFit.getValue()) > Precision::Confusion())) { - // - // if (!shape.isNull()) - // shape = shape.makEOffsetFace(Fit.getValue(), - // InnerFit.getValue(), - // static_cast(FitJoin.getValue()), - // static_cast(InnerFitJoin.getValue())); - // if (!openshape.isNull()) - // openshape.makEOffset2D(Fit.getValue()); - // } +// if (doFit && (std::abs(Fit.getValue()) > Precision::Confusion() +// || std::abs(InnerFit.getValue()) > Precision::Confusion())) { +// +// if (!shape.isNull()) +// shape = shape.makeElementOffsetFace(Fit.getValue(), +// InnerFit.getValue(), +// static_cast(FitJoin.getValue()), +// static_cast(InnerFitJoin.getValue())); +// if (!openshape.isNull()) +// openshape.makeElementOffset2D(Fit.getValue()); +// } if (!openshape.isNull()) { if (shape.isNull()) { @@ -443,6 +328,121 @@ TopoShape ProfileBased::getTopoShapeVerifiedFace(bool silent, } } +// TODO: Toponaming April 2024 Deprecated in favor of TopoShape method. Remove when possible. +TopoDS_Shape ProfileBased::getVerifiedFace(bool silent) const { + + App::DocumentObject* result = Profile.getValue(); + const char* err = nullptr; + std::string _err; + + if (!result) { + err = "No profile linked"; + } + else if (AllowMultiFace.getValue()) { + try { + auto shape = getProfileShape(); + if (shape.isNull()) + err = "Linked shape object is empty"; + else { + auto faces = shape.getSubTopoShapes(TopAbs_FACE); + if (faces.empty()) { + if (!shape.hasSubShape(TopAbs_WIRE)) + shape = shape.makeWires(); + if (shape.hasSubShape(TopAbs_WIRE)) + shape = shape.makeFace(nullptr, "Part::FaceMakerBullseye"); + else + err = "Cannot make face from profile"; + } + else if (faces.size() == 1) + shape = faces.front(); + else + shape = TopoShape().makeCompound(faces); + } + if (!err) + return shape.getShape(); + } + catch (Standard_Failure& e) { + _err = e.GetMessageString(); + err = _err.c_str(); + } + } + else { + if (result->isDerivedFrom()) { + + auto wires = getProfileWires(); + return Part::FaceMakerCheese::makeFace(wires); + } + else if (result->isDerivedFrom()) { + if (Profile.getSubValues().empty()) + err = "Linked object has no subshape specified"; + else { + + const Part::TopoShape& shape = Profile.getValue()->Shape.getShape(); + TopoDS_Shape sub = shape.getSubShape(Profile.getSubValues()[0].c_str()); + if (sub.ShapeType() == TopAbs_FACE) + return TopoDS::Face(sub); + else if (sub.ShapeType() == TopAbs_WIRE) { + + auto wire = TopoDS::Wire(sub); + if (!wire.Closed()) + err = "Linked wire is not closed"; + else { + BRepBuilderAPI_MakeFace mk(wire); + mk.Build(); + return TopoDS::Face(mk.Shape()); + } + } + else + err = "Linked Subshape cannot be used"; + } + } + else + err = "Linked object is neither Sketch, Part2DObject or Part::Feature"; + } + + if (!silent && err) { + throw Base::RuntimeError(err); + } + + return TopoDS_Face(); +} + +#ifdef FC_USE_TNP_FIX +TopoShape ProfileBased::getProfileShape() const +{ + TopoShape shape; + const auto& subs = Profile.getSubValues(); + auto profile = Profile.getValue(); + if (subs.empty()) { + shape = Part::Feature::getTopoShape(profile); + } + else { + std::vector shapes; + for (auto& sub : subs) { + shapes.push_back( + Part::Feature::getTopoShape(profile, sub.c_str(), /* needSubElement */ true)); + } + shape = TopoShape(shape.Tag).makeElementCompound(shapes); + } + if (shape.isNull()) { + throw Part::NullShapeException("Linked shape object is empty"); + } + return shape; +} +#else +Part::TopoShape ProfileBased::getProfileShape() const +{ + auto shape = getTopoShape(Profile.getValue()); + if (!shape.isNull() && !Profile.getSubValues().empty()) { + std::vector shapes; + for (auto& sub : Profile.getSubValues(true)) + shapes.emplace_back(shape.getSubShape(sub.c_str())); + shape = Part::TopoShape().makeCompound(shapes); + } + return shape; +} +#endif + // TODO: Toponaming April 2024 Deprecated in favor of TopoShape method. Remove when possible. std::vector ProfileBased::getProfileWires() const { @@ -920,13 +920,13 @@ void ProfileBased::addOffsetToFace(TopoShape& upToFace, const gp_Dir& dir, doubl double ProfileBased::getThroughAllLength() const { - TopoDS_Shape profileshape; + TopoShape profileshape; TopoShape base; - profileshape = getVerifiedFace(); + profileshape = getTopoShapeVerifiedFace(); base = getBaseTopoShape(); Bnd_Box box; BRepBndLib::Add(base.getShape(), box); - BRepBndLib::Add(profileshape, box); + BRepBndLib::Add(profileshape.getShape(), box); box.SetGap(0.0); // The diagonal of the bounding box, plus 1% extra to eliminate risk of // co-planar issues, gives a length that is guaranteed to go through all. @@ -1297,11 +1297,11 @@ double ProfileBased::getReversedAngle(const Base::Vector3d & b, const Base::Vect { try { Part::Feature* obj = getVerifiedObject(); - TopoDS_Shape sketchshape = getVerifiedFace(); + TopoShape sketchshape = getTopoShapeVerifiedFace(); // get centre of gravity of the sketch face GProp_GProps props; - BRepGProp::SurfaceProperties(sketchshape, props); + BRepGProp::SurfaceProperties(sketchshape.getShape(), props); gp_Pnt cog = props.CentreOfMass(); Base::Vector3d p_cog(cog.X(), cog.Y(), cog.Z()); // get direction to cog from its projection on the revolve axis @@ -1479,7 +1479,7 @@ Base::Vector3d ProfileBased::getProfileNormal() const { // For newer version, do not do fitting, as it may flip the face normal for // some reason. - TopoShape shape = getVerifiedFace(true); //, _ProfileBasedVersion.getValue() <= 0); + TopoShape shape = getTopoShapeVerifiedFace(true); //, _ProfileBasedVersion.getValue() <= 0); gp_Pln pln; if (shape.findPlane(pln)) { diff --git a/src/Mod/PartDesign/PartDesignTests/TestTopologicalNamingProblem.py b/src/Mod/PartDesign/PartDesignTests/TestTopologicalNamingProblem.py index 38d6485808..159b8adfa0 100644 --- a/src/Mod/PartDesign/PartDesignTests/TestTopologicalNamingProblem.py +++ b/src/Mod/PartDesign/PartDesignTests/TestTopologicalNamingProblem.py @@ -108,6 +108,28 @@ class TestTopologicalNamingProblem(unittest.TestCase): else: self.assertTrue(self.Pad2.isValid()) # TNP problem is not present with ElementMaps + def testPartDesignElementMapSketch(self): + """ Test that creating a sketch results in a correct element map. """ + # Arrange + body = self.Doc.addObject('PartDesign::Body', 'Body') + sketch = self.Doc.addObject('Sketcher::SketchObject', 'SketchPad') + body.addObject(sketch) + TestSketcherApp.CreateRectangleSketch(sketch, (0, 0), (1, 1)) + # Act + self.Doc.recompute() + if body.Shape.ElementMapVersion == "": # Should be '4' as of Mar 2023. + return + reverseMap = sketch.Shape.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")] + # Assert + self.assertEqual(sketch.Shape.ElementMapSize,9) + self.assertEqual(len(reverseMap),9) + self.assertEqual(len(faces),1) + self.assertEqual(len(edges),4) + self.assertEqual(len(vertexes),4) + def testPartDesignElementMapPad(self): """ Test that padding a sketch results in a correct element map. Note that comprehensive testing of the geometric functionality of the Pad is in TestPad.py """