From eed35f20ba5547453a85fd88498794c8fcb418cf Mon Sep 17 00:00:00 2001 From: Antoine Colombier <7086688+acolombier@users.noreply.github.com> Date: Thu, 25 Jul 2024 19:48:54 +0100 Subject: [PATCH] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: JoergAtGithub <64457745+JoergAtGithub@users.noreply.github.com> Co-authored-by: Daniel Schürmann --- src/analyzer/analyzerwaveform.cpp | 10 ++++--- src/main.cpp | 12 ++++---- src/mixer/basetrackplayer.cpp | 4 +-- src/qml/qmlplayerproxy.h | 2 +- src/qml/qmlstemsmodel.cpp | 2 +- src/skin/legacy/legacyskinparser.cpp | 4 +-- .../allshader/waveformrendererstem.cpp | 28 ++++++++++--------- src/widget/wstemcontrol.cpp | 24 ++++++++-------- 8 files changed, 45 insertions(+), 41 deletions(-) diff --git a/src/analyzer/analyzerwaveform.cpp b/src/analyzer/analyzerwaveform.cpp index da42ddd3dc14..f55e18a186b8 100644 --- a/src/analyzer/analyzerwaveform.cpp +++ b/src/analyzer/analyzerwaveform.cpp @@ -104,7 +104,7 @@ bool AnalyzerWaveform::shouldAnalyze(TrackPointer tio) const { ConstWaveformPointer pLoadedTrackWaveform; ConstWaveformPointer pLoadedTrackWaveformSummary; #ifdef __STEM__ - bool bIsStemTrack = !tio->getStemInfo().isEmpty(); + bool isStemTrack = !tio->getStemInfo().isEmpty(); #endif TrackId trackId = tio->getId(); @@ -146,12 +146,14 @@ bool AnalyzerWaveform::shouldAnalyze(TrackPointer tio) const { } #ifdef __STEM__ - // If the waveform was generated without stem information, we need to regenerate + // If the waveform was generated without stem information but the track has + // some, or the waveform contains stem information but the track doesn't + // have any, we need to regenerate the waveform. if (!missingWaveform && ((!pTrackWaveform.isNull() && - pTrackWaveform->hasStem() != bIsStemTrack) || + pTrackWaveform->hasStem() != isStemTrack) || (!pLoadedTrackWaveform.isNull() && - pLoadedTrackWaveform->hasStem() != bIsStemTrack))) { + pLoadedTrackWaveform->hasStem() != isStemTrack))) { missingWaveform = true; } #endif diff --git a/src/main.cpp b/src/main.cpp index 9dcaef3609f5..c9330ecfd147 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -65,9 +65,9 @@ int runMixxx(MixxxApplication* pApp, const CmdlineArgs& args) { // This scope ensures that `MixxxMainWindow` is destroyed *before* // CoreServices is shut down. Otherwise a debug assertion complaining about // leaked COs may be triggered. - MixxxMainWindow* mainWindow = MixxxMainWindow::createInstance(pCoreServices); + MixxxMainWindow* pMainWindow = MixxxMainWindow::createInstance(pCoreServices); pApp->processEvents(); - pApp->installEventFilter(mainWindow); + pApp->installEventFilter(pMainWindow); #if defined(__WINDOWS__) WindowsEventHandler winEventHandler; @@ -76,7 +76,7 @@ int runMixxx(MixxxApplication* pApp, const CmdlineArgs& args) { QObject::connect(pCoreServices.get(), &mixxx::CoreServices::initializationProgressUpdate, - mainWindow, + pMainWindow, &MixxxMainWindow::initializationProgressUpdate); // The size of cached pixmaps increases with the square of devicePixelRatio @@ -89,9 +89,9 @@ int runMixxx(MixxxApplication* pApp, const CmdlineArgs& args) { #ifdef MIXXX_USE_QOPENGL // Will call initialize when the initial wglwidget's // qopenglwindow has been exposed - mainWindow->initializeQOpenGL(); + pMainWindow->initializeQOpenGL(); #else - mainWindow->initialize(); + pMainWindow->initialize(); #endif pCoreServices->getControllerManager()->setUpDevices(); @@ -102,7 +102,7 @@ int runMixxx(MixxxApplication* pApp, const CmdlineArgs& args) { exitCode = kFatalErrorOnStartupExitCode; } else { qDebug() << "Displaying main window"; - mainWindow->show(); + pMainWindow->show(); qDebug() << "Running Mixxx"; exitCode = pApp->exec(); diff --git a/src/mixer/basetrackplayer.cpp b/src/mixer/basetrackplayer.cpp index b08809b01a1e..401d4edf6735 100644 --- a/src/mixer/basetrackplayer.cpp +++ b/src/mixer/basetrackplayer.cpp @@ -384,7 +384,7 @@ void BaseTrackPlayerImpl::loadTrack(TrackPointer pTrack) { ConfigKey(m_pChannelToCloneFrom->getGroup(), "loop_end_position"))); #ifdef __STEM__ - auto pDeckToClone = qobject_cast(m_pChannelToCloneFrom); + auto* pDeckToClone = qobject_cast(m_pChannelToCloneFrom); if (pDeckToClone && m_pLoadedTrack && m_pLoadedTrack->hasStem() && m_pChannel) { m_pChannel->cloneStemState(pDeckToClone); } @@ -692,7 +692,7 @@ void BaseTrackPlayerImpl::slotTrackLoaded(TrackPointer pNewTrack, const auto& stemInfo = m_pLoadedTrack->getStemInfo(); DEBUG_ASSERT(stemInfo.size() <= kMaxSupportedStem); int stemIdx = 0; - for (const auto& stemCo : m_pStemColors) { + for (const auto& stemColorCo : m_pStemColors) { auto color = kNoTrackColor; if (stemIdx < stemInfo.size()) { color = trackColorToDouble(mixxx::RgbColor::fromQColor( diff --git a/src/qml/qmlplayerproxy.h b/src/qml/qmlplayerproxy.h index b796a909d9ea..1c80cf7f1b61 100644 --- a/src/qml/qmlplayerproxy.h +++ b/src/qml/qmlplayerproxy.h @@ -155,7 +155,7 @@ class QmlPlayerProxy : public QObject { QPointer m_pTrackPlayer; TrackPointer m_pCurrentTrack; QmlBeatsModel* m_pBeatsModel; - QmlCuesModel* m_pHotcuesModel; + parented_ptr m_pHotcuesModel; #ifdef __STEM__ std::unique_ptr m_pStemsModel; #endif diff --git a/src/qml/qmlstemsmodel.cpp b/src/qml/qmlstemsmodel.cpp index 730a5df76b7e..2fa27b688da1 100644 --- a/src/qml/qmlstemsmodel.cpp +++ b/src/qml/qmlstemsmodel.cpp @@ -20,7 +20,7 @@ QmlStemsModel::QmlStemsModel( void QmlStemsModel::setStems(QList stems) { beginResetModel(); - m_stems = QList(std::move(stems)); + m_stems = std::move(stems); endResetModel(); } diff --git a/src/skin/legacy/legacyskinparser.cpp b/src/skin/legacy/legacyskinparser.cpp index 7ff3430c6f48..958d2776c4d5 100644 --- a/src/skin/legacy/legacyskinparser.cpp +++ b/src/skin/legacy/legacyskinparser.cpp @@ -93,7 +93,7 @@ using mixxx::skin::SkinManifest; #ifdef __STEM__ namespace { constexpr int kMaxSupportedStem = 4; -} +} // anonymous namespace #endif /// This QSet allows to make use of the implicit sharing @@ -1041,7 +1041,7 @@ QWidget* LegacySkinParser::parseVisual(const QDomElement& node) { for (int i = 0; i < kMaxSupportedStem; i++) { m_pContext->setVariable("StemGroup", group); m_pContext->setVariable("StemIdx", QString::number(i + 1)); - auto widget = parseWidgetGroup(stem); + auto* pWidget = parseWidgetGroup(stem); setupSize(stem, widget); viewer->stemControlWidget()->addControl(widget); } diff --git a/src/waveform/renderers/allshader/waveformrendererstem.cpp b/src/waveform/renderers/allshader/waveformrendererstem.cpp index 6b2c0210d21f..c8922ae0ee30 100644 --- a/src/waveform/renderers/allshader/waveformrendererstem.cpp +++ b/src/waveform/renderers/allshader/waveformrendererstem.cpp @@ -13,7 +13,7 @@ namespace { constexpr int kMaxSupportedStem = 4; -} +} // anonymous namespace namespace allshader { @@ -120,14 +120,16 @@ void WaveformRendererStem::paintGL() { const double maxSamplingRange = visualIncrementPerPixel / 2.0; - for (int pos = 0; pos < length; ++pos) { - for (int s = 0; s < 4; s++) { - for (int z = 0; z < 2; z++) { - QColor stemColor = stemInfo[s].getColor(); + for (int visualIdx = 0; visualIdx < length; ++visualIdx) { + for (int stemIdx = 0; stemIdx < 4; stemIdx++) { + // Stem is drawn twice with different opacity level, this allow to + // see the maximum signal by transparency + for (int layerIdx = 0; layerIdx < 2; layerIdx++) { + QColor stemColor = stemInfo[stemIdx].getColor(); float color_r = stemColor.redF(), color_g = stemColor.greenF(), color_b = stemColor.blueF(), - color_a = stemColor.alphaF() * (z ? 0.75f : 0.15f); + color_a = stemColor.alphaF() * (layerIdx ? 0.75f : 0.15f); const int visualFrameStart = std::lround(xVisualFrame - maxSamplingRange); const int visualFrameStop = std::lround(xVisualFrame + maxSamplingRange); @@ -135,7 +137,7 @@ void WaveformRendererStem::paintGL() { const int visualIndexStop = std::min(std::max(visualFrameStop, visualFrameStart + 1) * 2, dataSize - 1); - const float fpos = static_cast(pos); + const float fVisualIdx = static_cast(visualIdx); // Find the max values for current eq in the waveform data. // - Max of left and right @@ -145,7 +147,7 @@ void WaveformRendererStem::paintGL() { for (int i = visualIndexStart + chn; i < visualIndexStop + chn; i += 2) { const WaveformData& waveformData = data[i]; - u8max = math_max(u8max, waveformData.stems[s]); + u8max = math_max(u8max, waveformData.stems[stemIdx]); } } @@ -153,17 +155,17 @@ void WaveformRendererStem::paintGL() { float max = static_cast(u8max); // Apply the gains - if (z) { - max *= m_pStemMute[s]->toBool() + if (layerIdx) { + max *= m_pStemMute[stemIdx]->toBool() ? 0.f - : static_cast(m_pStemGain[s]->get()); + : static_cast(m_pStemGain[stemIdx]->get()); } // Lines are thin rectangles // shawdow - m_vertices.addRectangle(fpos - 0.5f, + m_vertices.addRectangle(fVisualIdx - 0.5f, halfBreadth - heightFactor * max, - fpos + 0.5f, + fVisualIdx + 0.5f, m_isSlipRenderer ? halfBreadth : halfBreadth + heightFactor * max); m_colors.addForRectangle(color_r, color_g, color_b, color_a); diff --git a/src/widget/wstemcontrol.cpp b/src/widget/wstemcontrol.cpp index 398d639578df..fc39fc2f21f0 100644 --- a/src/widget/wstemcontrol.cpp +++ b/src/widget/wstemcontrol.cpp @@ -28,27 +28,27 @@ namespace { // component directly void ensureWidgetRatio(QLayout* layout) { for (int i = 0; i < layout->count(); i++) { - auto w = layout->itemAt(i)->widget(); - if (!qobject_cast(w) && !qobject_cast(w)) { + auto pWidget = layout->itemAt(i)->widget(); + if (!qobject_cast(pWidget) && !qobject_cast(pWidget)) { continue; } - float ratio = static_cast(w->maximumWidth()) / - static_cast(w->maximumHeight()); - int maxSize = qMin(w->width(), w->height()); - QSize currentSize = w->size(); + float ratio = static_cast(pWidget->maximumWidth()) / + static_cast(pWidget->maximumHeight()); + int maxSize = qMin(pWidget->width(), pWidget->height()); + QSize currentSize = pWidget->size(); QSize newSize; if (ratio > 1) { - newSize = QSize(maxSize, static_cast(static_cast(maxSize) / ratio)); + newSize = QSize(maxSize, static_cast(maxSize / ratio)); } else { - newSize = QSize(static_cast(static_cast(maxSize) * ratio), maxSize); + newSize = QSize(static_cast(maxSize * ratio), maxSize); } if (currentSize == newSize) { continue; } - w->resize(newSize); + pWidget->resize(newSize); layout->itemAt(i)->invalidate(); } } @@ -57,7 +57,7 @@ void ensureWidgetRatio(QLayout* layout) { WStemControlBox::WStemControlBox( const QString& group, QWidget* parent) : WWidgetGroup(parent), m_group(group), m_hasStem(false), m_displayed(true) { - auto pLayout = new QVBoxLayout(this); + auto* pLayout = make_parented(this); pLayout->setSpacing(0); pLayout->setContentsMargins(0, 0, 0, 0); setLayout(pLayout); @@ -104,7 +104,7 @@ void WStemControlBox::slotTrackLoaded(TrackPointer track) { } void WStemControlBox::addControl(QWidget* control) { - auto pWidget = std::make_unique(control, this, m_group, m_stemControl.size()); + auto* pWidget = std::make_unique(control, this, m_group, m_stemControl.size()); layout()->addWidget(pWidget.get()); m_stemControl.push_back(std::move(pWidget)); } @@ -114,7 +114,7 @@ WStemControl::WStemControl(QWidget* widgetGroup, QWidget* parent, const QString& m_widget(widgetGroup), m_mutedCo(std::make_unique( group, QString("stem_%1_mute").arg(stemIdx + 1))) { - auto pLayout = new QHBoxLayout(this); + auto* pLayout = make_parented(this); pLayout->setSpacing(0); pLayout->setContentsMargins(0, 0, 0, 0);