ardour/livetrax
13
0
Fork 0
Code

temporal: fix major conceptual error managing Point reference to owner map

Browse Source 
When TempoMap::copy_points() is called, the new points are intended to belong
to the (nascent) new map. But the copy constructor for the points leaves the
_map member of a Point unchanged, and so the new points reference the old
map (forever!). ::copy_points() must reset each Point to reference the new map.

Refactored the object that has the _map member, so that we could limit access
to its ::set_map() method to TempoMap.
This commit is contained in:
Paul Davis 2022-06-16 22:11:56 -06:00
parent cdf98a1bd7
commit f60b35483d
2 changed files with 20 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
libs/temporal/tempo.cc
View File

@ -58,7 +58,7 @@ Point::add_state (XMLNode & node) const
}
Point::Point (TempoMap const & map, XMLNode const & node)
: _map (&map)
: MapOwned (map)
{
if (!node.get_property (X_("sclock"), _sclock)) {
throw failed_constructor();
@ -781,8 +781,9 @@ TempoMap::copy_points (TempoMap const & other)
sort (p.begin(), p.end(), Point::ptr_sclock_comparator());
for (std::vector<Point*>::iterator pi = p.begin(); pi != p.end(); ++pi) {
_points.push_back (**pi);
for (auto & pi : p) {
pi->set_map (*this);
_points.push_back (*pi);
}
}

21
libs/temporal/temporal/tempo.h
View File

@ -60,6 +60,20 @@ namespace Temporal {
class Meter;
class TempoMap;
class MapOwned {
protected:
MapOwned (TempoMap const & map) : _map (&map) {}
~MapOwned () {}
public:
TempoMap const & map() const { return *_map; }
protected:
friend class TempoMap;
void set_map (TempoMap const & map) { _map = &map; }
TempoMap const * _map;
};
/* Conceptually, Point is similar to timepos_t. However, whereas timepos_t can
* use the TempoMap to translate between time domains, Point cannot. Why not?
* Because Point is foundational in building the tempo map, and we cannot
@ -68,9 +82,9 @@ class TempoMap;
*/
typedef boost::intrusive::list_base_hook<boost::intrusive::tag<struct point_tag>> point_hook;
class /*LIBTEMPORAL_API*/ Point : public point_hook {
class /*LIBTEMPORAL_API*/ Point : public point_hook, public MapOwned {
public:
LIBTEMPORAL_API Point (TempoMap const & map, superclock_t sc, Beats const & b, BBT_Time const & bbt) : _sclock (sc), _quarters (b), _bbt (bbt), _map (&map) {}
LIBTEMPORAL_API Point (TempoMap const & map, superclock_t sc, Beats const & b, BBT_Time const & bbt) : MapOwned (map), _sclock (sc), _quarters (b), _bbt (bbt) {}
LIBTEMPORAL_API Point (TempoMap const & map, XMLNode const &);
LIBTEMPORAL_API virtual ~Point() {}
@ -127,13 +141,10 @@ class /*LIBTEMPORAL_API*/ Point : public point_hook {
LIBTEMPORAL_API inline bool operator== (Point const & other) const { return _sclock == other._sclock; }
LIBTEMPORAL_API inline bool operator!= (Point const & other) const { return _sclock != other._sclock; }
LIBTEMPORAL_API TempoMap const & map() const { return *_map; }
protected:
superclock_t _sclock;
Beats _quarters;
BBT_Time _bbt;
TempoMap const * _map;
void add_state (XMLNode &) const;