From 0eb191f61d9de3829c30d36738a52b7063a43927 Mon Sep 17 00:00:00 2001 From: dvlierop2 Date: Sun, 12 Apr 2009 11:38:21 +0000 Subject: [PATCH] Fix snapping during constrained translation, when only snapping the node closest to the mouse pointer --- src/display/snap-indicator.cpp | 2 +- src/line-snapper.cpp | 2 +- src/seltrans.cpp | 38 ++++++++++++++++------------------ src/snapped-point.cpp | 35 +++++++++++++++++-------------- 4 files changed, 40 insertions(+), 37 deletions(-) diff --git a/src/display/snap-indicator.cpp b/src/display/snap-indicator.cpp index 240fe9545..39222d174 100644 --- a/src/display/snap-indicator.cpp +++ b/src/display/snap-indicator.cpp @@ -42,7 +42,7 @@ SnapIndicator::~SnapIndicator() void SnapIndicator::set_new_snaptarget(Inkscape::SnappedPoint const p) { - remove_snaptarget(); + remove_snaptarget(); //only display one snaptarget at a time g_assert(_desktop != NULL); diff --git a/src/line-snapper.cpp b/src/line-snapper.cpp index a18fb8518..f6a6be74e 100644 --- a/src/line-snapper.cpp +++ b/src/line-snapper.cpp @@ -91,7 +91,7 @@ void Inkscape::LineSnapper::constrainedSnap(SnappedConstraints &sc, Geom::Point t(t_2geom); if (k == Geom::intersects) { - const Geom::Coord dist = L2(t - p); + const Geom::Coord dist = Geom::L2(t - p); if (dist < getSnapperTolerance()) { // When doing a constrained snap, we're already at an intersection. // This snappoint is therefore fully constrained, so there's no need diff --git a/src/seltrans.cpp b/src/seltrans.cpp index 1ee7faff8..816a471cf 100644 --- a/src/seltrans.cpp +++ b/src/seltrans.cpp @@ -1416,25 +1416,23 @@ void Inkscape::SelTrans::moveTo(Geom::Point const &xy, guint state) /* Snap to things, and also constrain to horizontal or vertical movement */ - for (unsigned int dim = 0; dim < 2; dim++) { - // 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() - s.push_back(m.constrainedSnapTranslation(Inkscape::SnapPreferences::SNAPPOINT_BBOX, - _bbox_points, - _point, - Inkscape::Snapper::ConstraintLine(component_vectors[dim]), - dxy)); - - s.push_back(m.constrainedSnapTranslation(Inkscape::SnapPreferences::SNAPPOINT_NODE, - _snap_points, - _point, - Inkscape::Snapper::ConstraintLine(component_vectors[dim]), - dxy)); - } - + unsigned int 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() + s.push_back(m.constrainedSnapTranslation(Inkscape::SnapPreferences::SNAPPOINT_BBOX, + _bbox_points, + _point, + Inkscape::Snapper::ConstraintLine(component_vectors[dim]), + dxy)); + + s.push_back(m.constrainedSnapTranslation(Inkscape::SnapPreferences::SNAPPOINT_NODE, + _snap_points, + _point, + Inkscape::Snapper::ConstraintLine(component_vectors[dim]), + dxy)); } else { // Let's leave this timer code here for a while. I'll probably need it in the near future (Diederik van Lierop) @@ -1443,7 +1441,7 @@ void Inkscape::SelTrans::moveTo(Geom::Point const &xy, guint state) g_get_current_time(&starttime); */ /* Snap to things with no constraint */ - s.push_back(m.freeSnapTranslation(Inkscape::SnapPreferences::SNAPPOINT_BBOX, _bbox_points, _point, dxy)); + s.push_back(m.freeSnapTranslation(Inkscape::SnapPreferences::SNAPPOINT_BBOX, _bbox_points, _point, dxy)); s.push_back(m.freeSnapTranslation(Inkscape::SnapPreferences::SNAPPOINT_NODE, _snap_points, _point, dxy)); /*g_get_current_time(&endtime); diff --git a/src/snapped-point.cpp b/src/snapped-point.cpp index 7e9a16a66..2d028c6d7 100644 --- a/src/snapped-point.cpp +++ b/src/snapped-point.cpp @@ -73,7 +73,7 @@ bool getClosestSP(std::list &list, Inkscape::SnappedPoin bool success = false; for (std::list::const_iterator i = list.begin(); i != list.end(); i++) { - if ((i == list.begin()) || (*i).getSnapDistance() < result.getSnapDistance()) { + if ((i == list.begin()) || (*i).getSnapDistance() < result.getSnapDistance()) { result = *i; success = true; } @@ -92,7 +92,10 @@ bool Inkscape::SnappedPoint::isOtherSnapBetter(Inkscape::SnappedPoint const &oth // there's more than one). It is not useful when trying to find the best snapped target point. // (both the snap distance and the pointer distance are measured in document pixels, not in screen pixels) if (weighted) { - // Weight factor: controls which node should be preferred for snapping, which is either + + Geom::Coord const dist_pointer_other = other_one.getPointerDistance(); + Geom::Coord const dist_pointer_this = getPointerDistance(); + // Weight factor: controls which node should be preferred for snapping, which is either // the node with the closest snap (w = 0), or the node closest to the mousepointer (w = 1) Inkscape::Preferences *prefs = Inkscape::Preferences::get(); double w = prefs->getDoubleLimited("/options/snapweight/value", 0.5, 0, 1); @@ -100,19 +103,21 @@ bool Inkscape::SnappedPoint::isOtherSnapBetter(Inkscape::SnappedPoint const &oth w = 1; } if (w > 0) { - // When accounting for the distance to the mouse pointer, then at least one of the snapped points should - // have that distance set. If not, then this is a bug. Either "weighted" must be set to false, or the - // mouse pointer distance must be set. - g_assert(getPointerDistance() != NR_HUGE || other_one.getPointerDistance() != NR_HUGE); - // The snap distance will always be smaller than the tolerance set for the snapper. The pointer distance can - // however be very large. To compare these in a fair way, we will have to normalize these metrics first - // The closest pointer distance will be normalized to 1.0; the other one will be > 1.0 - // The snap distance will be normalized to 1.0 if it's equal to the snapper tolerance - double const norm_p = std::min(getPointerDistance(), other_one.getPointerDistance()); - double const norm_t_other = std::min(50.0, other_one.getTolerance()); - double const norm_t_this = std::min(50.0, getTolerance()); - dist_other = w * other_one.getPointerDistance() / norm_p + (1-w) * dist_other / norm_t_other; - dist_this = w * getPointerDistance() / norm_p + (1-w) * dist_this / norm_t_this; + if (!(w == 1 && dist_pointer_this == dist_pointer_other)) { + // When accounting for the distance to the mouse pointer, then at least one of the snapped points should + // have that distance set. If not, then this is a bug. Either "weighted" must be set to false, or the + // mouse pointer distance must be set. + g_assert(dist_pointer_this != NR_HUGE || dist_pointer_other != NR_HUGE); + // The snap distance will always be smaller than the tolerance set for the snapper. The pointer distance can + // however be very large. To compare these in a fair way, we will have to normalize these metrics first + // The closest pointer distance will be normalized to 1.0; the other one will be > 1.0 + // The snap distance will be normalized to 1.0 if it's equal to the snapper tolerance + double const norm_p = std::min(dist_pointer_this, dist_pointer_other); + double const norm_t_other = std::min(50.0, other_one.getTolerance()); + double const norm_t_this = std::min(50.0, getTolerance()); + dist_other = w * dist_pointer_other / norm_p + (1-w) * dist_other / norm_t_other; + dist_this = w * dist_pointer_this / norm_p + (1-w) * dist_this / norm_t_this; + } } } -- 2.30.2