Refactoring of the code that handles transformations and snapping in the selector tool and the node tool. Splitting large chunks of code into some small classes, and eliminating some wrapper methods which were all just too similar

(bzr r14363)

1 files changed, 103 insertions, 100 deletions

diff --git a/src/seltrans.cpp b/src/seltrans.cpp
index f7562923f..17d704975 100644
--- a/src/seltrans.cpp
+++ b/src/seltrans.cpp
@@ -31,6 +31,7 @@
 #include "knot.h"
 #include "message-stack.h"
 #include "snap.h"
+#include "pure-transform.h"
 #include "selection.h"
 #include "ui/tools/select-tool.h"
 #include "sp-item.h"
@@ -897,10 +898,7 @@ gboolean Inkscape::SelTrans::scaleRequest(Geom::Point &pt, guint state)
         // When scaling by an integer, snapping is not needed
     } else {
         // In all other cases we should try to snap now
-        SnapManager &m = _desktop->namedview->snap_manager;
-        m.setup(_desktop, false, _items_const);
-
-        Inkscape::SnappedPoint bb, sn;
+        Inkscape::PureScale  *bb, *sn;
 
         if ((state & GDK_CONTROL_MASK) || _desktop->isToolboxButtonActive ("lock")) {
             // Scale is locked to a 1:1 aspect ratio, so that s[X] must be made to equal s[Y].
@@ -915,30 +913,37 @@ gboolean Inkscape::SelTrans::scaleRequest(Geom::Point &pt, guint state)
             }
 
             // Snap along a suitable constraint vector from the origin.
-            bb = m.constrainedSnapScale(_bbox_points, _point, default_scale, _origin_for_bboxpoints);
-            sn = m.constrainedSnapScale(_snap_points, _point, geom_scale, _origin_for_specpoints);
+
+            bb = new Inkscape::PureScaleConstrained(default_scale, _origin_for_bboxpoints);
+            sn = new Inkscape::PureScaleConstrained(geom_scale, _origin_for_specpoints);
         } else {
             /* Scale aspect ratio is unlocked */
-            bb = m.freeSnapScale(_bbox_points, _point, default_scale, _origin_for_bboxpoints);
-            sn = m.freeSnapScale(_snap_points, _point, geom_scale, _origin_for_specpoints);
+            bb = new Inkscape::PureScale(default_scale, _origin_for_bboxpoints, false);
+            sn = new Inkscape::PureScale(geom_scale, _origin_for_specpoints, false);
         }
+        SnapManager &m = _desktop->namedview->snap_manager;
+        m.setup(_desktop, false, _items_const);
+        m.snapTransformed(_bbox_points, _point, (*bb));
+        m.snapTransformed(_snap_points, _point, (*sn));
+        m.unSetup();
 
         // These lines below are duplicated in stretchRequest
-        if (bb.getSnapped() || sn.getSnapped()) {
-            if (bb.getSnapped()) {
-                if (!bb.isOtherSnapBetter(sn, false)) {
+        //TODO: Eliminate this code duplication
+        if (bb->best_snapped_point.getSnapped() || sn->best_snapped_point.getSnapped()) {
+            if (bb->best_snapped_point.getSnapped()) {
+                if (!bb->best_snapped_point.isOtherSnapBetter(sn->best_snapped_point, false)) {
                     // We snapped the bbox (which is either visual or geometric)
-                    _desktop->snapindicator->set_new_snaptarget(bb);
-                    default_scale = Geom::Scale(bb.getTransformation());
+                    _desktop->snapindicator->set_new_snaptarget(bb->best_snapped_point);
+                    default_scale = bb->getScaleSnapped();
                     // Calculate the new transformation and update the handle position
                     pt = _calcAbsAffineDefault(default_scale);
                 }
-            } else if (sn.getSnapped()) {
-                _desktop->snapindicator->set_new_snaptarget(sn);
+            } else if (sn->best_snapped_point.getSnapped()) {
+                _desktop->snapindicator->set_new_snaptarget(sn->best_snapped_point);
                 // 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());
+                geom_scale = sn->getScaleSnapped();
                 pt = _calcAbsAffineGeom(geom_scale);
             }
         } else {
@@ -946,7 +951,9 @@ gboolean Inkscape::SelTrans::scaleRequest(Geom::Point &pt, guint state)
             _calcAbsAffineDefault(default_scale);
             _desktop->snapindicator->remove_snaptarget();
         }
-        m.unSetup();
+
+        delete bb;
+        delete sn;
     }
 
     /* Status text */
@@ -1003,21 +1010,22 @@ gboolean Inkscape::SelTrans::stretchRequest(SPSelTransHandle const &handle, Geom
         SnapManager &m = _desktop->namedview->snap_manager;
         m.setup(_desktop, false, _items_const);
 
-        Inkscape::SnappedPoint bb, sn;
-        g_assert(bb.getSnapped() == false); // Check initialization to catch any regression
-
         bool symmetrical = state & GDK_CONTROL_MASK;
 
-        bb = m.constrainedSnapStretch(_bbox_points, _point, Geom::Coord(default_scale[axis]), _origin_for_bboxpoints, Geom::Dim2(axis), symmetrical);
-        sn = m.constrainedSnapStretch(_snap_points, _point, Geom::Coord(geom_scale[axis]), _origin_for_specpoints, Geom::Dim2(axis), symmetrical);
+        Inkscape::PureStretchConstrained bb = Inkscape::PureStretchConstrained(Geom::Coord(default_scale[axis]), _origin_for_bboxpoints, Geom::Dim2(axis), symmetrical);
+        Inkscape::PureStretchConstrained sn = Inkscape::PureStretchConstrained(Geom::Coord(geom_scale[axis]), _origin_for_specpoints, Geom::Dim2(axis), symmetrical);
 
-        if (bb.getSnapped()) {
+        m.snapTransformed(_bbox_points, _point, bb);
+        m.snapTransformed(_snap_points, _point, sn);
+        m.unSetup();
+
+        if (bb.best_snapped_point.getSnapped()) {
             // We snapped the bbox (which is either visual or geometric)
-            default_scale[axis] = bb.getTransformation()[axis];
+            default_scale[axis] = bb.getMagnitude();
         }
 
-        if (sn.getSnapped()) {
-            geom_scale[axis] = sn.getTransformation()[axis];
+        if (sn.best_snapped_point.getSnapped()) {
+            geom_scale[axis] = sn.getMagnitude();
         }
 
         if (symmetrical) {
@@ -1028,21 +1036,21 @@ gboolean Inkscape::SelTrans::stretchRequest(SPSelTransHandle const &handle, Geom
         }
 
         // These lines below are duplicated in scaleRequest
-        if (bb.getSnapped() || sn.getSnapped()) {
-            if (bb.getSnapped()) {
-                if (!bb.isOtherSnapBetter(sn, false)) {
+        if (bb.best_snapped_point.getSnapped() || sn.best_snapped_point.getSnapped()) {
+            if (bb.best_snapped_point.getSnapped()) {
+                if (!bb.best_snapped_point.isOtherSnapBetter(sn.best_snapped_point, false)) {
                     // We snapped the bbox (which is either visual or geometric)
-                    _desktop->snapindicator->set_new_snaptarget(bb);
-                    default_scale = Geom::Scale(bb.getTransformation());
+                    _desktop->snapindicator->set_new_snaptarget(bb.best_snapped_point);
+                    default_scale = bb.getStretchSnapped();
                     // Calculate the new transformation and update the handle position
                     pt = _calcAbsAffineDefault(default_scale);
                 }
-            } else if (sn.getSnapped()) {
-                _desktop->snapindicator->set_new_snaptarget(sn);
+            } else if (sn.best_snapped_point.getSnapped()) {
+                _desktop->snapindicator->set_new_snaptarget(sn.best_snapped_point);
                 // 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());
+                geom_scale = sn.getStretchSnapped();
                 pt = _calcAbsAffineGeom(geom_scale);
             }
         } else {
@@ -1050,8 +1058,6 @@ gboolean Inkscape::SelTrans::stretchRequest(SPSelTransHandle const &handle, Geom
             _calcAbsAffineDefault(default_scale);
             _desktop->snapindicator->remove_snaptarget();
         }
-
-        m.unSetup();
     }
 
     // status text
@@ -1156,16 +1162,14 @@ gboolean Inkscape::SelTrans::skewRequest(SPSelTransHandle const &handle, Geom::P
         SnapManager &m = _desktop->namedview->snap_manager;
         m.setup(_desktop, false, _items_const);
 
-        Geom::Point cvec; cvec[dim_b] = 1.;
-        Inkscape::Snapper::SnapConstraint const constraint(cvec);
-        // When skewing, we cannot snap the corners of the bounding box, see the comment in "constrainedSnapSkew" for details
-        Geom::Point const s(skew[dim_a], scale[dim_a]);
-        Inkscape::SnappedPoint sn = m.constrainedSnapSkew(_snap_points, _point, constraint, s, _origin, Geom::Dim2(dim_b));
+        // When skewing, we cannot snap the corners of the bounding box, see the comment in PureSkewConstrained for details
+        Inkscape::PureSkewConstrained sn = Inkscape::PureSkewConstrained(skew[dim_a], scale[dim_a], _origin, Geom::Dim2(dim_b));
+        m.snapTransformed(_snap_points, _point, sn);
 
-        if (sn.getSnapped()) {
+        if (sn.best_snapped_point.getSnapped()) {
             // We snapped something, so change the skew to reflect it
-            skew[dim_a] = sn.getTransformation()[0];
-             _desktop->snapindicator->set_new_snaptarget(sn);
+            skew[dim_a] = sn.getSkewSnapped();
+             _desktop->snapindicator->set_new_snaptarget(sn.best_snapped_point);
         } else {
             _desktop->snapindicator->remove_snaptarget();
         }
@@ -1243,13 +1247,14 @@ gboolean Inkscape::SelTrans::rotateRequest(Geom::Point &pt, guint state)
         SnapManager &m = _desktop->namedview->snap_manager;
         m.setup(_desktop, false, _items_const);
         // When rotating, we cannot snap the corners of the bounding box, see the comment in "constrainedSnapRotate" for details
-        Inkscape::SnappedPoint sn = m.constrainedSnapRotate(_snap_points, _point, radians, _origin);
+        Inkscape::PureRotateConstrained sn = Inkscape::PureRotateConstrained(radians, _origin);
+        m.snapTransformed(_snap_points, _point, sn);
         m.unSetup();
 
-        if (sn.getSnapped()) {
-            _desktop->snapindicator->set_new_snaptarget(sn);
+        if (sn.best_snapped_point.getSnapped()) {
+            _desktop->snapindicator->set_new_snaptarget(sn.best_snapped_point);
             // We snapped something, so change the rotation to reflect it
-            radians = sn.getTransformation()[0];
+            radians = sn.getAngleSnapped();
             r1 = Geom::Rotate(0);
             r2 = Geom::Rotate(radians);
         } else {
@@ -1352,30 +1357,19 @@ void Inkscape::SelTrans::moveTo(Geom::Point const &xy, guint state)
     bool const control = (state & GDK_CONTROL_MASK);
     bool const shift = (state & GDK_SHIFT_MASK);
 
-    if (alt) {
-
-        // Alt pressed means: move only by integer multiples of the grid spacing
-
-        if (control) { // ... if also constrained to the orthogonal axes
-            if (fabs(dxy[Geom::X]) > fabs(dxy[Geom::Y])) {
-                dxy[Geom::Y] = 0;
-            } else {
-                dxy[Geom::X] = 0;
-            }
+    if (control) { // constrained to the orthogonal axes
+        if (fabs(dxy[Geom::X]) > fabs(dxy[Geom::Y])) {
+            dxy[Geom::Y] = 0;
+        } else {
+            dxy[Geom::X] = 0;
         }
+    }
+
+    if (alt) {// Alt pressed means: move only by integer multiples of the grid spacing
         m.setup(_desktop, true, _items_const);
         dxy = m.multipleOfGridPitch(dxy, _point);
         m.unSetup();
-    } else if (shift) {
-        if (control) { // shift & control: constrained movement without snapping
-            if (fabs(dxy[Geom::X]) > fabs(dxy[Geom::Y])) {
-                dxy[Geom::Y] = 0;
-            } else {
-                dxy[Geom::X] = 0;
-            }
-        }
-    } else { //!shift: with snapping
-
+    } else if (!shift) { //!shift: with snapping
         /* We're snapping to things, possibly with a constraint to horizontal or
         ** vertical movement.  Obtain a list of possible translations and then
         ** pick the smallest.
@@ -1386,52 +1380,55 @@ void Inkscape::SelTrans::moveTo(Geom::Point const &xy, guint state)
         /* This will be our list of possible translations */
         std::list<Inkscape::SnappedPoint> s;
 
+        Inkscape::PureTranslate *bb, *sn;
+
         if (control) { // constrained movement with snapping
 
             /* Snap to things, and also constrain to horizontal or vertical movement */
 
-            unsigned int dim = fabs(dxy[Geom::X]) > fabs(dxy[Geom::Y]) ? Geom::X : Geom::Y;
+            Geom::Dim2 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()
-            Geom::Point cvec; cvec[dim] = 1.;
-            s.push_back(m.constrainedSnapTranslate(_bbox_points,
-                                                     _point,
-                                                     Inkscape::Snapper::SnapConstraint(cvec),
-                                                     dxy));
-
-            s.push_back(m.constrainedSnapTranslate(_snap_points,
-                                                     _point,
-                                                     Inkscape::Snapper::SnapConstraint(cvec),
-                                                     dxy));
+            // individually for each point to be snapped; this will be handled however by snapTransformed()
+            bb = new Inkscape::PureTranslateConstrained(dxy[dim], dim);
+            sn = new Inkscape::PureTranslateConstrained(dxy[dim], dim);
         } else { // !control
-
-            // Let's leave this timer code here for a while. I'll probably need it in the near future (Diederik van Lierop)
-            /* GTimeVal starttime;
-            GTimeVal endtime;
-            g_get_current_time(&starttime); */
-
             /* Snap to things with no constraint */
-            s.push_back(m.freeSnapTranslate(_bbox_points, _point, dxy));
-            s.push_back(m.freeSnapTranslate(_snap_points, _point, dxy));
-
-              /*g_get_current_time(&endtime);
-              double elapsed = ((((double)endtime.tv_sec - starttime.tv_sec) * G_USEC_PER_SEC + (endtime.tv_usec - starttime.tv_usec))) / 1000.0;
-              std::cout << "Time spent snapping: " << elapsed << std::endl; */
+            bb = new Inkscape::PureTranslate(dxy);
+            sn = new Inkscape::PureTranslate(dxy);
         }
+        // Let's leave this timer code here for a while. I'll probably need it in the near future (Diederik van Lierop)
+        /* GTimeVal starttime;
+        GTimeVal endtime;
+        g_get_current_time(&starttime); */
+
+        m.snapTransformed(_bbox_points, _point, (*bb));
+        m.snapTransformed(_snap_points, _point, (*sn));
         m.unSetup();
 
+        /*g_get_current_time(&endtime);
+        double elapsed = ((((double)endtime.tv_sec - starttime.tv_sec) * G_USEC_PER_SEC + (endtime.tv_usec - starttime.tv_usec))) / 1000.0;
+        std::cout << "Time spent snapping: " << elapsed << std::endl; */
+
         /* Pick one */
         Inkscape::SnappedPoint best_snapped_point;
-        for (std::list<Inkscape::SnappedPoint>::const_iterator i = s.begin(); i != s.end(); ++i) {
-            if (i->getSnapped()) {
-                if (best_snapped_point.isOtherSnapBetter(*i, true)) {
-                    best_snapped_point = *i;
-                    dxy = i->getTransformation();
-                }
-            }
+
+        bool sn_is_best = sn->best_snapped_point.getSnapped();
+        bool bb_is_best = bb->best_snapped_point.getSnapped();
+
+        if (bb_is_best && sn_is_best) {
+            sn_is_best = bb->best_snapped_point.isOtherSnapBetter(sn->best_snapped_point, true);
+            bb_is_best = !sn_is_best;
+        }
+
+        if (sn_is_best) {
+            best_snapped_point = sn->best_snapped_point;
+            dxy = sn->getTranslationSnapped();
+        } else if (bb_is_best) {
+            best_snapped_point = bb->best_snapped_point;
+            dxy = bb->getTranslationSnapped();
         }
 
         if (best_snapped_point.getSnapped()) {
@@ -1609,7 +1606,7 @@ void Inkscape::SelTrans::_keepClosestPointOnly(Geom::Point const &p)
     }
 
 }
-
+// TODO: This code is duplicated in transform-handle-set.cpp; fix this!
 void Inkscape::SelTrans::getNextClosestPoint(bool reverse)
 {
     Inkscape::Preferences *prefs = Inkscape::Preferences::get();
@@ -1635,6 +1632,12 @@ void Inkscape::SelTrans::getNextClosestPoint(bool reverse)
             } else {
                 _snap_points.push_back(*_all_snap_sources_iter);
             }
+
+            // Show the updated snap source now; otherwise it won't be shown until the selection is being moved again
+            SnapManager &m = _desktop->namedview->snap_manager;
+            m.setup(_desktop);
+            m.displaySnapsource(*_all_snap_sources_iter);
+            m.unSetup();
         }
     }
 }