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.
SynthstromAudible · Jul 16, 2024 · 5e7da72 · 5e7da72
1 parent 39fdcda
commit 5e7da72
Show file tree

Hide file tree

Showing 11 changed files with 73 additions and 25 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -5,16 +5,30 @@
 #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) {
 	// This is tricky because we don't want zero: that would be "off".
 	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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 <cstdint>
+
+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) { ; }