From 82e41125da40dc129427f4b6353c0f7c510cd92a Mon Sep 17 00:00:00 2001
From: Abdullah Tahiri <abdullah.tahiri.yo@gmail.com>
Date: Mon, 7 Sep 2015 14:20:47 +0200
Subject: [PATCH] Sketcher bug/feature request: arePointsCoincident/coincidence
 creation

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

1. SketchObject::arePointsCoincident upgraded to check for indirect coincidence.
2. Coincidence constraint creation now checks for indirect coincidences and avoids
creating redundant coincidence constraints (for example during box selection).
---
 src/Mod/Sketcher/App/SketchObject.cpp       | 33 ++++++++++++++-------
 src/Mod/Sketcher/Gui/CommandConstraints.cpp | 16 ++--------
 2 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp
index ba51a70b3b29..b67b4ab4b061 100644
--- a/src/Mod/Sketcher/App/SketchObject.cpp
+++ b/src/Mod/Sketcher/App/SketchObject.cpp
@@ -3205,17 +3205,30 @@ bool SketchObject::arePointsCoincident(int GeoId1, PointPos PosId1,
 {
     if (GeoId1 == GeoId2 && PosId1 == PosId2)
         return true;
-
-    const std::vector<Constraint *> &constraints = this->Constraints.getValues();
-    for (std::vector<Constraint *>::const_iterator it=constraints.begin();
-         it != constraints.end(); ++it) {
-        if ((*it)->Type == Sketcher::Coincident)
-            if (((*it)->First == GeoId1 && (*it)->FirstPos == PosId1 &&
-                 (*it)->Second == GeoId2 && (*it)->SecondPos == PosId2) ||
-                ((*it)->First == GeoId2 && (*it)->FirstPos == PosId2 &&
-                 (*it)->Second == GeoId1 && (*it)->SecondPos == PosId1))
-                return true;
+    
+    const std::vector< std::map<int, Sketcher::PointPos> > coincidenttree = getCoincidenceGroups();
+    
+    for(std::vector< std::map<int, Sketcher::PointPos> >::const_iterator it = coincidenttree.begin(); it != coincidenttree.end(); ++it) {
+        
+        std::map<int, Sketcher::PointPos>::const_iterator geoId1iterator;
+        
+        geoId1iterator = (*it).find(GeoId1);
+        
+        if( geoId1iterator != (*it).end()) {
+            // If First is in this set
+            std::map<int, Sketcher::PointPos>::const_iterator geoId2iterator;
+            
+            geoId2iterator = (*it).find(GeoId2);
+            
+            if( geoId2iterator != (*it).end()) {
+                // If Second is in this set
+                if ((*geoId1iterator).second == PosId1 &&
+                    (*geoId2iterator).second == PosId2)
+                    return true;                  
+            }
+        }
     }
+
     return false;
 }
 
diff --git a/src/Mod/Sketcher/Gui/CommandConstraints.cpp b/src/Mod/Sketcher/Gui/CommandConstraints.cpp
index e3a593fd357e..789f1e0d496e 100644
--- a/src/Mod/Sketcher/Gui/CommandConstraints.cpp
+++ b/src/Mod/Sketcher/Gui/CommandConstraints.cpp
@@ -862,7 +862,6 @@ void CmdSketcherConstrainCoincident::activated(int iMsg)
     // get the needed lists and objects
     const std::vector<std::string> &SubNames = selection[0].getSubNames();
     Sketcher::SketchObject* Obj = dynamic_cast<Sketcher::SketchObject*>(selection[0].getObject());
-    const std::vector< Sketcher::Constraint * > &vals = Obj->Constraints.getValues();
 
     if (SubNames.size() < 2) {
         QMessageBox::warning(Gui::getMainWindow(), QObject::tr("Wrong selection"),
@@ -891,19 +890,8 @@ void CmdSketcherConstrainCoincident::activated(int iMsg)
     for (std::size_t i=1; i<SubNames.size(); i++) {
         getIdsFromName(SubNames[i], Obj, GeoId2, PosId2);
 
-        // check if any of the coincident constraints exist
-        bool constraintExists=false;
-
-        for (std::vector< Sketcher::Constraint * >::const_iterator it= vals.begin(); it != vals.end(); ++it) {
-            if ((*it)->Type == Sketcher::Coincident && (
-               ((*it)->First == GeoId1 && (*it)->FirstPos == PosId1 &&
-                (*it)->Second == GeoId2 && (*it)->SecondPos == PosId2  ) ||
-               ((*it)->First == GeoId2 && (*it)->FirstPos == PosId2 &&
-                (*it)->Second == GeoId1 && (*it)->SecondPos == PosId1  ) ) ) {
-                constraintExists=true;
-                break;
-            }
-        }
+        // check if this coincidence is already enforced (even indirectly)
+        bool constraintExists=Obj->arePointsCoincident(GeoId1,PosId1,GeoId2,PosId2);
 
         if (!constraintExists) {
             constraintsAdded = true;