+ fixes #1956: FreeCAD 0.14.370x hangs when attempting to edit sketch…

… containing ellipse

src/Mod/Sketcher/App/Constraint.h

@@ -31,7 +31,11 @@
 
 namespace Sketcher
 {
-
+/*!
+ Important note: New constraint types must be always added at the end but before 'NumConstraintTypes'.
+ This is mandatory in order to keep the handling of constraint types upward compatible which means that
+ this program version ignores later introduced constraint types when reading them from a project file.
+ */
 enum ConstraintType {
     None = 0,
     Coincident = 1,
@@ -49,7 +53,8 @@ enum ConstraintType {
     PointOnObject = 13,
     Symmetric = 14,
     InternalAlignment = 15,
-    SnellsLaw = 16
+    SnellsLaw = 16,
+    NumConstraintTypes // must be the last item!
 };
 
 enum InternalAlignmentType {

src/Mod/Sketcher/App/PropertyConstraintList.cpp

@@ -288,7 +288,14 @@ void PropertyConstraintList::Restore(Base::XMLReader &reader)
     for (int i = 0; i < count; i++) {
         Constraint *newC = new Constraint();
         newC->Restore(reader);
-        values.push_back(newC);
+        // To keep upward compatibility ignore unknown constraint types
+        if (newC->Type < Sketcher::NumConstraintTypes) {
+            values.push_back(newC);
+        }
+        else {
+            // reading a new constraint type which this version cannot handle
+            delete newC;
+        }
     }
 
     reader.readEndElement("ConstraintList");

src/Mod/Sketcher/App/SketchObject.cpp

@@ -511,6 +511,41 @@ void SketchObject::acceptGeometry()
     rebuildVertexIndex();
 }
 
+bool SketchObject::isSupportedGeometry(const Part::Geometry *geo) const
+{
+    if (geo->getTypeId() == Part::GeomPoint::getClassTypeId() ||
+        geo->getTypeId() == Part::GeomCircle::getClassTypeId() ||
+        geo->getTypeId() == Part::GeomEllipse::getClassTypeId() ||
+        geo->getTypeId() == Part::GeomArcOfCircle::getClassTypeId() ||
+        geo->getTypeId() == Part::GeomArcOfEllipse::getClassTypeId() ||
+        geo->getTypeId() == Part::GeomLineSegment::getClassTypeId()) {
+        return true;
+    }
+    if (geo->getTypeId() == Part::GeomTrimmedCurve::getClassTypeId()) {
+        Handle_Geom_TrimmedCurve trim = Handle_Geom_TrimmedCurve::DownCast(geo->handle());
+        Handle_Geom_Circle circle = Handle_Geom_Circle::DownCast(trim->BasisCurve());
+        Handle_Geom_Ellipse ellipse = Handle_Geom_Ellipse::DownCast(trim->BasisCurve());
+        if (!circle.IsNull() || !ellipse.IsNull()) {
+            return true;
+        }
+    }
+    return false;
+}
+
+std::vector<Part::Geometry *> SketchObject::supportedGeometry(const std::vector<Part::Geometry *> &geoList) const
+{
+    std::vector<Part::Geometry *> supportedGeoList;
+    supportedGeoList.reserve(geoList.size());
+    // read-in geometry that the sketcher cannot handle
+    for (std::vector<Part::Geometry*>::const_iterator it = geoList.begin(); it != geoList.end(); ++it) {
+        if (isSupportedGeometry(*it)) {
+            supportedGeoList.push_back(*it);
+        }
+    }
+
+    return supportedGeoList;
+}
+
 int SketchObject::addGeometry(const std::vector<Part::Geometry *> &geoList, bool construction/*=false*/)
 {
     const std::vector< Part::Geometry * > &vals = getInternalGeometry();
@@ -3751,6 +3786,15 @@ void SketchObject::Restore(XMLReader &reader)
 
 void SketchObject::onChanged(const App::Property* prop)
 {
+    if (isRestoring() && prop == &Geometry) {
+        std::vector<Part::Geometry*> geom = Geometry.getValues();
+        std::vector<Part::Geometry*> supportedGeom = supportedGeometry(geom);
+        // To keep upward compatibility ignore unsupported geometry types
+        if (supportedGeom.size() != geom.size()) {
+            Geometry.setValues(supportedGeom);
+            return;
+        }
+    }
     if (prop == &Geometry || prop == &Constraints) {
         Constraints.checkGeometry(getCompleteGeometry());
     }

src/Mod/Sketcher/App/SketchObject.h

@@ -72,6 +72,12 @@ class SketcherExport SketchObject : public Part::Part2DObject
     */
     bool noRecomputes;
 
+    /*!
+     \brief Returns true if the sketcher supports the given geometry
+     \param geo - the geometry
+     \retval bool - true if the geometry is supported
+     */
+    bool isSupportedGeometry(const Part::Geometry *geo) const;
     /// add unspecified geometry
     int addGeometry(const Part::Geometry *geo, bool construction=false);
     /// add unspecified geometry
@@ -264,6 +270,12 @@ class SketcherExport SketchObject : public Part::Part2DObject
 
     void constraintsRenamed(const std::map<App::ObjectIdentifier, App::ObjectIdentifier> &renamed);
     void constraintsRemoved(const std::set<App::ObjectIdentifier> &removed);
+    /*!
+     \brief Returns a list of supported geometries from the input list
+     \param geoList - the geometry list
+     \retval list - the supported geometry list
+     */
+    std::vector<Part::Geometry *> supportedGeometry(const std::vector<Part::Geometry *> &geoList) const;
 
 private:
     std::vector<Part::Geometry *> ExternalGeo;

src/Mod/Sketcher/Gui/ViewProviderSketch.cpp

@@ -2309,7 +2309,7 @@ void ViewProviderSketch::updateColor(void)
         bool hasMaterial = false;
 
         SoMaterial *m = 0;
-        if (!hasDatumLabel && type != Sketcher::Coincident && type !=InternalAlignment) {
+        if (!hasDatumLabel && type != Sketcher::Coincident && type != Sketcher::InternalAlignment) {
             hasMaterial = true;
             m = dynamic_cast<SoMaterial *>(s->getChild(CONSTRAINT_SEPARATOR_INDEX_MATERIAL_OR_DATUMLABEL));
         }
@@ -3187,6 +3187,7 @@ void ViewProviderSketch::draw(bool temp)
         if ((*it)->Type != edit->vConstrType[i]) {
             // clearing the type vector will force a rebuild of the visual nodes
             edit->vConstrType.clear();
+            //TODO: The 'goto' here is unsafe as it can happen that we cause an endless loop (see bug #0001956).
             goto Restart;
         }
         try{//because calculateNormalAtPoint, used in there, can throw