From 3150041423a199189c9b6ab292a078c51d9670c2 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sun, 29 Aug 2010 01:09:05 +0000 Subject: [PATCH] (Hopefully) clarify operator= and copy construction behaviour of the Property hierarchy. Also make operator= copy the value but NOT the property ID; this stops e.g. a = b giving a the property ID of b and confusing things. Fixes some problems with save/restore of region sync position. git-svn-id: svn://localhost/ardour2/branches/3.0@7707 d708f5d6-7413-0410-9779-e7cbd77b26cf --- libs/ardour/ardour/audioplaylist.h | 3 ++ libs/ardour/ardour/playlist.h | 3 ++ libs/ardour/audio_playlist.cc | 7 ++++ libs/ardour/audioregion.cc | 12 +++--- libs/ardour/playlist.cc | 7 ++++ libs/ardour/region.cc | 46 ++++++++++---------- libs/pbd/pbd/properties.h | 67 ++++++++++++++++++------------ libs/pbd/pbd/property_basics.h | 9 +++- libs/pbd/pbd/sequence_property.h | 9 ++++ 9 files changed, 107 insertions(+), 56 deletions(-) diff --git a/libs/ardour/ardour/audioplaylist.h b/libs/ardour/ardour/audioplaylist.h index f2d60bce8b..39a19d8ed1 100644 --- a/libs/ardour/ardour/audioplaylist.h +++ b/libs/ardour/ardour/audioplaylist.h @@ -54,6 +54,9 @@ private: CrossfadeListProperty* clone () const; CrossfadeListProperty* create () const; + /* copy construction only by ourselves */ + CrossfadeListProperty (CrossfadeListProperty const & p); + friend class AudioPlaylist; /* we live and die with our playlist, no lifetime management needed */ AudioPlaylist& _playlist; diff --git a/libs/ardour/ardour/playlist.h b/libs/ardour/ardour/playlist.h index a1a7bb752a..feea056ac3 100644 --- a/libs/ardour/ardour/playlist.h +++ b/libs/ardour/ardour/playlist.h @@ -70,6 +70,9 @@ class RegionListProperty : public PBD::SequenceProperty > > (p) + , _playlist (p._playlist) +{ + +} + CrossfadeListProperty * CrossfadeListProperty::create () const diff --git a/libs/ardour/audioregion.cc b/libs/ardour/audioregion.cc index 7212cf8e4c..7cc8e00998 100644 --- a/libs/ardour/audioregion.cc +++ b/libs/ardour/audioregion.cc @@ -103,12 +103,12 @@ AudioRegion::register_properties () , _scale_amplitude (Properties::scale_amplitude, 1.0) #define AUDIOREGION_COPY_STATE(other) \ - _envelope_active (other->_envelope_active) \ - , _default_fade_in (other->_default_fade_in) \ - , _default_fade_out (other->_default_fade_out) \ - , _fade_in_active (other->_fade_in_active) \ - , _fade_out_active (other->_fade_out_active) \ - , _scale_amplitude (other->_scale_amplitude) + _envelope_active (Properties::envelope_active, other->_envelope_active) \ + , _default_fade_in (Properties::default_fade_in, other->_default_fade_in) \ + , _default_fade_out (Properties::default_fade_out, other->_default_fade_out) \ + , _fade_in_active (Properties::fade_in_active, other->_fade_in_active) \ + , _fade_out_active (Properties::fade_out_active, other->_fade_out_active) \ + , _scale_amplitude (Properties::scale_amplitude, other->_scale_amplitude) /* a Session will reset these to its chosen defaults by calling AudioRegion::set_default_fade() */ void diff --git a/libs/ardour/playlist.cc b/libs/ardour/playlist.cc index 4a5d65c903..87ed1ca03b 100644 --- a/libs/ardour/playlist.cc +++ b/libs/ardour/playlist.cc @@ -113,6 +113,13 @@ RegionListProperty::RegionListProperty (Playlist& pl) } +RegionListProperty::RegionListProperty (RegionListProperty const & p) + : PBD::SequenceProperty > > (p) + , _playlist (p._playlist) +{ + +} + RegionListProperty * RegionListProperty::clone () const { diff --git a/libs/ardour/region.cc b/libs/ardour/region.cc index 73f42bc4eb..43506c78be 100644 --- a/libs/ardour/region.cc +++ b/libs/ardour/region.cc @@ -182,29 +182,29 @@ Region::register_properties () , _position_lock_style (Properties::position_lock_style, _type == DataType::AUDIO ? AudioTime : MusicTime) #define REGION_COPY_STATE(other) \ - _muted (other->_muted) \ - , _opaque (other->_opaque) \ - , _locked (other->_locked) \ - , _automatic (other->_automatic) \ - , _whole_file (other->_whole_file) \ - , _import (other->_import) \ - , _external (other->_external) \ - , _sync_marked (other->_sync_marked) \ - , _left_of_split (other->_left_of_split) \ - , _right_of_split (other->_right_of_split) \ - , _hidden (other->_hidden) \ - , _position_locked (other->_position_locked) \ - , _valid_transients (other->_valid_transients) \ - , _start(other->_start) \ - , _length(other->_length) \ - , _position(other->_position) \ - , _sync_position(other->_sync_position) \ - , _layer (other->_layer) \ - , _ancestral_start (other->_ancestral_start) \ - , _ancestral_length (other->_ancestral_length) \ - , _stretch (other->_stretch) \ - , _shift (other->_shift) \ - , _position_lock_style (other->_position_lock_style) + _muted (Properties::muted, other->_muted) \ + , _opaque (Properties::opaque, other->_opaque) \ + , _locked (Properties::locked, other->_locked) \ + , _automatic (Properties::automatic, other->_automatic) \ + , _whole_file (Properties::whole_file, other->_whole_file) \ + , _import (Properties::import, other->_import) \ + , _external (Properties::external, other->_external) \ + , _sync_marked (Properties::sync_marked, other->_sync_marked) \ + , _left_of_split (Properties::left_of_split, other->_left_of_split) \ + , _right_of_split (Properties::right_of_split, other->_right_of_split) \ + , _hidden (Properties::hidden, other->_hidden) \ + , _position_locked (Properties::position_locked, other->_position_locked) \ + , _valid_transients (Properties::valid_transients, other->_valid_transients) \ + , _start(Properties::start, other->_start) \ + , _length(Properties::length, other->_length) \ + , _position(Properties::position, other->_position) \ + , _sync_position(Properties::sync_position, other->_sync_position) \ + , _layer (Properties::layer, other->_layer) \ + , _ancestral_start (Properties::ancestral_start, other->_ancestral_start) \ + , _ancestral_length (Properties::ancestral_length, other->_ancestral_length) \ + , _stretch (Properties::stretch, other->_stretch) \ + , _shift (Properties::shift, other->_shift) \ + , _position_lock_style (Properties::position_lock_style, other->_position_lock_style) /* derived-from-derived constructor (no sources in constructor) */ Region::Region (Session& s, framepos_t start, framecnt_t length, const string& name, DataType type) diff --git a/libs/pbd/pbd/properties.h b/libs/pbd/pbd/properties.h index bd4846dc7e..95426ec473 100644 --- a/libs/pbd/pbd/properties.h +++ b/libs/pbd/pbd/properties.h @@ -51,22 +51,26 @@ public: , _current (c) , _old (o) {} - - PropertyTemplate& operator=(PropertyTemplate const& s) { - /* XXX: isn't there a nicer place to do this? */ - _have_old = s._have_old; - _property_id = s._property_id; - - _current = s._current; - _old = s._old; - return *this; - } + + PropertyTemplate (PropertyDescriptor p, PropertyTemplate const & s) + : PropertyBase (p.property_id) + , _have_old (false) + , _current (s._current) + {} T & operator=(T const& v) { set (v); return _current; } + /* This will mean that, if fred and jim are both PropertyTemplates, + * fred = jim will result in fred taking on jim's current value, + * but NOT jim's property ID. + */ + PropertyTemplate & operator= (PropertyTemplate const & p) { + set (p._current); + } + T & operator+=(T const& v) { set (_current + v); return _current; @@ -140,13 +144,6 @@ public: } protected: - /** Constructs a PropertyTemplate with a default - value for _old and _current. - */ - - PropertyTemplate (PropertyDescriptor p) - : PropertyBase (p.property_id) - {} void set (T const& v) { if (v != _current) { @@ -175,6 +172,12 @@ protected: bool _have_old; T _current; T _old; + +private: + /* disallow copy-construction; it's not obvious whether it should mean + a copy of just the value, or the value and property ID. + */ + PropertyTemplate (PropertyTemplate const &); }; template @@ -198,8 +201,12 @@ public: : PropertyTemplate (q, o, c) {} + Property (PropertyDescriptor q, Property const& v) + : PropertyTemplate (q, v) + {} + Property* clone () const { - return new Property (*this); + return new Property (this->property_id(), this->_old, this->_current); } Property* clone_from_xml (const XMLNode& node) const { @@ -230,9 +237,8 @@ public: private: friend class PropertyFactory; - Property (PropertyDescriptor q) - : PropertyTemplate (q) - {} + /* no copy-construction */ + Property (Property const &); /* Note that we do not set a locale for the streams used * in to_string() or from_string(), because we want the @@ -265,15 +271,19 @@ template<> class Property : public PropertyTemplate { public: - Property (PropertyDescriptor q, std::string const& v) - : PropertyTemplate (q, v) + Property (PropertyDescriptor d, std::string const & v) + : PropertyTemplate (d, v) {} + Property (PropertyDescriptor d, std::string const & o, std::string const & c) + : PropertyTemplate (d, o, c) + {} + Property* clone () const { - return new Property (*this); + return new Property (this->property_id(), _old, _current); } - - std::string & operator=(std::string const& v) { + + std::string & operator= (std::string const& v) { this->set (v); return this->_current; } @@ -287,6 +297,8 @@ private: return s; } + /* no copy-construction */ + Property (Property const &); }; template @@ -310,6 +322,9 @@ private: T from_string (std::string const & s) const { return static_cast (string_2_enum (s, this->_current)); } + + /* no copy-construction */ + EnumProperty (EnumProperty const &); }; } /* namespace PBD */ diff --git a/libs/pbd/pbd/property_basics.h b/libs/pbd/pbd/property_basics.h index 145e84abfc..2c35c4bbc2 100644 --- a/libs/pbd/pbd/property_basics.h +++ b/libs/pbd/pbd/property_basics.h @@ -136,8 +136,15 @@ public: return _property_id == pid; } -protected: +protected: + /* copy construction only by subclasses */ + PropertyBase (PropertyBase const & b) + : _property_id (b._property_id) + {} + +private: PropertyID _property_id; + }; } diff --git a/libs/pbd/pbd/sequence_property.h b/libs/pbd/pbd/sequence_property.h index 4b494d4a8d..7d94a4be6b 100644 --- a/libs/pbd/pbd/sequence_property.h +++ b/libs/pbd/pbd/sequence_property.h @@ -344,6 +344,15 @@ class SequenceProperty : public PropertyBase const ChangeRecord& changes () const { return _changes; } protected: + + /* copy construction only by subclasses */ + SequenceProperty (SequenceProperty const & p) + : PropertyBase (p) + , _val (p._val) + , _changes (p._changes) + , _update_callback (p._update_callback) + {} + Container _val; ///< our actual container of things ChangeRecord _changes; ///< changes to the container (adds/removes) that have happened since clear_changes() was last called boost::function _update_callback; -- 2.30.2