From 6c67fb73ec52983d35fd033f2bcea8722a93d868 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Fri, 14 Jun 2024 01:07:31 +0200 Subject: [PATCH 1/2] use enum class for waveform overview type --- src/preferences/dialog/dlgprefwaveform.cpp | 68 +++++++++++++++------- src/preferences/dialog/dlgprefwaveform.h | 6 +- src/widget/woverview.cpp | 22 +++---- src/widget/woverview.h | 11 +++- 4 files changed, 70 insertions(+), 37 deletions(-) diff --git a/src/preferences/dialog/dlgprefwaveform.cpp b/src/preferences/dialog/dlgprefwaveform.cpp index fbee90e4d4b..de65ffc9cfd 100644 --- a/src/preferences/dialog/dlgprefwaveform.cpp +++ b/src/preferences/dialog/dlgprefwaveform.cpp @@ -1,6 +1,8 @@ #include "preferences/dialog/dlgprefwaveform.h" -#include "control/controlobject.h" +#include + +#include "control/controlpushbutton.h" #include "library/dao/analysisdao.h" #include "library/library.h" #include "moc_dlgprefwaveform.cpp" @@ -8,6 +10,12 @@ #include "util/db/dbconnectionpooled.h" #include "waveform/renderers/waveformwidgetrenderer.h" #include "waveform/waveformwidgetfactory.h" +#include "widget/woverview.h" + +namespace { +const ConfigKey kOverviewTypeCfgKey(QStringLiteral("[Waveform]"), + QStringLiteral("WaveformOverviewType")); +} // namespace DlgPrefWaveform::DlgPrefWaveform( QWidget* pParent, @@ -19,13 +27,27 @@ DlgPrefWaveform::DlgPrefWaveform( setupUi(this); // Waveform overview init - waveformOverviewComboBox->addItem(tr("Filtered")); // "0" - waveformOverviewComboBox->addItem(tr("HSV")); // "1" - waveformOverviewComboBox->addItem(tr("RGB")); // "2" - m_pTypeControl = std::make_unique( - ConfigKey(QStringLiteral("[Waveform]"), - QStringLiteral("WaveformOverviewType"))); + waveformOverviewComboBox->addItem( + tr("Filtered"), QVariant::fromValue(WOverview::Type::Filtered)); + waveformOverviewComboBox->addItem(tr("HSV"), QVariant::fromValue(WOverview::Type::HSV)); + waveformOverviewComboBox->addItem(tr("RGB"), QVariant::fromValue(WOverview::Type::RGB)); + m_pTypeControl = std::make_unique(kOverviewTypeCfgKey); + m_pTypeControl->setStates(QMetaEnum::fromType().keyCount()); m_pTypeControl->setReadOnly(); + // Update the control with the config value + WOverview::Type overviewType = + m_pConfig->getValue(kOverviewTypeCfgKey, WOverview::Type::RGB); + int cfgTypeIndex = waveformOverviewComboBox->findData(QVariant::fromValue(overviewType)); + if (cfgTypeIndex == -1) { + // Invalid config value, set default type RGB and write it to config + waveformOverviewComboBox->setCurrentIndex( + waveformOverviewComboBox->findData(QVariant::fromValue(WOverview::Type::RGB))); + m_pConfig->setValue(kOverviewTypeCfgKey, cfgTypeIndex); + } else { + waveformOverviewComboBox->setCurrentIndex(cfgTypeIndex); + } + // Set the control used by WOverview + m_pTypeControl->forceSet(cfgTypeIndex); // Populate waveform options. WaveformWidgetFactory* factory = WaveformWidgetFactory::instance(); @@ -204,13 +226,14 @@ void DlgPrefWaveform::slotUpdate() { factory->getUntilMarkAlign())); untilMarkTextPointSizeSpinBox->setValue(factory->getUntilMarkTextPointSize()); - // By default we set RGB woverview = "2" - int overviewType = m_pConfig->getValue( - ConfigKey("[Waveform]","WaveformOverviewType"), 2); - if (overviewType != waveformOverviewComboBox->currentIndex()) { - waveformOverviewComboBox->setCurrentIndex(overviewType); + WOverview::Type cfgOverviewType = + m_pConfig->getValue(kOverviewTypeCfgKey, WOverview::Type::RGB); + // Assumes the combobox index is in sync with the ControlPushButton + if (cfgOverviewType != waveformOverviewComboBox->currentData().value()) { + int cfgOverviewTypeIndex = + waveformOverviewComboBox->findData(QVariant::fromValue(cfgOverviewType)); + waveformOverviewComboBox->setCurrentIndex(cfgOverviewTypeIndex); } - slotSetWaveformOverviewType(overviewType); WaveformSettings waveformSettings(m_pConfig); enableWaveformCaching->setChecked(waveformSettings.waveformCachingEnabled()); @@ -220,10 +243,6 @@ void DlgPrefWaveform::slotUpdate() { } void DlgPrefWaveform::slotApply() { - ConfigValue overviewtype = ConfigValue(waveformOverviewComboBox->currentIndex()); - if (overviewtype != m_pConfig->get(ConfigKey("[Waveform]", "WaveformOverviewType"))) { - m_pConfig->set(ConfigKey("[Waveform]", "WaveformOverviewType"), overviewtype); - } WaveformSettings waveformSettings(m_pConfig); waveformSettings.setWaveformCachingEnabled(enableWaveformCaching->isChecked()); waveformSettings.setWaveformGenerationWithAnalysisEnabled( @@ -252,7 +271,8 @@ void DlgPrefWaveform::slotResetToDefaults() { synchronizeZoomCheckBox->setChecked(true); // RGB overview. - waveformOverviewComboBox->setCurrentIndex(2); + waveformOverviewComboBox->setCurrentIndex( + waveformOverviewComboBox->findData(QVariant::fromValue(WOverview::Type::RGB))); // Don't normalize overview. normalizeOverviewCheckBox->setChecked(false); @@ -303,9 +323,13 @@ void DlgPrefWaveform::updateEnableUntilMark() { requiresGLSLLabel->setVisible(!enabled); } -void DlgPrefWaveform::slotSetWaveformOverviewType(int index) { - m_pConfig->set(ConfigKey("[Waveform]","WaveformOverviewType"), ConfigValue(index)); - m_pTypeControl->forceSet(index); +void DlgPrefWaveform::slotSetWaveformOverviewType() { + // Apply immediately + QVariant comboboxData = waveformOverviewComboBox->currentData(); + DEBUG_ASSERT(comboboxData.canConvert()); + auto type = comboboxData.value(); + m_pConfig->setValue(kOverviewTypeCfgKey, type); + m_pTypeControl->forceSet(static_cast(type)); } void DlgPrefWaveform::slotSetDefaultZoom(int index) { @@ -351,6 +375,8 @@ void DlgPrefWaveform::slotClearCachedWaveforms() { } void DlgPrefWaveform::slotSetBeatGridAlpha(int alpha) { + // TODO(xxx) For consistency set this in WaveformWidgetFactory like + // the other waveform controls. m_pConfig->setValue(ConfigKey("[Waveform]", "beatGridAlpha"), alpha); WaveformWidgetFactory::instance()->setDisplayBeatGridAlpha(alpha); } diff --git a/src/preferences/dialog/dlgprefwaveform.h b/src/preferences/dialog/dlgprefwaveform.h index 67a9ff9916b..931b8b4639e 100644 --- a/src/preferences/dialog/dlgprefwaveform.h +++ b/src/preferences/dialog/dlgprefwaveform.h @@ -6,7 +6,7 @@ #include "preferences/dialog/ui_dlgprefwaveformdlg.h" #include "preferences/usersettings.h" -class ControlObject; +class ControlPushButton; class Library; class DlgPrefWaveform : public DlgPreferencePage, public Ui::DlgPrefWaveformDlg { @@ -27,7 +27,7 @@ class DlgPrefWaveform : public DlgPreferencePage, public Ui::DlgPrefWaveformDlg private slots: void slotSetFrameRate(int frameRate); void slotSetWaveformType(int index); - void slotSetWaveformOverviewType(int index); + void slotSetWaveformOverviewType(); void slotSetDefaultZoom(int index); void slotSetZoomSynchronization(bool checked); void slotSetVisualGainAll(double gain); @@ -50,7 +50,7 @@ class DlgPrefWaveform : public DlgPreferencePage, public Ui::DlgPrefWaveformDlg void notifyRebootNecessary(); void updateEnableUntilMark(); - std::unique_ptr m_pTypeControl; + std::unique_ptr m_pTypeControl; UserSettingsPointer m_pConfig; std::shared_ptr m_pLibrary; diff --git a/src/widget/woverview.cpp b/src/widget/woverview.cpp index 35be6fe3d22..daa20b911fb 100644 --- a/src/widget/woverview.cpp +++ b/src/widget/woverview.cpp @@ -34,7 +34,7 @@ WOverview::WOverview( : WWidget(parent), m_group(group), m_pConfig(pConfig), - m_type(-1), + m_type(Type::RGB), m_actualCompletion(0), m_pixmapDone(false), m_waveformPeak(-1.0), @@ -65,6 +65,7 @@ WOverview::WOverview( // Needed to recalculate range durations when rate slider is moved without the deck playing m_pRateRatioControl->connectValueChanged( this, &WOverview::onRateRatioChange); + // TODO(xxx) Use PollingControlProxy where possible m_trackSampleRateControl = make_parented( m_group, QStringLiteral("track_samplerate"), this, ControlFlag::NoAssertIfMissing); m_trackSamplesControl = make_parented( @@ -80,8 +81,8 @@ WOverview::WOverview( QStringLiteral("[Waveform]"), QStringLiteral("WaveformOverviewType"), this); - m_pTypeControl->connectValueChanged(this, &WOverview::slotTypeChanged); - slotTypeChanged(m_pTypeControl->get()); + m_pTypeControl->connectValueChanged(this, &WOverview::slotTypeControlChanged); + slotTypeControlChanged(m_pTypeControl->get()); m_pPassthroughLabel = make_parented(this); @@ -400,11 +401,10 @@ void WOverview::onPassthroughChange(double v) { update(); } -void WOverview::slotTypeChanged(double v) { - int type = static_cast(v); - VERIFY_OR_DEBUG_ASSERT(type >= 0 && type <= 2) { - type = 2; - } +void WOverview::slotTypeControlChanged(double v) { + // Assert that v is in enum range to prevent UB. + DEBUG_ASSERT(v >= 0 && v < QMetaEnum::fromType().keyCount()); + Type type = static_cast(static_cast(v)); if (type == m_type) { return; } @@ -1288,11 +1288,11 @@ bool WOverview::drawNextPixmapPart() { QPainter painter(&m_waveformSourceImage); painter.translate(0.0, static_cast(m_waveformSourceImage.height()) / 2.0); - if (m_type == 0) { + if (m_type == Type::Filtered) { drawNextPixmapPartLMH(&painter, pWaveform, nextCompletion); - } else if (m_type == 1) { + } else if (m_type == Type::HSV) { drawNextPixmapPartHSV(&painter, pWaveform, nextCompletion); - } else { + } else { // Type::RGB: drawNextPixmapPartRGB(&painter, pWaveform, nextCompletion); } diff --git a/src/widget/woverview.h b/src/widget/woverview.h index 12613fdd6af..35b598ec65b 100644 --- a/src/widget/woverview.h +++ b/src/widget/woverview.h @@ -32,6 +32,13 @@ class WOverview : public WWidget, public TrackDropTarget { void setup(const QDomNode& node, const SkinContext& context); virtual void initWithTrack(TrackPointer pTrack); + enum class Type { + Filtered, + HSV, + RGB, + }; + Q_ENUM(Type); + public slots: void onConnectedControlChanged(double dParameter, double dValue) override; void slotTrackLoaded(TrackPointer pTrack); @@ -66,7 +73,7 @@ class WOverview : public WWidget, public TrackDropTarget { void slotWaveformSummaryUpdated(); void slotCueMenuPopupAboutToHide(); - void slotTypeChanged(double v); + void slotTypeControlChanged(double v); private: // Append the waveform overview pixmap according to available data @@ -133,7 +140,7 @@ class WOverview : public WWidget, public TrackDropTarget { const QString m_group; UserSettingsPointer m_pConfig; - int m_type; + Type m_type; int m_actualCompletion; bool m_pixmapDone; float m_waveformPeak; From 8d552b94e8d2493f73702d6cc30df4bdfd2d62a1 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Mon, 17 Jun 2024 14:46:40 +0200 Subject: [PATCH 2/2] WOverview: use PollingControlProxy, remove ControlFlag::NoAssertIfMissing --- src/widget/woverview.cpp | 24 +++++++++++++----------- src/widget/woverview.h | 8 ++++---- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/widget/woverview.cpp b/src/widget/woverview.cpp index daa20b911fb..1aae0bfb0fc 100644 --- a/src/widget/woverview.cpp +++ b/src/widget/woverview.cpp @@ -56,7 +56,16 @@ WOverview::WOverview( m_analyzerProgress(kAnalyzerProgressUnknown), m_trackLoaded(false), m_pHoveredMark(nullptr), - m_scaleFactor(1.0) { + m_scaleFactor(1.0), + m_trackSampleRateControl( + m_group, + QStringLiteral("track_samplerate")), + m_trackSamplesControl( + m_group, + QStringLiteral("track_samples")), + m_playpositionControl( + m_group, + QStringLiteral("playposition")) { m_endOfTrackControl = make_parented( m_group, QStringLiteral("end_of_track"), this, ControlFlag::NoAssertIfMissing); m_endOfTrackControl->connectValueChanged(this, &WOverview::onEndOfTrackChange); @@ -65,13 +74,6 @@ WOverview::WOverview( // Needed to recalculate range durations when rate slider is moved without the deck playing m_pRateRatioControl->connectValueChanged( this, &WOverview::onRateRatioChange); - // TODO(xxx) Use PollingControlProxy where possible - m_trackSampleRateControl = make_parented( - m_group, QStringLiteral("track_samplerate"), this, ControlFlag::NoAssertIfMissing); - m_trackSamplesControl = make_parented( - m_group, QStringLiteral("track_samples"), this); - m_playpositionControl = make_parented( - m_group, QStringLiteral("playposition"), this, ControlFlag::NoAssertIfMissing); m_pPassthroughControl = make_parented( m_group, QStringLiteral("passthrough"), this, ControlFlag::NoAssertIfMissing); m_pPassthroughControl->connectValueChanged(this, &WOverview::onPassthroughChange); @@ -1002,7 +1004,7 @@ void WOverview::drawMarks(QPainter* pPainter, const float offset, const float ga double markSamples = pMark->getSamplePosition(); double trackSamples = getTrackSamples(); - double currentPositionSamples = m_playpositionControl->get() * trackSamples; + double currentPositionSamples = m_playpositionControl.get() * trackSamples; double markTime = samplePositionToSeconds(markSamples); double markTimeRemaining = samplePositionToSeconds(trackSamples - markSamples); double markTimeDistance = samplePositionToSeconds(markSamples - currentPositionSamples); @@ -1122,7 +1124,7 @@ void WOverview::drawTimeRuler(QPainter* pPainter) { qreal timePositionTillEnd = samplePositionToSeconds( (1 - widgetPositionFraction) * trackSamples); qreal timeDistance = samplePositionToSeconds( - (widgetPositionFraction - m_playpositionControl->get()) * trackSamples); + (widgetPositionFraction - m_playpositionControl.get()) * trackSamples); QString timeText = mixxx::Duration::formatTime(timePosition) + " -" + mixxx::Duration::formatTime(timePositionTillEnd); @@ -1543,7 +1545,7 @@ void WOverview::paintText(const QString& text, QPainter* pPainter) { double WOverview::samplePositionToSeconds(double sample) { double trackTime = sample / - (m_trackSampleRateControl->get() * mixxx::kEngineChannelCount); + (m_trackSampleRateControl.get() * mixxx::kEngineChannelCount); return trackTime / m_pRateRatioControl->get(); } diff --git a/src/widget/woverview.h b/src/widget/woverview.h index 35b598ec65b..b6b85093937 100644 --- a/src/widget/woverview.h +++ b/src/widget/woverview.h @@ -127,7 +127,7 @@ class WOverview : public WWidget, public TrackDropTarget { double getTrackSamples() const { if (m_trackLoaded) { - return m_trackSamplesControl->get(); + return m_trackSamplesControl.get(); } else { // Ignore the value, because the engine can still have the old track // during loading @@ -183,9 +183,9 @@ class WOverview : public WWidget, public TrackDropTarget { parented_ptr m_endOfTrackControl; parented_ptr m_pRateRatioControl; - parented_ptr m_trackSampleRateControl; - parented_ptr m_trackSamplesControl; - parented_ptr m_playpositionControl; + PollingControlProxy m_trackSampleRateControl; + PollingControlProxy m_trackSamplesControl; + PollingControlProxy m_playpositionControl; parented_ptr m_pPassthroughControl; parented_ptr m_pTypeControl;