From 224b3ec7d799d00d4af167f45db73cec15084b0f Mon Sep 17 00:00:00 2001 From: Abdullah Tahiri Date: Sun, 6 Sep 2015 11:38:51 +0200 Subject: [PATCH] Sketcher: Improvement: Horizontal/Vertical Autoconstraint creation with External Geometry ===================================================================================== This fixes a bug related to: http://www.freecadweb.org/tracker/view.php?id=2093 that during creation of a geometric element if a vertical/horiz autoconstraint is to be enforced, it is not enforced if the endpoints of the geometric element under creation are coincident with external geometry. According to the discussion here: http://forum.freecadweb.org/viewtopic.php?f=10&t=12254&sid=eacf5bdee068cb71cc54dc5a62a6849d&start=20#p99359 this fixes the bug. It does not fulfil the request on the ticket as it was decided to still allow an explicit addition of a vertical/horizontal constraint, as it may be needed in some cases and the user expects to be able to add them, even if it will lead to an overconstrained sketch. How to reproduce? 1. Create a rectange 2. Pad it 3. Create a new sketch on a face 4. link two corners as "external geometry" (but not the ones of a diagonal) 5. Create a line coincident with the first and second corners, so that the line is horizontal or vertical In master it will force horiz or vert leading to a overconstrained sketch. With this patch, the horiz/vert will not be enforced in this case. --- src/Mod/Sketcher/App/SketchObject.cpp | 89 ++++++++++++++++++++++ src/Mod/Sketcher/App/SketchObject.h | 8 +- src/Mod/Sketcher/Gui/DrawSketchHandler.cpp | 36 +++++++-- 3 files changed, 124 insertions(+), 9 deletions(-) diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp index af093df4efd6..ba51a70b3b29 100644 --- a/src/Mod/Sketcher/App/SketchObject.cpp +++ b/src/Mod/Sketcher/App/SketchObject.cpp @@ -3074,6 +3074,95 @@ void SketchObject::rebuildVertexIndex(void) } } +const std::vector< std::map > SketchObject::getCoincidenceGroups() +{ + // this function is different from that in getCoincidentPoints in that: + // - getCoincidentPoints only considers direct coincidence (the points that are linked via a single coincidence) + // - this function provides an array of maps of points, each map containing the points that are coincident by virtue + // of any number of interrelated coincidence constraints (if coincidence 1-2 and coincidence 2-3, {1,2,3} are in that set) + + const std::vector< Sketcher::Constraint * > &vals = Constraints.getValues(); + + std::vector< std::map > coincidenttree; + // push the constraints + for (std::vector< Sketcher::Constraint * >::const_iterator it= vals.begin();it != vals.end(); ++it) { + if( (*it)->Type == Sketcher::Coincident ) { + int firstpresentin=-1; + int secondpresentin=-1; + + int i=0; + + for(std::vector< std::map >::const_iterator iti = coincidenttree.begin(); iti != coincidenttree.end(); ++iti,i++) { + // First + std::map::const_iterator filiterator; + filiterator = (*iti).find((*it)->First); + if( filiterator != (*iti).end()) { + if((*it)->FirstPos == (*filiterator).second) + firstpresentin = i; + } + // Second + filiterator = (*iti).find((*it)->Second); + if( filiterator != (*iti).end()) { + if((*it)->SecondPos == (*filiterator).second) + secondpresentin = i; + } + } + + if ( firstpresentin!=-1 && secondpresentin!=-1) { + // we have to merge those sets into one + coincidenttree[firstpresentin].insert(coincidenttree[secondpresentin].begin(), coincidenttree[secondpresentin].end()); + coincidenttree.erase(coincidenttree.begin()+secondpresentin); + } + else if ( firstpresentin==-1 && secondpresentin==-1 ) { + // we do not have any of the values, so create a setCursor + std::map tmp; + tmp.insert(std::pair((*it)->First,(*it)->FirstPos)); + tmp.insert(std::pair((*it)->Second,(*it)->SecondPos)); + coincidenttree.push_back(tmp); + } + else if ( firstpresentin != -1 ) { + // add to existing group + coincidenttree[firstpresentin].insert(std::pair((*it)->Second,(*it)->SecondPos)); + } + else { // secondpresentin != -1 + // add to existing group + coincidenttree[secondpresentin].insert(std::pair((*it)->First,(*it)->FirstPos)); + } + + } + } + + return coincidenttree; +} + +void SketchObject::isCoincidentWithExternalGeometry(int GeoId, bool &start_external, bool &mid_external, bool &end_external) { + + start_external=false; + mid_external=false; + end_external=false; + + const std::vector< std::map > coincidenttree = getCoincidenceGroups(); + + for(std::vector< std::map >::const_iterator it = coincidenttree.begin(); it != coincidenttree.end(); ++it) { + + std::map::const_iterator geoId1iterator; + + geoId1iterator = (*it).find(GeoId); + + if( geoId1iterator != (*it).end()) { + // If First is in this set and the first key in this ordered element key is external + if( (*it).begin()->first < 0 ) { + if( (*geoId1iterator).second == Sketcher::start ) + start_external=true; + else if ( (*geoId1iterator).second == Sketcher::mid ) + mid_external=true; + else if ( (*geoId1iterator).second == Sketcher::end ) + end_external=true; + } + } + } +} + void SketchObject::getCoincidentPoints(int GeoId, PointPos PosId, std::vector &GeoIdList, std::vector &PosIdList) { diff --git a/src/Mod/Sketcher/App/SketchObject.h b/src/Mod/Sketcher/App/SketchObject.h index 5d58fb46a115..48119ee7a5f2 100644 --- a/src/Mod/Sketcher/App/SketchObject.h +++ b/src/Mod/Sketcher/App/SketchObject.h @@ -176,7 +176,13 @@ class SketcherExport SketchObject : public Part::Part2DObject /// retrieves for a GeoId and PosId the Vertex number int getVertexIndexGeoPos(int GeoId, PointPos PosId) const; - /// retrieves for a Vertex number a list with all coincident points + // retrieves an array of maps, each map containing the points that are coincidence by virtue of + // any number of direct or indirect coincidence constraints + const std::vector< std::map > getCoincidenceGroups(); + // returns if the given geoId is fixed (coincident) with external geometry on any of the possible relevant points + void isCoincidentWithExternalGeometry(int GeoId, bool &start_external, bool &mid_external, bool &end_external); + + /// retrieves for a Vertex number a list with all coincident points (sharing a single coincidence constraint) void getCoincidentPoints(int GeoId, PointPos PosId, std::vector &GeoIdList, std::vector &PosIdList); void getCoincidentPoints(int VertexId, std::vector &GeoIdList, std::vector &PosIdList); diff --git a/src/Mod/Sketcher/Gui/DrawSketchHandler.cpp b/src/Mod/Sketcher/Gui/DrawSketchHandler.cpp index 97a95974ed7a..9b35cd7972e5 100644 --- a/src/Mod/Sketcher/Gui/DrawSketchHandler.cpp +++ b/src/Mod/Sketcher/Gui/DrawSketchHandler.cpp @@ -428,16 +428,36 @@ void DrawSketchHandler::createAutoConstraints(const std::vector ); } break; case Sketcher::Horizontal: { - Gui::Command::doCommand(Gui::Command::Doc,"App.ActiveDocument.%s.addConstraint(Sketcher.Constraint('Horizontal',%i)) " - ,sketchgui->getObject()->getNameInDocument() - ,geoId1 - ); + + bool start_external; + bool mid_external; + bool end_external; + + dynamic_cast((sketchgui->getObject()))->isCoincidentWithExternalGeometry(geoId1, start_external, mid_external, end_external); + + if( !(start_external && end_external) ) { + Gui::Command::doCommand(Gui::Command::Doc,"App.ActiveDocument.%s.addConstraint(Sketcher.Constraint('Horizontal',%i)) " + ,sketchgui->getObject()->getNameInDocument() + ,geoId1 + ); + } + } break; case Sketcher::Vertical: { - Gui::Command::doCommand(Gui::Command::Doc,"App.ActiveDocument.%s.addConstraint(Sketcher.Constraint('Vertical',%i)) " - ,sketchgui->getObject()->getNameInDocument() - ,geoId1 - ); + + bool start_external; + bool mid_external; + bool end_external; + + dynamic_cast((sketchgui->getObject()))->isCoincidentWithExternalGeometry(geoId1, start_external, mid_external, end_external); + + if( !(start_external && end_external) ) { + Gui::Command::doCommand(Gui::Command::Doc,"App.ActiveDocument.%s.addConstraint(Sketcher.Constraint('Vertical',%i)) " + ,sketchgui->getObject()->getNameInDocument() + ,geoId1 + ); + } + } break; case Sketcher::Tangent: { Sketcher::SketchObject* Obj = dynamic_cast(sketchgui->getObject());