From: mpv <mpv@opencascade.com>
Date: Mon, 11 Apr 2016 13:08:54 +0000 (+0300)
Subject: Fixes for crashes and bad behavior on delete of features and Parts. For now delete... 
X-Git-Tag: V_2.3.0~248
X-Git-Url: http://git.salome-platform.org/gitweb/?a=commitdiff_plain;h=bc13ffbc8c0dbdce3805bf9362ee8a3c05adf9b4;p=modules%2Fshaper.git

Fixes for crashes and bad behavior on delete of features and Parts. For now delete of Part with content crashes the application (problem in OB).
---

diff --git a/src/FeaturesPlugin/FeaturesPlugin_CompositeSketch.cpp b/src/FeaturesPlugin/FeaturesPlugin_CompositeSketch.cpp
index b8bd1ee23..5e7c13d43 100644
--- a/src/FeaturesPlugin/FeaturesPlugin_CompositeSketch.cpp
+++ b/src/FeaturesPlugin/FeaturesPlugin_CompositeSketch.cpp
@@ -104,19 +104,6 @@ void FeaturesPlugin_CompositeSketch::removeFeature(std::shared_ptr<ModelAPI_Feat
   reference(SKETCH_ID())->setValue(ObjectPtr());
 }
 
-//=================================================================================================
-void FeaturesPlugin_CompositeSketch::erase()
-{
-  if(data().get() && data()->isValid()) { // on abort of sketch of this composite it may be invalid
-    FeaturePtr aSketch = std::dynamic_pointer_cast<ModelAPI_Feature>(reference(SKETCH_ID())->value());
-    if(aSketch.get() && aSketch->data()->isValid()) {
-      document()->removeFeature(aSketch);
-    }
-  }
-
-  ModelAPI_CompositeFeature::erase();
-}
-
 //=================================================================================================
 void FeaturesPlugin_CompositeSketch::getBaseShapes(ListOfShape& theBaseShapesList,
                                                    const bool theIsMakeShells)
diff --git a/src/FeaturesPlugin/FeaturesPlugin_CompositeSketch.h b/src/FeaturesPlugin/FeaturesPlugin_CompositeSketch.h
index f83fe924b..21479418c 100644
--- a/src/FeaturesPlugin/FeaturesPlugin_CompositeSketch.h
+++ b/src/FeaturesPlugin/FeaturesPlugin_CompositeSketch.h
@@ -52,9 +52,6 @@ public:
   /// structures of the owner (the remove from the document will be done outside just after).
   FEATURESPLUGIN_EXPORT virtual void removeFeature(std::shared_ptr<ModelAPI_Feature> theFeature);
 
-  /// Removes also all sub-sketch.
-  FEATURESPLUGIN_EXPORT virtual void erase();
-
 protected:
   enum InitFlags {
     InitSketchLauncher   = 1 << 0,
diff --git a/src/GeomAPI/GeomAPI_PlanarEdges.cpp b/src/GeomAPI/GeomAPI_PlanarEdges.cpp
index e87ce4d89..e49d39e6d 100644
--- a/src/GeomAPI/GeomAPI_PlanarEdges.cpp
+++ b/src/GeomAPI/GeomAPI_PlanarEdges.cpp
@@ -40,7 +40,6 @@ void GeomAPI_PlanarEdges::addEdge(std::shared_ptr<GeomAPI_Shape> theEdge)
 std::list<std::shared_ptr<GeomAPI_Shape> > GeomAPI_PlanarEdges::getEdges()
 {
   TopoDS_Shape& aShape = const_cast<TopoDS_Shape&>(impl<TopoDS_Shape>());
-  //BRepTools_WireExplorer aWireExp(TopoDS::Wire(aShape));
   TopExp_Explorer aWireExp(aShape, TopAbs_EDGE);
   std::list<std::shared_ptr<GeomAPI_Shape> > aResult;
   for (; aWireExp.More(); aWireExp.Next()) {
@@ -101,4 +100,24 @@ void GeomAPI_PlanarEdges::setPlane(const std::shared_ptr<GeomAPI_Pnt>& theOrigin
                                    const std::shared_ptr<GeomAPI_Dir>& theNorm)
 {
   myPlane = std::shared_ptr<GeomAPI_Ax3>(new GeomAPI_Ax3(theOrigin, theDirX, theNorm));
-}
\ No newline at end of file
+}
+
+bool GeomAPI_PlanarEdges::isEqual(const std::shared_ptr<GeomAPI_Shape> theShape) const
+{
+  if (!theShape.get())
+    return false;
+  TopoDS_Shape& aMyShape = const_cast<TopoDS_Shape&>(impl<TopoDS_Shape>());
+  TopoDS_Shape aTheShape = theShape->impl<TopoDS_Shape>();
+  TopExp_Explorer aMyExp(aMyShape, TopAbs_EDGE);
+  TopExp_Explorer aTheExp(aTheShape, TopAbs_EDGE);
+  for (; aMyExp.More() && aTheExp.More(); aMyExp.Next(), aTheExp.Next()) {
+    // check that edge by edge all geometrically matches
+    std::shared_ptr<GeomAPI_Edge> aMyEdge(new GeomAPI_Edge);
+    aMyEdge->setImpl<TopoDS_Shape>(new TopoDS_Shape(aMyExp.Current()));
+    std::shared_ptr<GeomAPI_Edge> aTheEdge(new GeomAPI_Edge);
+    aTheEdge->setImpl<TopoDS_Shape>(new TopoDS_Shape(aTheExp.Current()));
+    if (!aMyEdge->isEqual(aTheEdge))
+      return false;
+  }
+  return !(aMyExp.More() || aTheExp.More());
+}
diff --git a/src/GeomAPI/GeomAPI_PlanarEdges.h b/src/GeomAPI/GeomAPI_PlanarEdges.h
index 5628f40e8..15fb093f0 100644
--- a/src/GeomAPI/GeomAPI_PlanarEdges.h
+++ b/src/GeomAPI/GeomAPI_PlanarEdges.h
@@ -65,6 +65,10 @@ class GeomAPI_PlanarEdges : public GeomAPI_Shape
                                const std::shared_ptr<GeomAPI_Dir>& theDirX,
                                const std::shared_ptr<GeomAPI_Dir>& theNorm);
 
+  /// Returns whether the shapes are equal
+  GEOMAPI_EXPORT 
+  virtual bool isEqual(const std::shared_ptr<GeomAPI_Shape> theShape) const;
+
 private:
 
   std::shared_ptr<GeomAPI_Ax3> myPlane;
diff --git a/src/GeomValidators/GeomValidators_ZeroOffset.cpp b/src/GeomValidators/GeomValidators_ZeroOffset.cpp
index 1ba9664d9..f898a2297 100644
--- a/src/GeomValidators/GeomValidators_ZeroOffset.cpp
+++ b/src/GeomValidators/GeomValidators_ZeroOffset.cpp
@@ -140,9 +140,17 @@ bool GeomValidators_ZeroOffset::isValid(const std::shared_ptr<ModelAPI_Feature>&
     isPlanesCoincident = true;
   } else if(aFromShape.get() && aToShape.get()) {
     std::shared_ptr<GeomAPI_Face> aFromFace(new GeomAPI_Face(aFromShape));
+    if (aFromFace->isNull()) {
+      theError = "From face selection is invalid.";
+      return false;
+    }
     std::shared_ptr<GeomAPI_Pln>  aFromPln = aFromFace->getPlane();
 
     std::shared_ptr<GeomAPI_Face> aToFace(new GeomAPI_Face(aToShape));
+    if (aToFace->isNull()) {
+      theError = "To face selection is invalid.";
+      return false;
+    }
     std::shared_ptr<GeomAPI_Pln>  aToPln = aToFace->getPlane();
 
     if(aFromPln.get()) {
@@ -152,8 +160,16 @@ bool GeomValidators_ZeroOffset::isValid(const std::shared_ptr<ModelAPI_Feature>&
     std::shared_ptr<GeomAPI_Face> aFace;
     if(aFromShape.get()) {
       aFace.reset(new GeomAPI_Face(aFromShape));
+      if (aFace->isNull()) {
+        theError = "From face selection is invalid.";
+        return false;
+      }
     } else {
       aFace.reset(new GeomAPI_Face(aToShape));
+      if (aFace->isNull()) {
+        theError = "To face selection is invalid.";
+        return false;
+      }
     }
     std::shared_ptr<GeomAPI_Pln> aPln = aFace->getPlane();
     if(aPln.get()) {
diff --git a/src/Model/Model_AttributeReference.cpp b/src/Model/Model_AttributeReference.cpp
index 40ad13ad9..d63b4e373 100644
--- a/src/Model/Model_AttributeReference.cpp
+++ b/src/Model/Model_AttributeReference.cpp
@@ -126,8 +126,4 @@ void Model_AttributeReference::setObject(const std::shared_ptr<ModelAPI_Object>&
 }
 
 Model_AttributeReference::~Model_AttributeReference()
-{
-  std::shared_ptr<Model_Document> aDoc =
-    std::dynamic_pointer_cast<Model_Document>(owner()->document());
-  TDF_Label aLab = myRef->Get();
-}
+{}
diff --git a/src/Model/Model_Data.cpp b/src/Model/Model_Data.cpp
index e9d808ade..c907ea63f 100644
--- a/src/Model/Model_Data.cpp
+++ b/src/Model/Model_Data.cpp
@@ -302,8 +302,24 @@ void Model_Data::blockSendAttributeUpdated(const bool theBlock)
 
 void Model_Data::erase()
 {
-  if (!myLab.IsNull())
+  if (!myLab.IsNull()) {
+    // remove in order to clear back references in other objects
+    std::list<std::pair<std::string, std::list<ObjectPtr> > > aRefs;
+    referencesToObjects(aRefs);
+    std::list<std::pair<std::string, std::list<ObjectPtr> > >::iterator anAttrIter = aRefs.begin();
+    for(; anAttrIter != aRefs.end(); anAttrIter++) {
+      std::list<ObjectPtr>::iterator aReferenced = anAttrIter->second.begin();
+      for(; aReferenced != anAttrIter->second.end(); aReferenced++) {
+        if (aReferenced->get() && (*aReferenced)->data()->isValid()) {
+          std::shared_ptr<Model_Data> aData = 
+            std::dynamic_pointer_cast<Model_Data>((*aReferenced)->data());
+          aData->removeBackReference(myAttrs[anAttrIter->first]);
+        }
+      }
+    }
+    myAttrs.clear();
     myLab.ForgetAllAttributes();
+  }
 }
 
 // indexes in the state array
diff --git a/src/Model/Model_Document.cpp b/src/Model/Model_Document.cpp
index 09469b85b..2c4bb4bd8 100755
--- a/src/Model/Model_Document.cpp
+++ b/src/Model/Model_Document.cpp
@@ -621,7 +621,7 @@ void Model_Document::abortOperation()
   for (; aSubIter != aSubs.end(); aSubIter++)
     subDoc(*aSubIter)->abortOperation();
   // references may be changed because they are set in attributes on the fly
-  myObjs->synchronizeFeatures(aDeltaLabels, true, isRoot());
+  myObjs->synchronizeFeatures(aDeltaLabels, true, false, isRoot());
 }
 
 bool Model_Document::isOperation() const
@@ -687,7 +687,7 @@ void Model_Document::undoInternal(const bool theWithSubs, const bool theSynchron
   }
   // after undo of all sub-documents to avoid updates on not-modified data (issue 370)
   if (theSynchronize) {
-    myObjs->synchronizeFeatures(aDeltaLabels, true, isRoot());
+    myObjs->synchronizeFeatures(aDeltaLabels, true, false, isRoot());
     // update the current features status
     setCurrentFeature(currentFeature(false), false);
   }
@@ -734,7 +734,7 @@ void Model_Document::redo()
     subDoc(*aSubIter)->redo();
 
   // after redo of all sub-documents to avoid updates on not-modified data (issue 370)
-  myObjs->synchronizeFeatures(aDeltaLabels, true, isRoot());
+  myObjs->synchronizeFeatures(aDeltaLabels, true, false, isRoot());
   // update the current features status
   setCurrentFeature(currentFeature(false), false);
 }
diff --git a/src/Model/Model_Objects.cpp b/src/Model/Model_Objects.cpp
index 6a4752d09..11365789d 100644
--- a/src/Model/Model_Objects.cpp
+++ b/src/Model/Model_Objects.cpp
@@ -54,7 +54,7 @@ void Model_Objects::setOwner(DocumentPtr theDoc)
   myDoc = theDoc;
   // update all fields and recreate features and result objects if needed
   TDF_LabelList aNoUpdated;
-  synchronizeFeatures(aNoUpdated, true, true);
+  synchronizeFeatures(aNoUpdated, true, true, true);
   myHistory.clear();
 }
 
@@ -240,7 +240,7 @@ void Model_Objects::removeFeature(FeaturePtr theFeature)
     for(; aRefIter != aRefs.end(); aRefIter++) {
       std::shared_ptr<ModelAPI_CompositeFeature> aComposite = 
         std::dynamic_pointer_cast<ModelAPI_CompositeFeature>(*aRefIter);
-      if (aComposite.get() && aComposite->data()->isValid()) {
+      if (aComposite.get()) {
         aComposite->removeFeature(theFeature);
       }
     }
@@ -606,7 +606,8 @@ std::shared_ptr<ModelAPI_Feature> Model_Objects::featureById(const int theId)
 }
 
 void Model_Objects::synchronizeFeatures(
-  const TDF_LabelList& theUpdated, const bool theUpdateReferences, const bool theFlush)
+  const TDF_LabelList& theUpdated, const bool theUpdateReferences, 
+  const bool theOpen, const bool theFlush)
 {
   Model_Document* anOwner = std::dynamic_pointer_cast<Model_Document>(myDoc).get();
   if (!anOwner) // this may happen on creation of document: nothing there, so nothing to synchronize
@@ -728,7 +729,8 @@ void Model_Objects::synchronizeFeatures(
     myHistory.clear();
   }
 
-  anOwner->executeFeatures() = false;
+  if (theOpen)
+    anOwner->executeFeatures() = false;
   aLoop->activateFlushes(isActive);
 
   if (theFlush) {
@@ -740,7 +742,8 @@ void Model_Objects::synchronizeFeatures(
     aLoop->flush(aRedispEvent);
     aLoop->flush(aToHideEvent);
   }
-  anOwner->executeFeatures() = true;
+  if (theOpen)
+    anOwner->executeFeatures() = true;
 }
 
 /// synchronises back references for the given object basing on the collected data
diff --git a/src/Model/Model_Objects.h b/src/Model/Model_Objects.h
index 740a3d542..517311b1e 100644
--- a/src/Model/Model_Objects.h
+++ b/src/Model/Model_Objects.h
@@ -144,9 +144,10 @@ class Model_Objects
   //! Synchronizes myFeatures list with the updated document
   //! \param theUpdated list of labels that are marked as modified, so featrues must be also
   //! \param theUpdateReferences causes the update of back-references
+  //! \param theOpen - on open nothing must be reexecuted, except not persistent results
   //! \param theFlush makes flush all events in the end of all modifications of this method
   void synchronizeFeatures(const TDF_LabelList& theUpdated, const bool theUpdateReferences,
-    const bool theFlush);
+    const bool theOpen, const bool theFlush);
   //! Synchronizes the BackReferences list in Data of Features and Results
   void synchronizeBackRefs();
 
diff --git a/src/Model/Model_Session.cpp b/src/Model/Model_Session.cpp
index e336d1652..4525cbf4a 100644
--- a/src/Model/Model_Session.cpp
+++ b/src/Model/Model_Session.cpp
@@ -263,7 +263,7 @@ void Model_Session::setActiveDocument(
         bool aWasChecked = myCheckTransactions;
         setCheckTransactions(false);
         TDF_LabelList anEmptyUpdated;
-        aDoc->objects()->synchronizeFeatures(anEmptyUpdated, true, true);
+        aDoc->objects()->synchronizeFeatures(anEmptyUpdated, true, true, true);
         if (aWasChecked)
             setCheckTransactions(true);
       }
@@ -326,7 +326,7 @@ std::shared_ptr<ModelAPI_Document> Model_Session::copy(
   TDF_CopyTool::Copy(aDS, aRT);
 
   TDF_LabelList anEmptyUpdated;
-  aNew->objects()->synchronizeFeatures(anEmptyUpdated, true, true);
+  aNew->objects()->synchronizeFeatures(anEmptyUpdated, true, true, true);
   return aNew;
 }
 
diff --git a/src/Model/Model_Update.cpp b/src/Model/Model_Update.cpp
index d88c1fec3..bb7b68163 100644
--- a/src/Model/Model_Update.cpp
+++ b/src/Model/Model_Update.cpp
@@ -59,9 +59,16 @@ Model_Update::Model_Update()
   myIsProcessed = false;
 }
 
-void Model_Update::addModified(FeaturePtr theFeature, FeaturePtr theReason) {
+bool Model_Update::addModified(FeaturePtr theFeature, FeaturePtr theReason) {
+
   if (!theFeature->data()->isValid())
-    return; // delete an extrusion created on the sketch
+    return false; // delete an extrusion created on the sketch
+
+  if (theFeature->isPersistentResult()) {
+    if (!std::dynamic_pointer_cast<Model_Document>((theFeature)->document())->executeFeatures())
+      return false;
+  }
+
   if (!theFeature->isPreviewNeeded() && !myIsFinish) {
     myProcessOnFinish.insert(theFeature);
 #ifdef DEB_UPDATE
@@ -73,7 +80,7 @@ void Model_Update::addModified(FeaturePtr theFeature, FeaturePtr theReason) {
       static ModelAPI_ValidatorsFactory* aFactory = ModelAPI_Session::get()->validators();
       aFactory->validate(theFeature); // need to be validated to update the "Apply" state if not previewed
     }
-    return;
+    return true;
   }
   if (myModified.find(theFeature) != myModified.end()) {
     if (theReason.get()) {
@@ -82,7 +89,7 @@ void Model_Update::addModified(FeaturePtr theFeature, FeaturePtr theReason) {
 #endif
       myModified[theFeature].insert(theReason);
     }
-    return; // already is marked as modified, so, nothing to do, it will be processed
+    return true;
   }
   // do not add the disabled, but possibly the sub-elements are not disabled
   bool aIsDisabled = theFeature->isDisabled();
@@ -103,7 +110,7 @@ void Model_Update::addModified(FeaturePtr theFeature, FeaturePtr theReason) {
     if (theFeature->data()->execState() == ModelAPI_StateDone)
       theFeature->data()->execState(ModelAPI_StateMustBeUpdated);
     else 
-      return; // do not need iteration deeply if it is already marked as modified or so
+      return true; // do not need iteration deeply if it is already marked as modified or so
 #ifdef DEB_UPDATE
     std::cout<<"*** Set modified state "<<theFeature->name()<<std::endl;
 #endif
@@ -139,7 +146,7 @@ void Model_Update::addModified(FeaturePtr theFeature, FeaturePtr theReason) {
     if (aPart.get())
       addModified(aPart, theFeature);
   }
-  return;
+  return true;
 }
 
 void Model_Update::processEvent(const std::shared_ptr<Events_Message>& theMessage)
@@ -186,14 +193,10 @@ void Model_Update::processEvent(const std::shared_ptr<Events_Message>& theMessag
         myIsParamUpdated = true;
       }
       // on undo/redo, abort do not update persisten features
-      bool anUpdateOnlyNotPersistent = 
-        !std::dynamic_pointer_cast<Model_Document>((*anObjIter)->document())->executeFeatures();
       FeaturePtr anUpdated = std::dynamic_pointer_cast<ModelAPI_Feature>(*anObjIter);
       if (anUpdated.get()) {
-        if (!anUpdateOnlyNotPersistent || !anUpdated->isPersistentResult()) {
-          addModified(anUpdated, FeaturePtr());
+        if (addModified(anUpdated, FeaturePtr()))
           aSomeModified = true;
-        }
       } else { // process the updated result as update of features that refers to this result
         const std::set<std::shared_ptr<ModelAPI_Attribute> >& aRefs = (*anObjIter)->data()->refsToMe();
         std::set<std::shared_ptr<ModelAPI_Attribute> >::const_iterator aRefIter = aRefs.cbegin();
@@ -201,9 +204,9 @@ void Model_Update::processEvent(const std::shared_ptr<Events_Message>& theMessag
           if (!(*aRefIter)->owner()->data()->isValid())
             continue;
           FeaturePtr anUpdated = std::dynamic_pointer_cast<ModelAPI_Feature>((*aRefIter)->owner());
-          if (anUpdated.get() && (!anUpdateOnlyNotPersistent || !anUpdated->isPersistentResult())) {
-            addModified(anUpdated, FeaturePtr());
-            aSomeModified = true;
+          if (anUpdated.get()) {
+            if (addModified(anUpdated, FeaturePtr()))
+              aSomeModified = true;
           }
         }
       }
@@ -294,6 +297,11 @@ bool Model_Update::processFeature(FeaturePtr theFeature)
     return false;
   }
 
+  if (theFeature->isPersistentResult()) {
+    if (!std::dynamic_pointer_cast<Model_Document>((theFeature)->document())->executeFeatures())
+      return false;
+  }
+
   // check this feature is not yet checked or processed
   bool aIsModified = myModified.find(theFeature) != myModified.end();
   if (!aIsModified && myIsFinish) { // get info about the modification for features without preview
@@ -399,42 +407,26 @@ bool Model_Update::processFeature(FeaturePtr theFeature)
   // this checking must be after the composite feature sub-elements processing:
   // composite feature status may depend on it's subelements
   if (theFeature->data()->execState() == ModelAPI_StateInvalidArgument) {
+  #ifdef DEB_UPDATE
+    std::cout<<"Invalid args "<<theFeature->name()<<std::endl;
+  #endif
     theFeature->eraseResults();
     redisplayWithResults(theFeature, ModelAPI_StateInvalidArgument); // result also must be updated
     return true; // so, feature is modified (results are erased)
   }
 
-  // On abort, undo or redo execute is not needed: results in document are updated automatically
-  // But redisplay is needed: results are updated, must be also updated in the viewer.
-  if (!std::dynamic_pointer_cast<Model_Document>(theFeature->document())->executeFeatures()) {
-    if (!theFeature->isPersistentResult()) { // not persistent must be re-executed on abort, etc.
-      ModelAPI_ExecState aState = theFeature->data()->execState();
-      if (aFactory->validate(theFeature)) {
-        executeFeature(theFeature);
-      } else {
-        theFeature->eraseResults();
-        redisplayWithResults(theFeature, ModelAPI_StateInvalidArgument); // result also must be updated
-      }
-    } else {
-      redisplayWithResults(theFeature, ModelAPI_StateNothing);
-      if (theFeature->data()->execState() == ModelAPI_StateMustBeUpdated) { // it is done (in the tree)
-        theFeature->data()->execState(ModelAPI_StateDone);
-      }
+  // execute feature if it must be updated
+  ModelAPI_ExecState aState = theFeature->data()->execState();
+  if (aFactory->validate(theFeature)) {
+    if (!isPostponedMain) {
+      executeFeature(theFeature);
     }
   } else {
-    // execute feature if it must be updated
-    ModelAPI_ExecState aState = theFeature->data()->execState();
-    if (aFactory->validate(theFeature)) {
-      if (!isPostponedMain) {
-        executeFeature(theFeature);
-      }
-    } else {
-      #ifdef DEB_UPDATE
-        std::cout<<"Feature is not valid, erase results "<<theFeature->name()<<std::endl;
-      #endif
-      theFeature->eraseResults();
-      redisplayWithResults(theFeature, ModelAPI_StateInvalidArgument); // result also must be updated
-    }
+    #ifdef DEB_UPDATE
+      std::cout<<"Feature is not valid, erase results "<<theFeature->name()<<std::endl;
+    #endif
+    theFeature->eraseResults();
+    redisplayWithResults(theFeature, ModelAPI_StateInvalidArgument); // result also must be updated
   }
   return true;
 }
@@ -660,6 +652,7 @@ void Model_Update::executeFeature(FeaturePtr theFeature)
 
 void Model_Update::updateStability(void* theSender)
 {
+  static ModelAPI_ValidatorsFactory* aFactory = ModelAPI_Session::get()->validators();
   if (theSender) {
     bool added = false; // object may be was crated
     ModelAPI_Object* aSender = static_cast<ModelAPI_Object*>(theSender);
@@ -676,6 +669,8 @@ void Model_Update::updateStability(void* theSender)
           aSender->data()->referencesToObjects(aRefs);
           std::list<std::pair<std::string, std::list<ObjectPtr> > >::iterator aRefIt = aRefs.begin();
           for(; aRefIt != aRefs.end(); aRefIt++) {
+            if (!aFactory->isConcealed(aFeatureSender->getKind(), aRefIt->first))
+              continue; // take into account only concealed references (do not remove the sketch constraint and the edge on constraint edit)
             std::list<ObjectPtr>& aRefFeaturesList = aRefIt->second;
             std::list<ObjectPtr>::iterator aReferenced = aRefFeaturesList.begin();
             for(; aReferenced != aRefFeaturesList.end(); aReferenced++) {
diff --git a/src/Model/Model_Update.h b/src/Model/Model_Update.h
index 060b5c9d9..106b48cbc 100644
--- a/src/Model/Model_Update.h
+++ b/src/Model/Model_Update.h
@@ -52,7 +52,8 @@ protected:
   /// Appends the new modified feature to the myModified, clears myProcessed if needed
   /// Returns true if some feature really was marked as modified
   /// theReason is the object that causes modification of this feature
-  void addModified(
+  /// returns true if something reallsy was added to the modified and must be processed
+  bool addModified(
     std::shared_ptr<ModelAPI_Feature> theFeature, std::shared_ptr<ModelAPI_Feature> theReason);
 
   /// Recoursively checks and updates features if needed (calls the execute method)
diff --git a/src/ModelAPI/ModelAPI_CompositeFeature.cpp b/src/ModelAPI/ModelAPI_CompositeFeature.cpp
index 7913a8bde..34066ecd2 100644
--- a/src/ModelAPI/ModelAPI_CompositeFeature.cpp
+++ b/src/ModelAPI/ModelAPI_CompositeFeature.cpp
@@ -16,3 +16,16 @@ void ModelAPI_CompositeFeature::exchangeIDs(
 {
   // by default nothing is in the implementation
 }
+
+void ModelAPI_CompositeFeature::erase()
+{
+  // erase all sub-features
+  for(int a = numberOfSubs(); a > 0; a--) {
+    FeaturePtr aFeature = subFeature(a - 1);
+    if (aFeature.get()) {
+      // subs are referenced from sketch, but must be removed for sure, so not checkings
+      aFeature->document()->removeFeature(aFeature);
+    }
+  }
+  ModelAPI_Feature::erase();
+}
diff --git a/src/ModelAPI/ModelAPI_CompositeFeature.h b/src/ModelAPI/ModelAPI_CompositeFeature.h
index bc31a21eb..ab58f092f 100644
--- a/src/ModelAPI/ModelAPI_CompositeFeature.h
+++ b/src/ModelAPI/ModelAPI_CompositeFeature.h
@@ -43,6 +43,9 @@ public:
   /// Exchanges IDs of two given features: needed for more correct naming in some cases (issue 769)
   MODELAPI_EXPORT virtual void exchangeIDs(std::shared_ptr<ModelAPI_Feature> theFeature1,
     std::shared_ptr<ModelAPI_Feature> theFeature2);
+
+  /// in addition removes all subs
+  MODELAPI_EXPORT virtual void erase();
 };
 
 //! Pointer on the composite feature object
diff --git a/src/SketchPlugin/SketchPlugin_Sketch.cpp b/src/SketchPlugin/SketchPlugin_Sketch.cpp
index 0e2b9acd3..354666bc9 100644
--- a/src/SketchPlugin/SketchPlugin_Sketch.cpp
+++ b/src/SketchPlugin/SketchPlugin_Sketch.cpp
@@ -206,22 +206,6 @@ bool SketchPlugin_Sketch::isSub(ObjectPtr theObject) const
 }
 
 
-void SketchPlugin_Sketch::erase()
-{
-  std::shared_ptr<ModelAPI_AttributeRefList> aRefList = std::dynamic_pointer_cast<
-      ModelAPI_AttributeRefList>(data()->attribute(SketchPlugin_Sketch::FEATURES_ID()));
-  std::list<ObjectPtr> aFeatures = aRefList->list();
-  std::list<ObjectPtr>::const_iterator anIt = aFeatures.begin();
-  for (; anIt != aFeatures.end(); anIt++) {
-    FeaturePtr aFeature = std::dynamic_pointer_cast<ModelAPI_Feature>(*anIt);
-    if (aFeature) {
-      // subs are referenced from sketch, but must be removed for sure, so not checkings
-      document()->removeFeature(aFeature);
-    }
-  }
-  ModelAPI_CompositeFeature::erase();
-}
-
 void SketchPlugin_Sketch::attributeChanged(const std::string& theID) {
   if (theID == SketchPlugin_SketchEntity::EXTERNAL_ID()) {
     std::shared_ptr<GeomAPI_Shape> aSelection = 
diff --git a/src/SketchPlugin/SketchPlugin_Sketch.h b/src/SketchPlugin/SketchPlugin_Sketch.h
index d97ea573d..d541415f3 100644
--- a/src/SketchPlugin/SketchPlugin_Sketch.h
+++ b/src/SketchPlugin/SketchPlugin_Sketch.h
@@ -169,9 +169,6 @@ class SketchPlugin_Sketch : public ModelAPI_CompositeFeature, public GeomAPI_ICu
   }
 
 
-  /// removes also all sub-sketch elements
-  SKETCHPLUGIN_EXPORT virtual void erase();
-
   /// appends a feature to the sketch sub-elements container
   SKETCHPLUGIN_EXPORT virtual std::shared_ptr<ModelAPI_Feature> addFeature(std::string theID);