From 0641e243e0b9246cf3f55af95ada809a5bc2058a Mon Sep 17 00:00:00 2001 From: Abdullah Tahiri Date: Wed, 10 Feb 2021 18:49:56 +0100 Subject: [PATCH] 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 | 59 ++++++++++++++++++++++++++------- src/Mod/Sketcher/App/Sketch.h | 6 ++++ 2 files changed, 53 insertions(+), 12 deletions(-) diff --git a/src/Mod/Sketcher/App/Sketch.cpp b/src/Mod/Sketcher/App/Sketch.cpp index 4a1cf68b3d..86a21cb61c 100644 --- a/src/Mod/Sketcher/App/Sketch.cpp +++ b/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::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(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 knots; std::vector mult; - std::vector::const_iterator it3; + // This is the code that should be here when/if b-spline gets its full implementation in the solver. + /*std::vector::const_iterator it3; std::vector::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::iterator it = Constrs.begin();it!=Constrs.end();++it){ + for (std::vector::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; } diff --git a/src/Mod/Sketcher/App/Sketch.h b/src/Mod/Sketcher/App/Sketch.h index db846229db..626576b27e 100644 --- a/src/Mod/Sketcher/App/Sketch.h +++ b/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);