Fix snapping issue in selector tool

Fixed bugs:
  - https://launchpad.net/bugs/1255764

(bzr r13361)

3 files changed, 48 insertions, 32 deletions

diff --git a/src/display/snap-indicator.cpp b/src/display/snap-indicator.cpp
index 627bfb2ab..3a8358174 100644
--- a/src/display/snap-indicator.cpp
+++ b/src/display/snap-indicator.cpp
@@ -73,6 +73,7 @@ SnapIndicator::set_new_snaptarget(Inkscape::SnappedPoint const &p, bool pre_snap
         switch (p.getTarget()) {
             case SNAPTARGET_UNDEFINED:
                 target_name = _("UNDEFINED");
+                g_warning("Snap target has not been specified");
                 break;
             case SNAPTARGET_GRID:
                 target_name = _("grid line");
@@ -172,7 +173,7 @@ SnapIndicator::set_new_snaptarget(Inkscape::SnappedPoint const &p, bool pre_snap
                 target_name = _("constraint");
                 break;
             default:
-                g_warning("Snap target has not yet been defined!");
+                g_warning("Snap target not in SnapTargetType enum");
                 break;
         }
 
@@ -180,6 +181,7 @@ SnapIndicator::set_new_snaptarget(Inkscape::SnappedPoint const &p, bool pre_snap
         switch (p.getSource()) {
             case SNAPSOURCE_UNDEFINED:
                 source_name = _("UNDEFINED");
+                g_warning("Snap source has not been specified");
                 break;
             case SNAPSOURCE_BBOX_CORNER:
                 source_name = _("Bounding box corner");
@@ -235,7 +237,7 @@ SnapIndicator::set_new_snaptarget(Inkscape::SnappedPoint const &p, bool pre_snap
                 source_name = _("Multiple of grid spacing");
                 break;
             default:
-                g_warning("Snap source has not yet been defined!");
+                g_warning("Snap source not in SnapSourceType enum");
                 break;
         }
         //std::cout << "Snapped " << source_name << " to " << target_name << std::endl;
diff --git a/src/seltrans.cpp b/src/seltrans.cpp
index 7708c999e..fa2441847 100644
--- a/src/seltrans.cpp
+++ b/src/seltrans.cpp
@@ -929,23 +929,29 @@ gboolean Inkscape::SelTrans::scaleRequest(Geom::Point &pt, guint state)
             sn = m.freeSnapScale(_snap_points, _point, geom_scale, _origin_for_specpoints);
         }
 
-        if (!(bb.getSnapped() || sn.getSnapped())) {
+        std::cout << bb.getSnapped() << " | " << sn.getSnapped() << std::endl;
+        // These lines below are duplicated in stretchRequest
+        if (bb.getSnapped() || sn.getSnapped()) {
+            if (bb.getSnapped()) {
+                if (!bb.isOtherSnapBetter(sn, false)) {
+                    // We snapped the bbox (which is either visual or geometric)
+                    _desktop->snapindicator->set_new_snaptarget(bb);
+                    default_scale = Geom::Scale(bb.getTransformation());
+                    // Calculate the new transformation and update the handle position
+                    pt = _calcAbsAffineDefault(default_scale);
+                }
+            } else if (sn.getSnapped()) {
+                _desktop->snapindicator->set_new_snaptarget(sn);
+                // We snapped the special points (e.g. nodes), which are not at the visual bbox
+                // The handle location however (pt) might however be at the visual bbox, so we
+                // will have to calculate pt taking the stroke width into account
+                geom_scale = Geom::Scale(sn.getTransformation());
+                pt = _calcAbsAffineGeom(geom_scale);
+            }
+        } else {
             // We didn't snap at all! Don't update the handle position, just calculate the new transformation
             _calcAbsAffineDefault(default_scale);
             _desktop->snapindicator->remove_snaptarget();
-        } else if (bb.getSnapped() && !bb.isOtherSnapBetter(sn, false)) {
-            // We snapped the bbox (which is either visual or geometric)
-            _desktop->snapindicator->set_new_snaptarget(bb);
-            default_scale = Geom::Scale(bb.getTransformation());
-            // Calculate the new transformation and update the handle position
-            pt = _calcAbsAffineDefault(default_scale);
-        } else {
-            _desktop->snapindicator->set_new_snaptarget(sn);
-            // We snapped the special points (e.g. nodes), which are not at the visual bbox
-            // The handle location however (pt) might however be at the visual bbox, so we
-            // will have to calculate pt taking the stroke width into account
-            geom_scale = Geom::Scale(sn.getTransformation());
-            pt = _calcAbsAffineGeom(geom_scale);
         }
         m.unSetup();
     }
@@ -1028,20 +1034,28 @@ gboolean Inkscape::SelTrans::stretchRequest(SPSelTransHandle const &handle, Geom
             geom_scale[perp] = fabs(geom_scale[axis]);
         }
 
-        if (!(bb.getSnapped() || sn.getSnapped())) {
+        // These lines below are duplicated in scaleRequest
+        if (bb.getSnapped() || sn.getSnapped()) {
+            if (bb.getSnapped()) {
+                if (!bb.isOtherSnapBetter(sn, false)) {
+                    // We snapped the bbox (which is either visual or geometric)
+                    _desktop->snapindicator->set_new_snaptarget(bb);
+                    default_scale = Geom::Scale(bb.getTransformation());
+                    // Calculate the new transformation and update the handle position
+                    pt = _calcAbsAffineDefault(default_scale);
+                }
+            } else if (sn.getSnapped()) {
+                _desktop->snapindicator->set_new_snaptarget(sn);
+                // We snapped the special points (e.g. nodes), which are not at the visual bbox
+                // The handle location however (pt) might however be at the visual bbox, so we
+                // will have to calculate pt taking the stroke width into account
+                geom_scale = Geom::Scale(sn.getTransformation());
+                pt = _calcAbsAffineGeom(geom_scale);
+            }
+        } else {
             // We didn't snap at all! Don't update the handle position, just calculate the new transformation
             _calcAbsAffineDefault(default_scale);
             _desktop->snapindicator->remove_snaptarget();
-        } else if (bb.getSnapped() && !bb.isOtherSnapBetter(sn, false)) {
-            _desktop->snapindicator->set_new_snaptarget(bb);
-            // Calculate the new transformation and update the handle position
-            pt = _calcAbsAffineDefault(default_scale);
-        } else {
-            _desktop->snapindicator->set_new_snaptarget(sn);
-            // We snapped the special points (e.g. nodes), which are not at the visual bbox
-            // The handle location however (pt) might however be at the visual bbox, so we
-            // will have to calculate pt taking the stroke width into account
-            pt = _calcAbsAffineGeom(geom_scale);
         }
 
         m.unSetup();
diff --git a/src/snap.cpp b/src/snap.cpp
index ecf799cdc..ea6322e37 100644
--- a/src/snap.cpp
+++ b/src/snap.cpp
@@ -239,7 +239,7 @@ Inkscape::SnappedPoint SnapManager::constrainedSnap(Inkscape::SnapCandidatePoint
         // Snapping the mouse pointer instead of the constrained position of the knot allows
         // to snap to things which don't intersect with the constraint line; this is basically
         // then just a freesnap with the constraint applied afterwards
-        // We'll only to this if we're dragging a single handle, and for example not when transforming an object in the selector tool
+        // We'll only do this if we're dragging a single handle, and for example not when transforming an object in the selector tool
         result = freeSnap(p, bbox_to_snap);
         if (result.getSnapped()) {
             // only change the snap indicator if we really snapped to something
@@ -570,7 +570,7 @@ Inkscape::SnappedPoint SnapManager::_snapTransformed(
                 // be collected. Therefore we enforce that the first SnapCandidatePoint that is to be freeSnapped always
                 // has source_num == 0;
                 // TODO: This is a bit ugly so fix this; do we need sourcenum for anything else? if we don't then get rid
-                // of it and explicitely communicate to the object snapper that this is a first point
+                // of it and explicitly communicate to the object snapper that this is a first point
                 if (first_free_snap) {
                     (*j).setSourceNum(0);
                     first_free_snap = false;
@@ -616,8 +616,8 @@ Inkscape::SnappedPoint SnapManager::_snapTransformed(
                 // and the scaling factor for the other direction should remain
                 // untouched (unless scaling is uniform of course)
                 for (int index = 0; index < 2; index++) {
-                    if (fabs(b[index]) > 1e-6) { // if SCALING CAN occur in this direction
-                        if (fabs(fabs(a[index]/b[index]) - fabs(transformation[index])) > 1e-12) { // if SNAPPING DID occur in this direction
+                    if (fabs(b[index]) > 1e-4) { // if SCALING CAN occur in this direction
+                        if (fabs(fabs(a[index]/b[index]) - fabs(transformation[index])) > 1e-7) { // if SNAPPING DID occur in this direction
                             result[index] = a[index] / b[index]; // then calculate it!
                         }
                         // we might have left result[1-index] = Geom::infinity()
@@ -669,7 +669,7 @@ Inkscape::SnappedPoint SnapManager::_snapTransformed(
             case ROTATE:
                 // a is vector to snapped point; b is vector to original point; now lets calculate angle between a and b
                 result[0] = atan2(Geom::dot(Geom::rot90(b), a), Geom::dot(b, a));
-                result[1] = result[1]; // how else should we store an angle in a point ;-)
+                result[1] = result[0]; // dummy value; how else should we store an angle in a point ;-)
                 if (Geom::L2(b) < 1e-9) { // points too close to the rotation center will not move. Don't try to snap these
                     // as they will always yield a perfect snap result if they're already snapped beforehand (e.g.
                     // when the transformation center has been snapped to a grid intersection in the selector tool)