From cfdbf79b2b221df044195f96b19c22a2446b2227 Mon Sep 17 00:00:00 2001 From: bgbsww Date: Sun, 14 Jan 2024 12:19:58 -0500 Subject: [PATCH 1/9] Initial code of mapSubElement Test --- src/Mod/Part/App/TopoShape.h | 2 +- tests/src/Mod/Part/App/TopoShapeExpansion.cpp | 87 +++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/src/Mod/Part/App/TopoShape.h b/src/Mod/Part/App/TopoShape.h index ad4ef2a780..61a63bca40 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/tests/src/Mod/Part/App/TopoShapeExpansion.cpp b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp index 964f346f88..360898d323 100644 --- a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -161,4 +161,91 @@ TEST_F(TopoShapeExpansionTest, makeElementCompoundTwoCubes) // 26 subshapes each } + +TEST_F(TopoShapeExpansionTest, mapSubElement) +{ + // Arrange + auto [cube1, cube2] = CreateTwoCubes(); + Part::TopoShape cube1TS {cube1}; + cube1TS.Tag = 1; + Part::TopoShape cube2TS {cube2}; + cube2TS.Tag = 2; + auto [cube3, cube4] = CreateTwoCubes(); + Part::TopoShape cube3TS {cube3}; + cube3TS.Tag = 3; + Part::TopoShape cube4TS {cube4}; + cube4TS.Tag = 4; + std::vector subShapes = cube1TS.getSubTopoShapes(TopAbs_FACE); + Part::TopoShape face1 = subShapes.front(); + face1.Tag = 3; + Part::TopoShape topoShape, topoShape1; + + // Act + int fs1 = topoShape.findShape(cube1TS.getShape()); + topoShape.setShape(cube1TS); + // cube1TS.mapSubElement(face1); // This throws an exception. Is that right? + // The cache ancestry only works on TopAbs_SHAPE so this is likely an invalid call, + // but do we defend against it or expect the exception? + + topoShape.mapSubElement( + cube1TS); // Once we do this map, it's in the ancestry cache forevermore + int fs2 = topoShape.findShape(cube1TS.getShape()); + int fs3 = topoShape.findShape(face1.getShape()); + int size1 = topoShape.getElementMap().size(); + int size0 = cube1TS.getElementMap().size(); + + // Assert + EXPECT_EQ(fs1, 0); + EXPECT_EQ(fs2, 1); + EXPECT_EQ(fs3, 1); + EXPECT_EQ(0, size0); + EXPECT_EQ(26, size1); + + // Act + topoShape.setShape(TopoDS_Shape()); + int fs4 = topoShape.findShape(cube1TS.getShape()); + topoShape.setShape(cube1, true); + // No mapSubElement required, it keeps finding it now + int fs5 = topoShape.findShape(cube1TS.getShape()); + topoShape.setShape(cube1, false); + int fs6 = topoShape.findShape(cube1TS.getShape()); + // Assert + EXPECT_EQ(fs4, 0); + EXPECT_EQ(fs5, 1); + EXPECT_EQ(fs6, 1); + + // Act + Part::TopoShape topoShape2, topoShape3; + topoShape2.setShape(cube2TS); + topoShape2.mapSubElement(cube2TS, nullptr, true); + int fs7 = topoShape2.findShape(cube2TS.getShape()); + int fs8 = topoShape2.findShape(face1.getShape()); + + topoShape3.setShape(cube3TS); + topoShape3.mapSubElement(cube3TS, "Sample", true); + int fs9 = topoShape3.findShape(cube3TS.getShape()); + int fs10 = topoShape3.findShape(face1.getShape()); + + topoShape.makeElementCompound({cube1TS, cube2TS}); + int fs11 = topoShape.findShape(cube2TS.getShape()); + int size2 = topoShape.getElementMap().size(); + // Assert + EXPECT_EQ(fs7, 1); + EXPECT_EQ(fs8, 0); + EXPECT_EQ(fs9, 1); + EXPECT_EQ(fs10, 0); + EXPECT_EQ(fs11, 2); + EXPECT_EQ(52, size2); + + // Act + topoShape2.makeElementCompound({cube3TS, cube4TS}); + topoShape2.mapSubElement(cube3TS, nullptr, true); + topoShape3.makeElementCompound({topoShape, topoShape2}); + int fs12 = topoShape3.findShape(cube4TS.getShape()); + int size4 = topoShape3.getElementMap().size(); + // Assert + EXPECT_EQ(104, size4); + EXPECT_EQ(fs12, 4); +} + // NOLINTEND(readability-magic-numbers,cppcoreguidelines-avoid-magic-numbers) From a2d15df6df0605b38e25c05aca5ea3c0ea4b1f1d Mon Sep 17 00:00:00 2001 From: bgbsww Date: Thu, 18 Jan 2024 13:15:21 -0500 Subject: [PATCH 2/9] Split test cases --- tests/src/Mod/Part/App/TopoShapeExpansion.cpp | 67 ++++++++++++++----- 1 file changed, 51 insertions(+), 16 deletions(-) diff --git a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp index 360898d323..3d71be8070 100644 --- a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -162,23 +162,19 @@ TEST_F(TopoShapeExpansionTest, makeElementCompoundTwoCubes) } -TEST_F(TopoShapeExpansionTest, mapSubElement) +TEST_F(TopoShapeExpansionTest, mapSubElementOneCube) { // Arrange auto [cube1, cube2] = CreateTwoCubes(); - Part::TopoShape cube1TS {cube1}; - cube1TS.Tag = 1; - Part::TopoShape cube2TS {cube2}; - cube2TS.Tag = 2; auto [cube3, cube4] = CreateTwoCubes(); - Part::TopoShape cube3TS {cube3}; - cube3TS.Tag = 3; - Part::TopoShape cube4TS {cube4}; - cube4TS.Tag = 4; + Part::TopoShape cube1TS {cube1}; + Part::TopoShape cube2TS {cube2}; + cube1TS.Tag = 1; + cube2TS.Tag = 2; std::vector subShapes = cube1TS.getSubTopoShapes(TopAbs_FACE); Part::TopoShape face1 = subShapes.front(); - face1.Tag = 3; - Part::TopoShape topoShape, topoShape1; + face1.Tag = 5; + Part::TopoShape topoShape; // Act int fs1 = topoShape.findShape(cube1TS.getShape()); @@ -187,12 +183,11 @@ TEST_F(TopoShapeExpansionTest, mapSubElement) // The cache ancestry only works on TopAbs_SHAPE so this is likely an invalid call, // but do we defend against it or expect the exception? - topoShape.mapSubElement( - cube1TS); // Once we do this map, it's in the ancestry cache forevermore + topoShape.mapSubElement(cube1TS); int fs2 = topoShape.findShape(cube1TS.getShape()); int fs3 = topoShape.findShape(face1.getShape()); - int size1 = topoShape.getElementMap().size(); int size0 = cube1TS.getElementMap().size(); + int size1 = topoShape.getElementMap().size(); // Assert EXPECT_EQ(fs1, 0); @@ -200,19 +195,58 @@ TEST_F(TopoShapeExpansionTest, mapSubElement) EXPECT_EQ(fs3, 1); EXPECT_EQ(0, size0); EXPECT_EQ(26, size1); +} + +TEST_F(TopoShapeExpansionTest, mapSubElementSetReset) +{ + // Arrange + auto [cube1, cube2] = CreateTwoCubes(); + auto [cube3, cube4] = CreateTwoCubes(); + 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; + std::vector subShapes = cube1TS.getSubTopoShapes(TopAbs_FACE); + Part::TopoShape face1 = subShapes.front(); + face1.Tag = 5; + Part::TopoShape topoShape, topoShape1; // Act topoShape.setShape(TopoDS_Shape()); int fs4 = topoShape.findShape(cube1TS.getShape()); - topoShape.setShape(cube1, true); + topoShape.setShape(cube1, false); + // topoShape.mapSubElement(cube1TS); // No mapSubElement required, it keeps finding it now int fs5 = topoShape.findShape(cube1TS.getShape()); - topoShape.setShape(cube1, false); + topoShape.setShape(cube1, true); int fs6 = topoShape.findShape(cube1TS.getShape()); // Assert EXPECT_EQ(fs4, 0); EXPECT_EQ(fs5, 1); EXPECT_EQ(fs6, 1); +} + +TEST_F(TopoShapeExpansionTest, mapSubElementCompoundCubes) +{ + // Arrange + auto [cube1, cube2] = CreateTwoCubes(); + auto [cube3, cube4] = CreateTwoCubes(); + 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; + std::vector subShapes = cube1TS.getSubTopoShapes(TopAbs_FACE); + Part::TopoShape face1 = subShapes.front(); + face1.Tag = 5; + Part::TopoShape topoShape, topoShape1; // Act Part::TopoShape topoShape2, topoShape3; @@ -248,4 +282,5 @@ TEST_F(TopoShapeExpansionTest, mapSubElement) EXPECT_EQ(fs12, 4); } + // NOLINTEND(readability-magic-numbers,cppcoreguidelines-avoid-magic-numbers) From 2699c748c816546c1d8ff06e382c35c54fcf7f6a Mon Sep 17 00:00:00 2001 From: bgbsww Date: Fri, 19 Jan 2024 10:49:47 -0500 Subject: [PATCH 3/9] Improve tests --- src/Mod/Part/App/TopoShapeExpansion.cpp | 2 + tests/src/Mod/Part/App/TopoShapeExpansion.cpp | 181 +++++++++--------- 2 files changed, 94 insertions(+), 89 deletions(-) diff --git a/src/Mod/Part/App/TopoShapeExpansion.cpp b/src/Mod/Part/App/TopoShapeExpansion.cpp index d5b5b5227b..17a33b4edd 100644 --- a/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -376,6 +376,8 @@ 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 3d71be8070..d58efa4492 100644 --- a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -162,125 +162,128 @@ TEST_F(TopoShapeExpansionTest, makeElementCompoundTwoCubes) } -TEST_F(TopoShapeExpansionTest, mapSubElementOneCube) +TEST_F(TopoShapeExpansionTest, mapSubElementNames) { // Arrange auto [cube1, cube2] = CreateTwoCubes(); - auto [cube3, cube4] = CreateTwoCubes(); Part::TopoShape cube1TS {cube1}; Part::TopoShape cube2TS {cube2}; cube1TS.Tag = 1; cube2TS.Tag = 2; + Part::TopoShape topoShape, topoShape1; + + // Act std::vector subShapes = cube1TS.getSubTopoShapes(TopAbs_FACE); Part::TopoShape face1 = subShapes.front(); face1.Tag = 5; - Part::TopoShape topoShape; - // Act - int fs1 = topoShape.findShape(cube1TS.getShape()); - topoShape.setShape(cube1TS); - // cube1TS.mapSubElement(face1); // This throws an exception. Is that right? - // The cache ancestry only works on TopAbs_SHAPE so this is likely an invalid call, - // but do we defend against it or expect the exception? + int fs1 = topoShape1.findShape(cube1); + topoShape.setShape(cube2TS); + topoShape1.makeElementCompound({cube1TS, cube2TS}); + int fs2 = topoShape1.findShape(cube1); - topoShape.mapSubElement(cube1TS); - int fs2 = topoShape.findShape(cube1TS.getShape()); - int fs3 = topoShape.findShape(face1.getShape()); - int size0 = cube1TS.getElementMap().size(); - int size1 = topoShape.getElementMap().size(); + 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_THROW(cube1TS.mapSubElement(face1), Part::NullShapeException); // No subshapes EXPECT_EQ(fs1, 0); EXPECT_EQ(fs2, 1); - EXPECT_EQ(fs3, 1); - EXPECT_EQ(0, size0); - EXPECT_EQ(26, size1); + EXPECT_FALSE(tds1.IsNull()); + EXPECT_TRUE(tds2.IsNull()); + EXPECT_FALSE(tds3.IsNull()); + EXPECT_FALSE(tds4.IsNull()); + EXPECT_TRUE(tds5.IsNull()); } -TEST_F(TopoShapeExpansionTest, mapSubElementSetReset) +TEST_F(TopoShapeExpansionTest, mapSubElementCacheType) { // Arrange auto [cube1, cube2] = CreateTwoCubes(); - auto [cube3, cube4] = CreateTwoCubes(); 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; - std::vector subShapes = cube1TS.getSubTopoShapes(TopAbs_FACE); - Part::TopoShape face1 = subShapes.front(); - face1.Tag = 5; - Part::TopoShape topoShape, topoShape1; - - // Act - topoShape.setShape(TopoDS_Shape()); - int fs4 = topoShape.findShape(cube1TS.getShape()); - topoShape.setShape(cube1, false); - // topoShape.mapSubElement(cube1TS); - // No mapSubElement required, it keeps finding it now - int fs5 = topoShape.findShape(cube1TS.getShape()); - topoShape.setShape(cube1, true); - int fs6 = topoShape.findShape(cube1TS.getShape()); - // Assert - EXPECT_EQ(fs4, 0); - EXPECT_EQ(fs5, 1); - EXPECT_EQ(fs6, 1); -} - -TEST_F(TopoShapeExpansionTest, mapSubElementCompoundCubes) -{ - // Arrange - auto [cube1, cube2] = CreateTwoCubes(); - auto [cube3, cube4] = CreateTwoCubes(); - 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; - std::vector subShapes = cube1TS.getSubTopoShapes(TopAbs_FACE); - Part::TopoShape face1 = subShapes.front(); - face1.Tag = 5; - Part::TopoShape topoShape, topoShape1; - - // Act - Part::TopoShape topoShape2, topoShape3; - topoShape2.setShape(cube2TS); - topoShape2.mapSubElement(cube2TS, nullptr, true); - int fs7 = topoShape2.findShape(cube2TS.getShape()); - int fs8 = topoShape2.findShape(face1.getShape()); - - topoShape3.setShape(cube3TS); - topoShape3.mapSubElement(cube3TS, "Sample", true); - int fs9 = topoShape3.findShape(cube3TS.getShape()); - int fs10 = topoShape3.findShape(face1.getShape()); - + Part::TopoShape topoShape; topoShape.makeElementCompound({cube1TS, cube2TS}); - int fs11 = topoShape.findShape(cube2TS.getShape()); - int size2 = topoShape.getElementMap().size(); - // Assert - EXPECT_EQ(fs7, 1); - EXPECT_EQ(fs8, 0); - EXPECT_EQ(fs9, 1); - EXPECT_EQ(fs10, 0); - EXPECT_EQ(fs11, 2); - EXPECT_EQ(52, size2); + 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, mapSubElementCacheAncestor) +{ + // 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 - topoShape2.makeElementCompound({cube3TS, cube4TS}); - topoShape2.mapSubElement(cube3TS, nullptr, true); - topoShape3.makeElementCompound({topoShape, topoShape2}); - int fs12 = topoShape3.findShape(cube4TS.getShape()); - int size4 = topoShape3.getElementMap().size(); + int fa1 = topoShape.findAncestor(cube2, TopAbs_COMPOUND); + TopoDS_Shape tds1 = topoShape.findAncestorShape(cube1, TopAbs_COMPOUND); + // Assert - EXPECT_EQ(104, size4); - EXPECT_EQ(fs12, 4); + EXPECT_EQ(fa1, 1); + EXPECT_TRUE(tds1.IsEqual(topoShape.getShape())); } +TEST_F(TopoShapeExpansionTest, mapSubElementCacheAncestors) +{ + // 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; + topoShape.makeElementCompound({cube1TS, cube2TS}); + topoShape.mapSubElement(cube2TS, nullptr, false); + topoShape1.makeElementCompound({cube3TS, cube4TS}); + topoShape1.mapSubElement(cube3TS, nullptr, true); + topoShape2.makeElementCompound({topoShape, topoShape1}); + topoShape2.mapSubElement(cube2TS, nullptr, false); + + // Act + auto ancestorList = topoShape2.findAncestors(cube2, TopAbs_COMPOUND); + auto ancestorShapeList = topoShape2.findAncestorsShapes(cube2, TopAbs_COMPOUND); + + // Assert + EXPECT_EQ(ancestorList.size(), 1); + EXPECT_EQ(ancestorList.front(), 1); + EXPECT_EQ(ancestorShapeList.size(), 1); + // EXPECT_TRUE(ancestorShapeList.front().IsEqual(topoShape.getShape())); + // EXPECT_TRUE(ancestorShapeList.back().IsEqual(topoShape1.getShape())); +} + +// void initCache(int reset = 0) const; // Can't see any path to visibility to test if this worked. +// std::vector findAncestors(const TopoDS_Shape& subshape, TopAbs_ShapeEnum type) const; // +// DONE std::vector findAncestorsShapes(const TopoDS_Shape& subshape, +// TopAbs_ShapeEnum type) const; // UNSURE + + // NOLINTEND(readability-magic-numbers,cppcoreguidelines-avoid-magic-numbers) From 22b952f8e42e5be81e9e8c31d527f4db0e59c30a Mon Sep 17 00:00:00 2001 From: bgbsww Date: Fri, 19 Jan 2024 20:59:19 -0500 Subject: [PATCH 4/9] Improve tests --- tests/src/Mod/Part/App/TopoShapeExpansion.cpp | 73 +++++++++++++------ 1 file changed, 49 insertions(+), 24 deletions(-) diff --git a/tests/src/Mod/Part/App/TopoShapeExpansion.cpp b/tests/src/Mod/Part/App/TopoShapeExpansion.cpp index d58efa4492..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: @@ -162,7 +163,23 @@ TEST_F(TopoShapeExpansionTest, makeElementCompoundTwoCubes) } -TEST_F(TopoShapeExpansionTest, mapSubElementNames) +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(); @@ -173,10 +190,6 @@ TEST_F(TopoShapeExpansionTest, mapSubElementNames) Part::TopoShape topoShape, topoShape1; // Act - std::vector subShapes = cube1TS.getSubTopoShapes(TopAbs_FACE); - Part::TopoShape face1 = subShapes.front(); - face1.Tag = 5; - int fs1 = topoShape1.findShape(cube1); topoShape.setShape(cube2TS); topoShape1.makeElementCompound({cube1TS, cube2TS}); @@ -189,7 +202,6 @@ TEST_F(TopoShapeExpansionTest, mapSubElementNames) TopoDS_Shape tds5 = topoShape1.findShape("NonExistentName"); // Invalid Name // Assert - EXPECT_THROW(cube1TS.mapSubElement(face1), Part::NullShapeException); // No subshapes EXPECT_EQ(fs1, 0); EXPECT_EQ(fs2, 1); EXPECT_FALSE(tds1.IsNull()); @@ -199,7 +211,7 @@ TEST_F(TopoShapeExpansionTest, mapSubElementNames) EXPECT_TRUE(tds5.IsNull()); } -TEST_F(TopoShapeExpansionTest, mapSubElementCacheType) +TEST_F(TopoShapeExpansionTest, mapSubElementFindShapeByType) { // Arrange auto [cube1, cube2] = CreateTwoCubes(); @@ -221,7 +233,7 @@ TEST_F(TopoShapeExpansionTest, mapSubElementCacheType) } -TEST_F(TopoShapeExpansionTest, mapSubElementCacheAncestor) +TEST_F(TopoShapeExpansionTest, mapSubElementFindAncestor) { // Arrange auto [cube1, cube2] = CreateTwoCubes(); @@ -243,7 +255,7 @@ TEST_F(TopoShapeExpansionTest, mapSubElementCacheAncestor) } -TEST_F(TopoShapeExpansionTest, mapSubElementCacheAncestors) +TEST_F(TopoShapeExpansionTest, mapSubElementFindAncestors) { // Arrange auto [cube1, cube2] = CreateTwoCubes(); @@ -261,29 +273,42 @@ TEST_F(TopoShapeExpansionTest, mapSubElementCacheAncestors) cube3TS.Tag = 3; cube4TS.Tag = 4; Part::TopoShape topoShape, topoShape1, topoShape2; + Part::TopoShape topoShape3, topoShape4, topoShape5, topoShape6; topoShape.makeElementCompound({cube1TS, cube2TS}); - topoShape.mapSubElement(cube2TS, nullptr, false); topoShape1.makeElementCompound({cube3TS, cube4TS}); - topoShape1.mapSubElement(cube3TS, nullptr, true); - topoShape2.makeElementCompound({topoShape, topoShape1}); - topoShape2.mapSubElement(cube2TS, nullptr, false); + 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 = topoShape2.findAncestors(cube2, TopAbs_COMPOUND); - auto ancestorShapeList = topoShape2.findAncestorsShapes(cube2, TopAbs_COMPOUND); + 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(), 1); + EXPECT_EQ(ancestorList.size(), 2); EXPECT_EQ(ancestorList.front(), 1); - EXPECT_EQ(ancestorShapeList.size(), 1); - // EXPECT_TRUE(ancestorShapeList.front().IsEqual(topoShape.getShape())); - // EXPECT_TRUE(ancestorShapeList.back().IsEqual(topoShape1.getShape())); + EXPECT_EQ(ancestorList.back(), 1); + EXPECT_EQ(ancestorShapeList.size(), 2); + EXPECT_TRUE(ancestorShapeList.front().IsEqual(topoShape6.getShape())); + EXPECT_TRUE(ancestorShapeList.back().IsEqual(topoShape6.getShape())); } -// void initCache(int reset = 0) const; // Can't see any path to visibility to test if this worked. -// std::vector findAncestors(const TopoDS_Shape& subshape, TopAbs_ShapeEnum type) const; // -// DONE std::vector findAncestorsShapes(const TopoDS_Shape& subshape, -// TopAbs_ShapeEnum type) const; // UNSURE - // NOLINTEND(readability-magic-numbers,cppcoreguidelines-avoid-magic-numbers) From 3fd886d58f48737818abaefc9af278a5c70d826c Mon Sep 17 00:00:00 2001 From: bgbsww <120601209+bgbsww@users.noreply.github.com> Date: Mon, 22 Jan 2024 12:12:21 -0500 Subject: [PATCH 5/9] Update src/Mod/Part/App/TopoShapeExpansion.cpp Co-authored-by: Chris Hennes --- src/Mod/Part/App/TopoShapeExpansion.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Mod/Part/App/TopoShapeExpansion.cpp b/src/Mod/Part/App/TopoShapeExpansion.cpp index 17a33b4edd..63b4faf718 100644 --- a/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -376,8 +376,9 @@ void TopoShape::mapSubElementTypeForShape(const TopoShape& other, } std::ostringstream ss; char elementType {shapeName(type)[0]}; - if ( ! elementMap() ) + if ( ! elementMap() ) { FC_THROWM(NullShapeException, "No element map"); + } elementMap()->encodeElementName(elementType, name, ss, &sids, Tag, op, other.Tag); elementMap()->setElementName(element, name, Tag, &sids); } From 0bef2e927becc85a50787e32a71e2554dc2a514f Mon Sep 17 00:00:00 2001 From: Max Wilfinger Date: Sat, 13 Jan 2024 17:33:02 +0100 Subject: [PATCH 6/9] [Sketcher] enable BSpline commands in contextual right click menu --- .../splines/Sketcher_BSplineApproximate.svg | 31 +++++- .../splines/Sketcher_BSplineInsertKnot.svg | 6 +- .../icons/splines/Sketcher_JoinCurves.svg | 28 ++++- src/Mod/Sketcher/Gui/ViewProviderSketch.cpp | 101 ++++++++++++++---- 4 files changed, 134 insertions(+), 32 deletions(-) diff --git a/src/Mod/Sketcher/Gui/Resources/icons/splines/Sketcher_BSplineApproximate.svg b/src/Mod/Sketcher/Gui/Resources/icons/splines/Sketcher_BSplineApproximate.svg index 66e92f507b..e0c8e5ac32 100644 --- a/src/Mod/Sketcher/Gui/Resources/icons/splines/Sketcher_BSplineApproximate.svg +++ b/src/Mod/Sketcher/Gui/Resources/icons/splines/Sketcher_BSplineApproximate.svg @@ -15,6 +15,17 @@ xmlns:dc="http://purl.org/dc/elements/1.1/"> + + + + + @@ -464,7 +484,8 @@ transform="matrix(0.1460346,0,0,0.1460346,-220.10298,-55.131225)"> + id="g4428-3-2" + style="display:inline"> diff --git a/src/Mod/Sketcher/Gui/Resources/icons/splines/Sketcher_BSplineInsertKnot.svg b/src/Mod/Sketcher/Gui/Resources/icons/splines/Sketcher_BSplineInsertKnot.svg index fa676a240d..569e11ffd3 100644 --- a/src/Mod/Sketcher/Gui/Resources/icons/splines/Sketcher_BSplineInsertKnot.svg +++ b/src/Mod/Sketcher/Gui/Resources/icons/splines/Sketcher_BSplineInsertKnot.svg @@ -380,7 +380,7 @@ id="path3826" /> + diff --git a/src/Mod/Sketcher/Gui/Resources/icons/splines/Sketcher_JoinCurves.svg b/src/Mod/Sketcher/Gui/Resources/icons/splines/Sketcher_JoinCurves.svg index 0ac32bc5e1..3cf03594f5 100644 --- a/src/Mod/Sketcher/Gui/Resources/icons/splines/Sketcher_JoinCurves.svg +++ b/src/Mod/Sketcher/Gui/Resources/icons/splines/Sketcher_JoinCurves.svg @@ -261,11 +261,11 @@ @@ -316,6 +316,15 @@ y1="35.978416" x2="25.988253" y2="29.916241" /> + @@ -368,7 +377,7 @@ + transform="matrix(-1,0,0,1,47.987644,0)"> + + + + diff --git a/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp b/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp index dd3b963bc0..05f7db40c7 100644 --- a/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp +++ b/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp @@ -3886,6 +3886,11 @@ void ViewProviderSketch::generateContextMenu() int selectedConics = 0; int selectedPoints = 0; int selectedConstraints = 0; + int selectedBsplines = 0; + int selectedBsplineKnots = 0; + int selectedOrigin = 0; + int selectedEndPoints = 0; + bool onlyOrigin = false; Gui::MenuItem menu; menu.setCommand("Sketcher context"); @@ -3896,31 +3901,70 @@ void ViewProviderSketch::generateContextMenu() // if something is selected, count different elements in the current selection if (selection.size() > 0) { const std::vector SubNames = selection[0].getSubNames(); - - for (auto& name : SubNames) { - if (name.substr(0, 4) == "Edge") { - ++selectedEdges; - + const Sketcher::SketchObject* obj; + if (selection[0].getObject()->isDerivedFrom()) { + obj = static_cast(selection[0].getObject()); + for (auto& name : SubNames) { int geoId = std::atoi(name.substr(4, 4000).c_str()) - 1; - if (geoId >= 0) { - const Part::Geometry* geo = getSketchObject()->getGeometry(geoId); - if (isLineSegment(*geo)) { - ++selectedLines; - } - else { - ++selectedConics; + const Part::Geometry* geo = getSketchObject()->getGeometry(geoId); + if (name.substr(0, 4) == "Edge") { + ++selectedEdges; + + if (geoId >= 0) { + if (isLineSegment(*geo)) { + ++selectedLines; + } + else if (geo->is()) { + ++selectedBsplines; + } + else { + ++selectedConics; + } } } - } - else if (name.substr(0, 4) == "Vert") { - ++selectedPoints; - } - else if (name.substr(0, 4) == "Cons") { - ++selectedConstraints; + else if (name.substr(0, 4) == "Vert") { + ++selectedPoints; + Sketcher::PointPos posId; + getIdsFromName(name, obj, geoId, posId); + if (isBsplineKnotOrEndPoint(obj, geoId, posId)) { + ++selectedBsplineKnots; + } + if (Sketcher::PointPos::start != posId || Sketcher::PointPos::end != posId) { + ++selectedEndPoints; + } + } + else if (name.substr(0, 4) == "Cons") { + ++selectedConstraints; + } + else if (name.substr(2, 5) == "Axis") { + ++selectedEdges; + ++selectedLines; + ++selectedOrigin; + } + else if (name.substr(0, 4) == "Root") { + ++selectedPoints; + ++selectedOrigin; + } } } + if (selectedPoints + selectedEdges == selectedOrigin) { + onlyOrigin = true; + } // build context menu items depending on the selection - if (selectedEdges >= 1 && selectedPoints == 0) { + if (selectedBsplines > 0 && selectedBsplines == selectedEdges && selectedPoints == 0 + && !onlyOrigin) { + menu << "Sketcher_BSplineInsertKnot" + << "Sketcher_BSplineIncreaseDegree" + << "Sketcher_BSplineDecreaseDegree"; + } + else if (selectedBsplineKnots > 0 && selectedBsplineKnots == selectedPoints + && selectedEdges == 0 && !onlyOrigin) { + if (selectedBsplineKnots == 1) { + menu << "Sketcher_BSplineIncreaseKnotMultiplicity" + << "Sketcher_BSplineDecreaseKnotMultiplicity"; + } + } + if (selectedEdges >= 1 && selectedPoints == 0 && selectedBsplines == 0 && !onlyOrigin) { menu << "Sketcher_Dimension"; if (selectedConics == 0) { menu << "Sketcher_ConstrainHorVer" @@ -3948,9 +3992,9 @@ void ViewProviderSketch::generateContextMenu() menu << "Sketcher_ConstrainTangent"; } } - else if (selectedEdges == 1 && selectedPoints >= 1) { + else if (selectedEdges == 1 && selectedPoints >= 1 && !onlyOrigin) { menu << "Sketcher_Dimension"; - if (selectedConics == 0) { + if (selectedConics == 0 && selectedBsplines == 0) { menu << "Sketcher_ConstrainCoincidentUnified" << "Sketcher_ConstrainHorVer" << "Sketcher_ConstrainVertical" @@ -3958,6 +4002,10 @@ void ViewProviderSketch::generateContextMenu() if (selectedPoints == 2) { menu << "Sketcher_ConstrainSymmetric"; } + if (selectedPoints == 1) { + menu << "Sketcher_ConstrainPerpendicular" + << "Sketcher_ConstrainTangent"; + } } else { menu << "Sketcher_ConstrainCoincidentUnified" @@ -3965,7 +4013,7 @@ void ViewProviderSketch::generateContextMenu() << "Sketcher_ConstrainTangent"; } } - else if (selectedEdges == 0 && selectedPoints >= 1) { + else if (selectedEdges == 0 && selectedPoints >= 1 && !onlyOrigin) { menu << "Sketcher_Dimension"; if (selectedPoints > 1) { @@ -3974,8 +4022,15 @@ void ViewProviderSketch::generateContextMenu() << "Sketcher_ConstrainVertical" << "Sketcher_ConstrainHorizontal"; } + if (selectedPoints == 2) { + menu << "Sketcher_ConstrainPerpendicular" + << "Sketcher_ConstrainTangent"; + if (selectedEndPoints == 2) { + menu << "Sketcher_JoinCurves"; + } + } } - else if (selectedLines >= 1 && selectedPoints >= 1) { + else if (selectedLines >= 1 && selectedPoints >= 1 && !onlyOrigin) { menu << "Sketcher_Dimension" << "Sketcher_ConstrainHorVer" << "Sketcher_ConstrainVertical" From c3091337c51726850d9893009bd788d322514d0a Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Mon, 22 Jan 2024 12:44:28 -0600 Subject: [PATCH 7/9] Part/TopoShapeMapper: Add missing PreCompiled include --- src/Mod/Part/App/TopoShapeMapper.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Mod/Part/App/TopoShapeMapper.cpp b/src/Mod/Part/App/TopoShapeMapper.cpp index 69f9653d18..4268a433d2 100644 --- a/src/Mod/Part/App/TopoShapeMapper.cpp +++ b/src/Mod/Part/App/TopoShapeMapper.cpp @@ -1,3 +1,5 @@ +#include "PreCompiled.h" + #include "TopoShapeMapper.h" namespace Part From deed6df1b2efd426d96ee7db3f5922d0d31cb8ae Mon Sep 17 00:00:00 2001 From: wandererfan Date: Mon, 22 Jan 2024 09:46:34 -0500 Subject: [PATCH 8/9] [TD]fix line number not saved for cosmetic edge --- src/Mod/TechDraw/App/Cosmetic.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/Mod/TechDraw/App/Cosmetic.cpp b/src/Mod/TechDraw/App/Cosmetic.cpp index aa47615f21..5b65996ff1 100644 --- a/src/Mod/TechDraw/App/Cosmetic.cpp +++ b/src/Mod/TechDraw/App/Cosmetic.cpp @@ -257,6 +257,8 @@ unsigned int CosmeticEdge::getMemSize () const void CosmeticEdge::Save(Base::Writer &writer) const { + // TODO: this should be using m_format->Save(writer) instead of saving the individual + // fields. writer.Stream() << writer.ind() << "