From 8f373840d0a408d43d2a6712c638463637890e97 Mon Sep 17 00:00:00 2001 From: Diederik van Lierop Date: Sat, 3 May 2014 23:37:36 +0200 Subject: Fix bounding box cache issues in general, and prevent the selector tool from updating anything in case of identity affines (which prevents the bounding box from being invalidated) Fixed bugs: - https://launchpad.net/bugs/1256597 (bzr r13333) --- src/seltrans.cpp | 71 ++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 43 insertions(+), 28 deletions(-) (limited to 'src/seltrans.cpp') diff --git a/src/seltrans.cpp b/src/seltrans.cpp index 60a0bfa11..7708c999e 100644 --- a/src/seltrans.cpp +++ b/src/seltrans.cpp @@ -377,6 +377,12 @@ void Inkscape::SelTrans::transform(Geom::Affine const &rel_affine, Geom::Point c g_return_if_fail(_grabbed); g_return_if_fail(!_empty); + // E.g. scaling a perfectly vertical line in horizontal direction will not work, and will produce an identity affine + + if (rel_affine.isIdentity()) { + return; + } + Geom::Affine const affine( Geom::Translate(-norm) * rel_affine * Geom::Translate(norm) ); if (_show == SHOW_CONTENT) { @@ -389,6 +395,7 @@ void Inkscape::SelTrans::transform(Geom::Affine const &rel_affine, Geom::Point c } Geom::Affine const &prev_transform = _items_affines[i]; item.set_i2d_affine(prev_transform * affine); + // The new affine will only have been applied if the transformation is different from the previous one, see SPItem::set_item_transform } } else { if (_bbox) { @@ -439,21 +446,25 @@ void Inkscape::SelTrans::ungrab() _message_context.clear(); if (!_empty && _changed) { - sp_selection_apply_affine(selection, _current_relative_affine, (_show == SHOW_OUTLINE)? true : false); - if (_center) { - *_center *= _current_relative_affine; - _center_is_set = true; - } + if (!_current_relative_affine.isIdentity()) { // we can have a identity affine + // when trying to stretch a perfectly vertical line in horizontal direction, which will not be allowed by the handles; -// If dragging showed content live, sp_selection_apply_affine cannot change the centers -// appropriately - it does not know the original positions of the centers (all objects already have -// the new bboxes). So we need to reset the centers from our saved array. - if (_show != SHOW_OUTLINE && !_current_relative_affine.isTranslation()) { - for (unsigned i = 0; i < _items_centers.size(); i++) { - SPItem *currentItem = _items[i]; - if (currentItem->isCenterSet()) { // only if it's already set - currentItem->setCenter (_items_centers[i] * _current_relative_affine); - currentItem->updateRepr(); + sp_selection_apply_affine(selection, _current_relative_affine, (_show == SHOW_OUTLINE)? true : false); + if (_center) { + *_center *= _current_relative_affine; + _center_is_set = true; + } + + // If dragging showed content live, sp_selection_apply_affine cannot change the centers + // appropriately - it does not know the original positions of the centers (all objects already have + // the new bboxes). So we need to reset the centers from our saved array. + if (_show != SHOW_OUTLINE && !_current_relative_affine.isTranslation()) { + for (unsigned i = 0; i < _items_centers.size(); i++) { + SPItem *currentItem = _items[i]; + if (currentItem->isCenterSet()) { // only if it's already set + currentItem->setCenter (_items_centers[i] * _current_relative_affine); + currentItem->updateRepr(); + } } } } @@ -463,18 +474,22 @@ void Inkscape::SelTrans::ungrab() _items_affines.clear(); _items_centers.clear(); - if (_current_relative_affine.isTranslation()) { - DocumentUndo::done(sp_desktop_document(_desktop), SP_VERB_CONTEXT_SELECT, - _("Move")); - } else if (_current_relative_affine.withoutTranslation().isScale()) { - DocumentUndo::done(sp_desktop_document(_desktop), SP_VERB_CONTEXT_SELECT, - _("Scale")); - } else if (_current_relative_affine.withoutTranslation().isRotation()) { - DocumentUndo::done(sp_desktop_document(_desktop), SP_VERB_CONTEXT_SELECT, - _("Rotate")); - } else { - DocumentUndo::done(sp_desktop_document(_desktop), SP_VERB_CONTEXT_SELECT, - _("Skew")); + if (!_current_relative_affine.isIdentity()) { // we can have a identity affine + // when trying to stretch a perfectly vertical line in horizontal direction, which will not be allowed + // by the handles; this would be identified as a (zero) translation by isTranslation() + if (_current_relative_affine.isTranslation()) { + DocumentUndo::done(sp_desktop_document(_desktop), SP_VERB_CONTEXT_SELECT, + _("Move")); + } else if (_current_relative_affine.withoutTranslation().isScale()) { + DocumentUndo::done(sp_desktop_document(_desktop), SP_VERB_CONTEXT_SELECT, + _("Scale")); + } else if (_current_relative_affine.withoutTranslation().isRotation()) { + DocumentUndo::done(sp_desktop_document(_desktop), SP_VERB_CONTEXT_SELECT, + _("Rotate")); + } else { + DocumentUndo::done(sp_desktop_document(_desktop), SP_VERB_CONTEXT_SELECT, + _("Skew")); + } } } else { @@ -506,7 +521,7 @@ void Inkscape::SelTrans::stamp() bool fixup = !_grabbed; if ( fixup && _stamp_cache ) { - // TODO - give a proper fix. Simple temproary work-around for the grab() issue + // TODO - give a proper fix. Simple temporary work-around for the grab() issue g_slist_free(_stamp_cache); _stamp_cache = NULL; } @@ -565,7 +580,7 @@ void Inkscape::SelTrans::stamp() } if ( fixup && _stamp_cache ) { - // TODO - give a proper fix. Simple temproary work-around for the grab() issue + // TODO - give a proper fix. Simple temporary work-around for the grab() issue g_slist_free(_stamp_cache); _stamp_cache = NULL; } -- cgit v1.2.3 From 5a16d98495719cf201a03c36246c75072c97b596 Mon Sep 17 00:00:00 2001 From: Diederik van Lierop Date: Mon, 12 May 2014 21:34:58 +0200 Subject: Fix snapping issue in selector tool Fixed bugs: - https://launchpad.net/bugs/1255764 (bzr r13361) --- src/seltrans.cpp | 64 ++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 39 insertions(+), 25 deletions(-) (limited to 'src/seltrans.cpp') diff --git a/src/seltrans.cpp b/src/seltrans.cpp index 7708c999e..fa2441847 100644 --- a/src/seltrans.cpp +++ b/src/seltrans.cpp @@ -929,23 +929,29 @@ gboolean Inkscape::SelTrans::scaleRequest(Geom::Point &pt, guint state) sn = m.freeSnapScale(_snap_points, _point, geom_scale, _origin_for_specpoints); } - if (!(bb.getSnapped() || sn.getSnapped())) { + std::cout << bb.getSnapped() << " | " << sn.getSnapped() << std::endl; + // These lines below are duplicated in stretchRequest + if (bb.getSnapped() || sn.getSnapped()) { + if (bb.getSnapped()) { + if (!bb.isOtherSnapBetter(sn, false)) { + // We snapped the bbox (which is either visual or geometric) + _desktop->snapindicator->set_new_snaptarget(bb); + default_scale = Geom::Scale(bb.getTransformation()); + // Calculate the new transformation and update the handle position + pt = _calcAbsAffineDefault(default_scale); + } + } else if (sn.getSnapped()) { + _desktop->snapindicator->set_new_snaptarget(sn); + // We snapped the special points (e.g. nodes), which are not at the visual bbox + // The handle location however (pt) might however be at the visual bbox, so we + // will have to calculate pt taking the stroke width into account + geom_scale = Geom::Scale(sn.getTransformation()); + pt = _calcAbsAffineGeom(geom_scale); + } + } else { // We didn't snap at all! Don't update the handle position, just calculate the new transformation _calcAbsAffineDefault(default_scale); _desktop->snapindicator->remove_snaptarget(); - } else if (bb.getSnapped() && !bb.isOtherSnapBetter(sn, false)) { - // We snapped the bbox (which is either visual or geometric) - _desktop->snapindicator->set_new_snaptarget(bb); - default_scale = Geom::Scale(bb.getTransformation()); - // Calculate the new transformation and update the handle position - pt = _calcAbsAffineDefault(default_scale); - } else { - _desktop->snapindicator->set_new_snaptarget(sn); - // We snapped the special points (e.g. nodes), which are not at the visual bbox - // The handle location however (pt) might however be at the visual bbox, so we - // will have to calculate pt taking the stroke width into account - geom_scale = Geom::Scale(sn.getTransformation()); - pt = _calcAbsAffineGeom(geom_scale); } m.unSetup(); } @@ -1028,20 +1034,28 @@ gboolean Inkscape::SelTrans::stretchRequest(SPSelTransHandle const &handle, Geom geom_scale[perp] = fabs(geom_scale[axis]); } - if (!(bb.getSnapped() || sn.getSnapped())) { + // These lines below are duplicated in scaleRequest + if (bb.getSnapped() || sn.getSnapped()) { + if (bb.getSnapped()) { + if (!bb.isOtherSnapBetter(sn, false)) { + // We snapped the bbox (which is either visual or geometric) + _desktop->snapindicator->set_new_snaptarget(bb); + default_scale = Geom::Scale(bb.getTransformation()); + // Calculate the new transformation and update the handle position + pt = _calcAbsAffineDefault(default_scale); + } + } else if (sn.getSnapped()) { + _desktop->snapindicator->set_new_snaptarget(sn); + // We snapped the special points (e.g. nodes), which are not at the visual bbox + // The handle location however (pt) might however be at the visual bbox, so we + // will have to calculate pt taking the stroke width into account + geom_scale = Geom::Scale(sn.getTransformation()); + pt = _calcAbsAffineGeom(geom_scale); + } + } else { // We didn't snap at all! Don't update the handle position, just calculate the new transformation _calcAbsAffineDefault(default_scale); _desktop->snapindicator->remove_snaptarget(); - } else if (bb.getSnapped() && !bb.isOtherSnapBetter(sn, false)) { - _desktop->snapindicator->set_new_snaptarget(bb); - // Calculate the new transformation and update the handle position - pt = _calcAbsAffineDefault(default_scale); - } else { - _desktop->snapindicator->set_new_snaptarget(sn); - // We snapped the special points (e.g. nodes), which are not at the visual bbox - // The handle location however (pt) might however be at the visual bbox, so we - // will have to calculate pt taking the stroke width into account - pt = _calcAbsAffineGeom(geom_scale); } m.unSetup(); -- cgit v1.2.3 From bdc1f59ae6a11600ba11e848b4e5a76bf5cc92a5 Mon Sep 17 00:00:00 2001 From: Diederik van Lierop Date: Thu, 15 May 2014 22:12:29 +0200 Subject: Remove debugging output (bzr r13379) --- src/seltrans.cpp | 1 - 1 file changed, 1 deletion(-) (limited to 'src/seltrans.cpp') diff --git a/src/seltrans.cpp b/src/seltrans.cpp index fa2441847..d6f31f073 100644 --- a/src/seltrans.cpp +++ b/src/seltrans.cpp @@ -929,7 +929,6 @@ gboolean Inkscape::SelTrans::scaleRequest(Geom::Point &pt, guint state) sn = m.freeSnapScale(_snap_points, _point, geom_scale, _origin_for_specpoints); } - std::cout << bb.getSnapped() << " | " << sn.getSnapped() << std::endl; // These lines below are duplicated in stretchRequest if (bb.getSnapped() || sn.getSnapped()) { if (bb.getSnapped()) { -- cgit v1.2.3 From c5a3e612c363f761171123cf62f65c85808c296e Mon Sep 17 00:00:00 2001 From: Diederik van Lierop Date: Sat, 5 Jul 2014 15:23:33 +0200 Subject: Fix regression that prevented snapping back to original location, caused by rev. 13333 Fixed bugs: - https://launchpad.net/bugs/1337170 (bzr r13446) --- src/seltrans.cpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'src/seltrans.cpp') diff --git a/src/seltrans.cpp b/src/seltrans.cpp index d6f31f073..d16e02e42 100644 --- a/src/seltrans.cpp +++ b/src/seltrans.cpp @@ -377,12 +377,6 @@ void Inkscape::SelTrans::transform(Geom::Affine const &rel_affine, Geom::Point c g_return_if_fail(_grabbed); g_return_if_fail(!_empty); - // E.g. scaling a perfectly vertical line in horizontal direction will not work, and will produce an identity affine - - if (rel_affine.isIdentity()) { - return; - } - Geom::Affine const affine( Geom::Translate(-norm) * rel_affine * Geom::Translate(norm) ); if (_show == SHOW_CONTENT) { @@ -1327,6 +1321,16 @@ void Inkscape::SelTrans::stretch(SPSelTransHandle const &/*handle*/, Geom::Point void Inkscape::SelTrans::scale(Geom::Point &/*pt*/, guint /*state*/) { + // E.g. scaling a perfectly vertical line in horizontal direction will not work, and will produce an identity affine + // Applying a transformation is useless, so we will not attempt to do so because this might trigger other bugs + // (see https://bugs.launchpad.net/inkscape/+bug/1256597) + // We check for this here and not in transform because identity transformations are perfectly fine for for example + // translations (e.g. a translation of (0,0), which occurs when snapping a point back to its original location) + + if (_absolute_affine.isIdentity()) { + return; + } + transform(_absolute_affine, Geom::Point(0, 0)); // we have already accounted for origin, so pass 0,0 } -- cgit v1.2.3 From ac8283706beb41435d25ea5d880fe44d68679803 Mon Sep 17 00:00:00 2001 From: Diederik van Lierop Date: Fri, 11 Jul 2014 20:28:15 +0200 Subject: Fix regression introduced by rev 13446 / 13333 Fixed bugs: - https://launchpad.net/bugs/1256597 - https://launchpad.net/bugs/1340011 (bzr r13450) --- src/seltrans.cpp | 10 ---------- 1 file changed, 10 deletions(-) (limited to 'src/seltrans.cpp') diff --git a/src/seltrans.cpp b/src/seltrans.cpp index d16e02e42..4b1a3a5fa 100644 --- a/src/seltrans.cpp +++ b/src/seltrans.cpp @@ -1321,16 +1321,6 @@ void Inkscape::SelTrans::stretch(SPSelTransHandle const &/*handle*/, Geom::Point void Inkscape::SelTrans::scale(Geom::Point &/*pt*/, guint /*state*/) { - // E.g. scaling a perfectly vertical line in horizontal direction will not work, and will produce an identity affine - // Applying a transformation is useless, so we will not attempt to do so because this might trigger other bugs - // (see https://bugs.launchpad.net/inkscape/+bug/1256597) - // We check for this here and not in transform because identity transformations are perfectly fine for for example - // translations (e.g. a translation of (0,0), which occurs when snapping a point back to its original location) - - if (_absolute_affine.isIdentity()) { - return; - } - transform(_absolute_affine, Geom::Point(0, 0)); // we have already accounted for origin, so pass 0,0 } -- cgit v1.2.3 From 64fb7253a1d2771c4f844d1c2266a90f328895b5 Mon Sep 17 00:00:00 2001 From: Diederik van Lierop Date: Wed, 30 Jul 2014 22:47:29 +0200 Subject: Limit the number of paths to be used as snap targets, to keep Inkscape responsive in very complex drawings Fixed bugs: - https://launchpad.net/bugs/1348959 (bzr r13483) --- src/seltrans.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'src/seltrans.cpp') diff --git a/src/seltrans.cpp b/src/seltrans.cpp index 4b1a3a5fa..6b8cd19bb 100644 --- a/src/seltrans.cpp +++ b/src/seltrans.cpp @@ -304,6 +304,7 @@ void Inkscape::SelTrans::grab(Geom::Point const &p, gdouble x, gdouble y, bool s /* Snapping a huge number of nodes will take way too long, so limit the number of snappable nodes A typical user would rarely ever try to snap such a large number of nodes anyway, because (s)he would hardly be able to discern which node would be snapping */ + std::cout << "Warning: limit of 200 snap sources reached, some will be ignored" << std::endl; _snap_points.resize(200); // Unfortunately, by now we will have lost the font-baseline snappoints :-( } -- cgit v1.2.3 From 549d5631f51d2f3948c7321d1e30e89688a50664 Mon Sep 17 00:00:00 2001 From: Diederik van Lierop Date: Sat, 23 Aug 2014 07:56:55 +0200 Subject: Fix skewing bug that could lead to infinite transforms Fixed bugs: - https://launchpad.net/bugs/1266499 (bzr r13530) --- src/seltrans.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) (limited to 'src/seltrans.cpp') diff --git a/src/seltrans.cpp b/src/seltrans.cpp index 6b8cd19bb..e15249f94 100644 --- a/src/seltrans.cpp +++ b/src/seltrans.cpp @@ -9,7 +9,7 @@ * Abhishek Sharma * * Copyright (C) 1999-2002 Lauris Kaplinski - * Copyright (C) 1999-2008 Authors + * Copyright (C) 1999-2014 Authors * * Released under GNU GPL, read the file 'COPYING' for more information */ @@ -1109,10 +1109,17 @@ gboolean Inkscape::SelTrans::skewRequest(SPSelTransHandle const &handle, Geom::P break; } + // _point and _origin are noisy, ranging from 1 to 1e-9 or even smaller; this is due to the + // limited SVG output precision, which can be arbitrarily set in the preferences Geom::Point const initial_delta = _point - _origin; - if (fabs(initial_delta[dim_a]) < 1e-15) { - return false; + // The handle and the origin shouldn't be too close to each other; let's check for that! + // Due to the limited resolution though (see above), we'd better use a relative error here + if (_bbox) { + Geom::Coord d = (*_bbox).dimensions()[dim_a]; + if (fabs(initial_delta[dim_a]/d) < 1e-4) { + return false; + } } // Calculate the scale factors, which can be either visual or geometric -- cgit v1.2.3 From 1626feb62df11c7157e084c305cbf93624eedc58 Mon Sep 17 00:00:00 2001 From: Nicolas Dufour Date: Wed, 29 Oct 2014 20:23:46 +0100 Subject: i18n. Fixing untranslated strings. (bzr r13640) --- src/seltrans.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/seltrans.cpp') diff --git a/src/seltrans.cpp b/src/seltrans.cpp index e15249f94..96c7fb49b 100644 --- a/src/seltrans.cpp +++ b/src/seltrans.cpp @@ -651,7 +651,7 @@ void Inkscape::SelTrans::_makeHandles() { for (int i = 0; i < NUMHANDS; i++) { SPSelTransTypeInfo info = handtypes[hands[i].type]; - knots[i] = new SPKnot(_desktop, info.tip); + knots[i] = new SPKnot(_desktop, _(info.tip)); knots[i]->setShape(SP_CTRL_SHAPE_BITMAP); knots[i]->setSize(13); -- cgit v1.2.3