Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scale-Aware-Accidentals #3243

Open
wants to merge 38 commits into
base: community
Choose a base branch
from

Conversation

todd-gochenour
Copy link
Contributor

@todd-gochenour todd-gochenour commented Jan 8, 2025

  • Key scales can display either sharps or flats. The major keys of G, D, A, E, B and F# all display sharps and C, F, Db, Ab, Eb and Bb all display flats. Scale modes and exotic minor scales will use the accidental of its relative major key. Accidentals only apply to black keys on a piano. Db, Eb, F#, Ab and Bb are the five black keys with their accidentals. In the key of E the third degree will display G#. In the key of Eb the fourth degree will display Ab. The are enharmonically equivalent.
  • With the 7SEG display, where before a sharp was represented with the decimal point now a sharp is represented using the upper left vertical segment and a flat is represented using the lower left vertical segment.
  • A refactor of the function noteCodeToString() will consolidate logic and lookup tables into preset_scales.cpp where it can utilize scale information to determine if the relative major key of the current song is sharp or flat. The function signature takes parameters currentSong->key.rootNote and currentSong->getCurrentScale() when making this decision.
  • Areas affected by this change include InstrumentClipMinder, AutomationView, ChordLibrary, ChordKeyboard, KeyboardScreen, MultiRange and KeyRange.
  • Notes are shown as a popup when pressing the audition pad of a synth instrument clip and in the KeyboardScreen when pressing any note pad. Press a pad within the ChordKeyboard and ChordLibrary views and the chord name will popup with the appropriate accidental based on key. Notes are shown in the OLED screen layout when scrolling vertically in AutomationView>Velocity and when scrolling horizontally defining a default key range in Settings>Defaults>Key. Create a multisample synth and the SYNTH-BROWSE shortcut will show a Note Range list for each sample loaded.

Copy link
Contributor

github-actions bot commented Jan 8, 2025

Test Results

106 tests  ±0   106 ✅ ±0   0s ⏱️ ±0s
 16 suites ±0     0 💤 ±0 
 16 files   ±0     0 ❌ ±0 

Results for commit 8dc1d6e. ± Comparison against base commit 0d79ad6.

♻️ This comment has been updated with latest results.

@todd-gochenour
Copy link
Contributor Author

Test Script:

  1. On startup with synth Rich Saw Bass and a default key in C and scale is Major. Note that the root note is 0 with a new song and not 60 (C3). C3 and C4 are top and bottom audition pads lit to show they are the root.
  2. Press SCALE quickly and now the notes are chromatic. Press the note above C3 and it will read D&3. '&' is used because there isn't a flat character available.
  3. Press SCALE quickly to return to diatonic C major. Step up to the 3rd pad above C3, the F note, and press SCALE-PAD to change key to F Major.
  4. Step up to the 3rd pad above F, notice that it reads B&3, and press SCALE-PAD to change to the key of Bb. Step up to the 3rd above that and SCALE-PAD the key of Eb.
  5. Step down one pad from Eb and SCALE-PAD the key of D. Step up to the 3rd pad above D, notice that it reads F#3, and press SCALE-PAD to change the key to F#. The key of F# has the most accidentals of any key at 6. Step down one pad and it shows F3 instead of E#3, enharmonically correct but not technically accurate.
  6. Step down 2 steps down from F# to D# and SCALE-PAD to change the key to Eb. Key signatures will always favor the the fewest accidentals, so Eb with three flats is favored over D# with 9 sharps.

@mweigt
Copy link
Contributor

mweigt commented Jan 8, 2025

note above C3 and it will read D&3. '&' is used because there isn't a flat character available.

Why not use "b", since that is very much what the flat sign looks like, e.g. "Db3"?

@todd-gochenour
Copy link
Contributor Author

Test runthrough video: https://www.youtube.com/watch?v=fTYfCT8OS74

@todd-gochenour
Copy link
Contributor Author

There are no lower case letters on the Deluge. Only upper case. '&' is the closest shape and is temporary until a new glyph is generated.

@m-m-adams
Copy link
Collaborator

m-m-adams commented Jan 9, 2025

Oh man this has been one of my most wanted deluge features forever. Does it change the song scale or is the accidental ignored for scale determination?

Nevermind, this is smarter naming of notes in the scale/chromatic mode and not accidental support

@todd-gochenour
Copy link
Contributor Author

todd-gochenour commented Jan 9, 2025

It was a surprise to me when I first picked D# (from key of E) as the new key and it popped up 'E& Major' for a second and then every altered note (black key) was displayed in flats. I picked A# (from the key of B) for the new key and it popped up 'B& Major'. When I went chromatic and picked G& it changed to F#. I got this key signature behavior for free.

@todd-gochenour
Copy link
Contributor Author

Isn't it a convention for parameter changes to first show the current value before changing to the next? SHIFT-SCALE is not doing this.

@seangoodvibes
Copy link
Collaborator

Isn't it a convention for parameter changes to first show the current value before changing to the next? SHIFT-SCALE is not doing this.

It depends. It's applied on some things but not all things (eg note probability, iteration shows current value before changing it).

@todd-gochenour
Copy link
Contributor Author

Can I apply it to SHIFT-SCALE ?

@seangoodvibes
Copy link
Collaborator

Can I apply it to SHIFT-SCALE ?

Sure, we can see how it feels

@todd-gochenour
Copy link
Contributor Author

todd-gochenour commented Jan 10, 2025

Tested C Lydian for augmented 4th F# in relative major of G.
Tested C Mixolydian for 7 degree of B& in relative major of F.
Tested C Locrian for everything flat except C and F in relative major of D&.
Tested C Melodic Minor for flat 3rd of E& treated like a dorian minor
Tested C Harmonic Minor for flat 3rd, flat 6th of A& and natural 7th of B.
Test Failed C Hungarian Minor for flat 3rd, sharp 4 of G& (should be F#), flat 6th and natural 7th.
Test Failed C Marva treated as a major with flat 2, G& (should be F#). Ditto.
Tested C Arabian treated as a major with flat 5, flat 6 and flat 7
Tested C Whole Tone treated as a major with flat 5, flat 6 and flat 7
Tested C Blues with E&, G& and B&
Tested C Pentatonic Minor with E& and B&.
Tested C Hirajosh with E& and A&.

Not sure yet how to handle the mixture of sharps and flats in the Hungarian Minor and Marva scales. There are also the cases E#, F&, B# and C& which show the natural enharmonic note because accidentals in this code only apply to the black keys of a piano.

@todd-gochenour
Copy link
Contributor Author

Tested D& Major with B, C#, D#, E, F#, G#, A#
Tested D& Minor B, C#, D, E, F#, G, A
Test Failed B Lydian for augmented 4th F (should be E#).
Tested D& Mixolydian for 7 degree of A in relative major of E.
Tested D& Locrian with no sharps or flats.
Tested D& Melodic Minor B, C#, D, E, F#, G#, A#
Tested D& Harmonic Minor B, C#, D, E, F#, G, A#
Test Failed B Hungarian Minor B, C#, D, F (Should be E#), F#, G, A#
Tested D& Marva B, C, D#, F, F#, G#, A#
Tested D& Arabian B, C#, D#, E, F, G, A
Tested D& Whole Tone B, C#, D#, F, G, A
Tested D& Blues B, D, E, F, F#, A (F is added flat 5 blue note)
Tested D& Pentatonic Minor B, D, E, F#, A
Tested D& Hirajosh B, C#, D, F#, G

The Deluge strategy did not handle displaying E# before this change either.

Tested D& Major with D&, E&, F, G&, A&, B&, C
Tested D& Minor Became C# minor with C#, D#, E, F#, G#, A, B with relative major of F#.
Tested D& Dorian Became C# minor with relative major of B
Tested D& Lydian D&, E&, F, G, A&, B&, C with relative major of A&.
Tested D& Mixolydian became C# minor D#, D#, F, F#, G#, A#, B
Tested D& Locrian C#, D, E, F#, G, A, B
Test failed D& Melodic Minor C#, D#, E, F#, G#, A#, C (should be B#)
Test failed D& Harmonic Minor C#, D#, E, F#, G#, A, C (should be B#)
Test failed D& Hungarian Minor C#, D#, E, G(as augmented 4 is F##?), G#, A, C (should be B#)
Tested D& Marva D&, D (maybe E&&?), F, G, A&, B&, C
Tested D& Arabian D&, E&, F, G&, G, A, B
Tested D& Whole Tone D&, E&, F, G, A, B
Tested D& Blues C#, E, F#, G, G#, B
Tested D& Pentatonic Minor C#, E, F#, G#, B
Tested D& Hirajosh C#, D#, E, G#, A

The Deluge strategy did not handle displaying B# before this change either. There are cases where double accidentals like F## or E&& may be technically correct, but the enharmonic equivalent of G and D are easier to comprehend. There will be cases where a scale contains two of the same note letter, one with an accidental and one without.

before cycling scales on subsequent presses.
@todd-gochenour
Copy link
Contributor Author

Added logic so that SHIFT-SCALE shows current scale on first press before cycling scales on subsequent presses. This logic applied to instrument clip view and to keyboard screen. Release of the SHIFT key resets the test for first press.

@seangoodvibes
Copy link
Collaborator

Added logic so that SHIFT-SCALE shows current scale on first press before cycling scales on subsequent presses. This logic applied to instrument clip view and to keyboard screen. Release of the SHIFT key resets the test for first press.

Instead of creating a new function "shift has changed" I think you should leverage the logic that is used for other "previews" which basically shows a pop up of a "type"

And then before changing scale you check if the popup of that type is present. If yes, change scale.

Look for the code "display->hasPopupOfType"

For examples of this behaviour

@seangoodvibes
Copy link
Collaborator

renderAutomationEditorDisplayOLED() doesn't print a note. My bad. The second reference is renderNoteEditorDisplay7SEG().

Open question about how to distinquish flat from sharp on 7SEG. Perhaps an '_' for flat and '.' for sharp?

Open question about how to create the flat glyph, also.

To create new fonts, check fonts.c

With that file open if you do a control + f and search for "1" and then turn your head sideways you'll see how the fonts are created.

Re 7seg im not the best person for that, id ask @m-m-adams what they think as they have a 7seg

image

@todd-gochenour
Copy link
Contributor Author

todd-gochenour commented Jan 12, 2025

In implementing the flat glyph, I first tried adding 'b' one step after '~' in the font description table and wrote special handling in Canvas::getCharIndex() to deal with this character placed out of ASCII order. Unfortunately, there are synth names that use lowercase letters like 'Double' and these b characters aren't becoming uppercase.

I have to use some ascii character to represent flat. Tilde, backtick, dollar sign are all options. I'm already using '&' as it looks the closest to 'b' of all the various options. I didn't find the use of '&' in any file names. The simple solution is to change the '&' glyph to look like a 'b'.

'#' and '&' represent sharp and flat. I changed the chord names from flat5 to &5 and flat9 to &9.

Character \x81 represents the flat glpyh.

@sapphire-arches
Copy link
Collaborator

You could add it in the "high bit" ASCII range (i.e. chars >127) -- these should never appear in filenames that we support today. You'd need to add logic in Canvas::getCharIndex to handle it.

If you want to test 7seg behavior on an OLED, you can press SHIFT+LEARN+AFFECT ENTIRE with 7seg emulation mode set to Toggle (see community_features.md, search for "Emulated Display").

* 'FLAT' found in chordName is converted to char 129
for display.
@todd-gochenour
Copy link
Contributor Author

todd-gochenour commented Jan 12, 2025

Flat glyph now represented by uint8_t value of 129. Glyph added after tilde in fonts.cpp and index will convert char 129 to ('~' + 1). The chord names restored to 'FLAT5' and 'FLAT9'. I couldn't figure a way to add char 129 to a const char *. The code \x81 is the hex number representation and so the chord name had a string with \815 where the 5 is literal but compiler did not like it. Instead, logic in KeyboardLayoutChord::drawChordName() will detect 'FLAT' in chord name and convert it to char 129 before display. Chord name will read Ab-7b5b9.

In testing this on 7SEG I'm seeing unusual behavior in the Chord Library where sometimes it reads 'Cant', other times it says 'Soon' and other times it acts like a shortcut to a parameter modifier with the pad flashing.

and flat as lower left vertical segment.
@todd-gochenour
Copy link
Contributor Author

7SEG Displays sharp as upper left vertical segment (segment 2) and flat as lower left vertical segment (segment 3). They appear as ticks next to the note name. The decimal dot is no longer used to signify an accidental.

@jkschwartz
Copy link

Do we have any performance testing tools to see if adding this calculation to each key press has a performance impact? 1.2 already has some performance issues on the isometric keyboard view: #3168

@sapphire-arches
Copy link
Collaborator

Do we have any performance testing tools to see if adding this calculation to each key press has a performance impact? 1.2 already has some performance issues on the isometric keyboard view: #3168

The calculations added here are near trivial, I doubt they will have performance impact. We don't have great tools for that though, if you have ideas please join us on Discord or leave something in the discussions tab.

#3168 doesn't look like a performance bug, it seems like something is going wrong in the keyboard state reconciliation logic.

Test Settings>Defaults>Key
* rootNote set to given noteCode when not provided as parameter
* Tested MultiRange menu option
* Adjusted KeyRange highlight of right side to line up with note display
*
@todd-gochenour
Copy link
Contributor Author

todd-gochenour commented Jan 14, 2025

  • Tested MultiRange menu option. Procedure is: New Song, New Synth, SHIFT-BROWSE, locate folder with multisamples, press SELECT and hold to see LOAD FILES(S) menu and press SELECT again to choose MULTISAMPLES. Press SHIFT-BROWSE again and MultiRange menu appears. Db, Eb, F#, Ab and Bb appear in ranges.
  • Removed &isNatural parameter from noteCodeToString() as it is obsolete as 7SEG doesn't showDot for sharps anymore, and MultiRange will instead calculate left/right string length using strlen().
  • When rootNote is omitted from call noteCodeToString(), it will use noteCode as the root. This allows simple calls to noteCodeToString(note, buffer)
  • Adjusted KeyRange highlight position for the right side of the range (Default Key: C-F#) to line up correctly with note display (F#). The dash is half the width of a digit. Use horizontal encoder to shift highlight to the right.

@todd-gochenour
Copy link
Contributor Author

My development and testing are complete. This PR is ready for review.

@jkschwartz
Copy link

I started testing this change tonight. So far it is working great both with new and old songs. One item / observation- some scales are not possible with this change, right? If someone wanted put something together in D#, they would just use Eb?

@todd-gochenour
Copy link
Contributor Author

todd-gochenour commented Jan 17, 2025

If someone wanted put something together in D#, they would just use Eb?

Yes, as both keys are enharmonically equivalent. Eb has 3 flats, Bb, Eb and Ab. D# has 9 sharps, and as there are only 7 notes in a scale, 2 of these sharps are double sharps, D#, E#, F##, G#, A#, B#, C##. The scale with the least number of accidentals is favored.

If you pick D# as your new key, the display converts to Eb automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants