From 7822fc0af09999cc7b05cc97870b39a8f1ad80cf Mon Sep 17 00:00:00 2001 From: DigArtRoks <69391149+DigArtRoks@users.noreply.github.com> Date: Fri, 28 Aug 2020 22:35:38 +0200 Subject: [PATCH 1/2] Fix for Mixer volume percentage labels are off by a factor of 100 (LMMS#2340) Add m_conversionFactor to the AutomatableModelView. This factor will be applied to the model->value when displaying it in the contextmenu of the control for the reset and copy actions. The factor will be applied when copying the value to the clipboard. When pasting from the clipboard, the value will be divided by the factor. Remove the model->displayValue() calls when updating the reset/copy/paste action text labels as this gives e.g. in the Equalizer the wrong action text for the Frequency knobs. In the Fader class, remove the m_displayConversion variable but rather use the new m_conversionFactor variable. Rewire the setDisplayConversion() function to set the m_conversionFactor to 1.0 or 100.0. Faders in FxMixer show now the correct context menu. Copying and pasting values between faders or even volume knobs in tracks shows consistent behavior. Other faders (like in Eq) show the old behavior. --- include/AutomatableModelView.h | 4 ++++ include/Fader.h | 5 ++--- src/gui/AutomatableModelView.cpp | 25 +++++++++++++++++------ src/gui/widgets/Fader.cpp | 35 ++++++++++---------------------- 4 files changed, 36 insertions(+), 33 deletions(-) diff --git a/include/AutomatableModelView.h b/include/AutomatableModelView.h index 1bcbd97d6fe..d5fa76a9a00 100644 --- a/include/AutomatableModelView.h +++ b/include/AutomatableModelView.h @@ -69,12 +69,16 @@ class LMMS_EXPORT AutomatableModelView : public ModelView void addDefaultActions( QMenu* menu ); + void setConversionFactor( float factor ); + float getConversionFactor(); + protected: virtual void mousePressEvent( QMouseEvent* event ); QString m_description; QString m_unit; + float m_conversionFactor; // Factor to be applied when the m_model->value is displayed } ; diff --git a/include/Fader.h b/include/Fader.h index 2072154459d..be31cc890a4 100644 --- a/include/Fader.h +++ b/include/Fader.h @@ -98,7 +98,7 @@ class LMMS_EXPORT Fader : public QWidget, public FloatModelView void setDisplayConversion( bool b ) { - m_displayConversion = b; + m_conversionFactor = b ? 100.0 : 1.0; } inline void setHintText( const QString & _txt_before, @@ -154,8 +154,7 @@ class LMMS_EXPORT Fader : public QWidget, public FloatModelView QPixmap * m_back; QPixmap * m_leds; QPixmap * m_knob; - - bool m_displayConversion; + bool m_levelsDisplayedInDBFS; int m_moveStartPoint; diff --git a/src/gui/AutomatableModelView.cpp b/src/gui/AutomatableModelView.cpp index 039c75c9953..71f3ff8a3bb 100644 --- a/src/gui/AutomatableModelView.cpp +++ b/src/gui/AutomatableModelView.cpp @@ -42,7 +42,8 @@ static float floatFromClipboard(bool* ok=nullptr); AutomatableModelView::AutomatableModelView( ::Model* model, QWidget* _this ) : - ModelView( model, _this ) + ModelView( model, _this ), + m_conversionFactor( 1.0 ) { widget()->setAcceptDrops( true ); widget()->setCursor( QCursor( embed::getIconPixmap( "hand" ), 3, 3 ) ); @@ -56,14 +57,14 @@ void AutomatableModelView::addDefaultActions( QMenu* menu ) menu->addAction( embed::getIconPixmap( "reload" ), AutomatableModel::tr( "&Reset (%1%2)" ). - arg( model->displayValue( model->initValue() ) ). + arg( model->initValue() * m_conversionFactor ). arg( m_unit ), model, SLOT( reset() ) ); menu->addSeparator(); menu->addAction( embed::getIconPixmap( "edit_copy" ), AutomatableModel::tr( "&Copy value (%1%2)" ). - arg( model->displayValue( model->value() ) ). + arg( model->value() * m_conversionFactor ). arg( m_unit ), amvSlots, SLOT( copyToClipboard() ) ); @@ -71,7 +72,7 @@ void AutomatableModelView::addDefaultActions( QMenu* menu ) const float valueToPaste = floatFromClipboard(&canPaste); const QString pasteDesc = canPaste ? AutomatableModel::tr( "&Paste value (%1%2)"). - arg( model->displayValue( valueToPaste ) ). + arg( valueToPaste ). arg( m_unit ) : AutomatableModel::tr( "&Paste value"); QAction* pasteAction = menu->addAction( embed::getIconPixmap( "edit_paste" ), @@ -155,8 +156,20 @@ void AutomatableModelView::mousePressEvent( QMouseEvent* event ) } +void AutomatableModelView::setConversionFactor( float factor ) +{ + if( factor != 0.0 ) + { + m_conversionFactor = factor; + } +} +float AutomatableModelView::getConversionFactor() +{ + return m_conversionFactor; +} + AutomatableModelViewSlots::AutomatableModelViewSlots( AutomatableModelView* amv, QObject* parent ) : QObject(), @@ -243,7 +256,7 @@ void AutomatableModelViewSlots::unlinkAllModels() void AutomatableModelViewSlots::copyToClipboard() { QClipboard* clipboard = QApplication::clipboard(); - clipboard->setText(QString::number(m_amv->value())); + clipboard->setText(QString::number(m_amv->value() * m_amv->getConversionFactor())); } void AutomatableModelViewSlots::pasteFromClipboard() @@ -251,7 +264,7 @@ void AutomatableModelViewSlots::pasteFromClipboard() bool isNumber = false; const float number = floatFromClipboard(&isNumber); if (isNumber) { - m_amv->modelUntyped()->setValue(number); + m_amv->modelUntyped()->setValue(number / m_amv->getConversionFactor()); } } diff --git a/src/gui/widgets/Fader.cpp b/src/gui/widgets/Fader.cpp index 4317066ab65..f19ebdb76ce 100644 --- a/src/gui/widgets/Fader.cpp +++ b/src/gui/widgets/Fader.cpp @@ -72,7 +72,6 @@ Fader::Fader( FloatModel * _model, const QString & _name, QWidget * _parent ) : m_persistentPeak_R( 0.0 ), m_fMinPeak( 0.01f ), m_fMaxPeak( 1.1 ), - m_displayConversion( true ), m_levelsDisplayedInDBFS(false), m_moveStartPoint( -1 ), m_startValue( 0 ), @@ -102,6 +101,8 @@ Fader::Fader( FloatModel * _model, const QString & _name, QWidget * _parent ) : m_knob = s_knob; init(_model, _name); + + m_conversionFactor = 100.0; } @@ -114,7 +115,6 @@ Fader::Fader( FloatModel * model, const QString & name, QWidget * parent, QPixma m_persistentPeak_R( 0.0 ), m_fMinPeak( 0.01f ), m_fMaxPeak( 1.1 ), - m_displayConversion( false ), m_levelsDisplayedInDBFS(false), m_moveStartPoint( -1 ), m_startValue( 0 ), @@ -217,26 +217,13 @@ void Fader::mouseDoubleClickEvent( QMouseEvent* mouseEvent ) bool ok; float newValue; // TODO: dbV handling - if( m_displayConversion ) - { - newValue = QInputDialog::getDouble( this, tr( "Set value" ), - tr( "Please enter a new value between %1 and %2:" ). - arg( model()->minValue() * 100 ). - arg( model()->maxValue() * 100 ), - model()->getRoundedValue() * 100, - model()->minValue() * 100, - model()->maxValue() * 100, model()->getDigitCount(), &ok ) * 0.01f; - } - else - { - newValue = QInputDialog::getDouble( this, tr( "Set value" ), - tr( "Please enter a new value between %1 and %2:" ). - arg( model()->minValue() ). - arg( model()->maxValue() ), - model()->getRoundedValue(), - model()->minValue(), - model()->maxValue(), model()->getDigitCount(), &ok ); - } + newValue = QInputDialog::getDouble( this, tr( "Set value" ), + tr( "Please enter a new value between %1 and %2:" ). + arg( model()->minValue() * m_conversionFactor ). + arg( model()->maxValue() * m_conversionFactor ), + model()->getRoundedValue() * m_conversionFactor, + model()->minValue() * m_conversionFactor, + model()->maxValue() * m_conversionFactor, model()->getDigitCount(), &ok ) / m_conversionFactor; if( ok ) { @@ -330,14 +317,14 @@ void Fader::setPeak_R( float fPeak ) // update tooltip showing value and adjust position while changing fader value void Fader::updateTextFloat() { - if( ConfigManager::inst()->value( "app", "displaydbfs" ).toInt() && m_displayConversion ) + if( ConfigManager::inst()->value( "app", "displaydbfs" ).toInt() && ( m_conversionFactor == 100.0 ) ) { s_textFloat->setText( QString("Volume: %1 dBFS"). arg( ampToDbfs( model()->value() ), 3, 'f', 2 ) ); } else { - s_textFloat->setText( m_description + " " + QString("%1 ").arg( m_displayConversion ? model()->value() * 100 : model()->value() ) + " " + m_unit ); + s_textFloat->setText( m_description + " " + QString("%1 ").arg( model()->value() * m_conversionFactor ) + " " + m_unit ); } s_textFloat->moveGlobal( this, QPoint( width() - ( *m_knob ).width() - 5, knobPosY() - 46 ) ); } From 083f7954a05b227af8e91aab6ba8fb6cbc9ee9f6 Mon Sep 17 00:00:00 2001 From: DigArtRoks <69391149+DigArtRoks@users.noreply.github.com> Date: Fri, 11 Sep 2020 22:55:47 +0200 Subject: [PATCH 2/2] Remove spaces and unneeded parentheses after review. --- include/AutomatableModelView.h | 2 +- src/gui/widgets/Fader.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/AutomatableModelView.h b/include/AutomatableModelView.h index d5fa76a9a00..029ea16fba7 100644 --- a/include/AutomatableModelView.h +++ b/include/AutomatableModelView.h @@ -78,7 +78,7 @@ class LMMS_EXPORT AutomatableModelView : public ModelView QString m_description; QString m_unit; - float m_conversionFactor; // Factor to be applied when the m_model->value is displayed + float m_conversionFactor; // Factor to be applied when the m_model->value is displayed } ; diff --git a/src/gui/widgets/Fader.cpp b/src/gui/widgets/Fader.cpp index f19ebdb76ce..d250ff98c9b 100644 --- a/src/gui/widgets/Fader.cpp +++ b/src/gui/widgets/Fader.cpp @@ -317,7 +317,7 @@ void Fader::setPeak_R( float fPeak ) // update tooltip showing value and adjust position while changing fader value void Fader::updateTextFloat() { - if( ConfigManager::inst()->value( "app", "displaydbfs" ).toInt() && ( m_conversionFactor == 100.0 ) ) + if( ConfigManager::inst()->value( "app", "displaydbfs" ).toInt() && m_conversionFactor == 100.0 ) { s_textFloat->setText( QString("Volume: %1 dBFS"). arg( ampToDbfs( model()->value() ), 3, 'f', 2 ) );