Gui: add coinRemoveAllChildren to work around Coin3D bug

See bug description: https://bitbucket.org/Coin3D/coin/pull-requests/119/fix-sochildlist-auditing/diff Because of path based rendering (SoFCPathAnnotation) in mouse over highlight, this bug causes crash more frequently here comparing to upstream. All C++ calling of SoGroup::removeAllChildren() is replaced by Gui::coinRemoveAllChildren(), and python code is fixed by monkey patching SoGroup.removeAllChildren() in FreeCADGuiInit.py.
FreeCAD · Jul 14, 2019 · 53961ec · 53961ec
1 parent a415b35
commit 53961ec
Show file tree

Hide file tree

Showing 36 changed files with 115 additions and 55 deletions.
diff --git a/src/Gui/Application.h b/src/Gui/Application.h
@@ -243,6 +243,8 @@ class GuiExport Application
     static PyObject* sInsert                   (PyObject *self,PyObject *args); // open Python scripts
     static PyObject* sExport                   (PyObject *self,PyObject *args);
 
+    static PyObject* sCoinRemoveAllChildren    (PyObject *self,PyObject *args);
+
     static PyObject* sActiveDocument           (PyObject *self,PyObject *args);
     static PyObject* sSetActiveDocument        (PyObject *self,PyObject *args);
     static PyObject* sActiveView               (PyObject *self,PyObject *args);

diff --git a/src/Gui/ApplicationPy.cpp b/src/Gui/ApplicationPy.cpp
@@ -197,6 +197,9 @@ PyMethodDef Application::Methods[] = {
      "removeDocumentObserver() -> None\n\n"
      "Remove an added document observer."},
 
+  {"coinRemoveAllChildren",     (PyCFunction) Application::sCoinRemoveAllChildren, METH_VARARGS,
+   "Remove all children from a group node"},
+
   {NULL, NULL, 0, NULL}		/* Sentinel */
 };
 
@@ -1327,3 +1330,18 @@ PyObject* Application::sRemoveDocObserver(PyObject * /*self*/, PyObject *args)
         Py_Return;
     } PY_CATCH;
 }
+
+PyObject* Application::sCoinRemoveAllChildren(PyObject * /*self*/, PyObject *args)
+{
+    PyObject *pynode;
+    if (!PyArg_ParseTuple(args, "O", &pynode))
+        return NULL;
+
+    PY_TRY {
+        void* ptr = 0;
+        Base::Interpreter().convertSWIGPointerObj("pivy.coin","_p_SoGroup", pynode, &ptr, 0);
+        coinRemoveAllChildren(reinterpret_cast<SoGroup*>(ptr));
+        Py_Return;
+    }PY_CATCH;
+}
+
diff --git a/src/Gui/Document.cpp b/src/Gui/Document.cpp
@@ -1559,7 +1559,7 @@ void Document::handleChildren3D(ViewProvider* viewProvider)
         if (childGroup->getNumChildren() != static_cast<int>(children.size())) {
 
             rebuild = true;
-            childGroup->removeAllChildren();
+            Gui::coinRemoveAllChildren(childGroup);
 
             for (std::vector<App::DocumentObject*>::iterator it=children.begin();it!=children.end();++it) {
                 ViewProvider* ChildViewProvider = getViewProvider(*it);

diff --git a/src/Gui/FreeCADGuiInit.py b/src/Gui/FreeCADGuiInit.py
@@ -171,6 +171,21 @@ def InitApplications():
 
 Gui.addWorkbench(NoneWorkbench())
 
+# Monkey patching pivy.coin.SoGroup.removeAllChildren to work around a bug
+# https://bitbucket.org/Coin3D/coin/pull-requests/119/fix-sochildlist-auditing/diff
+
+def _SoGroup_init(self,*args):
+    import types
+    _SoGroup_init_orig(self,*args)
+    self.removeAllChildren = \
+        types.MethodType(FreeCADGui.coinRemoveAllChildren,self)
+try:
+    from pivy import coin
+    _SoGroup_init_orig = coin.SoGroup.__init__
+    coin.SoGroup.__init__ = _SoGroup_init
+except Exception:
+    pass
+
 # init modules
 InitApplications()
 

diff --git a/src/Gui/ManualAlignment.cpp b/src/Gui/ManualAlignment.cpp
@@ -848,8 +848,8 @@ void ManualAlignment::startAlignment(Base::Type mousemodel)
 void ManualAlignment::continueAlignment()
 {
     myFixedGroup.clearPoints();
-    d->picksepLeft->removeAllChildren();
-    d->picksepRight->removeAllChildren();
+    coinRemoveAllChildren(d->picksepLeft);
+    coinRemoveAllChildren(d->picksepRight);
 
     if (!myAlignModel.isEmpty()) {
         AlignmentGroup& grp = myAlignModel.activeGroup();
@@ -891,8 +891,8 @@ void ManualAlignment::reset()
     myFixedGroup.setAlignable(false);
     myFixedGroup.clear();
 
-    d->picksepLeft->removeAllChildren();
-    d->picksepRight->removeAllChildren();
+    coinRemoveAllChildren(d->picksepLeft);
+    coinRemoveAllChildren(d->picksepRight);
 
     if (myDocument) {
         this->connectDocumentDeletedObject.disconnect();
@@ -1186,8 +1186,8 @@ void ManualAlignment::onClear()
     myAlignModel.activeGroup().clear();
     myFixedGroup.clear();
 
-    d->picksepLeft->removeAllChildren();
-    d->picksepRight->removeAllChildren();
+    coinRemoveAllChildren(d->picksepLeft);
+    coinRemoveAllChildren(d->picksepRight);
 }
 
 void ManualAlignment::onCancel()

diff --git a/src/Gui/SoFCColorGradient.cpp b/src/Gui/SoFCColorGradient.cpp
@@ -39,6 +39,7 @@
 #include "DlgSettingsColorGradientImp.h"
 #include "MainWindow.h"
 #include "MDIView.h"
+#include "ViewProvider.h"
 
 using namespace Gui;
 
@@ -83,7 +84,7 @@ void SoFCColorGradient::finish()
 
 void SoFCColorGradient::setMarkerLabel( const SoMFString& label )
 {
-    labels->removeAllChildren();
+    coinRemoveAllChildren(labels);
 
     float fH=8.0f;
     int num = label.getNum();
@@ -294,7 +295,7 @@ void SoFCColorGradient::rebuildGradient()
 
     // first clear the children
     if ( getNumChildren() > 0 )
-        removeAllChildren();
+        coinRemoveAllChildren(this);
     addChild(ttype);
     addChild(labels);
     addChild(coords);

diff --git a/src/Gui/SoFCColorLegend.cpp b/src/Gui/SoFCColorLegend.cpp
@@ -34,6 +34,7 @@
 # include <Inventor/nodes/SoTransform.h>
 #endif
 
+#include "ViewProvider.h"
 #include "SoFCColorLegend.h"
 
 
@@ -81,7 +82,7 @@ void SoFCColorLegend::finish()
 
 void SoFCColorLegend::setMarkerLabel( const SoMFString& label )
 {
-  labels->removeAllChildren();
+  coinRemoveAllChildren(labels);
 
   int num = label.getNum();
   if ( num > 1 )
@@ -234,7 +235,7 @@ void SoFCColorLegend::setColorModel( App::ColorGradient::TColorModel tModel )
 
   // first clear the children
   if ( getNumChildren() > 0 )
-    removeAllChildren();
+    coinRemoveAllChildren(this);
   addChild(labels);
   addChild(coords);
   addChild(mat);

diff --git a/src/Gui/View3DInventorViewer.cpp b/src/Gui/View3DInventorViewer.cpp
@@ -611,7 +611,7 @@ View3DInventorViewer::~View3DInventorViewer()
     // the root node but isn't destroyed when closing this viewer so
     // that it prevents all children from being deleted. To reduce this
     // likelihood we explicitly remove all child nodes now.
-    this->pcViewProviderRoot->removeAllChildren();
+    coinRemoveAllChildren(this->pcViewProviderRoot);
     this->pcViewProviderRoot->unref();
     this->pcViewProviderRoot = 0;
     this->backlight->unref();
@@ -3307,8 +3307,8 @@ void View3DInventorViewer::turnAllDimensionsOff()
 
 void View3DInventorViewer::eraseAllDimensions()
 {
-    static_cast<SoSwitch*>(dimensionRoot->getChild(0))->removeAllChildren();
-    static_cast<SoSwitch*>(dimensionRoot->getChild(1))->removeAllChildren();
+    coinRemoveAllChildren(static_cast<SoSwitch*>(dimensionRoot->getChild(0)));
+    coinRemoveAllChildren(static_cast<SoSwitch*>(dimensionRoot->getChild(1)));
 }
 
 void View3DInventorViewer::turn3dDimensionsOn()

diff --git a/src/Gui/ViewProvider.cpp b/src/Gui/ViewProvider.cpp
@@ -68,6 +68,24 @@ using namespace std;
 using namespace Gui;
 
 
+namespace Gui {
+
+void coinRemoveAllChildren(SoGroup *group) {
+    if(!group)
+        return;
+    int count = group->getNumChildren();
+    if(!count)
+        return;
+    FC_TRACE("coin remove all children " << count);
+    SbBool autonotify = group->enableNotify(FALSE);
+    for(;count>0;--count)
+        group->removeChild(count-1);
+    group->enableNotify(autonotify);
+    group->touch();
+}
+
+} // namespace Gui
+
 //**************************************************************************
 //**************************************************************************
 // ViewProvider

diff --git a/src/Gui/ViewProvider.h b/src/Gui/ViewProvider.h
@@ -102,6 +102,11 @@ class CoinPtr: public boost::intrusive_ptr<T> {
     }
 };
 
+/** Helper function to deal with bug in SoNode::removeAllChildren()
+ *
+ * @sa https://bitbucket.org/Coin3D/coin/pull-requests/119/fix-sochildlist-auditing/diff
+ */
+void GuiExport coinRemoveAllChildren(SoGroup *node);
 
 /** General interface for all visual stuff in FreeCAD
   * This class is used to generate and handle all around

diff --git a/src/Gui/ViewProviderInventorObject.cpp b/src/Gui/ViewProviderInventorObject.cpp
@@ -95,7 +95,7 @@ void ViewProviderInventorObject::updateData(const App::Property* prop)
         // read from buffer
         SoInput in;
         std::string buffer = ivObj->Buffer.getValue();
-        pcBuffer->removeAllChildren();
+        coinRemoveAllChildren(pcBuffer);
         if (buffer.empty()) return;
         in.setBuffer((void *)buffer.c_str(), buffer.size());
         SoSeparator * node = SoDB::readAll(&in);
@@ -112,7 +112,7 @@ void ViewProviderInventorObject::updateData(const App::Property* prop)
         QString fn = QString::fromUtf8(filename);
         QFile file(fn);
         SoInput in;
-        pcFile->removeAllChildren();
+        coinRemoveAllChildren(pcFile);
         if (!fn.isEmpty() && file.open(QFile::ReadOnly)) {
             QByteArray buffer = file.readAll();
             in.setBuffer((void *)buffer.constData(), buffer.length());

diff --git a/src/Gui/ViewProviderVRMLObject.cpp b/src/Gui/ViewProviderVRMLObject.cpp
@@ -216,7 +216,7 @@ void ViewProviderVRMLObject::updateData(const App::Property* prop)
         QString fn = QString::fromUtf8(filename);
         QFile file(fn);
         SoInput in;
-        pcVRML->removeAllChildren();
+        coinRemoveAllChildren(pcVRML);
         if (!fn.isEmpty() && file.open(QFile::ReadOnly)) {
             QFileInfo fi(fn);
             QByteArray filepath = fi.absolutePath().toUtf8();

diff --git a/src/Mod/Fem/Gui/ViewProviderFemConstraintBearing.cpp b/src/Mod/Fem/Gui/ViewProviderFemConstraintBearing.cpp
@@ -118,7 +118,7 @@ void ViewProviderFemConstraintBearing::updateData(const App::Property* prop)
 
     if (strcmp(prop->getName(),"BasePoint") == 0) {
         // Remove and recreate the symbol
-        pShapeSep->removeAllChildren();
+        Gui::coinRemoveAllChildren(pShapeSep);
 
         // This should always point outside of the cylinder
         Base::Vector3d normal = pcConstraint->NormalDirection.getValue();

diff --git a/src/Mod/Fem/Gui/ViewProviderFemConstraintContact.cpp b/src/Mod/Fem/Gui/ViewProviderFemConstraintContact.cpp
@@ -121,7 +121,7 @@ void ViewProviderFemConstraintContact::updateData(const App::Property* prop)
         std::vector<Base::Vector3d>::const_iterator n = normals.begin();
 
         // Points and Normals are always updated together
-        pShapeSep->removeAllChildren();
+        Gui::coinRemoveAllChildren(pShapeSep);
 
         for (std::vector<Base::Vector3d>::const_iterator p = points.begin(); p != points.end(); p++) {
             //Define base and normal directions

diff --git a/src/Mod/Fem/Gui/ViewProviderFemConstraintDisplacement.cpp b/src/Mod/Fem/Gui/ViewProviderFemConstraintDisplacement.cpp
@@ -186,7 +186,7 @@ void ViewProviderFemConstraintDisplacement::updateData(const App::Property* prop
         int idrotz = 0;
 #else
         // Note: Points and Normals are always updated together
-        pShapeSep->removeAllChildren();
+        Gui::coinRemoveAllChildren(pShapeSep);
 #endif
 
         for (std::vector<Base::Vector3d>::const_iterator p = points.begin(); p != points.end(); p++) {

diff --git a/src/Mod/Fem/Gui/ViewProviderFemConstraintFixed.cpp b/src/Mod/Fem/Gui/ViewProviderFemConstraintFixed.cpp
@@ -143,7 +143,7 @@ void ViewProviderFemConstraintFixed::updateData(const App::Property* prop)
         int idx = 0;
 #else
         // Note: Points and Normals are always updated together
-        pShapeSep->removeAllChildren();
+        Gui::coinRemoveAllChildren(pShapeSep);
 #endif
 
         for (std::vector<Base::Vector3d>::const_iterator p = points.begin(); p != points.end(); p++) {

diff --git a/src/Mod/Fem/Gui/ViewProviderFemConstraintFluidBoundary.cpp b/src/Mod/Fem/Gui/ViewProviderFemConstraintFluidBoundary.cpp
@@ -170,7 +170,7 @@ void ViewProviderFemConstraintFluidBoundary::updateData(const App::Property* pro
             int idx = 0;
 #else
             // Redraw all arrows
-            pShapeSep->removeAllChildren();
+            Gui::coinRemoveAllChildren(pShapeSep);
 #endif
             // This should always point outside of the solid
             Base::Vector3d normal = pcConstraint->NormalDirection.getValue();
@@ -272,7 +272,7 @@ void ViewProviderFemConstraintFluidBoundary::updateData(const App::Property* pro
             int idx = 0;
 #else
             // Note: Points and Normals are always updated together
-            pShapeSep->removeAllChildren();
+            Gui::coinRemoveAllChildren(pShapeSep);
 #endif
 
             for (std::vector<Base::Vector3d>::const_iterator p = points.begin(); p != points.end(); p++) {

diff --git a/src/Mod/Fem/Gui/ViewProviderFemConstraintForce.cpp b/src/Mod/Fem/Gui/ViewProviderFemConstraintForce.cpp
@@ -139,7 +139,7 @@ void ViewProviderFemConstraintForce::updateData(const App::Property* prop)
         int idx = 0;
 #else
         // Redraw all arrows
-        pShapeSep->removeAllChildren();
+        Gui::coinRemoveAllChildren(pShapeSep);
 #endif
         // This should always point outside of the solid
         Base::Vector3d normal = pcConstraint->NormalDirection.getValue();

diff --git a/src/Mod/Fem/Gui/ViewProviderFemConstraintGear.cpp b/src/Mod/Fem/Gui/ViewProviderFemConstraintGear.cpp
@@ -115,7 +115,7 @@ void ViewProviderFemConstraintGear::updateData(const App::Property* prop)
     if (strcmp(prop->getName(),"BasePoint") == 0) {
         if (pcConstraint->Height.getValue() > Precision::Confusion()) {
             // Remove and recreate the symbol
-            pShapeSep->removeAllChildren();
+            Gui::coinRemoveAllChildren(pShapeSep);
 
             Base::Vector3d base = pcConstraint->BasePoint.getValue();
             Base::Vector3d axis = pcConstraint->Axis.getValue();

diff --git a/src/Mod/Fem/Gui/ViewProviderFemConstraintHeatflux.cpp b/src/Mod/Fem/Gui/ViewProviderFemConstraintHeatflux.cpp
@@ -120,7 +120,7 @@ void ViewProviderFemConstraintHeatflux::updateData(const App::Property* prop)
         std::vector<Base::Vector3d>::const_iterator n = normals.begin();
 
         // Note: Points and Normals are always updated together
-        pShapeSep->removeAllChildren();
+        Gui::coinRemoveAllChildren(pShapeSep);
 
         for (std::vector<Base::Vector3d>::const_iterator p = points.begin(); p != points.end(); p++) {
             //Define base and normal directions

diff --git a/src/Mod/Fem/Gui/ViewProviderFemConstraintPlaneRotation.cpp b/src/Mod/Fem/Gui/ViewProviderFemConstraintPlaneRotation.cpp
@@ -119,7 +119,7 @@ void ViewProviderFemConstraintPlaneRotation::updateData(const App::Property* pro
         std::vector<Base::Vector3d>::const_iterator n = normals.begin();
 
         // Points and Normals are always updated together
-        pShapeSep->removeAllChildren();
+        Gui::coinRemoveAllChildren(pShapeSep);
 
         for (std::vector<Base::Vector3d>::const_iterator p = points.begin(); p != points.end(); p++) {
             //Define base and normal directions

diff --git a/src/Mod/Fem/Gui/ViewProviderFemConstraintPressure.cpp b/src/Mod/Fem/Gui/ViewProviderFemConstraintPressure.cpp
@@ -127,7 +127,7 @@ void ViewProviderFemConstraintPressure::updateData(const App::Property* prop)
         int idx = 0;
 #else
         // Redraw all arrows
-        pShapeSep->removeAllChildren();
+        Gui::coinRemoveAllChildren(pShapeSep);
 #endif
 
         for (std::vector<Base::Vector3d>::const_iterator p = points.begin(); p != points.end(); p++) {

diff --git a/src/Mod/Fem/Gui/ViewProviderFemConstraintPulley.cpp b/src/Mod/Fem/Gui/ViewProviderFemConstraintPulley.cpp
@@ -114,7 +114,7 @@ void ViewProviderFemConstraintPulley::updateData(const App::Property* prop)
     if (strcmp(prop->getName(),"BasePoint") == 0) {
         if (pcConstraint->Height.getValue() > Precision::Confusion()) {
             // Remove and recreate the symbol
-            pShapeSep->removeAllChildren();
+            Gui::coinRemoveAllChildren(pShapeSep);
 
             // This should always point outside of the cylinder
             Base::Vector3d base = pcConstraint->BasePoint.getValue();

diff --git a/src/Mod/Fem/Gui/ViewProviderFemConstraintTemperature.cpp b/src/Mod/Fem/Gui/ViewProviderFemConstraintTemperature.cpp
@@ -119,7 +119,7 @@ void ViewProviderFemConstraintTemperature::updateData(const App::Property* prop)
         std::vector<Base::Vector3d>::const_iterator n = normals.begin();
 
         // Note: Points and Normals are always updated together
-        pShapeSep->removeAllChildren();
+        Gui::coinRemoveAllChildren(pShapeSep);
 
         for (std::vector<Base::Vector3d>::const_iterator p = points.begin(); p != points.end(); p++) {
             //Define base and normal directions

diff --git a/src/Mod/Fem/Gui/ViewProviderFemConstraintTransform.cpp b/src/Mod/Fem/Gui/ViewProviderFemConstraintTransform.cpp
@@ -135,7 +135,7 @@ void ViewProviderFemConstraintTransform::updateData(const App::Property* prop)
         std::vector<Base::Vector3d>::const_iterator n = normals.begin();
 
         // Points and Normals are always updated together
-        pShapeSep->removeAllChildren();
+        Gui::coinRemoveAllChildren(pShapeSep);
 
         for (std::vector<Base::Vector3d>::const_iterator p = points.begin(); p != points.end(); p++) {
             SbVec3f base(p->x, p->y, p->z);
@@ -269,7 +269,7 @@ void ViewProviderFemConstraintTransform::updateData(const App::Property* prop)
     } else if (transform_type == "Cylindrical") {
 
         // Points and Normals are always updated together
-        pShapeSep->removeAllChildren();
+        Gui::coinRemoveAllChildren(pShapeSep);
 
         const std::vector<Base::Vector3d>& points = pcConstraint->Points.getValues();
         const std::vector<Base::Vector3d>& normals = pcConstraint->Normals.getValues();

diff --git a/src/Mod/Inspection/Gui/ViewProviderInspection.cpp b/src/Mod/Inspection/Gui/ViewProviderInspection.cpp
@@ -232,7 +232,7 @@ void ViewProviderInspection::updateData(const App::Property* prop)
                 }
             }
 
-            this->pcLinkRoot->removeAllChildren();
+            Gui::coinRemoveAllChildren(this->pcLinkRoot);
             this->pcLinkRoot->addChild(this->pcCoords);
             this->pcCoords->point.setNum(points.size());
             SbVec3f* pts = this->pcCoords->point.startEditing();