From 0279e59bf90766665dada51cfdc3f008dfa7cfc8 Mon Sep 17 00:00:00 2001 From: dvlierop Date: Sun, 21 Jan 2024 18:43:18 +0100 Subject: [PATCH 1/4] 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 6b3532dbff..61c1f5f4db 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 a866f1ab989a31436f4301abec82a9bad7c65cc8 Mon Sep 17 00:00:00 2001 From: dvlierop Date: Sun, 28 Jan 2024 11:21:29 +0100 Subject: [PATCH 2/4] Account for x/y attributes of clone when snapping to path too --- src/object-snapper.cpp | 8 +++++--- src/object/sp-use.cpp | 39 ++++++++++++++++++++------------------- src/object/sp-use.h | 2 ++ src/snap-candidate.h | 4 ++-- 4 files changed, 29 insertions(+), 24 deletions(-) diff --git a/src/object-snapper.cpp b/src/object-snapper.cpp index 61c1f5f4db..2c1e4d51af 100644 --- a/src/object-snapper.cpp +++ b/src/object-snapper.cpp @@ -345,8 +345,9 @@ void Inkscape::ObjectSnapper::_collectPaths(Geom::Point /*p*/, // Snap to the text baseline Text::Layout const *layout = te_get_layout(static_cast(root_item)); if (layout != nullptr && layout->outputExists()) { + Geom::Affine transform = root_item->i2dt_affine() * _candidate.additional_affine * _snapmanager->getDesktop()->doc2dt(); auto pv = Geom::PathVector(); - pv.push_back(layout->baseline() * root_item->i2dt_affine() * _candidate.additional_affine * _snapmanager->getDesktop()->doc2dt()); + pv.push_back(layout->baseline() * transform); _paths_to_snap_to->push_back(SnapCandidatePath(std::move(pv), SNAPTARGET_TEXT_BASELINE, Geom::OptRect())); } } @@ -363,9 +364,10 @@ void Inkscape::ObjectSnapper::_collectPaths(Geom::Point /*p*/, if (!very_complex_path && root_item && _snapmanager->snapprefs.isTargetSnappable(SNAPTARGET_PATH, SNAPTARGET_PATH_INTERSECTION)) { if (auto const shape = cast(root_item)) { if (auto const curve = shape->curve()) { + Geom::Affine transform = (use ? Geom::Affine(use->get_xy_offset()): Geom::Affine()) * root_item->i2dt_affine() * _candidate.additional_affine * _snapmanager->getDesktop()->doc2dt(); // (_edit_transform * _i2d_transform); auto pv = curve->get_pathvector(); - pv *= root_item->i2dt_affine() * _candidate.additional_affine * _snapmanager->getDesktop()->doc2dt(); // (_edit_transform * _i2d_transform); - _paths_to_snap_to->push_back(SnapCandidatePath(std::move(pv), SNAPTARGET_PATH, Geom::OptRect())); // Perhaps for speed, get a reference to the Geom::pathvector, and store the transformation besides it. + pv *= transform; + _paths_to_snap_to->push_back(SnapCandidatePath(std::move(pv), SNAPTARGET_PATH, Geom::OptRect())); // Perhaps for speed, get a reference to the Geom::pathvector, and store the transformation besides it } } } diff --git a/src/object/sp-use.cpp b/src/object/sp-use.cpp index 7f909667c9..dc6be94c03 100644 --- a/src/object/sp-use.cpp +++ b/src/object/sp-use.cpp @@ -239,19 +239,15 @@ std::optional SPUse::documentExactBounds() const } void SPUse::print(SPPrintContext* ctx) { - bool translated = false; - - if ((this->x._set && this->x.computed != 0) || (this->y._set && this->y.computed != 0)) { - Geom::Affine tp(Geom::Translate(this->x.computed, this->y.computed)); - ctx->bind(tp, 1.0); - translated = true; + if (has_xy_offset()) { + ctx->bind(Geom::Translate(this->x.computed, this->y.computed), 1.0); } if (this->child) { this->child->invoke_print(ctx); } - if (translated) { + if (has_xy_offset()) { ctx->release(); } } @@ -467,10 +463,8 @@ Geom::Affine SPUse::get_root_transform() const // right-side) of the transform attribute on the generated 'g', where x and y // represent the values of the x and y attributes on the 'use' element." - http://www.w3.org/TR/SVG11/struct.html#UseElement auto *i_use = cast(i_tem); - if (i_use) { - if ((i_use->x._set && i_use->x.computed != 0) || (i_use->y._set && i_use->y.computed != 0)) { - t = t * Geom::Translate(i_use->x._set ? i_use->x.computed : 0, i_use->y._set ? i_use->y.computed : 0); - } + if (i_use && i_use->has_xy_offset()) { + t = t * i_use->get_xy_offset(); } t *= i_tem->transform; @@ -486,14 +480,22 @@ Geom::Affine SPUse::get_parent_transform() const { Geom::Affine t(Geom::identity()); - if ((this->x._set && this->x.computed != 0) || (this->y._set && this->y.computed != 0)) { - t *= Geom::Translate(this->x._set ? this->x.computed : 0, this->y._set ? this->y.computed : 0); + if (has_xy_offset()) { + t *= get_xy_offset(); } t *= this->transform; return t; } +bool SPUse::has_xy_offset() const { + return (this->x._set && this->x.computed != 0) || (this->y._set && this->y.computed != 0); +} + +Geom::Translate SPUse::get_xy_offset() const { + return Geom::Translate(this->x._set ? this->x.computed : 0, this->y._set ? this->y.computed : 0); +} + /** * Sensing a movement of the original, this function attempts to compensate for it in such a way * that the clone stays unmoved or moves in parallel (depending on user setting) regardless of the @@ -853,13 +855,12 @@ void SPUse::snappoints(std::vector &p, Inkscape::S // 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); + if (has_xy_offset()) { // 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); - } + Geom::Point offset_dt = get_xy_offset().vector() * i2dt_affine().withoutTranslation(); + for (auto& it: vec_pts) { + it.movePoint(offset_dt); + } } // Return the offsetted snap candidate points in vector p diff --git a/src/object/sp-use.h b/src/object/sp-use.h index bc42c988af..d52aec7441 100644 --- a/src/object/sp-use.h +++ b/src/object/sp-use.h @@ -70,6 +70,8 @@ public: SPItem *get_original() const; Geom::Affine get_parent_transform() const; Geom::Affine get_root_transform() const; + bool has_xy_offset() const; + Geom::Translate get_xy_offset() const; SPItem *trueOriginal() const; bool anyInChain(bool (*predicate)(SPItem const *)) const; diff --git a/src/snap-candidate.h b/src/snap-candidate.h index 32461a20f4..db87179f09 100644 --- a/src/snap-candidate.h +++ b/src/snap-candidate.h @@ -86,8 +86,8 @@ public: inline bool considerForAlignment() const {return _alignment;} - inline void setPoint(Geom::Point &pt) {_point = pt;} - inline void movePoint(Geom::Point &pt) {_point += pt;} + inline void setPoint(const Geom::Point &pt) {_point = pt;} + inline void movePoint(const Geom::Point &pt) {_point += pt;} private: // Coordinates of the point -- GitLab From 6640026485d7a235b490511dd7fbe365c181225a Mon Sep 17 00:00:00 2001 From: dvlierop Date: Sun, 28 Jan 2024 17:10:48 +0100 Subject: [PATCH 3/4] Improved readability; added comments --- src/object-snapper.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/object-snapper.cpp b/src/object-snapper.cpp index 2c1e4d51af..4a6fceae71 100644 --- a/src/object-snapper.cpp +++ b/src/object-snapper.cpp @@ -364,7 +364,10 @@ void Inkscape::ObjectSnapper::_collectPaths(Geom::Point /*p*/, if (!very_complex_path && root_item && _snapmanager->snapprefs.isTargetSnappable(SNAPTARGET_PATH, SNAPTARGET_PATH_INTERSECTION)) { if (auto const shape = cast(root_item)) { if (auto const curve = shape->curve()) { - Geom::Affine transform = (use ? Geom::Affine(use->get_xy_offset()): Geom::Affine()) * root_item->i2dt_affine() * _candidate.additional_affine * _snapmanager->getDesktop()->doc2dt(); // (_edit_transform * _i2d_transform); + Geom::Affine transform = use ? use->get_xy_offset(): Geom::Affine(); // If we're dealing with an SPUse, then account for any X/Y offset + transform *= root_item->i2dt_affine(); // Because all snapping calculations are done in desktop coordinates + transform *= _candidate.additional_affine; // Only used for snapping to masks or clips; see SnapManager::_findCandidates() + transform *= _snapmanager->getDesktop()->doc2dt(); // Account for inverted y-axis auto pv = curve->get_pathvector(); pv *= transform; _paths_to_snap_to->push_back(SnapCandidatePath(std::move(pv), SNAPTARGET_PATH, Geom::OptRect())); // Perhaps for speed, get a reference to the Geom::pathvector, and store the transformation besides it -- GitLab From 19f9b17adcc841b851c2799221d5af6f7ad31c06 Mon Sep 17 00:00:00 2001 From: dvlierop Date: Sun, 28 Jan 2024 17:12:32 +0100 Subject: [PATCH 4/4] Remove unused variable --- src/object-snapper.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/object-snapper.cpp b/src/object-snapper.cpp index 4a6fceae71..1777662976 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); -- GitLab