Base LPE refactor

9 files changed, 113 insertions, 55 deletions

diff --git a/src/ui/dialog/livepatheffect-editor.cpp b/src/ui/dialog/livepatheffect-editor.cpp
index ec8ae4f31..6cf7f807a 100644
--- a/src/ui/dialog/livepatheffect-editor.cpp
+++ b/src/ui/dialog/livepatheffect-editor.cpp
@@ -42,6 +42,7 @@
 #include "object/sp-text.h"
 
 #include "ui/icon-names.h"
+#include "ui/tools/node-tool.h"
 #include "ui/widget/imagetoggler.h"
 
 namespace Inkscape {
@@ -205,42 +206,16 @@ LivePathEffectEditor::showParams(LivePathEffect::Effect& effect)
         lpe_changed = false;
         return;
     }
-    bool expanderopen = false;
-    Gtk::Widget * defaultswidget = effect.defaultParamSet();
-
     if (effectwidget) {
-         if (defaultswidget) {
-            Gtk::Expander * expander = NULL;
-            std::vector<Gtk::Widget *> childs = dynamic_cast<Gtk::Box *> (effectwidget)->get_children();
-            if (childs.size()) {
-                std::vector<Gtk::Widget *> childs_default = dynamic_cast<Gtk::Box *> (childs[childs.size()-1])->get_children();
-                if ((expander = dynamic_cast<Gtk::Expander *>(childs_default[childs_default.size()-1]))){
-                    expanderopen = expander->get_expanded();
-                }
-            }
-        }
         effectcontrol_vbox.remove(*effectwidget);
         delete effectwidget;
         effectwidget = NULL;
     }
-
+    effectwidget = effect.newWidget();
     effectcontrol_frame.set_label(effect.getName());
+    effectcontrol_vbox.pack_start(*effectwidget, true, true);
 
-    effectwidget = effect.newWidget();
-    if (effectwidget) {
-        
-        if (defaultswidget) {
-            Gtk::Expander * expander = NULL;
-            std::vector<Gtk::Widget *> childs_default = dynamic_cast<Gtk::Box *> (defaultswidget)->get_children();
-            if ((expander = dynamic_cast<Gtk::Expander *>(childs_default[childs_default.size()-1]))){
-                expander->set_expanded(expanderopen);
-            }
-            dynamic_cast<Gtk::Box *> (effectwidget)->pack_start(*defaultswidget, true, true);
-        }
-        effectcontrol_vbox.pack_start(*effectwidget, true, true);
-    }
     button_remove.show();
-
     status_label.hide();
     effectcontrol_frame.show();
     effectcontrol_vbox.show_all_children();
@@ -254,7 +229,7 @@ LivePathEffectEditor::selectInList(LivePathEffect::Effect* effect)
 {
     Gtk::TreeNodeChildren chi = effectlist_view.get_model()->children();
     for (Gtk::TreeIter ci = chi.begin() ; ci != chi.end(); ci++) {
-        if (ci->get_value(columns.lperef)->lpeobject->get_lpe() == effect)
+        if (ci->get_value(columns.lperef)->lpeobject->get_lpe() == effect && effectlist_view.get_selection())
             effectlist_view.get_selection()->select(ci);
     }
 }
@@ -580,6 +555,16 @@ void LivePathEffectEditor::on_effect_selection_changed()
             if (effect) {
                 lpe_changed = true;
                 showParams(*effect);
+                //To reload knots and helper paths
+                Inkscape::Selection *sel = _getSelection();
+                if ( sel && !sel->isEmpty() ) {
+                    SPItem *item = sel->singleItem();
+                    if (item) {
+                        sel->clear();
+                        sel->add(item);
+                         Inkscape::UI::Tools::sp_update_helperpath();
+                    }
+                }
             }
         }
     }
diff --git a/src/ui/shape-editor-knotholders.cpp b/src/ui/shape-editor-knotholders.cpp
index 5fb677f27..885f3ef40 100644
--- a/src/ui/shape-editor-knotholders.cpp
+++ b/src/ui/shape-editor-knotholders.cpp
@@ -107,13 +107,7 @@ KnotHolder *createKnotHolder(SPItem *item, SPDesktop *desktop)
 {
     KnotHolder *knotholder = NULL;
 
-    SPLPEItem *lpe = dynamic_cast<SPLPEItem *>(item);
-    if (lpe &&
-        lpe->getCurrentLPE() &&
-        lpe->getCurrentLPE()->isVisible() &&
-        lpe->getCurrentLPE()->providesKnotholder()) {
-        knotholder = sp_lpe_knot_holder(lpe, desktop);
-    } else if (dynamic_cast<SPRect *>(item)) {
+    if (dynamic_cast<SPRect *>(item)) {
         knotholder = new RectKnotHolder(desktop, item, NULL);
     } else if (dynamic_cast<SPBox3D *>(item)) {
         knotholder = new Box3DKnotHolder(desktop, item, NULL);
@@ -139,6 +133,20 @@ KnotHolder *createKnotHolder(SPItem *item, SPDesktop *desktop)
     return knotholder;
 }
 
+KnotHolder *createLPEKnotHolder(SPItem *item, SPDesktop *desktop)
+{
+    KnotHolder *knotholder = NULL;
+
+    SPLPEItem *lpe = dynamic_cast<SPLPEItem *>(item);
+    if (lpe &&
+        lpe->getCurrentLPE() &&
+        lpe->getCurrentLPE()->isVisible() &&
+        lpe->getCurrentLPE()->providesKnotholder()) {
+        knotholder = sp_lpe_knot_holder(lpe, desktop);
+    }
+    return knotholder;
+}
+
 }
 } // namespace Inkscape
 
diff --git a/src/ui/shape-editor.cpp b/src/ui/shape-editor.cpp
index 3a5aec056..b40bec86f 100644
--- a/src/ui/shape-editor.cpp
+++ b/src/ui/shape-editor.cpp
@@ -29,13 +29,16 @@ namespace Inkscape {
 namespace UI {
 
 KnotHolder *createKnotHolder(SPItem *item, SPDesktop *desktop);
+KnotHolder *createLPEKnotHolder(SPItem *item, SPDesktop *desktop);
 
 bool ShapeEditor::_blockSetItem = false;
 
 ShapeEditor::ShapeEditor(SPDesktop *dt, Geom::Affine edit_transform) :
     desktop(dt),
     knotholder(nullptr),
+    lpeknotholder(nullptr),
     knotholder_listener_attached_for(nullptr),
+    lpeknotholder_listener_attached_for(nullptr),
     _edit_transform(edit_transform)
 {
 }
@@ -58,25 +61,43 @@ void ShapeEditor::unset_item(bool keep_knotholder) {
             this->knotholder = NULL;
         }
     }
+    if (this->lpeknotholder) {
+        Inkscape::XML::Node *old_repr = this->lpeknotholder->repr;
+        if (old_repr && old_repr == lpeknotholder_listener_attached_for) {
+            sp_repr_remove_listener_by_data(old_repr, this);
+            Inkscape::GC::release(old_repr);
+            lpeknotholder_listener_attached_for = NULL;
+        }
+
+        if (!keep_knotholder) {
+            delete this->lpeknotholder;
+            this->lpeknotholder = NULL;
+        }
+    }
 }
 
 bool ShapeEditor::has_knotholder() {
-    return this->knotholder != NULL;
+    return this->knotholder != NULL || this->lpeknotholder != NULL;
 }
 
 void ShapeEditor::update_knotholder() {
     if (this->knotholder)
         this->knotholder->update_knots();
+    if (this->lpeknotholder)
+        this->lpeknotholder->update_knots();
 }
 
 bool ShapeEditor::has_local_change() {
-    return (this->knotholder && this->knotholder->local_change != 0);
+    return (this->knotholder && this->knotholder->local_change != 0) || (this->lpeknotholder && this->lpeknotholder->local_change != 0);
 }
 
 void ShapeEditor::decrement_local_change() {
     if (this->knotholder) {
         this->knotholder->local_change = FALSE;
     }
+    if (this->lpeknotholder) {
+        this->lpeknotholder->local_change = FALSE;
+    }
 }
 
 void ShapeEditor::event_attr_changed(Inkscape::XML::Node * node, gchar const *name, gchar const *, gchar const *, bool, void *data)
@@ -123,6 +144,10 @@ void ShapeEditor::set_item(SPItem *item, bool keep_knotholder) {
             // only recreate knotholder if none is present
             this->knotholder = createKnotHolder(item, desktop);
         }
+        if (!this->lpeknotholder) {
+            // only recreate knotholder if none is present
+            this->lpeknotholder = createLPEKnotHolder(item, desktop);
+        }
         if (this->knotholder) {
             this->knotholder->setEditTransform(_edit_transform);
             this->knotholder->update_knots();
@@ -134,6 +159,17 @@ void ShapeEditor::set_item(SPItem *item, bool keep_knotholder) {
                 knotholder_listener_attached_for = repr;
             }
         }
+        if (this->lpeknotholder) {
+            this->lpeknotholder->setEditTransform(_edit_transform);
+            this->lpeknotholder->update_knots();
+            // setting new listener
+            repr = this->lpeknotholder->repr;
+            if (repr != lpeknotholder_listener_attached_for) {
+                Inkscape::GC::anchor(repr);
+                sp_repr_add_listener(repr, &shapeeditor_repr_events, this);
+                lpeknotholder_listener_attached_for = repr;
+            }
+        }
     }
 }
 
@@ -145,6 +181,9 @@ void ShapeEditor::reset_item(bool keep_knotholder)
     if (knotholder) {
         SPObject *obj = desktop->getDocument()->getObjectByRepr(knotholder_listener_attached_for); /// note that it is not certain that this is an SPItem; it could be a LivePathEffectObject.
         set_item(SP_ITEM(obj), keep_knotholder);
+    } else if (lpeknotholder) {
+        SPObject *obj = desktop->getDocument()->getObjectByRepr(lpeknotholder_listener_attached_for); /// note that it is not certain that this is an SPItem; it could be a LivePathEffectObject.
+        set_item(SP_ITEM(obj), keep_knotholder);
     }
 }
 
@@ -155,7 +194,9 @@ bool ShapeEditor::knot_mouseover() const {
     if (this->knotholder) {
         return knotholder->knot_mouseover();
     }
-    
+    if (this->lpeknotholder) {
+        return lpeknotholder->knot_mouseover();
+    }
 
     return false;
 }
diff --git a/src/ui/shape-editor.h b/src/ui/shape-editor.h
index e30b2d60b..67bce1c98 100644
--- a/src/ui/shape-editor.h
+++ b/src/ui/shape-editor.h
@@ -36,6 +36,7 @@ public:
 
     bool knot_mouseover() const;
     KnotHolder *knotholder;
+    KnotHolder *lpeknotholder;
     bool has_knotholder();
     static void blockSetItem(bool b) { _blockSetItem = b; } // kludge
     static void event_attr_changed(Inkscape::XML::Node * /*repr*/, char const *name, char const * /*old_value*/,
@@ -46,6 +47,7 @@ private:
 
     SPDesktop *desktop;
     Inkscape::XML::Node *knotholder_listener_attached_for;
+    Inkscape::XML::Node *lpeknotholder_listener_attached_for;
     Geom::Affine _edit_transform;
 };
 
diff --git a/src/ui/tool/path-manipulator.cpp b/src/ui/tool/path-manipulator.cpp
index 55e171420..dcf0a4d4b 100644
--- a/src/ui/tool/path-manipulator.cpp
+++ b/src/ui/tool/path-manipulator.cpp
@@ -190,7 +190,7 @@ void PathManipulator::writeXML()
 {
     if (!_live_outline)
         _updateOutline();
-    if (!_live_objects)
+    if (_live_objects)
         _setGeometry();
 
     if (!_path) return;
@@ -1152,6 +1152,9 @@ void PathManipulator::_createControlPointsFromGeometry()
             ++i;
         }
     }
+    if (pathv.empty()) {
+        return;
+    }
     _spcurve->set_pathvector(pathv);
 
     pathv *= (_edit_transform * _i2d_transform);
@@ -1358,6 +1361,23 @@ void PathManipulator::_createGeometryFromControlPoints(bool alert_LPE)
     }
     builder.flush();
     Geom::PathVector pathv = builder.peek() * (_edit_transform * _i2d_transform).inverse();
+    for (Geom::PathVector::iterator i = pathv.begin(); i != pathv.end(); ) {
+        // NOTE: this utilizes the fact that Geom::PathVector is an std::vector.
+        // When we erase an element, the next one slides into position,
+        // so we do not increment the iterator even though it is theoretically invalidated.
+        if (i->empty()) {
+            i = pathv.erase(i);
+        } else {
+            ++i;
+        }
+    }
+    if (pathv.empty()) {
+        return;
+    }
+
+    if (_spcurve->get_pathvector() == pathv) {
+        return;
+    }
     _spcurve->set_pathvector(pathv);
     if (alert_LPE) {
         /// \todo note that _path can be an Inkscape::LivePathEffect::Effect* too, kind of confusing, rework member naming?
@@ -1371,7 +1391,6 @@ void PathManipulator::_createGeometryFromControlPoints(bool alert_LPE)
             }
         }
     }
-
     if (_live_outline)
         _updateOutline();
     if (_live_objects)
@@ -1465,7 +1484,7 @@ void PathManipulator::_getGeometry()
         }
     } else {
         _spcurve->unref();
-        _spcurve = _path->get_curve_for_edit();
+        _spcurve = _path->getCurveForEdit();
         // never allow NULL to sneak in here!
         if (_spcurve == NULL) {
             _spcurve = new SPCurve();
@@ -1477,7 +1496,6 @@ void PathManipulator::_getGeometry()
 void PathManipulator::_setGeometry()
 {
     using namespace Inkscape::LivePathEffect;
-
     if (!_lpe_key.empty()) {
         // copied from nodepath.cpp
         // NOTE: if we are editing an LPE param, _path is not actually an SPPath, it is
@@ -1485,17 +1503,22 @@ void PathManipulator::_setGeometry()
         Effect *lpe = LIVEPATHEFFECT(_path)->get_lpe();
         if (lpe) {
             PathParam *pathparam = dynamic_cast<PathParam *>(lpe->getParameter(_lpe_key.data()));
+            if (pathparam->get_pathvector() == _spcurve->get_pathvector()) {
+                return; //False we dont update LPE
+            }
             pathparam->set_new_value(_spcurve->get_pathvector(), false);
             LIVEPATHEFFECT(_path)->requestModified(SP_OBJECT_MODIFIED_FLAG);
         }
     } else {
+        // return true to leave the decission on empty to the caller. 
+        // Maybe the path become empty and we want to update to empty
         if (empty()) return;
-        if (SPCurve * original = _path->get_original_curve()){
+        if (SPCurve * original = _path->getCurveBeforeLPE()){
             if(!_spcurve->is_equal(original)) {
-                _path->set_original_curve(_spcurve, false, false);
+                _path->setCurveBeforeLPE(_spcurve);
                 delete original;
             }
-        } else if(!_spcurve->is_equal(_path->get_curve())) {
+        } else if(!_spcurve->is_equal(_path->getCurve(true))) {
             _path->setCurve(_spcurve, false);
         }
     }
@@ -1658,7 +1681,6 @@ Geom::Coord PathManipulator::_updateDragPoint(Geom::Point const &evp)
 
     Geom::Affine to_desktop = _edit_transform * _i2d_transform;
     Geom::PathVector pv = _spcurve->get_pathvector();
-
     boost::optional<Geom::PathVectorTime> pvp =
         pv.nearestTime(_desktop->w2d(evp) * to_desktop.inverse());
     if (!pvp) return dist;
diff --git a/src/ui/tools/connector-tool.cpp b/src/ui/tools/connector-tool.cpp
index ec55ab80b..20b309a98 100644
--- a/src/ui/tools/connector-tool.cpp
+++ b/src/ui/tools/connector-tool.cpp
@@ -617,7 +617,7 @@ bool ConnectorTool::_handleMotionNotify(GdkEventMotion const &mevent)
         Geom::Affine i2d ( (this->clickeditem)->i2dt_affine() );
         Geom::Affine d2i = i2d.inverse();
         SPPath *path = SP_PATH(this->clickeditem);
-        SPCurve *curve = path->get_curve();
+        SPCurve *curve = path->getCurve(true);
         if (this->clickedhandle == this->endpt_handle[0]) {
             Geom::Point o = this->endpt_handle[1]->pos;
             curve->stretch_endpoints(p * d2i, o * d2i);
@@ -628,7 +628,7 @@ bool ConnectorTool::_handleMotionNotify(GdkEventMotion const &mevent)
         sp_conn_reroute_path_immediate(path);
 
         // Copy this to the temporary visible path
-        this->red_curve = path->get_curve_for_edit();
+        this->red_curve = path->getCurveForEdit();
         this->red_curve->transform(i2d);
 
         sp_canvas_bpath_set_bpath(SP_CANVAS_BPATH(this->red_bpath), this->red_curve, true);
@@ -1041,7 +1041,7 @@ static gboolean endpt_handler(SPKnot */*knot*/, GdkEvent *event, ConnectorTool *
             }
 
             // Show the red path for dragging.
-            cc->red_curve = SP_PATH(cc->clickeditem)->get_curve_for_edit();
+            cc->red_curve = SP_PATH(cc->clickeditem)->getCurveForEdit();
             Geom::Affine i2d = (cc->clickeditem)->i2dt_affine();
             cc->red_curve->transform(i2d);
             sp_canvas_bpath_set_bpath(SP_CANVAS_BPATH(cc->red_bpath), cc->red_curve, true);
@@ -1135,7 +1135,7 @@ void ConnectorTool::cc_set_active_conn(SPItem *item)
 {
     g_assert( SP_IS_PATH(item) );
 
-    const SPCurve *curve = SP_PATH(item)->get_curve_reference();
+    const SPCurve *curve = SP_PATH(item)->getCurveForEdit(true);
     Geom::Affine i2dt = item->i2dt_affine();
 
     if (this->active_conn == item) {
@@ -1275,7 +1275,7 @@ static bool cc_item_is_shape(SPItem *item)
 bool cc_item_is_connector(SPItem *item)
 {
     if (SP_IS_PATH(item)) {
-        bool closed = SP_PATH(item)->get_curve_reference()->is_closed();
+        bool closed = SP_PATH(item)->getCurveForEdit(true)->is_closed();
         if (SP_PATH(item)->connEndPair.isAutoRoutingConn() && !closed) {
             // To be considered a connector, an object must be a non-closed
             // path that is marked with a "inkscape:connector-type" attribute.
diff --git a/src/ui/tools/freehand-base.cpp b/src/ui/tools/freehand-base.cpp
index 26d812394..b57aa2fb0 100644
--- a/src/ui/tools/freehand-base.cpp
+++ b/src/ui/tools/freehand-base.cpp
@@ -645,7 +645,7 @@ static void spdc_attach_selection(FreehandBase *dc, Inkscape::Selection */*sel*/
 
         // Curve list
         // We keep it in desktop coordinates to eliminate calculation errors
-        SPCurve *norm = SP_PATH(item)->get_curve_for_edit();
+        SPCurve *norm = SP_PATH(item)->getCurveForEdit();
         norm->transform((dc->white_item)->i2dt_affine());
         g_return_if_fail( norm != NULL );
         dc->white_curves = norm->split();
diff --git a/src/ui/tools/node-tool.cpp b/src/ui/tools/node-tool.cpp
index 6e586b0e9..d54adba89 100644
--- a/src/ui/tools/node-tool.cpp
+++ b/src/ui/tools/node-tool.cpp
@@ -533,7 +533,7 @@ bool NodeTool::root_handler(GdkEvent* event) {
             }
 
             this->flashed_item = over_item;
-            SPCurve *c = SP_SHAPE(over_item)->getCurveBeforeLPE();
+            SPCurve *c = SP_SHAPE(over_item)->getCurveForEdit();
 
             if (!c) {
                 break; // break out when curve doesn't exist
diff --git a/src/ui/widget/selected-style.cpp b/src/ui/widget/selected-style.cpp
index d317fa216..3988ca544 100644
--- a/src/ui/widget/selected-style.cpp
+++ b/src/ui/widget/selected-style.cpp
@@ -109,7 +109,7 @@ typedef enum {
 // code, those warnings are actually desired. They say "Hey! Fix this". We
 // definitely don't want to hide/ignore them. --JonCruz
 static const GtkTargetEntry ui_drop_target_entries [] = {
-    {"application/x-color", 0, APP_X_COLOR}
+    {g_strdup("application/x-color"), 0, APP_X_COLOR}
 };
 
 static guint nui_drop_target_entries = G_N_ELEMENTS(ui_drop_target_entries);