changeset 1761:ee7fd2c01d87 by-id

Work on management of alignment-related models

--- a/base/ById.cpp	Mon Jul 08 10:08:08 2019 +0100
+++ b/base/ById.cpp	Fri Jul 12 13:57:42 2019 +0100
@@ -70,6 +70,8 @@
         if (id == IdAlloc::NO_ID) {
             throw std::logic_error("item id should never be NO_ID");
         }
+        SVCERR << "ById::add(#" << id << ") of type "
+               << typeid(*item.get()).name() << endl;
         QMutexLocker locker(&m_mutex);
         if (m_items.find(id) != m_items.end()) {
             SVCERR << "ById::add: item with id " << id
@@ -87,7 +89,7 @@
         if (id == IdAlloc::NO_ID) {
             return;
         }
-        SVCERR << "ById::release(" << id << ")" << endl;
+        SVCERR << "ById::release(#" << id << ")" << endl;
         QMutexLocker locker(&m_mutex);
         if (m_items.find(id) == m_items.end()) {
             SVCERR << "ById::release: unknown item id " << id << endl;

--- a/data/model/AlignmentModel.h	Mon Jul 08 10:08:08 2019 +0100
+++ b/data/model/AlignmentModel.h	Fri Jul 12 13:57:42 2019 +0100
@@ -82,7 +82,8 @@
     ModelId m_reference;
     ModelId m_aligned;
 
-    ModelId m_pathSource; // a SparseTimeValueModel
+    ModelId m_pathSource; // a SparseTimeValueModel, which we need a
+                          // handle on only while it's still being generated
 
     mutable std::unique_ptr<Path> m_path;
     mutable std::unique_ptr<Path> m_reversePath;

--- a/data/model/Model.cpp	Mon Jul 08 10:08:08 2019 +0100
+++ b/data/model/Model.cpp	Fri Jul 12 13:57:42 2019 +0100
@@ -25,12 +25,6 @@
 Model::~Model()
 {
     SVDEBUG << "Model::~Model: " << this << " with id " << getId() << endl;
-    //!!! see notes in header - sort this out
-    /*
-    if (!m_alignmentModel.isNone()) {
-        ModelById::release(m_alignmentModel);
-    }
-    */
 }
 
 void
@@ -50,15 +44,15 @@
 {
     SVDEBUG << "Model(" << this << "): accepting alignment model "
             << alignmentModel << endl;
-    
-    if (!m_alignmentModel.isNone()) {
-        ModelById::release(m_alignmentModel);
+
+    if (auto model = ModelById::get(m_alignmentModel)) {
+        disconnect(model.get(), SIGNAL(completionChanged(ModelId)),
+                   this, SIGNAL(alignmentCompletionChanged(ModelId)));
     }
     
     m_alignmentModel = alignmentModel;
 
-    auto model = ModelById::get(m_alignmentModel);
-    if (model) {
+    if (auto model = ModelById::get(m_alignmentModel)) {
         connect(model.get(), SIGNAL(completionChanged(ModelId)),
                 this, SIGNAL(alignmentCompletionChanged(ModelId)));
     }

--- a/data/model/Model.h	Mon Jul 08 10:08:08 2019 +0100
+++ b/data/model/Model.h	Fri Jul 12 13:57:42 2019 +0100
@@ -200,13 +200,7 @@
     /**
      * Specify an alignment between this model's timeline and that of
      * a reference model. The alignment model, of type AlignmentModel,
-     * records both the reference and the alignment. This model "takes
-     * ownership" of alignmentModel, in that we take responsibility
-     * for calling ModelById::release() for it from our own destructor
-     * (no other class needs to know about the alignment model).
-
-     *!!! I don't think the above is a good idea - I think document
-          should record alignment models and release them
+     * records both the reference and the alignment.
      */
     virtual void setAlignment(ModelId alignmentModel);