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

@@ -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;
 }
 

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;