1) Fix bug #178312

2) Fix constrained snapping to objects

(bzr r4314)

12 files changed, 340 insertions, 149 deletions

diff --git a/src/libnr/nr-path.cpp b/src/libnr/nr-path.cpp
index 03882a7fd..a0011c9dc 100644
--- a/src/libnr/nr-path.cpp
+++ b/src/libnr/nr-path.cpp
@@ -503,3 +503,22 @@ nr_path_matrix_bbox_union(NRBPath const *bpath, NR::Matrix const &m,
     }
 }
 
+NArtBpath *nr_path_from_rect(NRRect const &r)
+{
+    NArtBpath *path = g_new (NArtBpath, 6);
+
+    path[0].code = NR_MOVETO;
+    path[0].setC(3, NR::Point(r.x0, r.y0));
+    path[1].code = NR_LINETO;
+    path[1].setC(3, NR::Point(r.x1, r.y0)); 
+    path[2].code = NR_LINETO;
+    path[2].setC(3, NR::Point(r.x1, r.y1)); 
+    path[3].code = NR_LINETO;
+    path[3].setC(3, NR::Point(r.x0, r.y1));
+    path[4].code = NR_LINETO;
+    path[4].setC(3, NR::Point(r.x0, r.y0));    
+    path[5].code = NR_END;
+
+    return path;
+}
+
diff --git a/src/libnr/nr-path.h b/src/libnr/nr-path.h
index 95b4f726b..6fdc85694 100644
--- a/src/libnr/nr-path.h
+++ b/src/libnr/nr-path.h
@@ -46,6 +46,8 @@ void nr_path_matrix_point_bbox_wind_distance (NRBPath *bpath, NR::Matrix const &
 
 void nr_path_matrix_bbox_union(NRBPath const *bpath, NR::Matrix const &m, NRRect *bbox);
 
+NArtBpath *nr_path_from_rect(NRRect const &r);
+
 #endif
 
 /*
diff --git a/src/libnr/nr-point-fns.cpp b/src/libnr/nr-point-fns.cpp
index a18e878b4..4428ed9b3 100644
--- a/src/libnr/nr-point-fns.cpp
+++ b/src/libnr/nr-point-fns.cpp
@@ -103,6 +103,24 @@ get_offset_between_points (NR::Point p, NR::Point begin, NR::Point end)
     return (r / length);
 }
 
+NR::Point
+project_on_linesegment(NR::Point const p, NR::Point const p1, NR::Point const p2) 
+{
+    // p_proj = projection of p on the linesegment running from p1 to p2
+    // p_proj = p1 + u (p2 - p1)
+    // calculate u according to "Minimum Distance between a Point and a Line"
+    // see http://local.wasp.uwa.edu.au/~pbourke/geometry/pointline/        
+    
+    if (p1 == p2) { // to avoid div. by zero below
+        return p;
+    }
+    
+    NR::Point const d1(p-p1); // delta 1
+    NR::Point const d2(p2-p1); // delta 2
+    double const u = (d1[NR::X] * d2[NR::X] + d1[NR::Y] * d2[NR::Y]) / (NR::L2(d2) * NR::L2(d2));
+    
+    return (p1 + u*(p2-p1));
+}
 
 /*
   Local Variables:
diff --git a/src/libnr/nr-point-fns.h b/src/libnr/nr-point-fns.h
index a8c38466f..e927725b4 100644
--- a/src/libnr/nr-point-fns.h
+++ b/src/libnr/nr-point-fns.h
@@ -96,6 +96,8 @@ NR::Point snap_vector_midpoint (NR::Point p, NR::Point begin, NR::Point end, dou
 
 double get_offset_between_points (NR::Point p, NR::Point begin, NR::Point end);
 
+NR::Point project_on_linesegment(NR::Point const p, NR::Point const p1, NR::Point const p2); 
+
 #endif /* !__NR_POINT_OPS_H__ */
 
 /*
diff --git a/src/line-snapper.cpp b/src/line-snapper.cpp
index 2fa08e1f1..7fc6a32a4 100644
--- a/src/line-snapper.cpp
+++ b/src/line-snapper.cpp
@@ -40,20 +40,10 @@ void Inkscape::LineSnapper::_doFreeSnap(SnappedConstraints &sc,
     for (LineList::const_iterator i = lines.begin(); i != lines.end(); i++) {
         NR::Point const p1 = i->second; // point at guide/grid line
         NR::Point const p2 = p1 + NR::rot90(i->first); // 2nd point at guide/grid line
-        
         // std::cout << "  line through " << i->second << " with normal " << i->first;
+        g_assert(i->first != NR::Point(0,0)); // we cannot project on an linesegment of zero length
         
-        g_assert(i->first != NR::Point(0,0)); // otherwise we'll have div. by zero because NR::L2(d2) = 0
-        
-        // p_proj = projection of p on the grid/guide line running from p1 to p2
-        // p_proj = p1 + u (p2 - p1)
-        // calculate u according to "Minimum Distance between a Point and a Line"
-        // see http://local.wasp.uwa.edu.au/~pbourke/geometry/pointline/        
-        NR::Point const d1(p-p1); // delta 1
-        NR::Point const d2(p2-p1); // delta 1
-        double const u = (d1[NR::X] * d2[NR::X] + d1[NR::Y] * d2[NR::Y]) / (NR::L2(d2) * NR::L2(d2));
-        
-        NR::Point const p_proj(p1 + u*(p2-p1));
+        NR::Point const p_proj = project_on_linesegment(p, p1, p2);
         NR::Coord const dist = NR::L2(p_proj - p);
         //Store any line that's within snapping range
         if (dist < getDistance()) {
@@ -97,9 +87,12 @@ void Inkscape::LineSnapper::_doConstrainedSnap(SnappedConstraints &sc,
 
         if (k == Geom::intersects) {
             const NR::Coord dist = L2(t - p);
-            //Store any line that's within snapping range
             if (dist < getDistance()) {
-				_addSnappedLine(sc, t, dist, c.getDirection(), t);
+				// When doing a constrained snap, we're already at an intersection.
+                // This snappoint is therefore fully constrained, so there's no need
+                // to look for additional intersections; just return the snapped point
+                // and forget about the line
+                sc.points.push_back(SnappedPoint(t, dist)); 
             }
         }
     }
diff --git a/src/object-snapper.cpp b/src/object-snapper.cpp
index 84955cf6d..71c790c17 100644
--- a/src/object-snapper.cpp
+++ b/src/object-snapper.cpp
@@ -4,14 +4,19 @@
  *
  * Authors:
  *   Carl Hetherington <inkscape@carlh.net>
+ *   Diederik van Lierop <mail@diedenrezi.nl>
  *
- * Copyright (C) 2005 Authors
+ * Copyright (C) 2005 - 2007 Authors
  *
  * Released under GNU GPL, read the file 'COPYING' for more information
  */
 
 #include "libnr/n-art-bpath.h"
+#include "libnr/nr-path.h"
 #include "libnr/nr-rect-ops.h"
+#include "libnr/nr-point-fns.h"
+#include "live_effects/n-art-bpath-2geom.h"
+#include "2geom/path-intersection.h"
 #include "document.h"
 #include "sp-namedview.h"
 #include "sp-image.h"
@@ -33,6 +38,7 @@ Inkscape::ObjectSnapper::ObjectSnapper(SPNamedView const *nv, NR::Coord const d)
 {
     _candidates = new std::vector<SPItem*>;
     _points_to_snap_to = new std::vector<NR::Point>;
+    _bpaths_to_snap_to = new std::vector<NArtBpath*>;
     _paths_to_snap_to = new std::vector<Path*>;
 }
 
@@ -44,16 +50,14 @@ Inkscape::ObjectSnapper::~ObjectSnapper()
     _points_to_snap_to->clear();
     delete _points_to_snap_to;
 
-    for (std::vector<Path*>::const_iterator k = _paths_to_snap_to->begin(); k != _paths_to_snap_to->end(); k++) {
-        delete *k;
-    }
-    _paths_to_snap_to->clear();
+    _clear_paths();
     delete _paths_to_snap_to;
+    delete _bpaths_to_snap_to;
 }
 
 /**
- *    Find all items within snapping range.
- *     \param r Pointer to the current document
+ *  Find all items within snapping range.
+ *  \param r Pointer to the current document
  *  \param it List of items to ignore
  *  \param first_point If true then this point is the first one from a whole bunch of points
  *  \param points_to_snap The whole bunch of points, all from the same selection and having the same transformation
@@ -113,33 +117,30 @@ void Inkscape::ObjectSnapper::_findCandidates(SPObject* r,
 }
 
 
-void Inkscape::ObjectSnapper::_snapNodes(SnappedConstraints &sc,
-                      					 Inkscape::Snapper::PointType const &t,
-                                         NR::Point const &p,
-                                         bool const &first_point,
-                                         DimensionToSnap const snap_dim) const
+void Inkscape::ObjectSnapper::_collectNodes(Inkscape::Snapper::PointType const &t,
+                                         bool const &first_point) const
 {
-    bool success = false;
-
-    // Determine the type of bounding box we should snap to
-    SPItem::BBoxType bbox_type = SPItem::GEOMETRIC_BBOX;
-    if (_snap_to_bboxnode) {
-        gchar const *prefs_bbox = prefs_get_string_attribute("tools.select", "bounding_box");
-        bbox_type = (prefs_bbox != NULL && strcmp(prefs_bbox, "geometric")==0)? SPItem::GEOMETRIC_BBOX : SPItem::APPROXIMATE_BBOX;
-    }
-
-    bool p_is_a_node = t & Inkscape::Snapper::SNAPPOINT_NODE;
-    bool p_is_a_bbox = t & Inkscape::Snapper::SNAPPOINT_BBOX;
-    bool p_is_a_guide = t & Inkscape::Snapper::SNAPPOINT_GUIDE;
-
-    // A point considered for snapping should be either a node, a bbox corner or a guide. Pick only ONE!
-    g_assert(!(p_is_a_node && p_is_a_bbox || p_is_a_bbox && p_is_a_guide || p_is_a_node && p_is_a_guide));
-
     // Now, let's first collect all points to snap to. If we have a whole bunch of points to snap,
     // e.g. when translating an item using the selector tool, then we will only do this for the
-    // first point and store the collection for later use. This dramatically improves the performance
+    // first point and store the collection for later use. This significantly improves the performance
     if (first_point) {
         _points_to_snap_to->clear();
+        
+         // Determine the type of bounding box we should snap to
+        SPItem::BBoxType bbox_type = SPItem::GEOMETRIC_BBOX;
+        
+        bool p_is_a_node = t & Inkscape::Snapper::SNAPPOINT_NODE;
+        bool p_is_a_bbox = t & Inkscape::Snapper::SNAPPOINT_BBOX;
+        bool p_is_a_guide = t & Inkscape::Snapper::SNAPPOINT_GUIDE;
+        
+        // A point considered for snapping should be either a node, a bbox corner or a guide. Pick only ONE!
+        g_assert(!(p_is_a_node && p_is_a_bbox || p_is_a_bbox && p_is_a_guide || p_is_a_node && p_is_a_guide));        
+        
+        if (_snap_to_bboxnode) {
+            gchar const *prefs_bbox = prefs_get_string_attribute("tools.select", "bounding_box");
+            bbox_type = (prefs_bbox != NULL && strcmp(prefs_bbox, "geometric")==0)? SPItem::GEOMETRIC_BBOX : SPItem::APPROXIMATE_BBOX;
+        }
+
         for (std::vector<SPItem*>::const_iterator i = _candidates->begin(); i != _candidates->end(); i++) {
             //NR::Matrix i2doc(NR::identity());
             SPItem *root_item = *i;
@@ -167,10 +168,21 @@ void Inkscape::ObjectSnapper::_snapNodes(SnappedConstraints &sc,
             }
         }
     }
+}
 
-    //Do the snapping, using all the nodes and corners collected above
+void Inkscape::ObjectSnapper::_snapNodes(SnappedConstraints &sc,
+                                         Inkscape::Snapper::PointType const &t,
+                                         NR::Point const &p,
+                                         bool const &first_point,
+                                         DimensionToSnap const snap_dim) const
+{
+    _collectNodes(t, first_point);
+    
+    //Do the snapping, using all the nodes and corners collected before
     NR::Point snapped_point;        
     SnappedPoint s;
+    bool success = false;
+    
     for (std::vector<NR::Point>::const_iterator k = _points_to_snap_to->begin(); k != _points_to_snap_to->end(); k++) {
         /* Try to snap to this node of the path */
         NR::Coord dist = NR_HUGE;
@@ -201,36 +213,25 @@ void Inkscape::ObjectSnapper::_snapNodes(SnappedConstraints &sc,
 }
 
 
-void Inkscape::ObjectSnapper::_snapPaths(SnappedConstraints &sc,
-                      					 Inkscape::Snapper::PointType const &t,
-                                         NR::Point const &p,
+void Inkscape::ObjectSnapper::_collectPaths(Inkscape::Snapper::PointType const &t,
                                          bool const &first_point) const
 {
-    bool success = false;
-    /* FIXME: this seems like a hack.  Perhaps Snappers should be
-    ** in SPDesktop rather than SPNamedView?
-    */
-    SPDesktop const *desktop = SP_ACTIVE_DESKTOP;
-
-    NR::Point const p_doc = desktop->dt2doc(p);
-
-    // Determine the type of bounding box we should snap to
-    SPItem::BBoxType bbox_type = SPItem::GEOMETRIC_BBOX;
-    if (_snap_to_bboxpath) {
-        gchar const *prefs_bbox = prefs_get_string_attribute("tools.select", "bounding_box");
-        bbox_type = (prefs_bbox != NULL && strcmp(prefs_bbox, "geometric")==0)? SPItem::GEOMETRIC_BBOX : SPItem::APPROXIMATE_BBOX;
-    }
-
-    bool p_is_a_node = t & Inkscape::Snapper::SNAPPOINT_NODE;
-
     // Now, let's first collect all paths to snap to. If we have a whole bunch of points to snap,
     // e.g. when translating an item using the selector tool, then we will only do this for the
-    // first point and store the collection for later use. This dramatically improves the performance
+    // first point and store the collection for later use. This significantly improves the performance
     if (first_point) {
-        for (std::vector<Path*>::const_iterator k = _paths_to_snap_to->begin(); k != _paths_to_snap_to->end(); k++) {
-            delete *k;
+        _clear_paths();
+        
+        // Determine the type of bounding box we should snap to
+        SPItem::BBoxType bbox_type = SPItem::GEOMETRIC_BBOX;
+        
+        bool p_is_a_node = t & Inkscape::Snapper::SNAPPOINT_NODE;
+        
+        if (_snap_to_bboxpath) {
+            gchar const *prefs_bbox = prefs_get_string_attribute("tools.select", "bounding_box");
+            bbox_type = (prefs_bbox != NULL && strcmp(prefs_bbox, "geometric")==0)? SPItem::GEOMETRIC_BBOX : SPItem::APPROXIMATE_BBOX;
         }
-        _paths_to_snap_to->clear();
+            
         for (std::vector<SPItem*>::const_iterator i = _candidates->begin(); i != _candidates->end(); i++) {
 
             /* Transform the requested snap point to this item's coordinates */
@@ -269,44 +270,63 @@ void Inkscape::ObjectSnapper::_snapPaths(SnappedConstraints &sc,
                         very_complex_path = sp_nodes_in_path(SP_PATH(root_item)) > 500;
                     }
 
-                     if (!very_lenghty_prose && !very_complex_path) {
-                        _paths_to_snap_to->push_back(Path_for_item(root_item, true, true));
-                     }
+                    if (!very_lenghty_prose && !very_complex_path) {
+                        SPCurve *curve = curve_for_item(root_item); 
+                        if (curve) {
+                            NArtBpath *bpath = bpath_for_curve(root_item, curve, true, true);
+                            _bpaths_to_snap_to->push_back(bpath);
+                            // Because in bpath_for_curve we set doTransformation to true, we
+                            // will get a dupe of the path, which must be freed at some point
+                            sp_curve_unref(curve);
+                        }
+                    }
                 }
             }
 
             //Add the item's bounding box to snap to
             if (_snap_to_bboxpath) {
                 if (!(_strict_snapping && p_is_a_node)) {
-                    //This will get ugly... rect -> curve -> bpath
                     NRRect rect;
                     sp_item_invoke_bbox(root_item, &rect, i2doc, TRUE, bbox_type);
-                    NR::Maybe<NR::Rect> bbox = rect.upgrade();
-                    SPCurve *curve = sp_curve_new_from_rect(bbox);
-                    if (curve) {
-                        NArtBpath *bpath = SP_CURVE_BPATH(curve);
-                        if (bpath) {
-                            Path *path = bpath_to_Path(bpath);
-                            if (path) {
-                                _paths_to_snap_to->push_back(path);
-                            }
-                            delete bpath;
-                        }
-                        delete curve;
-                    }
+                    NArtBpath *bpath = nr_path_from_rect(rect);
+                    _bpaths_to_snap_to->push_back(bpath);
                 }
             }
         }
     }
-
-    //Now we can finally do the real snapping, using the paths collected above
+}
+    
+void Inkscape::ObjectSnapper::_snapPaths(SnappedConstraints &sc,
+                                     Inkscape::Snapper::PointType const &t,
+                                     NR::Point const &p,
+                                     bool const &first_point) const
+{
+    _collectPaths(t, first_point);
+    
+    // Now we can finally do the real snapping, using the paths collected above
     SnappedPoint s;
+    bool success = false;
+    
+    /* FIXME: this seems like a hack.  Perhaps Snappers should be
+    ** in SPDesktop rather than SPNamedView?
+    */
+    SPDesktop const *desktop = SP_ACTIVE_DESKTOP;    
+    NR::Point const p_doc = desktop->dt2doc(p);    
+    
+    // Convert all bpaths to Paths, because here we really must have Paths
+    // (whereas in _snapPathsConstrained we will use the original bpaths)
+    if (first_point) {
+        for (std::vector<NArtBpath*>::const_iterator k = _bpaths_to_snap_to->begin(); k != _bpaths_to_snap_to->end(); k++) {
+            Path *path = bpath_to_Path(*k);
+            if (path) {
+                path->ConvertWithBackData(0.01); //This is extremely time consuming!
+                _paths_to_snap_to->push_back(path);
+            }    
+        }
+    }
+    
     for (std::vector<Path*>::const_iterator k = _paths_to_snap_to->begin(); k != _paths_to_snap_to->end(); k++) {
         if (*k) {
-            if (first_point) {
-                (*k)->ConvertWithBackData(0.01); //This is extremely time consuming!
-            }
-
             /* Look for the nearest position on this SPItem to our snap point */
             NR::Maybe<Path::cut_position> const o = get_nearest_position_on_Path(*k, p_doc);
             if (o && o->t >= 0 && o->t <= 1) {
@@ -343,12 +363,71 @@ void Inkscape::ObjectSnapper::_snapPaths(SnappedConstraints &sc,
     }
 }
 
+void Inkscape::ObjectSnapper::_snapPathsConstrained(SnappedConstraints &sc,
+                                     Inkscape::Snapper::PointType const &t,
+                                     NR::Point const &p,
+                                     bool const &first_point,
+                                     ConstraintLine const &c) const
+{
+    _collectPaths(t, first_point);
+    
+    // Now we can finally do the real snapping, using the paths collected above
+    
+    /* FIXME: this seems like a hack.  Perhaps Snappers should be
+    ** in SPDesktop rather than SPNamedView?
+    */
+    SPDesktop const *desktop = SP_ACTIVE_DESKTOP;    
+    NR::Point const p_doc = desktop->dt2doc(p);    
+    
+    NR::Point direction_vector = c.getDirection();
+    if (!is_zero(direction_vector)) {
+        direction_vector = NR::unit_vector(direction_vector);
+    }
+    
+    NR::Point const p1_on_cl = c.hasPoint() ? c.getPoint() : p;
+    NR::Point const p2_on_cl = p1_on_cl + direction_vector;
+    
+    // The intersection point of the constraint line with any path, 
+    // must lie within two points on the constraintline: p_min_on_cl and p_max_on_cl
+    // The distance between those points is twice the max. snapping distance
+    NR::Point const p_proj_on_cl = project_on_linesegment(p, p1_on_cl, p2_on_cl);
+    NR::Point const p_min_on_cl = desktop->dt2doc(p_proj_on_cl - getDistance() * direction_vector);    
+    NR::Point const p_max_on_cl = desktop->dt2doc(p_proj_on_cl + getDistance() * direction_vector);
+    
+    Geom::Path cl;
+    cl.start(p_min_on_cl.to_2geom());
+    cl.appendNew<Geom::LineSegment>(p_max_on_cl.to_2geom());
+        
+    for (std::vector<NArtBpath*>::const_iterator k = _bpaths_to_snap_to->begin(); k != _bpaths_to_snap_to->end(); k++) {
+        if (*k) {                        
+            // convert a Path object (see src/livarot/Path.h) to a 2geom's path object (see 2geom/path.h)
+            // TODO (Diederik) Only do this once for the first point, needs some storage of pointers in a member variable
+            std::vector<Geom::Path> path_2geom = BPath_to_2GeomPath(*k);  
+            
+            for (std::vector<Geom::Path>::const_iterator l = path_2geom.begin(); l != path_2geom.end(); l++) {
+                Geom::SimpleCrosser sxr;
+                Geom::Crossings crossings = sxr.crossings(*l, cl);
+                for (std::vector<Geom::Crossing>::const_iterator m = crossings.begin(); m != crossings.end(); m++) {
+                    // Reconstruct the point of intersection
+                    NR::Point p_inters = p_min_on_cl + ((*m).tb) * (p_max_on_cl - p_min_on_cl);
+                    // When it's within snapping range, then return it
+                    // (within snapping range == between p_min_on_cl and p_max_on_cl == 0 < tb < 1)
+                    if ((*m).tb >= 0 && (*m).tb <= 1 ) {
+                        SnappedPoint s(desktop->doc2dt(p_inters), NR::L2(p_proj_on_cl - p_inters));
+                        sc.points.push_back(s);    
+                    }  
+                } 
+            }
+        }
+    }
+}
+
 
 void Inkscape::ObjectSnapper::_doFreeSnap(SnappedConstraints &sc,
                                             Inkscape::Snapper::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,
                                             std::list<SPItem const *> const &it) const
 {
     if ( NULL == _named_view ) {
@@ -375,13 +454,30 @@ void Inkscape::ObjectSnapper::_doConstrainedSnap( SnappedConstraints &sc,
                                                   NR::Point const &p,
                                                   bool const &first_point,
                                                   std::vector<NR::Point> &points_to_snap,
-                                                  ConstraintLine const &/*c*/,
+                                                  ConstraintLine const &c,
                                                   std::list<SPItem const *> const &it) const
 {
-    /* FIXME: this needs implementing properly; I think we have to do the
-    ** intersection of c with the objects.
-    */
-    _doFreeSnap(sc, t, p, first_point, points_to_snap, it);
+    if ( NULL == _named_view ) {
+        return;
+    }
+
+    /* Get a list of all the SPItems that we will try to snap to */
+    if (first_point) {
+        _findCandidates(sp_document_root(_named_view->document), it, first_point, points_to_snap, SNAP_XY);
+    }
+    
+    // A constrained snap, is a snap in only one degree of freedom (specified by the constraint line).
+    // This is usefull for example when scaling an object while maintaining a fixed aspect ratio. It's
+    // nodes are only allowed to move in one direction (i.e. in one degree of freedom).
+    
+    // When snapping to objects, we either snap to their nodes or their paths. It is however very
+    // unlikely that any node will be exactly at the constrained line, so for a constrained snap
+    // to objects we will only consider the object's paths. Beside, the nodes will be at these paths,
+    // so we will more or less snap to them anyhow.   
+
+    if (_snap_to_itempath || _snap_to_bboxpath) {
+        _snapPathsConstrained(sc, t, p, first_point, c);
+    }
 }
 
 
@@ -414,6 +510,18 @@ bool Inkscape::ObjectSnapper::ThisSnapperMightSnap() const
     return (_snap_enabled && _snap_from != 0 && snap_to_something);
 }
 
+void Inkscape::ObjectSnapper::_clear_paths() const 
+{
+    for (std::vector<NArtBpath*>::const_iterator k = _bpaths_to_snap_to->begin(); k != _bpaths_to_snap_to->end(); k++) {
+        g_free(*k);
+    }
+    _bpaths_to_snap_to->clear();
+    
+    for (std::vector<Path*>::const_iterator k = _paths_to_snap_to->begin(); k != _paths_to_snap_to->end(); k++) {
+        delete *k;    
+    }
+    _paths_to_snap_to->clear();
+}
 
 /*
   Local Variables:
diff --git a/src/object-snapper.h b/src/object-snapper.h
index b3bf481ad..c16b42580 100644
--- a/src/object-snapper.h
+++ b/src/object-snapper.h
@@ -7,8 +7,9 @@
  *
  * Authors:
  *   Carl Hetherington <inkscape@carlh.net>
+ *   Diederik van Lierop <mail@diedenrezi.nl>
  *
- * Copyright (C) 2005 Authors 
+ * Copyright (C) 2005 - 2007 Authors 
  *
  * Released under GNU GPL, read the file 'COPYING' for more information
  */
@@ -87,6 +88,7 @@ private:
   //store some lists of candidates, points and paths, so we don't have to rebuild them for each point we want to snap
   std::vector<SPItem*> *_candidates; 
   std::vector<NR::Point> *_points_to_snap_to;
+  std::vector<NArtBpath*> *_bpaths_to_snap_to;
   std::vector<Path*> *_paths_to_snap_to;
   void _doFreeSnap(SnappedConstraints &sc,
                       Inkscape::Snapper::PointType const &t,
@@ -114,11 +116,24 @@ private:
                       NR::Point const &p, 
                       bool const &first_point,
                       DimensionToSnap const snap_dim) const;
-                      
+
+  void _collectNodes(Inkscape::Snapper::PointType const &t,
+                  bool const &first_point) const;
+  
   void _snapPaths(SnappedConstraints &sc,
                       Inkscape::Snapper::PointType const &t, 
                       NR::Point const &p,
                       bool const &first_point) const;
+                      
+  void _snapPathsConstrained(SnappedConstraints &sc,
+                 Inkscape::Snapper::PointType const &t,
+                 NR::Point const &p,
+                 bool const &first_point,
+                 ConstraintLine const &c) const;
+  
+  void _collectPaths(Inkscape::Snapper::PointType const &t, 
+                  bool const &first_point) const;
+  void _clear_paths() const;
   
   bool _snap_to_itemnode;
   bool _snap_to_itempath;
diff --git a/src/seltrans.cpp b/src/seltrans.cpp
index 785cf7d60..a747e6b4a 100644
--- a/src/seltrans.cpp
+++ b/src/seltrans.cpp
@@ -907,28 +907,15 @@ gboolean Inkscape::SelTrans::scaleRequest(NR::Point &pt, guint state)
             }
 
         // Snap along a suitable constraint vector from the origin.
-
-        // The inclination of the constraint vector is calculated from the aspect ratio
-        NR::Point bbox_dim = _bbox->dimensions();
-        double const aspect_ratio = bbox_dim[1] / bbox_dim[0]; // = height / width
-
-        // Determine direction of the constraint vector
-        NR::Point const cv = NR::Point(
-            pt[NR::X] > _origin[NR::X] ? 1 : -1,
-            pt[NR::Y] > _origin[NR::Y] ? aspect_ratio : -aspect_ratio
-            );
-
         std::pair<NR::scale, bool> bb = m.constrainedSnapScale(Snapper::SNAPPOINT_BBOX,
                                                                _bbox_points,
                                                                it,
-                                                               Snapper::ConstraintLine(_origin_for_bboxpoints, cv),
                                                                s,
                                                                _origin_for_bboxpoints);
 
         std::pair<NR::scale, bool> sn = m.constrainedSnapScale(Snapper::SNAPPOINT_NODE,
                                                                _snap_points,
                                                                it,
-                                                               Snapper::ConstraintLine(_origin_for_specpoints, cv),
                                                                s,
                                                                _origin_for_specpoints);
 
diff --git a/src/snap.cpp b/src/snap.cpp
index 0d7a25ad6..b4e284271 100644
--- a/src/snap.cpp
+++ b/src/snap.cpp
@@ -9,6 +9,7 @@
  *   Frank Felfe <innerspace@iname.com>
  *   Nathan Hurst <njh@njhurst.com>
  *   Carl Hetherington <inkscape@carlh.net>
+ *   Diederik van Lierop <mail@diedenrezi.nl>
  *
  * Copyright (C) 2006-2007 Johan Engelen <johan@shouraizou.nl>
  * Copyrigth (C) 2004      Nathan Hurst
@@ -207,6 +208,9 @@ Inkscape::SnappedPoint SnapManager::freeSnap(Inkscape::Snapper::PointType t,
                                              std::vector<NR::Point> &points_to_snap,
                                              std::list<SPItem const *> const &it) const
 {
+    if (!SomeSnapperMightSnap()) {
+        return Inkscape::SnappedPoint(p, NR_HUGE);
+    }
     
     SnappedConstraints sc;        
     
@@ -266,6 +270,9 @@ Inkscape::SnappedPoint SnapManager::constrainedSnap(Inkscape::Snapper::PointType
                                                     Inkscape::Snapper::ConstraintLine const &c,
                                                     std::list<SPItem const *> const &it) const
 {
+    if (!SomeSnapperMightSnap()) {
+        return Inkscape::SnappedPoint(p, NR_HUGE);
+    }
     
     SnappedConstraints sc;
         
@@ -280,6 +287,9 @@ Inkscape::SnappedPoint SnapManager::constrainedSnap(Inkscape::Snapper::PointType
 Inkscape::SnappedPoint SnapManager::guideSnap(NR::Point const &p,
                                              NR::Point const &guide_normal) const
 {
+    if (!object.ThisSnapperMightSnap()) {
+        return Inkscape::SnappedPoint(p, NR_HUGE);
+    }
     
     // This method is used to snap a guide to nodes, while dragging the guide around
     Inkscape::ObjectSnapper::DimensionToSnap snap_dim;
@@ -397,9 +407,21 @@ std::pair<NR::Point, bool> SnapManager::_snapTransformed(
 
     for (std::vector<NR::Point>::const_iterator i = points.begin(); i != points.end(); i++) {
         
-        /* Snap it */
-        Inkscape::SnappedPoint const snapped = constrained ?
-            constrainedSnap(type, *j, i == points.begin(), transformed_points, constraint, ignore) : freeSnap(type, *j, i == points.begin(), transformed_points, ignore);
+        /* Snap it */        
+        Inkscape::SnappedPoint snapped;
+                
+        if (constrained) {    
+            Inkscape::Snapper::ConstraintLine dedicated_constraint = constraint;
+            if (transformation_type == SCALE) {
+                // When constrained scaling, each point will have its own unique constraint line,
+                // running from the scaling origin to the original untransformed point. We will
+                // calculate that line here 
+                dedicated_constraint = Inkscape::Snapper::ConstraintLine(origin, (*i) - origin);
+            }
+            snapped = constrainedSnap(type, *j, i == points.begin(), transformed_points, dedicated_constraint, ignore);
+        } else {
+            snapped = freeSnap(type, *j, i == points.begin(), transformed_points, ignore);
+        }
 
         NR::Point result;
         NR::Coord metric = NR_HUGE;
@@ -428,8 +450,14 @@ std::pair<NR::Point, bool> SnapManager::_snapTransformed(
                 {
                     NR::Point const a = (snapped.getPoint() - origin);
                     NR::Point const b = (*i - origin);
-                    result = NR::Point(a[NR::X] / b[NR::X], a[NR::Y] / b[NR::Y]);
+                    // This is the scaling that results after snapping
+                    result = NR::Point(a[NR::X] / b[NR::X], a[NR::Y] / b[NR::Y]); 
+                    // Compare the resulting scaling with the desired scaling
                     metric = std::abs(NR::L2(result) - NR::L2(transformation));
+                    // TODO: (Diederik) This only works for snapping of the diagonals 
+                    // as the resulting scale cannot be calculated for points aligned
+                    // vertically or horizontally to the origin, and therefore the the
+                    // metric will also be useless. BTW, what about protection for 1/0?  
                     break;
                 }
                 case STRETCH:
@@ -564,12 +592,11 @@ std::pair<NR::scale, bool> SnapManager::freeSnapScale(Inkscape::Snapper::PointTy
 std::pair<NR::scale, bool> SnapManager::constrainedSnapScale(Inkscape::Snapper::PointType t,
                                                              std::vector<NR::Point> const &p,
                                                              std::list<SPItem const *> const &it,
-                                                             Inkscape::Snapper::ConstraintLine const &c,
                                                              NR::scale const &s,
                                                              NR::Point const &o) const
 {
     return _snapTransformed(
-        t, p, it, true, c, SCALE, NR::Point(s[NR::X], s[NR::Y]), o, NR::X, false
+        t, p, it, true, NR::Point(), SCALE, NR::Point(s[NR::X], s[NR::Y]), o, NR::X, false
         );
 }
 
diff --git a/src/snap.h b/src/snap.h
index db7ec8a89..ee9dbefac 100644
--- a/src/snap.h
+++ b/src/snap.h
@@ -9,6 +9,7 @@
  *   Lauris Kaplinski <lauris@kaplinski.com>
  *   Frank Felfe <innerspace@iname.com>
  *   Carl Hetherington <inkscape@carlh.net>
+ *   Diederik van Lierop <mail@diedenrezi.nl>
  *
  * Copyright (C) 2006-2007 Johan Engelen <johan@shouraizou.nl>
  * Copyright (C) 2000-2002 Lauris Kaplinski
@@ -90,7 +91,6 @@ public:
     std::pair<NR::scale, bool> constrainedSnapScale(Inkscape::Snapper::PointType t,
                                                     std::vector<NR::Point> const &p,
                                                     std::list<SPItem const *> const &it,
-                                                    Inkscape::Snapper::ConstraintLine const &c,
                                                     NR::scale const &s,
                                                     NR::Point const &o) const;
 
diff --git a/src/splivarot.cpp b/src/splivarot.cpp
index fff70eb25..6c083bc30 100644
--- a/src/splivarot.cpp
+++ b/src/splivarot.cpp
@@ -1711,13 +1711,56 @@ Ancetre(Inkscape::XML::Node *a, Inkscape::XML::Node *who)
 Path *
 Path_for_item(SPItem *item, bool doTransformation, bool transformFull)
 {
-    SPCurve *curve = NULL;
+    SPCurve *curve = curve_for_item(item);
+    NArtBpath *bpath = bpath_for_curve(item, curve, doTransformation, transformFull);
+    
+    if (bpath == NULL) {
+        return NULL;
+    }
+    
+    Path *dest = bpath_to_Path(bpath);
+
+    if (doTransformation) {
+        g_free(bpath); // see comment in bpath_for_curve
+    }
+    
+    sp_curve_unref(curve);
+    
+    return dest;
+}
+
+NArtBpath *
+bpath_for_curve(SPItem *item, SPCurve *curve, bool doTransformation, bool transformFull)
+{
+    if (curve == NULL)
+        return NULL;
+
+    NArtBpath *bpath = SP_CURVE_BPATH(curve);
+    if (bpath == NULL) {
+        return NULL;
+    }
+    
+    if (doTransformation) {
+        NArtBpath *new_bpath; // we will get a duplicate which has to be freed at some point!
+        if (transformFull) {
+            new_bpath = nr_artpath_affine(bpath, sp_item_i2doc_affine(item));
+        } else {
+            new_bpath = nr_artpath_affine(bpath, item->transform);
+        }
+        bpath = new_bpath;
+    }
+
+    return bpath;
+}
 
+SPCurve* curve_for_item(SPItem *item)
+{
     if (!item)
         return NULL;
 
-    if (SP_IS_SHAPE(item))
-    {
+    SPCurve *curve = NULL;
+
+    if (SP_IS_SHAPE(item)) {
         if (SP_IS_PATH(item)) {
             curve = sp_path_get_curve_for_edit(SP_PATH(item));
         } else {
@@ -1732,33 +1775,8 @@ Path_for_item(SPItem *item, bool doTransformation, bool transformFull)
     {
         curve = sp_image_get_curve(SP_IMAGE(item));
     }
-
-    if (!curve)
-        return NULL;
-
-    NArtBpath *bpath = SP_CURVE_BPATH(curve);
-    if (bpath == NULL)
-        return NULL;
-
-    if ( doTransformation ) {
-        if (transformFull)
-            bpath = nr_artpath_affine(SP_CURVE_BPATH(curve), sp_item_i2doc_affine(item));
-        else
-            bpath = nr_artpath_affine(SP_CURVE_BPATH(curve), item->transform);
-        sp_curve_unref(curve);
-        curve=NULL;
-    } else {
-        bpath=SP_CURVE_BPATH(curve);
-    }
-
-    Path *dest = bpath_to_Path(bpath);
-
-    if ( doTransformation ) {
-        if ( bpath ) g_free(bpath);
-    } else {
-        sp_curve_unref(curve);
-    }
-    return dest;
+    
+    return curve; // do not forget to unref the curve at some point!
 }
 
 Path *bpath_to_Path(NArtBpath const *bpath) {
diff --git a/src/splivarot.h b/src/splivarot.h
index 3d01f41e0..c0a7fb38b 100644
--- a/src/splivarot.h
+++ b/src/splivarot.h
@@ -43,6 +43,8 @@ void sp_selected_path_outline ();
 void sp_selected_path_simplify ();
 
 Path *Path_for_item(SPItem *item, bool doTransformation, bool transformFull = true);
+NArtBpath *bpath_for_curve(SPItem *item, SPCurve *curve, bool doTransformation, bool transformFull);
+SPCurve *curve_for_item(SPItem *item);
 NR::Maybe<Path::cut_position> get_nearest_position_on_Path(Path *path, NR::Point p);
 NR::Point get_point_on_Path(Path *path, int piece, double t);
 Path *bpath_to_Path(NArtBpath const *bpath);