Inkscape::XML::Node::addChildAtPos

9 files changed, 52 insertions, 80 deletions

diff --git a/src/extension/effect.cpp b/src/extension/effect.cpp
index a380c9710..4bca1e189 100644
--- a/src/extension/effect.cpp
+++ b/src/extension/effect.cpp
@@ -196,10 +196,12 @@ Effect::merge_menu (Inkscape::XML::Node * base,
     } // start != NULL
 
     if (tomerge != nullptr) {
-        base->appendChild(tomerge);
+        if (position != -1) {
+            base->addChildAtPos(tomerge, position);
+        } else {
+            base->appendChild(tomerge);
+        }
         Inkscape::GC::release(tomerge);
-        if (position != -1)
-            tomerge->setPosition(position);
     }
 
     if (pattern != nullptr) {
diff --git a/src/object/sp-item-group.cpp b/src/object/sp-item-group.cpp
index 77608d4cb..be09e4227 100644
--- a/src/object/sp-item-group.cpp
+++ b/src/object/sp-item-group.cpp
@@ -625,10 +625,9 @@ sp_item_group_ungroup (SPGroup *group, std::vector<SPItem*> &children, bool do_d
     for (auto i=items.rbegin();i!=items.rend();++i) {
         Inkscape::XML::Node *repr = *i;
         // add item
-        prepr->appendChild(repr);
         // restore position; since the items list was prepended (i.e. reverse), we now add
         // all children at the same pos, which inverts the order once again
-        repr->setPosition(pos > 0 ? pos : 0);
+        prepr->addChildAtPos(repr, pos);
 
         // fill in the children list if non-null
         SPItem *item = static_cast<SPItem *>(doc->getObjectByRepr(repr));
diff --git a/src/path-chemistry.cpp b/src/path-chemistry.cpp
index 4ff6d6420..a0f66b3fd 100644
--- a/src/path-chemistry.cpp
+++ b/src/path-chemistry.cpp
@@ -171,10 +171,9 @@ ObjectSet::combine(bool skip_undo)
         g_free(dstring);
 
         // add the new group to the parent of the topmost
-        parent->appendChild(repr);
-
         // move to the position of the topmost, reduced by the number of deleted items
-        repr->setPosition(position > 0 ? position : 0);
+        parent->addChildAtPos(repr, position > 0 ? position : 0);
+
         if ( !skip_undo ) {
             DocumentUndo::done(doc, SP_VERB_SELECTION_COMBINE, 
                                _("Combine"));
@@ -261,10 +260,8 @@ ObjectSet::breakApart(bool skip_undo)
             g_free(str);
             
             // add the new repr to the parent
-            parent->appendChild(repr);
-
             // move to the saved position
-            repr->setPosition(pos > 0 ? pos : 0);
+            parent->addChildAtPos(repr, pos);
 
             // if it's the first one, restore id
             if (curve == *(list.begin()))
@@ -452,10 +449,7 @@ sp_item_list_to_curves(const std::vector<SPItem*> &items, std::vector<SPItem*>&
         // restore class
         repr->setAttribute("class", class_attr);
         // add the new repr to the parent
-        parent->appendChild(repr);
-
-        // move to the saved position
-        repr->setPosition(pos > 0 ? pos : 0);
+        parent->addChildAtPos(repr, pos);
 
         /* Buglet: We don't re-add the (new version of the) object to the selection of any other
          * desktops where it was previously selected. */
diff --git a/src/selection-chemistry.cpp b/src/selection-chemistry.cpp
index 332192048..4543de57b 100755
--- a/src/selection-chemistry.cpp
+++ b/src/selection-chemistry.cpp
@@ -776,10 +776,7 @@ Inkscape::XML::Node* ObjectSet::group() {
     }
 
     // Add the new group to the topmost members' parent
-    topmost_parent->appendChild(group);
-
-    // Move to the position of the topmost, reduced by the number of items deleted from topmost_parent
-    group->setPosition(topmost + 1);
+    topmost_parent->addChildAtPos(group, topmost + 1);
 
     set(doc->getObjectByRepr(group));
     DocumentUndo::done(doc, SP_VERB_SELECTION_GROUP,
@@ -3172,9 +3169,8 @@ void ObjectSet::toSymbol()
 
         gchar * title = the_group->title();
         if (title) {
-            symbol_repr->appendChild(title_repr);
+            symbol_repr->addChildAtPos(title_repr, 0);
             title_repr->appendChild(xml_doc->createTextNode(title));
-            title_repr->setPosition(0);
             Inkscape::GC::release(title_repr);
         }
         g_free(title);
@@ -3184,8 +3180,7 @@ void ObjectSet::toSymbol()
             Inkscape::XML::Node *desc_repr = xml_doc->createElement("svg:desc");
             desc_repr->setContent(desc);
             desc_repr->appendChild(xml_doc->createTextNode(desc));
-            symbol_repr->appendChild(desc_repr);
-            desc_repr->setPosition(1);
+            symbol_repr->addChildAtPos(desc_repr, 1);
             Inkscape::GC::release(desc_repr);
         }
         g_free(desc);
@@ -3208,17 +3203,15 @@ void ObjectSet::toSymbol()
     for (std::vector<SPObject*>::const_reverse_iterator i=items_.rbegin();i!=items_.rend();++i){
         gchar* title = (*i)->title();
         if (!single_group && !settitle && title) {
-            symbol_repr->appendChild(title_repr);
+            symbol_repr->addChildAtPos(title_repr, 0);
             title_repr->appendChild(xml_doc->createTextNode(title));
-            title_repr->setPosition(0);
             Inkscape::GC::release(title_repr);
             gchar * desc = (*i)->desc();
             if (desc) {
                 Inkscape::XML::Node *desc_repr = xml_doc->createElement("svg:desc");
                 desc_repr->appendChild(xml_doc->createTextNode(desc));
-                symbol_repr->appendChild(desc_repr);
+                symbol_repr->addChildAtPos(desc_repr, 1);
                 Inkscape::GC::release(desc_repr);
-                desc_repr->setPosition(1);
             }
             g_free(desc);
             settitle = true;
@@ -3431,8 +3424,7 @@ void ObjectSet::tile(bool apply)
         sp_repr_set_svg_double(rect, "y", bbox.top());
 
         // restore parent and position
-        parent->getRepr()->appendChild(rect);
-        rect->setPosition(pos > 0 ? pos : 0);
+        parent->getRepr()->addChildAtPos(rect, pos);
         SPItem *rectangle = static_cast<SPItem *>(document()->getObjectByRepr(rect));
 
         Inkscape::GC::release(rect);
@@ -3773,10 +3765,7 @@ void ObjectSet::createBitmapCopy()
         g_free(c);
 
         // add the new repr to the parent
-        parent->appendChild(repr);
-
-        // move to the saved position
-        repr->setPosition(pos > 0 ? pos + 1 : 1);
+        parent->addChildAtPos(repr, pos + 1);
 
         // Set selection to the new image
         clear();
@@ -3873,8 +3862,7 @@ void ObjectSet::setClipGroup()
 
     Inkscape::XML::Node *outer = xml_doc->createElement("svg:g");
     outer->appendChild(inner);
-    topmost_parent->appendChild(outer);
-    outer->setPosition(topmost + 1);
+    topmost_parent->addChildAtPos(outer, topmost + 1);
 
     Inkscape::XML::Node *clone = xml_doc->createElement("svg:use");
     clone->setAttribute("x", "0", false);
@@ -4049,11 +4037,9 @@ void ObjectSet::setClipGroup()
             group->setAttribute("inkscape:groupmode", "maskhelper");
 
             Inkscape::XML::Node *spnew = current->duplicate(xml_doc);
-            gint position = current->position();
-            current->parent()->appendChild(group);
+            current->parent()->addChild(group, current);
             sp_repr_unparent(current);
             group->appendChild(spnew);
-            group->setPosition(position);
 
             // Apply clip/mask to group instead
             apply_mask_to = group;
@@ -4185,15 +4171,14 @@ void ObjectSet::unsetMask(const bool apply_clip_path, const bool skip_undo) {
 
         // remember parent and position of the item to which the clippath/mask was applied
         Inkscape::XML::Node *parent = (referenced_object.second)->getRepr()->parent();
-        gint pos = (referenced_object.second)->getRepr()->position();
+        Inkscape::XML::Node *ref_repr = referenced_object.second->getRepr();
 
         // Iterate through all clipped paths / masks
         for (auto i=items_to_move.rbegin();i!=items_to_move.rend();++i) {
             Inkscape::XML::Node *repr = *i;
 
             // insert into parent, restore pos
-            parent->appendChild(repr);
-            repr->setPosition((pos + 1) > 0 ? (pos + 1) : 0);
+            parent->addChild(repr, ref_repr);
 
             SPItem *mask_item = static_cast<SPItem *>(document()->getObjectByRepr(repr));
             items_to_select.push_back(mask_item);
diff --git a/src/seltrans.cpp b/src/seltrans.cpp
index c173c9474..3773219a3 100644
--- a/src/seltrans.cpp
+++ b/src/seltrans.cpp
@@ -549,17 +549,13 @@ void Inkscape::SelTrans::stamp()
             SPItem *original_item = *x;
             Inkscape::XML::Node *original_repr = original_item->getRepr();
 
-            // remember the position of the item
-            gint pos = original_repr->position();
             // remember parent
             Inkscape::XML::Node *parent = original_repr->parent();
 
             Inkscape::XML::Node *copy_repr = original_repr->duplicate(parent->document());
 
             // add the new repr to the parent
-            parent->appendChild(copy_repr);
-            // move to the saved position
-            copy_repr->setPosition(pos > 0 ? pos : 0);
+            parent->addChild(copy_repr, original_repr->prev());
 
             SPItem *copy_item = (SPItem *) _desktop->getDocument()->getObjectByRepr(copy_repr);
 
diff --git a/src/splivarot.cpp b/src/splivarot.cpp
index 1a61b4e3d..ebd6b3c7e 100644
--- a/src/splivarot.cpp
+++ b/src/splivarot.cpp
@@ -797,10 +797,8 @@ BoolOpErrors Inkscape::ObjectSet::pathBoolOp(bool_op bop, const bool skip_undo,
             repr->setAttribute("transform", transform);
 
             // add the new repr to the parent
-            parent->appendChild(repr);
-
             // move to the saved position
-            repr->setPosition(pos > 0 ? pos : 0);
+            parent->addChildAtPos(repr, pos);
 
             add(doc->getObjectByRepr(repr));
             Inkscape::GC::release(repr);
@@ -825,8 +823,7 @@ BoolOpErrors Inkscape::ObjectSet::pathBoolOp(bool_op bop, const bool skip_undo,
 
         repr->setAttribute("transform", transform);
 
-        parent->appendChild(repr);
-        repr->setPosition(pos > 0 ? pos : 0);
+        parent->addChildAtPos(repr, pos);
 
         add(doc->getObjectByRepr(repr));
         Inkscape::GC::release(repr);
@@ -862,9 +859,7 @@ void sp_selected_path_outline_add_marker( SPObject *marker_object, Geom::Affine
 
     if (marker_item->getRepr()) {
         Inkscape::XML::Node *m_repr = marker_item->getRepr()->duplicate(xml_doc);
-        g_repr->appendChild(m_repr);
-        //There is a special group to markers with this reverse the order in clussion
-        m_repr->setPosition(0);
+        g_repr->addChildAtPos(m_repr, 0);
         SPItem *marker_item = (SPItem *) doc->getObjectByRepr(m_repr);
         marker_item->doWriteTransform(tr);
         if (!legacy) {
@@ -1339,9 +1334,8 @@ sp_item_path_outline(SPItem *item, SPDesktop *desktop, bool legacy)
                 g_repr->setAttribute("style", nullptr);
 
                 // add the group to the parent
-                parent->appendChild(g_repr);
                 // move to the saved position
-                g_repr->setPosition(pos > 0 ? pos : 0);
+                parent->addChildAtPos(g_repr, pos);
 
                 //The fill
                 Inkscape::XML::Node *fill = nullptr;
@@ -1368,8 +1362,7 @@ sp_item_path_outline(SPItem *item, SPDesktop *desktop, bool legacy)
                 if(SP_SHAPE(item)->hasMarkers ()) {
                     if (!legacy) {
                         markers = xml_doc->createElement("svg:g");
-                        g_repr->appendChild(markers);
-                        markers->setPosition(pos > 0 ? pos : 0);
+                        g_repr->addChildAtPos(markers, pos);
                     } else {
                         markers = g_repr;
                     }
@@ -1555,9 +1548,9 @@ sp_item_path_outline(SPItem *item, SPDesktop *desktop, bool legacy)
 
                 sp_repr_css_attr_unref(r_style);
                 if (unique) {
-                    parent->appendChild(out);
+                    g_assert(out != g_repr);
+                    parent->addChild(out, g_repr);
                     parent->removeChild(g_repr);
-                    out->setPosition(pos > 0 ? pos : 0);
                 }
                 out->setAttribute("transform", item->getRepr()->attribute("transform"));
                 //bug lp:1290573 : completely destroy the old object first
@@ -1812,10 +1805,8 @@ void sp_selected_path_create_offset_object(SPDesktop *desktop, int expand, bool
         }
 
         // add the new repr to the parent
-        parent->appendChild(repr);
-
         // move to the saved position
-        repr->setPosition(pos > 0 ? pos : 0);
+        parent->addChildAtPos(repr, pos);
 
         SPItem *nitem = reinterpret_cast<SPItem *>(desktop->getDocument()->getObjectByRepr(repr));
 
@@ -2021,10 +2012,8 @@ sp_selected_path_do_offset(SPDesktop *desktop, bool expand, double prefOffset)
             g_free(str);
 
             // add the new repr to the parent
-            parent->appendChild(repr);
-
             // move to the saved position
-            repr->setPosition(pos > 0 ? pos : 0);
+            parent->addChildAtPos(repr, pos);
 
             SPItem *newitem = (SPItem *) desktop->getDocument()->getObjectByRepr(repr);
 
@@ -2138,10 +2127,8 @@ sp_selected_path_simplify_item(SPDesktop *desktop,
     g_free(str);
 
     // add the new repr to the parent
-    parent->appendChild(repr);
-
     // move to the saved position
-    repr->setPosition(pos > 0 ? pos : 0);
+    parent->addChildAtPos(repr, pos);
 
     SPItem *newitem = (SPItem *) desktop->getDocument()->getObjectByRepr(repr);
 
diff --git a/src/ui/tools/tweak-tool.cpp b/src/ui/tools/tweak-tool.cpp
index 4e64ed4de..0c5e83b55 100644
--- a/src/ui/tools/tweak-tool.cpp
+++ b/src/ui/tools/tweak-tool.cpp
@@ -601,9 +601,8 @@ sp_tweak_dilate_recursive (Inkscape::Selection *selection, SPItem *item, Geom::P
                     // restore id
                     newrepr->setAttribute("id", id);
                     // add the new repr to the parent
-                    parent->appendChild(newrepr);
                     // move to the saved position
-                    newrepr->setPosition(pos > 0 ? pos : 0);
+                    parent->addChildAtPos(newrepr, pos);
 
                     if (is_selected)
                         selection->add(newrepr);
diff --git a/src/xml/node.h b/src/xml/node.h
index c32508eec..17e09b5fa 100644
--- a/src/xml/node.h
+++ b/src/xml/node.h
@@ -351,7 +351,21 @@ public:
      * @param after The node after which the inserted node should be placed, or NULL
      */
     virtual void addChild(Node *child, Node *after)=0;
-    
+
+    /**
+     * @brief Insert another node as a child of this node
+     *
+     * This is more efficient than appendChild() + setPosition().
+     *
+     * @param child The node to insert
+     * @param pos The position in parent's child order
+     */
+    void addChildAtPos(Node *child, unsigned pos)
+    {
+        Node *after = (pos == 0) ? nullptr : nthChild(pos - 1);
+        addChild(child, after);
+    }
+
     /**
      * @brief Append a node as the last child of this node
      * @param child The node to append
diff --git a/src/xml/simple-node.cpp b/src/xml/simple-node.cpp
index b628d678a..47f66ff85 100644
--- a/src/xml/simple-node.cpp
+++ b/src/xml/simple-node.cpp
@@ -773,21 +773,17 @@ void SimpleNode::mergeFrom(Node const *src, gchar const *key, bool extension, bo
             Node *rch=sp_repr_lookup_child(this, key, id);
             if (rch && (!extension || rch->equal(child, false))) {
                 rch->mergeFrom(child, key, extension);
+                continue;
             } else {
                 if(rch) {
                     removeChild(rch);
                 }
-                guint pos = child->position();
-                rch = child->duplicate(_document);
-                appendChild(rch);
-                rch->setPosition(pos);
-                rch->release();
             }
-        } else {
+        }
+        {
             guint pos = child->position();
             Node *rch=child->duplicate(_document);
-            appendChild(rch);
-            rch->setPosition(pos);
+            addChildAtPos(rch, pos);
             rch->release();
         }
     }