Add community feature setting to allow for notes to be displayed with either sharps or flats #3203
Conversation
Updating conditionals, working on move to defaults menu
| char accidential = !useFlats ? '#' : 'b'; | ||
|
|
||
|
|
||
| *(buffer++) = !useFlats ? noteCodeToNoteLetter[lower] : noteCodeToNoteLetterFlats[lower]; |
There was a problem hiding this comment.
This appears in a lot of places now. Might be nicer to have getNoteLetterAndAccidental(int32_t node, char& letter, char& accidental) which checks the runtimeFeature and provides them, instead of duplicating the code.
There was a problem hiding this comment.
(There is no meaningful performance impact from an additional function call in display code, and a possible benefit from reduced code size by reducing duplication.)
There was a problem hiding this comment.
Could be made inline if there's a desire for avoiding the stack, but I agree, a function would be more DRY.
There was a problem hiding this comment.
Yeah that makes sense. Where should I put the function since it would be used from multiple files?
| @@ -481,10 +481,15 @@ const int16_t lanczosKernelA16[1025] = { | |||
| 0, }; | |||
|
|
|||
|
|
|||
|
|
|||
| //C C# D D# E F F# G G# A A# B | |||
| const uint8_t noteCodeToNoteLetter[12] = {67, 67, 68, 68, 69, 70, 70, 71, 71, 65, 65, 66}; | |||
There was a problem hiding this comment.
{'C', 'C', 'D', ...} etc instead of raw ascii codes, preferably.
There was a problem hiding this comment.
I am weary to change the existing constant. What would the implications of that change be? Would I need to update the places that use the constant?
| const uint8_t noteCodeToNoteLetter[12] = {67, 67, 68, 68, 69, 70, 70, 71, 71, 65, 65, 66}; | ||
| const bool noteCodeIsSharp[12] = {0, 1, 0, 1, 0, 0, 1, 0, 1, 0, 1, 0}; | ||
|
|
||
| //C Db D Eb E F Gb G Ab A Bb B | ||
| const uint8_t noteCodeToNoteLetterFlats[12] = {67, 68, 68, 69, 69, 70, 71, 71, 65, 65, 66, 66}; |
There was a problem hiding this comment.
{'C', 'C', 'D', ...} etc instead of raw ascii codes, preferably.
| const uint8_t noteCodeToNoteLetter[12] = {67, 67, 68, 68, 69, 70, 70, 71, 71, 65, 65, 66}; | ||
| const bool noteCodeIsSharp[12] = {0, 1, 0, 1, 0, 0, 1, 0, 1, 0, 1, 0}; | ||
|
|
||
| //C Db D Eb E F Gb G Ab A Bb B | ||
| const uint8_t noteCodeToNoteLetterFlats[12] = {67, 68, 68, 69, 69, 70, 71, 71, 65, 65, 66, 66}; | ||
| const bool noteCodeIsFlat[12] = {0, 1, 0, 1, 0, 0, 1, 0, 1, 0, 1, 0}; |
There was a problem hiding this comment.
This is the same table as notecodeIsSharp, can have just one table noteHasAccidental.
There was a problem hiding this comment.
I didn't end up using noteCodeIsFlat anyways, I should delete. I think I'd rather not change the existing constants and will just use noteCodeIsSharp
I have and I think that would be ideal but I want to start with something simpler. This is my first contribution to the deluge community firmware so I don't really know what I'm doing. |
|
I would like to encourage implementation of scale-aware use of accidentals. This should be pretty easy now that you solved the problem of displaying those accidentals in the UI. Rather than a user preference as the switch, we could use any old circle of fifths as a guide for which keys are sharp and which keys are flat. Sharps: D G E A F# B (1 - 6 sharps) The awkward enharmonics like Cb and E# are not an option. The goal is to minimize the number of accidentals in a given key. I would be happy to assist you in writing the function to accomplish this enhancement. I too am new to Deluge community development and so this would be a great learning opportunity. |
|
|
Hi Todd A scale aware version of this would be nice but I don't believe it will be that easy. We would need to keep track of all the different scale types for each key. As the image you included shows, G major has 1 sharp, and G minor has 2 flats. G Phrygian has 3 flats, etc. Or for custom scales, we'd need a way for the user to pick sharps or flats. |
Could you just minimize on accidentals to pick which one to use? So solve the problem for flats and sharps, then pick the one with less? (completely ignoring performance ofc) |
|
The Deluge already has the ability to keep track of different scale modes of the Major scale and other alternative scale types that don't follow the step pattern of WWHWWWH of the major scale. What's the accidental preference of any key/scale? Say it's G minor, with 2 flats. G minor is the 6th mode of Bb and so favors flats. G Phyrigian is the 3rd mode of Eb so favors flats. The system already lets you pick scale with the SHIFT-SCALE option. It's all just math from the root note. When the user has to reconfigure the system to choose either sharps or flats, then no matter what, half of the scales will always be wrong. If I pick flats as my preference, then G Major shows a Gb for the seventh degree. This is wrong. If I pick sharps, then there is no Bb key, I can only pick A#. |
|
@todd-gochenour I am aware of the different scale types available in the deluge. That was part of my concern above. While it could be calculated, I still think it should be configurable for custom scales. Or what about some of the more esoteric scales available like Hungarian minor which uses both sharps and flats? Perhaps a clip/song/default setting would be better? So you can set an overall default for what you usually use (I'd pick flats because I write a lot in F and Bb) but one can override that at the song and clip level. That may introduce a few conditionals each time a key is pressed. Side question: can you set things at the clip level? I don't believe I can find a menu for clip settings. If we were to go down the route of calculating it how would you imagine it would work? I don't think we'd want to calculate it each time a key is pressed. Perhaps when picking the scale we could set the type of accidental to use on the clip object? Or we could pre-calculate it and store it somewhere? I think the scale-aware version would then be a configurable option in the community firmware settings. Edit: Can scales be set per clip? Perhaps just a song setting will be sufficient. |
|
Key and scale cannot be set at the clip level, only at the song level. The calculation is trivial arithmetic and has no impact on performance. Exotic scales are treated as major unless they have a minor third, then they are minor. The Deluge is not supporting modes of any exotic scales, only the modes of the major scale. I am not aware of a scale that uses both sharps and flats simultaneously. If you work in the key of F, you should see displayed a Bb for the fourth note in the scale without having to switch a user preference. If you work in G, then you should see an F# for the seventh note degree. I am currently writing a function in preset_scales.cpp called getAccidental(rootNote,scale) which will return the proper accidental preference to use for a given key and scale. You can then use this function in noteCodeToString() to determine whether to display sharps or flats. There will be an override of the function noteCodeToString() that accepts parameters rootNote and scale as given from the current song. |
|
I just posted Scale-Aware-Accidentals #3243 which implements the strategy that we discussed here. I don't think we need a user preference option now. |
|
I'll leave it to the community to decide if we should do it that way but I guess I'll close this for now |
|
Work is being done differently under #3243 |
Add community feature setting to allow for notes to be displayed with either sharps or flats
with the setting enabled: on oled displays C, DB, D, EB, E, F etc
on 7SEG: C, D., D, E., F, etc