From fc3086a96ea7dbc479fab8ee683f6a015082cb84 Mon Sep 17 00:00:00 2001
From: Ajinkya Dahale <AjinkyaDahale@users.noreply.github.com>
Date: Tue, 21 Mar 2023 21:26:40 +0530
Subject: [PATCH] [Sketcher] Fix some coincidence issues in B-spline drawing

See https://github.com/FreeCAD/FreeCAD/pull/8530#issuecomment-1474824366.

When there are already existing points and coincidence auto-constraints are
added in the process of making a B-spline (either by control points or
interpolation), unintended behaviour can happen.

Additionally, when creating B-spline by interpolation, if consecutive points are
coincident (or very close to each other), the OCCT algorithm fails. This is also
prevented in this commit.
---
 .../Sketcher/Gui/DrawSketchHandlerBSpline.h   | 14 ++++++++--
 .../DrawSketchHandlerBSplineByInterpolation.h | 28 ++++++++++++++-----
 2 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/src/Mod/Sketcher/Gui/DrawSketchHandlerBSpline.h b/src/Mod/Sketcher/Gui/DrawSketchHandlerBSpline.h
index 02cfdd2adf..d7c1484eb7 100644
--- a/src/Mod/Sketcher/Gui/DrawSketchHandlerBSpline.h
+++ b/src/Mod/Sketcher/Gui/DrawSketchHandlerBSpline.h
@@ -141,8 +141,18 @@ public:
 
             // check if coincident with first pole
             for(auto & ac : sugConstr.back()) {
-                if( ac.Type == Sketcher::Coincident && ac.GeoId == poleGeoIds[0] && ac.PosId == Sketcher::PointPos::mid ) {
-                    IsClosed = true;
+                if (ac.Type == Sketcher::Coincident) {
+                    if (ac.GeoId == poleGeoIds[0] && ac.PosId == Sketcher::PointPos::mid)
+                        IsClosed = true;
+                    else {
+                        // The coincidence with first point may be indirect
+                        const auto coincidents =
+                            static_cast<Sketcher::SketchObject*>(sketchgui->getObject())
+                            ->getAllCoincidentPoints(ac.GeoId, ac.PosId);
+                        if (coincidents.find(poleGeoIds[0]) != coincidents.end() &&
+                            coincidents.at(poleGeoIds[0]) == Sketcher::PointPos::mid)
+                            IsClosed = true;
+                    }
                 }
             }
 
diff --git a/src/Mod/Sketcher/Gui/DrawSketchHandlerBSplineByInterpolation.h b/src/Mod/Sketcher/Gui/DrawSketchHandlerBSplineByInterpolation.h
index 14fec6b4fb..ef2f06a240 100644
--- a/src/Mod/Sketcher/Gui/DrawSketchHandlerBSplineByInterpolation.h
+++ b/src/Mod/Sketcher/Gui/DrawSketchHandlerBSplineByInterpolation.h
@@ -136,17 +136,31 @@ public:
             addSugConstraint();
         }
         else if (Mode == STATUS_SEEK_ADDITIONAL_POINTS) {
-            BSplineKnots.push_back(onSketchPos);
-            BSplineMults.push_back(1);// NOTE: not strictly true for end-points
-
-            // check if coincident with first knot
+            // check if coincidence issues with first or last added knot
             for (auto& ac : sugConstr.back()) {
-                if (ac.Type == Sketcher::Coincident && ac.GeoId == knotGeoIds[0]
-                    && ac.PosId == Sketcher::PointPos::start) {
-                    IsClosed = true;
+                if (ac.Type == Sketcher::Coincident) {
+                    if (ac.GeoId == knotGeoIds[0] && ac.PosId == Sketcher::PointPos::start)
+                        IsClosed = true;
+                    else {
+                        // The coincidence with first point may be indirect
+                        const auto coincidents =
+                            static_cast<Sketcher::SketchObject*>(sketchgui->getObject())
+                            ->getAllCoincidentPoints(ac.GeoId, ac.PosId);
+                        if (coincidents.find(knotGeoIds[0]) != coincidents.end() &&
+                            coincidents.at(knotGeoIds[0]) == Sketcher::PointPos::start)
+                            IsClosed = true;
+                        else if (coincidents.find(knotGeoIds.back()) != coincidents.end() &&
+                                 coincidents.at(knotGeoIds.back()) == Sketcher::PointPos::start) {
+                            return true;// OCCT doesn't allow consecutive points being coincident
+                        }
+
+                    }
                 }
             }
 
+            BSplineKnots.push_back(onSketchPos);
+            BSplineMults.push_back(1);// NOTE: not strictly true for end-points
+
             if (IsClosed) {
                 Mode = STATUS_CLOSE;