=== modified file 'src/document.cpp' --- src/document.cpp 2014-03-27 01:33:44 +0000 +++ src/document.cpp 2014-06-10 23:39:40 +0000 @@ -1529,7 +1529,7 @@ /** - * Paste SVG defs from the document retrieved from the clipboard into the active document. + * Paste SVG defs from the document retrieved from the clipboard or imported document into the active document. * @param clipdoc The document to paste. * @pre @c clipdoc != NULL and pasting into the active document is possible. */ @@ -1540,27 +1540,117 @@ Inkscape::XML::Node *target_defs = this->getDefs()->getRepr(); prevent_id_clashes(source, this); - + + int stagger=0; + + /* Note, "clipboard" throughout the comments means "the document that is either the clipboard + or an imported document", as importDefs is called in both contexts. + + The order of the records in the clipboard is unpredictable and there may be both + forward and backwards references to other records within it. There may be definitions in + the clipboard that duplicate definitions in the present document OR that duplicate other + definitions in the clipboard. (Inkscape will not have created these, but they may be read + in from other SVG sources.) + + There are 3 passes to clean this up: + + In the first find and mark definitions in the clipboard that are duplicates of those in the + present document. Change the ID to "RESERVED_FOR_INKSCAPE_DUPLICATE_DEF_XXXXXXXXX". + (Inkscape will not reuse an ID, and the XXXXXXXXX keeps it from automatically creating new ones.) + References in the clipboard to the old clipboard name are converted to the name used + in the current document. + + In the second find and mark definitions in the clipboard that are duplicates of earlier + definitions in the clipbard. Unfortunately this is O(n^2) and could be very slow for a large + SVG with thousands of definitions. As before, references are adjusted to reflect the name + going forward. + + In the final cycle copy over those records not marked with that ID. + + If an SVG file uses the special ID it will cause problems! + + If this function is called because of the paste of a true clipboard the caller will have passed in a + COPY of the clipboard items. That is good, because this routine modifies that document. If the calling + behavior ever changes, so that the same document is passed in on multiple pastes, this routine will break + as in the following example: + 1. Paste clipboard containing B same as A into document containing A. Result, B is dropped + and all references to it will point to A. + 2. Paste same clipboard into a new document. It will not contain A, so there will be unsatisfied + references in that window. + */ + + std::string DuplicateDefString = "RESERVED_FOR_INKSCAPE_DUPLICATE_DEF"; + + /* First pass: remove duplicates in clipboard of definitions in document */ for (Inkscape::XML::Node *def = defs->firstChild() ; def ; def = def->next()) { - gboolean duplicate = false; + /* If this clipboard has been pasted into one document, and is now being pasted into another, + or pasted again into the same, it will already have been processed. If we detect that then + skip the rest of this pass. */ + + Glib::ustring defid = def->attribute("id"); + if( defid.find( DuplicateDefString ) != Glib::ustring::npos )break; + SPObject *src = source->getObjectByRepr(def); // Prevent duplicates of solid swatches by checking if equivalent swatch already exists if (src && SP_IS_GRADIENT(src)) { - SPGradient *gr = SP_GRADIENT(src); + SPGradient *s_gr = SP_GRADIENT(src); for (SPObject *trg = this->getDefs()->firstChild() ; trg ; trg = trg->getNext()) { - if (trg && SP_IS_GRADIENT(trg) && src != trg) { - if (gr->isEquivalent(SP_GRADIENT(trg)) && - gr->isAligned(SP_GRADIENT(trg))) { - // Change object references to the existing equivalent gradient - change_def_references(src, trg); - duplicate = true; - break; + if (trg && (src != trg) && SP_IS_GRADIENT(trg)) { + SPGradient *t_gr = SP_GRADIENT(trg); + if (t_gr && s_gr->isEquivalent(t_gr)) { + // Change object references to the existing equivalent gradient + Glib::ustring newid = trg->getId(); + if(newid != defid){ // id could be the same if it is a second paste into the same document + change_def_references(src, trg); + } + gchar *longid = g_strdup_printf("%s_%9.9d", DuplicateDefString.c_str(), stagger++); + def->setAttribute("id", longid ); + g_free(longid); + // do NOT break here, there could be more than 1 duplicate! } } } } + } + + /* Second pass: remove duplicates in clipboard of earlier definitions in clipboard */ + for (Inkscape::XML::Node *def = defs->firstChild() ; def ; def = def->next()) { + Glib::ustring defid = def->attribute("id"); + if( defid.find( DuplicateDefString ) != Glib::ustring::npos )continue; // this one already handled + SPObject *src = source->getObjectByRepr(def); + if (src && SP_IS_GRADIENT(src)) { + SPGradient *s_gr = SP_GRADIENT(src); + for (Inkscape::XML::Node *laterDef = def->next() ; laterDef ; laterDef = laterDef->next()) { + SPObject *trg = source->getObjectByRepr(laterDef); + if(trg && (src != trg) && SP_IS_GRADIENT(trg)) { + Glib::ustring newid = trg->getId(); + if( newid.find( DuplicateDefString ) != Glib::ustring::npos )continue; // this one already handled + SPGradient *t_gr = SP_GRADIENT(trg); + if (t_gr && s_gr->isEquivalent(t_gr)) { + // Change object references to the existing equivalent gradient + // two id's in the clipboard should never be the same, so always change references + change_def_references(trg, src); + gchar *longid = g_strdup_printf("%s_%9.9d", DuplicateDefString.c_str(), stagger++); + laterDef->setAttribute("id", longid ); + g_free(longid); + // do NOT break here, there could be more than 1 duplicate! + } + } + } + } + } + + /* Final pass: copy over those parts which are not duplicates */ + for (Inkscape::XML::Node *def = defs->firstChild() ; def ; def = def->next()) { + + /* Ignore duplicate defs marked in the first pass */ + Glib::ustring defid = def->attribute("id"); + if( defid.find( DuplicateDefString ) != Glib::ustring::npos )continue; + + bool duplicate = false; + SPObject *src = source->getObjectByRepr(def); // Prevent duplication of symbols... could be more clever. // The tag "_inkscape_duplicate" is added to "id" by ClipboardManagerImpl::copySymbol(). @@ -1597,7 +1687,6 @@ Inkscape::GC::release(dup); } } - } /* === modified file 'src/id-clash.cpp' --- src/id-clash.cpp 2014-04-25 05:46:24 +0000 +++ src/id-clash.cpp 2014-06-10 22:41:28 +0000 @@ -216,8 +216,7 @@ if (cd_obj && SP_IS_GRADIENT(cd_obj)) { SPGradient *cd_gr = SP_GRADIENT(cd_obj); - if ( cd_gr->isEquivalent(SP_GRADIENT(elem)) && - cd_gr->isAligned(SP_GRADIENT(elem))) { + if ( cd_gr->isEquivalent(SP_GRADIENT(elem))) { fix_clashing_ids = false; } } @@ -326,8 +325,26 @@ std::list::const_iterator it; const std::list::const_iterator it_end = pos->second.end(); for (it = pos->second.begin(); it != it_end; ++it) { - if (it->type == REF_STYLE) { + if (it->type == REF_HREF) { + gchar *new_uri = g_strdup_printf("#%s", to_obj->getId()); + it->elem->getRepr()->setAttribute(it->attr, new_uri); + g_free(new_uri); + } else if (it->type == REF_STYLE) { sp_style_set_property_url(it->elem, it->attr, to_obj, false); + } else if (it->type == REF_URL) { + gchar *url = g_strdup_printf("url(#%s)", to_obj->getId()); + it->elem->getRepr()->setAttribute(it->attr, url); + g_free(url); + } else if (it->type == REF_CLIPBOARD) { + SPCSSAttr *style = sp_repr_css_attr(it->elem->getRepr(), "style"); + gchar *url = g_strdup_printf("url(#%s)", to_obj->getId()); + sp_repr_css_set_property(style, it->attr, url); + g_free(url); + Glib::ustring style_string; + sp_repr_css_write_string(style, style_string); + it->elem->getRepr()->setAttribute("style", style_string.c_str()); + } else { + g_assert(0); // shouldn't happen } } } === modified file 'src/sp-gradient.cpp' --- src/sp-gradient.cpp 2014-03-27 01:33:44 +0000 +++ src/sp-gradient.cpp 2014-06-10 23:39:13 +0000 @@ -117,12 +117,17 @@ if (this->getStopCount() != that->getStopCount()) { break; } if (this->hasStops() != that->hasStops()) { break; } if (!this->getVector() || !that->getVector()) { break; } - if ( (SP_IS_LINEARGRADIENT(this) && SP_IS_LINEARGRADIENT(that)) || - (SP_IS_RADIALGRADIENT(this) && SP_IS_RADIALGRADIENT(that)) || - (SP_IS_MESHGRADIENT(this) && SP_IS_MESHGRADIENT(that))) { - /* OK! */ - } - else { break; } + if (this->isSwatch() != that->isSwatch()) { break; } + if ( this->isSwatch() ){ + // drop down to check stops. + } + else if ( + (SP_IS_LINEARGRADIENT(this) && SP_IS_LINEARGRADIENT(that)) || + (SP_IS_RADIALGRADIENT(this) && SP_IS_RADIALGRADIENT(that)) || + (SP_IS_MESHGRADIENT(this) && SP_IS_MESHGRADIENT(that))) { + if(!this->isAligned(that))break; + } + else { break; } // this should never happen, some unhandled type of gradient SPStop *as = this->getVector()->getFirstStop(); SPStop *bs = that->getVector()->getFirstStop(); @@ -156,6 +161,18 @@ { bool status = FALSE; + /* Some gradients have coordinates/other values specified, some don't. + yes/yes check the coordinates/other values + no/no aligned (because both have all default values) + yes/no not aligned + no/yes not aligned + It is NOT safe to just compare the computed values because if that field has + not been set the computed value could be full of garbage. + + In theory the yes/no and no/yes cases could be aligned if the specified value + matches the default value. + */ + while(1){ // not really a loop, used to avoid deep nesting or multiple exit points from function if(this->gradientTransform_set != that->gradientTransform_set) { break; } if(this->gradientTransform_set && @@ -164,31 +181,45 @@ SPLinearGradient *sg=SP_LINEARGRADIENT(this); SPLinearGradient *tg=SP_LINEARGRADIENT(that); - if( !sg->x1._set || !tg->x1._set || // assume that if these are set so will be all the others - (sg->x1.computed != tg->x1.computed) || - (sg->y1.computed != tg->y1.computed) || - (sg->x2.computed != tg->x2.computed) || - (sg->y2.computed != tg->y2.computed) - ) { break; } + if( sg->x1._set != tg->x1._set) { break; } + if( sg->y1._set != tg->y1._set) { break; } + if( sg->x2._set != tg->x2._set) { break; } + if( sg->y2._set != tg->y2._set) { break; } + if( sg->x1._set && sg->y1._set && sg->x2._set && sg->y2._set) { + if( (sg->x1.computed != tg->x1.computed) || + (sg->y1.computed != tg->y1.computed) || + (sg->x2.computed != tg->x2.computed) || + (sg->y2.computed != tg->y2.computed) ) { break; } + } else if( sg->x1._set || sg->y1._set || sg->x2._set || sg->y2._set) { break; } // some mix of set and not set + // none set? assume aligned and fall through } else if (SP_IS_RADIALGRADIENT(this) && SP_IS_LINEARGRADIENT(that)) { SPRadialGradient *sg=SP_RADIALGRADIENT(this); SPRadialGradient *tg=SP_RADIALGRADIENT(that); - if( !sg->cx._set || !tg->cx._set || // assume that if these are set so will be all the others - (sg->cx.computed != tg->cx.computed) || - (sg->cy.computed != tg->cy.computed) || - (sg->r.computed != tg->r.computed ) || - (sg->fx.computed != tg->fx.computed) || - (sg->fy.computed != tg->fy.computed) - ) { break; } + + if( sg->cx._set != tg->cx._set) { break; } + if( sg->cy._set != tg->cy._set) { break; } + if( sg->r._set != tg->r._set) { break; } + if( sg->fx._set != tg->fx._set) { break; } + if( sg->fy._set != tg->fy._set) { break; } + if( sg->cx._set && sg->cy._set && sg->fx._set && sg->fy._set && sg->r._set) { + if( (sg->cx.computed != tg->cx.computed) || + (sg->cy.computed != tg->cy.computed) || + (sg->r.computed != tg->r.computed ) || + (sg->fx.computed != tg->fx.computed) || + (sg->fy.computed != tg->fy.computed) ) { break; } + } else if( sg->cx._set || sg->cy._set || sg->fx._set || sg->fy._set || sg->r._set ) { break; } // some mix of set and not set + // none set? assume aligned and fall through } else if (SP_IS_MESHGRADIENT(this) && SP_IS_MESHGRADIENT(that)) { SPMeshGradient *sg=SP_MESHGRADIENT(this); SPMeshGradient *tg=SP_MESHGRADIENT(that); - if( !sg->x._set || !tg->x._set || - !sg->y._set || !tg->y._set || - (sg->x.computed != tg->x.computed) || - (sg->y.computed != tg->y.computed) - ) { break; } + if( sg->x._set != !tg->x._set) { break; } + if( sg->y._set != !tg->y._set) { break; } + if( sg->x._set && sg->y._set) { + if( (sg->x.computed != tg->x.computed) || + (sg->y.computed != tg->y.computed) ) { break; } + } else if( sg->x._set || sg->y._set) { break; } // some mix of set and not set + // none set? assume aligned and fall through } else { break; }