From 4a8201a23789409308c0a22f4c59b210c1f997a0 Mon Sep 17 00:00:00 2001
From: wmayer <wmayer@users.sourceforge.net>
Date: Fri, 23 Oct 2020 11:26:15 +0200
Subject: [PATCH] Sketch: [skip ci] handle possible crashes when decreasing
 degree of a spline

---
 src/Mod/Sketcher/App/SketchObject.cpp           | 14 +++++++++++++-
 src/Mod/Sketcher/App/SketchObjectPyImp.cpp      | 16 +++++-----------
 src/Mod/Sketcher/Gui/CommandSketcherBSpline.cpp |  5 +++++
 3 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp
index cab7d3bf4b..0870537a3f 100644
--- a/src/Mod/Sketcher/App/SketchObject.cpp
+++ b/src/Mod/Sketcher/App/SketchObject.cpp
@@ -5219,7 +5219,11 @@ bool SketchObject::decreaseBSplineDegree(int GeoId, int degreedecrement /*= 1*/)
     try {
         int cdegree = bspline->getDegree();
 
-        bool ok = bspline->approximate(Precision::Confusion(), 20, cdegree-degreedecrement, 0);
+        // degree must be >= 1
+        int maxdegree = cdegree - degreedecrement;
+        if (maxdegree == 0)
+            return false;
+        bool ok = bspline->approximate(Precision::Confusion(), 20, maxdegree, 0);
         if (!ok)
             return false;
     }
@@ -5228,6 +5232,9 @@ bool SketchObject::decreaseBSplineDegree(int GeoId, int degreedecrement /*= 1*/)
         return false;
     }
 
+    // FIXME: Avoid to delete the whole geometry but only delete invalid constraints
+    // and unused construction geometries
+#if 0
     const std::vector< Part::Geometry * > &vals = getInternalGeometry();
 
     std::vector< Part::Geometry * > newVals(vals);
@@ -5236,6 +5243,11 @@ bool SketchObject::decreaseBSplineDegree(int GeoId, int degreedecrement /*= 1*/)
 
     // AcceptGeometry called from onChanged
     Geometry.setValues(newVals);
+#else
+    delGeometry(GeoId);
+    int newId = addGeometry(bspline.release());
+    exposeInternalGeometry(newId);
+#endif
 
     return true;
 }
diff --git a/src/Mod/Sketcher/App/SketchObjectPyImp.cpp b/src/Mod/Sketcher/App/SketchObjectPyImp.cpp
index d1dc97a2fd..d9a10413a3 100644
--- a/src/Mod/Sketcher/App/SketchObjectPyImp.cpp
+++ b/src/Mod/Sketcher/App/SketchObjectPyImp.cpp
@@ -1399,19 +1399,13 @@ PyObject* SketchObjectPy::increaseBSplineDegree(PyObject *args)
 PyObject* SketchObjectPy::decreaseBSplineDegree(PyObject *args)
 {
     int GeoId;
-    int incr = 1;
+    int decr = 1;
 
-    if (!PyArg_ParseTuple(args, "i|i", &GeoId, &incr))
-        return 0;
+    if (!PyArg_ParseTuple(args, "i|i", &GeoId, &decr))
+        return nullptr;
 
-    if (this->getSketchObjectPtr()->decreaseBSplineDegree(GeoId, incr)==false) {
-        std::stringstream str;
-        str << "Degree decrease failed for: " << GeoId;
-        PyErr_SetString(PyExc_ValueError, str.str().c_str());
-        return 0;
-    }
-
-    Py_Return;
+    bool ok = this->getSketchObjectPtr()->decreaseBSplineDegree(GeoId, decr);
+    return Py_BuildValue("O", (ok ? Py_True : Py_False));
 }
 
 PyObject* SketchObjectPy::modifyBSplineKnotMultiplicity(PyObject *args)
diff --git a/src/Mod/Sketcher/Gui/CommandSketcherBSpline.cpp b/src/Mod/Sketcher/Gui/CommandSketcherBSpline.cpp
index cfd210a4ed..290d738e9f 100644
--- a/src/Mod/Sketcher/Gui/CommandSketcherBSpline.cpp
+++ b/src/Mod/Sketcher/Gui/CommandSketcherBSpline.cpp
@@ -500,6 +500,8 @@ void CmdSketcherDecreaseDegree::activated(int iMsg)
         return;
     }
 
+    getSelection().clearSelection();
+
     // get the needed lists and objects
     const std::vector<std::string> &SubNames = selection[0].getSubNames();
     Sketcher::SketchObject* Obj = static_cast<Sketcher::SketchObject*>(selection[0].getObject());
@@ -517,7 +519,10 @@ void CmdSketcherDecreaseDegree::activated(int iMsg)
             if (geo->getTypeId() == Part::GeomBSplineCurve::getClassTypeId()) {
                 Gui::cmdAppObjectArgs(selection[0].getObject(), "decreaseBSplineDegree(%d) ", GeoId);
                 // add new control points
+                // Currently exposeInternalGeometry is called from within decreaseBSplineDegree because
+                // the old spline is deleted and a new one is added so that the GeoId is invalid afterwards
                 //Gui::cmdAppObjectArgs(selection[0].getObject(), "exposeInternalGeometry(%d)", GeoId);
+                break; // cannot handle more than spline because the GeoIds will be invalidated after the first change
             }
             else {
                 ignored = true;