From c345e4e7287738c923faf094b1f0d772e8b4aeee Mon Sep 17 00:00:00 2001 From: wmayer Date: Tue, 25 Sep 2018 22:51:33 +0200 Subject: [PATCH] fixes 0003469: FreeCAD crashes on edge selection during fillet edge operation on extruded geometry --- src/Gui/Selection.cpp | 41 +++++++++++++++++++++------------- src/Mod/Part/App/TopoShape.cpp | 2 ++ 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/src/Gui/Selection.cpp b/src/Gui/Selection.cpp index b4af4c5951..9e05d11a24 100644 --- a/src/Gui/Selection.cpp +++ b/src/Gui/Selection.cpp @@ -813,7 +813,11 @@ bool SelectionSingleton::addSelection(const SelectionObject& obj) void SelectionSingleton::rmvSelection(const char* pDocName, const char* pObjectName, const char* pSubName) { - std::vector rmvList; + bool foundSelection = false; + std::string tmpDocName; + std::string tmpFeaName; + std::string tmpSubName; + std::string tmpTypName; for (std::list<_SelObj>::iterator It = _SelList.begin();It != _SelList.end();) { if ((It->DocName == pDocName && !pObjectName) || @@ -821,25 +825,16 @@ void SelectionSingleton::rmvSelection(const char* pDocName, const char* pObjectN (It->DocName == pDocName && pObjectName && It->FeatName == pObjectName && pSubName && It->SubName == pSubName)) { // save in tmp. string vars - std::string tmpDocName = It->DocName; - std::string tmpFeaName = It->FeatName; - std::string tmpSubName = It->SubName; - std::string tmpTypName = It->TypeName; + tmpDocName = It->DocName; + tmpFeaName = It->FeatName; + tmpSubName = It->SubName; + tmpTypName = It->TypeName; // destroy the _SelObj item It = _SelList.erase(It); - SelectionChanges Chng; - Chng.pDocName = tmpDocName.c_str(); - Chng.pObjectName = tmpFeaName.c_str(); - Chng.pSubName = tmpSubName.c_str(); - Chng.pTypeName = tmpTypName.c_str(); - Chng.Type = SelectionChanges::RmvSelection; + foundSelection = true; - Notify(Chng); - signalSelectionChanged(Chng); - - rmvList.push_back(Chng); #ifdef FC_DEBUG Base::Console().Log("Sel : Rmv Selection \"%s.%s.%s\"\n",pDocName,pObjectName,pSubName); #endif @@ -848,6 +843,22 @@ void SelectionSingleton::rmvSelection(const char* pDocName, const char* pObjectN ++It; } } + + // NOTE: It can happen that there are nested calls of rmvSelection() + // so that it's not safe to invoke the notifications inside the loop + // as this can invalidate the iterators and thus leads to undefined + // behaviour. + // So, the notification is done after the loop, see also #0003469 + if (foundSelection) { + SelectionChanges Chng; + Chng.pDocName = tmpDocName.c_str(); + Chng.pObjectName = tmpFeaName.c_str(); + Chng.pSubName = tmpSubName.c_str(); + Chng.pTypeName = tmpTypName.c_str(); + Chng.Type = SelectionChanges::RmvSelection; + Notify(Chng); + signalSelectionChanged(Chng); + } } void SelectionSingleton::setSelection(const char* pDocName, const std::vector& sel) diff --git a/src/Mod/Part/App/TopoShape.cpp b/src/Mod/Part/App/TopoShape.cpp index 2aea7da5c4..5dc0f1c24d 100644 --- a/src/Mod/Part/App/TopoShape.cpp +++ b/src/Mod/Part/App/TopoShape.cpp @@ -3460,6 +3460,8 @@ TopoDS_Shape TopoShape::defeaturing(const std::vector& s) const */ TopoDS_Shape TopoShape::makeShell(const TopoDS_Shape& input) const { + // For comparison see also: + // GEOMImpl_BooleanDriver::makeCompoundShellFromFaces if (input.IsNull()) return input; if (input.ShapeType() != TopAbs_COMPOUND)