From 41da8043be215780097a4cc2cf556b275a48b715 Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Sun, 11 Aug 2024 10:41:37 +0200 Subject: [PATCH 01/11] Don't save MIDI information in presets Ensure that no MIDI information is stored in presets. Add the new protected method `isSimpleSerializing` that can be used by sub classes to find out if they are temporarily in preset saving mode or not. Please note that the existing implementation is very brittle because an instance might accidentally stay in that mode. Use the information in `InstrumentTrack::saveTrackSpecificSettings`. --- include/Track.h | 6 ++++++ src/tracks/InstrumentTrack.cpp | 6 +++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/include/Track.h b/include/Track.h index db33900f5da..a2c839cc5cb 100644 --- a/include/Track.h +++ b/include/Track.h @@ -209,6 +209,12 @@ public slots: void toggleSolo(); +protected: + bool isSimpleSerializing() const + { + return m_simpleSerializingMode; + } + private: TrackContainer* m_trackContainer; Type m_type; diff --git a/src/tracks/InstrumentTrack.cpp b/src/tracks/InstrumentTrack.cpp index be18fc9e4ca..0ee6c4622ed 100644 --- a/src/tracks/InstrumentTrack.cpp +++ b/src/tracks/InstrumentTrack.cpp @@ -868,7 +868,11 @@ void InstrumentTrack::saveTrackSpecificSettings( QDomDocument& doc, QDomElement bool hasAuto = m_hasAutoMidiDev; autoAssignMidiDevice(false); - m_midiPort.saveState( doc, thisElement ); + // Only save the MIDI port information if we are not saving a preset. + if (!isSimpleSerializing()) + { + m_midiPort.saveState( doc, thisElement ); + } autoAssignMidiDevice(hasAuto); } From b1134d1c911f082b551877293f0ff03e68f7b4a9 Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Sun, 11 Aug 2024 10:42:11 +0200 Subject: [PATCH 02/11] Fix wrong comment --- src/tracks/InstrumentTrack.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tracks/InstrumentTrack.cpp b/src/tracks/InstrumentTrack.cpp index 0ee6c4622ed..c28019f0ef7 100644 --- a/src/tracks/InstrumentTrack.cpp +++ b/src/tracks/InstrumentTrack.cpp @@ -860,7 +860,7 @@ void InstrumentTrack::saveTrackSpecificSettings( QDomDocument& doc, QDomElement // Save the midi port info if we are not in song saving mode, e.g. in // track cloning mode or if we are in song saving mode and the user - // has chosen to discard the MIDI connections. + // has chosen not to discard the MIDI connections. if (!Engine::getSong()->isSavingProject() || !Engine::getSong()->getSaveOptions().discardMIDIConnections.value()) { From 3f8bef620053aaa2280e8d55ce3498b14845c7b2 Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Sun, 11 Aug 2024 10:52:19 +0200 Subject: [PATCH 03/11] Reduce private member accesses Reduce direct accesses to `m_simpleSerializingMode` by using the method `isSimpleSerializing` and extending `setSimpleSerializing` with a parameter. --- include/Track.h | 4 ++-- src/core/Track.cpp | 12 +++++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/include/Track.h b/include/Track.h index a2c839cc5cb..3db3a8c0730 100644 --- a/include/Track.h +++ b/include/Track.h @@ -115,9 +115,9 @@ class LMMS_EXPORT Track : public Model, public JournallingObject void saveSettings( QDomDocument & doc, QDomElement & element ) override; void loadSettings( const QDomElement & element ) override; - void setSimpleSerializing() + void setSimpleSerializing(bool simpleSerializing = true) { - m_simpleSerializingMode = true; + m_simpleSerializingMode = simpleSerializing; } // -- for usage by Clip only --------------- diff --git a/src/core/Track.cpp b/src/core/Track.cpp index 7ff73e71891..936a1c883c3 100644 --- a/src/core/Track.cpp +++ b/src/core/Track.cpp @@ -191,7 +191,7 @@ Track* Track::clone() */ void Track::saveSettings( QDomDocument & doc, QDomElement & element ) { - if( !m_simpleSerializingMode ) + if (!isSimpleSerializing()) { element.setTagName( "track" ); } @@ -217,9 +217,9 @@ void Track::saveSettings( QDomDocument & doc, QDomElement & element ) element.appendChild( tsDe ); saveTrackSpecificSettings( doc, tsDe ); - if( m_simpleSerializingMode ) + if (isSimpleSerializing()) { - m_simpleSerializingMode = false; + setSimpleSerializing(false); return; } @@ -267,7 +267,7 @@ void Track::loadSettings( const QDomElement & element ) setColor(QColor{element.attribute("color")}); } - if( m_simpleSerializingMode ) + if (isSimpleSerializing()) { QDomNode node = element.firstChild(); while( !node.isNull() ) @@ -279,7 +279,9 @@ void Track::loadSettings( const QDomElement & element ) } node = node.nextSibling(); } - m_simpleSerializingMode = false; + + setSimpleSerializing(false); + return; } From 00235c7cef5f915ac4a74b13e5ddf4574bd9fbaf Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Sun, 11 Aug 2024 10:57:03 +0200 Subject: [PATCH 04/11] Rename "SimpleSerializing" Rename "SimpleSerializing" to "PresetMode" everywhere because that is what this mode describes in fact. --- include/Track.h | 10 +++++----- src/core/Track.cpp | 12 ++++++------ src/gui/editors/TrackContainerView.cpp | 2 +- src/gui/instrument/InstrumentTrackWindow.cpp | 2 +- src/tracks/InstrumentTrack.cpp | 4 ++-- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/include/Track.h b/include/Track.h index 3db3a8c0730..3eb995d8676 100644 --- a/include/Track.h +++ b/include/Track.h @@ -115,9 +115,9 @@ class LMMS_EXPORT Track : public Model, public JournallingObject void saveSettings( QDomDocument & doc, QDomElement & element ) override; void loadSettings( const QDomElement & element ) override; - void setSimpleSerializing(bool simpleSerializing = true) + void setPresetMode(bool presetMode = true) { - m_simpleSerializingMode = simpleSerializing; + m_presetMode = presetMode; } // -- for usage by Clip only --------------- @@ -210,9 +210,9 @@ public slots: void toggleSolo(); protected: - bool isSimpleSerializing() const + bool isPresetMode() const { - return m_simpleSerializingMode; + return m_presetMode; } private: @@ -228,7 +228,7 @@ public slots: BoolModel m_soloModel; bool m_mutedBeforeSolo; - bool m_simpleSerializingMode; + bool m_presetMode; clipVector m_clips; diff --git a/src/core/Track.cpp b/src/core/Track.cpp index 936a1c883c3..c45365a5ecc 100644 --- a/src/core/Track.cpp +++ b/src/core/Track.cpp @@ -63,7 +63,7 @@ Track::Track( Type type, TrackContainer * tc ) : m_name(), /*!< The track's name */ m_mutedModel( false, this, tr( "Mute" ) ), /*!< For controlling track muting */ m_soloModel( false, this, tr( "Solo" ) ), /*!< For controlling track soloing */ - m_simpleSerializingMode( false ), + m_presetMode( false ), m_clips() /*!< The clips (segments) */ { m_trackContainer->addTrack( this ); @@ -191,7 +191,7 @@ Track* Track::clone() */ void Track::saveSettings( QDomDocument & doc, QDomElement & element ) { - if (!isSimpleSerializing()) + if (!isPresetMode()) { element.setTagName( "track" ); } @@ -217,9 +217,9 @@ void Track::saveSettings( QDomDocument & doc, QDomElement & element ) element.appendChild( tsDe ); saveTrackSpecificSettings( doc, tsDe ); - if (isSimpleSerializing()) + if (isPresetMode()) { - setSimpleSerializing(false); + setPresetMode(false); return; } @@ -267,7 +267,7 @@ void Track::loadSettings( const QDomElement & element ) setColor(QColor{element.attribute("color")}); } - if (isSimpleSerializing()) + if (isPresetMode()) { QDomNode node = element.firstChild(); while( !node.isNull() ) @@ -280,7 +280,7 @@ void Track::loadSettings( const QDomElement & element ) node = node.nextSibling(); } - setSimpleSerializing(false); + setPresetMode(false); return; } diff --git a/src/gui/editors/TrackContainerView.cpp b/src/gui/editors/TrackContainerView.cpp index 60a46838041..6593247109b 100644 --- a/src/gui/editors/TrackContainerView.cpp +++ b/src/gui/editors/TrackContainerView.cpp @@ -416,7 +416,7 @@ void TrackContainerView::dropEvent( QDropEvent * _de ) { DataFile dataFile( value ); auto it = dynamic_cast(Track::create(Track::Type::Instrument, m_tc)); - it->setSimpleSerializing(); + it->setPresetMode(); it->loadSettings( dataFile.content().toElement() ); //it->toggledInstrumentTrackButton( true ); _de->accept(); diff --git a/src/gui/instrument/InstrumentTrackWindow.cpp b/src/gui/instrument/InstrumentTrackWindow.cpp index 1c9c93a09f9..31f01c4c235 100644 --- a/src/gui/instrument/InstrumentTrackWindow.cpp +++ b/src/gui/instrument/InstrumentTrackWindow.cpp @@ -424,7 +424,7 @@ void InstrumentTrackWindow::saveSettingsBtnClicked() DataFile dataFile(DataFile::Type::InstrumentTrackSettings); QDomElement& content(dataFile.content()); - m_track->setSimpleSerializing(); + m_track->setPresetMode(); m_track->saveSettings(dataFile, content); //We don't want to save muted & solo settings when we're saving a preset content.setAttribute("muted", 0); diff --git a/src/tracks/InstrumentTrack.cpp b/src/tracks/InstrumentTrack.cpp index c28019f0ef7..2e49479eea4 100644 --- a/src/tracks/InstrumentTrack.cpp +++ b/src/tracks/InstrumentTrack.cpp @@ -869,7 +869,7 @@ void InstrumentTrack::saveTrackSpecificSettings( QDomDocument& doc, QDomElement autoAssignMidiDevice(false); // Only save the MIDI port information if we are not saving a preset. - if (!isSimpleSerializing()) + if (!isPresetMode()) { m_midiPort.saveState( doc, thisElement ); } @@ -1011,7 +1011,7 @@ void InstrumentTrack::replaceInstrument(DataFile dataFile) int mixerChannel = mixerChannelModel()->value(); InstrumentTrack::removeMidiPortNode(dataFile); - setSimpleSerializing(); + setPresetMode(); //Replacing an instrument shouldn't change the solo/mute state. bool oldMute = isMuted(); From 9a45c97b5e1aad4b9e3aa56618e5ba1a91a3ed59 Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Sun, 11 Aug 2024 12:31:33 +0200 Subject: [PATCH 05/11] Minor code style fixup --- src/core/Track.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/Track.cpp b/src/core/Track.cpp index c45365a5ecc..b08c517a5ba 100644 --- a/src/core/Track.cpp +++ b/src/core/Track.cpp @@ -63,7 +63,7 @@ Track::Track( Type type, TrackContainer * tc ) : m_name(), /*!< The track's name */ m_mutedModel( false, this, tr( "Mute" ) ), /*!< For controlling track muting */ m_soloModel( false, this, tr( "Solo" ) ), /*!< For controlling track soloing */ - m_presetMode( false ), + m_presetMode(false), m_clips() /*!< The clips (segments) */ { m_trackContainer->addTrack( this ); From c822c10a68a512392f12233987634b515b74b1a4 Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Sun, 11 Aug 2024 15:24:28 +0200 Subject: [PATCH 06/11] Code style --- src/tracks/InstrumentTrack.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tracks/InstrumentTrack.cpp b/src/tracks/InstrumentTrack.cpp index 2e49479eea4..53e1a21e691 100644 --- a/src/tracks/InstrumentTrack.cpp +++ b/src/tracks/InstrumentTrack.cpp @@ -871,7 +871,7 @@ void InstrumentTrack::saveTrackSpecificSettings( QDomDocument& doc, QDomElement // Only save the MIDI port information if we are not saving a preset. if (!isPresetMode()) { - m_midiPort.saveState( doc, thisElement ); + m_midiPort.saveState(doc, thisElement); } autoAssignMidiDevice(hasAuto); From a1c276debf59d278182f0ebd9658e43c70bca015 Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Mon, 12 Aug 2024 17:01:43 +0200 Subject: [PATCH 07/11] Code style and initialization Make `isPresetMode` a one-liner and initialize `m_presetMode` in the header instead of in the constructor (list). --- include/Track.h | 7 ++----- src/core/Track.cpp | 1 - 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/include/Track.h b/include/Track.h index 3eb995d8676..5bbc6fb00fa 100644 --- a/include/Track.h +++ b/include/Track.h @@ -210,10 +210,7 @@ public slots: void toggleSolo(); protected: - bool isPresetMode() const - { - return m_presetMode; - } + bool isPresetMode() const { return m_presetMode; } private: TrackContainer* m_trackContainer; @@ -228,7 +225,7 @@ public slots: BoolModel m_soloModel; bool m_mutedBeforeSolo; - bool m_presetMode; + bool m_presetMode = false; clipVector m_clips; diff --git a/src/core/Track.cpp b/src/core/Track.cpp index b08c517a5ba..a8342c46663 100644 --- a/src/core/Track.cpp +++ b/src/core/Track.cpp @@ -63,7 +63,6 @@ Track::Track( Type type, TrackContainer * tc ) : m_name(), /*!< The track's name */ m_mutedModel( false, this, tr( "Mute" ) ), /*!< For controlling track muting */ m_soloModel( false, this, tr( "Solo" ) ), /*!< For controlling track soloing */ - m_presetMode(false), m_clips() /*!< The clips (segments) */ { m_trackContainer->addTrack( this ); From eb8633a2e08c4bf3eae6564c602102b87019616e Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Wed, 14 Aug 2024 17:04:51 +0200 Subject: [PATCH 08/11] Enable protected setPresetMode Enable to make `setPresetMode` protected. This is accomplished by adding the new methods `savePreset` and `loadPreset` to `InstrumentTrack`. These methods manage the calling of `setPresetMode` so that callers won't have to know about this implementation detail. Then `saveSettings` and `loadSettings` are called respectively which in turn will delegate to `Track`. All context that use `loadPreset` or `savePreset` already known that they are dealing with an `InstrumentTrack` which is the reason why this works: * Loading of a preset file * Saving a preset * Replacement of an instrument Therefore in these place `loadPreset` and `savePreset` can be called and it's not necessary anymore to call `setPresetMode` there. --- include/InstrumentTrack.h | 3 +++ include/Track.h | 9 ++++----- src/gui/editors/TrackContainerView.cpp | 4 ++-- src/gui/instrument/InstrumentTrackWindow.cpp | 3 +-- src/tracks/InstrumentTrack.cpp | 13 +++++++++++-- 5 files changed, 21 insertions(+), 11 deletions(-) diff --git a/include/InstrumentTrack.h b/include/InstrumentTrack.h index 1e46fb0cb8c..7ad59dba3ef 100644 --- a/include/InstrumentTrack.h +++ b/include/InstrumentTrack.h @@ -137,6 +137,9 @@ class LMMS_EXPORT InstrumentTrack : public Track, public MidiEventProcessor QDomElement & _parent ) override; void loadTrackSpecificSettings( const QDomElement & _this ) override; + void savePreset(QDomDocument & doc, QDomElement & element); + void loadPreset(const QDomElement & element); + using Track::setJournalling; diff --git a/include/Track.h b/include/Track.h index 5bbc6fb00fa..d167c15bf7e 100644 --- a/include/Track.h +++ b/include/Track.h @@ -115,11 +115,6 @@ class LMMS_EXPORT Track : public Model, public JournallingObject void saveSettings( QDomDocument & doc, QDomElement & element ) override; void loadSettings( const QDomElement & element ) override; - void setPresetMode(bool presetMode = true) - { - m_presetMode = presetMode; - } - // -- for usage by Clip only --------------- Clip * addClip( Clip * clip ); void removeClip( Clip * clip ); @@ -211,6 +206,10 @@ public slots: protected: bool isPresetMode() const { return m_presetMode; } + void setPresetMode(bool presetMode = true) + { + m_presetMode = presetMode; + } private: TrackContainer* m_trackContainer; diff --git a/src/gui/editors/TrackContainerView.cpp b/src/gui/editors/TrackContainerView.cpp index 6593247109b..0ab80751831 100644 --- a/src/gui/editors/TrackContainerView.cpp +++ b/src/gui/editors/TrackContainerView.cpp @@ -416,8 +416,8 @@ void TrackContainerView::dropEvent( QDropEvent * _de ) { DataFile dataFile( value ); auto it = dynamic_cast(Track::create(Track::Type::Instrument, m_tc)); - it->setPresetMode(); - it->loadSettings( dataFile.content().toElement() ); + it->loadPreset(dataFile.content().toElement()); + //it->toggledInstrumentTrackButton( true ); _de->accept(); } diff --git a/src/gui/instrument/InstrumentTrackWindow.cpp b/src/gui/instrument/InstrumentTrackWindow.cpp index 31f01c4c235..1d1d2d73cc2 100644 --- a/src/gui/instrument/InstrumentTrackWindow.cpp +++ b/src/gui/instrument/InstrumentTrackWindow.cpp @@ -424,8 +424,7 @@ void InstrumentTrackWindow::saveSettingsBtnClicked() DataFile dataFile(DataFile::Type::InstrumentTrackSettings); QDomElement& content(dataFile.content()); - m_track->setPresetMode(); - m_track->saveSettings(dataFile, content); + m_track->savePreset(dataFile, content); //We don't want to save muted & solo settings when we're saving a preset content.setAttribute("muted", 0); content.setAttribute("solo", 0); diff --git a/src/tracks/InstrumentTrack.cpp b/src/tracks/InstrumentTrack.cpp index 53e1a21e691..53b6c15f768 100644 --- a/src/tracks/InstrumentTrack.cpp +++ b/src/tracks/InstrumentTrack.cpp @@ -994,7 +994,17 @@ void InstrumentTrack::loadTrackSpecificSettings( const QDomElement & thisElement unlock(); } +void InstrumentTrack::savePreset(QDomDocument & doc, QDomElement & element) +{ + setPresetMode(); + saveSettings(doc, element); +} +void InstrumentTrack::loadPreset(const QDomElement & element) +{ + setPresetMode(); + loadSettings(element); +} void InstrumentTrack::setPreviewMode( const bool value ) @@ -1011,14 +1021,13 @@ void InstrumentTrack::replaceInstrument(DataFile dataFile) int mixerChannel = mixerChannelModel()->value(); InstrumentTrack::removeMidiPortNode(dataFile); - setPresetMode(); //Replacing an instrument shouldn't change the solo/mute state. bool oldMute = isMuted(); bool oldSolo = isSolo(); bool oldMutedBeforeSolo = isMutedBeforeSolo(); - loadSettings(dataFile.content().toElement()); + loadPreset(dataFile.content().toElement()); setMuted(oldMute); setSolo(oldSolo); From e3bccb03f687cc3f584d6d2a55baa8e6f0604c40 Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Fri, 16 Aug 2024 17:56:59 +0200 Subject: [PATCH 09/11] Add guard to safely reset bools Add the new local class `BoolGuard` which uses RAII to safely set a boolean to a value and later reset it to the old value. The guard is needed to ensure that the boolean `m_presetMode` is safely set to `true` in `savePreset` and `loadPreset` and reset even in case of exceptions. For this to work `m_presetMode` had to be changed to protected visibility. On this other hand this enables the removal of `setPresetMode`. Add comments to the two places where previously the mode was reset so that people do not wonder why nothing is reset and potentially put the code back in there. --- include/Track.h | 9 +++------ src/core/Track.cpp | 4 ++-- src/tracks/InstrumentTrack.cpp | 34 ++++++++++++++++++++++++++++++++-- 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/include/Track.h b/include/Track.h index d167c15bf7e..2907423bfc4 100644 --- a/include/Track.h +++ b/include/Track.h @@ -206,10 +206,9 @@ public slots: protected: bool isPresetMode() const { return m_presetMode; } - void setPresetMode(bool presetMode = true) - { - m_presetMode = presetMode; - } + +protected: + bool m_presetMode = false; private: TrackContainer* m_trackContainer; @@ -224,8 +223,6 @@ public slots: BoolModel m_soloModel; bool m_mutedBeforeSolo; - bool m_presetMode = false; - clipVector m_clips; QMutex m_processingLock; diff --git a/src/core/Track.cpp b/src/core/Track.cpp index a8342c46663..6921264ae6f 100644 --- a/src/core/Track.cpp +++ b/src/core/Track.cpp @@ -218,7 +218,7 @@ void Track::saveSettings( QDomDocument & doc, QDomElement & element ) if (isPresetMode()) { - setPresetMode(false); + // No need to unset preset mode here as this will done by the guard in InstrumentTrack::savePreset return; } @@ -279,7 +279,7 @@ void Track::loadSettings( const QDomElement & element ) node = node.nextSibling(); } - setPresetMode(false); + // No need to unset preset mode here as this will done by the guard in InstrumentTrack::loadPreset return; } diff --git a/src/tracks/InstrumentTrack.cpp b/src/tracks/InstrumentTrack.cpp index 53b6c15f768..c09de5629ea 100644 --- a/src/tracks/InstrumentTrack.cpp +++ b/src/tracks/InstrumentTrack.cpp @@ -994,15 +994,45 @@ void InstrumentTrack::loadTrackSpecificSettings( const QDomElement & thisElement unlock(); } +/** + * @brief RAII "guard" used to safely change and reset booleans even during exceptions + * + * Needed to safely reset m_presetMode in savePreset and loadPreset. For this reason + * only defined locally because at least the pattern used by these methods should not + * spread outside. + */ +class BoolGuard +{ +public: + BoolGuard(bool& member, bool temporaryValue) : + m_member(member), + m_oldValue(member) + { + m_member = temporaryValue; + } + + ~BoolGuard() + { + m_member = m_oldValue; + } + + BoolGuard(const BoolGuard&) = delete; + BoolGuard& operator=(const BoolGuard&) = delete; + +private: + bool & m_member; + bool const m_oldValue; +}; + void InstrumentTrack::savePreset(QDomDocument & doc, QDomElement & element) { - setPresetMode(); + BoolGuard guard(m_presetMode, true); saveSettings(doc, element); } void InstrumentTrack::loadPreset(const QDomElement & element) { - setPresetMode(); + BoolGuard guard(m_presetMode, true); loadSettings(element); } From 327aa9e999d49e7736b682d685b19026657bc607 Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Sat, 17 Aug 2024 09:54:15 +0200 Subject: [PATCH 10/11] Get rid of m_presetMode and `isPresetMode()` Get rid of the protected member `m_presetMode` and its corresponding method `isPresetMode()`. This is accomplished by introducing two new methods `saveTrack` and `loadTrack`. These methods have a similar interface to `saveSettings` and `loadSettings` but they additionally contain a boolean which indicates if a preset is saved/loaded or a whole track. Both new methods contain the previous code of `saveSettings` and `loadSettings`. The latter two now only delegate to the new methods assuming that the full track is to be stored/loaded if called via the overridden methods `saveSettings` and `loadSettings`. Adjust `saveTrackSpecificSettings` so that it also passes information of whether a preset or a whole track is stored. This leads to changes in the interfaces of `AutomationTrack`, `InstrumentTrack`, `PatternTrack` and `SampleTrack`. Only the implementation of `InstrumentTrack` uses the new information though. Remove the `BoolGuard` class and adjust the implementation of `InstrumentTrack::savePreset` and `InstrumentTrack::loadPreset` to simply delegate to the `saveTrack` and `loadTrack` methods with the correct parameters. The methods `savePreset` and `loadPreset` are kept because they make the code more readable. Implementation notes --------------------- It could be considered to move `savePreset` and `loadPreset` into `Track`. The method `loadTrackSpecificSettings` was not adjusted with an additional boolean because for now no implementation needs to explicitly know if a preset or a track is restored. It could be considered to make the interfaces symmetrical though. --- include/AutomationTrack.h | 3 +-- include/InstrumentTrack.h | 3 +-- include/PatternTrack.h | 3 +-- include/SampleTrack.h | 3 +-- include/Track.h | 11 +++------- src/core/Track.cpp | 34 +++++++++++++++-------------- src/tracks/AutomationTrack.cpp | 3 +-- src/tracks/InstrumentTrack.cpp | 40 ++++------------------------------ src/tracks/PatternTrack.cpp | 2 +- src/tracks/SampleTrack.cpp | 3 +-- 10 files changed, 32 insertions(+), 73 deletions(-) diff --git a/include/AutomationTrack.h b/include/AutomationTrack.h index 64c2cc43aea..9bcb537f41e 100644 --- a/include/AutomationTrack.h +++ b/include/AutomationTrack.h @@ -50,8 +50,7 @@ class AutomationTrack : public Track gui::TrackView * createView( gui::TrackContainerView* ) override; Clip* createClip(const TimePos & pos) override; - void saveTrackSpecificSettings( QDomDocument & _doc, - QDomElement & _parent ) override; + void saveTrackSpecificSettings(QDomDocument& doc, QDomElement& parent, bool presetMode) override; void loadTrackSpecificSettings( const QDomElement & _this ) override; private: diff --git a/include/InstrumentTrack.h b/include/InstrumentTrack.h index 7ad59dba3ef..686adfd8b55 100644 --- a/include/InstrumentTrack.h +++ b/include/InstrumentTrack.h @@ -133,8 +133,7 @@ class LMMS_EXPORT InstrumentTrack : public Track, public MidiEventProcessor // called by track - void saveTrackSpecificSettings( QDomDocument & _doc, - QDomElement & _parent ) override; + void saveTrackSpecificSettings(QDomDocument& doc, QDomElement& parent, bool presetMode) override; void loadTrackSpecificSettings( const QDomElement & _this ) override; void savePreset(QDomDocument & doc, QDomElement & element); diff --git a/include/PatternTrack.h b/include/PatternTrack.h index 1c0c610d255..2568fc91e16 100644 --- a/include/PatternTrack.h +++ b/include/PatternTrack.h @@ -57,8 +57,7 @@ class LMMS_EXPORT PatternTrack : public Track gui::TrackView * createView( gui::TrackContainerView* tcv ) override; Clip* createClip(const TimePos & pos) override; - void saveTrackSpecificSettings( QDomDocument & _doc, - QDomElement & _parent ) override; + void saveTrackSpecificSettings(QDomDocument& doc, QDomElement& parent, bool presetMode) override; void loadTrackSpecificSettings( const QDomElement & _this ) override; static PatternTrack* findPatternTrack(int pattern_num); diff --git a/include/SampleTrack.h b/include/SampleTrack.h index f71f01cd1a7..e79df0c2c04 100644 --- a/include/SampleTrack.h +++ b/include/SampleTrack.h @@ -54,8 +54,7 @@ class SampleTrack : public Track Clip* createClip(const TimePos & pos) override; - void saveTrackSpecificSettings( QDomDocument & _doc, - QDomElement & _parent ) override; + void saveTrackSpecificSettings(QDomDocument& doc, QDomElement& parent, bool presetMode) override; void loadTrackSpecificSettings( const QDomElement & _this ) override; inline IntModel * mixerChannelModel() diff --git a/include/Track.h b/include/Track.h index 2907423bfc4..b1093b578f2 100644 --- a/include/Track.h +++ b/include/Track.h @@ -107,11 +107,12 @@ class LMMS_EXPORT Track : public Model, public JournallingObject virtual gui::TrackView * createView( gui::TrackContainerView * view ) = 0; virtual Clip * createClip( const TimePos & pos ) = 0; - virtual void saveTrackSpecificSettings( QDomDocument & doc, - QDomElement & parent ) = 0; + virtual void saveTrackSpecificSettings(QDomDocument& doc, QDomElement& parent, bool presetMode) = 0; virtual void loadTrackSpecificSettings( const QDomElement & element ) = 0; + void saveTrack(QDomDocument& doc, QDomElement& element, bool presetMode); + void loadTrack(const QDomElement& element, bool presetMode); void saveSettings( QDomDocument & doc, QDomElement & element ) override; void loadSettings( const QDomElement & element ) override; @@ -204,12 +205,6 @@ public slots: void toggleSolo(); -protected: - bool isPresetMode() const { return m_presetMode; } - -protected: - bool m_presetMode = false; - private: TrackContainer* m_trackContainer; Type m_type; diff --git a/src/core/Track.cpp b/src/core/Track.cpp index 6921264ae6f..290f758673c 100644 --- a/src/core/Track.cpp +++ b/src/core/Track.cpp @@ -173,10 +173,6 @@ Track* Track::clone() } - - - - /*! \brief Save this track's settings to file * * We save the track type and its muted state and solo state, then append the track- @@ -185,12 +181,13 @@ Track* Track::clone() * * \param doc The QDomDocument to use to save * \param element The The QDomElement to save into + * \param presetMode Describes whether to save the track as a preset or not. * \todo Does this accurately describe the parameters? I think not!? * \todo Save the track height */ -void Track::saveSettings( QDomDocument & doc, QDomElement & element ) +void Track::saveTrack(QDomDocument& doc, QDomElement& element, bool presetMode) { - if (!isPresetMode()) + if (!presetMode) { element.setTagName( "track" ); } @@ -214,11 +211,10 @@ void Track::saveSettings( QDomDocument & doc, QDomElement & element ) QDomElement tsDe = doc.createElement( nodeName() ); // let actual track (InstrumentTrack, PatternTrack, SampleTrack etc.) save its settings element.appendChild( tsDe ); - saveTrackSpecificSettings( doc, tsDe ); + saveTrackSpecificSettings(doc, tsDe, presetMode); - if (isPresetMode()) + if (presetMode) { - // No need to unset preset mode here as this will done by the guard in InstrumentTrack::savePreset return; } @@ -229,9 +225,6 @@ void Track::saveSettings( QDomDocument & doc, QDomElement & element ) } } - - - /*! \brief Load the settings from a file * * We load the track's type and muted state and solo state, then clear out our @@ -242,9 +235,10 @@ void Track::saveSettings( QDomDocument & doc, QDomElement & element ) * one at a time. * * \param element the QDomElement to load track settings from + * \param presetMode Indicates if a preset or a full track is loaded * \todo Load the track height. */ -void Track::loadSettings( const QDomElement & element ) +void Track::loadTrack(const QDomElement& element, bool presetMode) { if( static_cast(element.attribute( "type" ).toInt()) != type() ) { @@ -266,7 +260,7 @@ void Track::loadSettings( const QDomElement & element ) setColor(QColor{element.attribute("color")}); } - if (isPresetMode()) + if (presetMode) { QDomNode node = element.firstChild(); while( !node.isNull() ) @@ -279,8 +273,6 @@ void Track::loadSettings( const QDomElement & element ) node = node.nextSibling(); } - // No need to unset preset mode here as this will done by the guard in InstrumentTrack::loadPreset - return; } @@ -317,6 +309,16 @@ void Track::loadSettings( const QDomElement & element ) } } +void Track::saveSettings(QDomDocument& doc, QDomElement& element) +{ + saveTrack(doc, element, false); +} + +void Track::loadSettings(const QDomElement& element) +{ + loadTrack(element, false); +} + diff --git a/src/tracks/AutomationTrack.cpp b/src/tracks/AutomationTrack.cpp index e353197f877..1ff30ac7680 100644 --- a/src/tracks/AutomationTrack.cpp +++ b/src/tracks/AutomationTrack.cpp @@ -66,8 +66,7 @@ Clip* AutomationTrack::createClip(const TimePos & pos) -void AutomationTrack::saveTrackSpecificSettings( QDomDocument & _doc, - QDomElement & _this ) +void AutomationTrack::saveTrackSpecificSettings(QDomDocument& doc, QDomElement& _this, bool presetMode) { } diff --git a/src/tracks/InstrumentTrack.cpp b/src/tracks/InstrumentTrack.cpp index c09de5629ea..c3c80315161 100644 --- a/src/tracks/InstrumentTrack.cpp +++ b/src/tracks/InstrumentTrack.cpp @@ -821,7 +821,7 @@ gui::TrackView* InstrumentTrack::createView( gui::TrackContainerView* tcv ) -void InstrumentTrack::saveTrackSpecificSettings( QDomDocument& doc, QDomElement & thisElement ) +void InstrumentTrack::saveTrackSpecificSettings(QDomDocument& doc, QDomElement& thisElement, bool presetMode) { m_volumeModel.saveSettings( doc, thisElement, "vol" ); m_panningModel.saveSettings( doc, thisElement, "pan" ); @@ -869,7 +869,7 @@ void InstrumentTrack::saveTrackSpecificSettings( QDomDocument& doc, QDomElement autoAssignMidiDevice(false); // Only save the MIDI port information if we are not saving a preset. - if (!isPresetMode()) + if (!presetMode) { m_midiPort.saveState(doc, thisElement); } @@ -994,46 +994,14 @@ void InstrumentTrack::loadTrackSpecificSettings( const QDomElement & thisElement unlock(); } -/** - * @brief RAII "guard" used to safely change and reset booleans even during exceptions - * - * Needed to safely reset m_presetMode in savePreset and loadPreset. For this reason - * only defined locally because at least the pattern used by these methods should not - * spread outside. - */ -class BoolGuard -{ -public: - BoolGuard(bool& member, bool temporaryValue) : - m_member(member), - m_oldValue(member) - { - m_member = temporaryValue; - } - - ~BoolGuard() - { - m_member = m_oldValue; - } - - BoolGuard(const BoolGuard&) = delete; - BoolGuard& operator=(const BoolGuard&) = delete; - -private: - bool & m_member; - bool const m_oldValue; -}; - void InstrumentTrack::savePreset(QDomDocument & doc, QDomElement & element) { - BoolGuard guard(m_presetMode, true); - saveSettings(doc, element); + saveTrack(doc, element, true); } void InstrumentTrack::loadPreset(const QDomElement & element) { - BoolGuard guard(m_presetMode, true); - loadSettings(element); + loadTrack(element, true); } diff --git a/src/tracks/PatternTrack.cpp b/src/tracks/PatternTrack.cpp index bdde4780c50..697a7c2a8fb 100644 --- a/src/tracks/PatternTrack.cpp +++ b/src/tracks/PatternTrack.cpp @@ -154,7 +154,7 @@ Clip* PatternTrack::createClip(const TimePos & pos) -void PatternTrack::saveTrackSpecificSettings(QDomDocument& doc, QDomElement& _this) +void PatternTrack::saveTrackSpecificSettings(QDomDocument& doc, QDomElement& _this, bool presetMode) { // _this.setAttribute( "icon", m_trackLabel->pixmapFile() ); /* _this.setAttribute( "current", s_infoMap[this] == diff --git a/src/tracks/SampleTrack.cpp b/src/tracks/SampleTrack.cpp index 13050285668..609043efe12 100644 --- a/src/tracks/SampleTrack.cpp +++ b/src/tracks/SampleTrack.cpp @@ -188,8 +188,7 @@ Clip * SampleTrack::createClip(const TimePos & pos) -void SampleTrack::saveTrackSpecificSettings( QDomDocument & _doc, - QDomElement & _this ) +void SampleTrack::saveTrackSpecificSettings(QDomDocument& _doc, QDomElement& _this, bool presetMode) { m_audioPort.effects()->saveState( _doc, _this ); #if 0 From 4e2158bbccbbd71ab165a021893ad8c5f2ab19de Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Sun, 18 Aug 2024 09:25:21 +0200 Subject: [PATCH 11/11] Move preset methods into Track Move the methods `savePreset` and `loadPreset` into track because it potentially makes sense for all types of tracks to save a preset. Make `Track::saveTrack` and `Track::loadTrack` private because the public interface for saving and loading full tracks is `saveSettings` and `loadSettings` and for saving and loading presets it's `savePreset` and `loadPreset`. --- include/InstrumentTrack.h | 3 --- include/Track.h | 10 ++++++++-- src/core/Track.cpp | 12 ++++++++++++ src/tracks/InstrumentTrack.cpp | 8 -------- 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/include/InstrumentTrack.h b/include/InstrumentTrack.h index 686adfd8b55..689c962cdab 100644 --- a/include/InstrumentTrack.h +++ b/include/InstrumentTrack.h @@ -136,9 +136,6 @@ class LMMS_EXPORT InstrumentTrack : public Track, public MidiEventProcessor void saveTrackSpecificSettings(QDomDocument& doc, QDomElement& parent, bool presetMode) override; void loadTrackSpecificSettings( const QDomElement & _this ) override; - void savePreset(QDomDocument & doc, QDomElement & element); - void loadPreset(const QDomElement & element); - using Track::setJournalling; diff --git a/include/Track.h b/include/Track.h index b1093b578f2..52d43f6e115 100644 --- a/include/Track.h +++ b/include/Track.h @@ -110,9 +110,11 @@ class LMMS_EXPORT Track : public Model, public JournallingObject virtual void saveTrackSpecificSettings(QDomDocument& doc, QDomElement& parent, bool presetMode) = 0; virtual void loadTrackSpecificSettings( const QDomElement & element ) = 0; + // Saving and loading of presets which do not necessarily contain all the track information + void savePreset(QDomDocument & doc, QDomElement & element); + void loadPreset(const QDomElement & element); - void saveTrack(QDomDocument& doc, QDomElement& element, bool presetMode); - void loadTrack(const QDomElement& element, bool presetMode); + // Saving and loading of full tracks void saveSettings( QDomDocument & doc, QDomElement & element ) override; void loadSettings( const QDomElement & element ) override; @@ -205,6 +207,10 @@ public slots: void toggleSolo(); +private: + void saveTrack(QDomDocument& doc, QDomElement& element, bool presetMode); + void loadTrack(const QDomElement& element, bool presetMode); + private: TrackContainer* m_trackContainer; Type m_type; diff --git a/src/core/Track.cpp b/src/core/Track.cpp index 290f758673c..e44475d9374 100644 --- a/src/core/Track.cpp +++ b/src/core/Track.cpp @@ -309,13 +309,25 @@ void Track::loadTrack(const QDomElement& element, bool presetMode) } } +void Track::savePreset(QDomDocument & doc, QDomElement & element) +{ + saveTrack(doc, element, true); +} + +void Track::loadPreset(const QDomElement & element) +{ + loadTrack(element, true); +} + void Track::saveSettings(QDomDocument& doc, QDomElement& element) { + // Assume that everything should be saved if we are called through SerializingObject::saveSettings saveTrack(doc, element, false); } void Track::loadSettings(const QDomElement& element) { + // Assume that everything should be loaded if we are called through SerializingObject::loadSettings loadTrack(element, false); } diff --git a/src/tracks/InstrumentTrack.cpp b/src/tracks/InstrumentTrack.cpp index c3c80315161..66992bc4f6d 100644 --- a/src/tracks/InstrumentTrack.cpp +++ b/src/tracks/InstrumentTrack.cpp @@ -994,15 +994,7 @@ void InstrumentTrack::loadTrackSpecificSettings( const QDomElement & thisElement unlock(); } -void InstrumentTrack::savePreset(QDomDocument & doc, QDomElement & element) -{ - saveTrack(doc, element, true); -} -void InstrumentTrack::loadPreset(const QDomElement & element) -{ - loadTrack(element, true); -} void InstrumentTrack::setPreviewMode( const bool value )