Add a constrained snap method that takes multiple constraints. This reduces the code...

diff --git a/src/snap.cpp b/src/snap.cpp
index 810e2dd8b1632507583e81ef577869a3c61c6238..bcacb81e2328416447589344e395ee48408b52d5 100644 (file)
--- a/src/snap.cpp
+++ b/src/snap.cpp
@@ -389,6 +389,68 @@ Inkscape::SnappedPoint SnapManager::constrainedSnap(Inkscape::SnapCandidatePoint
     return no_snap;
 }
 
+/* See the documentation for constrainedSnap() directly above for more details.
+ * The difference is that multipleConstrainedSnaps() will take a list of constraints instead of a single one,
+ * and will try to snap the SnapCandidatePoint to all of the provided constraints and see which one fits best
+ *  \param p Source point to be snapped
+ *  \param constraints List of directions or lines along which snapping must occur
+ *  \param bbox_to_snap Bounding box hulling the set of points, all from the same selection and having the same transformation
+ */
+
+
+Inkscape::SnappedPoint SnapManager::multipleConstrainedSnaps(Inkscape::SnapCandidatePoint const &p,
+                                                    std::vector<Inkscape::Snapper::SnapConstraint> const &constraints,
+                                                    Geom::OptRect const &bbox_to_snap) const
+{
+
+    Inkscape::SnappedPoint no_snap = Inkscape::SnappedPoint(p.getPoint(), p.getSourceType(), p.getSourceNum(), Inkscape::SNAPTARGET_CONSTRAINT, NR_HUGE, 0, false, true, false);
+    if (constraints.size() == 0) {
+        return no_snap;
+    }
+
+    SnappedConstraints sc;
+    SnapperList const snappers = getSnappers();
+    std::vector<Geom::Point> projections;
+    bool snapping_is_futile = !someSnapperMightSnap();
+
+    // Iterate over the constraints
+    for (std::vector<Inkscape::Snapper::SnapConstraint>::const_iterator c = constraints.begin(); c != constraints.end(); c++) {
+        // Project the mouse pointer onto the constraint; In case we don't snap then we will
+        // return the projection onto the constraint, such that the constraint is always enforced
+        Geom::Point pp = (*c).projection(p.getPoint());
+        projections.push_back(pp);
+        // Try to snap to the constraint
+        if (!snapping_is_futile) {
+            for (SnapperList::const_iterator i = snappers.begin(); i != snappers.end(); i++) {
+                (*i)->constrainedSnap(sc, p, bbox_to_snap, *c, &_items_to_ignore);
+            }
+        }
+    }
+
+    Inkscape::SnappedPoint result = findBestSnap(p, sc, true);
+
+    if (result.getSnapped()) {
+        // only change the snap indicator if we really snapped to something
+        if (_snapindicator) {
+            _desktop->snapindicator->set_new_snaptarget(result);
+        }
+        return result;
+    }
+
+    // So we didn't snap, but we still need to return a point on one of the constraints
+    // Find out which of the constraints yielded the closest projection of point p
+    no_snap.setPoint(projections.front());
+    for (std::vector<Geom::Point>::iterator pp = projections.begin(); pp != projections.end(); pp++) {
+        if (pp != projections.begin()) {
+            if (Geom::L2(*pp - p.getPoint()) < Geom::L2(no_snap.getPoint() - p.getPoint())) {
+                no_snap.setPoint(*pp);
+            }
+        }
+    }
+
+    return no_snap;
+}
+
 /**
  *  \brief Try to snap a point of a guide to another guide or to a node
  *

diff --git a/src/snap.h b/src/snap.h
index f740f3c62b900d93a12f1194c3e31a8308e697e4..c85c51963a9652efd5fdd0408375080e4330d1c9 100644 (file)
--- a/src/snap.h
+++ b/src/snap.h
@@ -129,6 +129,10 @@ public:
                                            Inkscape::Snapper::SnapConstraint const &constraint,
                                            Geom::OptRect const &bbox_to_snap = Geom::OptRect()) const;
 
+    Inkscape::SnappedPoint multipleConstrainedSnaps(Inkscape::SnapCandidatePoint const &p,
+                                                        std::vector<Inkscape::Snapper::SnapConstraint> const &constraints,
+                                                        Geom::OptRect const &bbox_to_snap = Geom::OptRect()) const;
+
     void guideFreeSnap(Geom::Point &p, Geom::Point const &guide_normal, SPGuideDragType drag_type) const;
     void guideConstrainedSnap(Geom::Point &p, SPGuide const &guideline) const;
 

diff --git a/src/snapped-point.h b/src/snapped-point.h
index 05e954e1e4ea6aaeb18a73ad0872f41ccb8936e9..4b4882ee41bae54370ad8a108155e0eef0b83fb1 100644 (file)
--- a/src/snapped-point.h
+++ b/src/snapped-point.h
@@ -54,7 +54,9 @@ public:
      * has occurred; A check should be implemented in the calling code
      * to check for snapping. Use this method only when really needed, e.g.
      * when the calling code is trying to snap multiple points and must
-     * determine itself which point is most appropriate
+     * determine itself which point is most appropriate, or when doing a
+     * constrainedSnap that also enforces projection onto the constraint (in
+     * which case you need the new point anyway, even if we didn't snap)
      */
     Geom::Point getPoint() const {return _point;}
     void setPoint(Geom::Point const &p) {_point = p;}

diff --git a/src/ui/tool/node.cpp b/src/ui/tool/node.cpp
index 69c09602e98a0cd179789bbc161feae102484708..a8582ccc5d551017508f3f4e7cf49b5fd0f022ef 100644 (file)
--- a/src/ui/tool/node.cpp
+++ b/src/ui/tool/node.cpp
@@ -270,9 +270,7 @@ void Handle::dragged(Geom::Point &new_pos, GdkEventMotion *event)
                 _parent->position() - node_away->position());
             Inkscape::SnappedPoint p;
             p = sm.constrainedSnap(Inkscape::SnapCandidatePoint(new_pos, SNAPSOURCE_NODE_HANDLE), cl);
-            if (p.getSnapped()) {
-                p.getPoint(new_pos);
-            }
+            new_pos = p.getPoint();
         } else {
             sm.freeSnapReturnByRef(new_pos, SNAPSOURCE_NODE_HANDLE);
         }
@@ -945,7 +943,7 @@ void Node::dragged(Geom::Point &new_pos, GdkEventMotion *event)
     // For a note on how snapping is implemented in Inkscape, see snap.h.
     SnapManager &sm = _desktop->namedview->snap_manager;
     bool snap = sm.someSnapperMightSnap();
-    std::vector<Inkscape::SnapCandidatePoint> unselected;
+    Inkscape::SnappedPoint sp;
     if (snap) {
         /* setup
          * TODO We are doing this every time a snap happens. It should once be done only once
@@ -955,6 +953,7 @@ void Node::dragged(Geom::Point &new_pos, GdkEventMotion *event)
          * TODO Snapping to unselected segments of selected paths doesn't work yet. */
 
         // Build the list of unselected nodes.
+        std::vector<Inkscape::SnapCandidatePoint> unselected;
         typedef ControlPointSelection::Set Set;
         Set &nodes = _selection.allPoints();
         for (Set::iterator i = nodes.begin(); i != nodes.end(); ++i) {
@@ -964,32 +963,22 @@ void Node::dragged(Geom::Point &new_pos, GdkEventMotion *event)
                 unselected.push_back(p);
             }
         }
-        sm.setupIgnoreSelection(_desktop, false, &unselected);
+        sm.setupIgnoreSelection(_desktop, true, &unselected);
     }
 
     if (held_control(*event)) {
         Geom::Point origin = _last_drag_origin();
-        Inkscape::SnappedPoint fp, bp, fpp, bpp;
+        std::vector<Inkscape::Snapper::SnapConstraint> constraints;
         if (held_alt(*event)) {
             // with Ctrl+Alt, constrain to handle lines
             // project the new position onto a handle line that is closer;
             // also snap to perpendiculars of handle lines
 
-            // TODO: this code is repetitive to the point of sillyness. Find a way
-            // to express this concisely by modifying the semantics of snapping calls.
-            // During a non-snap invocation, we should call constrainedSnap()
-            // anyway, but it should just return the closest point matching the constraint
-            // rather than snapping to an object. There should be comparison
-            // operators defined for snap results, to simplify determining the best one,
-            // or the snapping calls should take a reference to a snapping result and
-            // replace it with the current result if it's better.
-
             Inkscape::Preferences *prefs = Inkscape::Preferences::get();
             int snaps = prefs->getIntLimited("/options/rotationsnapsperpi/value", 12, 1, 1000);
             double min_angle = M_PI / snaps;
 
             boost::optional<Geom::Point> front_point, back_point, fperp_point, bperp_point;
-            boost::optional<Inkscape::Snapper::SnapConstraint> line_front, line_back, line_fperp, line_bperp;
             if (_front.isDegenerate()) {
                 if (_is_line_segment(this, _next()))
                     front_point = _next()->position() - origin;
@@ -1003,11 +992,11 @@ void Node::dragged(Geom::Point &new_pos, GdkEventMotion *event)
                 back_point = _back.relativePos();
             }
             if (front_point) {
-                line_front = Inkscape::Snapper::SnapConstraint(origin, *front_point);
+                constraints.push_back(Inkscape::Snapper::SnapConstraint(origin, *front_point));
                 fperp_point = Geom::rot90(*front_point);
             }
             if (back_point) {
-                line_back = Inkscape::Snapper::SnapConstraint(origin, *back_point);
+                constraints.push_back(Inkscape::Snapper::SnapConstraint(origin, *back_point));
                 bperp_point = Geom::rot90(*back_point);
             }
             // perpendiculars only snap when they are further than snap increment away
@@ -1016,100 +1005,26 @@ void Node::dragged(Geom::Point &new_pos, GdkEventMotion *event)
                 (fabs(Geom::angle_between(*fperp_point, *back_point)) > min_angle &&
                  fabs(Geom::angle_between(*fperp_point, *back_point)) < M_PI - min_angle)))
             {
-                line_fperp = Inkscape::Snapper::SnapConstraint(origin, *fperp_point);
+                constraints.push_back(Inkscape::Snapper::SnapConstraint(origin, *fperp_point));
             }
             if (bperp_point && (!front_point ||
                 (fabs(Geom::angle_between(*bperp_point, *front_point)) > min_angle &&
                  fabs(Geom::angle_between(*bperp_point, *front_point)) < M_PI - min_angle)))
             {
-                line_bperp = Inkscape::Snapper::SnapConstraint(origin, *bperp_point);
+                constraints.push_back(Inkscape::Snapper::SnapConstraint(origin, *bperp_point));
             }
 
-            // TODO: combine the snap and non-snap branches by modifying snap.h / snap.cpp
-            if (snap) {
-                if (line_front) {
-                    fp = sm.constrainedSnap(Inkscape::SnapCandidatePoint(new_pos,
-                        _snapSourceType()), *line_front);
-                }
-                if (line_back) {
-                    bp = sm.constrainedSnap(Inkscape::SnapCandidatePoint(new_pos,
-                        _snapSourceType()), *line_back);
-                }
-                if (line_fperp) {
-                    fpp = sm.constrainedSnap(Inkscape::SnapCandidatePoint(new_pos,
-                        _snapSourceType()), *line_fperp);
-                }
-                if (line_bperp) {
-                    bpp = sm.constrainedSnap(Inkscape::SnapCandidatePoint(new_pos,
-                        _snapSourceType()), *line_bperp);
-                }
-            }
-            if (fp.getSnapped() || bp.getSnapped() || fpp.getSnapped() || bpp.getSnapped()) {
-                if (fp.isOtherSnapBetter(bp, false)) {
-                    fp = bp;
-                }
-                if (fp.isOtherSnapBetter(fpp, false)) {
-                    fp = fpp;
-                }
-                if (fp.isOtherSnapBetter(bpp, false)) {
-                    fp = bpp;
-                }
-                fp.getPoint(new_pos);
-                _desktop->snapindicator->set_new_snaptarget(fp);
-            } else {
-                boost::optional<Geom::Point> pos;
-                if (line_front) {
-                    pos = line_front->projection(new_pos);
-                }
-                if (line_back) {
-                    Geom::Point pos2 = line_back->projection(new_pos);
-                    if (!pos || (pos && Geom::distance(new_pos, *pos) > Geom::distance(new_pos, pos2)))
-                        pos = pos2;
-                }
-                if (line_fperp) {
-                    Geom::Point pos2 = line_fperp->projection(new_pos);
-                    if (!pos || (pos && Geom::distance(new_pos, *pos) > Geom::distance(new_pos, pos2)))
-                        pos = pos2;
-                }
-                if (line_bperp) {
-                    Geom::Point pos2 = line_bperp->projection(new_pos);
-                    if (!pos || (pos && Geom::distance(new_pos, *pos) > Geom::distance(new_pos, pos2)))
-                        pos = pos2;
-                }
-                if (pos) {
-                    new_pos = *pos;
-                } else {
-                    new_pos = origin;
-                }
-            }
+            sp = sm.multipleConstrainedSnaps(Inkscape::SnapCandidatePoint(new_pos, _snapSourceType()), constraints);
         } else {
             // with Ctrl, constrain to axes
-            // TODO combine the two branches
-            if (snap) {
-                Inkscape::Snapper::SnapConstraint line_x(origin, Geom::Point(1, 0));
-                Inkscape::Snapper::SnapConstraint line_y(origin, Geom::Point(0, 1));
-                fp = sm.constrainedSnap(Inkscape::SnapCandidatePoint(new_pos, _snapSourceType()), line_x);
-                bp = sm.constrainedSnap(Inkscape::SnapCandidatePoint(new_pos, _snapSourceType()), line_y);
-            }
-            if (fp.getSnapped() || bp.getSnapped()) {
-                if (fp.isOtherSnapBetter(bp, false)) {
-                    fp = bp;
-                }
-                fp.getPoint(new_pos);
-                _desktop->snapindicator->set_new_snaptarget(fp);
-            } else {
-                Geom::Point origin = _last_drag_origin();
-                Geom::Point delta = new_pos - origin;
-                Geom::Dim2 d = (fabs(delta[Geom::X]) < fabs(delta[Geom::Y])) ? Geom::X : Geom::Y;
-                new_pos[d] = origin[d];
-            }
+            constraints.push_back(Inkscape::Snapper::SnapConstraint(origin, Geom::Point(1, 0)));
+            constraints.push_back(Inkscape::Snapper::SnapConstraint(origin, Geom::Point(0, 1)));
+            sp = sm.multipleConstrainedSnaps(Inkscape::SnapCandidatePoint(new_pos, _snapSourceType()), constraints);
         }
+        new_pos = sp.getPoint();
     } else if (snap) {
-        Inkscape::SnappedPoint p = sm.freeSnap(Inkscape::SnapCandidatePoint(new_pos, _snapSourceType()));
-        if (p.getSnapped()) {
-            p.getPoint(new_pos);
-            _desktop->snapindicator->set_new_snaptarget(p);
-        }
+        sp = sm.freeSnap(Inkscape::SnapCandidatePoint(new_pos, _snapSourceType()));
+        new_pos = sp.getPoint();
     }
 
     SelectableControlPoint::dragged(new_pos, event);