Fix gradient/mesh handles after object with gradient/mesh cloned.

Fixes https://bugs.launchpad.net/inkscape/+bug/453067
Fixes https://gitlab.com/inkscape/inkscape/issues/130

5 files changed, 63 insertions, 26 deletions

diff --git a/src/object/sp-object.cpp b/src/object/sp-object.cpp
index e82329d1b..764453ab3 100644
--- a/src/object/sp-object.cpp
+++ b/src/object/sp-object.cpp
@@ -267,8 +267,13 @@ SPObject *sp_object_unref(SPObject *object, SPObject *owner)
 
 void SPObject::hrefObject(SPObject* owner)
 {
-    hrefcount++;
-    _updateTotalHRefCount(1);
+    // if (owner) std::cout << "  owner: " << *owner << std::endl;
+
+    // If owner is a clone, do not increase hrefcount, it's already href'ed by original.
+    if (!owner || !owner->cloned) {
+        hrefcount++;
+        _updateTotalHRefCount(1);
+    }
 
     if(owner)
         hrefList.push_front(owner);
@@ -278,7 +283,10 @@ void SPObject::unhrefObject(SPObject* owner)
 {
     g_return_if_fail(hrefcount > 0);
 
-    hrefcount--;
+    if (!owner || !owner->cloned) {
+        hrefcount--;
+    }
+
     _updateTotalHRefCount(-1);
 
     if(owner)
@@ -1616,7 +1624,9 @@ void SPObject::recursivePrintTree( unsigned level )
     for (unsigned i = 0; i < level; ++i) {
         std::cout << "  ";
     }
-    std::cout << (getId()?getId():"No object id") << std::endl;
+    std::cout << (getId()?getId():"No object id")
+              << " clone: " << std::boolalpha << (bool)cloned
+              << " hrefcount: " << hrefcount << std::endl;
     for (auto& child: children) {
         child.recursivePrintTree(level + 1);
     }
@@ -1632,9 +1642,10 @@ void SPObject::objectTrace( std::string text, bool in, unsigned flags ) {
         std::cout << text << ":"
                   << " entrance: "
                   << (id?id:"null")
-                  << " uflags: " << uflags
-                  << " mflags: " << mflags
-                  << " flags: " << flags << std::endl;
+                  // << " uflags: " << uflags
+                  // << " mflags: " << mflags
+                  // << " flags: " << flags
+                  << std::endl;
         ++indent_level;
     } else {
         --indent_level;
@@ -1642,14 +1653,21 @@ void SPObject::objectTrace( std::string text, bool in, unsigned flags ) {
             std::cout << "  ";
         }
         std::cout << text << ":"
-                  << " exit: "
+                  << " exit:     "
                   << (id?id:"null")
-                  << " uflags: " << uflags
-                  << " mflags: " << mflags
-                  << " flags: " << flags << std::endl;
+                  // << " uflags: " << uflags
+                  // << " mflags: " << mflags
+                  // << " flags: " << flags
+                  << std::endl;
     }
 }
 
+std::ostream &operator<<(std::ostream &out, const SPObject &o)
+{
+    out << (o.getId()?o.getId():"No ID")
+        << " cloned: " << std::boolalpha << (bool)o.cloned;
+    return out;
+}
 /*
   Local Variables:
   mode:c++
diff --git a/src/object/sp-object.h b/src/object/sp-object.h
index 77e5947ad..35020a36e 100644
--- a/src/object/sp-object.h
+++ b/src/object/sp-object.h
@@ -841,6 +841,7 @@ public:
     void objectTrace( std::string, bool in=true, unsigned flags=0 );
 };
 
+std::ostream &operator<<(std::ostream &out, const SPObject &o);
 
 /**
  * Compares height of objects in tree.
diff --git a/src/object/uri-references.cpp b/src/object/uri-references.cpp
index 35c1d7e17..b257eecef 100644
--- a/src/object/uri-references.cpp
+++ b/src/object/uri-references.cpp
@@ -47,7 +47,10 @@ URIReference::URIReference(SPDocument *owner_document)
     g_assert(_owner_document != nullptr);
 }
 
-URIReference::~URIReference() { detach(); }
+URIReference::~URIReference()
+{
+    detach();
+}
 
 /*
  * The main ideas here are:
@@ -106,8 +109,6 @@ bool URIReference::_acceptObject(SPObject *obj) const
     return true;
 }
 
-
-
 void URIReference::attach(const URI &uri)
 {
     SPDocument *document = nullptr;
@@ -188,7 +189,6 @@ void URIReference::attach(const URI &uri)
 
     _setObject(document->getObjectById(id));
     _connection = document->connectIdChanged(id, sigc::mem_fun(*this, &URIReference::_setObject));
-
     g_free(id);
 }
 
@@ -213,12 +213,12 @@ void URIReference::_setObject(SPObject *obj)
     _obj = obj;
 
     _release_connection.disconnect();
-    if (_obj) {
+    if (_obj && (!_owner || !_owner->cloned)) {
         _obj->hrefObject(_owner);
         _release_connection = _obj->connectRelease(sigc::mem_fun(*this, &URIReference::_release));
     }
     _changed_signal.emit(old_obj, _obj);
-    if (old_obj) {
+    if (old_obj && (!_owner || !_owner->cloned)) {
         /* release the old object _after_ the signal emission */
         old_obj->unhrefObject(_owner);
     }
diff --git a/src/style-internal.cpp b/src/style-internal.cpp
index a1b2afd3d..80f7c1cea 100644
--- a/src/style-internal.cpp
+++ b/src/style-internal.cpp
@@ -1303,7 +1303,7 @@ SPIPaint::read( gchar const *str ) {
         if ( strneq(str, "url", 3) ) {
 
             // FIXME: THE FOLLOWING CODE SHOULD BE PUT IN A PRIVATE FUNCTION FOR REUSE
-            auto uri = extract_uri(str, &str);
+            auto uri = extract_uri(str, &str); // std::string
             if(uri.empty()) {
                 std::cerr << "SPIPaint::read: url is empty or invalid" << std::endl;
             } else if (!style ) {
@@ -1313,9 +1313,17 @@ SPIPaint::read( gchar const *str ) {
                 SPDocument *document = (style->object) ? style->object->document : nullptr;
 
                 // Create href if not done already
-                if (!value.href && document) {
-                    // std::cout << "  Creating value.href" << std::endl;
-                    value.href = new SPPaintServerReference(document);
+                if (!value.href) {
+
+                    if (style->object) {
+                        value.href = new SPPaintServerReference(style->object);
+                    } else if (document) {
+                        value.href = new SPPaintServerReference(document);
+                    } else {
+                        std::cerr << "SPIPaint::read: No valid object or document!" << std::endl;
+                        return;
+                    }
+
                     if (this == &style->fill) {
                         style->fill_ps_changed_connection = value.href->changedSignal().connect(sigc::bind(sigc::ptr_fun(sp_style_fill_paint_server_ref_changed), style));
                     } else {
diff --git a/src/style.cpp b/src/style.cpp
index b54d3feda..a985908b1 100644
--- a/src/style.cpp
+++ b/src/style.cpp
@@ -1229,11 +1229,21 @@ sp_repr_sel_eng()
 void
 sp_style_set_ipaint_to_uri(SPStyle *style, SPIPaint *paint, const Inkscape::URI *uri, SPDocument *document)
 {
-    // std::cout << "sp_style_set_ipaint_to_uri: Entrance: " << uri << "  " << (void*)document << std::endl;
-    // it may be that this style's SPIPaint has not yet created its URIReference;
-    // now that we have a document, we can create it here
-    if (!paint->value.href && document) {
-        paint->value.href = new SPPaintServerReference(document);
+    if (!paint->value.href) {
+
+        if (style->object) {
+            // Should not happen as href should have been created in SPIPaint. (TODO: Removed code duplication.)
+            paint->value.href = new SPPaintServerReference(style->object);
+
+        } else if (document) {
+            // Used by desktop style (no object to attach to!).
+            paint->value.href = new SPPaintServerReference(document);
+
+        } else {
+            std::cerr << "sp_style_set_ipaint_to_uri: No valid object or document!" << std::endl;
+            return;
+        }
+
         if (paint == &style->fill) {
             style->fill_ps_changed_connection = paint->value.href->changedSignal().connect(sigc::bind(sigc::ptr_fun(sp_style_fill_paint_server_ref_changed), style));
         } else {