Fix bug #1694111 also fixes noumerous bugfixes on LPE undo. And shapes are improved to only perform path effet one time each

Fixed bugs:
  - https://launchpad.net/bugs/1694111

(bzr r15713)

11 files changed, 55 insertions, 37 deletions

diff --git a/src/live_effects/effect.cpp b/src/live_effects/effect.cpp
index 3a628b243..21136353c 100644
--- a/src/live_effects/effect.cpp
+++ b/src/live_effects/effect.cpp
@@ -452,10 +452,13 @@ Effect::processObjects(LpeAction lpe_action)
             std::vector<SPItem*> item_selected;
             SPCSSAttr *css;
             Glib::ustring css_str;
+            SPItem *item = SP_ITEM(elemref);
             switch (lpe_action){
             case LPE_TO_OBJECTS:
-                if (SP_ITEM(elemref)->isHidden()) {
-                    elemref->deleteObject();
+                if (item->isHidden()) {
+                    sp_object_ref(item, 0 );
+                    item->deleteObject(true);
+                    sp_object_unref(item);
                 } else {
                     if (elemnode->attribute("inkscape:path-effect")) {
                         sp_item_list_to_curves(item_list, item_selected, item_to_select);
@@ -465,7 +468,9 @@ Effect::processObjects(LpeAction lpe_action)
                 break;
 
             case LPE_ERASE:
-                elemref->deleteObject();
+                sp_object_ref(item, 0 );
+                item->deleteObject(true);
+                sp_object_unref(item);
                 break;
 
             case LPE_VISIBILITY:
diff --git a/src/live_effects/lpe-copy_rotate.cpp b/src/live_effects/lpe-copy_rotate.cpp
index b29b5e493..3abcbf217 100644
--- a/src/live_effects/lpe-copy_rotate.cpp
+++ b/src/live_effects/lpe-copy_rotate.cpp
@@ -199,15 +199,13 @@ LPECopyRotate::cloneD(SPObject *orig, SPObject *dest, Geom::Affine transform, bo
         }
     }
     SPShape * shape =  SP_SHAPE(orig);
-    SPPath * path =  SP_PATH(dest);
-    if (shape && !path) {
+    if (shape && !SP_IS_PATH(dest)) {
         const char * id = dest->getId();
         Inkscape::XML::Node *dest_node = sp_selected_item_to_curved_repr(SP_ITEM(dest), 0);
         dest->updateRepr(xml_doc, dest_node, SP_OBJECT_WRITE_ALL);
         dest->getRepr()->setAttribute("d", id);
-        path =  SP_PATH(dest);
     }
-    if (path && shape) {
+    if (SP_IS_PATH(dest) && shape) {
         SPCurve *c = NULL;
         if (root) {
             c = new SPCurve();
@@ -216,7 +214,7 @@ LPECopyRotate::cloneD(SPObject *orig, SPObject *dest, Geom::Affine transform, bo
             c = shape->getCurve();
         }
         if (c) {
-            path->setCurve(c, TRUE);
+            SP_PATH(dest)->setCurve(c, TRUE);
             c->unref();
         } else {
             dest->getRepr()->setAttribute("d", NULL);
@@ -725,6 +723,7 @@ LPECopyRotate::doOnRemove (SPLPEItem const* /*lpeitem*/)
     //set "keep paths" hook on sp-lpe-item.cpp
     if (keep_paths) {
         processObjects(LPE_TO_OBJECTS);
+        items.clear();
         return;
     }
     processObjects(LPE_ERASE);
diff --git a/src/live_effects/lpe-mirror_symmetry.cpp b/src/live_effects/lpe-mirror_symmetry.cpp
index 3fcc4ae3d..5d80d65fe 100644
--- a/src/live_effects/lpe-mirror_symmetry.cpp
+++ b/src/live_effects/lpe-mirror_symmetry.cpp
@@ -320,6 +320,7 @@ LPEMirrorSymmetry::doOnRemove (SPLPEItem const* /*lpeitem*/)
     //set "keep paths" hook on sp-lpe-item.cpp
     if (keep_paths) {
         processObjects(LPE_TO_OBJECTS);
+        items.clear();
         return;
     }
     processObjects(LPE_ERASE);
diff --git a/src/live_effects/lpeobject.cpp b/src/live_effects/lpeobject.cpp
index b5b27c984..ca3ae46e0 100644
--- a/src/live_effects/lpeobject.cpp
+++ b/src/live_effects/lpeobject.cpp
@@ -112,6 +112,7 @@ void LivePathEffectObject::set(unsigned key, gchar const *value) {
                 this->effecttype_set = true;
             } else {
                 this->effecttype = Inkscape::LivePathEffect::INVALID_LPE;
+                this->lpe = NULL;
                 this->effecttype_set = false;
             }
 
diff --git a/src/sp-ellipse.cpp b/src/sp-ellipse.cpp
index 5dfc60f1a..30c1096ca 100644
--- a/src/sp-ellipse.cpp
+++ b/src/sp-ellipse.cpp
@@ -408,7 +408,7 @@ const char *SPGenericEllipse::displayName() const
 }
 
 // Create path for rendering shape on screen
-void SPGenericEllipse::set_shape()
+void SPGenericEllipse::set_shape(bool force)
 {
     // std::cout << "SPGenericEllipse::set_shape: Entrance" << std::endl;
     if (hasBrokenPathEffect()) {
@@ -475,6 +475,12 @@ void SPGenericEllipse::set_shape()
 
     /* Reset the shape's curve to the "original_curve"
      * This is very important for LPEs to work properly! (the bbox might be recalculated depending on the curve in shape)*/
+    if(this->getCurveBeforeLPE()) {
+        if(!force && this->getCurveBeforeLPE()->get_pathvector() == curve->get_pathvector()) {
+            curve->unref();
+            return;
+        }
+    }
     this->setCurveInsync(curve, TRUE);
     this->setCurveBeforeLPE(curve);
 
@@ -617,7 +623,7 @@ void SPGenericEllipse::modified(guint flags)
 
 void SPGenericEllipse::update_patheffect(bool write)
 {
-    this->set_shape();
+    this->set_shape(true);
 
     if (write) {
         Inkscape::XML::Node *repr = this->getRepr();
diff --git a/src/sp-ellipse.h b/src/sp-ellipse.h
index dafece625..a879c596d 100644
--- a/src/sp-ellipse.h
+++ b/src/sp-ellipse.h
@@ -64,7 +64,7 @@ public:
     virtual Inkscape::XML::Node *write(Inkscape::XML::Document *xml_doc, Inkscape::XML::Node *repr, unsigned int flags);
     virtual const char *displayName() const;
 
-    virtual void set_shape();
+    virtual void set_shape(bool force = false);
     virtual Geom::Affine set_transform(Geom::Affine const &xform);
 
     virtual void snappoints(std::vector<Inkscape::SnapCandidatePoint> &p, Inkscape::SnapPreferences const *snapprefs) const;
diff --git a/src/sp-lpe-item.cpp b/src/sp-lpe-item.cpp
index 35e7dfa3e..be886bdd2 100644
--- a/src/sp-lpe-item.cpp
+++ b/src/sp-lpe-item.cpp
@@ -122,17 +122,8 @@ void SPLPEItem::set(unsigned int key, gchar const* value) {
 
                 this->lpe_modified_connection_list->clear();
                 // Clear the path effect list
-                PathEffectList::iterator it = this->path_effect_list->begin();
-
-                while ( it != this->path_effect_list->end() )
-                {
-                    if (!value) {
-                        LivePathEffectObject *lpeobj = (*it)->lpeobject;
-                        Inkscape::LivePathEffect::Effect * lpe = lpeobj->get_lpe();
-                        if (lpe) {
-                            lpe->doOnRemove(this);
-                        }
-                    }
+                PathEffectList::iterator it =  this->path_effect_list->begin();
+                while ( it !=  this->path_effect_list->end()) {
                     (*it)->unlink();
                     delete *it;
                     it = this->path_effect_list->erase(it);
@@ -539,11 +530,11 @@ void SPLPEItem::removeCurrentPathEffect(bool keep_paths)
         return;
 
     if (Inkscape::LivePathEffect::Effect* effect_ = this->getCurrentLPE()) {
+        effect_->keep_paths = keep_paths;
         effect_->doOnRemove(this);
     }
     PathEffectList new_list = *this->path_effect_list;
     new_list.remove(lperef); //current lpe ref is always our 'own' pointer from the path_effect_list
-    *this->path_effect_list = new_list;
     this->getRepr()->setAttribute("inkscape:path-effect", patheffectlist_svg_string(new_list));
     if (!keep_paths) {
         // Make sure that ellipse is stored as <svg:circle> or <svg:ellipse> if possible.
@@ -563,15 +554,18 @@ void SPLPEItem::removeAllPathEffects(bool keep_paths)
         if (path_effect_list->empty()) {
             return;
         }
-
-        for (PathEffectList::const_iterator it = path_effect_list->begin(); it != path_effect_list->end(); ++it)
-        {
-            LivePathEffectObject *lpeobj = (*it)->lpeobject;
-            if (lpeobj) {
-                Inkscape::LivePathEffect::Effect * lpe = lpeobj->get_lpe();
-                lpe->keep_paths = true;
-            }
+    }
+    for (PathEffectList::const_iterator it = path_effect_list->begin(); it != path_effect_list->end(); ++it)
+    {
+        LivePathEffectObject *lpeobj = (*it)->lpeobject;
+        if (lpeobj) {
+            Inkscape::LivePathEffect::Effect * lpe = lpeobj->get_lpe();
+            lpe->keep_paths = keep_paths;
+            lpe->doOnRemove(this);
         }
+        (*it)->unlink();
+        delete *it;
+        it = this->path_effect_list->erase(it);
     }
 
     this->getRepr()->setAttribute("inkscape:path-effect", NULL);
diff --git a/src/sp-spiral.cpp b/src/sp-spiral.cpp
index 57eb918fe..d75db3daa 100644
--- a/src/sp-spiral.cpp
+++ b/src/sp-spiral.cpp
@@ -197,7 +197,7 @@ void SPSpiral::update(SPCtx *ctx, guint flags) {
 }
 
 void SPSpiral::update_patheffect(bool write) {
-    this->set_shape();
+    this->set_shape(true);
 
     if (write) {
         Inkscape::XML::Node *repr = this->getRepr();
@@ -310,7 +310,7 @@ void SPSpiral::fitAndDraw(SPCurve* c, double dstep, Geom::Point darray[], Geom::
     g_assert (is_unit_vector (hat2));
 }
 
-void SPSpiral::set_shape() {
+void SPSpiral::set_shape(bool force) {
     if (hasBrokenPathEffect()) {
         g_warning ("The spiral shape has unknown LPE on it! Convert to path to make it editable preserving the appearance; editing it as spiral will remove the bad LPE");
 
@@ -365,6 +365,12 @@ void SPSpiral::set_shape() {
 
     /* Reset the shape'scurve to the "original_curve"
      * This is very important for LPEs to work properly! (the bbox might be recalculated depending on the curve in shape)*/
+    if(this->getCurveBeforeLPE()) {
+        if(!force && this->getCurveBeforeLPE()->get_pathvector() == c->get_pathvector()) {
+            c->unref();
+            return;
+        }
+    }
     setCurveInsync( c, TRUE);
     setCurveBeforeLPE( c );
 
diff --git a/src/sp-spiral.h b/src/sp-spiral.h
index 94724685c..ebf4c9e28 100644
--- a/src/sp-spiral.h
+++ b/src/sp-spiral.h
@@ -70,7 +70,7 @@ public:
     virtual const char* displayName() const;
 	virtual char* description() const;
 
-	virtual void set_shape();
+	virtual void set_shape(bool force = false);
 	virtual void update_patheffect(bool write);
 
 private:
diff --git a/src/sp-star.cpp b/src/sp-star.cpp
index d112962a2..d7ee352c7 100644
--- a/src/sp-star.cpp
+++ b/src/sp-star.cpp
@@ -223,7 +223,7 @@ void SPStar::update(SPCtx *ctx, guint flags) {
 }
 
 void SPStar::update_patheffect(bool write) {
-    this->set_shape();
+    this->set_shape(true);
 
     if (write) {
         Inkscape::XML::Node *repr = this->getRepr();
@@ -363,7 +363,7 @@ sp_star_get_curvepoint (SPStar *star, SPStarPoint point, gint index, bool previ)
 #define NEXT false
 #define PREV true
 
-void SPStar::set_shape() {
+void SPStar::set_shape(bool force) {
     // perhaps we should convert all our shapes into LPEs without source path
     // and with knotholders for parameters, then this situation will be handled automatically
     // by disabling the entire stack (including the shape LPE)
@@ -446,6 +446,12 @@ void SPStar::set_shape() {
 
     /* Reset the shape'scurve to the "original_curve"
      * This is very important for LPEs to work properly! (the bbox might be recalculated depending on the curve in shape)*/
+    if(this->getCurveBeforeLPE()) {
+        if(!force && this->getCurveBeforeLPE()->get_pathvector() == c->get_pathvector()) {
+            c->unref();
+            return;
+        }
+    }
     this->setCurveInsync( c, TRUE);
     this->setCurveBeforeLPE( c );
 
diff --git a/src/sp-star.h b/src/sp-star.h
index 1ebe3298a..4519de869 100644
--- a/src/sp-star.h
+++ b/src/sp-star.h
@@ -55,7 +55,7 @@ public:
 	virtual void snappoints(std::vector<Inkscape::SnapCandidatePoint> &p, Inkscape::SnapPreferences const *snapprefs) const;
 
 	virtual void update_patheffect(bool write);
-	virtual void set_shape();
+	virtual void set_shape(bool force = false);
 	virtual Geom::Affine set_transform(Geom::Affine const& xform);
 };