much better fix for lpe forking, that can no longer turn into infinite loop

still, lpes are forked when referenced by clones...

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

(bzr r9460)

2 files changed, 33 insertions, 26 deletions

diff --git a/src/sp-lpe-item.cpp b/src/sp-lpe-item.cpp
index fb5eb8799..77b2db6f6 100644
--- a/src/sp-lpe-item.cpp
+++ b/src/sp-lpe-item.cpp
@@ -773,16 +773,20 @@ bool sp_lpe_item_set_current_path_effect(SPLPEItem *lpeitem, Inkscape::LivePathE
 }
 
 /**
+ * Writes a new "inkscape:path-effect" string to xml, where the old_lpeobjects are substituted by the new ones.
  *  Note that this method messes up the item's \c PathEffectList.
  */
-void sp_lpe_item_replace_path_effect(SPLPEItem *lpeitem, LivePathEffectObject * old_lpeobj,
-                                        LivePathEffectObject * new_lpeobj)
+void SPLPEItem::replacePathEffects( std::vector<LivePathEffectObject const *> const old_lpeobjs,
+                                    std::vector<LivePathEffectObject const *> const new_lpeobjs )
 {
     HRefList hreflist;
-    for (PathEffectList::const_iterator it = lpeitem->path_effect_list->begin(); it != lpeitem->path_effect_list->end(); ++it)
+    for (PathEffectList::const_iterator it = this->path_effect_list->begin(); it != this->path_effect_list->end(); ++it)
     {
-        if ((*it)->lpeobject == old_lpeobj) {
-            const gchar * repr_id = SP_OBJECT_REPR(new_lpeobj)->attribute("id");
+        LivePathEffectObject const * current_lpeobj = (*it)->lpeobject;
+        std::vector<LivePathEffectObject const *>::const_iterator found_it(std::find(old_lpeobjs.begin(), old_lpeobjs.end(), current_lpeobj));
+        if ( found_it != old_lpeobjs.end() ) {
+            std::vector<LivePathEffectObject const *>::difference_type found_index = std::distance (old_lpeobjs.begin(), found_it);
+            const gchar * repr_id = SP_OBJECT_REPR(new_lpeobjs[found_index])->attribute("id");
             gchar *hrefstr = g_strdup_printf("#%s", repr_id);
             hreflist.push_back( std::string(hrefstr) );
             g_free(hrefstr);
@@ -792,7 +796,7 @@ void sp_lpe_item_replace_path_effect(SPLPEItem *lpeitem, LivePathEffectObject *
         }
     }
     std::string r = hreflist_write_svg(hreflist);
-    SP_OBJECT_REPR(lpeitem)->setAttribute("inkscape:path-effect", r.c_str());
+    SP_OBJECT_REPR(this)->setAttribute("inkscape:path-effect", r.c_str());
 }
 
 /**
@@ -808,25 +812,26 @@ bool sp_lpe_item_fork_path_effects_if_necessary(SPLPEItem *lpeitem, unsigned int
     if ( sp_lpe_item_has_path_effect(lpeitem) ) {
         // If one of the path effects is used by 2 or more items, fork it
         // so that each object has its own independent copy of the effect.
-        // Forking messes up the path effect list, so after each fork,
-        // reload the list and recheck if more forking is required.
-        int maxloops = 20;
-        do {
-            forked = false;
-            PathEffectList effect_list =  sp_lpe_item_get_effect_list(lpeitem);
-            for (PathEffectList::iterator it = effect_list.begin(); it != effect_list.end(); it++)
-            {
-                LivePathEffectObject *lpeobj = (*it)->lpeobject;
-                if (lpeobj) {
-                    LivePathEffectObject *new_lpeobj = lpeobj->fork_private_if_necessary(nr_of_allowed_users);
-                    if (new_lpeobj != lpeobj) {
-                        sp_lpe_item_replace_path_effect(lpeitem, lpeobj, new_lpeobj);
-                        forked = true;
-                        break;  // forked, so break the for-loop and recheck
-                    }
+        // Note: replacing path effects messes up the path effect list
+
+        std::vector<LivePathEffectObject const *> old_lpeobjs, new_lpeobjs;
+        PathEffectList effect_list =  sp_lpe_item_get_effect_list(lpeitem);
+        for (PathEffectList::iterator it = effect_list.begin(); it != effect_list.end(); it++)
+        {
+            LivePathEffectObject *lpeobj = (*it)->lpeobject;
+            if (lpeobj) {
+                LivePathEffectObject *forked_lpeobj = lpeobj->fork_private_if_necessary(nr_of_allowed_users);
+                if (forked_lpeobj != lpeobj) {
+                    forked = true;
+                    old_lpeobjs.push_back(lpeobj);
+                    new_lpeobjs.push_back(forked_lpeobj);
                 }
             }
-        } while (forked && (maxloops-- > 1));
+        }
+
+        if (forked) {
+            lpeitem->replacePathEffects(old_lpeobjs, new_lpeobjs);
+        }
     }
 
     return forked;
diff --git a/src/sp-lpe-item.h b/src/sp-lpe-item.h
index 87425ba94..49edbf90a 100644
--- a/src/sp-lpe-item.h
+++ b/src/sp-lpe-item.h
@@ -35,7 +35,8 @@ namespace LivePathEffect{
 
 typedef std::list<Inkscape::LivePathEffect::LPEObjectReference *> PathEffectList;
 
-struct SPLPEItem : public SPItem {
+class SPLPEItem : public SPItem {
+public:
     int path_effects_enabled;
 
     PathEffectList* path_effect_list;
@@ -43,6 +44,9 @@ struct SPLPEItem : public SPItem {
 
     Inkscape::LivePathEffect::LPEObjectReference* current_path_effect;
     std::vector<Inkscape::Display::TemporaryItem*> lpe_helperpaths;
+
+    void replacePathEffects( std::vector<LivePathEffectObject const *> const old_lpeobjs,
+                             std::vector<LivePathEffectObject const *> const new_lpeobjs );
 };
 
 struct SPLPEItemClass {
@@ -57,8 +61,6 @@ void sp_lpe_item_update_patheffect (SPLPEItem *lpeitem, bool wholetree, bool wri
 bool sp_lpe_item_perform_path_effect(SPLPEItem *lpeitem, SPCurve *curve);
 void sp_lpe_item_add_path_effect(SPLPEItem *lpeitem, gchar *value, bool reset);
 void sp_lpe_item_add_path_effect(SPLPEItem *lpeitem, LivePathEffectObject * new_lpeobj);
-void sp_lpe_item_replace_path_effect(SPLPEItem *lpeitem, LivePathEffectObject * old_lpeobj,
-                                        LivePathEffectObject * new_lpeobj);
 bool sp_lpe_item_fork_path_effects_if_necessary(SPLPEItem *lpeitem, unsigned int nr_of_allowed_users = 1);
 void sp_lpe_item_remove_all_path_effects(SPLPEItem *lpeitem, bool keep_paths);
 void sp_lpe_item_remove_current_path_effect(SPLPEItem *lpeitem, bool keep_paths);