From 2e772bab1fd79af4fb980397cc321cd7273b3a35 Mon Sep 17 00:00:00 2001 From: "Zheng, Lei" Date: Tue, 13 Feb 2024 13:41:56 -0500 Subject: [PATCH 1/3] Toponaming/Part: move in makeElementDraft --- src/Mod/Part/App/TopoShape.h | 37 +++++++++++++++++++++++ src/Mod/Part/App/TopoShapeExpansion.cpp | 39 +++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/src/Mod/Part/App/TopoShape.h b/src/Mod/Part/App/TopoShape.h index 0aaa5d9fe7..e3673bbdb5 100644 --- a/src/Mod/Part/App/TopoShape.h +++ b/src/Mod/Part/App/TopoShape.h @@ -965,6 +965,43 @@ public: { return TopoShape(0, Hasher).makeElementBoolean(maker, *this, op, tol); } + /* Make draft shape + * + * @param source: the source shape + * @param faces: the faces of the source shape to make draft faces + * @param pullDirection: the pulling direction for making the draft + * @param angle: the angle of the draft + * @param neutralPlane: the neutral plane used as a reference to decide pulling direction + * @param retry: whether to keep going by skipping faces that failed to create draft + * @param op: optional string to be encoded into topo naming for indicating + * the operation + * + * @return The original content of this TopoShape is discarded and replaced + * with the new shape. The function returns the TopoShape itself as + * a self reference so that multiple operations can be carried out + * for the same shape in the same line of code. + */ + TopoShape &makeElementDraft(const TopoShape &source, const std::vector &faces, + const gp_Dir &pullDirection, double angle, const gp_Pln &neutralPlane, + bool retry=true, const char *op=nullptr); + /* Make draft shape + * + * @param source: the source shape + * @param faces: the faces of the source shape to make draft faces + * @param pullDirection: the pulling direction for making the draft + * @param angle: the angle of the draft + * @param neutralPlane: the neutral plane used as a reference to decide pulling direction + * @param retry: whether to keep going by skipping faces that failed to create draft + * @param op: optional string to be encoded into topo naming for indicating + * the operation + * + * @return Return the new shape. The TopoShape itself is not modified. + */ + TopoShape makeElementDraft(const std::vector &faces, + const gp_Dir &pullDirection, double angle, const gp_Pln &neutralPlane, + bool retry=true, const char *op=nullptr) const { + return TopoShape(0,Hasher).makeElementDraft(*this,faces,pullDirection,angle,neutralPlane,retry,op); + } /* Make a shell using this shape * @param silent: whether to throw exception on failure diff --git a/src/Mod/Part/App/TopoShapeExpansion.cpp b/src/Mod/Part/App/TopoShapeExpansion.cpp index 3194b0b76f..fca36f0c6c 100644 --- a/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -1811,6 +1811,45 @@ TopoShape& TopoShape::makeElementShape(BRepPrimAPI_MakeHalfSpace& mkShape, return makeShapeWithElementMap(mkShape.Solid(), MapperMaker(mkShape), {source}, op); } +TopoShape &TopoShape::makeElementDraft(const TopoShape &shape, const std::vector &_faces, + const gp_Dir &pullDirection, double angle, const gp_Pln &neutralPlane, + bool retry, const char *op) +{ + if(!op) op = Part::OpCodes::Draft; + + if(shape.isNull()) + HANDLE_NULL_SHAPE; + + std::vector faces(_faces); + bool done = true; + BRepOffsetAPI_DraftAngle mkDraft; + do { + if(faces.empty()) + FC_THROWM(Base::CADKernelError,"no faces can be used"); + + mkDraft.Init(shape.getShape()); + done = true; + for(auto it=faces.begin();it!=faces.end();++it) { + // TODO: What is the flag for? + mkDraft.Add(TopoDS::Face(it->getShape()), pullDirection, angle, neutralPlane); + if (!mkDraft.AddDone()) { + // Note: the function ProblematicShape returns the face on which the error occurred + // Note: mkDraft.Remove() stumbles on a bug in Draft_Modification::Remove() and is + // therefore unusable. See http://forum.freecadweb.org/viewtopic.php?f=10&t=3209&start=10#p25341 + // The only solution is to discard mkDraft and start over without the current face + // mkDraft.Remove(face); + FC_ERR("Failed to add some face for drafting, skip"); + done = false; + faces.erase(it); + break; + } + } + }while(retry && !done); + + mkDraft.Build(); + return makeElementShape(mkDraft,shape,op); +} + TopoShape& TopoShape::makeElementFace(const TopoShape& shape, const char* op, const char* maker, From c6ca3e41b616477b59197e4bf90e425da268b237 Mon Sep 17 00:00:00 2001 From: bgbsww Date: Wed, 14 Feb 2024 12:20:10 -0500 Subject: [PATCH 2/3] Toponaming/Part: Cleanup and test makeElementDraft --- src/Mod/Part/App/TopoShapeExpansion.cpp | 5 +- tests/src/Mod/Part/App/PartTestHelpers.cpp | 23 ++++++++ tests/src/Mod/Part/App/PartTestHelpers.h | 1 + tests/src/Mod/Part/App/TopoShapeExpansion.cpp | 55 ++++++++++++++++++- 4 files changed, 81 insertions(+), 3 deletions(-) diff --git a/src/Mod/Part/App/TopoShapeExpansion.cpp b/src/Mod/Part/App/TopoShapeExpansion.cpp index fca36f0c6c..adf55cf732 100644 --- a/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -41,6 +41,8 @@ #include #include #include +#include +#include #include #include #include @@ -68,7 +70,6 @@ #include "FaceMaker.h" #include -#include FC_LOG_LEVEL_INIT("TopoShape", true, true) // NOLINT @@ -1818,7 +1819,7 @@ TopoShape &TopoShape::makeElementDraft(const TopoShape &shape, const std::vector if(!op) op = Part::OpCodes::Draft; if(shape.isNull()) - HANDLE_NULL_SHAPE; + FC_THROWM(NullShapeException, "Null shape"); std::vector faces(_faces); bool done = true; diff --git a/tests/src/Mod/Part/App/PartTestHelpers.cpp b/tests/src/Mod/Part/App/PartTestHelpers.cpp index 36dc63f067..feb73e8d10 100644 --- a/tests/src/Mod/Part/App/PartTestHelpers.cpp +++ b/tests/src/Mod/Part/App/PartTestHelpers.cpp @@ -1,6 +1,7 @@ // SPDX-License-Identifier: LGPL-2.1-or-later #include +#include #include "PartTestHelpers.h" @@ -155,6 +156,28 @@ std::pair CreateTwoCubes() return {box1, box2}; } +std::pair CreateTwoTopoShapeCubes() +{ + auto [box1, box2] = CreateTwoCubes(); + std::vector vec; + long tag = 1L; + for (TopExp_Explorer exp(box1, TopAbs_FACE); exp.More(); exp.Next()) { + vec.emplace_back(TopoShape(exp.Current(), tag++)); + } + TopoShape box1ts; + box1ts.makeElementCompound(vec); + box1ts.Tag = tag++; + vec.clear(); + for (TopExp_Explorer exp(box2, TopAbs_FACE); exp.More(); exp.Next()) { + vec.emplace_back(TopoShape(exp.Current(), tag++)); + } + TopoShape box2ts; + box2ts.Tag = tag++; + box2ts.makeElementCompound(vec); + + return {box1ts, box2ts}; +} + } // namespace PartTestHelpers // NOLINTEND(readability-magic-numbers,cppcoreguidelines-avoid-magic-numbers) diff --git a/tests/src/Mod/Part/App/PartTestHelpers.h b/tests/src/Mod/Part/App/PartTestHelpers.h index 8b829e4ba6..a10e1f9e36 100644 --- a/tests/src/Mod/Part/App/PartTestHelpers.h +++ b/tests/src/Mod/Part/App/PartTestHelpers.h @@ -59,4 +59,5 @@ std::map elementMap(const TopoShape& shape); std::pair CreateTwoCubes(); +std::pair CreateTwoTopoShapeCubes(); } // namespace PartTestHelpers diff --git a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp index b047f62eb0..7b6458e603 100644 --- a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -13,7 +13,7 @@ #include #include #include -#include +#include #include #include @@ -867,4 +867,57 @@ TEST_F(TopoShapeExpansionTest, makeElementBooleanFuse) EXPECT_FLOAT_EQ(getVolume(result.getShape()), 1.75); } +TEST_F(TopoShapeExpansionTest, makeElementDraft) +{ // Draft as in Draft Angle or sloped sides for removing shapes from a mold. + // Arrange + auto [cube1, cube2] = CreateTwoCubes(); + TopoShape cube1TS {cube1, 1L}; + std::vector subShapes = cube1TS.getSubTopoShapes(TopAbs_FACE); + std::vector faces {subShapes[0], subShapes[1], subShapes[2], subShapes[3]}; + const gp_Dir pullDirection {0, 0, 1}; + double angle {M_PI * 10 + / 8}; // Angle should be between Pi and Pi * 1.5 ( 180 and 270 degrees ) + const gp_Pln plane {}; + // Act + TopoShape& result = cube1TS.makeElementDraft(cube1TS, faces, pullDirection, angle, plane); + auto elements = elementMap(result); + // Assert + EXPECT_EQ(elements.size(), 26); // Cubes have 6 Faces, 12 Edges, 8 Vertexes + EXPECT_NEAR(getVolume(result.getShape()), 4.3333333333, 1e-06); // Truncated pyramid +} + +TEST_F(TopoShapeExpansionTest, makeElementDraftTopoShapes) +{ + // Arrange + auto [cube1TS, cube2TS] = CreateTwoTopoShapeCubes(); + const gp_Dir pullDirection {0, 0, 1}; + double angle {M_PI * 10 + / 8}; // Angle should be between Pi and Pi * 1.5 ( 180 and 270 degrees ) + const gp_Pln plane {}; + // Act + TopoShape result3 = cube1TS.makeElementDraft(cube1TS.getSubTopoShapes(TopAbs_FACE), + pullDirection, + angle, + plane); // Non Reference call type + TopoShape result2 = cube1TS.makeElementDraft(cube1TS, + cube1TS.getSubTopoShapes(TopAbs_FACE), + pullDirection, + angle, + plane); // Bad use of Reference call + TopoShape& result = cube1TS.makeElementDraft(cube2TS, + cube2TS.getSubTopoShapes(TopAbs_FACE), + pullDirection, + angle, + plane); // Correct usage + auto elements = elementMap((result)); + // Assert + EXPECT_TRUE(result.getMappedChildElements().empty()); + EXPECT_EQ(elements.size(), 26); + EXPECT_EQ(elements.count(IndexedName("Face", 1)), 1); + EXPECT_EQ(elements[IndexedName("Face", 1)], MappedName("Face1;:G;DFT;:He:7,F")); + EXPECT_NEAR(getVolume(result.getShape()), 4.3333333333, 1e-06); // Truncated pyramid + EXPECT_EQ(result2.getElementMap().size(), 0); // No element map in non reference call. + EXPECT_EQ(result3.getElementMap().size(), 0); // No element map in non reference call. +} + // NOLINTEND(readability-magic-numbers,cppcoreguidelines-avoid-magic-numbers) From 8229ae07ff1294b25a1a8a1b24cd6babae3509c7 Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Wed, 14 Feb 2024 18:56:38 -0600 Subject: [PATCH 3/3] Part/Toponaming: Apply clang-format to TopoShapeExpansion --- src/Mod/Part/App/TopoShapeExpansion.cpp | 91 +++++++++++++++---------- 1 file changed, 54 insertions(+), 37 deletions(-) diff --git a/src/Mod/Part/App/TopoShapeExpansion.cpp b/src/Mod/Part/App/TopoShapeExpansion.cpp index adf55cf732..5ba877d180 100644 --- a/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -59,7 +59,7 @@ #endif #if OCC_VERSION_HEX >= 0x070500 -# include +#include #endif #include "modelRefine.h" @@ -601,12 +601,11 @@ struct NameKey long tag = 0; int shapetype = 0; - NameKey() - = default; - explicit NameKey(Data::MappedName n) + NameKey() = default; + explicit NameKey(Data::MappedName n) : name(std::move(n)) {} - NameKey(int type, Data::MappedName n) + NameKey(int type, Data::MappedName n) : name(std::move(n)) { // Order the shape type from vertex < edge < face < other. We'll rely @@ -796,9 +795,9 @@ TopoShape& TopoShape::makeShapeWithElementMap(const TopoDS_Shape& shape, // First, collect names from other shapes that generates or modifies the // new shape - for (auto& pinfo : infos) { // Walk Vertexes, then Edges, then Faces + for (auto& pinfo : infos) { // Walk Vertexes, then Edges, then Faces auto& info = *pinfo; - for (const auto & incomingShape : shapes) { + for (const auto& incomingShape : shapes) { if (!canMapElement(incomingShape)) { continue; } @@ -811,7 +810,8 @@ TopoShape& TopoShape::makeShapeWithElementMap(const TopoDS_Shape& shape, const auto& otherElement = otherMap.find(incomingShape._Shape, i); // Find all new objects that are a modification of the old object Data::ElementIDRefs sids; - NameKey key(info.type, + NameKey key( + info.type, incomingShape.getMappedName(Data::IndexedName::fromConst(info.shapetype, i), true, &sids)); @@ -851,7 +851,8 @@ TopoShape& TopoShape::makeShapeWithElementMap(const TopoDS_Shape& shape, continue; } - Data::IndexedName element = Data::IndexedName::fromConst(newInfo.shapetype, newShapeIndex); + Data::IndexedName element = + Data::IndexedName::fromConst(newInfo.shapetype, newShapeIndex); if (getMappedName(element)) { continue; } @@ -1163,11 +1164,13 @@ TopoShape& TopoShape::makeShapeWithElementMap(const TopoDS_Shape& shape, TopTools_IndexedMapOfShape submap; TopExp::MapShapes(info.find(elementCounter), next.type, submap); - for (int submapIndex = 1, infoCounter = 1; submapIndex <= submap.Extent(); ++submapIndex) { + for (int submapIndex = 1, infoCounter = 1; submapIndex <= submap.Extent(); + ++submapIndex) { ss.str(""); int elementIndex = next.find(submap(submapIndex)); assert(elementIndex); - Data::IndexedName indexedName = Data::IndexedName::fromConst(next.shapetype, elementIndex); + Data::IndexedName indexedName = + Data::IndexedName::fromConst(next.shapetype, elementIndex); if (getMappedName(indexedName)) { continue; } @@ -1201,7 +1204,7 @@ TopoShape& TopoShape::makeShapeWithElementMap(const TopoDS_Shape& shape, bool hasUnnamed = false; for (size_t ifo = 1; ifo < infos.size(); ++ifo) { auto& info = *infos.at(ifo); - auto& prev = *infos.at(ifo-1); + auto& prev = *infos.at(ifo - 1); for (int i = 1; i <= info.count(); ++i) { Data::IndexedName element = Data::IndexedName::fromConst(info.shapetype, i); if (getMappedName(element)) { @@ -1220,7 +1223,8 @@ TopoShape& TopoShape::makeShapeWithElementMap(const TopoDS_Shape& shape, for (; xp.More(); xp.Next()) { int previousElementIndex = prev.find(xp.Current()); assert(previousElementIndex); - Data::IndexedName prevElement = Data::IndexedName::fromConst(prev.shapetype, previousElementIndex); + Data::IndexedName prevElement = + Data::IndexedName::fromConst(prev.shapetype, previousElementIndex); if (!delayed && (newNames.count(prevElement) != 0U)) { names.clear(); break; @@ -1723,7 +1727,7 @@ struct MapperThruSections: MapperMaker const std::vector& generated(const TopoDS_Shape& s) const override { MapperMaker::generated(s); - if ( ! _res.empty()) { + if (!_res.empty()) { return _res; } try { @@ -1812,32 +1816,41 @@ TopoShape& TopoShape::makeElementShape(BRepPrimAPI_MakeHalfSpace& mkShape, return makeShapeWithElementMap(mkShape.Solid(), MapperMaker(mkShape), {source}, op); } -TopoShape &TopoShape::makeElementDraft(const TopoShape &shape, const std::vector &_faces, - const gp_Dir &pullDirection, double angle, const gp_Pln &neutralPlane, - bool retry, const char *op) +TopoShape& TopoShape::makeElementDraft(const TopoShape& shape, + const std::vector& _faces, + const gp_Dir& pullDirection, + double angle, + const gp_Pln& neutralPlane, + bool retry, + const char* op) { - if(!op) op = Part::OpCodes::Draft; + if (!op) { + op = Part::OpCodes::Draft; + } - if(shape.isNull()) + if (shape.isNull()) { FC_THROWM(NullShapeException, "Null shape"); + } std::vector faces(_faces); bool done = true; BRepOffsetAPI_DraftAngle mkDraft; do { - if(faces.empty()) - FC_THROWM(Base::CADKernelError,"no faces can be used"); + if (faces.empty()) { + FC_THROWM(Base::CADKernelError, "no faces can be used"); + } mkDraft.Init(shape.getShape()); done = true; - for(auto it=faces.begin();it!=faces.end();++it) { + for (auto it = faces.begin(); it != faces.end(); ++it) { // TODO: What is the flag for? mkDraft.Add(TopoDS::Face(it->getShape()), pullDirection, angle, neutralPlane); if (!mkDraft.AddDone()) { // Note: the function ProblematicShape returns the face on which the error occurred // Note: mkDraft.Remove() stumbles on a bug in Draft_Modification::Remove() and is - // therefore unusable. See http://forum.freecadweb.org/viewtopic.php?f=10&t=3209&start=10#p25341 - // The only solution is to discard mkDraft and start over without the current face + // therefore unusable. See + // http://forum.freecadweb.org/viewtopic.php?f=10&t=3209&start=10#p25341 The + // only solution is to discard mkDraft and start over without the current face // mkDraft.Remove(face); FC_ERR("Failed to add some face for drafting, skip"); done = false; @@ -1845,10 +1858,10 @@ TopoShape &TopoShape::makeElementDraft(const TopoShape &shape, const std::vector break; } } - }while(retry && !done); + } while (retry && !done); mkDraft.Build(); - return makeElementShape(mkDraft,shape,op); + return makeElementShape(mkDraft, shape, op); } TopoShape& TopoShape::makeElementFace(const TopoShape& shape, @@ -1927,18 +1940,20 @@ TopoShape& TopoShape::makeElementFace(const std::vector& shapes, return *this; } -class MyRefineMaker : public BRepBuilderAPI_RefineModel +class MyRefineMaker: public BRepBuilderAPI_RefineModel { public: - explicit MyRefineMaker(const TopoDS_Shape &s) - :BRepBuilderAPI_RefineModel(s) + explicit MyRefineMaker(const TopoDS_Shape& s) + : BRepBuilderAPI_RefineModel(s) {} - void populate(ShapeMapper &mapper) + void populate(ShapeMapper& mapper) { - for (TopTools_DataMapIteratorOfDataMapOfShapeListOfShape it(this->myModified); it.More(); it.Next()) - { - if (it.Key().IsNull()) continue; + for (TopTools_DataMapIteratorOfDataMapOfShapeListOfShape it(this->myModified); it.More(); + it.Next()) { + if (it.Key().IsNull()) { + continue; + } mapper.populate(MappingStatus::Generated, it.Key(), it.Value()); } } @@ -2429,8 +2444,10 @@ bool TopoShape::fixSolidOrientation() return false; } -TopoShape& -TopoShape::makeElementBoolean(const char* maker, const TopoShape& shape, const char* op, double tolerance) +TopoShape& TopoShape::makeElementBoolean(const char* maker, + const TopoShape& shape, + const char* op, + double tolerance) { return makeElementBoolean(maker, std::vector(1, shape), op, tolerance); } @@ -2613,8 +2630,8 @@ TopoShape& TopoShape::makeElementBoolean(const char* maker, #if OCC_VERSION_HEX >= 0x070500 // -1/22/2024 Removing the parameter. // if (PartParams::getParallelRunThreshold() > 0) { - mk->SetRunParallel(Standard_True); - OSD_Parallel::SetUseOcctThreads(Standard_True); + mk->SetRunParallel(Standard_True); + OSD_Parallel::SetUseOcctThreads(Standard_True); // } #else // 01/22/2024 This will be an extremely rare case, since we don't