Do not allow negative field values

Due to epsilon addition, subsequent fields can get negative. Always clamp them to zero.
crsib · Sep 27, 2023 · 0273ea1 · 0273ea1
1 parent 0f54c87
commit 0273ea1
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 27 deletions.
diff --git a/libraries/lib-numeric-formats/formatters/BeatsNumericConverterFormatter.cpp b/libraries/lib-numeric-formats/formatters/BeatsNumericConverterFormatter.cpp
@@ -249,7 +249,8 @@ class BeatsFormatter final :
       for (size_t fieldIndex = 0; fieldIndex < mFields.size(); ++fieldIndex)
       {
          const auto fieldLength = mFieldLengths[fieldIndex];
-         const auto fieldValue = static_cast<int>(std::floor(value * eps / fieldLength));
+         const auto fieldValue = std::max(
+            0, static_cast<int>(std::floor(value * eps / fieldLength)));
 
          result.fieldValueStrings[fieldIndex] = wxString::Format(
             mFields[fieldIndex].formatStr, fieldValue + mFieldValueOffset);

diff --git a/libraries/lib-numeric-formats/tests/NumericConverterTests.cpp b/libraries/lib-numeric-formats/tests/NumericConverterTests.cpp
@@ -195,37 +195,44 @@ TEST_CASE("BeatsNumericConverterFormatter", "")
    REQUIRE(durationFormatter->SingleStep(0.0, 4, true) == Approx(0.5));
    REQUIRE(durationFormatter->SingleStep(0.0, 3, true) == Approx(5));
 
-   timeSignature.SetTempo(117);
-   timeSignature.SetUpperTimeSignature(4);
-   timeSignature.SetLowerTimeSignature(4);
+   auto longFormatterTest = [&](double tempo)
+   {
+      timeSignature.SetTempo(tempo);
+      timeSignature.SetUpperTimeSignature(4);
+      timeSignature.SetLowerTimeSignature(4);
 
-   const auto t = *basicFormatter->StringToValue("009 bar 02 beat");
-   const auto r = basicFormatter->ValueToString(t, true).valueString;
+      //const auto t = *basicFormatter->StringToValue("009 bar 02 beat");
+      //const auto r = basicFormatter->ValueToString(t, true).valueString;
 
-   REQUIRE("009 bar 02 beat" == r);
+      //REQUIRE("009 bar 02 beat" == r);
 
-   for (int32_t bar = 0; bar < 100000; ++bar)
-   {
-      for (int32_t beat = 0; beat < 4; ++beat)
+      for (int32_t bar = 0; bar < 100000; ++bar)
       {
-         const auto value =
-            bar * timeSignature.GetBarDuration() + beat * timeSignature.GetBeatDuration();
-
-         basicFormatter->UpdateFormatForValue(value);
-
-         const auto formattedString =
-            wxString::Format("%03d bar %02d beat", bar + 1, beat + 1);
-
-         REQUIRE(
-            *basicFormatter->StringToValue(formattedString) == Approx(value));
-
-         REQUIRE(
-            basicFormatter
-               ->ValueToString(
-                  *basicFormatter->StringToValue(formattedString), true)
-               .valueString == formattedString);
+         for (int32_t beat = 0; beat < 4; ++beat)
+         {
+            const auto value = bar * timeSignature.GetBarDuration() +
+                               beat * timeSignature.GetBeatDuration();
+
+            basicFormatter->UpdateFormatForValue(value);
+
+            const auto formattedString =
+               wxString::Format("%03d bar %02d beat", bar + 1, beat + 1);
+
+            REQUIRE(
+               *basicFormatter->StringToValue(formattedString) ==
+               Approx(value));
+
+            REQUIRE(
+               basicFormatter
+                  ->ValueToString(
+                     *basicFormatter->StringToValue(formattedString), true)
+                  .valueString == formattedString);
+         }
       }
-   }
+   };
+
+   longFormatterTest(88);
+   longFormatterTest(117);
 }