From 8821d88c098ce886ed82df048c89c0106482a702 Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Sun, 9 Mar 2025 11:25:00 +0100 Subject: [PATCH] InstrumentSoundShaping refactoring (#7229) * Accessors for volume, cutoff, resonance Add private accessors for the volume, cutoff and resonance parameters (envelope and LFO). This makes the code less technical and more readable as it for example removes lots of static casts. The casts are now done in a special helper method that returns the parameters for a given target. * Remove EnvelopeAndLfoParameters array Remove the `EnvelopeAndLfoParameters` array and use explicit instances for volume, cutoff, resonance. This simplifies construction and initialization of the instances. Besides the array this also removes the `Target` enum and the `NumTargets` value. To simplify storage and retrieval of the parameters three private methods have been added which provide the node names of the volume, cutoff and resonance parameters. Adjust `InstrumentSoundShapingView` to the removed `NumTargets` property. * Use references instead of pointers Use references to the volume, cutoff and resonance parameters instead of pointers. * Remove friend relationship Remove the friend relationship between `InstrumentSoundShaping` and its related view by providing the models via getters. * Get rid of targetNames Get rid of `InstrumentSoundShaping::targetNames` by using translations and strings directly. Move the remaining stuff into `InstrumentSoundShapingView` until it is removed there as well. * Explicit EnvelopeAndLfoViews Remove the array of EnvelopeAndLfoViews and use dedicated instances instead. This also enables the final removal of the remaining `targetNames`. * Move the code of some getters Move the code of some getters into the header file. * Several code review changes Apply some code review proposals. Co-authored-by: Sotonye Atemie --------- Co-authored-by: Sotonye Atemie --- include/InstrumentSoundShaping.h | 40 +++-- include/InstrumentSoundShapingView.h | 8 +- src/core/InstrumentSoundShaping.cpp | 153 +++++++++--------- .../instrument/InstrumentSoundShapingView.cpp | 29 ++-- 4 files changed, 124 insertions(+), 106 deletions(-) diff --git a/include/InstrumentSoundShaping.h b/include/InstrumentSoundShaping.h index 7dfeaf58b..4bdaa9b3b 100644 --- a/include/InstrumentSoundShaping.h +++ b/include/InstrumentSoundShaping.h @@ -26,13 +26,13 @@ #define LMMS_INSTRUMENT_SOUND_SHAPING_H #include "ComboBoxModel.h" +#include "EnvelopeAndLfoParameters.h" namespace lmms { class InstrumentTrack; -class EnvelopeAndLfoParameters; class NotePlayHandle; class SampleFrame; @@ -52,14 +52,19 @@ public: void processAudioBuffer( SampleFrame* _ab, const fpp_t _frames, NotePlayHandle * _n ); - enum class Target - { - Volume, - Cut, - Resonance, - Count - } ; - constexpr static auto NumTargets = static_cast(Target::Count); + const EnvelopeAndLfoParameters& getVolumeParameters() const { return m_volumeParameters; } + EnvelopeAndLfoParameters& getVolumeParameters() { return m_volumeParameters; } + + const EnvelopeAndLfoParameters& getCutoffParameters() const { return m_cutoffParameters; } + EnvelopeAndLfoParameters& getCutoffParameters() { return m_cutoffParameters; } + + const EnvelopeAndLfoParameters& getResonanceParameters() const { return m_resonanceParameters; } + EnvelopeAndLfoParameters& getResonanceParameters() { return m_resonanceParameters; } + + BoolModel& getFilterEnabledModel() { return m_filterEnabledModel; } + ComboBoxModel& getFilterModel() { return m_filterModel; } + FloatModel& getFilterCutModel() { return m_filterCutModel; } + FloatModel& getFilterResModel() { return m_filterResModel; } f_cnt_t envFrames( const bool _only_vol = false ) const; f_cnt_t releaseFrames() const; @@ -74,22 +79,23 @@ public: return "eldata"; } +private: + QString getVolumeNodeName() const; + QString getCutoffNodeName() const; + QString getResonanceNodeName() const; private: - EnvelopeAndLfoParameters * m_envLfoParameters[NumTargets]; InstrumentTrack * m_instrumentTrack; + EnvelopeAndLfoParameters m_volumeParameters; + EnvelopeAndLfoParameters m_cutoffParameters; + EnvelopeAndLfoParameters m_resonanceParameters; + BoolModel m_filterEnabledModel; ComboBoxModel m_filterModel; FloatModel m_filterCutModel; FloatModel m_filterResModel; - - static const char *const targetNames[NumTargets][3]; - - - friend class gui::InstrumentSoundShapingView; - -} ; +}; } // namespace lmms diff --git a/include/InstrumentSoundShapingView.h b/include/InstrumentSoundShapingView.h index c9caea28c..b9e1fe82b 100644 --- a/include/InstrumentSoundShapingView.h +++ b/include/InstrumentSoundShapingView.h @@ -58,7 +58,10 @@ private: InstrumentSoundShaping * m_ss = nullptr; TabWidget * m_targetsTabWidget; - EnvelopeAndLfoView * m_envLfoViews[InstrumentSoundShaping::NumTargets]; + + EnvelopeAndLfoView* m_volumeView; + EnvelopeAndLfoView* m_cutoffView; + EnvelopeAndLfoView* m_resonanceView; // filter-stuff GroupBox * m_filterGroupBox; @@ -67,8 +70,7 @@ private: Knob * m_filterResKnob; QLabel* m_singleStreamInfoLabel; - -} ; +}; } // namespace lmms::gui diff --git a/src/core/InstrumentSoundShaping.cpp b/src/core/InstrumentSoundShaping.cpp index eca06f9a2..93f4fb45b 100644 --- a/src/core/InstrumentSoundShaping.cpp +++ b/src/core/InstrumentSoundShaping.cpp @@ -30,7 +30,6 @@ #include "BasicFilters.h" #include "embed.h" #include "Engine.h" -#include "EnvelopeAndLfoParameters.h" #include "Instrument.h" #include "InstrumentTrack.h" @@ -43,42 +42,21 @@ const float RES_MULTIPLIER = 2.0f; const float RES_PRECISION = 1000.0f; -// names for env- and lfo-targets - first is name being displayed to user -// and second one is used internally, e.g. for saving/restoring settings -const char *const InstrumentSoundShaping::targetNames[InstrumentSoundShaping::NumTargets][3] = -{ - { QT_TRANSLATE_NOOP("InstrumentSoundShaping", "VOLUME"), "vol", - QT_TRANSLATE_NOOP("InstrumentSoundShaping", "Volume") }, - { QT_TRANSLATE_NOOP("InstrumentSoundShaping", "CUTOFF"), "cut", - QT_TRANSLATE_NOOP("InstrumentSoundShaping", "Cutoff frequency") }, - { QT_TRANSLATE_NOOP("InstrumentSoundShaping", "RESO"), "res", - QT_TRANSLATE_NOOP("InstrumentSoundShaping", "Resonance") } -} ; - - - InstrumentSoundShaping::InstrumentSoundShaping( InstrumentTrack * _instrument_track ) : Model( _instrument_track, tr( "Envelopes/LFOs" ) ), m_instrumentTrack( _instrument_track ), + m_volumeParameters(1., this), + m_cutoffParameters(0., this), + m_resonanceParameters(0., this), m_filterEnabledModel( false, this ), m_filterModel( this, tr( "Filter type" ) ), m_filterCutModel( 14000.0, 1.0, 14000.0, 1.0, this, tr( "Cutoff frequency" ) ), m_filterResModel(0.5f, BasicFilters<>::minQ(), 10.f, 0.01f, this, tr("Q/Resonance")) { - for (auto i = std::size_t{0}; i < NumTargets; ++i) - { - float value_for_zero_amount = 0.0; - if( static_cast(i) == Target::Volume ) - { - value_for_zero_amount = 1.0; - } - m_envLfoParameters[i] = new EnvelopeAndLfoParameters( - value_for_zero_amount, - this ); - m_envLfoParameters[i]->setDisplayName( - tr( targetNames[i][2] ) ); - } + m_volumeParameters.setDisplayName(tr("Volume")); + m_cutoffParameters.setDisplayName(tr("Cutoff frequency")); + m_resonanceParameters.setDisplayName(tr("Resonance")); m_filterModel.addItem( tr( "Low-pass" ), std::make_unique( "filter_lp" ) ); m_filterModel.addItem( tr( "Hi-pass" ), std::make_unique( "filter_hp" ) ); @@ -119,7 +97,7 @@ float InstrumentSoundShaping::volumeLevel( NotePlayHandle* n, const f_cnt_t fram } float level; - m_envLfoParameters[static_cast(Target::Volume)]->fillLevel( &level, frame, envReleaseBegin, 1 ); + getVolumeParameters().fillLevel(&level, frame, envReleaseBegin, 1); return level; } @@ -148,6 +126,9 @@ void InstrumentSoundShaping::processAudioBuffer( SampleFrame* buffer, // only use filter, if it is really needed + auto& cutoffParameters = getCutoffParameters(); + auto& resonanceParameters = getResonanceParameters(); + if( m_filterEnabledModel.value() ) { QVarLengthArray cutBuffer(frames); @@ -162,20 +143,20 @@ void InstrumentSoundShaping::processAudioBuffer( SampleFrame* buffer, } n->m_filter->setFilterType( static_cast::FilterType>(m_filterModel.value()) ); - if( m_envLfoParameters[static_cast(Target::Cut)]->isUsed() ) + if (cutoffParameters.isUsed()) { - m_envLfoParameters[static_cast(Target::Cut)]->fillLevel( cutBuffer.data(), envTotalFrames, envReleaseBegin, frames ); + cutoffParameters.fillLevel(cutBuffer.data(), envTotalFrames, envReleaseBegin, frames); } - if( m_envLfoParameters[static_cast(Target::Resonance)]->isUsed() ) + + if (resonanceParameters.isUsed()) { - m_envLfoParameters[static_cast(Target::Resonance)]->fillLevel( resBuffer.data(), envTotalFrames, envReleaseBegin, frames ); + resonanceParameters.fillLevel(resBuffer.data(), envTotalFrames, envReleaseBegin, frames); } const float fcv = m_filterCutModel.value(); const float frv = m_filterResModel.value(); - if( m_envLfoParameters[static_cast(Target::Cut)]->isUsed() && - m_envLfoParameters[static_cast(Target::Resonance)]->isUsed() ) + if (cutoffParameters.isUsed() && resonanceParameters.isUsed()) { for( fpp_t frame = 0; frame < frames; ++frame ) { @@ -196,7 +177,7 @@ void InstrumentSoundShaping::processAudioBuffer( SampleFrame* buffer, buffer[frame][1] = n->m_filter->update( buffer[frame][1], 1 ); } } - else if( m_envLfoParameters[static_cast(Target::Cut)]->isUsed() ) + else if (cutoffParameters.isUsed()) { for( fpp_t frame = 0; frame < frames; ++frame ) { @@ -213,7 +194,7 @@ void InstrumentSoundShaping::processAudioBuffer( SampleFrame* buffer, buffer[frame][1] = n->m_filter->update( buffer[frame][1], 1 ); } } - else if( m_envLfoParameters[static_cast(Target::Resonance)]->isUsed() ) + else if(resonanceParameters.isUsed() ) { for( fpp_t frame = 0; frame < frames; ++frame ) { @@ -241,10 +222,12 @@ void InstrumentSoundShaping::processAudioBuffer( SampleFrame* buffer, } } - if( m_envLfoParameters[static_cast(Target::Volume)]->isUsed() ) + auto& volumeParameters = getVolumeParameters(); + + if (volumeParameters.isUsed()) { QVarLengthArray volBuffer(frames); - m_envLfoParameters[static_cast(Target::Volume)]->fillLevel( volBuffer.data(), envTotalFrames, envReleaseBegin, frames ); + volumeParameters.fillLevel(volBuffer.data(), envTotalFrames, envReleaseBegin, frames); for( fpp_t frame = 0; frame < frames; ++frame ) { @@ -275,19 +258,23 @@ void InstrumentSoundShaping::processAudioBuffer( SampleFrame* buffer, f_cnt_t InstrumentSoundShaping::envFrames( const bool _only_vol ) const { - f_cnt_t ret_val = m_envLfoParameters[static_cast(Target::Volume)]->PAHD_Frames(); + f_cnt_t ret_val = getVolumeParameters().PAHD_Frames(); - if( _only_vol == false ) + if (!_only_vol) { - for (auto i = static_cast(Target::Volume) + 1; i < NumTargets; ++i) + auto& cutoffParameters = getCutoffParameters(); + if (cutoffParameters.isUsed()) { - if( m_envLfoParameters[i]->isUsed() && - m_envLfoParameters[i]->PAHD_Frames() > ret_val ) - { - ret_val = m_envLfoParameters[i]->PAHD_Frames(); - } + ret_val = std::max(ret_val, cutoffParameters.PAHD_Frames()); + } + + auto& resonanceParameters = getResonanceParameters(); + if (resonanceParameters.isUsed()) + { + ret_val = std::max(ret_val, resonanceParameters.PAHD_Frames()); } } + return ret_val; } @@ -308,23 +295,33 @@ f_cnt_t InstrumentSoundShaping::releaseFrames() const return ret_val; } - if( m_envLfoParameters[static_cast(Target::Volume)]->isUsed() ) + auto& volumeParameters = getVolumeParameters(); + + if (volumeParameters.isUsed()) { - return m_envLfoParameters[static_cast(Target::Volume)]->releaseFrames(); + return volumeParameters.releaseFrames(); } - for (auto i = static_cast(Target::Volume) + 1; i < NumTargets; ++i) + auto& cutoffParameters = getCutoffParameters(); + if (cutoffParameters.isUsed()) { - if( m_envLfoParameters[i]->isUsed() ) - { - ret_val = std::max(ret_val, m_envLfoParameters[i]->releaseFrames()); - } + ret_val = std::max(ret_val, cutoffParameters.releaseFrames()); } + + auto& resonanceParameters = getResonanceParameters(); + if (resonanceParameters.isUsed()) + { + ret_val = std::max(ret_val, resonanceParameters.releaseFrames()); + } + return ret_val; } - +static void saveEnvelopeAndLFOParameters(EnvelopeAndLfoParameters& p, const QString & tagName, QDomDocument & _doc, QDomElement & _this) +{ + p.saveState(_doc, _this).setTagName(tagName); +} void InstrumentSoundShaping::saveSettings( QDomDocument & _doc, QDomElement & _this ) { @@ -333,12 +330,9 @@ void InstrumentSoundShaping::saveSettings( QDomDocument & _doc, QDomElement & _t m_filterResModel.saveSettings( _doc, _this, "fres" ); m_filterEnabledModel.saveSettings( _doc, _this, "fwet" ); - for (auto i = std::size_t{0}; i < NumTargets; ++i) - { - m_envLfoParameters[i]->saveState( _doc, _this ).setTagName( - m_envLfoParameters[i]->nodeName() + - QString( targetNames[i][1] ).toLower() ); - } + saveEnvelopeAndLFOParameters(getVolumeParameters(), getVolumeNodeName(), _doc, _this); + saveEnvelopeAndLFOParameters(getCutoffParameters(), getCutoffNodeName(), _doc, _this); + saveEnvelopeAndLFOParameters(getResonanceParameters(), getResonanceNodeName(), _doc, _this); } @@ -352,27 +346,42 @@ void InstrumentSoundShaping::loadSettings( const QDomElement & _this ) m_filterEnabledModel.loadSettings( _this, "fwet" ); QDomNode node = _this.firstChild(); - while( !node.isNull() ) + while (!node.isNull()) { - if( node.isElement() ) + if (node.isElement()) { - for (auto i = std::size_t{0}; i < NumTargets; ++i) + const auto nodeName = node.nodeName(); + if (nodeName == getVolumeNodeName()) { - if( node.nodeName() == - m_envLfoParameters[i]->nodeName() + - QString( targetNames[i][1] ). - toLower() ) - { - m_envLfoParameters[i]->restoreState( node.toElement() ); - } + getVolumeParameters().restoreState(node.toElement()); + } + else if (nodeName == getCutoffNodeName()) + { + getCutoffParameters().restoreState(node.toElement()); + } + else if (nodeName == getResonanceNodeName()) + { + getResonanceParameters().restoreState(node.toElement()); } } + node = node.nextSibling(); } } +QString InstrumentSoundShaping::getVolumeNodeName() const +{ + return getVolumeParameters().nodeName() + "vol"; +} +QString InstrumentSoundShaping::getCutoffNodeName() const +{ + return getCutoffParameters().nodeName() + "cut"; +} - +QString InstrumentSoundShaping::getResonanceNodeName() const +{ + return getResonanceParameters().nodeName() + "res"; +} } // namespace lmms diff --git a/src/gui/instrument/InstrumentSoundShapingView.cpp b/src/gui/instrument/InstrumentSoundShapingView.cpp index 45abace19..e0d6a6e98 100644 --- a/src/gui/instrument/InstrumentSoundShapingView.cpp +++ b/src/gui/instrument/InstrumentSoundShapingView.cpp @@ -48,12 +48,13 @@ InstrumentSoundShapingView::InstrumentSoundShapingView(QWidget* parent) : m_targetsTabWidget = new TabWidget(tr("TARGET"), this); - for (auto i = std::size_t{0}; i < InstrumentSoundShaping::NumTargets; ++i) - { - m_envLfoViews[i] = new EnvelopeAndLfoView(m_targetsTabWidget); - m_targetsTabWidget->addTab(m_envLfoViews[i], - tr(InstrumentSoundShaping::targetNames[i][0]), nullptr); - } + m_volumeView = new EnvelopeAndLfoView(m_targetsTabWidget); + m_cutoffView = new EnvelopeAndLfoView(m_targetsTabWidget); + m_resonanceView = new EnvelopeAndLfoView(m_targetsTabWidget); + + m_targetsTabWidget->addTab(m_volumeView, tr("VOLUME"), nullptr); + m_targetsTabWidget->addTab(m_cutoffView, tr("CUTOFF"), nullptr); + m_targetsTabWidget->addTab(m_resonanceView, tr("RESO"), nullptr); mainLayout->addWidget(m_targetsTabWidget, 1); @@ -111,14 +112,14 @@ void InstrumentSoundShapingView::setFunctionsHidden( bool hidden ) void InstrumentSoundShapingView::modelChanged() { m_ss = castModel(); - m_filterGroupBox->setModel( &m_ss->m_filterEnabledModel ); - m_filterComboBox->setModel( &m_ss->m_filterModel ); - m_filterCutKnob->setModel( &m_ss->m_filterCutModel ); - m_filterResKnob->setModel( &m_ss->m_filterResModel ); - for (auto i = std::size_t{0}; i < InstrumentSoundShaping::NumTargets; ++i) - { - m_envLfoViews[i]->setModel( m_ss->m_envLfoParameters[i] ); - } + m_filterGroupBox->setModel(&m_ss->getFilterEnabledModel()); + m_filterComboBox->setModel(&m_ss->getFilterModel()); + m_filterCutKnob->setModel(&m_ss->getFilterCutModel()); + m_filterResKnob->setModel(&m_ss->getFilterResModel()); + + m_volumeView->setModel(&m_ss->getVolumeParameters()); + m_cutoffView->setModel(&m_ss->getCutoffParameters()); + m_resonanceView->setModel(&m_ss->getResonanceParameters()); }