further simplification of YNote-stuff (SynthstromAudible#2360)

* extract Song::getYNoteWithinOctaveFromYNote() as MusicalKey::intervalOf()

- it returns the semitone offset from root below the note, so mixing
  it up with representation on the device doesn't seem necessary

* better NoteSet::operator[]

- return -1 for missing notes, don't assume root's existence

* MusicalKey::degreeOf() and NoteSet::degreeOf()

- allows simplifying InstrumentClip::replaceMusicalMode()

- which in turn makes Song::yNoteIsYVisualWithinOctave() dead code

src/deluge/io/midi/midi_transpose.cpp

@@ -33,16 +33,16 @@ void doTranspose(bool on, int32_t newNoteOrCC) {
 
 		if (controlMethod == MIDITransposeControlMethod::INKEY) {
 
-			uint8_t indexInMode = currentSong->getYNoteIndexInMode(newNoteOrCC);
-			if (indexInMode < 255) {
+			int8_t degree = currentSong->key.degreeOf(newNoteOrCC);
+			if (degree >= 0) {
 				int8_t octaves;
 				if (semitones < 0) {
 					octaves = ((semitones + 1) / 12) - 1;
 				}
 				else {
 					octaves = (semitones / 12);
 				}
-				int32_t steps = octaves * currentSong->key.modeNotes.count() + indexInMode;
+				int32_t steps = octaves * currentSong->key.modeNotes.count() + degree;
 
 				currentSong->transposeAllScaleModeClips(steps, false);
 

src/deluge/model/clip/instrument_clip.cpp

@@ -38,6 +38,7 @@
 #include "model/note/note.h"
 #include "model/scale/note_set.h"
 #include "model/scale/preset_scales.h"
+#include "model/scale/utils.h"
 #include "model/song/song.h"
 #include "modulation/midi/midi_param.h"
 #include "modulation/midi/midi_param_collection.h"
@@ -1247,19 +1248,20 @@ void InstrumentClip::replaceMusicalMode(uint8_t numModeNotes, int8_t changes[12]
 	if (!isScaleModeClip()) {
 		return;
 	}
-	// Find all NoteRows which belong to this yVisualWithinOctave, and change their note
+	// Find all NoteRows which belong to this scale, and change their note
+	//
+	// TODO: There probably should not be _any_ rows which don't below to the
+	// current scale? FREEZE_WITH_ERROR?
+	MusicalKey key = modelStack->song->key;
 	for (int32_t i = 0; i < noteRows.getNumElements(); i++) {
 		NoteRow* thisNoteRow = noteRows.getElement(i);
-		for (int32_t yVisualWithinOctave = 0; yVisualWithinOctave < numModeNotes; yVisualWithinOctave++) {
-			if (modelStack->song->yNoteIsYVisualWithinOctave(thisNoteRow->y, yVisualWithinOctave)) {
-				ModelStackWithNoteRow* modelStackWithNoteRow =
-				    modelStack->addNoteRow(getNoteRowId(thisNoteRow, i), thisNoteRow);
+		int8_t degree = key.degreeOf(thisNoteRow->y);
+		if (degree >= 0) {
+			ModelStackWithNoteRow* modelStackWithNoteRow =
+			    modelStack->addNoteRow(getNoteRowId(thisNoteRow, i), thisNoteRow);
 
-				thisNoteRow->stopCurrentlyPlayingNote(
-				    modelStackWithNoteRow); // Otherwise we'd leave a MIDI note playing
-				thisNoteRow->y += changes[yVisualWithinOctave];
-				break;
-			}
+			thisNoteRow->stopCurrentlyPlayingNote(modelStackWithNoteRow); // Otherwise we'd leave a MIDI note playing
+			thisNoteRow->y += changes[degree];
 		}
 	}
 }
@@ -1290,7 +1292,7 @@ void InstrumentClip::seeWhatNotesWithinOctaveArePresent(NoteSet& notesWithinOcta
 		NoteRow* thisNoteRow = noteRows.getElement(i);
 
 		if (!thisNoteRow->hasNoNotes()) {
-			notesWithinOctavePresent.add(song->getYNoteWithinOctaveFromYNote(thisNoteRow->getNoteCode()));
+			notesWithinOctavePresent.add(song->key.intervalOf(thisNoteRow->getNoteCode()));
 			i++;
 		}
 
@@ -1370,12 +1372,13 @@ void InstrumentClip::nudgeNotesVertically(int32_t direction, VerticalNudgeType t
 		// wanting to change less than an octave
 		else {
 			for (int32_t i = 0; i < noteRows.getNumElements(); i++) {
+				MusicalKey key = modelStack->song->key;
 				NoteRow* thisNoteRow = noteRows.getElement(i);
 				int32_t changeInSemitones = 0;
-				int32_t yNoteWithinOctave = modelStack->song->getYNoteWithinOctaveFromYNote(thisNoteRow->getNoteCode());
+				int32_t yNoteWithinOctave = key.intervalOf(thisNoteRow->getNoteCode());
 				int32_t oldModeNoteIndex = 0;
-				for (; oldModeNoteIndex < modelStack->song->key.modeNotes.count(); oldModeNoteIndex++) {
-					if (modelStack->song->key.modeNotes[oldModeNoteIndex] == yNoteWithinOctave) {
+				for (; oldModeNoteIndex < key.modeNotes.count(); oldModeNoteIndex++) {
+					if (key.modeNotes[oldModeNoteIndex] == yNoteWithinOctave) {
 						break;
 					}
 				}
@@ -3184,14 +3187,15 @@ void InstrumentClip::prepNoteRowsForExitingKitMode(Song* song) {
 
 	NoteRow* chosenNoteRow = NULL;
 	int32_t chosenNoteRowIndex;
+	MusicalKey key = song->key;
 
 	// If we're in scale mode...
 	if (inScaleMode) {
 
 		// See if any NoteRows are a root note
 		for (int32_t i = 0; i < noteRows.getNumElements(); i++) {
 			NoteRow* thisNoteRow = noteRows.getElement(i);
-			if (thisNoteRow->y != -32768 && song->getYNoteWithinOctaveFromYNote(thisNoteRow->y) == 0) {
+			if (thisNoteRow->y != -32768 && key.intervalOf(thisNoteRow->y) == 0) {
 				chosenNoteRow = thisNoteRow;
 				chosenNoteRowIndex = i;
 				break;
@@ -3208,7 +3212,7 @@ void InstrumentClip::prepNoteRowsForExitingKitMode(Song* song) {
 
 				// But, if we're in key-mode, make sure this yNote fits within the scale!
 				if (inScaleMode) {
-					uint8_t yNoteWithinOctave = song->getYNoteWithinOctaveFromYNote(thisNoteRow->y);
+					uint8_t yNoteWithinOctave = key.intervalOf(thisNoteRow->y);
 
 					// Make sure this yNote fits the scale/mode
 					if (!song->key.modeNotes.has(yNoteWithinOctave)) {

src/deluge/model/scale/musical_key.cpp

@@ -1,4 +1,6 @@
 #include "model/scale/musical_key.h"
+#include "util/const_functions.h"
+
 #include <cstdint>
 
 MusicalKey::MusicalKey() {
@@ -10,3 +12,11 @@ void MusicalKey::applyChanges(int8_t changes[12]) {
 	modeNotes.applyChanges(changes);
 	rootNote += changes[0];
 }
+
+uint8_t MusicalKey::intervalOf(int32_t noteCode) const {
+	return mod(noteCode - rootNote, 12);
+}
+
+int8_t MusicalKey::degreeOf(int32_t noteCode) const {
+	return modeNotes.degreeOf(intervalOf(noteCode));
+}

src/deluge/model/scale/musical_key.h

@@ -10,6 +10,12 @@
 class MusicalKey {
 public:
 	MusicalKey();
+	/** Returns semitone offset from root below the noteCode. */
+	uint8_t intervalOf(int32_t noteCode) const;
+	/** Returns degree of noteCode in the scale of they key,
+	 * -1 if the noteCode is note in key.
+	 */
+	int8_t degreeOf(int32_t nodeCode) const;
 	void applyChanges(int8_t changes[12]);
 	// TODO: make these priviate later, and maybe rename modeNotes
 	NoteSet modeNotes;

src/deluge/model/scale/note_set.cpp

@@ -21,16 +21,14 @@ void NoteSet::addUntrusted(uint8_t note) {
 	add(note);
 }
 
-uint8_t NoteSet::operator[](uint8_t index) const {
-	// Shift out the root note bit, ie. scale degree zero. We don't
-	// actually check it it's set, because counting scale degrees
-	// doesn't make sense if the root is not set.
-	uint16_t wip = bits >> 1;
-	uint8_t note = 0;
-	while (index--) {
+int8_t NoteSet::operator[](uint8_t index) const {
+	uint16_t wip = bits;
+	int8_t note = -1;
+	int8_t n = -1;
+	while (index > n++) {
 		if (!wip) {
 			// The desired degree does not exist.
-			return 0;
+			return -1;
 		}
 		// For each subsequent scale degree, find the number
 		// of semitones, increment the note counter
@@ -43,6 +41,19 @@ uint8_t NoteSet::operator[](uint8_t index) const {
 	return note;
 }
 
+int8_t NoteSet::degreeOf(uint8_t note) const {
+	if (has(note)) {
+		// Mask everything before the note
+		uint16_t mask = ~(0xffff << note);
+		// How many notes under mask?
+		uint16_t under = bits & mask;
+		return std::popcount(under);
+	}
+	else {
+		return -1;
+	}
+}
+
 void NoteSet::applyChanges(int8_t changes[12]) {
 	NoteSet newSet;
 	uint8_t n = 1;

src/deluge/model/scale/note_set.h

@@ -33,12 +33,20 @@ class NoteSet {
 	/** Like add(), but ensures note is in range and higher than previous notes.
 	 */
 	void addUntrusted(uint8_t note);
-	/** Return the note at specified scale degree as semitone offset from root.
+	/** Return the index'th note, or -1 if there aren't that many notes present.
+	 *
+	 * If the NoteSet has a 0 and represents a scale, then is a scale degree as
+	 * semitone offset from root.
 	 *
 	 * Ie. if a NoteSet has add(0), add(1), add(4), and optionally higher notes
 	 * added, notesSet[2] will return 4.
 	 */
-	uint8_t operator[](uint8_t index) const;
+	int8_t operator[](uint8_t index) const;
+	/** Returns number of notes lower than the note given, or -1 if the note is not present.
+	 *
+	 * This is the scale degree of the note if the NoteSet represents a scale and has a root.
+	 */
+	int8_t degreeOf(uint8_t note) const;
 	/** Applies changes specified by the array.
 	 *
 	 * Each element of the array describes a semitone offset

src/deluge/model/scale/utils.cpp

@@ -1,4 +1,5 @@
 #include "model/scale/utils.h"
+#include "util/const_functions.h"
 
 bool isSameNote(int16_t a, int16_t b) {
 	return (a - b) % 12 == 0;

src/deluge/model/song/song.cpp

@@ -43,6 +43,7 @@
 #include "model/instrument/midi_instrument.h"
 #include "model/sample/sample_recorder.h"
 #include "model/scale/preset_scales.h"
+#include "model/scale/utils.h"
 #include "model/settings/runtime_feature_settings.h"
 #include "model/song/clip_iterators.h"
 #include "model/voice/voice_sample.h"
@@ -602,27 +603,6 @@ void Song::setRootNote(int32_t newRootNote, InstrumentClip* clipToAvoidAdjusting
 	}
 }
 
-bool Song::yNoteIsYVisualWithinOctave(int32_t yNote, int32_t yVisualWithinOctave) {
-	int32_t yNoteWithinOctave = getYNoteWithinOctaveFromYNote(yNote);
-	return (key.modeNotes[yVisualWithinOctave] == yNoteWithinOctave);
-}
-
-uint8_t Song::getYNoteWithinOctaveFromYNote(int32_t yNote) {
-	uint16_t yNoteRelativeToRoot = yNote - key.rootNote + 132;
-	int32_t yNoteWithinOctave = yNoteRelativeToRoot % 12;
-	return yNoteWithinOctave;
-}
-
-uint8_t Song::getYNoteIndexInMode(int32_t yNote) {
-	uint8_t yNoteWithinOctave = (uint8_t)(yNote - key.rootNote + 132) % 12;
-	for (uint8_t i = 0; i < key.modeNotes.count(); i++) {
-		if (key.modeNotes[i] == yNoteWithinOctave) {
-			return i;
-		}
-	}
-	return 255;
-}
-
 /* Moves the intervals in the current modeNotes by some number of steps
     in a circular way. For example, starting in major
     and going up one step (change == 1) results in Dorian:
@@ -683,7 +663,7 @@ bool Song::isYNoteAllowed(int32_t yNote, bool inKeyMode) {
 	if (!inKeyMode) {
 		return true;
 	}
-	return key.modeNotes.has(getYNoteWithinOctaveFromYNote(yNote));
+	return key.modeNotes.has(key.intervalOf(yNote));
 }
 
 int32_t Song::getYVisualFromYNote(int32_t yNote, bool inKeyMode) {

src/deluge/model/song/song.h

@@ -93,8 +93,6 @@ class Song final : public TimelineCounter {
 	bool anyScaleModeClips();
 	void setRootNote(int32_t newRootNote, InstrumentClip* clipToAvoidAdjustingScrollFor = NULL);
 	void addMajorDependentModeNotes(uint8_t i, bool preferHigher, NoteSet& notesWithinOctavePresent);
-	bool yNoteIsYVisualWithinOctave(int32_t yNote, int32_t yVisualWithinOctave);
-	uint8_t getYNoteWithinOctaveFromYNote(int32_t yNote);
 	void changeMusicalMode(uint8_t yVisualWithinOctave, int8_t change);
 	void rotateMusicalMode(int8_t change);
 	void replaceMusicalMode(int8_t changes[], bool affectMIDITranspose);
@@ -240,7 +238,6 @@ class Song final : public TimelineCounter {
 	Error readFromFile(Deserializer& reader);
 	void writeToFile(StorageManager& bdsm);
 	void loadAllSamples(bool mayActuallyReadFiles = true);
-	uint8_t getYNoteIndexInMode(int32_t yNote);
 	void renderAudio(StereoSample* outputBuffer, int32_t numSamples, int32_t* reverbBuffer,
 	                 int32_t sideChainHitPending);
 	bool isYNoteAllowed(int32_t yNote, bool inKeyMode);

tests/unit/CMakeLists.txt

@@ -40,6 +40,7 @@ file(GLOB_RECURSE deluge_SOURCES
         # For scale tests
         ../../src/deluge/model/scale/musical_key.cpp
         ../../src/deluge/model/scale/note_set.cpp
+        ../../src/deluge/model/scale/preset_scales.cpp
         ../../src/deluge/model/scale/utils.cpp
         ../../src/deluge/model/scale/preset_scales.cpp
         # For clip iterator tests

tests/unit/scale_tests.cpp

@@ -126,6 +126,27 @@ TEST(NoteSetTest, applyChanges) {
 	CHECK_EQUAL(4, a.count());
 }
 
+TEST(NoteSetTest, degreeOfBasic) {
+	NoteSet a;
+	a.add(0);
+	a.add(2);
+	a.add(4);
+	CHECK_EQUAL(0, a.degreeOf(0));
+	CHECK_EQUAL(1, a.degreeOf(2));
+	CHECK_EQUAL(2, a.degreeOf(4));
+}
+
+TEST(NoteSetTest, degreeOfNotAScale) {
+	NoteSet a;
+	a.add(1);
+	a.add(2);
+	a.add(4);
+	CHECK_EQUAL(-1, a.degreeOf(0));
+	CHECK_EQUAL(0, a.degreeOf(1));
+	CHECK_EQUAL(1, a.degreeOf(2));
+	CHECK_EQUAL(2, a.degreeOf(4));
+}
+
 TEST(NoteSetTest, isSubsetOf) {
 	NoteSet a;
 	NoteSet b;
@@ -162,7 +183,7 @@ TEST(NoteSetTest, checkEqualAllowed) {
 TEST(NoteSetTest, subscript1) {
 	NoteSet a;
 	for (int i = 0; i < NoteSet::size; i++) {
-		CHECK_EQUAL(0, a[i]);
+		CHECK_EQUAL(-1, a[i]);
 	}
 }
 
@@ -213,6 +234,14 @@ TEST(NoteSetTest, subscript3) {
 	CHECK_EQUAL(11, a[7]);
 }
 
+TEST(NoteSetTest, subscript4) {
+	NoteSet a;
+	a.add(4);
+	a.add(7);
+	CHECK_EQUAL(4, a[0]);
+	CHECK_EQUAL(7, a[1]);
+}
+
 TEST(NoteSetTest, presetScaleId) {
 	CHECK_EQUAL(MAJOR_SCALE, presetScaleNotes[MAJOR_SCALE].presetScaleId());
 	CHECK_EQUAL(MINOR_SCALE, presetScaleNotes[MINOR_SCALE].presetScaleId());
@@ -310,6 +339,47 @@ TEST(MusicalKeyTest, ctor) {
 	CHECK_EQUAL(true, k.modeNotes.has(0));
 }
 
+TEST(MusicalKeyTest, intervalOf) {
+	MusicalKey key;
+	for (int8_t octave = -10; octave <= 10; octave++) {
+		for (uint8_t note = 0; note < 12; note++) {
+			for (uint8_t root = 0; root < 12; root++) {
+				key.rootNote = root;
+				if (root <= note) {
+					CHECK_EQUAL(note - root, key.intervalOf(note + octave * 12));
+				}
+				else {
+					// Consider: root B==11, note D=2, offset=3
+					CHECK_EQUAL(12 - root + note, key.intervalOf(note + octave * 12));
+				}
+			}
+		}
+	}
+}
+
+TEST(MusicalKeyTest, degreeOf) {
+	MusicalKey key;
+	key.rootNote = 9; // A
+	key.modeNotes = presetScaleNotes[MINOR_SCALE];
+
+	for (int octave = -2; octave <= 2; octave++) {
+		// In key
+		CHECK_EQUAL(0, key.degreeOf(9 + octave * 12));  // A
+		CHECK_EQUAL(1, key.degreeOf(11 + octave * 12)); // B
+		CHECK_EQUAL(2, key.degreeOf(0 + octave * 12));  // C
+		CHECK_EQUAL(3, key.degreeOf(2 + octave * 12));  // D
+		CHECK_EQUAL(4, key.degreeOf(4 + octave * 12));  // E
+		CHECK_EQUAL(5, key.degreeOf(5 + octave * 12));  // F
+		CHECK_EQUAL(6, key.degreeOf(7 + octave * 12));  // G
+		// Out of key
+		CHECK_EQUAL(-1, key.degreeOf(10 + octave * 12)); // A#
+		CHECK_EQUAL(-1, key.degreeOf(1 + octave * 12));  // C#
+		CHECK_EQUAL(-1, key.degreeOf(3 + octave * 12));  // D#
+		CHECK_EQUAL(-1, key.degreeOf(6 + octave * 12));  // F#
+		CHECK_EQUAL(-1, key.degreeOf(8 + octave * 12));  // G#
+	}
+}
+
 TEST_GROUP(UtilTest){};
 
 TEST(UtilTest, isSameNote) {