Rework smartCopy

If we are doing nonsense like composing algorithms in memory, checking for a NULL parent is not a viable way to detect when smartCopy has returned a loose transform. This is because root transforms created through algorithmCore or whatever also have NULL parents, but actually they will be deleted via QSharedPointers, so adding them to a parent chain is dangerous, and they can show up as child transforms if we say attach them as children of a stream or distribute transform as part of a parallelization scheme. So, instead we add an output parameter to smartCopy indicating if the returned pointer is a newly allocated transform, that is not already set up to be deleted.

Rework smartCopy
If we are doing nonsense like composing algorithms in memory, checking for a NULL parent is not a viable way to detect when smartCopy has returned a loose transform. This is because root transforms created through algorithmCore or whatever also have NULL parents, but actually they will be deleted via QSharedPointers, so adding them to a parent chain is dangerous, and they can show up as child transforms if we say attach them as children of a stream or distribute transform as part of a parallelization scheme. So, instead we add an output parameter to smartCopy indicating if the returned pointer is a newly allocated transform, that is not already set up to be deleted.
Charles Otto
1 parent a5768458
Showing 4 changed files with 25 additions and 15 deletions
openbr/openbr_plugin.h
openbr/plugins/meta.cpp
openbr/plugins/openbr_internal.h
openbr/plugins/stream.cpp
@@ -1209,7 +1209,9 @@ public:
      * and copy enough of their state that projectUpdate can safely be called on the original
      * instance, and the copy concurrently.
      */
-    virtual Transform * smartCopy() { return this;}
+    virtual Transform * smartCopy(bool & newTransform) { newTransform=false; return this;}
+
+    virtual Transform * smartCopy() {bool junk; return smartCopy(junk);}
     /*!
      * \brief Recursively retrieve a named event, returns NULL if an event is not found.
@@ -614,13 +614,20 @@ class DistributeTemplateTransform : public MetaTransform
 public:
-    Transform * smartCopy()
+    Transform * smartCopy(bool & newTransform)
     {
-        if (!transform->timeVarying())
+        if (!transform->timeVarying()) {
+            newTransform = false;
             return this;
+        }
+        newTransform = true;
         DistributeTemplateTransform * output = new DistributeTemplateTransform;
-        output->transform = transform->smartCopy();
+        bool newChild = false;
+        output->transform = transform->smartCopy(newChild);
+        if (newChild)
+            output->transform->setParent(output);
+
         return output;
     }
@@ -139,8 +139,9 @@ public:
      *\brief For transforms that don't do any training, this default implementation
      * which creates a new copy of the Transform from its description string is sufficient.
      */
-    virtual Transform * smartCopy()
+    virtual Transform * smartCopy(bool & newTransform)
     {
+        newTransform = true;
         return this->clone();
     }
@@ -250,10 +251,13 @@ public:
      * it creates a new copy of its own class, and gives that copy the child transforms
      * returned by calling smartCopy on this transforms children
      */
-    Transform * smartCopy()
+    Transform * smartCopy(bool & newTransform)
     {
-        if (!timeVarying())
+        if (!timeVarying()) {
+            newTransform = false;
             return this;
+        }
+        newTransform = true;
         QString name = metaObject()->className();
         name.replace("Transform","");
@@ -266,8 +270,9 @@ public:
         foreach(Transform* t, transforms )
         {
-            Transform * maybe_copy = t->smartCopy();
-            if (maybe_copy->parent() == NULL)
+            bool newItem = false;
+            Transform * maybe_copy = t->smartCopy(newItem);
+            if (newItem)
                 maybe_copy->setParent(output);
             output->transforms.append(maybe_copy);
         }
@@ -1092,10 +1092,6 @@ public:
         // dst is set to all output received by the final stage, along
         // with anything output via the calls to finalize.
-        //dst = collectionStage->getOutput();
-
-        // dst is set to all output received by the final stage, along
-        // with anything output via the calls to finalize.
         foreach(const TemplateList & list, collector->sets) {
             dst.append(list);
         }
@@ -1378,10 +1374,10 @@ public:
         basis.init();
     }
-    Transform * smartCopy()
+    Transform * smartCopy(bool & newTransform)
     {
         // We just want the DirectStream to begin with, so just return a copy of that.
-        DirectStreamTransform * res = (DirectStreamTransform *) basis.smartCopy();
+        DirectStreamTransform * res = (DirectStreamTransform *) basis.smartCopy(newTransform);
         res->activeFrames = this->activeFrames;
         return res;
     }