1) solve bug #177995

2) don't look for nearby intersections when doing a constrained snap

(bzr r4275)

4 files changed, 60 insertions, 53 deletions

diff --git a/src/snap.cpp b/src/snap.cpp
index 9faa4fae6..0d7a25ad6 100644
--- a/src/snap.cpp
+++ b/src/snap.cpp
@@ -216,7 +216,7 @@ Inkscape::SnappedPoint SnapManager::freeSnap(Inkscape::Snapper::PointType t,
         (*i)->freeSnap(sc, t, p, first_point, points_to_snap, it);
     }
 
-    return findBestSnap(p, sc);
+    return findBestSnap(p, sc, false);
 }
 
 /**
@@ -274,12 +274,14 @@ Inkscape::SnappedPoint SnapManager::constrainedSnap(Inkscape::Snapper::PointType
         (*i)->constrainedSnap(sc, t, p, first_point, points_to_snap, c, it);
     }
 
-    return findBestSnap(p, sc);
+    return findBestSnap(p, sc, true);
 }
 
 Inkscape::SnappedPoint SnapManager::guideSnap(NR::Point const &p,
                                              NR::Point const &guide_normal) const
 {
+    
+    // This method is used to snap a guide to nodes, while dragging the guide around
     Inkscape::ObjectSnapper::DimensionToSnap snap_dim;
     if (guide_normal == component_vectors[NR::Y]) {
         snap_dim = Inkscape::ObjectSnapper::SNAP_Y;
@@ -287,13 +289,17 @@ Inkscape::SnappedPoint SnapManager::guideSnap(NR::Point const &p,
         snap_dim = Inkscape::ObjectSnapper::SNAP_X;
     } else {
         g_warning("WARNING: snapping of angled guides is not supported yet!");
+        // this is because _snapnodes, called in object.guideSnap, cannot only handle
+        // vertical or horizontal lines for now....
+        // Rotating an agled guide will require some additional code, as it would be great to
+        // have it rotate around a snapped point
         snap_dim = Inkscape::ObjectSnapper::SNAP_XY;
     }
     
     SnappedConstraints sc;
     object.guideSnap(sc, p, snap_dim);
     
-    return findBestSnap(p, sc);    
+    return findBestSnap(p, sc, false);    
 }
 
 
@@ -625,7 +631,7 @@ std::pair<NR::Coord, bool> SnapManager::freeSnapSkew(Inkscape::Snapper::PointTyp
    return std::make_pair(r.first[d], r.second);
 }
 
-Inkscape::SnappedPoint SnapManager::findBestSnap(NR::Point const &p, SnappedConstraints &sc) const
+Inkscape::SnappedPoint SnapManager::findBestSnap(NR::Point const &p, SnappedConstraints &sc, bool constrained) const
 {
     NR::Coord const guide_sens = guide.getDistance();
     NR::Coord grid_sens = 0;
@@ -677,24 +683,32 @@ Inkscape::SnappedPoint SnapManager::findBestSnap(NR::Point const &p, SnappedCons
         sp_list.push_back(std::make_pair(Inkscape::SnappedPoint(closestGuideLine), NR_HUGE));
     }
     
-    // search for the closest snapped intersection of grid lines
-    Inkscape::SnappedPoint closestGridPoint;
-    if (getClosestIntersectionSL(sc.grid_lines, closestGridPoint)) {
-        sp_list.push_back(std::make_pair(closestGridPoint, NR_HUGE));
-    }
+    // When freely snapping to a grid/guide/path, only one degree of freedom is eliminated
+    // Therefore we will try get fully constrained by finding an intersection with another grid/guide/path 
     
-    // search for the closest snapped intersection of guide lines
-    Inkscape::SnappedPoint closestGuidePoint;
-    if (getClosestIntersectionSL(sc.guide_lines, closestGuidePoint)) {
-        sp_list.push_back(std::make_pair(closestGuidePoint, NR_HUGE));
-    }
-    
-    // search for the closest snapped intersection of grid with guide lines
-    if (_intersectionGG) {
-	    Inkscape::SnappedPoint closestGridGuidePoint;
-	    if (getClosestIntersectionSL(sc.grid_lines, sc.guide_lines, closestGridGuidePoint)) {
-	        sp_list.push_back(std::make_pair(closestGridGuidePoint, std::min(guide_sens, grid_sens)));
-	    }
+    // When doing a constrained snap however, we're already at an intersection of the constrained line and
+    // the grid/guide/path we're snapping to. This snappoint is therefore fully constrained, so there's
+    // no need to look for additional intersections
+    if (!constrained) {
+        // search for the closest snapped intersection of grid lines
+        Inkscape::SnappedPoint closestGridPoint;
+        if (getClosestIntersectionSL(sc.grid_lines, closestGridPoint)) {
+            sp_list.push_back(std::make_pair(closestGridPoint, NR_HUGE));
+        }
+        
+        // search for the closest snapped intersection of guide lines
+        Inkscape::SnappedPoint closestGuidePoint;
+        if (getClosestIntersectionSL(sc.guide_lines, closestGuidePoint)) {
+            sp_list.push_back(std::make_pair(closestGuidePoint, NR_HUGE));
+        }
+        
+        // search for the closest snapped intersection of grid with guide lines
+        if (_intersectionGG) {
+    	    Inkscape::SnappedPoint closestGridGuidePoint;
+    	    if (getClosestIntersectionSL(sc.grid_lines, sc.guide_lines, closestGridGuidePoint)) {
+    	        sp_list.push_back(std::make_pair(closestGridGuidePoint, std::min(guide_sens, grid_sens)));
+    	    }
+        }
     }
     
     // now let's see which snapped point gets a thumbs up
diff --git a/src/snap.h b/src/snap.h
index f7882fc61..db7ec8a89 100644
--- a/src/snap.h
+++ b/src/snap.h
@@ -181,7 +181,7 @@ private:
                                                 NR::Dim2 dim,
                                                 bool uniform) const;
                                                 
-    Inkscape::SnappedPoint findBestSnap(NR::Point const &p, SnappedConstraints &sc) const;
+    Inkscape::SnappedPoint findBestSnap(NR::Point const &p, SnappedConstraints &sc, bool constrained) const;
 };
 
 #endif /* !SEEN_SNAP_H */
diff --git a/src/snapper.cpp b/src/snapper.cpp
index 096a224d1..1daa1d151 100644
--- a/src/snapper.cpp
+++ b/src/snapper.cpp
@@ -172,11 +172,10 @@ void Inkscape::Snapper::constrainedSnap(SnappedConstraints &sc,
  */
 
 void Inkscape::Snapper::constrainedSnap(SnappedConstraints &sc, 
-                                                          
                                                           PointType const &t,
                                                           NR::Point const &p,
                                                           bool const &first_point,
-                                                           std::vector<NR::Point> &points_to_snap,                         
+                                                          std::vector<NR::Point> &points_to_snap,                         
                                                           ConstraintLine const &c,
                                                           std::list<SPItem const *> const &it) const
 {
diff --git a/src/sp-shape.cpp b/src/sp-shape.cpp
index 488dd9993..f7c9f0e71 100644
--- a/src/sp-shape.cpp
+++ b/src/sp-shape.cpp
@@ -1111,39 +1111,33 @@ static void sp_shape_snappoints(SPItem const *item, SnapPointsIter p)
     if (shape->curve == NULL) {
         return;
     }
-
+    
     NR::Matrix const i2d (sp_item_i2d_affine (item));
-
     NArtBpath const *b = SP_CURVE_BPATH(shape->curve);    
-    g_assert((b->code == NR_MOVETO) || (b->code == NR_MOVETO_OPEN));
- 
-    // Consider the first point in the path
-    NR::Point pos = b->c(3) * i2d;
-    if (b->code == NR_MOVETO_OPEN) { // Indicates the start of a open subpath, see nr-path-code.h
-        *p = pos;
-        // If at the other hand we're looking at a closed subpath, then we can
-        // skip this first point because it's coincident with the last point.  
-    }
-    b++;
     
-    // Cycle through the subsequent nodes in the path
-    while (b->code == NR_LINETO || b->code == NR_CURVETO) {
-        pos = b->c(3) * i2d; // this is the current node
-        
-        if (b->code == NR_LINETO || b[1].code == NR_LINETO || b[1].code == NR_END) {
-            // end points of a line segment are always considered for snapping
-            *p = pos; 
-        } else {        
-            NR::Point ppos, npos;
-            ppos = b->code == NR_CURVETO ? b->c(2) * i2d : pos; // backward handle 
-            npos = b[1].code == NR_CURVETO ? b[1].c(1) * i2d : pos; // forward handle
+    // Cycle through the nodes in the concatenated subpaths
+    while (b->code != NR_END) {
+        NR::Point pos = b->c(3) * i2d; // this is the current node
         
-            // Determine whether a node is at a smooth part of the path, by 
-            // calculating a measure for the collinearity of the handles
-            bool c1 = fabs (Inkscape::Util::triangle_area (pos, ppos, npos)) < 1; // points are (almost) collinear
-            bool c2 = NR::L2(pos - ppos) < 1e-6 || NR::L2(pos - npos) < 1e-6; // endnode, or a node with a retracted handle
-            if (!(c1 & !c2)) {
-                *p = pos; // only return non-smooth nodes ("cusps")
+        // NR_MOVETO Indicates the start of a closed subpath, see nr-path-code.h
+        // If we're looking at a closed subpath, then we can skip this first 
+        // point of the subpath because it's coincident with the last point.  
+        if (b->code != NR_MOVETO) {
+            if (b->code == NR_MOVETO_OPEN || b->code == NR_LINETO || b[1].code == NR_LINETO || b[1].code == NR_END) {
+                // end points of a line segment are always considered for snapping
+                *p = pos; 
+            } else {        
+                // g_assert(b->code == NR_CURVETO);
+                NR::Point ppos, npos;
+                ppos = b->code == NR_CURVETO ? b->c(2) * i2d : pos; // backward handle 
+                npos = b[1].code == NR_CURVETO ? b[1].c(1) * i2d : pos; // forward handle            
+                // Determine whether a node is at a smooth part of the path, by 
+                // calculating a measure for the collinearity of the handles
+                bool c1 = fabs (Inkscape::Util::triangle_area (pos, ppos, npos)) < 1; // points are (almost) collinear
+                bool c2 = NR::L2(pos - ppos) < 1e-6 || NR::L2(pos - npos) < 1e-6; // endnode, or a node with a retracted handle
+                if (!(c1 & !c2)) {
+                    *p = pos; // only return non-smooth nodes ("cusps")
+                }
             }
         }