Mercurial > hg > svcore
changeset 1677:f97d64b8674f single-point
Make XmlExportables store their export IDs and always obtain a new one, avoiding reuse when an object is allocated at the same heap location as a previous one. This makes the ID system stable enough to be used in the export tests.
author | Chris Cannam |
---|---|
date | Thu, 28 Mar 2019 11:55:02 +0000 |
parents | 3b51df7695a4 |
children | 1078f0ef3012 |
files | base/EventSeries.cpp base/XmlExportable.cpp base/XmlExportable.h data/model/AggregateWaveModel.cpp data/model/AlignmentModel.cpp data/model/EditableDenseThreeDimensionalModel.cpp data/model/ImageModel.h data/model/Model.cpp data/model/NoteModel.h data/model/PathModel.h data/model/RegionModel.h data/model/SparseOneDimensionalModel.h data/model/SparseTimeValueModel.h data/model/TextModel.h data/model/test/TestSparseModels.h |
diffstat | 15 files changed, 59 insertions(+), 43 deletions(-) [+] |
line wrap: on
line diff
--- a/base/EventSeries.cpp Thu Mar 28 10:39:24 2019 +0000 +++ b/base/EventSeries.cpp Thu Mar 28 11:55:02 2019 +0000 @@ -495,7 +495,7 @@ Event::ExportNameOptions options) const { out << indent << QString("<dataset id=\"%1\" %2>\n") - .arg(getObjectExportId(this)) + .arg(getExportId()) .arg(extraAttributes); for (const auto &p: m_events) {
--- a/base/XmlExportable.cpp Thu Mar 28 10:39:24 2019 +0000 +++ b/base/XmlExportable.cpp Thu Mar 28 11:55:02 2019 +0000 @@ -68,19 +68,17 @@ } int -XmlExportable::getObjectExportId(const void * object) +XmlExportable::getExportId() const { - static QMutex mutex; - QMutexLocker locker(&mutex); - - static std::map<const void *, int> idMap; - static int maxId = 0; - - if (idMap.find(object) == idMap.end()) { - idMap[object] = maxId++; + if (m_exportId == -1) { + static QMutex mutex; + static int lastId = 0; + QMutexLocker locker(&mutex); + if (m_exportId == -1) { + m_exportId = ++lastId; + } } - - return idMap[object]; + return m_exportId; }
--- a/base/XmlExportable.h Thu Mar 28 10:39:24 2019 +0000 +++ b/base/XmlExportable.h Thu Mar 28 11:55:02 2019 +0000 @@ -25,9 +25,17 @@ class XmlExportable { public: + XmlExportable() : m_exportId(-1) { } virtual ~XmlExportable() { } /** + * Return the numerical export identifier for this object. It's + * allocated the first time this is called, so objects on which + * this is never called do not get allocated one. + */ + int getExportId() const; + + /** * Stream this exportable object out to XML on a text stream. */ virtual void toXml(QTextStream &stream, @@ -46,7 +54,8 @@ static QString encodeColour(int r, int g, int b); - static int getObjectExportId(const void *); // thread-safe +private: + mutable int m_exportId; }; #endif
--- a/data/model/AggregateWaveModel.cpp Thu Mar 28 10:39:24 2019 +0000 +++ b/data/model/AggregateWaveModel.cpp Thu Mar 28 11:55:02 2019 +0000 @@ -233,7 +233,7 @@ { QStringList componentStrings; for (const auto &c: m_components) { - componentStrings.push_back(QString("%1").arg(getObjectExportId(c.model))); + componentStrings.push_back(QString("%1").arg(c.model->getExportId())); } Model::toXml(out, indent, QString("type=\"aggregatewave\" components=\"%1\" %2")
--- a/data/model/AlignmentModel.cpp Thu Mar 28 10:39:24 2019 +0000 +++ b/data/model/AlignmentModel.cpp Thu Mar 28 11:55:02 2019 +0000 @@ -417,9 +417,9 @@ Model::toXml(stream, indent, QString("type=\"alignment\" reference=\"%1\" aligned=\"%2\" path=\"%3\" %4") - .arg(getObjectExportId(m_reference)) - .arg(getObjectExportId(m_aligned)) - .arg(getObjectExportId(m_path)) + .arg(m_reference->getExportId()) + .arg(m_aligned->getExportId()) + .arg(m_path->getExportId()) .arg(extraAttributes)); }
--- a/data/model/EditableDenseThreeDimensionalModel.cpp Thu Mar 28 10:39:24 2019 +0000 +++ b/data/model/EditableDenseThreeDimensionalModel.cpp Thu Mar 28 11:55:02 2019 +0000 @@ -526,7 +526,11 @@ { QReadLocker locker(&m_lock); - // For historical reasons we read and write "resolution" as "windowSize" + // For historical reasons we read and write "resolution" as "windowSize". + + // Our dataset doesn't have its own export ID, we just use + // ours. Actually any model could do that, since datasets aren't + // in the same id-space as models when re-read SVDEBUG << "EditableDenseThreeDimensionalModel::toXml" << endl; @@ -537,13 +541,13 @@ .arg(m_yBinCount) .arg(m_minimum) .arg(m_maximum) - .arg(getObjectExportId(&m_data)) + .arg(getExportId()) .arg(m_startFrame) .arg(extraAttributes)); out << indent; out << QString("<dataset id=\"%1\" dimensions=\"3\" separator=\" \">\n") - .arg(getObjectExportId(&m_data)); + .arg(getExportId()); for (int i = 0; i < (int)m_binNames.size(); ++i) { if (m_binNames[i] != "") {
--- a/data/model/ImageModel.h Thu Mar 28 10:39:24 2019 +0000 +++ b/data/model/ImageModel.h Thu Mar 28 11:55:02 2019 +0000 @@ -253,7 +253,7 @@ .arg(m_resolution) .arg("true") // always true after model reaches 100% - // subsequent events are always notified - .arg(getObjectExportId(&m_events)) + .arg(m_events.getExportId()) .arg(extraAttributes)); Event::ExportNameOptions options;
--- a/data/model/Model.cpp Thu Mar 28 10:39:24 2019 +0000 +++ b/data/model/Model.cpp Thu Mar 28 11:55:02 2019 +0000 @@ -202,7 +202,7 @@ { stream << indent; stream << QString("<model id=\"%1\" name=\"%2\" sampleRate=\"%3\" start=\"%4\" end=\"%5\" %6/>\n") - .arg(getObjectExportId(this)) + .arg(getExportId()) .arg(encodeEntities(objectName())) .arg(getSampleRate()) .arg(getStartFrame())
--- a/data/model/NoteModel.h Thu Mar 28 10:39:24 2019 +0000 +++ b/data/model/NoteModel.h Thu Mar 28 11:55:02 2019 +0000 @@ -373,7 +373,7 @@ .arg(m_resolution) .arg("true") // always true after model reaches 100% - // subsequent events are always notified - .arg(getObjectExportId(&m_events)) + .arg(m_events.getExportId()) .arg(m_subtype == FLEXI_NOTE ? "flexinote" : "note") .arg(m_valueQuantization) .arg(m_valueMinimum)
--- a/data/model/PathModel.h Thu Mar 28 10:39:24 2019 +0000 +++ b/data/model/PathModel.h Thu Mar 28 11:55:02 2019 +0000 @@ -167,6 +167,10 @@ void toXml(QTextStream &out, QString indent = "", QString extraAttributes = "") const override { + + // Our dataset doesn't have its own export ID, we just use + // ours. Actually any model could do that, since datasets + // aren't in the same id-space as models when re-read Model::toXml (out, @@ -176,11 +180,11 @@ .arg(m_resolution) .arg("true") // always true after model reaches 100% - // subsequent points are always notified - .arg(getObjectExportId(&m_points)) + .arg(getExportId()) .arg(extraAttributes)); out << indent << QString("<dataset id=\"%1\" dimensions=\"2\">\n") - .arg(getObjectExportId(&m_points)); + .arg(getExportId()); for (PathPoint p: m_points) { p.toXml(out, indent + " ", "");
--- a/data/model/RegionModel.h Thu Mar 28 10:39:24 2019 +0000 +++ b/data/model/RegionModel.h Thu Mar 28 11:55:02 2019 +0000 @@ -316,7 +316,7 @@ .arg(m_resolution) .arg("true") // always true after model reaches 100% - // subsequent events are always notified - .arg(getObjectExportId(&m_events)) + .arg(m_events.getExportId()) .arg("region") .arg(m_valueQuantization) .arg(m_valueMinimum)
--- a/data/model/SparseOneDimensionalModel.h Thu Mar 28 10:39:24 2019 +0000 +++ b/data/model/SparseOneDimensionalModel.h Thu Mar 28 11:55:02 2019 +0000 @@ -285,7 +285,7 @@ .arg(m_resolution) .arg("true") // always true after model reaches 100% - // subsequent events are always notified - .arg(getObjectExportId(&m_events)) + .arg(m_events.getExportId()) .arg(extraAttributes)); m_events.toXml(out, indent, QString("dimensions=\"1\""));
--- a/data/model/SparseTimeValueModel.h Thu Mar 28 10:39:24 2019 +0000 +++ b/data/model/SparseTimeValueModel.h Thu Mar 28 11:55:02 2019 +0000 @@ -313,7 +313,7 @@ .arg(m_resolution) .arg("true") // always true after model reaches 100% - // subsequent events are always notified - .arg(getObjectExportId(&m_events)) + .arg(m_events.getExportId()) .arg(m_valueMinimum) .arg(m_valueMaximum) .arg(encodeEntities(m_units))
--- a/data/model/TextModel.h Thu Mar 28 10:39:24 2019 +0000 +++ b/data/model/TextModel.h Thu Mar 28 11:55:02 2019 +0000 @@ -248,7 +248,7 @@ .arg(m_resolution) .arg("true") // always true after model reaches 100% - // subsequent events are always notified - .arg(getObjectExportId(&m_events)) + .arg(m_events.getExportId()) .arg(extraAttributes)); Event::ExportNameOptions options;
--- a/data/model/test/TestSparseModels.h Thu Mar 28 10:39:24 2019 +0000 +++ b/data/model/test/TestSparseModels.h Thu Mar 28 11:55:02 2019 +0000 @@ -28,6 +28,11 @@ using namespace std; +// NB model & dataset IDs in the export tests are incremental, +// depending on how many have been exported in previous tests - so +// when adding or removing tests we may occasionally need to update +// the IDs in other ones + class TestSparseModels : public QObject { Q_OBJECT @@ -118,8 +123,8 @@ m.toXml(str); str.flush(); QString expected = - "<model id='1' name='This "&" that' sampleRate='100' start='20' end='60' type='sparse' dimensions='1' resolution='10' notifyOnAdd='true' dataset='0' />\n" - "<dataset id='0' dimensions='1'>\n" + "<model id='2' name='This "&" that' sampleRate='100' start='20' end='60' type='sparse' dimensions='1' resolution='10' notifyOnAdd='true' dataset='1' />\n" + "<dataset id='1' dimensions='1'>\n" " <point frame='20' />\n" " <point frame='20' label='Label &'">' />\n" " <point frame='50' />\n" @@ -202,14 +207,10 @@ QTextStream str(&xml, QIODevice::WriteOnly); m.toXml(str); str.flush(); - - //!!! This is not guaranteed - object export ids are in order - //!!! of model pointer value, which is not trustworthy - - //!!! replace them with something else QString expected = - "<model id='3' name='' sampleRate='100' start='20' end='80' type='sparse' dimensions='3' resolution='10' notifyOnAdd='true' dataset='2' subtype='note' valueQuantization='0' minimum='123.4' maximum='126.3' units='Hz' />\n" - "<dataset id='2' dimensions='3'>\n" + "<model id='4' name='' sampleRate='100' start='20' end='80' type='sparse' dimensions='3' resolution='10' notifyOnAdd='true' dataset='3' subtype='note' valueQuantization='0' minimum='123.4' maximum='126.3' units='Hz' />\n" + "<dataset id='3' dimensions='3'>\n" " <point frame='20' value='124.3' duration='10' level='0.9' label='note 2' />\n" " <point frame='20' value='123.4' duration='20' level='0.8' label='note 1' />\n" " <point frame='50' value='126.3' duration='30' level='0.9' label='note 3' />\n" @@ -236,8 +237,8 @@ str.flush(); QString expected = - "<model id='5' name='' sampleRate='100' start='20' end='80' type='sparse' dimensions='2' resolution='10' notifyOnAdd='true' dataset='4' subtype='text' />\n" - "<dataset id='4' dimensions='2'>\n" + "<model id='6' name='' sampleRate='100' start='20' end='60' type='sparse' dimensions='2' resolution='10' notifyOnAdd='true' dataset='5' subtype='text' />\n" + "<dataset id='5' dimensions='2'>\n" " <point frame='20' height='0' label='text 2' />\n" " <point frame='20' height='1' label='text 1' />\n" " <point frame='50' height='0.3' label='text 3' />\n" @@ -264,8 +265,8 @@ str.flush(); QString expected = - "<model id='7' name='' sampleRate='100' start='20' end='80' type='sparse' dimensions='2' resolution='10' notifyOnAdd='true' dataset='4' subtype='path' />\n" - "<dataset id='6' dimensions='2'>\n" + "<model id='7' name='' sampleRate='100' start='20' end='60' type='sparse' dimensions='2' resolution='10' notifyOnAdd='true' dataset='7' subtype='path' />\n" + "<dataset id='7' dimensions='2'>\n" " <point frame='20' mapframe='30' />\n" " <point frame='40' mapframe='60' />\n" " <point frame='50' mapframe='49' />\n" @@ -288,8 +289,8 @@ str.flush(); QString expected = - "<model id='4' name='' sampleRate='100' start='20' end='30' type='sparse' dimensions='1' resolution='10' notifyOnAdd='true' dataset='2' subtype='image' />\n" - "<dataset id='2' dimensions='1'>\n" + "<model id='9' name='' sampleRate='100' start='20' end='30' type='sparse' dimensions='1' resolution='10' notifyOnAdd='true' dataset='8' subtype='image' />\n" + "<dataset id='8' dimensions='1'>\n" " <point frame='20' label='a label' image='/path/to/thing.png' />\n" "</dataset>\n"; expected.replace("\'", "\"");