From: Diederik van Lierop Date: Fri, 29 Oct 2010 22:07:10 +0000 (+0200) Subject: - Constrained snap: proper implementation of the preference to snap the mouse pointer... X-Git-Url: https://git.tokkee.org/?a=commitdiff_plain;h=c13adc1aa8f34445131579baa56299b360480b5e;p=inkscape.git - Constrained snap: proper implementation of the preference to snap the mouse pointer or handle itself (instead of projecting it first onto the constraint) - Fix a crash in SnapManager::multipleConstrainedSnaps --- diff --git a/src/draw-context.cpp b/src/draw-context.cpp index 9bd67c3dd..ca68b3f6d 100644 --- a/src/draw-context.cpp +++ b/src/draw-context.cpp @@ -42,6 +42,7 @@ #include "sp-namedview.h" #include "live_effects/lpe-patternalongpath.h" #include "style.h" +#include "util/mathfns.h" static void sp_draw_context_class_init(SPDrawContextClass *klass); static void sp_draw_context_init(SPDrawContext *dc); @@ -468,7 +469,7 @@ spdc_attach_selection(SPDrawContext *dc, Inkscape::Selection */*sel*/) * \param dc draw context * \param p cursor point (to be changed by snapping) * \param o origin point - * \param state keyboard state to check if ctrl was pressed + * \param state keyboard state to check if ctrl or shift was pressed */ void spdc_endpoint_snap_rotation(SPEventContext const *const ec, Geom::Point &p, Geom::Point const &o, @@ -476,43 +477,41 @@ void spdc_endpoint_snap_rotation(SPEventContext const *const ec, Geom::Point &p, { Inkscape::Preferences *prefs = Inkscape::Preferences::get(); unsigned const snaps = abs(prefs->getInt("/options/rotationsnapsperpi/value", 12)); - /* 0 means no snapping. */ - - /* mirrored by fabs, so this corresponds to 15 degrees */ - Geom::Point best; /* best solution */ - double bn = NR_HUGE; /* best normal */ - double bdot = 0; - Geom::Point v = Geom::Point(0, 1); - double const r00 = cos(M_PI / snaps), r01 = sin(M_PI / snaps); - double const r10 = -r01, r11 = r00; - - Geom::Point delta = p - o; - - for (unsigned i = 0; i < snaps; i++) { - double const ndot = fabs(dot(v,Geom::rot90(delta))); - Geom::Point t(r00*v[Geom::X] + r01*v[Geom::Y], - r10*v[Geom::X] + r11*v[Geom::Y]); - if (ndot < bn) { - /* I think it is better numerically to use the normal, rather than the dot product - * to assess solutions, but I haven't proven it. */ - bn = ndot; - best = v; - bdot = dot(v, delta); + + if (snaps > 0) { // 0 means no snapping + // p is at an arbitrary angle. Now we should snap this angle to specific increments. + // For this we'll calculate the closest two angles, one at each side of the current angle + Geom::Line y_axis(Geom::Point(0, 0), Geom::Point(0, 1)); + Geom::Line p_line(o, p); + double angle = Geom::angle_between(y_axis, p_line); + double angle_incr = M_PI / snaps; + double angle_ceil = round_to_upper_multiple_plus(angle, angle_incr); + double angle_floor = round_to_lower_multiple_plus(angle, angle_incr); + // We have to angles now. The constrained snapper will try each of them and return the closest + // But first we should setup the snapper + + SnapManager &m = SP_EVENT_CONTEXT_DESKTOP(ec)->namedview->snap_manager; + m.setup(SP_EVENT_CONTEXT_DESKTOP(ec)); + bool snap_enabled = m.snapprefs.getSnapEnabledGlobally(); + if (state & GDK_SHIFT_MASK) { + // SHIFT disables all snapping, except the angular snapping. After all, the user explicitly asked for angular + // snapping by pressing CTRL, otherwise we wouldn't have arrived here. But although we temporarily disable + // the snapping here, we must still call for a constrained snap in order to apply the constraints (i.e. round + // to the nearest angle increment) + m.snapprefs.setSnapEnabledGlobally(false); } - v = t; - } - if (fabs(bdot) > 0) { - p = o + bdot * best; + // Now do the snapping... + std::vector constraints; + constraints.push_back(Inkscape::Snapper::SnapConstraint(Geom::Line(o, angle_ceil - M_PI/2))); + constraints.push_back(Inkscape::Snapper::SnapConstraint(Geom::Line(o, angle_floor - M_PI/2))); + + Inkscape::SnappedPoint sp = m.multipleConstrainedSnaps(Inkscape::SnapCandidatePoint(p, Inkscape::SNAPSOURCE_NODE_HANDLE), constraints); + p = sp.getPoint(); - if (!(state & GDK_SHIFT_MASK)) { //SHIFT disables all snapping, except the angular snapping above - //After all, the user explicitly asked for angular snapping by - //pressing CTRL - /* Snap it along best vector */ - SnapManager &m = SP_EVENT_CONTEXT_DESKTOP(ec)->namedview->snap_manager; - m.setup(SP_EVENT_CONTEXT_DESKTOP(ec)); - m.constrainedSnapReturnByRef(p, Inkscape::SNAPSOURCE_NODE_HANDLE, Inkscape::Snapper::SnapConstraint(o, best)); - m.unSetup(); + m.unSetup(); + if (state & GDK_SHIFT_MASK) { + m.snapprefs.setSnapEnabledGlobally(snap_enabled); // restore the original setting } } } diff --git a/src/knot-holder-entity.cpp b/src/knot-holder-entity.cpp index 0a449771e..24cfd8486 100644 --- a/src/knot-holder-entity.cpp +++ b/src/knot-holder-entity.cpp @@ -110,22 +110,10 @@ KnotHolderEntity::snap_knot_position_constrained(Geom::Point const &p, Inkscape: m.setup(desktop, true, item); Inkscape::Preferences *prefs = Inkscape::Preferences::get(); - if ((prefs->getBool("/options/snapmousepointer/value", false))) { // legacy behavior (pre v0.47) - // Snapping the mouse pointer instead of the constrained position of the knot allows to snap to - // things which don't intersect with the constraint line. This should be handled by the - // smart dynamic guides which are yet to be implemented, making this behavior more clean and - // transparent. With the current implementation it leads to unexpected results, and it doesn't - // allow accurately controlling what is being snapped to. - - // freeSnap() will try snapping point p. This will not take into account the constraint, which - // is therefore to be enforced after snap_knot_position_constrained() has finished - m.freeSnapReturnByRef(s, Inkscape::SNAPSOURCE_NODE_HANDLE); - } else { - // constrainedSnap() will first project the point p onto the constraint line and then try to snap along that line. - // This way the constraint is already enforced, no need to worry about that later on - Inkscape::Snapper::SnapConstraint transformed_constraint = Inkscape::Snapper::SnapConstraint(constraint.getPoint() * i2d, (constraint.getPoint() + constraint.getDirection()) * i2d - constraint.getPoint() * i2d); - m.constrainedSnapReturnByRef(s, Inkscape::SNAPSOURCE_NODE_HANDLE, transformed_constraint); - } + // constrainedSnap() will first project the point p onto the constraint line and then try to snap along that line. + // This way the constraint is already enforced, no need to worry about that later on + Inkscape::Snapper::SnapConstraint transformed_constraint = Inkscape::Snapper::SnapConstraint(constraint.getPoint() * i2d, (constraint.getPoint() + constraint.getDirection()) * i2d - constraint.getPoint() * i2d); + m.constrainedSnapReturnByRef(s, Inkscape::SNAPSOURCE_NODE_HANDLE, transformed_constraint); m.unSetup(); return s * i2d.inverse(); diff --git a/src/object-snapper.cpp b/src/object-snapper.cpp index 7e7e25921..51b494498 100644 --- a/src/object-snapper.cpp +++ b/src/object-snapper.cpp @@ -270,7 +270,7 @@ void Inkscape::ObjectSnapper::_snapNodes(SnappedConstraints &sc, { // Iterate through all nodes, find out which one is the closest to p, and snap to it! - _collectNodes(p.getSourceType(), p.getSourceNum() == 0); + _collectNodes(p.getSourceType(), p.getSourceNum() <= 0); if (unselected_nodes != NULL && unselected_nodes->size() > 0) { g_assert(_points_to_snap_to != NULL); @@ -454,7 +454,7 @@ void Inkscape::ObjectSnapper::_snapPaths(SnappedConstraints &sc, std::vector *unselected_nodes, SPPath const *selected_path) const { - _collectPaths(p.getPoint(), p.getSourceType(), p.getSourceNum() == 0); + _collectPaths(p.getPoint(), p.getSourceType(), p.getSourceNum() <= 0); // Now we can finally do the real snapping, using the paths collected above g_assert(_snapmanager->getDesktop() != NULL); @@ -462,7 +462,7 @@ void Inkscape::ObjectSnapper::_snapPaths(SnappedConstraints &sc, bool const node_tool_active = _snapmanager->snapprefs.getSnapToItemPath() && selected_path != NULL; - if (p.getSourceNum() == 0) { + if (p.getSourceNum() <= 0) { /* findCandidates() is used for snapping to both paths and nodes. It ignores the path that is * currently being edited, because that path requires special care: when snapping to nodes * only the unselected nodes of that path should be considered, and these will be passed on separately. @@ -559,7 +559,7 @@ void Inkscape::ObjectSnapper::_snapPathsConstrained(SnappedConstraints &sc, Geom::Point const &p_proj_on_constraint) const { - _collectPaths(p_proj_on_constraint, p.getSourceType(), p.getSourceNum() == 0); + _collectPaths(p_proj_on_constraint, p.getSourceType(), p.getSourceNum() <= 0); // Now we can finally do the real snapping, using the paths collected above @@ -612,6 +612,7 @@ void Inkscape::ObjectSnapper::_snapPathsConstrained(SnappedConstraints &sc, } //Geom::crossings will not consider the closing segment apparently, so we'll handle that separately here + //TODO: This should have been fixed in rev. #9859, which makes this workaround obsolete for(Geom::PathVector::iterator it_pv = k->path_vector->begin(); it_pv != k->path_vector->end(); ++it_pv) { if (it_pv->closed()) { // Get the closing linesegment and convert it to a path @@ -656,9 +657,9 @@ void Inkscape::ObjectSnapper::freeSnap(SnappedConstraints &sc, } /* Get a list of all the SPItems that we will try to snap to */ - if (p.getSourceNum() == 0) { + if (p.getSourceNum() <= 0) { Geom::Rect const local_bbox_to_snap = bbox_to_snap ? *bbox_to_snap : Geom::Rect(p.getPoint(), p.getPoint()); - _findCandidates(sp_document_root(_snapmanager->getDocument()), it, p.getSourceNum() == 0, local_bbox_to_snap, false, Geom::identity()); + _findCandidates(sp_document_root(_snapmanager->getDocument()), it, p.getSourceNum() <= 0, local_bbox_to_snap, false, Geom::identity()); } // TODO: Argh, UGLY! Get rid of this here, move this logic to the snap manager @@ -719,9 +720,9 @@ void Inkscape::ObjectSnapper::constrainedSnap( SnappedConstraints &sc, Geom::Point pp = c.projection(p.getPoint()); /* Get a list of all the SPItems that we will try to snap to */ - if (p.getSourceNum() == 0) { + if (p.getSourceNum() <= 0) { Geom::Rect const local_bbox_to_snap = bbox_to_snap ? *bbox_to_snap : Geom::Rect(pp, pp); - _findCandidates(sp_document_root(_snapmanager->getDocument()), it, p.getSourceNum() == 0, local_bbox_to_snap, false, Geom::identity()); + _findCandidates(sp_document_root(_snapmanager->getDocument()), it, p.getSourceNum() <= 0, local_bbox_to_snap, false, Geom::identity()); } // A constrained snap, is a snap in only one degree of freedom (specified by the constraint line). @@ -852,15 +853,15 @@ void Inkscape::getBBoxPoints(Geom::OptRect const bbox, // collect the corners of the bounding box for ( unsigned k = 0 ; k < 4 ; k++ ) { if (includeCorners) { - points->push_back(Inkscape::SnapCandidatePoint(bbox->corner(k), Inkscape::SNAPSOURCE_BBOX_CORNER, 0, Inkscape::SNAPTARGET_BBOX_CORNER, *bbox)); + points->push_back(Inkscape::SnapCandidatePoint(bbox->corner(k), Inkscape::SNAPSOURCE_BBOX_CORNER, -1, Inkscape::SNAPTARGET_BBOX_CORNER, *bbox)); } // optionally, collect the midpoints of the bounding box's edges too if (includeLineMidpoints) { - points->push_back(Inkscape::SnapCandidatePoint((bbox->corner(k) + bbox->corner((k+1) % 4))/2, Inkscape::SNAPSOURCE_BBOX_EDGE_MIDPOINT, 0, Inkscape::SNAPTARGET_BBOX_EDGE_MIDPOINT, *bbox)); + points->push_back(Inkscape::SnapCandidatePoint((bbox->corner(k) + bbox->corner((k+1) % 4))/2, Inkscape::SNAPSOURCE_BBOX_EDGE_MIDPOINT, -1, Inkscape::SNAPTARGET_BBOX_EDGE_MIDPOINT, *bbox)); } } if (includeObjectMidpoints) { - points->push_back(Inkscape::SnapCandidatePoint(bbox->midpoint(), Inkscape::SNAPSOURCE_BBOX_MIDPOINT, 0, Inkscape::SNAPTARGET_BBOX_MIDPOINT, *bbox)); + points->push_back(Inkscape::SnapCandidatePoint(bbox->midpoint(), Inkscape::SNAPSOURCE_BBOX_MIDPOINT, -1, Inkscape::SNAPTARGET_BBOX_MIDPOINT, *bbox)); } } } diff --git a/src/snap-candidate.h b/src/snap-candidate.h index 5c2834403..772800be5 100644 --- a/src/snap-candidate.h +++ b/src/snap-candidate.h @@ -38,7 +38,7 @@ public: _source_type(source), _target_type(target) { - _source_num = 0; + _source_num = -1; _target_bbox = Geom::OptRect(); } @@ -46,13 +46,14 @@ public: : _point(point), _source_type(source), _target_type(Inkscape::SNAPTARGET_UNDEFINED), - _source_num(0) + _source_num(-1) { _target_bbox = Geom::OptRect(); } inline Geom::Point const & getPoint() const {return _point;} inline Inkscape::SnapSourceType getSourceType() const {return _source_type;} + bool isSingleHandle() const {return (_source_type == SNAPSOURCE_NODE_HANDLE || _source_type == SNAPSOURCE_OTHER_HANDLE) && _source_num == -1;} inline Inkscape::SnapTargetType getTargetType() const {return _target_type;} inline long getSourceNum() const {return _source_num;} void setSourceNum(long num) {_source_num = num;} @@ -68,7 +69,9 @@ private: Inkscape::SnapSourceType _source_type; Inkscape::SnapTargetType _target_type; - //Sequence number of the source point within the set of points that is to be snapped. Starting at zero + //Sequence number of the source point within the set of points that is to be snapped. + // - Starts counting at zero, but only if there might be more points following (e.g. in the selector tool) + // - Minus one (-1) if we're sure that we have only a single point long _source_num; // If this is a target and it belongs to a bounding box, e.g. when the target type is diff --git a/src/snap.cpp b/src/snap.cpp index e14ef6ae9..1f3753600 100644 --- a/src/snap.cpp +++ b/src/snap.cpp @@ -376,13 +376,34 @@ Inkscape::SnappedPoint SnapManager::constrainedSnap(Inkscape::SnapCandidatePoint return no_snap; } + Inkscape::SnappedPoint result = no_snap; + + Inkscape::Preferences *prefs = Inkscape::Preferences::get(); + if ((prefs->getBool("/options/snapmousepointer/value", false)) && p.isSingleHandle()) { + // Snapping the mouse pointer instead of the constrained position of the knot allows + // to snap to things which don't intersect with the constraint line; this is basically + // then just a freesnap with the constraint applied afterwards + // We'll only to this if we're dragging a single handle, and for example not when transforming an object in the selector tool + result = freeSnap(p, bbox_to_snap); + if (result.getSnapped()) { + // only change the snap indicator if we really snapped to something + if (_snapindicator && _desktop) { + _desktop->snapindicator->set_new_snaptarget(result); + } + // Apply the constraint + result.setPoint(constraint.projection(result.getPoint())); + return result; + } + return no_snap; + } + SnappedConstraints sc; SnapperList const snappers = getSnappers(); for (SnapperList::const_iterator i = snappers.begin(); i != snappers.end(); i++) { (*i)->constrainedSnap(sc, p, bbox_to_snap, constraint, &_items_to_ignore, _unselected_nodes); } - Inkscape::SnappedPoint result = findBestSnap(p, sc, true); + result = findBestSnap(p, sc, true); if (result.getSnapped()) { // only change the snap indicator if we really snapped to something @@ -418,38 +439,67 @@ Inkscape::SnappedPoint SnapManager::multipleConstrainedSnaps(Inkscape::SnapCandi std::vector projections; bool snapping_is_futile = !someSnapperMightSnap(); - // Iterate over the constraints + Inkscape::SnappedPoint result = no_snap; + + Inkscape::Preferences *prefs = Inkscape::Preferences::get(); + bool snap_mouse = prefs->getBool("/options/snapmousepointer/value", false); + for (std::vector::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,_unselected_nodes); + } + + if (snap_mouse && p.isSingleHandle()) { + // Snapping the mouse pointer instead of the constrained position of the knot allows + // to snap to things which don't intersect with the constraint line; this is basically + // then just a freesnap with the constraint applied afterwards + // We'll only to this if we're dragging a single handle, and for example not when transforming an object in the selector tool + result = freeSnap(p, bbox_to_snap); + } else { + // Iterate over the constraints + for (std::vector::const_iterator c = constraints.begin(); c != constraints.end(); c++) { + // 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,_unselected_nodes); + } } } + result = findBestSnap(p, sc, true); } - Inkscape::SnappedPoint result = findBestSnap(p, sc, true); - if (result.getSnapped()) { // only change the snap indicator if we really snapped to something if (_snapindicator && _desktop) { _desktop->snapindicator->set_new_snaptarget(result); } + if (snap_mouse) { + // We still have to apply the constraint, because so far we only tried a freeSnap + Geom::Point result_closest; + for (std::vector::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 result_p = (*c).projection(result.getPoint()); + if (c == constraints.begin() || (Geom::L2(result_p - p.getPoint()) < Geom::L2(result_closest - p.getPoint()))) { + result_closest = result_p; + } + } + result.setPoint(result_closest); + } 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::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); } + } else { + no_snap.setPoint(projections.front()); } } diff --git a/src/snapped-curve.cpp b/src/snapped-curve.cpp index 77bc8280c..5deb4c449 100644 --- a/src/snapped-curve.cpp +++ b/src/snapped-curve.cpp @@ -47,7 +47,7 @@ Inkscape::SnappedCurve::SnappedCurve() _at_intersection = false; _fully_constrained = false; _source = SNAPSOURCE_UNDEFINED; - _source_num = 0; + _source_num = -1; _target = SNAPTARGET_UNDEFINED; _target_bbox = Geom::OptRect(); } diff --git a/src/snapped-line.cpp b/src/snapped-line.cpp index da17ff81a..090eadf0a 100644 --- a/src/snapped-line.cpp +++ b/src/snapped-line.cpp @@ -33,7 +33,7 @@ Inkscape::SnappedLineSegment::SnappedLineSegment() _end_point_of_line = Geom::Point(0,0); _point = Geom::Point(0,0); _source = SNAPSOURCE_UNDEFINED; - _source_num = 0; + _source_num = -1; _target = SNAPTARGET_UNDEFINED; _distance = NR_HUGE; _tolerance = 1; @@ -111,7 +111,7 @@ Inkscape::SnappedLine::SnappedLine() _normal_to_line = Geom::Point(0,0); _point_on_line = Geom::Point(0,0); _source = SNAPSOURCE_UNDEFINED; - _source_num = 0; + _source_num = -1; _target = SNAPTARGET_UNDEFINED; _distance = NR_HUGE; _tolerance = 1; diff --git a/src/snapped-point.cpp b/src/snapped-point.cpp index 22daf9103..8f774f793 100644 --- a/src/snapped-point.cpp +++ b/src/snapped-point.cpp @@ -61,7 +61,7 @@ Inkscape::SnappedPoint::SnappedPoint() { _point = Geom::Point(0,0); _source = SNAPSOURCE_UNDEFINED, - _source_num = 0, + _source_num = -1, _target = SNAPTARGET_UNDEFINED, _at_intersection = false; _constrained_snap = false; @@ -81,7 +81,7 @@ Inkscape::SnappedPoint::SnappedPoint(Geom::Point const &p) { _point = p; _source = SNAPSOURCE_UNDEFINED, - _source_num = 0, + _source_num = -1, _target = SNAPTARGET_UNDEFINED, _at_intersection = false; _fully_constrained = false; diff --git a/src/snapped-point.h b/src/snapped-point.h index a28712e85..b2a9fa1ce 100644 --- a/src/snapped-point.h +++ b/src/snapped-point.h @@ -97,7 +97,7 @@ public: protected: Geom::Point _point; // Location of the snapped point SnapSourceType _source; // Describes what snapped - long _source_num; // Sequence number of the source point that snapped, if that point is part of a set of points. (starting at zero) + long _source_num; // Sequence number of the source point that snapped, if that point is part of a set of points. (starting at zero if we might have a set of points; -1 if we only have a single point) SnapTargetType _target; // Describes to what we've snapped to bool _at_intersection; // If true, the snapped point is at an intersection bool _constrained_snap; // If true, then the snapped point was found when looking for a constrained snap diff --git a/src/util/mathfns.h b/src/util/mathfns.h index 20c06145a..e208ca93f 100644 --- a/src/util/mathfns.h +++ b/src/util/mathfns.h @@ -48,7 +48,7 @@ inline double round_to_nearest_multiple_plus(double x, double const c1, double c * If c1==0 (and c0 is finite), then returns +/-inf. This makes grid spacing of zero * mean "ignore the grid in this dimension". */ -inline double round_to_lower_multiple_plus(double x, double const c1, double const c0) +inline double round_to_lower_multiple_plus(double x, double const c1, double const c0 = 0) { return floor((x - c0) / c1) * c1 + c0; } @@ -60,7 +60,7 @@ inline double round_to_lower_multiple_plus(double x, double const c1, double con * If c1==0 (and c0 is finite), then returns +/-inf. This makes grid spacing of zero * mean "ignore the grid in this dimension". */ -inline double round_to_upper_multiple_plus(double x, double const c1, double const c0) +inline double round_to_upper_multiple_plus(double x, double const c1, double const c0 = 0) { return ceil((x - c0) / c1) * c1 + c0; }