View Issue Details

IDProjectCategoryView StatusLast Update
0001248OpenMPT[All Projects] Feature Requestpublic2019-09-09 11:10
Reporterdem1 Assigned To 
PrioritynormalSeverityfeatureReproducibilityN/A
Status newResolutionopen 
Product Version 
Target VersionFixed in Version 
Summary0001248: Relative Chords with notes below the root
Description

Hello,

Currently, when using the Chord Editor with "Relative" base mode, you can only choose chords with notes above the root. I would like to have the option to include notes up to one octave below the root.

Thank you,
Derren

TagsNo tags attached.
Has the bug occurred in previous versions?
Tested code revision (in case you know it)

Activities

Saga Musix

Saga Musix

2019-08-26 19:00

administrator   ~0004028

Definitely an interesting idea but sadly not entirely trivial to implement with the current chord-related code. In particular the visual chord editor will get tricky to use - so far it constructs chords simply from the bottom note to the top note. But what if you click on a note below the current root note - will it become the new root note, or will it become an additional note below the root?

dem1

dem1

2019-08-30 20:29

reporter   ~0004033

I'd think in that case it should be an additional note below the root - I think it makes sense to have the root note permanently set to C in "Relative" mode, like it is now. I would just prefer that it be the C one octave higher, to allow inversions. That would be consistent with how it works in Absolute mode, I think, since the only way to change the root then is to change the base key.
Are you saying that this would conflict with building chords from the bottom up?
Is all of the relevant chord-related code in PatternEditorDialogs?

Saga Musix

Saga Musix

2019-08-30 21:11

administrator   ~0004034

Are you saying that this would conflict with building chords from the bottom up?

I'm just not sure if the UI would still work intuitively or how much work it would be to make it intuitive.

Is all of the relevant chord-related code in PatternEditorDialogs?

The chord editor, yes, but there is code outside of that which needs to be touched as well (chord keys are unsigned integers, etc.)

dem1

dem1

2019-09-09 01:29

reporter   ~0004051

Here's what I'm thinking so far - is this a good approach?

  • Make the keyboard three octaves long. In CChordEditor::OnInitDialog(), change m_Keyboard.Init(m_hWnd, 2) to m_Keyboard.Init(m_hWnd, 3). Change the for loop to run from inotes=-13 to <24; change the inotes < 0 condition to inotes <= -13.
  • Indicate visually which note is the root note - maybe write "X" inside the root note's reddot, or 1, 2, and 3 in the other notes' reddot, or draw the root note with a different coloured dot. I have no idea how much work it took to make the sample map editor do this. Hopefully this can be done by changing only CChordEditor.
  • To this end, change the length of the for loop in CChordEditor::UpdateKeyboard to 3 * 12. Change note = 0 to note = 12. Below that, change if(i == note) b = CKeyboardControl::KEYFLAG_REDDOT; to KEYFLAG_BRIGHTDOT, and rearrange the if... else if so that the base note is checked last (if a user clicks the C- key while the editor is in relative mode there should be a red dot, otherwise it should be the root note colour.)
  • Leave the chord keys unsigned, so that a 0 indicates a note one octave below the root in relative mode, and the C one octave below the current note in absolute mode. Change the default chords starting on line 386 of TrackerSettings::TrackerSettings(conf) to be 12 higher (5, 8, 11 become 17, 20, 23.)
  • In CViewPattern::ConstructChord(), subtract 12 in the definition of chordNote. Check for underflows inside the for loop - if a special note were intended, like when TempStopNote calls this function, it would get caught before the loop. If there is an underflow, do not include the note.
  • Change the length of the for loop in CChordEditor::OnKeyboardNotify to 3 * 12.

Note that I haven't been able to make or test any of these changes yet. Do I need to pay for Visual Studio in order to compile OpenMPT?

StarWolf3000

StarWolf3000

2019-09-09 06:23

reporter   ~0004053

There's a Community Edition since version 2015, so just download and install that one. It is not recommended to install the Express editions.

Saga Musix

Saga Musix

2019-09-09 10:44

administrator   ~0004054

Visual Studio 2017 or 2019 Community edition will do, but you need to make sure that you install MFC (it's not checked by default in the installer).

Some parts of your approach are a bit flawed, in the sense that they will break already saved chords (the whole adding / subtracting 12 thing), this is why I wanted to actually have negative offsets for chords in the MPTChord struct. If you are interested, I can send you what I have done so far, which is basically everything except for the chord editor. Then you could take on from there.

Saga Musix

Saga Musix

2019-09-09 11:10

administrator   ~0004055

Here is my current progress, which should leave all the remaining work to be in the chord editor. I have chosen a -2/+2 octave range for symmetry reasons.



RelativeChords.patch (5,016 bytes)
Index: mptrack/PatternEditorDialogs.cpp
===================================================================
--- mptrack/PatternEditorDialogs.cpp	(revision 12033)
+++ mptrack/PatternEditorDialogs.cpp	(working copy)
@@ -880,7 +880,7 @@
 	CMainFrame *pMainFrm;
 
 	CDialog::OnInitDialog();
-	m_Keyboard.Init(m_hWnd, 2);
+	m_Keyboard.Init(m_hWnd, 4);
 	pMainFrm = CMainFrame::GetMainFrame();
 	if (!pMainFrm) return TRUE;
 	// Fills the shortcut key combo box
@@ -893,20 +893,20 @@
 
 	// Minor notes
 	CString s;
-	for (int inotes=-1; inotes<24; inotes++)
+	for (int inotes = -25; inotes < 24; inotes++)
 	{
-		if(inotes < 0)
+		if(inotes == -25)
 		{
 			s = _T("--");
 		} else
 		{
 			s = mpt::ToCString(CSoundFile::GetDefaultNoteName(inotes % 12));
-			if(inotes >= 12)
-				s.AppendFormat(_T(" (+%d)"), inotes / 12);
+			if(inotes < 0 || inotes >= 12)
+				s.AppendFormat(_T(" (%c%d)"), inotes < 0 ? '-' : '+', inotes / 12);
 		}
-		m_CbnNote1.AddString(s);
-		m_CbnNote2.AddString(s);
-		m_CbnNote3.AddString(s);
+		m_CbnNote1.SetItemData(m_CbnNote1.AddString(s), inotes);
+		m_CbnNote1.SetItemData(m_CbnNote2.AddString(s), inotes);
+		m_CbnNote1.SetItemData(m_CbnNote3.AddString(s), inotes);
 	}
 	// Update Dialog
 	OnChordChanged();
@@ -932,11 +932,11 @@
 	chord.notes[0] = NOTE_NONE;
 	chord.notes[1] = NOTE_NONE;
 	chord.notes[2] = NOTE_NONE;
-	for(UINT i = 0; i < 2 * 12; i++)
+	for(UINT i = 0; i < 4 * 12; i++)
 	{
 		if(chord.key == MPTChord::relativeMode)
 		{
-			if(!i) continue;
+			if(i == 2 * 12) continue;
 		} else
 		{
 			if(i == chord.key % 12u) continue;
@@ -949,7 +949,7 @@
 			if ((cnote < 3) || (i == (UINT)nKey))
 			{
 				UINT k = (cnote < 3) ? cnote : 2;
-				chord.notes[k] = static_cast<BYTE>(i+1);
+				chord.notes[k] = static_cast<int8>(i + 1);
 				if (cnote < 3) cnote++;
 			}
 		}
Index: mptrack/TrackerSettings.cpp
===================================================================
--- mptrack/TrackerSettings.cpp	(revision 12033)
+++ mptrack/TrackerSettings.cpp	(working copy)
@@ -393,15 +393,15 @@
 		if(ichord < 12)
 		{
 			// Major Chords
-			Chords[ichord].notes[0] = (uint8)(ichord+5);
-			Chords[ichord].notes[1] = (uint8)(ichord+8);
-			Chords[ichord].notes[2] = (uint8)(ichord+11);
+			Chords[ichord].notes[0] = (int8)(ichord+5);
+			Chords[ichord].notes[1] = (int8)(ichord+8);
+			Chords[ichord].notes[2] = (int8)(ichord+11);
 		} else if(ichord < 24)
 		{
 			// Minor Chords
-			Chords[ichord].notes[0] = (uint8)(ichord-8);
-			Chords[ichord].notes[1] = (uint8)(ichord-4);
-			Chords[ichord].notes[2] = (uint8)(ichord-1);
+			Chords[ichord].notes[0] = (int8)(ichord-8);
+			Chords[ichord].notes[1] = (int8)(ichord-4);
+			Chords[ichord].notes[2] = (int8)(ichord-1);
 		}
 	}
 
@@ -1238,9 +1238,9 @@
 			if((chord & 0xFFFFFFC0) || (!chords[i].notes[0]))
 			{
 				chords[i].key = (uint8)(chord & 0x3F);
-				chords[i].notes[0] = (uint8)((chord >> 6) & 0x3F);
-				chords[i].notes[1] = (uint8)((chord >> 12) & 0x3F);
-				chords[i].notes[2] = (uint8)((chord >> 18) & 0x3F);
+				chords[i].notes[0] = static_cast<int8>(((chord >> 6) & 0x3F) | (0xC0 * ((chord >> 11) & 1)));
+				chords[i].notes[1] = static_cast<int8>(((chord >> 12) & 0x3F) | (0xC0 * ((chord >> 17) & 1)));
+				chords[i].notes[2] = static_cast<int8>(((chord >> 18) & 0x3F) | (0xC0 * ((chord >> 23) & 1)));
 			}
 		}
 	}
@@ -1251,7 +1251,7 @@
 {
 	for(std::size_t i = 0; i < std::size(chords); i++)
 	{
-		int32 s = (chords[i].key) | (chords[i].notes[0] << 6) | (chords[i].notes[1] << 12) | (chords[i].notes[2] << 18);
+		int32 s = (chords[i].key) | ((chords[i].notes[0] & 0x3F) << 6) | ((chords[i].notes[1] & 0x3F) << 12) | ((chords[i].notes[2] & 0x3F) << 18);
 		mpt::ustring note = mpt::format(U_("%1%2"))(mpt::ustring(NoteNamesSharp[i % 12]), i / 12);
 		conf.Write<int32>(U_("Chords"), note, s);
 	}
Index: mptrack/TrackerSettings.h
===================================================================
--- mptrack/TrackerSettings.h	(revision 12033)
+++ mptrack/TrackerSettings.h	(working copy)
@@ -193,7 +193,7 @@
 	};
 
 	uint8 key;			// Base note
-	uint8 notes[3];		// Additional chord notes
+	int8 notes[3];		// Additional chord notes
 };
 
 using MPTChords = std::array<MPTChord, 3 * 12>;	// 3 octaves
Index: mptrack/View_pat.cpp
===================================================================
--- mptrack/View_pat.cpp	(revision 12033)
+++ mptrack/View_pat.cpp	(working copy)
@@ -5195,7 +5195,7 @@
 	{
 		if(cnote)
 		{
-			ModCommand::NOTE chordNote = key - NOTE_MIN;
+			int32 chordNote = key - NOTE_MIN;
 			if(!relativeMode)
 			{
 				// Only use octave information from the base key
@@ -5202,9 +5202,9 @@
 				chordNote = (chordNote / 12) * 12;
 			}
 			chordNote += cnote;
-			if(specs.HasNote(chordNote))
+			if(chordNote >= NOTE_MIN && chordNote <= NOTE_MAX && specs.HasNote(static_cast<ModCommand::NOTE>(chordNote)))
 			{
-				outNotes[numNotes++] = chordNote;
+				outNotes[numNotes++] = static_cast<ModCommand::NOTE>(chordNote);
 			}
 		}
 	}
RelativeChords.patch (5,016 bytes)

Issue History

Date Modified Username Field Change
2019-08-11 18:39 dem1 New Issue
2019-08-26 19:00 Saga Musix Note Added: 0004028
2019-08-30 20:29 dem1 Note Added: 0004033
2019-08-30 21:11 Saga Musix Note Added: 0004034
2019-09-09 01:29 dem1 Note Added: 0004051
2019-09-09 06:23 StarWolf3000 Note Added: 0004053
2019-09-09 10:44 Saga Musix Note Added: 0004054
2019-09-09 11:10 Saga Musix File Added: RelativeChords.patch
2019-09-09 11:10 Saga Musix Note Added: 0004055