Apply suggestions from code review

Co-authored-by: JoergAtGithub <[email protected]> Co-authored-by: Daniel Schürmann <[email protected]>
mixxxdj · Jul 25, 2024 · eed35f2 · eed35f2
1 parent 36af073
commit eed35f2
Show file tree

Hide file tree

Showing 8 changed files with 45 additions and 41 deletions.
diff --git 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
@@ -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
@@ -384,7 +384,7 @@ void BaseTrackPlayerImpl::loadTrack(TrackPointer pTrack) {
                 ConfigKey(m_pChannelToCloneFrom->getGroup(), "loop_end_position")));
 
 #ifdef __STEM__
-        auto pDeckToClone = qobject_cast<EngineDeck*>(m_pChannelToCloneFrom);
+        auto* pDeckToClone = qobject_cast<EngineDeck*>(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
@@ -155,7 +155,7 @@ class QmlPlayerProxy : public QObject {
     QPointer<BaseTrackPlayer> m_pTrackPlayer;
     TrackPointer m_pCurrentTrack;
     QmlBeatsModel* m_pBeatsModel;
-    QmlCuesModel* m_pHotcuesModel;
+    parented_ptr<QmlCuesModel> m_pHotcuesModel;
 #ifdef __STEM__
     std::unique_ptr<QmlStemsModel> m_pStemsModel;
 #endif

diff --git a/src/qml/qmlstemsmodel.cpp b/src/qml/qmlstemsmodel.cpp
@@ -20,7 +20,7 @@ QmlStemsModel::QmlStemsModel(
 
 void QmlStemsModel::setStems(QList<StemInfo> stems) {
     beginResetModel();
-    m_stems = QList<StemInfo>(std::move(stems));
+    m_stems = std::move(stems);
     endResetModel();
 }
 

diff --git 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
@@ -13,7 +13,7 @@
 
 namespace {
 constexpr int kMaxSupportedStem = 4;
-}
+} // anonymous namespace
 
 namespace allshader {
 
@@ -120,22 +120,24 @@ 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);
 
                 const int visualIndexStart = std::max(visualFrameStart * 2, 0);
                 const int visualIndexStop =
                         std::min(std::max(visualFrameStop, visualFrameStart + 1) * 2, dataSize - 1);
 
-                const float fpos = static_cast<float>(pos);
+                const float fVisualIdx = static_cast<float>(visualIdx);
 
                 // Find the max values for current eq in the waveform data.
                 // - Max of left and right
@@ -145,25 +147,25 @@ 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]);
                     }
                 }
 
                 // Cast to float
                 float max = static_cast<float>(u8max);
 
                 // Apply the gains
-                if (z) {
-                    max *= m_pStemMute[s]->toBool()
+                if (layerIdx) {
+                    max *= m_pStemMute[stemIdx]->toBool()
                             ? 0.f
-                            : static_cast<float>(m_pStemGain[s]->get());
+                            : static_cast<float>(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
@@ -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<WKnobComposed*>(w) && !qobject_cast<WKnob*>(w)) {
+        auto pWidget = layout->itemAt(i)->widget();
+        if (!qobject_cast<WKnobComposed*>(pWidget) && !qobject_cast<WKnob*>(pWidget)) {
             continue;
         }
-        float ratio = static_cast<float>(w->maximumWidth()) /
-                static_cast<float>(w->maximumHeight());
-        int maxSize = qMin(w->width(), w->height());
-        QSize currentSize = w->size();
+        float ratio = static_cast<float>(pWidget->maximumWidth()) /
+                static_cast<float>(pWidget->maximumHeight());
+        int maxSize = qMin(pWidget->width(), pWidget->height());
+        QSize currentSize = pWidget->size();
         QSize newSize;
 
         if (ratio > 1) {
-            newSize = QSize(maxSize, static_cast<int>(static_cast<float>(maxSize) / ratio));
+            newSize = QSize(maxSize, static_cast<int>(maxSize / ratio));
         } else {
-            newSize = QSize(static_cast<int>(static_cast<float>(maxSize) * ratio), maxSize);
+            newSize = QSize(static_cast<int>(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<QVBoxLayout>(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<WStemControl>(control, this, m_group, m_stemControl.size());
+    auto* pWidget = std::make_unique<WStemControl>(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<ControlProxy>(
                   group, QString("stem_%1_mute").arg(stemIdx + 1))) {
-    auto pLayout = new QHBoxLayout(this);
+    auto* pLayout = make_parented<QHBoxLayout>(this);
     pLayout->setSpacing(0);
     pLayout->setContentsMargins(0, 0, 0, 0);