From 405bf03ce98c1ad5c22002acff9995fb69fd852d Mon Sep 17 00:00:00 2001 From: bgbsww Date: Tue, 20 Feb 2024 14:23:01 -0500 Subject: [PATCH] Clean and add tests for makeElementSolid --- src/Mod/Part/App/TopoShape.h | 9 +- src/Mod/Part/App/TopoShapeExpansion.cpp | 103 ++++++------------ tests/src/Mod/Part/App/TopoShapeExpansion.cpp | 30 ++++- 3 files changed, 65 insertions(+), 77 deletions(-) diff --git a/src/Mod/Part/App/TopoShape.h b/src/Mod/Part/App/TopoShape.h index f5b27c0eb3..80cdf1ca5d 100644 --- a/src/Mod/Part/App/TopoShape.h +++ b/src/Mod/Part/App/TopoShape.h @@ -1665,7 +1665,8 @@ public: * that multiple operations can be carried out for the same shape * in the same line of code. */ - TopoShape &makESolid(const std::vector &shapes, const char *op=nullptr); + // TODO: This does not appear to be called, and the implementation seems impossible +// TopoShape &makeElementSolid(const std::vector &shapes, const char *op=nullptr); /** Make a solid using shells or CompSolid * * @param shape: input shape of either a shell, a compound of shells, or a @@ -1679,7 +1680,7 @@ public: * that multiple operations can be carried out for the same shape * in the same line of code. */ - TopoShape &makESolid(const TopoShape &shape, const char *op=nullptr); + TopoShape &makeElementSolid(const TopoShape &shape, const char *op=nullptr); /** Make a solid using this shape * * @param op: optional string to be encoded into topo naming for indicating @@ -1688,8 +1689,8 @@ public: * @return The function returns a new solid using the shell or CompSolid * inside this shape. The shape itself is not modified. */ - TopoShape makESolid(const char *op=nullptr) const { - return TopoShape(0,Hasher).makESolid(*this,op); + TopoShape makeElementSolid(const char *op=nullptr) const { + return TopoShape(0,Hasher).makeElementSolid(*this,op); } diff --git a/src/Mod/Part/App/TopoShapeExpansion.cpp b/src/Mod/Part/App/TopoShapeExpansion.cpp index 45f0c3fd8e..bffe1c96cd 100644 --- a/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -50,6 +50,7 @@ #include #include #include +#include #include #include #include @@ -2641,99 +2642,61 @@ struct MapperThruSections: MapperMaker } }; -TopoShape &TopoShape::makESolid(const std::vector &shapes, const char *op) { - return makESolid(TopoShape().makECompound(shapes),op); -} +// TODO: This method does not appear to ever be called in the codebase, and it is probably +// broken, because using TopoShape() with no parameters means the result will not have an +// element Map. +//TopoShape& TopoShape::makeElementSolid(const std::vector& shapes, const char* op) +//{ +// return makeElementSolid(TopoShape().makeElementCompound(shapes), op); +//} -bool TopoShape::fixSolidOrientation() +TopoShape& TopoShape::makeElementSolid(const TopoShape& shape, const char* op) { - if (isNull()) - return false; - - if (shapeType() == TopAbs_SOLID) { - TopoDS_Solid solid = TopoDS::Solid(_Shape); - BRepLib::OrientClosedSolid(solid); - if (solid.IsEqual(_Shape)) - return false; - setShape(solid, false); - return true; + if (!op) { + op = Part::OpCodes::Solid; } - if (shapeType() == TopAbs_COMPOUND - || shapeType() == TopAbs_COMPSOLID) - { - auto shapes = getSubTopoShapes(); - bool touched = false; - for (auto &s : shapes) { - if (s.fixSolidOrientation()) - touched = true; - } - if (!touched) - return false; - - BRep_Builder builder; - if (shapeType() == TopAbs_COMPOUND) { - TopoDS_Compound comp; - builder.MakeCompound(comp); - for(auto &s : shapes) { - if (!s.isNull()) - builder.Add(comp, s.getShape()); - } - setShape(comp, false); - } else { - TopoDS_CompSolid comp; - builder.MakeCompSolid(comp); - for(auto &s : shapes) { - if (!s.isNull()) - builder.Add(comp, s.getShape()); - } - setShape(comp, false); - } - return true; + if (shape.isNull()) { + FC_THROWM(NullShapeException, "Null shape"); } - return false; -} - -TopoShape &TopoShape::makESolid(const TopoShape &shape, const char *op) { - if(!op) op = Part::OpCodes::Solid; - - if(shape.isNull()) - HANDLE_NULL_SHAPE; - - //first, if we were given a compsolid, try making a solid out of it + // first, if we were given a compsolid, try making a solid out of it TopoDS_CompSolid compsolid; - int count=0; - for(const auto &s : shape.getSubShapes(TopAbs_COMPSOLID)) { + int count = 0; + for (const auto& s : shape.getSubShapes(TopAbs_COMPSOLID)) { ++count; compsolid = TopoDS::CompSolid(s); - if (count > 1) + if (count > 1) { break; + } } if (count == 0) { - //no compsolids. Get shells... + // no compsolids. Get shells... BRepBuilderAPI_MakeSolid mkSolid; - count=0; - for (const auto &s : shape.getSubShapes(TopAbs_SHELL)) { + count = 0; + for (const auto& s : shape.getSubShapes(TopAbs_SHELL)) { ++count; mkSolid.Add(TopoDS::Shell(s)); } - if (count == 0)//no shells? - FC_THROWM(Base::CADKernelError,"No shells or compsolids found in shape"); + if (count == 0) { // no shells? + FC_THROWM(Base::CADKernelError, "No shells or compsolids found in shape"); + } - makEShape(mkSolid,shape,op); + makeElementShape(mkSolid, shape, op); TopoDS_Solid solid = TopoDS::Solid(_Shape); BRepLib::OrientClosedSolid(solid); setShape(solid, false); - - } else if (count == 1) { + } + else if (count == 1) { BRepBuilderAPI_MakeSolid mkSolid(compsolid); - makEShape(mkSolid,shape,op); - } else { // if (count > 1) - FC_THROWM(Base::CADKernelError,"Only one compsolid can be accepted. " - "Provided shape has more than one compsolid."); + makeElementShape(mkSolid, shape, op); + } + else { // if (count > 1) + FC_THROWM(Base::CADKernelError, + "Only one compsolid can be accepted. " + "Provided shape has more than one compsolid."); } return *this; } diff --git a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp index 96db4617d7..346f6379a2 100644 --- a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -1569,7 +1569,6 @@ TEST_F(TopoShapeExpansionTest, makeElementCut) "CUT;:H1:7,V);CUT;:H1:3c,E|Face6;:M;CUT;:H1:7,F;:U2;CUT;:H1:8,E);CUT;:H1:cb,F")); } -<<<<<<< HEAD TEST_F(TopoShapeExpansionTest, makeElementTransformWithoutMap) { // Arrange @@ -1665,6 +1664,31 @@ TEST_F(TopoShapeExpansionTest, makeElementGTransformWithMap) // Not testing _makeElementTransform as it is a thin wrapper that calls the same places as the four // preceding tests. -======= ->>>>>>> f6b3402577 (Toposhape/Part: Clean GeneralFuse, Fuse, Cut; add tests; tweak other tests) +TEST_F(TopoShapeExpansionTest, makeElementSolid) +{ + // Arrange + auto [cube1, cube2] = CreateTwoCubes(); + auto tr {gp_Trsf()}; + tr.SetTranslation(gp_Vec(gp_XYZ(-0.5, -0.5, 0))); + cube2.Move(TopLoc_Location(tr)); + TopoShape topoShape1 {cube1, 1L}; + TopoShape topoShape2 {cube2, 2L}; + // Act + TopExp_Explorer exp(topoShape1.getShape(), TopAbs_SHELL); + auto shell1 = exp.Current(); + exp.Init(topoShape2.getShape(), TopAbs_SHELL); + auto shell2 = exp.Current(); + TopoShape& topoShape3 = topoShape1.makeElementCompound({shell1, shell2}); + TopoShape& result = topoShape1.makeElementSolid(topoShape3); // Need the single parm form + auto elements = elementMap(result); + Base::BoundBox3d bb = result.getBoundBox(); + // Assert shape is correct + EXPECT_TRUE(PartTestHelpers::boxesMatch(bb, Base::BoundBox3d(0.0, -0.5, 0.0, 1.5, 1.0, 1.0))); + EXPECT_FLOAT_EQ(getVolume(result.getShape()), 2); + // Assert elementMap is correct + EXPECT_EQ(elements.size(), 52); + EXPECT_EQ(elements.count(IndexedName("Face", 1)), 1); + EXPECT_EQ(elements[IndexedName("Face", 1)], MappedName("Face1;SLD;:H1:4,F")); +} + // NOLINTEND(readability-magic-numbers,cppcoreguidelines-avoid-magic-numbers)