# HG changeset patch
# User Chris Cannam
# Date 1561472974 -3600
# Node ID d91ff235e69d30d175337af3d8ef5d0bc88b1a93
# Parent  c3b5564cfb782561e029407df5b5a47f9269a549
Some messing with Model and AlignmentModel

diff -r c3b5564cfb78 -r d91ff235e69d base/ById.h
--- a/base/ById.h	Mon Jun 24 14:28:17 2019 +0100
+++ b/base/ById.h	Tue Jun 25 15:29:34 2019 +0100
@@ -26,10 +26,24 @@
 
 template <typename T>
 struct SvId {
+    
     int id;
 
+    enum {
+        // The value NO_ID (-1) is never allocated by WithId
+        NO_ID = -1
+    };
+    
+    SvId() : id(NO_ID) {}
+
+    SvId(const SvId &) =default;
+    SvId &operator=(const SvId &) =default;
+
+    bool operator==(const SvId &other) const { return id == other.id; }
     bool operator<(const SvId &other) const { return id < other.id; }
 
+    bool isNone() const { return id == NO_ID; }
+
     QString toString() const {
         return QString("%1").arg(id);
     }
@@ -66,8 +80,12 @@
         int i = nextId;
         if (nextId == INT_MAX) {
             nextId = INT_MIN;
+        } else {
+            ++nextId;
+            if (nextId == 0 || nextId == Id::NO_ID) {
+                throw std::runtime_error("Internal ID limit exceeded!");
+            }
         }
-        ++nextId;
         return i;
     }
 };
diff -r c3b5564cfb78 -r d91ff235e69d data/model/AlignmentModel.cpp
--- a/data/model/AlignmentModel.cpp	Mon Jun 24 14:28:17 2019 +0100
+++ b/data/model/AlignmentModel.cpp	Tue Jun 25 15:29:34 2019 +0100
@@ -19,34 +19,37 @@
 
 //#define DEBUG_ALIGNMENT_MODEL 1
 
-AlignmentModel::AlignmentModel(Model *reference,
-                               Model *aligned,
-                               SparseTimeValueModel *path) :
+AlignmentModel::AlignmentModel(ModelId reference,
+                               ModelId aligned,
+                               ModelId pathSource) :
     m_reference(reference),
     m_aligned(aligned),
-    m_rawPath(path),
+    m_pathSource(pathSource),
     m_path(nullptr),
     m_reversePath(nullptr),
     m_pathBegun(false),
     m_pathComplete(false)
 {
-    if (m_rawPath) {
+    auto pathSourceModel = ModelById::getAs<SparseTimeValueModel>(pathSource);
+    
+    if (pathSourceModel) {
 
-        connect(m_rawPath, SIGNAL(modelChanged()),
-                this, SLOT(pathChanged()));
+        connect(pathSourceModel.get(), SIGNAL(modelChanged()),
+                this, SLOT(pathSourceChanged()));
 
-        connect(m_rawPath, SIGNAL(modelChangedWithin(sv_frame_t, sv_frame_t)),
-                this, SLOT(pathChangedWithin(sv_frame_t, sv_frame_t)));
+        connect(pathSourceModel.get(),
+                SIGNAL(modelChangedWithin(sv_frame_t, sv_frame_t)),
+                this, SLOT(pathSourceChangedWithin(sv_frame_t, sv_frame_t)));
         
-        connect(m_rawPath, SIGNAL(completionChanged()),
-                this, SLOT(pathCompletionChanged()));
+        connect(pathSourceModel.get(), SIGNAL(completionChanged()),
+                this, SLOT(pathSourceCompletionChanged()));
 
         constructPath();
         constructReversePath();
-    }
 
-    if (m_rawPath && m_rawPath->isReady()) {
-        pathCompletionChanged();
+        if (pathSourceModel->isReady()) {
+            pathSourceCompletionChanged();
+        }
     }
 
     if (m_reference == m_aligned) {
@@ -63,21 +66,21 @@
     SVCERR << "AlignmentModel(" << this << ")::~AlignmentModel()" << endl;
 #endif
 
-    if (m_rawPath) m_rawPath->aboutToDelete();
-    delete m_rawPath;
+//!!!    if (m_pathSource) m_pathSource->aboutToDelete();
+//    delete m_pathSource;
 
-    if (m_path) m_path->aboutToDelete();
-    delete m_path;
+//    if (m_path) m_path->aboutToDelete();
+//    delete m_path;
 
-    if (m_reversePath) m_reversePath->aboutToDelete();
-    delete m_reversePath;
+//    if (m_reversePath) m_reversePath->aboutToDelete();
+//    delete m_reversePath;
 }
 
 bool
 AlignmentModel::isOK() const
 {
     if (m_error != "") return false;
-    if (m_rawPath) return m_rawPath->isOK();
+    if (m_pathSource) return m_pathSource->isOK();
     return true;
 }
 
@@ -106,7 +109,7 @@
 bool
 AlignmentModel::isReady(int *completion) const
 {
-    if (!m_pathBegun && m_rawPath) {
+    if (!m_pathBegun && m_pathSource) {
         if (completion) *completion = 0;
 #ifdef DEBUG_ALIGNMENT_MODEL
         SVCERR << "AlignmentModel::isReady: path not begun" << endl;
@@ -120,7 +123,7 @@
 #endif
         return true;
     }
-    if (!m_rawPath) {
+    if (!m_pathSource) {
         // lack of raw path could mean path is complete (in which case
         // m_pathComplete true above) or else no alignment has been
         // set at all yet (this case)
@@ -130,7 +133,7 @@
 #endif
         return false;
     }
-    return m_rawPath->isReady(completion);
+    return m_pathSource->isReady(completion);
 }
 
 const ZoomConstraint *
@@ -158,7 +161,7 @@
     cerr << "AlignmentModel::toReference(" << frame << ")" << endl;
 #endif
     if (!m_path) {
-        if (!m_rawPath) return frame;
+        if (!m_pathSource) return frame;
         constructPath();
     }
     return align(m_path, frame);
@@ -171,25 +174,25 @@
     cerr << "AlignmentModel::fromReference(" << frame << ")" << endl;
 #endif
     if (!m_reversePath) {
-        if (!m_rawPath) return frame;
+        if (!m_pathSource) return frame;
         constructReversePath();
     }
     return align(m_reversePath, frame);
 }
 
 void
-AlignmentModel::pathChanged()
+AlignmentModel::pathSourceChanged()
 {
     if (m_pathComplete) {
         cerr << "AlignmentModel: deleting raw path model" << endl;
-        if (m_rawPath) m_rawPath->aboutToDelete();
-        delete m_rawPath;
-        m_rawPath = nullptr;
+        if (m_pathSource) m_pathSource->aboutToDelete();
+        delete m_pathSource;
+        m_pathSource = nullptr;
     }
 }
 
 void
-AlignmentModel::pathChangedWithin(sv_frame_t, sv_frame_t)
+AlignmentModel::pathSourceChangedWithin(sv_frame_t, sv_frame_t)
 {
     if (!m_pathComplete) return;
     constructPath();
@@ -197,15 +200,15 @@
 }    
 
 void
-AlignmentModel::pathCompletionChanged()
+AlignmentModel::pathSourceCompletionChanged()
 {
-    if (!m_rawPath) return;
+    if (!m_pathSource) return;
     m_pathBegun = true;
 
     if (!m_pathComplete) {
 
         int completion = 0;
-        m_rawPath->isReady(&completion);
+        m_pathSource->isReady(&completion);
 
 #ifdef DEBUG_ALIGNMENT_MODEL
         SVCERR << "AlignmentModel::pathCompletionChanged: completion = "
@@ -232,20 +235,20 @@
 AlignmentModel::constructPath() const
 {
     if (!m_path) {
-        if (!m_rawPath) {
+        if (!m_pathSource) {
             cerr << "ERROR: AlignmentModel::constructPath: "
                       << "No raw path available" << endl;
             return;
         }
         m_path = new PathModel
-            (m_rawPath->getSampleRate(), m_rawPath->getResolution(), false);
+            (m_pathSource->getSampleRate(), m_pathSource->getResolution(), false);
     } else {
-        if (!m_rawPath) return;
+        if (!m_pathSource) return;
     }
         
     m_path->clear();
 
-    EventVector points = m_rawPath->getAllEvents();
+    EventVector points = m_pathSource->getAllEvents();
 
     for (const auto &p: points) {
         sv_frame_t frame = p.getFrame();
@@ -368,28 +371,28 @@
 void
 AlignmentModel::setPathFrom(SparseTimeValueModel *rawpath)
 {
-    if (m_rawPath) m_rawPath->aboutToDelete();
-    delete m_rawPath;
+    if (m_pathSource) m_pathSource->aboutToDelete();
+    delete m_pathSource;
 
-    m_rawPath = rawpath;
+    m_pathSource = rawpath;
 
-    if (!m_rawPath) {
+    if (!m_pathSource) {
         return;
     }
 
-    connect(m_rawPath, SIGNAL(modelChanged()),
+    connect(m_pathSource, SIGNAL(modelChanged()),
             this, SLOT(pathChanged()));
 
-    connect(m_rawPath, SIGNAL(modelChangedWithin(sv_frame_t, sv_frame_t)),
+    connect(m_pathSource, SIGNAL(modelChangedWithin(sv_frame_t, sv_frame_t)),
             this, SLOT(pathChangedWithin(sv_frame_t, sv_frame_t)));
         
-    connect(m_rawPath, SIGNAL(completionChanged()),
+    connect(m_pathSource, SIGNAL(completionChanged()),
             this, SLOT(pathCompletionChanged()));
     
     constructPath();
     constructReversePath();
 
-    if (m_rawPath->isReady()) {
+    if (m_pathSource->isReady()) {
         pathCompletionChanged();
     }        
 }
diff -r c3b5564cfb78 -r d91ff235e69d data/model/AlignmentModel.h
--- a/data/model/AlignmentModel.h	Mon Jun 24 14:28:17 2019 +0100
+++ b/data/model/AlignmentModel.h	Tue Jun 25 15:29:34 2019 +0100
@@ -30,9 +30,9 @@
     Q_OBJECT
 
 public:
-    AlignmentModel(Model *reference,
-                   Model *aligned,
-                   SparseTimeValueModel *path);
+    AlignmentModel(ModelId reference /* any model */,
+                   ModelId aligned /* any model */,
+                   ModelId path /* a SparseTimeValueModel */);
     ~AlignmentModel();
 
     bool isOK() const override;
@@ -53,14 +53,14 @@
 
     QString getTypeName() const override { return tr("Alignment"); }
 
-    const Model *getReferenceModel() const;
-    const Model *getAlignedModel() const;
+    ModelId getReferenceModel() const;
+    ModelId getAlignedModel() const;
 
     sv_frame_t toReference(sv_frame_t frame) const;
     sv_frame_t fromReference(sv_frame_t frame) const;
 
-    void setPathFrom(SparseTimeValueModel *rawpath);
-    void setPath(PathModel *path);
+    void setPathFrom(ModelId pathSource); // a SparseTimeValueModel
+    void setPath(const PathModel &path);
 
     void toXml(QTextStream &stream,
                        QString indent = "",
@@ -77,17 +77,18 @@
     void completionChanged();
 
 protected slots:
-    void pathChanged();
-    void pathChangedWithin(sv_frame_t startFrame, sv_frame_t endFrame);
-    void pathCompletionChanged();
+    void pathSourceChanged();
+    void pathSourceChangedWithin(sv_frame_t startFrame, sv_frame_t endFrame);
+    void pathSourceCompletionChanged();
 
 protected:
-    Model *m_reference; // I don't own this
-    Model *m_aligned; // I don't own this
+    ModelId m_reference;
+    ModelId m_aligned;
 
-    SparseTimeValueModel *m_rawPath; // I own this
-    mutable PathModel *m_path; // I own this
-    mutable PathModel *m_reversePath; // I own this
+    ModelId m_pathSource; // a SparseTimeValueModel
+
+    std::unique_ptr<PathModel> m_path;
+    std::unique_ptr<PathModel> m_reversePath;
     bool m_pathBegun;
     bool m_pathComplete;
     QString m_error;
@@ -95,7 +96,7 @@
     void constructPath() const;
     void constructReversePath() const;
 
-    sv_frame_t align(PathModel *path, sv_frame_t frame) const;
+    sv_frame_t align(const PathModel &path, sv_frame_t frame) const;
 };
 
 #endif
diff -r c3b5564cfb78 -r d91ff235e69d data/model/Model.cpp
--- a/data/model/Model.cpp	Mon Jun 24 14:28:17 2019 +0100
+++ b/data/model/Model.cpp	Tue Jun 25 15:29:34 2019 +0100
@@ -34,30 +34,40 @@
                 << endl;
     }
 
-    if (m_alignment) {
-        m_alignment->aboutToDelete();
-        delete m_alignment;
+    if (!m_alignmentModel.isNone()) {
+        ModelById::release(m_alignmentModel);
     }
 }
 
 void
-Model::setSourceModel(Model *model)
+Model::setSourceModel(ModelId modelId)
 {
+/*!!!
     if (m_sourceModel) {
         disconnect(m_sourceModel, SIGNAL(aboutToBeDeleted()),
                    this, SLOT(sourceModelAboutToBeDeleted()));
     }
+*/
+    
+    m_sourceModel = modelId;
 
-    m_sourceModel = model;
-
+    auto model = ModelById::get(m_sourceModel);
+    if (model) {
+        connect(model.get(), SIGNAL(alignmentCompletionChanged()),
+                this, SIGNAL(alignmentCompletionChanged()));
+    }
+        
+    
+/*
     if (m_sourceModel) {
         connect(m_sourceModel, SIGNAL(alignmentCompletionChanged()),
                 this, SIGNAL(alignmentCompletionChanged()));
         connect(m_sourceModel, SIGNAL(aboutToBeDeleted()),
                 this, SLOT(sourceModelAboutToBeDeleted()));
     }
+*/
 }
-
+/*!!!
 void
 Model::aboutToDelete()
 {
@@ -83,40 +93,38 @@
 {
     m_sourceModel = nullptr;
 }
-
+*/
 void
-Model::setAlignment(AlignmentModel *alignment)
+Model::setAlignment(ModelId alignmentModel)
 {
     SVDEBUG << "Model(" << this << "): accepting alignment model "
-            << alignment << endl;
+            << alignmentModel << endl;
     
-    if (m_alignment) {
-        m_alignment->aboutToDelete();
-        delete m_alignment;
+    if (!m_alignmentModel.isNone()) {
+        ModelById::release(m_alignmentModel);
     }
     
-    m_alignment = alignment;
+    m_alignmentModel = alignmentModel;
 
-    if (m_alignment) {
-        connect(m_alignment, SIGNAL(completionChanged()),
+    auto model = ModelById::get(m_alignmentModel);
+    if (model) {
+        connect(model.get(), SIGNAL(completionChanged()),
                 this, SIGNAL(alignmentCompletionChanged()));
     }
 }
 
-const AlignmentModel *
+const ModelId
 Model::getAlignment() const
 {
-    return m_alignment;
+    return m_alignmentModel;
 }
 
-const Model *
+const ModelId
 Model::getAlignmentReference() const
 {
-    if (!m_alignment) {
-        if (m_sourceModel) return m_sourceModel->getAlignmentReference();
-        return nullptr;
-    }
-    return m_alignment->getReferenceModel();
+    auto model = ModelById::getAs<AlignmentModel>(m_alignmentModel);
+    if (model) return model->getReferenceModel();
+    else return {};
 }
 
 sv_frame_t
diff -r c3b5564cfb78 -r d91ff235e69d data/model/Model.h
--- a/data/model/Model.h	Mon Jun 24 14:28:17 2019 +0100
+++ b/data/model/Model.h	Tue Jun 25 15:29:34 2019 +0100
@@ -40,6 +40,8 @@
     Q_OBJECT
 
 public:
+    typedef Id ModelId;
+    
     virtual ~Model();
 
     /**
@@ -139,17 +141,22 @@
      * to this will depend on the model's context -- it's possible
      * nothing at all will change.
      */
+    //!!! aim to lose this
+    /*!!!
     virtual void abandon() {
         m_abandoning = true;
     }
-
+    */
+    
     /**
      * Query whether the model has been marked as abandoning.
      */
+    //!!! aim to lose this
+    /*!!!
     virtual bool isAbandoning() const { 
         return m_abandoning;
     }
-
+    */
     /**
      * Return true if the model has finished loading or calculating
      * all its data, for a model that is capable of calculating in a
@@ -200,28 +207,33 @@
     }
 
     /**
-     * If this model was derived from another, return the model it was
-     * derived from.  The assumption is that the source model's
-     * alignment will also apply to this model, unless some other
-     * property (such as a specific alignment model set on this model)
-     * indicates otherwise.
+     * If this model was derived from another, return the id of the
+     * model it was derived from.  The assumption is that the source
+     * model's alignment will also apply to this model, unless some
+     * other property (such as a specific alignment model set on this
+     * model) indicates otherwise.
      */
-    virtual Model *getSourceModel() const {
+    virtual ModelId getSourceModel() const {
         return m_sourceModel;
     }
 
     /**
      * Set the source model for this model.
      */
-    virtual void setSourceModel(Model *model);
+    virtual void setSourceModel(ModelId model);
 
     /**
-     * Specify an aligment between this model's timeline and that of a
-     * reference model.  The alignment model records both the
-     * reference and the alignment.  This model takes ownership of the
-     * alignment model.
+     * 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
      */
-    virtual void setAlignment(AlignmentModel *alignment);
+    virtual void setAlignment(ModelId alignmentModel);
 
     /**
      * Retrieve the alignment model for this model.  This is not a
@@ -232,13 +244,13 @@
      * application for this function is in streaming out alignments to
      * the session file.
      */
-    virtual const AlignmentModel *getAlignment() const;
+    virtual const ModelId getAlignment() const;
 
     /**
      * Return the reference model for the current alignment timeline,
      * if any.
      */
-    virtual const Model *getAlignmentReference() const;
+    virtual const ModelId getAlignmentReference() const;
 
     /**
      * Return the frame number of the reference model that corresponds
@@ -282,10 +294,12 @@
                                           sv_frame_t startFrame,
                                           sv_frame_t duration) const = 0;
 
+    /*!!!
 public slots:
     void aboutToDelete();
     void sourceModelAboutToBeDeleted();
-
+    */
+    
 signals:
     /**
      * Emitted when a model has been edited (or more data retrieved
@@ -328,29 +342,27 @@
      * will be emitted before the actual deletion.
      */
     //!!! our goal is to get rid of (the need for) this
-    void aboutToBeDeleted();
+//!!!    void aboutToBeDeleted();
 
 protected:
     Model() :
-        m_sourceModel(0), 
-        m_alignment(0), 
-        m_abandoning(false), 
-        m_aboutToDelete(false),
+//!!!        m_abandoning(false), 
+//!!!        m_aboutToDelete(false),
         m_extendTo(0) { }
 
     // Not provided.
-    Model(const Model &);
-    Model &operator=(const Model &); 
+    Model(const Model &) =delete;
+    Model &operator=(const Model &) =delete;
 
-    Model *m_sourceModel;
-    AlignmentModel *m_alignment;
+    ModelId m_sourceModel;
+    ModelId m_alignmentModel;
     QString m_typeUri;
-    bool m_abandoning;
-    bool m_aboutToDelete;
+//!!!    bool m_abandoning;
+//!!!    bool m_aboutToDelete;
     sv_frame_t m_extendTo;
 };
 
-typedef Model::Id ModelId;
+typedef Model::ModelId ModelId;
 typedef StaticById<Model, ModelId> ModelById;
 
 #endif
diff -r c3b5564cfb78 -r d91ff235e69d data/model/PathModel.h
--- a/data/model/PathModel.h	Mon Jun 24 14:28:17 2019 +0100
+++ b/data/model/PathModel.h	Tue Jun 25 15:29:34 2019 +0100
@@ -57,6 +57,12 @@
     }
 };
 
+//!!! pretty sure there is no good reason for this to be a model any
+//!!! more - it used to use implementation inheritance from
+//!!! SparseModel but that's no longer a thing. Should just be a
+//!!! simple container now, to be passed around by value/reference
+//!!! rather than on heap
+
 class PathModel : public Model
 {
 public: