Unified range checking behavior for BeatJumpSize and BeatLoopSize between GUI-Widget and controller CO

src/engine/controls/loopingcontrol.cpp

@@ -17,6 +17,9 @@
 namespace {
 constexpr mixxx::audio::FrameDiff_t kMinimumAudibleLoopSizeFrames = 150;
 
+constexpr double kMinBeatJumpSize = 0.03125;
+constexpr double kMaxBeatJumpSize = 512;
+
 // returns true if a is valid and is fairly close to target (within +/- 1 frame).
 bool positionNear(mixxx::audio::FramePos a, mixxx::audio::FramePos target) {
     return a.isValid() && a > target - 1 && a < target + 1;
@@ -175,6 +178,9 @@ LoopingControl::LoopingControl(const QString& group,
             this, &LoopingControl::slotBeatJump, Qt::DirectConnection);
     m_pCOBeatJumpSize = new ControlObject(ConfigKey(group, "beatjump_size"),
                                           true, false, false, 4.0);
+    m_pCOBeatJumpSize->connectValueChangeRequest(this,
+            &LoopingControl::slotBeatJumpSizeChangeRequest,
+            Qt::DirectConnection);
 
     m_pCOBeatJumpSizeHalve = new ControlPushButton(ConfigKey(group, "beatjump_size_halve"));
     connect(m_pCOBeatJumpSizeHalve,
@@ -340,15 +346,15 @@ void LoopingControl::slotLoopHalve(double pressed) {
         return;
     }
 
-    slotBeatLoop(m_pCOBeatLoopSize->get() / 2.0, true, false);
+    m_pCOBeatLoopSize->set(m_pCOBeatLoopSize->get() / 2.0);
 }
 
 void LoopingControl::slotLoopDouble(double pressed) {
     if (pressed <= 0.0) {
         return;
     }
 
-    slotBeatLoop(m_pCOBeatLoopSize->get() * 2.0, true, false);
+    m_pCOBeatLoopSize->set(m_pCOBeatLoopSize->get() * 2.0);
 }
 
 void LoopingControl::process(const double dRate,
@@ -1429,6 +1435,14 @@ void LoopingControl::slotBeatLoop(double beats, bool keepStartPoint, bool enable
 void LoopingControl::slotBeatLoopSizeChangeRequest(double beats) {
     // slotBeatLoop will call m_pCOBeatLoopSize->setAndConfirm if
     // new beatloop_size is valid
+
+    double maxBeatSize = s_dBeatSizes[sizeof(s_dBeatSizes) / sizeof(s_dBeatSizes[0]) - 1];
+    double minBeatSize = s_dBeatSizes[0];
+    if ((beats < minBeatSize) || (beats > maxBeatSize)) {
+        // Don't clamp the value here to not fall out of a measure
+        return;
+    }
+
     slotBeatLoop(beats, true, false);
 }
 
@@ -1495,6 +1509,14 @@ void LoopingControl::slotBeatJump(double beats) {
     }
 }
 
+void LoopingControl::slotBeatJumpSizeChangeRequest(double beats) {
+    if ((beats < kMinBeatJumpSize) || (beats > kMaxBeatJumpSize)) {
+        // Don't clamp the value here to not fall out of a measure
+        return;
+    }
+    m_pCOBeatJumpSize->setAndConfirm(beats);
+}
+
 void LoopingControl::slotBeatJumpSizeHalve(double pressed) {
     if (pressed > 0) {
         m_pCOBeatJumpSize->set(m_pCOBeatJumpSize->get() / 2);

src/engine/controls/loopingcontrol.h

@@ -87,6 +87,7 @@ class LoopingControl : public EngineControl {
 
     // Jump forward or backward by beats.
     void slotBeatJump(double beats);
+    void slotBeatJumpSizeChangeRequest(double beats);
     void slotBeatJumpSizeHalve(double pressed);
     void slotBeatJumpSizeDouble(double pressed);
     void slotBeatJumpForward(double pressed);

src/test/looping_control_test.cpp

@@ -67,6 +67,10 @@ class LoopingControlTest : public MockedEngineBackendTest {
         m_pButtonBeatLoopActivate = std::make_unique<PollingControlProxy>(
                 m_sGroup1, "beatloop_activate");
         m_pBeatJumpSize = std::make_unique<PollingControlProxy>(m_sGroup1, "beatjump_size");
+        m_pButtonBeatJumpSizeDouble = std::make_unique<PollingControlProxy>(
+                m_sGroup1, "beatjump_size_double");
+        m_pButtonBeatJumpSizeHalve = std::make_unique<PollingControlProxy>(
+                m_sGroup1, "beatjump_size_halve");
         m_pButtonBeatJumpForward = std::make_unique<PollingControlProxy>(
                 m_sGroup1, "beatjump_forward");
         m_pButtonBeatJumpBackward = std::make_unique<PollingControlProxy>(
@@ -121,6 +125,8 @@ class LoopingControlTest : public MockedEngineBackendTest {
     std::unique_ptr<PollingControlProxy> m_pBeatLoopSize;
     std::unique_ptr<PollingControlProxy> m_pButtonBeatLoopActivate;
     std::unique_ptr<PollingControlProxy> m_pBeatJumpSize;
+    std::unique_ptr<PollingControlProxy> m_pButtonBeatJumpSizeHalve;
+    std::unique_ptr<PollingControlProxy> m_pButtonBeatJumpSizeDouble;
     std::unique_ptr<PollingControlProxy> m_pButtonBeatJumpForward;
     std::unique_ptr<PollingControlProxy> m_pButtonBeatJumpBackward;
     std::unique_ptr<PollingControlProxy> m_pButtonBeatLoopRoll1Activate;
@@ -712,6 +718,45 @@ TEST_F(LoopingControlTest, BeatLoopSize_IsSetByNumberedControl) {
     EXPECT_EQ(2.0, m_pBeatLoopSize->get());
 }
 
+TEST_F(LoopingControlTest, BeatLoopSize_SetRangeCheck) {
+    m_pBeatLoopSize->set(512.0);
+    EXPECT_EQ(512, m_pBeatLoopSize->get());
+
+    m_pBeatLoopSize->set(150.0);
+    EXPECT_EQ(150, m_pBeatLoopSize->get());
+
+    m_pBeatLoopSize->set(513.0);
+    EXPECT_EQ(150, m_pBeatLoopSize->get());
+
+    m_pButtonLoopDouble->set(1.0);
+    m_pButtonLoopDouble->set(0.0);
+    EXPECT_EQ(300.0, m_pBeatLoopSize->get());
+
+    m_pButtonLoopDouble->set(1.0);
+    m_pButtonLoopDouble->set(0.0);
+    EXPECT_EQ(300.0, m_pBeatLoopSize->get());
+
+    m_pBeatLoopSize->set(1 / 32.0);
+    EXPECT_EQ(1 / 32.0, m_pBeatLoopSize->get());
+
+    m_pBeatLoopSize->set(1 / 10.0);
+    EXPECT_EQ(1 / 10.0, m_pBeatLoopSize->get());
+
+    m_pBeatLoopSize->set(1 / 33.0);
+    EXPECT_EQ(1 / 10.0, m_pBeatLoopSize->get());
+
+    m_pBeatLoopSize->set(0);
+    EXPECT_EQ(1 / 10.0, m_pBeatLoopSize->get());
+
+    m_pButtonLoopHalve->set(1.0);
+    m_pButtonLoopHalve->set(0.0);
+    EXPECT_EQ(1 / 20.0, m_pBeatLoopSize->get());
+
+    m_pButtonLoopHalve->set(1.0);
+    m_pButtonLoopHalve->set(0.0);
+    EXPECT_EQ(1 / 20.0, m_pBeatLoopSize->get());
+}
+
 TEST_F(LoopingControlTest, BeatLoopSize_SetDoesNotStartLoop) {
     m_pTrack1->trySetBpm(120.0);
     m_pBeatLoopSize->set(16.0);
@@ -814,6 +859,45 @@ TEST_F(LoopingControlTest, LegacyBeatLoopControl) {
     EXPECT_EQ(6.0, m_pBeatLoopSize->get());
 }
 
+TEST_F(LoopingControlTest, BeatjumpSize_SetRangeCheck) {
+    m_pBeatJumpSize->set(512.0);
+    EXPECT_EQ(512, m_pBeatJumpSize->get());
+
+    m_pBeatJumpSize->set(150.0);
+    EXPECT_EQ(150, m_pBeatJumpSize->get());
+
+    m_pBeatJumpSize->set(513.0);
+    EXPECT_EQ(150, m_pBeatJumpSize->get());
+
+    m_pButtonBeatJumpSizeDouble->set(1.0);
+    m_pButtonBeatJumpSizeDouble->set(0.0);
+    EXPECT_EQ(300.0, m_pBeatJumpSize->get());
+
+    m_pButtonBeatJumpSizeDouble->set(1.0);
+    m_pButtonBeatJumpSizeDouble->set(0.0);
+    EXPECT_EQ(300.0, m_pBeatJumpSize->get());
+
+    m_pBeatJumpSize->set(1 / 32.0);
+    EXPECT_EQ(1 / 32.0, m_pBeatJumpSize->get());
+
+    m_pBeatJumpSize->set(1 / 10.0);
+    EXPECT_EQ(1 / 10.0, m_pBeatJumpSize->get());
+
+    m_pBeatJumpSize->set(1 / 33.0);
+    EXPECT_EQ(1 / 10.0, m_pBeatJumpSize->get());
+
+    m_pBeatJumpSize->set(0);
+    EXPECT_EQ(1 / 10.0, m_pBeatJumpSize->get());
+
+    m_pButtonBeatJumpSizeHalve->set(1.0);
+    m_pButtonBeatJumpSizeHalve->set(0.0);
+    EXPECT_EQ(1 / 20.0, m_pBeatJumpSize->get());
+
+    m_pButtonBeatJumpSizeHalve->set(1.0);
+    m_pButtonBeatJumpSizeHalve->set(0.0);
+    EXPECT_EQ(1 / 20.0, m_pBeatJumpSize->get());
+}
+
 TEST_F(LoopingControlTest, Beatjump_JumpsByBeats) {
     const auto bpm = mixxx::Bpm{120};
     m_pTrack1->trySetBpm(bpm);

src/widget/wbeatspinbox.cpp

@@ -54,12 +54,7 @@ void WBeatSpinBox::stepBy(int steps) {
     QString temp = text();
     int cursorPos = lineEdit()->cursorPosition();
     if (validate(temp, cursorPos) == QValidator::Acceptable) {
-        double editValue = valueFromText(temp);
-        newValue = editValue * pow(2, steps);
-        if (newValue < minimum() || newValue > maximum()) {
-            // don't clamp the value here to not fall out of a measure
-            newValue = editValue;
-        }
+        newValue = valueFromText(temp) * pow(2, steps);
     } else {
         // here we have an unacceptable edit, going back to the old value first
         newValue = oldValue;