From c2aa64ea2a2778e4b78b16962281d0e0659de249 Mon Sep 17 00:00:00 2001 From: "Johan B. C. Engelen" Date: Sat, 15 May 2010 01:08:45 +0200 Subject: fix bug when forking LPE stack Fixed bugs: - https://launchpad.net/bugs/578969 - https://launchpad.net/bugs/505400 (bzr r9419) --- src/sp-lpe-item.cpp | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src/sp-lpe-item.cpp') diff --git a/src/sp-lpe-item.cpp b/src/sp-lpe-item.cpp index 1bb500dd2..740690887 100644 --- a/src/sp-lpe-item.cpp +++ b/src/sp-lpe-item.cpp @@ -772,6 +772,9 @@ bool sp_lpe_item_set_current_path_effect(SPLPEItem *lpeitem, Inkscape::LivePathE return false; } +/** + * 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) { -- cgit v1.2.3 From 4118824234670a01ef4caad683817567ea84e6ae Mon Sep 17 00:00:00 2001 From: "Johan B. C. Engelen" Date: Tue, 18 May 2010 21:17:03 +0200 Subject: better fix for lpe stack forking Fixed bugs: - https://launchpad.net/bugs/578969 (bzr r9431) --- src/sp-lpe-item.cpp | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) (limited to 'src/sp-lpe-item.cpp') diff --git a/src/sp-lpe-item.cpp b/src/sp-lpe-item.cpp index 740690887..cc718f85e 100644 --- a/src/sp-lpe-item.cpp +++ b/src/sp-lpe-item.cpp @@ -795,6 +795,42 @@ void sp_lpe_item_replace_path_effect(SPLPEItem *lpeitem, LivePathEffectObject * SP_OBJECT_REPR(lpeitem)->setAttribute("inkscape:path-effect", r.c_str()); } +/** + * Check all effects in the stack if they are used by other items, and fork them if so. + * It is not recommended to fork the effects by yourself calling LivePathEffectObject::fork_private_if_necessary, + * use this method instead. + * Returns true if one or more effects were forked; returns false if nothing was done. + */ +bool sp_lpe_item_fork_path_effects_if_necessary(SPLPEItem *lpeitem, unsigned int nr_of_allowed_users) +{ + bool forked = false; + + 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. + 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 + } + } + } + } while (forked); + } + + return forked; +} + // Enable or disable the path effects of the item. // The counter allows nested calls static void sp_lpe_item_enable_path_effects(SPLPEItem *lpeitem, bool enable) -- cgit v1.2.3 From 7ae156aea7bf24a78d143b9645b8aa60ceb1cbb5 Mon Sep 17 00:00:00 2001 From: "Johan B. C. Engelen" Date: Wed, 26 May 2010 21:49:32 +0200 Subject: temporary fix, to stop lpe clone freeze (bzr r9452) --- src/sp-lpe-item.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/sp-lpe-item.cpp') diff --git a/src/sp-lpe-item.cpp b/src/sp-lpe-item.cpp index cc718f85e..fb5eb8799 100644 --- a/src/sp-lpe-item.cpp +++ b/src/sp-lpe-item.cpp @@ -810,6 +810,7 @@ bool sp_lpe_item_fork_path_effects_if_necessary(SPLPEItem *lpeitem, unsigned int // 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); @@ -825,7 +826,7 @@ bool sp_lpe_item_fork_path_effects_if_necessary(SPLPEItem *lpeitem, unsigned int } } } - } while (forked); + } while (forked && (maxloops-- > 1)); } return forked; -- cgit v1.2.3 From ac4a65efa875859acfaecda3b5a17ccc35bfd3cc Mon Sep 17 00:00:00 2001 From: "Johan B. C. Engelen" Date: Mon, 31 May 2010 21:54:45 +0200 Subject: 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) --- src/sp-lpe-item.cpp | 51 ++++++++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 23 deletions(-) (limited to 'src/sp-lpe-item.cpp') 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 const old_lpeobjs, + std::vector 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::const_iterator found_it(std::find(old_lpeobjs.begin(), old_lpeobjs.end(), current_lpeobj)); + if ( found_it != old_lpeobjs.end() ) { + std::vector::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 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; -- cgit v1.2.3 From 21821f40a5af994fbc77babb135fcc6b141fc297 Mon Sep 17 00:00:00 2001 From: "Johan B. C. Engelen" Date: Tue, 1 Jun 2010 23:17:09 +0200 Subject: i think this is the final fix for the LPE forking bug with clones. Fixed bugs: - https://launchpad.net/bugs/578969 (bzr r9469) --- src/sp-lpe-item.cpp | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/sp-lpe-item.cpp') diff --git a/src/sp-lpe-item.cpp b/src/sp-lpe-item.cpp index 77b2db6f6..71352ed98 100644 --- a/src/sp-lpe-item.cpp +++ b/src/sp-lpe-item.cpp @@ -814,6 +814,10 @@ bool sp_lpe_item_fork_path_effects_if_necessary(SPLPEItem *lpeitem, unsigned int // so that each object has its own independent copy of the effect. // Note: replacing path effects messes up the path effect list + // Clones of the LPEItem will increase the refcount of the lpeobjects. + // Therefore, nr_of_allowed_users should be increased with the number of clones (i.e. refs to the lpeitem) + nr_of_allowed_users += SP_OBJECT(lpeitem)->hrefcount; + std::vector 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++) -- cgit v1.2.3