# HG changeset patch
# User Chris Cannam
# Date 1553774102 0
# Node ID f97d64b8674ffdb12c2dc56e8d0a8065c3a91b17
# Parent  3b51df7695a4b942883d6916f79b67beb06fbb8d
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.

diff -r 3b51df7695a4 -r f97d64b8674f base/EventSeries.cpp
--- 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) {
diff -r 3b51df7695a4 -r f97d64b8674f base/XmlExportable.cpp
--- 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;
 }
 
 
diff -r 3b51df7695a4 -r f97d64b8674f base/XmlExportable.h
--- 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
diff -r 3b51df7695a4 -r f97d64b8674f data/model/AggregateWaveModel.cpp
--- 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")
diff -r 3b51df7695a4 -r f97d64b8674f data/model/AlignmentModel.cpp
--- 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));
 }
 
diff -r 3b51df7695a4 -r f97d64b8674f data/model/EditableDenseThreeDimensionalModel.cpp
--- 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] != "") {
diff -r 3b51df7695a4 -r f97d64b8674f data/model/ImageModel.h
--- 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;
diff -r 3b51df7695a4 -r f97d64b8674f data/model/Model.cpp
--- 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())
diff -r 3b51df7695a4 -r f97d64b8674f data/model/NoteModel.h
--- 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)
diff -r 3b51df7695a4 -r f97d64b8674f data/model/PathModel.h
--- 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 + "  ", "");
diff -r 3b51df7695a4 -r f97d64b8674f data/model/RegionModel.h
--- 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)
diff -r 3b51df7695a4 -r f97d64b8674f data/model/SparseOneDimensionalModel.h
--- 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\""));
diff -r 3b51df7695a4 -r f97d64b8674f data/model/SparseTimeValueModel.h
--- 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))
diff -r 3b51df7695a4 -r f97d64b8674f data/model/TextModel.h
--- 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;
diff -r 3b51df7695a4 -r f97d64b8674f data/model/test/TestSparseModels.h
--- 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 &quot;&amp;&quot; 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 &quot;&amp;&quot; 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 &amp;&apos;&quot;&gt;' />\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("\'", "\"");