Merge pull request #12885 from bgbsww/bgbsww-toponamingAttachment

Toponaming/Part:  Bring over attacher differences

src/Mod/Part/App/Attacher.h

@@ -30,8 +30,10 @@
 
 #include <GProp_GProps.hxx>
 
+#include <App/DocumentObserver.h>
 #include <App/GeoFeature.h>
 #include <App/PropertyLinks.h>
+#include <App/PropertyStandard.h>
 #include <Base/Exception.h>
 #include <Base/Placement.h>
 
@@ -222,7 +224,17 @@ public: //methods
                       const Base::Placement &attachmentOffset = Base::Placement());
     virtual void setUp(const AttachEngine &another);
     virtual AttachEngine* copy() const = 0;
-    virtual Base::Placement calculateAttachedPlacement(const Base::Placement& origPlacement) const = 0;
+
+    Base::Placement calculateAttachedPlacement(
+        const Base::Placement &origPlacement, bool *subChanged=0);
+
+    virtual Base::Placement _calculateAttachedPlacement(
+        const std::vector<App::DocumentObject*> &objs,
+        const std::vector<std::string> &subs,
+        const Base::Placement &origPlacement) const = 0;
+
+    void setReferences(const App::PropertyLinkSubList &references);
+    void setReferences(const std::vector<App::SubObjectT> &references);
 
     /**
      * @brief placementFactory calculates placement from Z axis direction,
@@ -348,6 +360,9 @@ public://helper functions that may be useful outside of the class
 
     static GProp_GProps getInertialPropsOfShape(const std::vector<const TopoDS_Shape*> &shapes);
 
+    std::vector<App::DocumentObject*> getRefObjects() const;
+    const std::vector<std::string> &getSubValues() const {return subnames;}
+
     /**
      * @brief verifyReferencesAreSafe: checks if pointers in references still
      * point to objects contained in open documents. This guarantees the links
@@ -362,6 +377,10 @@ public: //enums
 
 public: //members
     App::PropertyLinkSubList references;
+    std::string docName;
+    std::vector<std::string> objNames;
+    std::vector<std::string> subnames;
+    std::vector<std::string> shadowSubs;
 
     eMapMode mapMode = mmDeactivated;
     bool mapReverse = false;
@@ -405,7 +424,8 @@ protected:
         ret.push_back(rt4);
         return ret;
     }
-    static void readLinks(const App::PropertyLinkSubList &references, std::vector<App::GeoFeature *> &geofs,
+    static void readLinks(const std::vector<App::DocumentObject*> &objs,
+                          const std::vector<std::string> &subs, std::vector<App::GeoFeature *> &geofs,
                           std::vector<const TopoDS_Shape*>& shapes, std::vector<TopoDS_Shape> &storage,
                           std::vector<eRefType> &types);
 
@@ -420,7 +440,10 @@ class PartExport AttachEngine3D : public AttachEngine
 public:
     AttachEngine3D();
     AttachEngine3D* copy() const override;
-    Base::Placement calculateAttachedPlacement(const Base::Placement& origPlacement) const override;
+    Base::Placement _calculateAttachedPlacement(
+        const std::vector<App::DocumentObject*> &objs,
+        const std::vector<std::string> &subs,
+        const Base::Placement &origPlacement) const override;
 private:
     double calculateFoldAngle(gp_Vec axA, gp_Vec axB, gp_Vec edA, gp_Vec edB) const;
 };
@@ -432,7 +455,10 @@ class PartExport AttachEnginePlane : public AttachEngine
 public:
     AttachEnginePlane();
     AttachEnginePlane* copy() const override;
-    Base::Placement calculateAttachedPlacement(const Base::Placement& origPlacement) const override;
+    Base::Placement _calculateAttachedPlacement(
+        const std::vector<App::DocumentObject*> &objs,
+        const std::vector<std::string> &subs,
+        const Base::Placement &origPlacement) const override;
 };
 
 //attacher specialized for datum lines
@@ -442,7 +468,10 @@ class PartExport AttachEngineLine : public AttachEngine
 public:
     AttachEngineLine();
     AttachEngineLine* copy() const override;
-    Base::Placement calculateAttachedPlacement(const Base::Placement& origPlacement) const override;
+    Base::Placement _calculateAttachedPlacement(
+        const std::vector<App::DocumentObject*> &objs,
+        const std::vector<std::string> &subs,
+        const Base::Placement &origPlacement) const override;
 };
 
 //attacher specialized for datum points
@@ -452,7 +481,10 @@ class PartExport AttachEnginePoint : public AttachEngine
 public:
     AttachEnginePoint();
     AttachEnginePoint* copy() const override;
-    Base::Placement calculateAttachedPlacement(const Base::Placement& origPlacement) const override;
+    Base::Placement _calculateAttachedPlacement(
+        const std::vector<App::DocumentObject*> &objs,
+        const std::vector<std::string> &subs,
+        const Base::Placement &origPlacement) const override;
 
 private:
     gp_Pnt getProximityPoint(eMapMode mode, const TopoDS_Shape& s1, const TopoDS_Shape& s2) const;

src/Mod/Part/App/TopoShape.cpp

@@ -336,50 +336,19 @@ Data::Segment* TopoShape::getSubElement(const char* Type, unsigned long n) const
     return new ShapeSegment(getSubShape(temp.c_str()));
 }
 
-TopoDS_Shape TopoShape::getSubShape(const char* Type, bool silent) const
-{
-    auto res = shapeTypeAndIndex(Type);
-    return getSubShape(res.first,res.second,silent);
+TopoDS_Shape TopoShape::getSubShape(const char* Type, bool silent) const {
+    TopoShape s(*this);
+    s.Tag = 0;
+    return s.getSubTopoShape(Type,silent).getShape();
 }
 
-TopoDS_Shape TopoShape::getSubShape(TopAbs_ShapeEnum type, int index, bool silent) const
-{
-    if(index <= 0) {
-        if(silent)
-            return {};
-        // TODO: Is this message clear?  Should we complain about the negative index instead
-        Standard_Failure::Raise("Unsupported sub-shape type");
-    }
-
-    if (this->_Shape.IsNull()) {
-        if(silent)
-            return {};
-        Standard_Failure::Raise("Cannot get sub-shape from empty shape");
-    }
-
-    try {
-        if(type == TopAbs_SHAPE) {
-            int i=1;
-            for(TopoDS_Iterator it(_Shape);it.More();it.Next(),++i) {
-                if(i == index)
-                    return it.Value();
-            }
-        } else {
-            TopTools_IndexedMapOfShape anIndices;
-            TopExp::MapShapes(this->_Shape, type, anIndices);
-            if(index <= anIndices.Extent())
-                return anIndices.FindKey(index);
-        }
-    } catch(Standard_Failure &) {
-        if(silent)
-            return {};
-        throw;
-    }
-    if(!silent)
-        Standard_Failure::Raise("Index out of bound");
-    return {};
+TopoDS_Shape TopoShape::getSubShape(TopAbs_ShapeEnum type, int idx, bool silent) const {
+    TopoShape s(*this);
+    s.Tag = 0;
+    return s.getSubTopoShape(type,idx,silent).getShape();
 }
 
+
 unsigned long TopoShape::countSubShapes(const char* Type) const
 {
     if(!Type)

src/Mod/Part/TestPartApp.py

@@ -170,7 +170,9 @@ class PartTestBSplineCurve(unittest.TestCase):
             box.getElement("InvalidName")
         with self.assertRaises(ValueError):
             box.getElement("Face6_abc")
-        with self.assertRaises(Part.OCCError):
+        # getSubTopoShape now catches this before it gets to OCC, so the error changes:
+        # with self.assertRaises(Part.OCCError):
+        with self.assertRaises(IndexError):
             box.getElement("Face7")
 
     def tearDown(self):

tests/src/Mod/Part/App/Attacher.cpp

@@ -0,0 +1,107 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+
+#include "gtest/gtest.h"
+#include "PartTestHelpers.h"
+
+#include <src/App/InitApplication.h>
+#include <App/Document.h>
+#include <Mod/Part/App/Attacher.h>
+
+using namespace Part;
+using namespace Attacher;
+using namespace PartTestHelpers;
+
+/*
+ * Testing note:  It looks like there are about 45 different attachment modes, and these tests all
+ * only look at one of them - to prove that adding elementMap code doesn't break anything.  A
+ * comprehensive test of the Attacher would definitely want to try many more code paths.
+ */
+
+class AttacherTest: public ::testing::Test, public PartTestHelpers::PartTestHelperClass
+{
+protected:
+    static void SetUpTestSuite()
+    {
+        tests::initApplication();
+    }
+
+    void SetUp() override
+    {
+        createTestDoc();
+        _boxes[1]->MapReversed.setValue(false);
+        _boxes[1]->AttachmentSupport.setValue(_boxes[0]);
+        _boxes[1]->MapPathParameter.setValue(0.0);
+        _boxes[1]->MapMode.setValue("ObjectXY");  // There are lots of attachment modes!
+        _boxes[1]->recomputeFeature();
+    }
+
+    void TearDown() override
+    {
+        App::GetApplication().closeDocument(_docName.c_str());
+    }
+
+    App::Document* getDocument() const
+    {
+        return _doc;
+    }
+
+protected:
+};
+
+TEST_F(AttacherTest, TestSetReferences)
+{
+    auto& attacher = _boxes[1]->attacher();
+    EXPECT_EQ(attacher.getRefObjects().size(), 1);
+}
+
+TEST_F(AttacherTest, TestSuggestMapModes)
+{
+    auto& attacher = _boxes[1]->attacher();
+    SuggestResult result;
+    attacher.suggestMapModes(result);
+    EXPECT_EQ(result.allApplicableModes.size(), 4);
+    EXPECT_EQ(result.allApplicableModes[0], mmObjectXY);
+    EXPECT_EQ(result.allApplicableModes[1], mmObjectXZ);
+    EXPECT_EQ(result.allApplicableModes[2], mmObjectYZ);
+    EXPECT_EQ(result.allApplicableModes[3], mmInertialCS);
+}
+
+TEST_F(AttacherTest, TestGetShapeType)
+{
+    auto& attacher = _boxes[1]->attacher();
+    auto subObjects = _boxes[1]->getSubObjects();
+    auto shapeType = attacher.getShapeType(_boxes[1], "Vertex2");
+    EXPECT_EQ(shapeType, TopAbs_COMPSOLID);
+}
+
+TEST_F(AttacherTest, TestGetInertialPropsOfShape)
+{
+    auto& attacher = _boxes[1]->attacher();
+    std::vector<const TopoDS_Shape*> result;
+    auto faces = _boxes[1]->Shape.getShape().getSubShapes(TopAbs_FACE);
+    result.emplace_back(&faces[0]);
+    auto shapeType = attacher.getInertialPropsOfShape(result);
+    EXPECT_EQ(result.size(), 1);
+    EXPECT_EQ(shapeType.Mass(), 6.0);
+}
+
+TEST_F(AttacherTest, TestGetRefObjects)
+{
+    auto& attacher = _boxes[1]->attacher();
+    auto docObjects = attacher.getRefObjects();
+    EXPECT_EQ(docObjects.size(), 1);
+    EXPECT_STREQ(docObjects.front()->getNameInDocument(), "Part__Box");
+}
+
+TEST_F(AttacherTest, TestCalculateAttachedPlacement)
+{
+    auto& attacher = _boxes[1]->attacher();
+    const Base::Placement orig;
+    auto placement = attacher.calculateAttachedPlacement(orig);
+    EXPECT_EQ(orig.getPosition().x, 0);
+    EXPECT_EQ(orig.getPosition().y, 0);
+    EXPECT_EQ(orig.getPosition().z, 0);
+    EXPECT_EQ(placement.getPosition().x, 0);
+    EXPECT_EQ(placement.getPosition().y, 0);
+    EXPECT_EQ(placement.getPosition().z, 0);
+}

tests/src/Mod/Part/App/CMakeLists.txt

@@ -2,6 +2,7 @@
 target_sources(
     Part_tests_run
         PRIVATE
+            ${CMAKE_CURRENT_SOURCE_DIR}/Attacher.cpp
             ${CMAKE_CURRENT_SOURCE_DIR}/BRepMesh.cpp
             ${CMAKE_CURRENT_SOURCE_DIR}/FeatureChamfer.cpp
             ${CMAKE_CURRENT_SOURCE_DIR}/FeatureCompound.cpp

tests/src/Mod/Part/App/TopoShape.cpp

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: LGPL-2.1-or-later
 
 #include "gtest/gtest.h"
+#include "PartTestHelpers.h"
 #include <Mod/Part/App/TopoShape.h>
 
 // clang-format off
@@ -87,4 +88,21 @@ TEST(TopoShape, TestTypeNull)
     EXPECT_EQ(Part::TopoShape::getTypeAndIndex(nullptr),
               std::make_pair(std::string(), 0UL));
 }
+
+TEST(TopoShape, TestGetSubshape)
+{
+    // Arrange
+    auto [cube1, cube2] = PartTestHelpers::CreateTwoTopoShapeCubes();
+    // Act
+    auto face = cube1.getSubShape("Face2");
+    auto vertex = cube2.getSubShape(TopAbs_VERTEX,2);
+    auto silentFail = cube1.getSubShape("NotThere", true);
+    // Assert
+    EXPECT_EQ(face.ShapeType(), TopAbs_FACE);
+    EXPECT_EQ(vertex.ShapeType(), TopAbs_VERTEX);
+    EXPECT_TRUE(silentFail.IsNull());
+    EXPECT_THROW(cube1.getSubShape("Face7"), Base::IndexError);          // Out of range
+    EXPECT_THROW(cube1.getSubShape("WOOHOO", false), Base::ValueError);  // Invalid
+}
+
 // clang-format on