From f37ca49e6d52886d40858e23acd1d3ed32f97a71 Mon Sep 17 00:00:00 2001 From: Hyunjin Song Date: Wed, 12 Sep 2018 11:02:40 +0900 Subject: [PATCH] Fix decimal separator handling (#4547) Makes LMMS can handle both periods and commas properly when loading real numbers. --- include/DataFile.h | 19 ------ include/LocaleHelper.h | 67 +++++++++++++++++++ plugins/MidiExport/MidiExport.cpp | 7 +- plugins/VstEffect/VstEffectControls.cpp | 9 +-- plugins/vestige/vestige.cpp | 9 +-- plugins/vst_base/VstPlugin.cpp | 3 +- .../zynaddsubfx/src/Misc/QtXmlWrapper.cpp | 9 +-- src/core/AutomatableModel.cpp | 5 +- src/core/AutomationPattern.cpp | 7 +- src/core/DataFile.cpp | 38 ++--------- src/core/Song.cpp | 4 -- src/gui/widgets/Knob.cpp | 3 +- src/tracks/InstrumentTrack.cpp | 2 - 13 files changed, 101 insertions(+), 81 deletions(-) create mode 100644 include/LocaleHelper.h diff --git a/include/DataFile.h b/include/DataFile.h index 6b6b1a98e6c..9a420135a29 100644 --- a/include/DataFile.h +++ b/include/DataFile.h @@ -84,25 +84,6 @@ class EXPORT DataFile : public QDomDocument return m_type; } - // small helper class for adjusting application's locale settings - // when loading or saving floating point values rendered to strings - class LocaleHelper - { - public: - enum Modes - { - ModeLoad, - ModeSave, - ModeCount - }; - typedef Modes Mode; - - LocaleHelper( Mode mode ); - ~LocaleHelper(); - - }; - - private: static Type type( const QString& typeName ); static QString typeName( Type type ); diff --git a/include/LocaleHelper.h b/include/LocaleHelper.h new file mode 100644 index 00000000000..c5d9d4c46c5 --- /dev/null +++ b/include/LocaleHelper.h @@ -0,0 +1,67 @@ +/* + * LocaleHelper.h - compatibility functions for handling decimal separators + * Providing helper functions which handle both periods and commas + * for decimal separators to load old projects correctly + * + * Copyright (c) 2014 Tobias Doerffel + * Copyright (c) 2018 Hyunjin Song + * + * This file is part of LMMS - https://lmms.io + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public + * License along with this program (see COPYING); if not, write to the + * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, + * Boston, MA 02110-1301 USA. + * + */ + +#ifndef LOCALEHELPER_H +#define LOCALEHELPER_H + +#include + +#include +#include + +namespace LocaleHelper +{ +inline double toDouble(QString str, bool* ok = nullptr) +{ + bool isOkay; + double value; + QLocale c(QLocale::C); + c.setNumberOptions(QLocale::RejectGroupSeparator); + value = c.toDouble(str, &isOkay); + if (!isOkay) + { + QLocale german(QLocale::German); + german.setNumberOptions(QLocale::RejectGroupSeparator); + value = german.toDouble(str, &isOkay); + } + if (ok != nullptr) {*ok = isOkay;} + return value; +} + +inline float toFloat(QString str, bool* ok = nullptr) +{ + double d = toDouble(str, ok); + if (!std::isinf(d) && std::fabs(d) > std::numeric_limits::max()) + { + if (ok != nullptr) {*ok = false;} + return 0.0f; + } + return static_cast(d); +} +} + +#endif // LOCALEHELPER_H diff --git a/plugins/MidiExport/MidiExport.cpp b/plugins/MidiExport/MidiExport.cpp index 1e20e9d40f4..65eaf46132b 100644 --- a/plugins/MidiExport/MidiExport.cpp +++ b/plugins/MidiExport/MidiExport.cpp @@ -36,6 +36,7 @@ #include "TrackContainer.h" #include "BBTrack.h" #include "InstrumentTrack.h" +#include "LocaleHelper.h" extern "C" @@ -133,7 +134,7 @@ bool MidiExport::tryExport(const TrackContainer::TrackList &tracks, { base_pitch += masterPitch; } - base_volume = it.attribute("volume", "100").toDouble()/100.0; + base_volume = LocaleHelper::toDouble(it.attribute("volume", "100"))/100.0; } if (n.nodeName() == "pattern") @@ -204,7 +205,7 @@ bool MidiExport::tryExport(const TrackContainer::TrackList &tracks, { base_pitch += masterPitch; } - base_volume = it.attribute("volume", "100").toDouble() / 100.0; + base_volume = LocaleHelper::toDouble(it.attribute("volume", "100")) / 100.0; } if (n.nodeName() == "pattern") @@ -273,7 +274,7 @@ void MidiExport::writePattern(MidiNoteVector &pat, QDomNode n, // TODO interpret pan="0" fxch="0" pitchrange="1" MidiNote mnote; mnote.pitch = qMax(0, qMin(127, note.attribute("key", "0").toInt() + base_pitch)); - mnote.volume = qMin(qRound(base_volume * note.attribute("vol", "100").toDouble()), 127); + mnote.volume = qMin(qRound(base_volume * LocaleHelper::toDouble(note.attribute("vol", "100"))), 127); mnote.time = base_time + note.attribute("pos", "0").toInt(); mnote.duration = note.attribute("len", "0").toInt(); pat.push_back(mnote); diff --git a/plugins/VstEffect/VstEffectControls.cpp b/plugins/VstEffect/VstEffectControls.cpp index 21d940ddc92..e5261d62587 100644 --- a/plugins/VstEffect/VstEffectControls.cpp +++ b/plugins/VstEffect/VstEffectControls.cpp @@ -27,6 +27,7 @@ #include "VstEffectControls.h" #include "VstEffect.h" +#include "LocaleHelper.h" #include "MainWindow.h" #include "GuiApplication.h" #include @@ -85,8 +86,8 @@ void VstEffectControls::loadSettings( const QDomElement & _this ) if( !( knobFModel[ i ]->isAutomated() || knobFModel[ i ]->controllerConnection() ) ) { - knobFModel[ i ]->setValue( (s_dumpValues.at( 2 ) ).toFloat() ); - knobFModel[ i ]->setInitValue( (s_dumpValues.at( 2 ) ).toFloat() ); + knobFModel[ i ]->setValue(LocaleHelper::toFloat(s_dumpValues.at(2))); + knobFModel[ i ]->setInitValue(LocaleHelper::toFloat(s_dumpValues.at(2))); } connect( knobFModel[i], SIGNAL( dataChanged() ), this, SLOT( setParameter() ) ); @@ -381,7 +382,7 @@ manageVSTEffectView::manageVSTEffectView( VstEffect * _eff, VstEffectControls * if( !hasKnobModel ) { sprintf( paramStr, "%d", i); - m_vi->knobFModel[ i ] = new FloatModel( ( s_dumpValues.at( 2 ) ).toFloat(), + m_vi->knobFModel[ i ] = new FloatModel( LocaleHelper::toFloat(s_dumpValues.at(2)), 0.0f, 1.0f, 0.01f, _eff, tr( paramStr ) ); } connect( m_vi->knobFModel[ i ], SIGNAL( dataChanged() ), this, @@ -445,7 +446,7 @@ void manageVSTEffectView::syncPlugin( void ) { sprintf( paramStr, "param%d", i ); s_dumpValues = dump[ paramStr ].split( ":" ); - f_value = ( s_dumpValues.at( 2 ) ).toFloat(); + f_value = LocaleHelper::toFloat(s_dumpValues.at(2)); m_vi2->knobFModel[ i ]->setAutomatedValue( f_value ); m_vi2->knobFModel[ i ]->setInitValue( f_value ); } diff --git a/plugins/vestige/vestige.cpp b/plugins/vestige/vestige.cpp index 540c8b5ce97..b958e8ddfd6 100644 --- a/plugins/vestige/vestige.cpp +++ b/plugins/vestige/vestige.cpp @@ -41,6 +41,7 @@ #include "InstrumentPlayHandle.h" #include "InstrumentTrack.h" #include "VstPlugin.h" +#include "LocaleHelper.h" #include "MainWindow.h" #include "Mixer.h" #include "GuiApplication.h" @@ -203,8 +204,8 @@ void vestigeInstrument::loadSettings( const QDomElement & _this ) if( !( knobFModel[ i ]->isAutomated() || knobFModel[ i ]->controllerConnection() ) ) { - knobFModel[ i ]->setValue( ( s_dumpValues.at( 2 )).toFloat() ); - knobFModel[ i ]->setInitValue( ( s_dumpValues.at( 2 )).toFloat() ); + knobFModel[ i ]->setValue(LocaleHelper::toFloat(s_dumpValues.at(2))); + knobFModel[ i ]->setInitValue(LocaleHelper::toFloat(s_dumpValues.at(2))); } connect( knobFModel[i], SIGNAL( dataChanged() ), this, SLOT( setParameter() ) ); @@ -991,7 +992,7 @@ manageVestigeInstrumentView::manageVestigeInstrumentView( Instrument * _instrume if( !hasKnobModel ) { sprintf( paramStr, "%d", i); - m_vi->knobFModel[ i ] = new FloatModel( (s_dumpValues.at( 2 )).toFloat(), + m_vi->knobFModel[ i ] = new FloatModel( LocaleHelper::toFloat(s_dumpValues.at(2)), 0.0f, 1.0f, 0.01f, castModel(), tr( paramStr ) ); } connect( m_vi->knobFModel[i], SIGNAL( dataChanged() ), this, SLOT( setParameter() ) ); @@ -1052,7 +1053,7 @@ void manageVestigeInstrumentView::syncPlugin( void ) { sprintf( paramStr, "param%d", i ); s_dumpValues = dump[ paramStr ].split( ":" ); - f_value = ( s_dumpValues.at( 2 ) ).toFloat(); + f_value = LocaleHelper::toFloat(s_dumpValues.at(2)); m_vi->knobFModel[ i ]->setAutomatedValue( f_value ); m_vi->knobFModel[ i ]->setInitValue( f_value ); } diff --git a/plugins/vst_base/VstPlugin.cpp b/plugins/vst_base/VstPlugin.cpp index 157665eb597..a97802bdc70 100644 --- a/plugins/vst_base/VstPlugin.cpp +++ b/plugins/vst_base/VstPlugin.cpp @@ -56,6 +56,7 @@ #include "ConfigManager.h" #include "GuiApplication.h" +#include "LocaleHelper.h" #include "MainWindow.h" #include "Mixer.h" #include "Song.h" @@ -299,7 +300,7 @@ void VstPlugin::setParameterDump( const QMap & _pdump ) { ( *it ).section( ':', 0, 0 ).toInt(), "", - ( *it ).section( ':', 2, -1 ).toFloat() + LocaleHelper::toFloat((*it).section(':', 2, -1)) } ; m.addInt( item.index ); m.addString( item.shortLabel ); diff --git a/plugins/zynaddsubfx/zynaddsubfx/src/Misc/QtXmlWrapper.cpp b/plugins/zynaddsubfx/zynaddsubfx/src/Misc/QtXmlWrapper.cpp index bab51382934..d4b6875a085 100644 --- a/plugins/zynaddsubfx/zynaddsubfx/src/Misc/QtXmlWrapper.cpp +++ b/plugins/zynaddsubfx/zynaddsubfx/src/Misc/QtXmlWrapper.cpp @@ -61,6 +61,7 @@ // Include LMMS headers #include "lmmsconfig.h" #include "IoHelper.h" +#include "LocaleHelper.h" struct XmlData @@ -228,14 +229,14 @@ int QtXmlWrapper::dosavefile(const char *filename, void QtXmlWrapper::addpar(const std::string &name, int val) { - d->addparams("par", 2, "name", name.c_str(), "value", stringFrom( - val).c_str()); + d->addparams("par", 2, "name", name.c_str(), "value", + QString::number(val).toLocal8Bit().constData()); } void QtXmlWrapper::addparreal(const std::string &name, float val) { d->addparams("par_real", 2, "name", name.c_str(), "value", - stringFrom(val).c_str()); + QString::number(val, 'f').toLocal8Bit().constData()); } void QtXmlWrapper::addparbool(const std::string &name, int val) @@ -510,7 +511,7 @@ float QtXmlWrapper::getparreal(const char *name, float defaultpar) const return defaultpar; } - return QLocale().toFloat( tmp.attribute( "value" ) ); + return LocaleHelper::toFloat( tmp.attribute( "value" ) ); } float QtXmlWrapper::getparreal(const char *name, diff --git a/src/core/AutomatableModel.cpp b/src/core/AutomatableModel.cpp index 0b2a1522bd4..ba81a589310 100644 --- a/src/core/AutomatableModel.cpp +++ b/src/core/AutomatableModel.cpp @@ -28,6 +28,7 @@ #include "AutomationPattern.h" #include "ControllerConnection.h" +#include "LocaleHelper.h" #include "Mixer.h" #include "ProjectJournal.h" @@ -183,7 +184,7 @@ void AutomatableModel::loadSettings( const QDomElement& element, const QString& if( node.isElement() ) { changeID( node.toElement().attribute( "id" ).toInt() ); - setValue( node.toElement().attribute( "value" ).toFloat() ); + setValue( LocaleHelper::toFloat( node.toElement().attribute( "value" ) ) ); if( node.toElement().hasAttribute( "scale_type" ) ) { if( node.toElement().attribute( "scale_type" ) == "linear" ) @@ -204,7 +205,7 @@ void AutomatableModel::loadSettings( const QDomElement& element, const QString& if( element.hasAttribute( name ) ) // attribute => read the element's value from the attribute list { - setInitValue( element.attribute( name ).toFloat() ); + setInitValue( LocaleHelper::toFloat( element.attribute( name ) ) ); } else { diff --git a/src/core/AutomationPattern.cpp b/src/core/AutomationPattern.cpp index 32b13f3f441..25da6defbdf 100644 --- a/src/core/AutomationPattern.cpp +++ b/src/core/AutomationPattern.cpp @@ -28,6 +28,7 @@ #include "AutomationPatternView.h" #include "AutomationTrack.h" +#include "LocaleHelper.h" #include "Note.h" #include "ProjectJournal.h" #include "BBTrackContainer.h" @@ -154,11 +155,11 @@ void AutomationPattern::setProgressionType( void AutomationPattern::setTension( QString _new_tension ) { bool ok; - float nt = _new_tension.toFloat( & ok ); + float nt = LocaleHelper::toFloat(_new_tension, & ok); if( ok && nt > -0.01 && nt < 1.01 ) { - m_tension = _new_tension.toFloat(); + m_tension = nt; } } @@ -595,7 +596,7 @@ void AutomationPattern::loadSettings( const QDomElement & _this ) if( element.tagName() == "time" ) { m_timeMap[element.attribute( "pos" ).toInt()] - = element.attribute( "value" ).toFloat(); + = LocaleHelper::toFloat(element.attribute("value")); } else if( element.tagName() == "object" ) { diff --git a/src/core/DataFile.cpp b/src/core/DataFile.cpp index dc2b1de5791..47df2561474 100644 --- a/src/core/DataFile.cpp +++ b/src/core/DataFile.cpp @@ -38,6 +38,7 @@ #include "Effect.h" #include "embed.h" #include "GuiApplication.h" +#include "LocaleHelper.h" #include "PluginFactory.h" #include "ProjectVersion.h" #include "SongEditor.h" @@ -65,37 +66,6 @@ DataFile::typeDescStruct -DataFile::LocaleHelper::LocaleHelper( Mode mode ) -{ - switch( mode ) - { - case ModeLoad: - // set a locale for which QString::fromFloat() returns valid values if - // floating point separator is a comma - otherwise we would fail to load - // older projects made by people from various countries due to their - // locale settings - QLocale::setDefault( QLocale::German ); - break; - - case ModeSave: - // set default locale to C so that floating point decimals are rendered to - // strings with periods as decimal point instead of commas in some countries - QLocale::setDefault( QLocale::C ); - - default: break; - } -} - - - -DataFile::LocaleHelper::~LocaleHelper() -{ - // revert to original locale - QLocale::setDefault( QLocale::system() ); -} - - - DataFile::DataFile( Type type ) : QDomDocument( "lmms-project" ), @@ -416,8 +386,8 @@ void DataFile::upgrade_0_2_1_20070501() QDomElement el = list.item( i ).toElement(); if( el.attribute( "vol" ) != "" ) { - el.setAttribute( "vol", el.attribute( - "vol" ).toFloat() * 100.0f ); + el.setAttribute( "vol", LocaleHelper::toFloat( + el.attribute( "vol" ) ) * 100.0f ); } else { @@ -543,7 +513,7 @@ void DataFile::upgrade_0_2_1_20070508() QDomElement el = list.item( i ).toElement(); if( el.hasAttribute( "vol" ) ) { - float value = el.attribute( "vol" ).toFloat(); + float value = LocaleHelper::toFloat( el.attribute( "vol" ) ); value = roundf( value * 0.585786438f ); el.setAttribute( "vol", value ); } diff --git a/src/core/Song.cpp b/src/core/Song.cpp index 217a8752ec8..1ebc684c196 100644 --- a/src/core/Song.cpp +++ b/src/core/Song.cpp @@ -1034,8 +1034,6 @@ void Song::loadProject( const QString & fileName ) clearErrors(); - DataFile::LocaleHelper localeHelper( DataFile::LocaleHelper::ModeLoad ); - Engine::mixer()->requestChangeInModel(); // get the header information from the DOM @@ -1191,8 +1189,6 @@ void Song::loadProject( const QString & fileName ) // only save current song as _filename and do nothing else bool Song::saveProjectFile( const QString & filename ) { - DataFile::LocaleHelper localeHelper( DataFile::LocaleHelper::ModeSave ); - DataFile dataFile( DataFile::SongProject ); m_tempoModel.saveSettings( dataFile, dataFile.head(), "bpm" ); diff --git a/src/gui/widgets/Knob.cpp b/src/gui/widgets/Knob.cpp index 73ef427585b..72c6c7f8bac 100644 --- a/src/gui/widgets/Knob.cpp +++ b/src/gui/widgets/Knob.cpp @@ -42,6 +42,7 @@ #include "embed.h" #include "gui_templates.h" #include "GuiApplication.h" +#include "LocaleHelper.h" #include "MainWindow.h" #include "ProjectJournal.h" #include "Song.h" @@ -560,7 +561,7 @@ void Knob::dropEvent( QDropEvent * _de ) QString val = StringPairDrag::decodeValue( _de ); if( type == "float_value" ) { - model()->setValue( val.toFloat() ); + model()->setValue( LocaleHelper::toFloat(val) ); _de->accept(); } else if( type == "automatable_model" ) diff --git a/src/tracks/InstrumentTrack.cpp b/src/tracks/InstrumentTrack.cpp index 90dbf11a684..d5ded0c7057 100644 --- a/src/tracks/InstrumentTrack.cpp +++ b/src/tracks/InstrumentTrack.cpp @@ -1618,8 +1618,6 @@ void InstrumentTrackWindow::saveSettingsBtnClicked() !sfd.selectedFiles().isEmpty() && !sfd.selectedFiles().first().isEmpty() ) { - DataFile::LocaleHelper localeHelper( DataFile::LocaleHelper::ModeSave ); - DataFile dataFile( DataFile::InstrumentTrackSettings ); m_track->setSimpleSerializing(); m_track->saveSettings( dataFile, dataFile.content() );