From 1397a686e882c774cfef2668f397b2c2c495df66 Mon Sep 17 00:00:00 2001 From: dvlierop2 Date: Wed, 5 Sep 2007 20:16:30 +0000 Subject: [PATCH] Improving the performance of the object snapper --- src/line-snapper.cpp | 10 +- src/line-snapper.h | 8 +- src/object-snapper.cpp | 420 +++++++++++++++++++++++------------------ src/object-snapper.h | 31 ++- src/seltrans.cpp | 3 +- src/snap.cpp | 76 +++++--- src/snap.h | 6 + src/snapper.cpp | 16 +- src/snapper.h | 13 +- 9 files changed, 359 insertions(+), 224 deletions(-) diff --git a/src/line-snapper.cpp b/src/line-snapper.cpp index 4c886fea5..cf46671c8 100644 --- a/src/line-snapper.cpp +++ b/src/line-snapper.cpp @@ -10,12 +10,14 @@ Inkscape::LineSnapper::LineSnapper(SPNamedView const *nv, NR::Coord const d) : S Inkscape::SnappedPoint Inkscape::LineSnapper::_doFreeSnap(Inkscape::Snapper::PointType const &t, NR::Point const &p, + bool const &f, + std::vector &points_to_snap, std::list const &it) const { /* Snap along x (ie to vertical lines) */ - Inkscape::SnappedPoint const v = _doConstrainedSnap(t, p, component_vectors[NR::X], it); + Inkscape::SnappedPoint const v = _doConstrainedSnap(t, p, f, points_to_snap, component_vectors[NR::X], it); /* Snap along y (ie to horizontal lines) */ - Inkscape::SnappedPoint const h = _doConstrainedSnap(t, p, component_vectors[NR::Y], it); + Inkscape::SnappedPoint const h = _doConstrainedSnap(t, p, f, points_to_snap, component_vectors[NR::Y], it); /* If we snapped to both, combine the two results. This is so that, for example, ** we snap nicely to the intersection of two guidelines. @@ -37,7 +39,9 @@ Inkscape::SnappedPoint Inkscape::LineSnapper::_doFreeSnap(Inkscape::Snapper::Poi Inkscape::SnappedPoint Inkscape::LineSnapper::_doConstrainedSnap(Inkscape::Snapper::PointType const &t, NR::Point const &p, - ConstraintLine const &c, + bool const &f, + std::vector &points_to_snap, + ConstraintLine const &c, std::list const &it) const { Inkscape::SnappedPoint s = SnappedPoint(p, NR_HUGE); diff --git a/src/line-snapper.h b/src/line-snapper.h index f6b467520..5d93c858e 100644 --- a/src/line-snapper.h +++ b/src/line-snapper.h @@ -27,11 +27,15 @@ protected: private: SnappedPoint _doFreeSnap(Inkscape::Snapper::PointType const &t, NR::Point const &p, - std::list const &it) const; + bool const &first_point, + std::vector &points_to_snap, + std::list const &it) const; SnappedPoint _doConstrainedSnap(Inkscape::Snapper::PointType const &t, NR::Point const &p, - ConstraintLine const &c, + bool const &first_point, + std::vector &points_to_snap, + ConstraintLine const &c, std::list const &it) const; /** diff --git a/src/object-snapper.cpp b/src/object-snapper.cpp index 785f80960..f253d3538 100644 --- a/src/object-snapper.cpp +++ b/src/object-snapper.cpp @@ -14,7 +14,6 @@ #include "libnr/nr-rect-ops.h" #include "document.h" #include "sp-namedview.h" -#include "sp-path.h" #include "sp-image.h" #include "sp-item-group.h" #include "sp-item.h" @@ -22,7 +21,6 @@ #include "display/curve.h" #include "desktop.h" #include "inkscape.h" -#include "splivarot.h" #include "prefs-utils.h" @@ -31,51 +29,84 @@ Inkscape::ObjectSnapper::ObjectSnapper(SPNamedView const *nv, NR::Coord const d) _snap_to_bboxnode(true), _snap_to_bboxpath(true), _strict_snapping(true), _include_item_center(false) { - + _candidates = new std::list; + _points_to_snap_to = new std::list; + _paths_to_snap_to = new std::list; } +Inkscape::ObjectSnapper::~ObjectSnapper() +{ + _candidates->clear(); //Don't delete the candidates themselves, as these are not ours! + delete _candidates; + + _points_to_snap_to->clear(); + delete _points_to_snap_to; + + for (std::list::const_iterator k = _paths_to_snap_to->begin(); k != _paths_to_snap_to->end(); k++) { + delete *k; + } + _paths_to_snap_to->clear(); + delete _paths_to_snap_to; +} /** - * \param p Point we are trying to snap (desktop coordinates) + * 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 + * \param DimensionToSnap Snap in X, Y, or both directions. */ -void Inkscape::ObjectSnapper::_findCandidates(std::list& c, - SPObject* r, +void Inkscape::ObjectSnapper::_findCandidates(SPObject* r, std::list const &it, - NR::Point const &p, + bool const &first_point, + std::vector &points_to_snap, DimensionToSnap const snap_dim) const { if (ThisSnapperMightSnap()) { SPDesktop const *desktop = SP_ACTIVE_DESKTOP; - for (SPObject* o = sp_object_first_child(r); o != NULL; o = SP_OBJECT_NEXT(o)) { - if (SP_IS_ITEM(o) && !SP_ITEM(o)->isLocked() && !desktop->itemIsHidden(SP_ITEM(o))) { - - /* See if this item is on the ignore list */ - std::list::const_iterator i = it.begin(); - while (i != it.end() && *i != o) { - i++; - } - - if (i == it.end()) { - /* See if the item is within range */ - if (SP_IS_GROUP(o)) { - _findCandidates(c, o, it, p, snap_dim); - } else { - NR::Maybe b = sp_item_bbox_desktop(SP_ITEM(o)); - if (b) { - NR::Point b_min = b->min(); - NR::Point b_max = b->max(); - double d = getDistance(); - bool withinX = (p[NR::X] >= b_min[NR::X] - d) && (p[NR::X] <= b_max[NR::X] + d); - bool withinY = (p[NR::Y] >= b_min[NR::Y] - d) && (p[NR::Y] <= b_max[NR::Y] + d); - if (snap_dim == SNAP_X && withinX || snap_dim == SNAP_Y && withinY || snap_dim == SNAP_XY && withinX && withinY) { - c.push_back(SP_ITEM(o)); + + if (first_point) { + _candidates->clear(); + + for (SPObject* o = sp_object_first_child(r); o != NULL; o = SP_OBJECT_NEXT(o)) { + if (SP_IS_ITEM(o) && !SP_ITEM(o)->isLocked() && !desktop->itemIsHidden(SP_ITEM(o))) { + + /* See if this item is on the ignore list */ + std::list::const_iterator i = it.begin(); + while (i != it.end() && *i != o) { + i++; + } + + if (i == it.end()) { + /* See if the item is within range */ + if (SP_IS_GROUP(o)) { + _findCandidates(o, it, false, points_to_snap, snap_dim); + } else { + // Now let's see if any of the snapping points is within + // snapping range of this object + NR::Maybe b = sp_item_bbox_desktop(SP_ITEM(o)); + if (b) { + for (std::vector::const_iterator i = points_to_snap.begin(); i != points_to_snap.end(); i++) { + NR::Point b_min = b->min(); + NR::Point b_max = b->max(); + double d = getDistance(); + bool withinX = ((*i)[NR::X] >= b_min[NR::X] - d) && ((*i)[NR::X] <= b_max[NR::X] + d); + bool withinY = ((*i)[NR::Y] >= b_min[NR::Y] - d) && ((*i)[NR::Y] <= b_max[NR::Y] + d); + if (snap_dim == SNAP_X && withinX || snap_dim == SNAP_Y && withinY || snap_dim == SNAP_XY && withinX && withinY) { + //We've found a point that is within snapping range + //of this object, so record it as a candidate + _candidates->push_back(SP_ITEM(o)); + break; + } + } } - } - } - } - } - } + } + } + } + } + } } } @@ -83,8 +114,8 @@ void Inkscape::ObjectSnapper::_findCandidates(std::list& c, void Inkscape::ObjectSnapper::_snapNodes(Inkscape::Snapper::PointType const &t, Inkscape::SnappedPoint &s, NR::Point const &p, - DimensionToSnap const snap_dim, - std::list const &cand) const + bool const &first_point, + DimensionToSnap const snap_dim) const { /* FIXME: this seems like a hack. Perhaps Snappers should be ** in SPDesktop rather than SPNamedView? @@ -105,83 +136,87 @@ void Inkscape::ObjectSnapper::_snapNodes(Inkscape::Snapper::PointType const &t, // 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)); - for (std::list::const_iterator i = cand.begin(); i != cand.end(); i++) { - - NR::Matrix i2doc(NR::identity()); - SPItem *root_item = NULL; - if (SP_IS_USE(*i)) { - i2doc = sp_use_get_root_transform(SP_USE(*i)); - root_item = sp_use_root(SP_USE(*i)); - } else { - i2doc = sp_item_i2doc_affine(*i); - root_item = *i; - } - - SPCurve *curve = NULL; - - if (SP_IS_SHAPE(root_item)) { - SPShape const *sh = SP_SHAPE(root_item); - curve = sh->curve; - } else if (SP_IS_IMAGE(root_item)) { - SPImage const *im = SP_IMAGE(root_item); - curve = im->curve; - } - - std::list points_to_snap_to; - - //Collect all nodes so we can snap to them - if (_snap_to_itemnode) { - if (!(_strict_snapping && !p_is_a_node) || p_is_a_guide) { - if (curve) { - int j = 0; - while (SP_CURVE_BPATH(curve)[j].code != NR_END) { - /* Get this node in desktop coordinates */ - NArtBpath const &bp = SP_CURVE_BPATH(curve)[j]; - points_to_snap_to.push_back(desktop->doc2dt(bp.c(3) * i2doc)); - j++; - } - if (_include_item_center) { - points_to_snap_to.push_back(root_item->getCenter()); - } - } - } - } - - //Collect the bounding box's corners so we can snap to them - if (_snap_to_bboxnode) { - if (!(_strict_snapping && !p_is_a_bbox) || p_is_a_guide) { - NR::Maybe b = sp_item_bbox_desktop(root_item, bbox_type); - if (b) { - for ( unsigned k = 0 ; k < 4 ; k++ ) { - points_to_snap_to.push_back(b->corner(k)); + // 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 + if (first_point) { + _points_to_snap_to->clear(); + for (std::list::const_iterator i = _candidates->begin(); i != _candidates->end(); i++) { + + NR::Matrix i2doc(NR::identity()); + SPItem *root_item = NULL; + if (SP_IS_USE(*i)) { + i2doc = sp_use_get_root_transform(SP_USE(*i)); + root_item = sp_use_root(SP_USE(*i)); + } else { + i2doc = sp_item_i2doc_affine(*i); + root_item = *i; + } + + SPCurve *curve = NULL; + + if (SP_IS_SHAPE(root_item)) { + SPShape const *sh = SP_SHAPE(root_item); + curve = sh->curve; + } else if (SP_IS_IMAGE(root_item)) { + SPImage const *im = SP_IMAGE(root_item); + curve = im->curve; + } + + //Collect all nodes so we can snap to them + if (_snap_to_itemnode) { + if (!(_strict_snapping && !p_is_a_node) || p_is_a_guide) { + if (curve) { + int j = 0; + while (SP_CURVE_BPATH(curve)[j].code != NR_END) { + /* Get this node in desktop coordinates */ + NArtBpath const &bp = SP_CURVE_BPATH(curve)[j]; + _points_to_snap_to->push_back(desktop->doc2dt(bp.c(3) * i2doc)); + j++; + } + if (_include_item_center) { + _points_to_snap_to->push_back(root_item->getCenter()); + } } - } - } - } - - //Do the snapping, using all the nodes and corners collected above - for (std::list::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; - NR::Point snapped_point; - switch (snap_dim) { - case SNAP_X: - dist = fabs((*k)[NR::X] - p[NR::X]); - snapped_point = NR::Point((*k)[NR::X], p[NR::Y]); - break; - case SNAP_Y: - dist = fabs((*k)[NR::Y] - p[NR::Y]); - snapped_point = NR::Point(p[NR::X], (*k)[NR::Y]); - break; - case SNAP_XY: - dist = NR::L2(*k - p); - snapped_point = *k; - break; - } - if (dist < getDistance() && dist < s.getDistance()) { - s = SnappedPoint(snapped_point, dist); - } + } + } + + //Collect the bounding box's corners so we can snap to them + if (_snap_to_bboxnode) { + if (!(_strict_snapping && !p_is_a_bbox) || p_is_a_guide) { + NR::Maybe b = sp_item_bbox_desktop(root_item, bbox_type); + if (b) { + for ( unsigned k = 0 ; k < 4 ; k++ ) { + _points_to_snap_to->push_back(b->corner(k)); + } + } + } + } + } + } + + //Do the snapping, using all the nodes and corners collected above + for (std::list::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; + NR::Point snapped_point; + switch (snap_dim) { + case SNAP_X: + dist = fabs((*k)[NR::X] - p[NR::X]); + snapped_point = NR::Point((*k)[NR::X], p[NR::Y]); + break; + case SNAP_Y: + dist = fabs((*k)[NR::Y] - p[NR::Y]); + snapped_point = NR::Point(p[NR::X], (*k)[NR::Y]); + break; + case SNAP_XY: + dist = NR::L2(*k - p); + snapped_point = *k; + break; } + if (dist < getDistance() && dist < s.getDistance()) { + s = SnappedPoint(snapped_point, dist); + } } } @@ -189,7 +224,7 @@ void Inkscape::ObjectSnapper::_snapNodes(Inkscape::Snapper::PointType const &t, void Inkscape::ObjectSnapper::_snapPaths(Inkscape::Snapper::PointType const &t, Inkscape::SnappedPoint &s, NR::Point const &p, - std::list const &cand) const + bool const &first_point) const { /* FIXME: this seems like a hack. Perhaps Snappers should be ** in SPDesktop rather than SPNamedView? @@ -207,91 +242,106 @@ void Inkscape::ObjectSnapper::_snapPaths(Inkscape::Snapper::PointType const &t, bool p_is_a_node = t & Inkscape::Snapper::SNAPPOINT_NODE; - for (std::list::const_iterator i = cand.begin(); i != cand.end(); i++) { - - /* Transform the requested snap point to this item's coordinates */ - NR::Matrix i2doc(NR::identity()); - SPItem *root_item = NULL; - /* We might have a clone at hand, so make sure we get the root item */ - if (SP_IS_USE(*i)) { - i2doc = sp_use_get_root_transform(SP_USE(*i)); - root_item = sp_use_root(SP_USE(*i)); - } else { - i2doc = sp_item_i2doc_affine(*i); - root_item = *i; - } - - //Build a list of all paths considered for snapping to - std::list paths_to_snap_to; - - //Add the item's path to snap to - if (_snap_to_itempath) { - if (!(_strict_snapping && !p_is_a_node)) { - paths_to_snap_to.push_back(Path_for_item(root_item, true, true)); - } - } - - //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 bbox = rect.upgrade(); - SPCurve *curve = sp_curve_new_from_rect(bbox); - NArtBpath *bpath = SP_CURVE_BPATH(curve); - Path *path = bpath_to_Path(bpath); - paths_to_snap_to.push_back(path); - delete curve; - delete bpath; - } - } - - //Now we can finally do the real snapping, using the paths collected above - for (std::list::const_iterator k = paths_to_snap_to.begin(); k != paths_to_snap_to.end(); k++) { - if (*k) { - (*k)->ConvertWithBackData(0.01); - - /* Look for the nearest position on this SPItem to our snap point */ - NR::Maybe const o = get_nearest_position_on_Path(*k, p_doc); - if (o && o->t >= 0 && o->t <= 1) { - - /* Convert the nearest point back to desktop coordinates */ - NR::Point const o_it = get_point_on_Path(*k, o->piece, o->t); - NR::Point const o_dt = desktop->doc2dt(o_it); - - NR::Coord const dist = NR::L2(o_dt - p); - if (dist < getDistance() && dist < s.getDistance()) { - s = SnappedPoint(o_dt, dist); - } - } - - delete *k; + // 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 + if (first_point) { + for (std::list::const_iterator k = _paths_to_snap_to->begin(); k != _paths_to_snap_to->end(); k++) { + delete *k; + } + _paths_to_snap_to->clear(); + for (std::list::const_iterator i = _candidates->begin(); i != _candidates->end(); i++) { + + /* Transform the requested snap point to this item's coordinates */ + NR::Matrix i2doc(NR::identity()); + SPItem *root_item = NULL; + /* We might have a clone at hand, so make sure we get the root item */ + if (SP_IS_USE(*i)) { + i2doc = sp_use_get_root_transform(SP_USE(*i)); + root_item = sp_use_root(SP_USE(*i)); + } else { + i2doc = sp_item_i2doc_affine(*i); + root_item = *i; + } + + //Build a list of all paths considered for snapping to + + //Add the item's path to snap to + if (_snap_to_itempath) { + if (!(_strict_snapping && !p_is_a_node)) { + _paths_to_snap_to->push_back(Path_for_item(root_item, true, true)); + } + } + + //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 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; + } + } } } } + + //Now we can finally do the real snapping, using the paths collected above + for (std::list::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 const o = get_nearest_position_on_Path(*k, p_doc); + if (o && o->t >= 0 && o->t <= 1) { + + /* Convert the nearest point back to desktop coordinates */ + NR::Point const o_it = get_point_on_Path(*k, o->piece, o->t); + NR::Point const o_dt = desktop->doc2dt(o_it); + + NR::Coord const dist = NR::L2(o_dt - p); + if (dist < getDistance() && dist < s.getDistance()) { + s = SnappedPoint(o_dt, dist); + } + } + } + } } Inkscape::SnappedPoint Inkscape::ObjectSnapper::_doFreeSnap(Inkscape::Snapper::PointType const &t, NR::Point const &p, + bool const &first_point, + std::vector &points_to_snap, std::list const &it) const { if ( NULL == _named_view ) { return SnappedPoint(p, NR_HUGE); } - /* Get a list of all the SPItems that we will try to snap to */ - std::list cand; - _findCandidates(cand, sp_document_root(_named_view->document), it, p, SNAP_XY); + /* Get a list of all the SPItems that we will try to snap to */ + _findCandidates(sp_document_root(_named_view->document), it, first_point, points_to_snap, SNAP_XY); - SnappedPoint s(p, NR_HUGE); + SnappedPoint s(p, NR_HUGE); if (_snap_to_itemnode || _snap_to_bboxnode) { - _snapNodes(t, s, p, SNAP_XY, cand); + _snapNodes(t, s, p, first_point, SNAP_XY); } if (_snap_to_itempath || _snap_to_bboxpath) { - _snapPaths(t, s, p, cand); + _snapPaths(t, s, p, first_point); } return s; @@ -301,13 +351,15 @@ Inkscape::SnappedPoint Inkscape::ObjectSnapper::_doFreeSnap(Inkscape::Snapper::P Inkscape::SnappedPoint Inkscape::ObjectSnapper::_doConstrainedSnap(Inkscape::Snapper::PointType const &t, NR::Point const &p, + bool const &first_point, + std::vector &points_to_snap, ConstraintLine const &c, std::list const &it) const { /* FIXME: this needs implementing properly; I think we have to do the ** intersection of c with the objects. */ - return _doFreeSnap(t, p, it); + return _doFreeSnap(t, p, first_point, points_to_snap, it); } @@ -322,10 +374,14 @@ Inkscape::SnappedPoint Inkscape::ObjectSnapper::guideSnap(NR::Point const &p, /* Get a list of all the SPItems that we will try to snap to */ std::list cand; std::list const it; //just an empty list - _findCandidates(cand, sp_document_root(_named_view->document), it, p, snap_dim); + + std::vector points_to_snap; + points_to_snap.push_back(p); + + _findCandidates(sp_document_root(_named_view->document), it, true, points_to_snap, snap_dim); SnappedPoint s(p, NR_HUGE); - _snapNodes(Inkscape::Snapper::SNAPPOINT_GUIDE, s, p, snap_dim, cand); + _snapNodes(Inkscape::Snapper::SNAPPOINT_GUIDE, s, p, true, snap_dim); return s; } diff --git a/src/object-snapper.h b/src/object-snapper.h index ad4cfc649..fa0cfa14c 100644 --- a/src/object-snapper.h +++ b/src/object-snapper.h @@ -14,6 +14,9 @@ */ #include "snapper.h" +#include "sp-path.h" +#include "splivarot.h" + struct SPNamedView; struct SPItem; @@ -27,6 +30,7 @@ class ObjectSnapper : public Snapper public: ObjectSnapper(SPNamedView const *nv, NR::Coord const d); + ~ObjectSnapper(); enum DimensionToSnap {SNAP_X, SNAP_Y, SNAP_XY}; @@ -75,36 +79,45 @@ public: } SnappedPoint guideSnap(NR::Point const &p, - DimensionToSnap const snap_dim) const; + DimensionToSnap const snap_dim) const; bool ThisSnapperMightSnap() const; 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::list *_candidates; + std::list *_points_to_snap_to; + std::list *_paths_to_snap_to; + SnappedPoint _doFreeSnap(Inkscape::Snapper::PointType const &t, NR::Point const &p, + bool const &first_point, + std::vector &points_to_snap, std::list const &it) const; SnappedPoint _doConstrainedSnap(Inkscape::Snapper::PointType const &t, NR::Point const &p, + bool const &first_point, + std::vector &points_to_snap, ConstraintLine const &c, std::list const &it) const; - - void _findCandidates(std::list& c, - SPObject* r, + + void _findCandidates(SPObject* r, std::list const &it, - NR::Point const &p, + bool const &first_point, + std::vector &points_to_snap, DimensionToSnap const snap_dim) const; void _snapNodes(Inkscape::Snapper::PointType const &t, Inkscape::SnappedPoint &s, NR::Point const &p, - DimensionToSnap const snap_dim, - std::list const &cand) const; + bool const &first_point, + DimensionToSnap const snap_dim) const; void _snapPaths(Inkscape::Snapper::PointType const &t, Inkscape::SnappedPoint &s, - NR::Point const &p, - std::list const &cand) const; + NR::Point const &p, + bool const &first_point) const; bool _snap_to_itemnode; bool _snap_to_itempath; diff --git a/src/seltrans.cpp b/src/seltrans.cpp index ad37e24cf..d9566d091 100644 --- a/src/seltrans.cpp +++ b/src/seltrans.cpp @@ -1522,8 +1522,7 @@ void Inkscape::SelTrans::moveTo(NR::Point const &xy, guint state) } else { /* Snap to things with no constraint */ - - s.push_back(m.freeSnapTranslation(Inkscape::Snapper::SNAPPOINT_BBOX, + s.push_back(m.freeSnapTranslation(Inkscape::Snapper::SNAPPOINT_BBOX, _bbox_points, it, dxy)); s.push_back(m.freeSnapTranslation(Inkscape::Snapper::SNAPPOINT_NODE, _snap_points, it, dxy)); diff --git a/src/snap.cpp b/src/snap.cpp index 07e76362d..7a451f5d5 100644 --- a/src/snap.cpp +++ b/src/snap.cpp @@ -173,7 +173,11 @@ Inkscape::SnappedPoint SnapManager::freeSnap(Inkscape::Snapper::PointType t, { std::list lit; lit.push_back(it); - return freeSnap(t, p, lit); + + std::vector points_to_snap; + points_to_snap.push_back(p); + + return freeSnap(t, p, true, points_to_snap, lit); } @@ -182,17 +186,21 @@ Inkscape::SnappedPoint SnapManager::freeSnap(Inkscape::Snapper::PointType t, * * \param t Type of point. * \param p Point. + * \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 * \param it List of items to ignore when snapping. * \return Snapped point. */ - -Inkscape::SnappedPoint SnapManager::freeSnap(Inkscape::Snapper::PointType t, + + Inkscape::SnappedPoint SnapManager::freeSnap(Inkscape::Snapper::PointType t, NR::Point const &p, + bool const &first_point, + std::vector &points_to_snap, std::list const &it) const { SnapperList const snappers = getSnappers(); - return freeSnap(t, p, it, snappers); + return freeSnap(t, p, first_point, points_to_snap, it, snappers); } /** @@ -200,6 +208,8 @@ Inkscape::SnappedPoint SnapManager::freeSnap(Inkscape::Snapper::PointType t, * * \param t Type of point. * \param p Point. + * \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 * \param it List of items to ignore when snapping. * \param snappers List of snappers to try to snap to * \return Snapped point. @@ -207,13 +217,15 @@ Inkscape::SnappedPoint SnapManager::freeSnap(Inkscape::Snapper::PointType t, Inkscape::SnappedPoint SnapManager::freeSnap(Inkscape::Snapper::PointType t, NR::Point const &p, + bool const &first_point, + std::vector &points_to_snap, std::list const &it, SnapperList const &snappers) const { Inkscape::SnappedPoint r(p, NR_HUGE); for (SnapperList::const_iterator i = snappers.begin(); i != snappers.end(); i++) { - Inkscape::SnappedPoint const s = (*i)->freeSnap(t, p, it); + Inkscape::SnappedPoint const s = (*i)->freeSnap(t, p, first_point, points_to_snap, it); if (s.getDistance() < r.getDistance()) { r = s; } @@ -228,7 +240,7 @@ Inkscape::SnappedPoint SnapManager::freeSnap(Inkscape::Snapper::PointType t, * \param t Type of point. * \param p Point. * \param it Item to ignore when snapping. - * \param snappers List of snappers to try to snap to + * \param snappers List of snappers to try to snap to * \return Snapped point. */ @@ -249,7 +261,7 @@ SnapManager::freeSnapAlways( Inkscape::Snapper::PointType t, * \param t Type of point. * \param p Point. * \param it List of items to ignore when snapping. - * \param snappers List of snappers to try to snap to + * \param snappers List of snappers to try to snap to * \return Snapped point. */ @@ -264,7 +276,9 @@ SnapManager::freeSnapAlways( Inkscape::Snapper::PointType t, for (SnapperList::iterator i = snappers.begin(); i != snappers.end(); i++) { gdouble const curr_gridsnap = (*i)->getDistance(); const_cast (*i)->setDistance(NR_HUGE); - Inkscape::SnappedPoint const s = (*i)->freeSnap(t, p, it); + std::vector points_to_snap; + points_to_snap.push_back(p); + Inkscape::SnappedPoint const s = (*i)->freeSnap(t, p, true, points_to_snap, it); const_cast (*i)->setDistance(curr_gridsnap); if (s.getDistance() < r.getDistance()) { @@ -295,7 +309,11 @@ Inkscape::SnappedPoint SnapManager::constrainedSnap(Inkscape::Snapper::PointType { std::list lit; lit.push_back(it); - return constrainedSnap(t, p, c, lit); + + std::vector points_to_snap; + points_to_snap.push_back(p); + + return constrainedSnap(t, p, true, points_to_snap, c, lit); } @@ -306,6 +324,8 @@ Inkscape::SnappedPoint SnapManager::constrainedSnap(Inkscape::Snapper::PointType * * \param t Type of point. * \param p Point. + * \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 * \param c Constraint line. * \param it List of items to ignore when snapping. * \return Snapped point. @@ -313,6 +333,8 @@ Inkscape::SnappedPoint SnapManager::constrainedSnap(Inkscape::Snapper::PointType Inkscape::SnappedPoint SnapManager::constrainedSnap(Inkscape::Snapper::PointType t, NR::Point const &p, + bool const &first_point, + std::vector &points_to_snap, Inkscape::Snapper::ConstraintLine const &c, std::list const &it) const { @@ -320,7 +342,7 @@ Inkscape::SnappedPoint SnapManager::constrainedSnap(Inkscape::Snapper::PointType SnapperList const snappers = getSnappers(); for (SnapperList::const_iterator i = snappers.begin(); i != snappers.end(); i++) { - Inkscape::SnappedPoint const s = (*i)->constrainedSnap(t, p, c, it); + Inkscape::SnappedPoint const s = (*i)->constrainedSnap(t, p, first_point, points_to_snap, c, it); if (s.getDistance() < r.getDistance()) { r = s; } @@ -334,7 +356,7 @@ Inkscape::SnappedPoint SnapManager::guideSnap(NR::Point const &p, { Inkscape::ObjectSnapper::DimensionToSnap snap_dim; if (guide.normal == component_vectors[NR::Y]) { - snap_dim = Inkscape::ObjectSnapper::SNAP_Y; + snap_dim = Inkscape::ObjectSnapper::SNAP_Y; } else if (guide.normal == component_vectors[NR::X]) { snap_dim = Inkscape::ObjectSnapper::SNAP_X; } else { @@ -384,15 +406,9 @@ std::pair SnapManager::_snapTransformed( if (SomeSnapperMightSnap() == false) { return std::make_pair(transformation, false); } - - /* The current best transformation */ - NR::Point best_transformation = transformation; - - /* The current best metric for the best transformation; lower is better, NR_HUGE - ** means that we haven't snapped anything. - */ - double best_metric = NR_HUGE; - + + std::vector transformed_points; + for (std::vector::const_iterator i = points.begin(); i != points.end(); i++) { /* Work out the transformed version of this point */ @@ -423,10 +439,26 @@ std::pair SnapManager::_snapTransformed( default: g_assert_not_reached(); } + + // add the current transformed point to the box hulling all transformed points + transformed_points.push_back(transformed); + } + + /* The current best transformation */ + NR::Point best_transformation = transformation; + /* The current best metric for the best transformation; lower is better, NR_HUGE + ** means that we haven't snapped anything. + */ + double best_metric = NR_HUGE; + + std::vector::const_iterator j = transformed_points.begin(); + + for (std::vector::const_iterator i = points.begin(); i != points.end(); i++) { + /* Snap it */ Inkscape::SnappedPoint const snapped = constrained ? - constrainedSnap(type, transformed, constraint, ignore) : freeSnap(type, transformed, ignore); + constrainedSnap(type, *j, i == points.begin(), transformed_points, constraint, ignore) : freeSnap(type, *j, i == points.begin(), transformed_points, ignore); if (snapped.getDistance() < NR_HUGE) { /* We snapped. Find the transformation that describes where the snapped point has @@ -473,6 +505,8 @@ std::pair SnapManager::_snapTransformed( best_metric = metric; } } + + j++; } // Using " < 1e6" instead of " < NR::HUGE" for catching some rounding errors diff --git a/src/snap.h b/src/snap.h index 276069fe6..cdd707d01 100644 --- a/src/snap.h +++ b/src/snap.h @@ -50,10 +50,14 @@ public: Inkscape::SnappedPoint freeSnap(Inkscape::Snapper::PointType t, NR::Point const &p, + bool const &first_point, + std::vector &points_to_snap, std::list const &it) const; Inkscape::SnappedPoint freeSnap( Inkscape::Snapper::PointType t, NR::Point const &p, + bool const &first_point, + std::vector &points_to_snap, std::list const &it, SnapperList const &snappers ) const; @@ -74,6 +78,8 @@ public: Inkscape::SnappedPoint constrainedSnap(Inkscape::Snapper::PointType t, NR::Point const &p, + bool const &first_point, + std::vector &points_to_snap, Inkscape::Snapper::ConstraintLine const &c, std::list const &it) const; diff --git a/src/snapper.cpp b/src/snapper.cpp index 5d39c6b02..17d7b7137 100644 --- a/src/snapper.cpp +++ b/src/snapper.cpp @@ -93,11 +93,13 @@ void Inkscape::Snapper::setEnabled(bool s) Inkscape::SnappedPoint Inkscape::Snapper::freeSnap(PointType const &t, NR::Point const &p, + bool const &first_point, + std::vector &points_to_snap, SPItem const *it) const { std::list lit; lit.push_back(it); - return freeSnap(t, p, lit); + return freeSnap(t, p, first_point, points_to_snap, lit); } @@ -114,13 +116,15 @@ Inkscape::SnappedPoint Inkscape::Snapper::freeSnap(PointType const &t, Inkscape::SnappedPoint Inkscape::Snapper::freeSnap(PointType const &t, NR::Point const &p, + bool const &first_point, + std::vector &points_to_snap, std::list const &it) const { if (_enabled == false || getSnapFrom(t) == false) { return SnappedPoint(p, NR_HUGE); } - return _doFreeSnap(t, p, it); + return _doFreeSnap(t, p, first_point, points_to_snap, it); } @@ -139,12 +143,14 @@ Inkscape::SnappedPoint Inkscape::Snapper::freeSnap(PointType const &t, Inkscape::SnappedPoint Inkscape::Snapper::constrainedSnap(PointType const &t, NR::Point const &p, + bool const &first_point, + std::vector &points_to_snap, ConstraintLine const &c, SPItem const *it) const { std::list lit; lit.push_back(it); - return constrainedSnap(t, p, c, lit); + return constrainedSnap(t, p, first_point, points_to_snap, c, lit); } @@ -161,6 +167,8 @@ Inkscape::SnappedPoint Inkscape::Snapper::constrainedSnap(PointType const &t, Inkscape::SnappedPoint Inkscape::Snapper::constrainedSnap(PointType const &t, NR::Point const &p, + bool const &first_point, + std::vector &points_to_snap, ConstraintLine const &c, std::list const &it) const { @@ -168,7 +176,7 @@ Inkscape::SnappedPoint Inkscape::Snapper::constrainedSnap(PointType const &t, return SnappedPoint(p, NR_HUGE); } - return _doConstrainedSnap(t, p, c, it); + return _doConstrainedSnap(t, p, first_point, points_to_snap, c, it); } /* diff --git a/src/snapper.h b/src/snapper.h index 085eb156e..5b6b4a258 100644 --- a/src/snapper.h +++ b/src/snapper.h @@ -48,15 +48,18 @@ public: */ virtual bool ThisSnapperMightSnap() const {return (_enabled && _snap_from != 0);} // will likely be overridden by derived classes - void setEnabled(bool s); SnappedPoint freeSnap(PointType const &t, NR::Point const &p, + bool const &first_point, + std::vector &points_to_snap, SPItem const *it) const; SnappedPoint freeSnap(PointType const &t, NR::Point const &p, + bool const &first_point, + std::vector &points_to_snap, std::list const &it) const; class ConstraintLine @@ -86,11 +89,15 @@ public: SnappedPoint constrainedSnap(PointType const &t, NR::Point const &p, + bool const &first_point, + std::vector &points_to_snap, ConstraintLine const &c, SPItem const *it) const; SnappedPoint constrainedSnap(PointType const &t, NR::Point const &p, + bool const &first_point, + std::vector &points_to_snap, ConstraintLine const &c, std::list const &it) const; protected: @@ -111,6 +118,8 @@ private: */ virtual SnappedPoint _doFreeSnap(PointType const &t, NR::Point const &p, + bool const &first_point, + std::vector &points_to_snap, std::list const &it) const = 0; /** @@ -125,6 +134,8 @@ private: */ virtual SnappedPoint _doConstrainedSnap(PointType const &t, NR::Point const &p, + bool const &first_point, + std::vector &points_to_snap, ConstraintLine const &c, std::list const &it) const = 0; -- 2.30.2