Mercurial > hg > svcore
changeset 1735:d91ff235e69d by-id
Some messing with Model and AlignmentModel
| author | Chris Cannam |
|---|---|
| date | Tue, 25 Jun 2019 15:29:34 +0100 |
| parents | c3b5564cfb78 |
| children | d9082ed16931 |
| files | base/ById.h data/model/AlignmentModel.cpp data/model/AlignmentModel.h data/model/Model.cpp data/model/Model.h data/model/PathModel.h |
| diffstat | 6 files changed, 164 insertions(+), 116 deletions(-) [+] |
line wrap: on
line diff
--- 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; } };
--- 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(); } }
--- 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
--- 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
--- 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
--- 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: