fixes 0004010: Box Selection + Part -> MakeCompound will crash FreeCAD

FreeCAD · Jun 12, 2019 · b60a736 · b60a736
1 parent 3e65306
commit b60a736
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 14 deletions.
diff --git a/src/Mod/Part/App/FeatureCompound.cpp b/src/Mod/Part/App/FeatureCompound.cpp
@@ -66,21 +66,29 @@ App::DocumentObjectExecReturn *Compound::execute(void)
         TopoDS_Compound comp;
         builder.MakeCompound(comp);
 
+        // avoid duplicates without changing the order
+        // See also ViewProviderCompound::updateData
+        std::set<DocumentObject*> tempLinks;
+
         const std::vector<DocumentObject*>& links = Links.getValues();
         for (std::vector<DocumentObject*>::const_iterator it = links.begin(); it != links.end(); ++it) {
             if (*it && (*it)->getTypeId().isDerivedFrom(Part::Feature::getClassTypeId())) {
                 Part::Feature* fea = static_cast<Part::Feature*>(*it);
-                const TopoDS_Shape& sh = fea->Shape.getValue();
-                if (!sh.IsNull()) {
-                    builder.Add(comp, sh);
-                    TopTools_IndexedMapOfShape faceMap;
-                    TopExp::MapShapes(sh, TopAbs_FACE, faceMap);
-                    ShapeHistory hist;
-                    hist.type = TopAbs_FACE;
-                    for (int i=1; i<=faceMap.Extent(); i++) {
-                        hist.shapeMap[i-1].push_back(countFaces++);
+
+                auto pos = tempLinks.insert(fea);
+                if (pos.second) {
+                    const TopoDS_Shape& sh = fea->Shape.getValue();
+                    if (!sh.IsNull()) {
+                        builder.Add(comp, sh);
+                        TopTools_IndexedMapOfShape faceMap;
+                        TopExp::MapShapes(sh, TopAbs_FACE, faceMap);
+                        ShapeHistory hist;
+                        hist.type = TopAbs_FACE;
+                        for (int i=1; i<=faceMap.Extent(); i++) {
+                            hist.shapeMap[i-1].push_back(countFaces++);
+                        }
+                        history.push_back(hist);
                     }
-                    history.push_back(hist);
                 }
             }
         }

diff --git a/src/Mod/Part/Gui/Command.cpp b/src/Mod/Part/Gui/Command.cpp
@@ -909,11 +909,15 @@ void CmdPartCompound::activated(int iMsg)
 
     std::vector<Gui::SelectionSingleton::SelObj> Sel = getSelection().getSelection();
     std::stringstream str;
-    std::vector<std::string> tempSelNames;
+
+    // avoid duplicates without changing the order
+    std::set<std::string> tempSelNames;
     str << "App.activeDocument()." << FeatName << ".Links = [";
-    for (std::vector<Gui::SelectionSingleton::SelObj>::iterator it = Sel.begin(); it != Sel.end(); ++it){
-        str << "App.activeDocument()." << it->FeatName << ",";
-        tempSelNames.push_back(it->FeatName);
+    for (std::vector<Gui::SelectionSingleton::SelObj>::iterator it = Sel.begin(); it != Sel.end(); ++it) {
+        auto pos = tempSelNames.insert(it->FeatName);
+        if (pos.second) {
+            str << "App.activeDocument()." << it->FeatName << ",";
+        }
     }
     str << "]";
 

diff --git a/src/Mod/Part/Gui/ViewProviderCompound.cpp b/src/Mod/Part/Gui/ViewProviderCompound.cpp
@@ -72,6 +72,24 @@ void ViewProviderCompound::updateData(const App::Property* prop)
             (prop)->getValues();
         Part::Compound* objComp = static_cast<Part::Compound*>(getObject());
         std::vector<App::DocumentObject*> sources = objComp->Links.getValues();
+
+        if (hist.size() != sources.size()) {
+            // avoid duplicates without changing the order
+            // See also Compound::execute
+            std::set<App::DocumentObject*> tempSources;
+            std::vector<App::DocumentObject*> filter;
+            for (std::vector<App::DocumentObject*>::iterator it = sources.begin(); it != sources.end(); ++it) {
+                Part::Feature* objBase = dynamic_cast<Part::Feature*>(*it);
+                if (objBase) {
+                    auto pos = tempSources.insert(objBase);
+                    if (pos.second) {
+                        filter.push_back(objBase);
+                    }
+                }
+            }
+
+            sources = filter;
+        }
         if (hist.size() != sources.size())
             return;