remove many unnecessary to_2geom and from_2geom calls

[inkscape.git] / src / knotholder.cpp

diff --git a/src/knotholder.cpp b/src/knotholder.cpp
index 781d5a3bca0f0ea5396378a3a64d1e883a785a52..f7935c7c33c9280230cbbec706f622317be60ea9 100644 (file)
--- a/src/knotholder.cpp
+++ b/src/knotholder.cpp
@@ -33,7 +33,6 @@
 
 #include "xml/repr.h" // for debugging only
 
-#include <libnr/nr-matrix-div.h>
 #include <glibmm/i18n.h>
 
 class SPDesktop;
@@ -60,17 +59,16 @@ KnotHolder::~KnotHolder() {
     g_object_unref(G_OBJECT(item));
     for(std::list<KnotHolderEntity *>::iterator i = entity.begin(); i != entity.end(); ++i) {
         KnotHolderEntity* e = (*i);
-        if (!e->isLPEParam()) {
-            // knotholder entity may be deleted
+        if (e->isDeletable()) {
             delete (*i);
         } else {
-            // we must not delete the entity since it's an LPE parameter,
+            // we must not delete the entity (since it's attached to an LPE parameter),
             // but the handle should be destroyed
             g_object_unref(e->knot);
         }
         (*i) = NULL;
     }
-    entity.clear(); // this shouldn't be necessary, though
+    entity.clear(); // is this necessary?
 }
 
 /**
@@ -128,12 +126,12 @@ KnotHolder::knot_clicked_handler(SPKnot *knot, guint state)
     }
 
     // for drag, this is done by ungrabbed_handler, but for click we must do it here
-    sp_document_done(SP_OBJECT_DOCUMENT(item), object_verb, 
+    sp_document_done(SP_OBJECT_DOCUMENT(item), object_verb,
                      _("Change handle"));
 }
 
 void
-KnotHolder::knot_moved_handler(SPKnot *knot, NR::Point const *p, guint state)
+KnotHolder::knot_moved_handler(SPKnot *knot, Geom::Point const &p, guint state)
 {
     // this was a local change and the knotholder does not need to be recreated:
     this->local_change = TRUE;
@@ -141,8 +139,8 @@ KnotHolder::knot_moved_handler(SPKnot *knot, NR::Point const *p, guint state)
     for(std::list<KnotHolderEntity *>::iterator i = this->entity.begin(); i != this->entity.end(); ++i) {
         KnotHolderEntity *e = *i;
         if (e->knot == knot) {
-            NR::Point const q = *p / sp_item_i2d_affine(item);
-            e->knot_set(q, e->knot->drag_origin / sp_item_i2d_affine(item), state);
+            NR::Point const q = p * sp_item_i2d_affine(item).inverse();
+            e->knot_set(q, e->knot->drag_origin * sp_item_i2d_affine(item).inverse(), state);
             break;
         }
     }
@@ -155,34 +153,32 @@ KnotHolder::knot_moved_handler(SPKnot *knot, NR::Point const *p, guint state)
 }
 
 void
-KnotHolder::knot_ungrabbed_handler(SPKnot *knot)
+KnotHolder::knot_ungrabbed_handler(SPKnot */*knot*/)
 {
     if (this->released) {
         this->released(this->item);
     } else {
         SPObject *object = (SPObject *) this->item;
+
+        // Caution: this call involves a screen update, which may process events, and as a
+        // result the knotholder may be destructed. So, after the updateRepr, we cannot use any
+        // fields of this knotholder (such as this->item), but only values we have saved beforehand
+        // (such as object).
         object->updateRepr();
 
         /* do cleanup tasks (e.g., for LPE items write the parameter values
          * that were changed by dragging the handle to SVG)
          */
-        if (SP_IS_LPE_ITEM(item)) {
-            // This writes all parameters to SVG. Is this sufficiently efficient or should we only write
-            // the ones that were changed (e.g., via the individual handles' onKnotUngrabbed() method?            
-            Inkscape::LivePathEffect::Effect *lpe = sp_lpe_item_get_current_lpe(SP_LPE_ITEM(item));
+        if (SP_IS_LPE_ITEM(object)) {
+            // This writes all parameters to SVG. Is this sufficiently efficient or should we only
+            // write the ones that were changed?
+
+            Inkscape::LivePathEffect::Effect *lpe = sp_lpe_item_get_current_lpe(SP_LPE_ITEM(object));
             if (lpe) {
                 LivePathEffectObject *lpeobj = lpe->getLPEObj();
                 SP_OBJECT(lpeobj)->updateRepr();
             }
         }
-        // this was once used to write individual parameter values to SVG but this is now done globally above;
-        // we leave the calls to onKnotUngrabbed, anyway, in case any other cleanup tasks need to be done
-        for(std::list<KnotHolderEntity *>::iterator i = this->entity.begin(); i != this->entity.end(); ++i) {
-            KnotHolderEntity *e = *i;
-            if (e->knot == knot) {
-                e->onKnotUngrabbed(); // for most KnotHolderEntitys this does nothing
-            }
-        }
 
         unsigned int object_verb = SP_VERB_NONE;
 
@@ -202,7 +198,7 @@ KnotHolder::knot_ungrabbed_handler(SPKnot *knot)
             else
                 object_verb = SP_VERB_SELECTION_DYNAMIC_OFFSET;
         }
-        
+
         sp_document_done(SP_OBJECT_DOCUMENT (object), object_verb,
                          _("Move handle"));
     }
@@ -227,10 +223,10 @@ KnotHolder::add_pattern_knotholder()
                           // TRANSLATORS: This refers to the pattern that's inside the object
                           _("<b>Move</b> the pattern fill inside the object"),
                           SP_KNOT_SHAPE_CROSS);
-        entity_angle->create(desktop, item, this,
+        entity_scale->create(desktop, item, this,
                              _("<b>Scale</b> the pattern fill uniformly"),
                              SP_KNOT_SHAPE_SQUARE, SP_KNOT_MODE_XOR);
-        entity_scale->create(desktop, item, this,
+        entity_angle->create(desktop, item, this,
                              _("<b>Rotate</b> the pattern fill; with <b>Ctrl</b> to snap angle"),
                              SP_KNOT_SHAPE_CIRCLE, SP_KNOT_MODE_XOR);
         entity.push_back(entity_xy);