Sketcher: Bug fix / improve B-Spline knot support

=================================================

Knot position is not calculated by the solver, but by OCCT when updating the
b-spline to conform to given pole positions, as mandated by the solver. Before
this commit, all constraints driving and non-driving operating on the knots required
and extra solve (from advanced solver dialog, or from the Python console), or a recompute
to be recomputed.

This commit introduces transparently re-solving at Sketch.cpp level if B-Splines are present,
so that when the Sketcher mandated solve returns, the geometry is fully solved.

src/Mod/Sketcher/App/Sketch.cpp

@@ -74,6 +74,7 @@ TYPESYSTEM_SOURCE(Sketcher::Sketch, Base::Persistence)
 Sketch::Sketch()
   : SolveTime(0)
   , RecalculateInitialSolutionWhileMovingPoint(false)
+  , resolveAfterGeometryUpdated(false)
   , GCSsys(), ConstraintsCounter(0)
   , isInitMove(false), isFine(true), moveStep(0)
   , defaultSolver(GCS::DogLeg)
@@ -99,6 +100,7 @@ void Sketch::clear(void)
     ArcsOfHyperbola.clear();
     ArcsOfParabola.clear();
     BSplines.clear();
+    resolveAfterGeometryUpdated = false;
 
     // deleting the doubles allocated with new
     for (std::vector<double*>::iterator it = Parameters.begin(); it != Parameters.end(); ++it)
@@ -653,7 +655,10 @@ int Sketch::addGeometry(const Part::Geometry *geo, bool fixed)
         return addArcOfParabola(*aop, fixed);
     } else if (geo->getTypeId() == GeomBSplineCurve::getClassTypeId()) { // add a bspline
         const GeomBSplineCurve *bsp = static_cast<const GeomBSplineCurve*>(geo);
-        // create the definition struct for that geom
+
+        // Current B-Spline implementation relies on OCCT calculations, so a second solve
+        // is necessary to update actual solver implementation to account for changes in B-Spline geometry
+        resolveAfterGeometryUpdated = true;
         return addBSpline(*bsp, fixed);
     }
     else {
@@ -3453,7 +3458,8 @@ bool Sketch::updateGeometry()
                 std::vector<double> knots;
                 std::vector<int> mult;
 
-                std::vector<double *>::const_iterator it3;
+                // This is the code that should be here when/if b-spline gets its full implementation in the solver.
+                /*std::vector<double *>::const_iterator it3;
                 std::vector<int>::const_iterator it4;
 
                 for( it3 = mybsp.knots.begin(), it4 = mybsp.mult.begin(); it3 != mybsp.knots.end() && it4 != mybsp.mult.end(); ++it3, ++it4) {
@@ -3461,7 +3467,14 @@ bool Sketch::updateGeometry()
                     mult.push_back((*it4));
                 }
 
-                bsp->setKnots(knots,mult);
+                bsp->setKnots(knots,mult);*/
+
+                // This is the code that needs to be here to take advantage of the current OCCT reliant implementation
+                // The current B-Spline implementation relies on OCCT for pole calculation, so the knots are set by the OCCT calculated values
+                auto occtknots = bsp->getKnots();
+
+                for(auto it3 = occtknots.begin() ; it3 != occtknots.end(); ++it3)
+                    knots.push_back(*it3);
 
                 #if OCC_VERSION_HEX >= 0x060900
                 int index = 0;
@@ -3473,7 +3486,14 @@ bool Sketch::updateGeometry()
                             auto pointf = GeometryFacade::getFacade(point);
 
                             if(pointf->getInternalType() == InternalType::BSplineKnotPoint) {
-                                point->setPoint(bsp->pointAtParameter(knots[index]));
+                                auto pointcoords = bsp->pointAtParameter(knots[index]);
+                                point->setPoint(pointcoords); // update the geompoint of the knot (geometry update)
+                                // Now we update the position of the points in the solver, so that any call to solve()
+                                // calculates constraints and positions based on the actual position of the knots.
+                                auto pointindex = getPointId(*it5, start);
+                                auto solverpoint = Points[pointindex];
+                                *(solverpoint.x) = pointcoords.x;
+                                *(solverpoint.y) = pointcoords.y;
                             }
                         }
                     }
@@ -3492,7 +3512,7 @@ bool Sketch::updateGeometry()
 
 bool Sketch::updateNonDrivingConstraints()
 {
-     for (std::vector<ConstrDef>::iterator it = Constrs.begin();it!=Constrs.end();++it){
+    for (std::vector<ConstrDef>::iterator it = Constrs.begin();it!=Constrs.end();++it){
         if(!(*it).driving) {
             if((*it).constr->Type==SnellsLaw) {
                 double n1 = *((*it).value);
@@ -3536,6 +3556,7 @@ bool Sketch::updateNonDrivingConstraints()
             }
         }
      }
+
     return true;
 }
 
@@ -3544,6 +3565,24 @@ bool Sketch::updateNonDrivingConstraints()
 int Sketch::solve(void)
 {
     Base::TimeInfo start_time;
+    std::string solvername;
+
+    auto result = internalSolve(solvername);
+
+    Base::TimeInfo end_time;
+
+    if(debugMode==GCS::Minimal || debugMode==GCS::IterationLevel){
+
+        Base::Console().Log("Sketcher::Solve()-%s-T:%s\n",solvername.c_str(),Base::TimeInfo::diffTime(start_time,end_time).c_str());
+    }
+
+    SolveTime = Base::TimeInfo::diffTimeF(start_time,end_time);
+
+    return result;
+}
+
+int Sketch::internalSolve(std::string & solvername, int level)
+{
     if (!isInitMove) { // make sure we are in single subsystem mode
         clearTemporaryConstraints();
         isFine = true;
@@ -3551,7 +3590,6 @@ int Sketch::solve(void)
 
     int ret = -1;
     bool valid_solution;
-    std::string solvername;
     int defaultsoltype = -1;
 
     if(isInitMove){
@@ -3674,14 +3712,11 @@ int Sketch::solve(void)
         } // soltype
     }
 
-    Base::TimeInfo end_time;
-
-    if(debugMode==GCS::Minimal || debugMode==GCS::IterationLevel){
-
-        Base::Console().Log("Sketcher::Solve()-%s-T:%s\n",solvername.c_str(),Base::TimeInfo::diffTime(start_time,end_time).c_str());
+    // For OCCT reliant geometry that needs an extra solve() for example to update non-driving constraints.
+    if (resolveAfterGeometryUpdated && ret == GCS::Success && level == 0) {
+        return internalSolve(solvername, 1);
     }
 
-    SolveTime = Base::TimeInfo::diffTimeF(start_time,end_time);
     return ret;
 }
 

src/Mod/Sketcher/App/Sketch.h

@@ -397,6 +397,10 @@ protected:
     float SolveTime;
     bool RecalculateInitialSolutionWhileMovingPoint;
 
+    // regulates a second solve for cases where there result of having update the geometry (e.g. via OCCT)
+    // needs to be taken into account by the solver (for example to provide the right value of non-driving constraints)
+    bool resolveAfterGeometryUpdated;
+
 protected:
     /// container element to store and work with the geometric elements of this sketch
     struct GeoDef {
@@ -498,6 +502,8 @@ private:
 
     void clearTemporaryConstraints(void);
 
+    int internalSolve(std::string & solvername, int level = 0);
+
     /// checks if the index bounds and converts negative indices to positive
     int checkGeoId(int geoId) const;
     GCS::Curve* getGCSCurveByGeoId(int geoId);