From 5e7da72a29e70d545efeb347135fe47652086f01 Mon Sep 17 00:00:00 2001 From: Nikodemus Siivola Date: Tue, 16 Jul 2024 19:55:53 +0300 Subject: [PATCH] fix defaultSwingInterval handling - Display the interval relative to defaultMagnitude, not current song magnitude. - When defaultMagnitude changes, adjust the defaultSwingInterval to compensate. - A bit of a kludge to keep tests running, should really make the newly mocked bits properly testable. --- src/deluge/gui/menu_item/defaults/magnitude.h | 11 +++++++-- .../gui/menu_item/defaults/swing_interval.h | 5 ++++ src/deluge/gui/menu_item/sync_level.cpp | 2 +- src/deluge/model/song/song.cpp | 23 +++++++------------ src/deluge/model/song/song.h | 1 + src/deluge/model/sync.cpp | 23 +++++++++++++++---- src/deluge/model/sync.h | 5 +++- src/deluge/playback/playback_handler.cpp | 4 ++-- src/deluge/util/functions.cpp | 13 +++++++++++ src/deluge/util/functions.h | 1 + tests/unit/mocks/function_mocks.h | 10 ++++++++ 11 files changed, 73 insertions(+), 25 deletions(-) create mode 100644 tests/unit/mocks/function_mocks.h diff --git a/src/deluge/gui/menu_item/defaults/magnitude.h b/src/deluge/gui/menu_item/defaults/magnitude.h index ace29fbfbf..2472b0bae5 100644 --- a/src/deluge/gui/menu_item/defaults/magnitude.h +++ b/src/deluge/gui/menu_item/defaults/magnitude.h @@ -18,6 +18,7 @@ #include "gui/menu_item/enumeration.h" #include "hid/display/display.h" #include "hid/display/oled.h" +#include "model/sync.h" #include "storage/flash_storage.h" #include "util/d_string.h" @@ -26,8 +27,14 @@ class Magnitude final : public Enumeration { public: using Enumeration::Enumeration; void readCurrentValue() override { this->setValue(FlashStorage::defaultMagnitude); } - void writeCurrentValue() override { FlashStorage::defaultMagnitude = this->getValue(); } - + void writeCurrentValue() override { + int32_t oldMagnitude = FlashStorage::defaultMagnitude; + FlashStorage::defaultMagnitude = this->getValue(); + // Adjust swing interval to keep the same musical meaning if possible: magnitude affects its interpretation. + int32_t delta = FlashStorage::defaultMagnitude - oldMagnitude; + int32_t oldSwing = FlashStorage::defaultSwingInterval; + FlashStorage::defaultSwingInterval = clampSwingIntervalSyncLevel(FlashStorage::defaultSwingInterval - delta); + } void drawPixelsForOled() override { char buffer[12]; deluge::hid::display::oled_canvas::Canvas& canvas = hid::display::OLED::main; diff --git a/src/deluge/gui/menu_item/defaults/swing_interval.h b/src/deluge/gui/menu_item/defaults/swing_interval.h index 2bdc582ae8..6e152e2a1a 100644 --- a/src/deluge/gui/menu_item/defaults/swing_interval.h +++ b/src/deluge/gui/menu_item/defaults/swing_interval.h @@ -32,6 +32,11 @@ class SwingInterval final : public Interval { // eg. Velocity immediately changes the default velocity of the current song, but tempo // and swing don't. So We don't either. } + +protected: + void getNoteLengthName(StringBuf& buffer) override { + syncValueToString(this->getValue(), buffer, FlashStorage::defaultMagnitude); + } }; } // namespace deluge::gui::menu_item::defaults diff --git a/src/deluge/gui/menu_item/sync_level.cpp b/src/deluge/gui/menu_item/sync_level.cpp index 3280a39a5e..d519bd78cc 100644 --- a/src/deluge/gui/menu_item/sync_level.cpp +++ b/src/deluge/gui/menu_item/sync_level.cpp @@ -35,7 +35,7 @@ void SyncLevel::drawValue() { } void SyncLevel::getNoteLengthName(StringBuf& buffer) { - syncValueToString(this->getValue(), buffer); + syncValueToString(this->getValue(), buffer, currentSong->getInputTickMagnitude()); } void SyncLevel::drawPixelsForOled() { diff --git a/src/deluge/model/song/song.cpp b/src/deluge/model/song/song.cpp index e677751cd8..6d84abffc4 100644 --- a/src/deluge/model/song/song.cpp +++ b/src/deluge/model/song/song.cpp @@ -1229,7 +1229,7 @@ void Song::writeToFile(StorageManager& bdsm) { writer.writeAttribute("timePerTimerTick", timePerTimerTickBig >> 32); writer.writeAttribute("timerTickFraction", (uint32_t)timePerTimerTickBig); writer.writeAttribute("rootNote", key.rootNote); - writer.writeAttribute("inputTickMagnitude", insideWorldTickMagnitude + insideWorldTickMagnitudeOffsetFromBPM); + writer.writeAttribute("inputTickMagnitude", getInputTickMagnitude()); writer.writeAttribute("swingAmount", swingAmount); writer.writeAbsoluteSyncLevelToFile(this, "swingInterval", (SyncLevel)swingInterval, true); @@ -3125,7 +3125,7 @@ void Song::setBPM(float tempoBPM, bool shouldLogAction) { void Song::setTempoFromParams(int32_t magnitude, int8_t whichValue, bool shouldLogAction) { float newBPM = metronomeValuesBPM[whichValue]; - magnitude += insideWorldTickMagnitude + insideWorldTickMagnitudeOffsetFromBPM; + magnitude += getInputTickMagnitude(); if (magnitude > 0) { newBPM /= ((uint32_t)1 << magnitude); } @@ -5190,15 +5190,8 @@ void Song::replaceOutputLowLevel(Output* newOutput, Output* oldOutput) { void Song::getNoteLengthName(StringBuf& buffer, uint32_t noteLength, char const* const notesString, bool clarifyPerColumn) const { - int32_t magnitude = -5 - (insideWorldTickMagnitude + insideWorldTickMagnitudeOffsetFromBPM); - uint32_t level = 3; - - while (level < noteLength) { - magnitude++; - level <<= 1; - } - - getNoteLengthNameFromMagnitude(buffer, magnitude, notesString, clarifyPerColumn); + getNoteLengthNameFromMagnitude(buffer, getNoteMagnitudeFfromNoteLength(noteLength, getInputTickMagnitude()), + notesString, clarifyPerColumn); } Instrument* Song::getNonAudioInstrumentToSwitchTo(OutputType newOutputType, Availability availabilityRequirement, @@ -5637,11 +5630,11 @@ void Song::changeSwingInterval(int32_t newValue) { } uint32_t Song::getQuarterNoteLength() { - return increaseMagnitude(24, insideWorldTickMagnitude + insideWorldTickMagnitudeOffsetFromBPM); + return increaseMagnitude(24, getInputTickMagnitude()); } uint32_t Song::getBarLength() { - return increaseMagnitude(96, insideWorldTickMagnitude + insideWorldTickMagnitudeOffsetFromBPM); + return increaseMagnitude(96, getInputTickMagnitude()); } // ----- PlayPositionCounter implementation ------- @@ -5723,7 +5716,7 @@ int32_t Song::convertSyncLevelFromFileValueToInternalValue(int32_t fileValue) { if (fileValue == 0) { return 0; // 0 means "off" } - int32_t internalValue = fileValue + 1 - (insideWorldTickMagnitude + insideWorldTickMagnitudeOffsetFromBPM); + int32_t internalValue = fileValue + 1 - (getInputTickMagnitude()); if (internalValue < 1) { internalValue = 1; } @@ -5739,7 +5732,7 @@ int32_t Song::convertSyncLevelFromInternalValueToFileValue(int32_t internalValue if (internalValue == 0) { return 0; // 0 means "off" } - int32_t fileValue = internalValue - 1 + (insideWorldTickMagnitude + insideWorldTickMagnitudeOffsetFromBPM); + int32_t fileValue = internalValue - 1 + (getInputTickMagnitude()); if (fileValue < 1) { fileValue = 1; } diff --git a/src/deluge/model/song/song.h b/src/deluge/model/song/song.h index a1933e999f..a764c68f09 100644 --- a/src/deluge/model/song/song.h +++ b/src/deluge/model/song/song.h @@ -147,6 +147,7 @@ class Song final : public TimelineCounter { int32_t convertSyncLevelFromInternalValueToFileValue(int32_t internalValue); String getSongFullPath(); void setSongFullPath(const char* fullPath); + int32_t getInputTickMagnitude() const { return insideWorldTickMagnitude + insideWorldTickMagnitudeOffsetFromBPM; } GlobalEffectableForSong globalEffectable; diff --git a/src/deluge/model/sync.cpp b/src/deluge/model/sync.cpp index dcb90ee8e4..ee516cd76a 100644 --- a/src/deluge/model/sync.cpp +++ b/src/deluge/model/sync.cpp @@ -5,9 +5,11 @@ #ifdef IN_UNIT_TESTS #include "display_mock.h" #include "song_mock.h" +#include "function_mocks.h" #else #include "hid/display/display.h" #include "model/song/song.h" +#include "util/functions.h" #endif int32_t wrapSwingIntervalSyncLevel(int32_t value) { @@ -15,6 +17,18 @@ int32_t wrapSwingIntervalSyncLevel(int32_t value) { return mod(value - 1, NUM_SWING_INTERVALS - 1) + 1; } +int32_t clampSwingIntervalSyncLevel(int32_t value) { + if (value < MIN_SWING_INERVAL) { + return MIN_SWING_INERVAL; + } + else if (value > MAX_SWING_INTERVAL) { + return MAX_SWING_INTERVAL; + } + else { + return value; + } +} + enum SyncType syncValueToSyncType(int32_t value) { if (value < SYNC_TYPE_TRIPLET) { return SYNC_TYPE_EVEN; @@ -39,7 +53,7 @@ enum SyncLevel syncValueToSyncLevel(int32_t option) { } } -void syncValueToString(uint32_t value, StringBuf& buffer) { +void syncValueToString(uint32_t value, StringBuf& buffer, int32_t tickMagnitude) { char const* typeStr = nullptr; enum SyncType type { syncValueToSyncType(value) }; enum SyncLevel level { syncValueToSyncLevel(value) }; @@ -61,12 +75,13 @@ void syncValueToString(uint32_t value, StringBuf& buffer) { break; } - currentSong->getNoteLengthName(buffer, noteLength, typeStr); + getNoteLengthNameFromMagnitude(buffer, getNoteMagnitudeFfromNoteLength(noteLength, tickMagnitude), typeStr, false); if (typeStr != nullptr) { - int32_t magnitudeLevelBars = SYNC_LEVEL_8TH - currentSong->insideWorldTickMagnitude; + int32_t magnitudeLevelBars = SYNC_LEVEL_8TH - tickMagnitude; if (((type == SYNC_TYPE_TRIPLET || type == SYNC_TYPE_DOTTED) && level <= magnitudeLevelBars) || display->have7SEG()) { - // On OLED, getNoteLengthName handles adding this for the non-bar levels. On 7seg, always append it + // On OLED, getNoteLengthNameForMagniture() handles adding this for the non-bar levels. On 7seg, always + // append it buffer.append(typeStr); } } diff --git a/src/deluge/model/sync.h b/src/deluge/model/sync.h index 57bde6353f..b77347b098 100644 --- a/src/deluge/model/sync.h +++ b/src/deluge/model/sync.h @@ -39,7 +39,10 @@ enum SyncLevel syncValueToSyncLevel(int32_t option); enum SyncType syncValueToSyncType(int32_t value); -void syncValueToString(uint32_t value, StringBuf& buffer); +void syncValueToString(uint32_t value, StringBuf& buffer, int32_t tickMagnitude); /** Modulus of value as a non-zero SyncLevel valid for Swing interval. */ int32_t wrapSwingIntervalSyncLevel(int32_t value); + +/** Clamp value to a valid Swing interval. */ +int32_t clampSwingIntervalSyncLevel(int32_t value); diff --git a/src/deluge/playback/playback_handler.cpp b/src/deluge/playback/playback_handler.cpp index 3840a2b60b..7fed8f7788 100644 --- a/src/deluge/playback/playback_handler.cpp +++ b/src/deluge/playback/playback_handler.cpp @@ -1829,7 +1829,7 @@ void PlaybackHandler::displaySwingInterval() { } text.append("\n"); } - syncValueToString(currentSong->swingInterval, text); + syncValueToString(currentSong->swingInterval, text, currentSong->getInputTickMagnitude()); display->popupTextTemporary(text.c_str(), DisplayPopupType::SWING); } @@ -1845,7 +1845,7 @@ void PlaybackHandler::displaySwingAmount() { text.appendInt(currentSong->swingAmount + 50); } text.append("\n"); - syncValueToString(currentSong->swingInterval, text); + syncValueToString(currentSong->swingInterval, text, currentSong->getInputTickMagnitude()); } else { if (currentSong->swingAmount == 0) { diff --git a/src/deluge/util/functions.cpp b/src/deluge/util/functions.cpp index 6a6370f883..91587a085b 100644 --- a/src/deluge/util/functions.cpp +++ b/src/deluge/util/functions.cpp @@ -2005,8 +2005,21 @@ bool shouldAbortLoading() { && (encoders::getEncoder(encoders::EncoderName::SELECT).detentPos || QwertyUI::predictionInterrupted)); } +int32_t getNoteMagnitudeFfromNoteLength(uint32_t noteLength, int32_t tickMagnitude) { + // Given a note length and a tick magnitude, figure out the _note magnitude_ + int32_t noteMagnitude = -5 - tickMagnitude; + uint32_t level = 3; + + while (level < noteLength) { + noteMagnitude++; + level <<= 1; + } + return noteMagnitude; +} + void getNoteLengthNameFromMagnitude(StringBuf& noteLengthBuf, int32_t magnitude, char const* const notesString, bool clarifyPerColumn) { + // Positive magnitudes are bars, negative magnitudes are divisions of bars. uint32_t division = (uint32_t)1 << (0 - magnitude); if (display->haveOLED()) { diff --git a/src/deluge/util/functions.h b/src/deluge/util/functions.h index 4eae55ce68..26e623682a 100644 --- a/src/deluge/util/functions.h +++ b/src/deluge/util/functions.h @@ -438,6 +438,7 @@ int32_t getHowManyCharsAreTheSame(char const* a, char const* b); void dimColour(uint8_t colour[3]); bool charCaseEqual(char firstChar, char secondChar); bool shouldAbortLoading(); +int32_t getNoteMagnitudeFfromNoteLength(uint32_t noteLength, int32_t tickMagnitude); /// buffer must have at least 5 characters on 7seg, or 30 for OLED void getNoteLengthNameFromMagnitude(StringBuf& buf, int32_t magnitude, char const* durrationSuffix = "-notes", bool clarifyPerColumn = false); diff --git a/tests/unit/mocks/function_mocks.h b/tests/unit/mocks/function_mocks.h new file mode 100644 index 0000000000..a3fa27449f --- /dev/null +++ b/tests/unit/mocks/function_mocks.h @@ -0,0 +1,10 @@ +#pragma once + +// TODO: Instead of this, make the real functions accessible (functions.h pulls in too much) +#include "util/d_string.h" + +#include + +int32_t getNoteMagnitudeFfromNoteLength(uint32_t noteLength, int32_t tickMagnitude) { return 1; } +void getNoteLengthNameFromMagnitude(StringBuf& buf, int32_t magnitude, char const* durrationSuffix = "-notes", + bool clarifyPerColumn = false) { ; }