CoreServices: Add ControlIndicatorTimer to remove GuiTick dependency

Previously, the `ControlIndicator` objects relied on the
`[Master],guiTickTime` and `[Master],guiTick50ms` that are managed by
the `GuiTick` class and processed in the vsync thread during waveform
processing. But the blinking of indicator controls is unrelated to
Waveforms and should still work when we get rid of the legacy skin
system and waveforms widgets.

This simple change introduces two new control objects:

- `[Master],indicator_250millis`
- `[Master],indicator_500millis`

Both switch between 0.0 and 1.0 every 250/500ms. `ControlIndicator` has
been adapted to use these new controls instead.

These controls may also be used by controller mappings that want to
implement custom blinking controls. Before, this was already possible
using a timer in JavaScript, but these were not synchronized with the
other blinking controls, such as `[ChannelN],play_indicator` or
`[ChannelN],cue_indicator` which made them more annoying than necessary.

CMakeLists.txt

@@ -431,6 +431,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL
   src/control/controleffectknob.cpp
   src/control/controlencoder.cpp
   src/control/controlindicator.cpp
+  src/control/controlindicatortimer.cpp
   src/control/controllinpotmeter.cpp
   src/control/controllogpotmeter.cpp
   src/control/controlmodel.cpp

src/control/controlindicator.cpp

@@ -7,11 +7,10 @@
 ControlIndicator::ControlIndicator(const ConfigKey& key)
         : ControlObject(key, false),
           m_blinkValue(OFF),
-          m_nextSwitchTime(0.0) {
-    // Tick time in audio buffer resolution
-    m_pCOTGuiTickTime = new ControlProxy("[Master]", "guiTickTime", this);
-    m_pCOTGuiTick50ms = new ControlProxy("[Master]", "guiTick50ms", this);
-    m_pCOTGuiTick50ms->connectValueChanged(this, &ControlIndicator::slotGuiTick50ms);
+          m_pCOIndicator250millis(make_parented<ControlProxy>(
+                  "[Master]", "indicator_250millis", this)),
+          m_pCOIndicator500millis(make_parented<ControlProxy>(
+                  "[Master]", "indicator_500millis", this)) {
     connect(this,
             &ControlIndicator::blinkValueChanged,
             this,
@@ -21,33 +20,39 @@ ControlIndicator::ControlIndicator(const ConfigKey& key)
 ControlIndicator::~ControlIndicator() {
 }
 
-void ControlIndicator::setBlinkValue(enum BlinkValue bv) {
-    if (m_blinkValue != bv) {
-        m_blinkValue = bv; // must be set at first, to avoid timer toggle
-        emit blinkValueChanged();
-    }
-}
-
-void ControlIndicator::slotGuiTick50ms(double cpuTime) {
-    if (m_nextSwitchTime <= cpuTime) {
+void ControlIndicator::setBlinkValue(enum BlinkValue newBlinkValue) {
+    if (m_blinkValue != newBlinkValue) {
+        // Disconnect old signals
         switch (m_blinkValue) {
+        case RATIO1TO1_250MS:
+            m_pCOIndicator250millis->disconnect(this);
+            break;
         case RATIO1TO1_500MS:
-            toggle(0.5);
+            m_pCOIndicator500millis->disconnect(this);
             break;
+        default:
+            // Nothing to do
+            break;
+        }
+
+        m_blinkValue = newBlinkValue;
+        switch (m_blinkValue) {
         case RATIO1TO1_250MS:
-            toggle(0.25);
+            m_pCOIndicator250millis->connectValueChanged(this, &ControlIndicator::slotValueChanged);
+            break;
+        case RATIO1TO1_500MS:
+            m_pCOIndicator500millis->connectValueChanged(this, &ControlIndicator::slotValueChanged);
             break;
-        case OFF: // fall through
-        case ON: // fall through
         default:
-            // nothing to do
+            // Nothing to do
             break;
         }
+        emit blinkValueChanged();
     }
 }
 
 void ControlIndicator::slotBlinkValueChanged() {
-    bool oldValue = toBool();
+    const bool oldValue = toBool();
 
     switch (m_blinkValue) {
     case OFF:
@@ -60,28 +65,14 @@ void ControlIndicator::slotBlinkValueChanged() {
             set(1.0);
         }
         break;
-    case RATIO1TO1_500MS:
-        toggle(0.5);
-        break;
     case RATIO1TO1_250MS:
-        toggle(0.25);
+        set(m_pCOIndicator250millis->get());
+        break;
+    case RATIO1TO1_500MS:
+        set(m_pCOIndicator500millis->get());
         break;
     default:
         // nothing to do
         break;
     }
 }
-
-void ControlIndicator::toggle(double duration) {
-    double tickTime = m_pCOTGuiTickTime->get();
-    double toggles = floor(tickTime / duration);
-    bool phase = fmod(toggles, 2) >= 1;
-    bool val = toBool();
-    if(val != phase) {
-        // Out of phase, wait until we are in phase
-        m_nextSwitchTime = (toggles + 2) * duration;
-    } else {
-        m_nextSwitchTime = (toggles + 1) * duration;
-    }
-    set(val ? 0.0 : 1.0);
-}

src/control/controlindicator.h

@@ -1,9 +1,14 @@
 #pragma once
 
 #include "control/controlobject.h"
-
-class ControlProxy;
-
+#include "control/controlproxy.h"
+#include "util/parented_ptr.h"
+
+/// This type of control is used for blinking buttons such as the "play" or
+/// "cue" button and makes buttons that using the same blink interval to light
+/// up at the same time in the UI and on controllers.
+//
+/// Requires `ControlIndicatorTimer` to be initialized first.
 class ControlIndicator : public ControlObject {
     Q_OBJECT
   public:
@@ -17,24 +22,19 @@ class ControlIndicator : public ControlObject {
     ControlIndicator(const ConfigKey& key);
     virtual ~ControlIndicator();
 
-    void setBlinkValue(enum BlinkValue bv);
+    void setBlinkValue(enum BlinkValue newBlinkValue);
 
   signals:
     void blinkValueChanged();
 
   private slots:
-    void slotGuiTick50ms(double cpuTime);
     void slotBlinkValueChanged();
+    void slotValueChanged(double value) {
+        set(value);
+    };
 
   private:
-    void toggle(double duration);
-    // set() is private, use setBlinkValue instead
-    // it must be called from the GUI thread only to a void
-    // race condition by toggle()
-    void set(double value) { ControlObject::set(value); };
-
     enum BlinkValue m_blinkValue;
-    double m_nextSwitchTime;
-    ControlProxy* m_pCOTGuiTickTime;
-    ControlProxy* m_pCOTGuiTick50ms;
+    parented_ptr<ControlProxy> m_pCOIndicator250millis;
+    parented_ptr<ControlProxy> m_pCOIndicator500millis;
 };

src/control/controlindicatortimer.cpp

@@ -0,0 +1,32 @@
+#include "control/controlindicatortimer.h"
+
+#include "control/controlobject.h"
+#include "moc_controlindicatortimer.cpp"
+
+namespace mixxx {
+
+ControlIndicatorTimer::ControlIndicatorTimer(QObject* pParent)
+        : QObject(pParent),
+          m_pCOIndicator250millis(std::make_unique<ControlObject>(
+                  ConfigKey("[Master]", "indicator_250millis"))),
+          m_pCOIndicator500millis(std::make_unique<ControlObject>(
+                  ConfigKey("[Master]", "indicator_500millis"))),
+          m_toggleIndicator500millisOnNextTimeout(true) {
+    m_pCOIndicator250millis->setReadOnly();
+    m_pCOIndicator500millis->setReadOnly();
+    connect(&m_timer, &QTimer::timeout, this, &ControlIndicatorTimer::slotTimeout);
+    m_timer.start(250);
+}
+
+void ControlIndicatorTimer::slotTimeout() {
+    // The timeout uses an interval of 250ms, so the 250ms indicator is always updated.
+    m_pCOIndicator250millis->forceSet(1 - m_pCOIndicator250millis->get());
+
+    // The 500ms indicator is only updated on every second call to the function.
+    if (m_toggleIndicator500millisOnNextTimeout) {
+        m_pCOIndicator500millis->forceSet(1 - m_pCOIndicator500millis->get());
+    }
+    m_toggleIndicator500millisOnNextTimeout = !m_toggleIndicator500millisOnNextTimeout;
+}
+
+} // namespace mixxx

src/control/controlindicatortimer.h

@@ -0,0 +1,33 @@
+#pragma once
+
+#include <QObject>
+#include <QTimer>
+#include <memory>
+
+#include "control/controlobject.h"
+
+namespace mixxx {
+
+/// This class provides two generic indicator controls that change their value
+/// in intervals of 250 milliseconds and 500 milliseconds. These controls are
+/// used by all `ControlIndicator` COs and may also be used to implement custom
+/// blinking buttons (e.g. for Explicit Sync Leader) on controllers. This
+/// ensures that all blinking buttons that use the same interval light up at the
+/// same time.
+class ControlIndicatorTimer : public QObject {
+    Q_OBJECT
+  public:
+    ControlIndicatorTimer(QObject* pParent = nullptr);
+
+  private slots:
+    /// Called every 250 milliseconds
+    void slotTimeout();
+
+  private:
+    QTimer m_timer;
+    std::unique_ptr<ControlObject> m_pCOIndicator250millis;
+    std::unique_ptr<ControlObject> m_pCOIndicator500millis;
+    bool m_toggleIndicator500millisOnNextTimeout;
+};
+
+} // namespace mixxx

src/coreservices.cpp

@@ -7,6 +7,7 @@
 #ifdef __BROADCAST__
 #include "broadcast/broadcastmanager.h"
 #endif
+#include "control/controlindicatortimer.h"
 #include "controllers/controllermanager.h"
 #include "controllers/keyboard/keyboardeventfilter.h"
 #include "database/mixxxdb.h"
@@ -107,6 +108,8 @@ CoreServices::CoreServices(const CmdlineArgs& args)
           m_cmdlineArgs(args) {
 }
 
+CoreServices::~CoreServices() = default;
+
 void CoreServices::initializeSettings() {
 #ifdef __APPLE__
     // TODO: At this point it is too late to provide the same settings path to all components
@@ -185,6 +188,8 @@ void CoreServices::initialize(QApplication* pApp) {
         exit(-1);
     }
 
+    m_pControlIndicatorTimer = std::make_unique<mixxx::ControlIndicatorTimer>(this);
+
     auto pChannelHandleFactory = std::make_shared<ChannelHandleFactory>();
 
     emit initializationProgressUpdate(20, tr("effects"));
@@ -545,6 +550,8 @@ void CoreServices::shutdown() {
 
     m_pTouchShift.reset();
 
+    m_pControlIndicatorTimer.reset();
+
     // Check for leaked ControlObjects and give warnings.
     {
         const QList<QSharedPointer<ControlDoublePrivate>> leakedControls =

src/coreservices.h

@@ -1,5 +1,7 @@
 #pragma once
 
+#include <memory>
+
 #include "control/controlpushbutton.h"
 #include "preferences/configobject.h"
 #include "preferences/constants.h"
@@ -27,6 +29,7 @@ class LV2Backend;
 
 namespace mixxx {
 
+class ControlIndicatorTimer;
 class DbConnectionPool;
 class ScreensaverManager;
 
@@ -35,7 +38,7 @@ class CoreServices : public QObject {
 
   public:
     CoreServices(const CmdlineArgs& args);
-    ~CoreServices() = default;
+    ~CoreServices();
 
     void initializeSettings();
     // FIXME: should be private, but WMainMenuBar needs it initialized early
@@ -116,6 +119,7 @@ class CoreServices : public QObject {
     bool initializeDatabase();
 
     std::shared_ptr<SettingsManager> m_pSettingsManager;
+    std::unique_ptr<mixxx::ControlIndicatorTimer> m_pControlIndicatorTimer;
     std::shared_ptr<EffectsManager> m_pEffectsManager;
     // owned by EffectsManager
     LV2Backend* m_pLV2Backend;

src/test/signalpathtest.h

@@ -6,6 +6,7 @@
 #include <QTest>
 #include <QtDebug>
 
+#include "control/controlindicatortimer.h"
 #include "control/controlobject.h"
 #include "effects/effectsmanager.h"
 #include "engine/bufferscalers/enginebufferscale.h"
@@ -27,7 +28,6 @@
 #include "util/memory.h"
 #include "util/sample.h"
 #include "util/types.h"
-#include "waveform/guitick.h"
 
 using ::testing::Return;
 using ::testing::_;
@@ -59,7 +59,7 @@ class TestEngineMaster : public EngineMaster {
 class BaseSignalPathTest : public MixxxTest, SoundSourceProviderRegistration {
   protected:
     BaseSignalPathTest() {
-        m_pGuiTick = std::make_unique<GuiTick>();
+        m_pControlIndicatorTimer = std::make_unique<mixxx::ControlIndicatorTimer>();
         m_pChannelHandleFactory = std::make_shared<ChannelHandleFactory>();
         m_pNumDecks = new ControlObject(ConfigKey(m_sMasterGroup, "num_decks"));
         m_pEffectsManager = new EffectsManager(NULL, config(), m_pChannelHandleFactory);
@@ -225,7 +225,7 @@ class BaseSignalPathTest : public MixxxTest, SoundSourceProviderRegistration {
 
     ChannelHandleFactoryPointer m_pChannelHandleFactory;
     ControlObject* m_pNumDecks;
-    std::unique_ptr<GuiTick> m_pGuiTick;
+    std::unique_ptr<mixxx::ControlIndicatorTimer> m_pControlIndicatorTimer;
     EffectsManager* m_pEffectsManager;
     EngineSync* m_pEngineSync;
     TestEngineMaster* m_pEngineMaster;