From 43050acdb5d59ae1b379e08d7607ca2b78d38a60 Mon Sep 17 00:00:00 2001 From: dvlierop Date: Sun, 21 Jan 2024 18:43:18 +0100 Subject: [PATCH 1/2] Account for x/y attributes of clones when snapping (fixes issue 2765) --- src/object-snapper.cpp | 7 +------ src/object/sp-use.cpp | 29 +++++++++++++++++++++++------ src/snap-candidate.h | 4 ++++ 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/object-snapper.cpp b/src/object-snapper.cpp index ad2bdd0e12..f4b29a771e 100644 --- a/src/object-snapper.cpp +++ b/src/object-snapper.cpp @@ -123,13 +123,7 @@ void Inkscape::ObjectSnapper::_collectNodes(SnapSourceType const &t, } for (const auto & _candidate : *_snapmanager->_obj_snapper_candidates) { - //Geom::Affine i2doc(Geom::identity()); SPItem *root_item = _candidate.item; - - auto use = cast(_candidate.item); - if (use) { - root_item = use->root(); - } g_return_if_fail(root_item); //Collect all nodes so we can snap to them @@ -176,6 +170,7 @@ void Inkscape::ObjectSnapper::_collectNodes(SnapSourceType const &t, } root_item->getSnappoints(*_points_to_snap_to, &_snapmanager->snapprefs); + SPObject *obj = (SPObject*)root_item; // restore the original snap preferences _snapmanager->snapprefs.setTargetSnappable(SNAPTARGET_PATH_INTERSECTION, old_pref); diff --git a/src/object/sp-use.cpp b/src/object/sp-use.cpp index ef1197e8e5..7f909667c9 100644 --- a/src/object/sp-use.cpp +++ b/src/object/sp-use.cpp @@ -35,6 +35,7 @@ #include "sp-shape.h" #include "sp-symbol.h" #include "sp-text.h" +#include "snap-candidate.h" #include "sp-use-reference.h" #include "sp-use.h" #include "style.h" @@ -832,21 +833,37 @@ SPItem *SPUse::get_original() const { SPItem *ref = nullptr; - if (this->ref){ - ref = this->ref->getObject(); - } + if (this->ref){ + ref = this->ref->getObject(); + } return ref; } void SPUse::snappoints(std::vector &p, Inkscape::SnapPreferences const *snapprefs) const { - SPItem const *root = this->root(); + SPItem const *child = this->child; - if (!root) { + if (!child) { return; } - root->snappoints(p, snapprefs); + // Collect the candidate snap points from the original item (i.e. from the child) + std::vector vec_pts; + child->snappoints(vec_pts, snapprefs); + + // Offset these snap candidate points if the X/Y attributes have been set for this item + // (see https://gitlab.com/inkscape/inkscape/-/issues/2765) + if ((this->x._set && this->x.computed != 0) || (this->y._set && this->y.computed != 0)) { + Geom::Point offset(this->x.computed, this->y.computed); + // All snappoints are in desktop coordinates, but the item's transformation is in document coordinates + Geom::Point offset_dt = offset*i2dt_affine().withoutTranslation(); + for (auto& it: vec_pts) { + it.movePoint(offset_dt); + } + } + + // Return the offsetted snap candidate points in vector p + std::move(vec_pts.begin(), vec_pts.end(), std::back_inserter(p)); } void SPUse::getLinked(std::vector &objects, LinkedObjectNature direction) const diff --git a/src/snap-candidate.h b/src/snap-candidate.h index e7f1e84ee5..32461a20f4 100644 --- a/src/snap-candidate.h +++ b/src/snap-candidate.h @@ -85,6 +85,10 @@ public: inline Geom::OptRect const getTargetBBox() const {return _target_bbox;} inline bool considerForAlignment() const {return _alignment;} + + inline void setPoint(Geom::Point &pt) {_point = pt;} + inline void movePoint(Geom::Point &pt) {_point += pt;} + private: // Coordinates of the point Geom::Point _point; -- GitLab From 34d3792f986c363b5b164b4d99d19a1c9bff7839 Mon Sep 17 00:00:00 2001 From: dvlierop Date: Sun, 21 Jan 2024 22:21:49 +0100 Subject: [PATCH 2/2] Make snapping to paths more reliable (fixes #4399) --- src/object-snapper.cpp | 1 - src/snap.cpp | 24 ++++-------------------- src/snapped-curve.cpp | 6 +++++- src/snapped-curve.h | 2 +- src/snapped-point.cpp | 18 ++++++++++++++++++ src/snapped-point.h | 2 ++ 6 files changed, 30 insertions(+), 23 deletions(-) diff --git a/src/object-snapper.cpp b/src/object-snapper.cpp index f4b29a771e..b947b3e870 100644 --- a/src/object-snapper.cpp +++ b/src/object-snapper.cpp @@ -170,7 +170,6 @@ void Inkscape::ObjectSnapper::_collectNodes(SnapSourceType const &t, } root_item->getSnappoints(*_points_to_snap_to, &_snapmanager->snapprefs); - SPObject *obj = (SPObject*)root_item; // restore the original snap preferences _snapmanager->snapprefs.setTargetSnappable(SNAPTARGET_PATH_INTERSECTION, old_pref); diff --git a/src/snap.cpp b/src/snap.cpp index f9ee29ef4a..4170c351ab 100644 --- a/src/snap.cpp +++ b/src/snap.cpp @@ -549,7 +549,7 @@ Inkscape::SnappedPoint SnapManager::findBestSnap(Inkscape::SnapCandidatePoint co // We might have collected the paths only to snap to their intersection, without the intention to snap to the paths themselves // Therefore we explicitly check whether the paths should be considered as snap targets themselves bool exclude_paths = !snapprefs.isTargetSnappable(Inkscape::SNAPTARGET_PATH); - if (getClosestCurve(isr.curves, closestCurve, exclude_paths)) { + if (getClosestCurve(isr.curves, closestCurve, exclude_paths, to_path_only)) { sp_list.emplace_back(closestCurve); } @@ -623,26 +623,10 @@ Inkscape::SnappedPoint SnapManager::findBestSnap(Inkscape::SnapCandidatePoint co // Filter out all snap targets that do NOT include a path; this is useful when we try to insert // a node in a path (on doubleclick in the node tool). We don't want to change the shape of the - // path, so the snapped point must be on a path, and not e.g. on a grid intersection + // path, so the snapped point must be on a path, and not e.g. on a grid intersection or on the + // bounding box if (to_path_only) { - std::list::iterator i = sp_list.begin(); - - while (i != sp_list.end()) { - Inkscape::SnapTargetType t = (*i).getTarget(); - if (t == Inkscape::SNAPTARGET_LINE_MIDPOINT || - t == Inkscape::SNAPTARGET_PATH || - t == Inkscape::SNAPTARGET_PATH_PERPENDICULAR || - t == Inkscape::SNAPTARGET_PATH_TANGENTIAL || - t == Inkscape::SNAPTARGET_PATH_INTERSECTION || - t == Inkscape::SNAPTARGET_PATH_GUIDE_INTERSECTION || - t == Inkscape::SNAPTARGET_PATH_CLIP || - t == Inkscape::SNAPTARGET_PATH_MASK || - t == Inkscape::SNAPTARGET_ELLIPSE_QUADRANT_POINT) { - ++i; - } else { - i = sp_list.erase(i); - } - } + sp_list.remove_if([](Inkscape::SnappedPoint sp) { return !sp.getOnPath(); }); } // now let's see which snapped point gets a thumbs up diff --git a/src/snapped-curve.cpp b/src/snapped-curve.cpp index 70ad72ee07..d74b101215 100644 --- a/src/snapped-curve.cpp +++ b/src/snapped-curve.cpp @@ -163,7 +163,7 @@ Inkscape::SnappedPoint Inkscape::SnappedCurve::intersect(SnappedLine const &line // search for the closest snapped line -bool getClosestCurve(std::list const &list, Inkscape::SnappedCurve &result, bool exclude_paths) +bool getClosestCurve(std::list const &list, Inkscape::SnappedCurve &result, bool exclude_paths, bool paths_only) { bool success = false; @@ -171,6 +171,10 @@ bool getClosestCurve(std::list const &list, Inkscape::Sn if (exclude_paths && ((*i).getTarget() == Inkscape::SNAPTARGET_PATH)) { continue; } + if (paths_only && (not (*i).getOnPath())) { + // This discards for example bounding box edges, page border, and text baseline + continue; + } if ((i == list.begin()) || (*i).getSnapDistance() < result.getSnapDistance()) { result = *i; success = true; diff --git a/src/snapped-curve.h b/src/snapped-curve.h index bbbfb09d45..b0883f4e58 100644 --- a/src/snapped-curve.h +++ b/src/snapped-curve.h @@ -39,7 +39,7 @@ private: } -bool getClosestCurve(std::list const &list, Inkscape::SnappedCurve &result, bool exclude_paths = false); +bool getClosestCurve(std::list const &list, Inkscape::SnappedCurve &result, bool exclude_paths = false, bool paths_only = false); bool getClosestIntersectionCS(std::list const &list, Geom::Point const &p, Inkscape::SnappedPoint &result, Geom::Affine dt2doc); bool getClosestIntersectionCL(std::list const &list1, std::list const &list2, Geom::Point const &p, Inkscape::SnappedPoint &result, Geom::Affine dt2doc); diff --git a/src/snapped-point.cpp b/src/snapped-point.cpp index 8350ccb7e9..4134d13441 100644 --- a/src/snapped-point.cpp +++ b/src/snapped-point.cpp @@ -431,6 +431,24 @@ bool Inkscape::SnappedPoint::isOtherSnapBetter(Inkscape::SnappedPoint const &oth return other_is_better; } +// Returns true if the snapped point is on a path (or on a line); returns false for e.g. bounding box edges, page border, and text baseline +bool Inkscape::SnappedPoint::getOnPath() const { + std::vector snaptargets_on_path{ + Inkscape::SNAPTARGET_LINE_MIDPOINT, + Inkscape::SNAPTARGET_PATH, + Inkscape::SNAPTARGET_PATH_PERPENDICULAR, + Inkscape::SNAPTARGET_PATH_TANGENTIAL, + Inkscape::SNAPTARGET_PATH_INTERSECTION, + Inkscape::SNAPTARGET_PATH_GUIDE_INTERSECTION, + Inkscape::SNAPTARGET_PATH_CLIP, + Inkscape::SNAPTARGET_PATH_MASK, + Inkscape::SNAPTARGET_ELLIPSE_QUADRANT_POINT + }; + + return std::find(snaptargets_on_path.begin(), snaptargets_on_path.end(), _target) != snaptargets_on_path.end(); +} + + /* Local Variables: mode:c++ diff --git a/src/snapped-point.h b/src/snapped-point.h index aa094971fa..6420bb658c 100644 --- a/src/snapped-point.h +++ b/src/snapped-point.h @@ -111,6 +111,8 @@ public: { return _alignment_target_type.has_value() ? *_alignment_target_type : SNAPTARGET_UNDEFINED; } + bool getOnPath() const; + void setTargetBBox(Geom::OptRect const target) {_target_bbox = target;} Geom::OptRect const getTargetBBox() const {return _target_bbox;} Geom::OptRect const getSourceBBox() const {return _source_bbox;} -- GitLab