Fix snapping during constrained translation, when only snapping the node closest...

diff --git a/src/display/snap-indicator.cpp b/src/display/snap-indicator.cpp
index 240fe9545184cb6c5b18f019dbb1747f6cde4035..39222d1742fe9d4740146d0eec22caffa31ca995 100644 (file)
--- a/src/display/snap-indicator.cpp
+++ b/src/display/snap-indicator.cpp
@@ -42,7 +42,7 @@ SnapIndicator::~SnapIndicator()
 void
 SnapIndicator::set_new_snaptarget(Inkscape::SnappedPoint const p)
 {
-       remove_snaptarget();
+       remove_snaptarget(); //only display one snaptarget at a time
 
     g_assert(_desktop != NULL);
 

diff --git a/src/line-snapper.cpp b/src/line-snapper.cpp
index a18fb8518bb7a0c82b299c040f43af45f0a005a1..f6a6be74e6456049dbb32bc06258c18597af08e0 100644 (file)
--- a/src/line-snapper.cpp
+++ b/src/line-snapper.cpp
@@ -91,7 +91,7 @@ void Inkscape::LineSnapper::constrainedSnap(SnappedConstraints &sc,
             Geom::Point t(t_2geom);
 
             if (k == Geom::intersects) {
-                const Geom::Coord dist = L2(t - p);
+                const Geom::Coord dist = Geom::L2(t - p);
                 if (dist < getSnapperTolerance()) {
                                // When doing a constrained snap, we're already at an intersection.
                     // This snappoint is therefore fully constrained, so there's no need

diff --git a/src/seltrans.cpp b/src/seltrans.cpp
index 1ee7faff8753e0b412717b3146821674f5eb657d..816a471cf4386f8e12c98c87c95f1e0215dea98d 100644 (file)
--- a/src/seltrans.cpp
+++ b/src/seltrans.cpp
@@ -1416,25 +1416,23 @@ void Inkscape::SelTrans::moveTo(Geom::Point const &xy, guint state)
 
             /* Snap to things, and also constrain to horizontal or vertical movement */
 
-            for (unsigned int dim = 0; dim < 2; dim++) {
-                // When doing a constrained translation, all points will move in the same direction, i.e.
-                // either horizontally or vertically. Therefore we only have to specify the direction of
-                // the constraint-line once. The constraint lines are parallel, but might not be colinear.
-                // Therefore we will have to set the point through which the constraint-line runs
-                // individually for each point to be snapped; this will be handled however by _snapTransformed()
-                s.push_back(m.constrainedSnapTranslation(Inkscape::SnapPreferences::SNAPPOINT_BBOX,
-                                                         _bbox_points,
-                                                         _point,
-                                                         Inkscape::Snapper::ConstraintLine(component_vectors[dim]),
-                                                         dxy));
-
-                s.push_back(m.constrainedSnapTranslation(Inkscape::SnapPreferences::SNAPPOINT_NODE,
-                                                         _snap_points,
-                                                         _point,
-                                                         Inkscape::Snapper::ConstraintLine(component_vectors[dim]),
-                                                         dxy));
-            }
-
+            unsigned int dim = fabs(dxy[Geom::X]) > fabs(dxy[Geom::Y]) ? Geom::X : Geom::Y;
+                       // When doing a constrained translation, all points will move in the same direction, i.e.
+                       // either horizontally or vertically. Therefore we only have to specify the direction of
+                       // the constraint-line once. The constraint lines are parallel, but might not be colinear.
+                       // Therefore we will have to set the point through which the constraint-line runs
+                       // individually for each point to be snapped; this will be handled however by _snapTransformed()
+                       s.push_back(m.constrainedSnapTranslation(Inkscape::SnapPreferences::SNAPPOINT_BBOX,
+                                                                                                        _bbox_points,
+                                                                                                        _point,
+                                                                                                        Inkscape::Snapper::ConstraintLine(component_vectors[dim]),
+                                                                                                        dxy));
+
+                       s.push_back(m.constrainedSnapTranslation(Inkscape::SnapPreferences::SNAPPOINT_NODE,
+                                                                                                        _snap_points,
+                                                                                                        _point,
+                                                                                                        Inkscape::Snapper::ConstraintLine(component_vectors[dim]),
+                                                                                                        dxy));
         } else {
 
             // Let's leave this timer code here for a while. I'll probably need it in the near future (Diederik van Lierop)
@@ -1443,7 +1441,7 @@ void Inkscape::SelTrans::moveTo(Geom::Point const &xy, guint state)
                g_get_current_time(&starttime); */
 
             /* Snap to things with no constraint */
-                       s.push_back(m.freeSnapTranslation(Inkscape::SnapPreferences::SNAPPOINT_BBOX, _bbox_points, _point, dxy));
+               s.push_back(m.freeSnapTranslation(Inkscape::SnapPreferences::SNAPPOINT_BBOX, _bbox_points, _point, dxy));
                s.push_back(m.freeSnapTranslation(Inkscape::SnapPreferences::SNAPPOINT_NODE, _snap_points, _point, dxy));
 
                /*g_get_current_time(&endtime);

diff --git a/src/snapped-point.cpp b/src/snapped-point.cpp
index 7e9a16a66e38b0e4205860209f9d88092c31d7df..2d028c6d78eb441b264e18b8a30f342577d630cb 100644 (file)
--- a/src/snapped-point.cpp
+++ b/src/snapped-point.cpp
@@ -73,7 +73,7 @@ bool getClosestSP(std::list<Inkscape::SnappedPoint> &list, Inkscape::SnappedPoin
     bool success = false;
 
     for (std::list<Inkscape::SnappedPoint>::const_iterator i = list.begin(); i != list.end(); i++) {
-        if ((i == list.begin()) || (*i).getSnapDistance() < result.getSnapDistance()) {
+       if ((i == list.begin()) || (*i).getSnapDistance() < result.getSnapDistance()) {
             result = *i;
             success = true;
         }
@@ -92,7 +92,10 @@ bool Inkscape::SnappedPoint::isOtherSnapBetter(Inkscape::SnappedPoint const &oth
     // there's more than one). It is not useful when trying to find the best snapped target point.
     // (both the snap distance and the pointer distance are measured in document pixels, not in screen pixels)
     if (weighted) {
-        // Weight factor: controls which node should be preferred for snapping, which is either
+
+       Geom::Coord const dist_pointer_other = other_one.getPointerDistance();
+       Geom::Coord const dist_pointer_this = getPointerDistance();
+       // Weight factor: controls which node should be preferred for snapping, which is either
         // the node with the closest snap (w = 0), or the node closest to the mousepointer (w = 1)
         Inkscape::Preferences *prefs = Inkscape::Preferences::get();
         double w = prefs->getDoubleLimited("/options/snapweight/value", 0.5, 0, 1);
@@ -100,19 +103,21 @@ bool Inkscape::SnappedPoint::isOtherSnapBetter(Inkscape::SnappedPoint const &oth
             w = 1;
         }
         if (w > 0) {
-            // When accounting for the distance to the mouse pointer, then at least one of the snapped points should
-            // have that distance set. If not, then this is a bug. Either "weighted" must be set to false, or the
-            // mouse pointer distance must be set.
-            g_assert(getPointerDistance() != NR_HUGE || other_one.getPointerDistance() != NR_HUGE);
-            // The snap distance will always be smaller than the tolerance set for the snapper. The pointer distance can
-            // however be very large. To compare these in a fair way, we will have to normalize these metrics first
-            // The closest pointer distance will be normalized to 1.0; the other one will be > 1.0
-            // The snap distance will be normalized to 1.0 if it's equal to the snapper tolerance
-            double const norm_p = std::min(getPointerDistance(), other_one.getPointerDistance());
-            double const norm_t_other = std::min(50.0, other_one.getTolerance());
-            double const norm_t_this = std::min(50.0, getTolerance());
-            dist_other = w * other_one.getPointerDistance() / norm_p + (1-w) * dist_other / norm_t_other;
-            dist_this = w * getPointerDistance() / norm_p + (1-w) * dist_this / norm_t_this;
+               if (!(w == 1 && dist_pointer_this == dist_pointer_other)) {
+                       // When accounting for the distance to the mouse pointer, then at least one of the snapped points should
+                               // have that distance set. If not, then this is a bug. Either "weighted" must be set to false, or the
+                               // mouse pointer distance must be set.
+                               g_assert(dist_pointer_this != NR_HUGE || dist_pointer_other != NR_HUGE);
+                               // The snap distance will always be smaller than the tolerance set for the snapper. The pointer distance can
+                               // however be very large. To compare these in a fair way, we will have to normalize these metrics first
+                               // The closest pointer distance will be normalized to 1.0; the other one will be > 1.0
+                               // The snap distance will be normalized to 1.0 if it's equal to the snapper tolerance
+                               double const norm_p = std::min(dist_pointer_this, dist_pointer_other);
+                               double const norm_t_other = std::min(50.0, other_one.getTolerance());
+                               double const norm_t_this = std::min(50.0, getTolerance());
+                               dist_other = w * dist_pointer_other / norm_p + (1-w) * dist_other / norm_t_other;
+                               dist_this = w * dist_pointer_this / norm_p + (1-w) * dist_this / norm_t_this;
+               }
         }
     }