diff --git a/src/Mod/Part/App/TopoShape.h b/src/Mod/Part/App/TopoShape.h index a80ce07946..788182e764 100644 --- a/src/Mod/Part/App/TopoShape.h +++ b/src/Mod/Part/App/TopoShape.h @@ -609,7 +609,7 @@ public: void copyElementMap(const TopoShape & topoShape, const char *op=nullptr); bool canMapElement(const TopoShape &other) const; void mapSubElement(const TopoShape &other,const char *op=nullptr, bool forceHasher=false); - void mapSubElement(const std::vector &shapes, const char *op); + void mapSubElement(const std::vector &shapes, const char *op=nullptr); bool hasPendingElementMap() const; diff --git a/src/Mod/Part/App/TopoShapeExpansion.cpp b/src/Mod/Part/App/TopoShapeExpansion.cpp index d5b5b5227b..63b4faf718 100644 --- a/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -376,6 +376,9 @@ void TopoShape::mapSubElementTypeForShape(const TopoShape& other, } std::ostringstream ss; char elementType {shapeName(type)[0]}; + if ( ! elementMap() ) { + FC_THROWM(NullShapeException, "No element map"); + } elementMap()->encodeElementName(elementType, name, ss, &sids, Tag, op, other.Tag); elementMap()->setElementName(element, name, Tag, &sids); } diff --git a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp index 964f346f88..1fd54c23b9 100644 --- a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -12,6 +12,7 @@ // NOLINTBEGIN(readability-magic-numbers,cppcoreguidelines-avoid-magic-numbers) + class TopoShapeExpansionTest: public ::testing::Test { protected: @@ -161,4 +162,153 @@ TEST_F(TopoShapeExpansionTest, makeElementCompoundTwoCubes) // 26 subshapes each } + +TEST_F(TopoShapeExpansionTest, mapSubElementInvalidParm) +{ + // Arrange + auto [cube1, cube2] = CreateTwoCubes(); + Part::TopoShape cube1TS {cube1}; + cube1TS.Tag = 1; + + // Act + std::vector subShapes = cube1TS.getSubTopoShapes(TopAbs_FACE); + Part::TopoShape face1 = subShapes.front(); + face1.Tag = 2; + + // Assert + EXPECT_THROW(cube1TS.mapSubElement(face1), Part::NullShapeException); // No subshapes +} + +TEST_F(TopoShapeExpansionTest, mapSubElementFindShapeByNames) +{ + // Arrange + auto [cube1, cube2] = CreateTwoCubes(); + Part::TopoShape cube1TS {cube1}; + Part::TopoShape cube2TS {cube2}; + cube1TS.Tag = 1; + cube2TS.Tag = 2; + Part::TopoShape topoShape, topoShape1; + + // Act + int fs1 = topoShape1.findShape(cube1); + topoShape.setShape(cube2TS); + topoShape1.makeElementCompound({cube1TS, cube2TS}); + int fs2 = topoShape1.findShape(cube1); + + TopoDS_Shape tds1 = topoShape.findShape("SubShape1"); + TopoDS_Shape tds2 = topoShape.findShape("SubShape2"); // Nonexistent + TopoDS_Shape tds3 = topoShape1.findShape("SubShape1"); + TopoDS_Shape tds4 = topoShape1.findShape("SubShape2"); + TopoDS_Shape tds5 = topoShape1.findShape("NonExistentName"); // Invalid Name + + // Assert + EXPECT_EQ(fs1, 0); + EXPECT_EQ(fs2, 1); + EXPECT_FALSE(tds1.IsNull()); + EXPECT_TRUE(tds2.IsNull()); + EXPECT_FALSE(tds3.IsNull()); + EXPECT_FALSE(tds4.IsNull()); + EXPECT_TRUE(tds5.IsNull()); +} + +TEST_F(TopoShapeExpansionTest, mapSubElementFindShapeByType) +{ + // Arrange + auto [cube1, cube2] = CreateTwoCubes(); + Part::TopoShape cube1TS {cube1}; + Part::TopoShape cube2TS {cube2}; + cube1TS.Tag = 1; + cube2TS.Tag = 2; + Part::TopoShape topoShape; + topoShape.makeElementCompound({cube1TS, cube2TS}); + topoShape.mapSubElement(cube2TS, "Name", false); + + // Act, Assert + for (int i = 1; i <= 12; i++) { + TopoDS_Shape dshape1 = topoShape.findShape(TopAbs_FACE, i); + EXPECT_FALSE(dshape1.IsNull()) << "Face num " << i; + } + TopoDS_Shape dshape1 = topoShape.findShape(TopAbs_FACE, 13); + EXPECT_TRUE(dshape1.IsNull()); +} + + +TEST_F(TopoShapeExpansionTest, mapSubElementFindAncestor) +{ + // Arrange + auto [cube1, cube2] = CreateTwoCubes(); + Part::TopoShape cube1TS {cube1}; + Part::TopoShape cube2TS {cube2}; + cube1TS.Tag = 1; + cube2TS.Tag = 2; + Part::TopoShape topoShape; + topoShape.makeElementCompound({cube1TS, cube2TS}); + topoShape.mapSubElement(cube2TS, "Name", false); + + // Act + int fa1 = topoShape.findAncestor(cube2, TopAbs_COMPOUND); + TopoDS_Shape tds1 = topoShape.findAncestorShape(cube1, TopAbs_COMPOUND); + + // Assert + EXPECT_EQ(fa1, 1); + EXPECT_TRUE(tds1.IsEqual(topoShape.getShape())); +} + + +TEST_F(TopoShapeExpansionTest, mapSubElementFindAncestors) +{ + // Arrange + auto [cube1, cube2] = CreateTwoCubes(); + auto [cube3, cube4] = CreateTwoCubes(); + auto tr {gp_Trsf()}; + tr.SetTranslation(gp_Vec(gp_XYZ(0, 1, 0))); + cube3.Move(TopLoc_Location(tr)); + cube4.Move(TopLoc_Location(tr)); + Part::TopoShape cube1TS {cube1}; + Part::TopoShape cube2TS {cube2}; + Part::TopoShape cube3TS {cube3}; + Part::TopoShape cube4TS {cube4}; + cube1TS.Tag = 1; + cube2TS.Tag = 2; + cube3TS.Tag = 3; + cube4TS.Tag = 4; + Part::TopoShape topoShape, topoShape1, topoShape2; + Part::TopoShape topoShape3, topoShape4, topoShape5, topoShape6; + topoShape.makeElementCompound({cube1TS, cube2TS}); + topoShape1.makeElementCompound({cube3TS, cube4TS}); + topoShape2.makeElementCompound({cube1TS, cube3TS}); + topoShape3.makeElementCompound({cube2TS, cube4TS}); + topoShape4.makeElementCompound({topoShape, topoShape1}); + topoShape5.makeElementCompound({topoShape2, topoShape3}); + topoShape6.makeElementCompound({topoShape4, topoShape5}); + topoShape6.mapSubElement(cube2TS, nullptr, false); + + // Act + auto ancestorList = topoShape6.findAncestors(cube3, TopAbs_COMPOUND); + auto ancestorShapeList = topoShape6.findAncestorsShapes(cube3, TopAbs_COMPOUND); + + // FIXME: It seems very strange that both of these ancestors calls return lists of two items + // that contain the same thing twice. What I expect is that the ancestors of cube3 would be + // topoShape6 topoShape5, topoShape3, topoShape2, and topoShape1. + // + // This is a very convoluted hierarchy, and the only way I could get more than one result from + // findAncestors. I guess it's possible that it's only intended to return a single result in + // almost all cases; that would mean that what it returns is the shape at the top of the tree. + // But that's exactly the shape we use to call it in the first place, so we already have it. + // + // Note that in the RT branch, findAncestorsShapes is called by GenericShapeMapper::init, + // TopoShape::makEChamfer and MapperPrism + // findAncestors is used in a dozen places. + // + + // Assert + EXPECT_EQ(ancestorList.size(), 2); + EXPECT_EQ(ancestorList.front(), 1); + EXPECT_EQ(ancestorList.back(), 1); + EXPECT_EQ(ancestorShapeList.size(), 2); + EXPECT_TRUE(ancestorShapeList.front().IsEqual(topoShape6.getShape())); + EXPECT_TRUE(ancestorShapeList.back().IsEqual(topoShape6.getShape())); +} + + // NOLINTEND(readability-magic-numbers,cppcoreguidelines-avoid-magic-numbers)