Second step: try to make helper curves respond faster (don't recreate/delete the...

diff --git a/src/live_effects/effect.cpp b/src/live_effects/effect.cpp
index 0497d0e375c3e63fa60611dc6dc1da30664a4545..9476f0377d15cc6255a32fe4602ddbda8de531a8 100644 (file)
--- a/src/live_effects/effect.cpp
+++ b/src/live_effects/effect.cpp
@@ -408,12 +408,16 @@ Effect::addHelperPaths(SPLPEItem *lpeitem, SPDesktop *desktop)
 {
     g_return_if_fail(SP_IS_PATH(lpeitem));
 
-    if (show_orig_path) {
+    if (providesKnotholder() && showOrigPath()) {
+        // TODO: we assume that if the LPE provides its own knotholder, there is no nodepath so we
+        // must create the helper curve for the original path manually; when we allow nodepaths and
+        // knotholders alongside each other, this needs to be rethought!
         SPCanvasItem *canvasitem = sp_nodepath_generate_helperpath(desktop, SP_PATH(lpeitem));
         // TODO: Make sure the tempitem doesn't get destroyed when the mouse leaves the item
         Inkscape::Display::TemporaryItem* tmpitem = desktop->add_temporary_canvasitem (canvasitem, 0);
         lpeitem->lpe_helperpaths.push_back(tmpitem);
     }
+
     addHelperPathsImpl(lpeitem, desktop);
 }
 

diff --git a/src/live_effects/effect.h b/src/live_effects/effect.h
index 32917f8c8e4531a88f89946a0d182d05912bbf7a..9bcbf51ab67c009a327580f846d41dbd3ccff199 100644 (file)
--- a/src/live_effects/effect.h
+++ b/src/live_effects/effect.h
@@ -116,6 +116,8 @@ public:
 
     virtual void transform_multiply(Geom::Matrix const& postmul, bool set);
 
+    // TODO: providesKnotholder() is currently used as an indicator of whether a nodepath is
+    // created for an item or not. When we allow both at the same time, this needs rethinking!
     bool providesKnotholder() { return (kh_entity_vector.size() > 0); }
     virtual LPEPathFlashType pathFlashType() { return DEFAULT; }
     void addHandles(KnotHolder *knotholder, SPDesktop *desktop, SPItem *item);
@@ -124,6 +126,7 @@ public:
     inline bool providesOwnFlashPaths() {
         return provides_own_flash_paths || show_orig_path;
     }
+    inline bool showOrigPath() { return show_orig_path; }
 
     Glib::ustring          getName();
     Inkscape::XML::Node *  getRepr();

diff --git a/src/live_effects/lpe-mirror_reflect.h b/src/live_effects/lpe-mirror_reflect.h
index a19aafb891101f640426b1f4d7497875615d9c69..b4f7029a0787e65c5a201c46db83a84047681da8 100644 (file)
--- a/src/live_effects/lpe-mirror_reflect.h
+++ b/src/live_effects/lpe-mirror_reflect.h
@@ -31,7 +31,7 @@ public:
     virtual void acceptParamPath (SPPath *param_path);
     virtual int acceptsNumParams() { return 2; }
 
-    virtual LPEPathFlashType pathFlashType() { return PERMANENT_FLASH; }
+    virtual LPEPathFlashType pathFlashType() { return SUPPRESS_FLASH; }
 
     virtual std::vector<Geom::Path> doEffect_path (std::vector<Geom::Path> const & path_in);
 

diff --git a/src/live_effects/lpe-tangent_to_curve.h b/src/live_effects/lpe-tangent_to_curve.h
index 8d9622c672cf95d888c8685fddf14ec6f081491a..464b692bfdc97fa94f49083f9930f1db0b47f768 100644 (file)
--- a/src/live_effects/lpe-tangent_to_curve.h
+++ b/src/live_effects/lpe-tangent_to_curve.h
@@ -38,7 +38,7 @@ public:
     virtual Geom::Piecewise<Geom::D2<Geom::SBasis> >
       doEffect_pwd2 (Geom::Piecewise<Geom::D2<Geom::SBasis> > const & pwd2_in);
 
-    virtual LPEPathFlashType pathFlashType() { return PERMANENT_FLASH; }
+    virtual LPEPathFlashType pathFlashType() { return SUPPRESS_FLASH; }
 
     /* the knotholder entity classes must be declared friends */
     friend class TtC::KnotHolderEntityLeftEnd;

diff --git a/src/node-context.cpp b/src/node-context.cpp
index 88ea1667d24e40632208f8e9225dbb13073fc3c7..b85e08b689c06163f090a82262b37405d539a32e 100644 (file)
--- a/src/node-context.cpp
+++ b/src/node-context.cpp
@@ -37,6 +37,8 @@
 #include "shape-editor.h"
 #include "live_effects/effect.h"
 
+#include "sp-lpe-item.h"
+
 // needed for flash nodepath upon mouseover:
 #include "display/canvas-bpath.h"
 #include "display/curve.h"
@@ -119,6 +121,12 @@ sp_node_context_dispose(GObject *object)
     nc->sel_changed_connection.disconnect();
     nc->sel_changed_connection.~connection();
 
+    // TODO: should this be here?
+    SPItem *item = sp_desktop_selection(ec->desktop)->singleItem();
+    if (item && SP_IS_LPE_ITEM(item)) {
+        sp_lpe_item_remove_temporary_canvasitems(SP_LPE_ITEM(item), ec->desktop);
+    }
+
     delete nc->shape_editor;
 
     if (nc->_node_message_context) {
@@ -138,11 +146,8 @@ sp_node_context_setup(SPEventContext *ec)
 
     Inkscape::Selection *selection = sp_desktop_selection (ec->desktop);
     nc->sel_changed_connection.disconnect();
-    nc->sel_modified_connection.disconnect();
     nc->sel_changed_connection =
         selection->connectChanged(sigc::bind(sigc::ptr_fun(&sp_node_context_selection_changed), (gpointer)nc));
-    nc->sel_modified_connection =
-        selection->connectModified(sigc::bind(sigc::ptr_fun(&sp_node_context_selection_modified), (gpointer)nc));
 
     SPItem *item = selection->singleItem();
 
@@ -212,27 +217,6 @@ sp_node_context_selection_changed(Inkscape::Selection *selection, gpointer data)
     nc->shape_editor->update_statusbar();
 }
 
-/**
-\brief  Callback that processes the "modified" signal on the selection;
-updates temporary canvasitems associated to LPEItems in the selection
-*/
-void
-sp_node_context_selection_modified(Inkscape::Selection *selection, guint /*flags*/, gpointer /*data*/)
-{
-    // TODO: do this for *all* items in the selection
-    SPItem *item = selection->singleItem();
-
-    // TODO: This is *very* inefficient! Can we avoid destroying and recreating the temporary
-    // items? Also, we should only update those that actually need it!
-    if (item && SP_IS_LPE_ITEM(item)) {
-        SPLPEItem *lpeitem = SP_LPE_ITEM(item);
-        SPDesktop *desktop = selection->desktop();
-
-        sp_lpe_item_remove_temporary_canvasitems(lpeitem, desktop);
-        sp_lpe_item_add_temporary_canvasitems(lpeitem, desktop);
-    }
-}
-
 void
 sp_node_context_show_modifier_tip(SPEventContext *event_context, GdkEvent *event)
 {

diff --git a/src/node-context.h b/src/node-context.h
index f67a32f6b7784fe3d211fcf3ddaf72e6edcca0c1..922569d27a0fb1da270dd3a8700c217a687a4cbe 100644 (file)
--- a/src/node-context.h
+++ b/src/node-context.h
@@ -48,7 +48,6 @@ struct SPNodeContext {
        bool rb_escaped;
 
        sigc::connection sel_changed_connection;
-       sigc::connection sel_modified_connection;
 
        Inkscape::MessageContext *_node_message_context;
 

diff --git a/src/nodepath.cpp b/src/nodepath.cpp
index ee8e5e1237983194f3f11632312b944615a5e0ce..c9456d17e64f1ef79abbc1bb1fd8735b0cac34fc 100644 (file)
--- a/src/nodepath.cpp
+++ b/src/nodepath.cpp
@@ -152,6 +152,20 @@ static void sp_nodepath_set_curve (Inkscape::NodePath::Path *np, SPCurve *curve)
 // active_node indicates mouseover node
 Inkscape::NodePath::Node * Inkscape::NodePath::Path::active_node = NULL;
 
+static void sp_nodepath_draw_helper_curve(Inkscape::NodePath::Path *np, SPDesktop *desktop) {
+    // Draw helper curve
+    if (np->show_helperpath) {
+        SPCurve *helper_curve = np->curve->copy();
+        helper_curve->transform(np->i2d );
+        np->helper_path = sp_canvas_bpath_new(sp_desktop_controls(desktop), helper_curve);
+        sp_canvas_bpath_set_stroke(SP_CANVAS_BPATH(np->helper_path), np->helperpath_rgba, np->helperpath_width, SP_STROKE_LINEJOIN_MITER, SP_STROKE_LINECAP_BUTT);
+        sp_canvas_bpath_set_fill(SP_CANVAS_BPATH(np->helper_path), 0, SP_WIND_RULE_NONZERO);
+        sp_canvas_item_move_to_z(np->helper_path, 0);
+        sp_canvas_item_show(np->helper_path);
+        helper_curve->unref();
+    }
+}
+
 /**
  * \brief Creates new nodepath from item
  */
@@ -208,7 +222,13 @@ Inkscape::NodePath::Path *sp_nodepath_new(SPDesktop *desktop, SPObject *object,
     np->helperpath_rgba = prefs_get_int_attribute("tools.nodes", "highlight_color", 0xff0000ff);
     np->helperpath_width = 1.0;
     np->curve = curve->copy();
-    np->show_helperpath = prefs_get_int_attribute ("tools.nodes", "show_helperpath",  0) == 1;
+    np->show_helperpath = (prefs_get_int_attribute ("tools.nodes", "show_helperpath",  0) == 1);
+    if (SP_IS_LPE_ITEM(object)) {
+        Inkscape::LivePathEffect::Effect *lpe = sp_lpe_item_get_current_lpe(SP_LPE_ITEM(object));
+        if (lpe && lpe->isVisible() && lpe->showOrigPath()) {
+            np->show_helperpath = true;
+        }            
+    }
     np->straight_path = false;
     if (IS_LIVEPATHEFFECT(object) && item) {
         np->item = item;
@@ -264,17 +284,7 @@ Inkscape::NodePath::Path *sp_nodepath_new(SPDesktop *desktop, SPObject *object,
     // create the livarot representation from the same item
     sp_nodepath_ensure_livarot_path(np);
 
-    // Draw helper curve
-    if (np->show_helperpath) {
-        SPCurve *helper_curve = np->curve->copy();
-        helper_curve->transform(np->i2d );
-        np->helper_path = sp_canvas_bpath_new(sp_desktop_controls(desktop), helper_curve);
-        sp_canvas_bpath_set_stroke(SP_CANVAS_BPATH(np->helper_path), np->helperpath_rgba, np->helperpath_width, SP_STROKE_LINEJOIN_MITER, SP_STROKE_LINECAP_BUTT);
-        sp_canvas_bpath_set_fill(SP_CANVAS_BPATH(np->helper_path), 0, SP_WIND_RULE_NONZERO);
-        sp_canvas_item_move_to_z(np->helper_path, 0);
-        sp_canvas_item_show(np->helper_path);
-        helper_curve->unref();
-    }
+    sp_nodepath_draw_helper_curve(np, desktop);
 
     return np;
 }

diff --git a/src/shape-editor.cpp b/src/shape-editor.cpp
index b975d3f79ac98fb4777db384cfa36bfa6fd31b28..d5712525aea92a5e52d9d155b97e2d59f70b84fb 100644 (file)
--- a/src/shape-editor.cpp
+++ b/src/shape-editor.cpp
@@ -192,13 +192,14 @@ void ShapeEditor::set_item(SPItem *item) {
     if (item) {
         if (SP_IS_LPE_ITEM(item)) {
             SPLPEItem *lpeitem = SP_LPE_ITEM(item);
-            if (!sp_lpe_item_get_current_lpe(lpeitem) ||   // if returns NULL, the whole expression evaluates to true and C++ will not call the otherwise crashing 2 functions below
-                !sp_lpe_item_get_current_lpe(lpeitem)->isVisible() ||
-                !sp_lpe_item_get_current_lpe(lpeitem)->providesKnotholder()) {
-                    // only create nodepath if the item either doesn't have an LPE
-                    // or the LPE is invisible or it doesn't provide a knotholder itself
-                    this->nodepath =
-                        sp_nodepath_new(desktop, item, (prefs_get_int_attribute("tools.nodes", "show_handles", 1) != 0));
+            Inkscape::LivePathEffect::Effect *lpe = sp_lpe_item_get_current_lpe(lpeitem);
+            if (!(lpe && lpe->isVisible() && lpe->providesKnotholder())) {
+                // only create nodepath if the item either doesn't have an LPE
+                // or the LPE is invisible or it doesn't provide a knotholder itself
+                this->nodepath = sp_nodepath_new(desktop, item,
+                                                 (prefs_get_int_attribute("tools.nodes", "show_handles", 1) != 0));
+            } else if (lpe && lpe->isVisible() && lpe->showOrigPath()) {
+                sp_lpe_item_add_temporary_canvasitems(lpeitem, desktop);
             }
         }