In TryConstrain(), reject redundant constraints in overconstrained gr…

…oups.

This keeps groups with allowed redundant constraints cleaner when
they are used together with automatic constraints.

src/constraint.cpp

@@ -106,10 +106,16 @@ hConstraint Constraint::Constrain(Constraint::Type type, hEntity ptA, hEntity pt
 hConstraint Constraint::TryConstrain(Constraint::Type type, hEntity ptA, hEntity ptB,
                                      hEntity entityA, hEntity entityB,
                                      bool other, bool other2) {
-    SolveResult solvedBefore = SS.TestRankForGroup(SS.GW.activeGroup);
+    int rankBefore, rankAfter;
+    SolveResult howBefore = SS.TestRankForGroup(SS.GW.activeGroup, &rankBefore);
     hConstraint hc = Constrain(type, ptA, ptB, entityA, entityB, other, other2);
-    SolveResult solvedAfter = SS.TestRankForGroup(SS.GW.activeGroup);
-    if(solvedBefore == SolveResult::OKAY && solvedAfter == SolveResult::REDUNDANT_OKAY) {
+    SolveResult howAfter = SS.TestRankForGroup(SS.GW.activeGroup, &rankAfter);
+    // There are two cases where the constraint is clearly redundant:
+    //   * If the group wasn't overconstrained and now it is;
+    //   * If the group was overconstrained, and adding the constraint doesn't change rank at all.
+    if((howBefore == SolveResult::OKAY && howAfter == SolveResult::REDUNDANT_OKAY) ||
+       (howBefore == SolveResult::REDUNDANT_OKAY && howAfter == SolveResult::REDUNDANT_OKAY &&
+            rankBefore == rankAfter)) {
         SK.constraint.RemoveById(hc);
         hc = {};
     }

src/generate.cpp

@@ -539,7 +539,8 @@ void SolveSpaceUI::SolveGroup(hGroup hg, bool andFindFree) {
     WriteEqSystemForGroup(hg);
     Group *g = SK.GetGroup(hg);
     g->solved.remove.Clear();
-    SolveResult how = sys.Solve(g, &(g->solved.dof),
+    SolveResult how = sys.Solve(g, NULL,
+                                   &(g->solved.dof),
                                    &(g->solved.remove),
                                    /*andFindBad=*/true,
                                    /*andFindFree=*/andFindFree,
@@ -551,12 +552,10 @@ void SolveSpaceUI::SolveGroup(hGroup hg, bool andFindFree) {
     FreeAllTemporary();
 }
 
-SolveResult SolveSpaceUI::TestRankForGroup(hGroup hg) {
+SolveResult SolveSpaceUI::TestRankForGroup(hGroup hg, int *rank) {
     WriteEqSystemForGroup(hg);
     Group *g = SK.GetGroup(hg);
-    SolveResult result = sys.SolveRank(g, NULL, NULL,
-                                       /*andFindBad=*/false,
-                                       /*andFindFree=*/false);
+    SolveResult result = sys.SolveRank(g, rank);
     FreeAllTemporary();
     return result;
 }

src/solvespace.h

@@ -289,7 +289,7 @@ class System {
 
     static const double RANK_MAG_TOLERANCE, CONVERGE_TOLERANCE;
     int CalculateRank();
-    bool TestRank();
+    bool TestRank(int *rank = NULL);
     static bool SolveLinearSystem(double X[], double A[][MAX_UNKNOWNS],
                                   double B[], int N);
     bool SolveLeastSquares();
@@ -308,11 +308,14 @@ class System {
     void MarkParamsFree(bool findFree);
     int CalculateDof();
 
-    SolveResult Solve(Group *g, int *dof, List<hConstraint> *bad,
-                      bool andFindBad, bool andFindFree, bool forceDofCheck = false);
+    SolveResult Solve(Group *g, int *rank = NULL, int *dof = NULL,
+                      List<hConstraint> *bad = NULL,
+                      bool andFindBad = false, bool andFindFree = false,
+                      bool forceDofCheck = false);
 
-    SolveResult SolveRank(Group *g, int *dof, List<hConstraint> *bad,
-                          bool andFindBad, bool andFindFree);
+    SolveResult SolveRank(Group *g, int *rank = NULL, int *dof = NULL,
+                          List<hConstraint> *bad = NULL,
+                          bool andFindBad = false, bool andFindFree = false);
 
     void Clear();
 };
@@ -776,7 +779,7 @@ class SolveSpaceUI {
                      bool genForBBox = false);
     void SolveGroup(hGroup hg, bool andFindFree);
     void SolveGroupAndReport(hGroup hg, bool andFindFree);
-    SolveResult TestRankForGroup(hGroup hg);
+    SolveResult TestRankForGroup(hGroup hg, int *rank = NULL);
     void WriteEqSystemForGroup(hGroup hg);
     void MarkDraggedParams();
     void ForceReferences();

src/system.cpp

@@ -170,9 +170,11 @@ int System::CalculateRank() {
     return rank;
 }
 
-bool System::TestRank() {
+bool System::TestRank(int *rank) {
     EvalJacobian();
-    return CalculateRank() == mat.m;
+    int jacobianRank = CalculateRank();
+    if(rank) *rank = jacobianRank;
+    return jacobianRank == mat.m;
 }
 
 bool System::SolveLinearSystem(double X[], double A[][MAX_UNKNOWNS],
@@ -396,7 +398,7 @@ void System::FindWhichToRemoveToFixJacobian(Group *g, List<hConstraint> *bad, bo
     }
 }
 
-SolveResult System::Solve(Group *g, int *dof, List<hConstraint> *bad,
+SolveResult System::Solve(Group *g, int *rank, int *dof, List<hConstraint> *bad,
                           bool andFindBad, bool andFindFree, bool forceDofCheck)
 {
     WriteEquationsExceptFor(Constraint::NO_CONSTRAINT, g);
@@ -459,14 +461,14 @@ SolveResult System::Solve(Group *g, int *dof, List<hConstraint> *bad,
         return SolveResult::TOO_MANY_UNKNOWNS;
     }
 
-    rankOk = TestRank();
+    rankOk = TestRank(rank);
 
     // And do the leftovers as one big system
     if(!NewtonSolve(0)) {
         goto didnt_converge;
     }
 
-    rankOk = TestRank();
+    rankOk = TestRank(rank);
     if(!rankOk) {
         if(!g->allowRedundant) {
             if(andFindBad) FindWhichToRemoveToFixJacobian(g, bad, forceDofCheck);
@@ -517,7 +519,7 @@ SolveResult System::Solve(Group *g, int *dof, List<hConstraint> *bad,
     return rankOk ? SolveResult::DIDNT_CONVERGE : SolveResult::REDUNDANT_DIDNT_CONVERGE;
 }
 
-SolveResult System::SolveRank(Group *g, int *dof, List<hConstraint> *bad,
+SolveResult System::SolveRank(Group *g, int *rank, int *dof, List<hConstraint> *bad,
                               bool andFindBad, bool andFindFree)
 {
     WriteEquationsExceptFor(Constraint::NO_CONSTRAINT, g);
@@ -532,15 +534,12 @@ SolveResult System::SolveRank(Group *g, int *dof, List<hConstraint> *bad,
         return SolveResult::TOO_MANY_UNKNOWNS;
     }
 
-    bool rankOk = TestRank();
+    bool rankOk = TestRank(rank);
     if(!rankOk) {
         if(!g->allowRedundant) {
             if(andFindBad) FindWhichToRemoveToFixJacobian(g, bad, /*forceDofCheck=*/true);
         }
     } else {
-        // This is not the full Jacobian, but any substitutions or single-eq
-        // solves removed one equation and one unknown, therefore no effect
-        // on the number of DOF.
         if(dof) *dof = CalculateDof();
         MarkParamsFree(andFindFree);
     }