-
Notifications
You must be signed in to change notification settings - Fork 81
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
Fix breakpoint note names #536
Conversation
Build for testing: Does it work as expected @Banana71? |
I must admit I have no idea what this feature is about and quite what the issue is, but if the original scaling is A-1 to C8, but MiniDexed is currently showing A1 to C9, then shouldn't the number of entries remain the same? But in adding a lower octave should we be removing the highest (C#8-C9) octave too? As I say, I don't really know what the feature is, but any talk of scaling that adds 12 new divisions sounds like the amount per division will now be different... Also the same note table is used in ToTransposeNote so that might need some adjusting too? Again I'm not quite clear what either of these has to achieve to know for sure. Kevin |
[ci skip]
I haven't tested the version yet but... C3 is 60 decimal const char CUIMenu::s_NoteName[112][5] = This is from your code: |
I set static const unsigned NoteC3 = 51; because C3 is the 51th element of the array (counting from 0), is my logic wrong? |
Hang on... in Musical terms, notes go A1, A#1, B1, C2, C#2... not A2, A#2, B2, C2... :) That whole table is wrong surely! Kevin |
Right, now that you are saying it... the table Banana71 posted from Dexed is correct though? |
@Banana71 posted it, but that has 112 entries. As I say, Dexed only supports 0..99 so I'm not quite sure what is happening still... Oh wait, that says [112] but counting I think it is [100] so that should be right? |
Ok, so for ToTransposeNote the UI limits it to the range 0 to 48 here: https://github.com/probonopd/MiniDexed/blob/main/src/uimenu.cpp#L279C13-L279C28 But the manual states: So that is why NoteC3 must be set to the index of middle C, so that the line https://github.com/probonopd/MiniDexed/blob/main/src/uimenu.cpp#L1066 Can scale the value to fit the note table. Update: And just checking the operation of the transpose menu in the current build, yes that is wrong too atm! This should fix that too. Kevin |
Build for testing: MiniDexed_2023-08-14-48d7112 |
Menus all seem to work and from what I can hear the transpose function is working for +/- 2 octaves (C1-C3-C5). Not really understanding what the Keyboard Scaling breakpoint does, I can't test that (although I can see the menus are doing what we want them to). Note: Tested in a Pi3 with SSD1306 display. Kevin |
@diyelectromusic thanks for testing. I don't entirely understand Keyboard Scaling breakpoint either, so... |
Thank you very much for your patience, the breakpoint is now displayed correctly. @probonopd and @diyelectromusic🎉🖖🏼
Keyboard Level Scaling controls the volume of each operator. The volume of the operators is influenced starting from the breakpoint. Together with velocity, very complex sounds are possible. A layered single sound can be heard in the sound sample. The high notes have hardly any overtones. In the middle range, the bells come through and the bass range sounds distorted depending on the velocity. KeyScaling.mp4 |
Thanks for your testing and explanations @Banana71.
Great to know. Hopefully this doesn't break anything else. |
#534
Not 100% sure whether this is the correct fix in
uimenu.cpp/.h
:Only one way to find out... try it out...