Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

use enum class for waveform overview type #13370

Merged
merged 2 commits into from
Jun 29, 2024
Merged

use enum class for waveform overview type #13370

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c67fb7
use enum class for waveform overview type
ronso0 Jun 13, 2024
8d552b9
WOverview: use PollingControlProxy, remove ControlFlag::NoAssertIfMis…
ronso0 Jun 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 47 additions & 21 deletions src/preferences/dialog/dlgprefwaveform.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,21 @@
#include "preferences/dialog/dlgprefwaveform.h"

#include "control/controlobject.h"
#include <QMetaEnum>

#include "control/controlpushbutton.h"
#include "library/dao/analysisdao.h"
#include "library/library.h"
#include "moc_dlgprefwaveform.cpp"
#include "preferences/waveformsettings.h"
#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,
Expand All @@ -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<ControlObject>(
ConfigKey(QStringLiteral("[Waveform]"),
QStringLiteral("WaveformOverviewType")));
waveformOverviewComboBox->addItem(
tr("Filtered"), QVariant::fromValue(WOverview::Type::Filtered));
Swiftb0y marked this conversation as resolved.
Show resolved Hide resolved
waveformOverviewComboBox->addItem(tr("HSV"), QVariant::fromValue(WOverview::Type::HSV));
waveformOverviewComboBox->addItem(tr("RGB"), QVariant::fromValue(WOverview::Type::RGB));
m_pTypeControl = std::make_unique<ControlPushButton>(kOverviewTypeCfgKey);
m_pTypeControl->setStates(QMetaEnum::fromType<WOverview::Type>().keyCount());
m_pTypeControl->setReadOnly();
// Update the control with the config value
WOverview::Type overviewType =
m_pConfig->getValue<WOverview::Type>(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();
Expand Down Expand Up @@ -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<WOverview::Type>(kOverviewTypeCfgKey, WOverview::Type::RGB);
// Assumes the combobox index is in sync with the ControlPushButton
if (cfgOverviewType != waveformOverviewComboBox->currentData().value<WOverview::Type>()) {
int cfgOverviewTypeIndex =
waveformOverviewComboBox->findData(QVariant::fromValue(cfgOverviewType));
waveformOverviewComboBox->setCurrentIndex(cfgOverviewTypeIndex);
}
slotSetWaveformOverviewType(overviewType);

WaveformSettings waveformSettings(m_pConfig);
enableWaveformCaching->setChecked(waveformSettings.waveformCachingEnabled());
Expand All @@ -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(
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<WOverview::Type>());
auto type = comboboxData.value<WOverview::Type>();
m_pConfig->setValue(kOverviewTypeCfgKey, type);
m_pTypeControl->forceSet(static_cast<double>(type));
}

void DlgPrefWaveform::slotSetDefaultZoom(int index) {
Expand Down Expand Up @@ -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);
}
Expand Down
6 changes: 3 additions & 3 deletions src/preferences/dialog/dlgprefwaveform.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
Expand All @@ -50,7 +50,7 @@ class DlgPrefWaveform : public DlgPreferencePage, public Ui::DlgPrefWaveformDlg
void notifyRebootNecessary();
void updateEnableUntilMark();

std::unique_ptr<ControlObject> m_pTypeControl;
std::unique_ptr<ControlPushButton> m_pTypeControl;

UserSettingsPointer m_pConfig;
std::shared_ptr<Library> m_pLibrary;
Expand Down
44 changes: 23 additions & 21 deletions src/widget/woverview.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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<ControlProxy>(
m_group, QStringLiteral("end_of_track"), this, ControlFlag::NoAssertIfMissing);
m_endOfTrackControl->connectValueChanged(this, &WOverview::onEndOfTrackChange);
Expand All @@ -65,12 +74,6 @@ WOverview::WOverview(
// Needed to recalculate range durations when rate slider is moved without the deck playing
m_pRateRatioControl->connectValueChanged(
this, &WOverview::onRateRatioChange);
m_trackSampleRateControl = make_parented<ControlProxy>(
m_group, QStringLiteral("track_samplerate"), this, ControlFlag::NoAssertIfMissing);
m_trackSamplesControl = make_parented<ControlProxy>(
m_group, QStringLiteral("track_samples"), this);
m_playpositionControl = make_parented<ControlProxy>(
m_group, QStringLiteral("playposition"), this, ControlFlag::NoAssertIfMissing);
m_pPassthroughControl = make_parented<ControlProxy>(
m_group, QStringLiteral("passthrough"), this, ControlFlag::NoAssertIfMissing);
m_pPassthroughControl->connectValueChanged(this, &WOverview::onPassthroughChange);
Expand All @@ -80,8 +83,8 @@ WOverview::WOverview(
QStringLiteral("[Waveform]"),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't understand those ControlFlag::NoAssertIfMissing flags above. Some controls are read when painting and there is no null check. Even if default controls return 0 there will a division by 0 at one place.
Should we remove those flags?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot imagine a valid reason that this is missing. Maybe a leftover from a refactoring?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, so I'll remove those.

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<QLabel>(this);

Expand Down Expand Up @@ -400,11 +403,10 @@ void WOverview::onPassthroughChange(double v) {
update();
}

void WOverview::slotTypeChanged(double v) {
int type = static_cast<int>(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<Type>().keyCount());
Type type = static_cast<Type>(static_cast<int>(v));
Swiftb0y marked this conversation as resolved.
Show resolved Hide resolved
if (type == m_type) {
return;
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -1288,11 +1290,11 @@ bool WOverview::drawNextPixmapPart() {
QPainter painter(&m_waveformSourceImage);
painter.translate(0.0, static_cast<double>(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);
}

Expand Down Expand Up @@ -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();
}

Expand Down
19 changes: 13 additions & 6 deletions src/widget/woverview.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -120,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
Expand All @@ -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;
Expand Down Expand Up @@ -176,9 +183,9 @@ class WOverview : public WWidget, public TrackDropTarget {

parented_ptr<ControlProxy> m_endOfTrackControl;
parented_ptr<ControlProxy> m_pRateRatioControl;
parented_ptr<ControlProxy> m_trackSampleRateControl;
parented_ptr<ControlProxy> m_trackSamplesControl;
parented_ptr<ControlProxy> m_playpositionControl;
PollingControlProxy m_trackSampleRateControl;
PollingControlProxy m_trackSamplesControl;
PollingControlProxy m_playpositionControl;
parented_ptr<ControlProxy> m_pPassthroughControl;
parented_ptr<ControlProxy> m_pTypeControl;

Expand Down
Loading