-
-
Notifications
You must be signed in to change notification settings - Fork 105
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
Update the kit velocity keyboard view's zoom levels behavior #3132
Update the kit velocity keyboard view's zoom levels behavior #3132
Conversation
Instead of having square pads, allow for rectangular pads, and choose dimensions to maximize the used control area.
Move some calculations outside of loops to make things more efficient. Adjusted brightness levels so that they are slightly brighter normally, and so that pressing the pads has a smaller increase. The pads flashed too bright before.
Went with a quadratic curve for brightness, as it perceptually matches the velocity levels more closely. Made the default brightness be maximum and reduce by half when pressing a pad. so 1 to 0.5 rather than 0.33 to 1 from the community branch. Made calculations more efficient, and functions clearer. Added comments.
Rather than using the common method for getting colors, which often led to adjacent pads having the same or similar colors, I set up a new calculation for it exclusive to this view. It is possibly more efficient than running through the previous way. By having a constant hue step, the pad colors for each sound stay consistent between the different zoom levels, which makes things much easier to keep track of. I will experiment to figure out what the optimum step is, but it seems pretty good as it is. Also slight tweak to velocityFromCoords(), changed to only use edge size for conditional checks, keep things more direct and clear, and eliminate one parameter lookup.
Adjusted color scheme to have best setup for contrast between adjacent pads at all zoom levels. Also made things a bit more efficient. Still need to clean up commented out code, and test some more things.
Made a bug and fixed it. Added a full 128 pad zoom level. Set up the amount that it dims based on the pad area, so that the smaller pads will dim more and the larger pads will dim less, to make for a more even overall brightness change between the different levels with a pad press. Also changed the display to show the pad area rather than the zoom level, since the zoom level is fairly meaningless. Plus this way people with triskaidekaphobia shouldn't get triggered.
changed zoom level to 0-12 rather than 1-13, to make things simpler, now that that isn't being displayed onscreen. Made code more efficient by preventing it from refreshing the screen when it was already at the limits for zoom level or scroll offset.
I figured out how to properly access the colourOffset parameter for the clip, so now the color shifting works. Added more comments, got logic set up properly now.
just tidying up.
…romAudible#3168 that I discovered. It is a slight efficiency boost to check the notes array from the end of it, where the active notes are kept, and then exit the loop when an inactive note is encountered. Fixed minor typo on notes_state, and left a comment in keyboard_screen.cpp. The root of the issue is likely in one of those two files, so I was investigating.
Fixed a bug I created where held notes would trigger when another note is pressed, but only the higher note. So had to go back to the previous way of iterating through the pressed notes array. Removed the unnecessary screen refresh that was happening from control button presses and releases, in keyboard_screen.cpp, thanks to Sean.Good.Vibes' suggestion. Tested and confirmed. Reconfigured the functions to be more efficient, and removed the noteFromCoords function as it was too small and inefficient to keep it. For the pad color calculations, I believe it will make more sense to just increment the note, and generate all the pads for each note in sequence. I will make that change next, I just wanted to capture the current working state.
Used the *= increment method for the quadratic curve for the brightness that stellar_aria suggested. Also fine-tuned the lowest brightness level for the different sized pads. Switched to an outer nested loop for the drum pads, with an inner loop for the sub-pads. Simpler to keep track of the note index that way. Made it slightly more efficient by ignoring pad presses on unlit pads. Changed the variables from camelCase to snake_case. Much better.
Instead of having square pads, allow for rectangular pads, and choose dimensions to maximize the used control area.
Move some calculations outside of loops to make things more efficient. Adjusted brightness levels so that they are slightly brighter normally, and so that pressing the pads has a smaller increase. The pads flashed too bright before.
Went with a quadratic curve for brightness, as it perceptually matches the velocity levels more closely. Made the default brightness be maximum and reduce by half when pressing a pad. so 1 to 0.5 rather than 0.33 to 1 from the community branch. Made calculations more efficient, and functions clearer. Added comments.
Rather than using the common method for getting colors, which often led to adjacent pads having the same or similar colors, I set up a new calculation for it exclusive to this view. It is possibly more efficient than running through the previous way. By having a constant hue step, the pad colors for each sound stay consistent between the different zoom levels, which makes things much easier to keep track of. I will experiment to figure out what the optimum step is, but it seems pretty good as it is. Also slight tweak to velocityFromCoords(), changed to only use edge size for conditional checks, keep things more direct and clear, and eliminate one parameter lookup.
Adjusted color scheme to have best setup for contrast between adjacent pads at all zoom levels. Also made things a bit more efficient. Still need to clean up commented out code, and test some more things.
Made a bug and fixed it. Added a full 128 pad zoom level. Set up the amount that it dims based on the pad area, so that the smaller pads will dim more and the larger pads will dim less, to make for a more even overall brightness change between the different levels with a pad press. Also changed the display to show the pad area rather than the zoom level, since the zoom level is fairly meaningless. Plus this way people with triskaidekaphobia shouldn't get triggered.
changed zoom level to 0-12 rather than 1-13, to make things simpler, now that that isn't being displayed onscreen. Made code more efficient by preventing it from refreshing the screen when it was already at the limits for zoom level or scroll offset.
I figured out how to properly access the colourOffset parameter for the clip, so now the color shifting works. Added more comments, got logic set up properly now.
just tidying up.
…romAudible#3168 that I discovered. It is a slight efficiency boost to check the notes array from the end of it, where the active notes are kept, and then exit the loop when an inactive note is encountered. Fixed minor typo on notes_state, and left a comment in keyboard_screen.cpp. The root of the issue is likely in one of those two files, so I was investigating.
Fixed a bug I created where held notes would trigger when another note is pressed, but only the higher note. So had to go back to the previous way of iterating through the pressed notes array. Removed the unnecessary screen refresh that was happening from control button presses and releases, in keyboard_screen.cpp, thanks to Sean.Good.Vibes' suggestion. Tested and confirmed. Reconfigured the functions to be more efficient, and removed the noteFromCoords function as it was too small and inefficient to keep it. For the pad color calculations, I believe it will make more sense to just increment the note, and generate all the pads for each note in sequence. I will make that change next, I just wanted to capture the current working state.
Used the *= increment method for the quadratic curve for the brightness that stellar_aria suggested. Also fine-tuned the lowest brightness level for the different sized pads. Switched to an outer nested loop for the drum pads, with an inner loop for the sub-pads. Simpler to keep track of the note index that way. Made it slightly more efficient by ignoring pad presses on unlit pads. Changed the variables from camelCase to snake_case. Much better.
7864822
to
ea5dba1
Compare
Refinements completed. Please review and test at your convenience. |
Sapphire Arches suggested it was not necessary to precalculate the pad colors, so I moved to calculating the colors when rerendering the pads. So it already has all the needed values, and doesn't need to look up all the rgb colors from a big table each time. simpler code, maybe more efficient to run?
If you want to write your own changelog entry, I think that's the last thing this PR needs and then it's good to merge. Thanks! |
Um, sorry, but I'm not sure what I need to do from this point. I hope I didn't screw it up. Do I need to edit the file at https://github.com/SynthstromAudible/DelugeFirmware/blob/community/CHANGELOG.md to include that with this PR? Do I add my changes to the 1.3.0 section there? Edit: Yeah, that must be it. I'll finish that up tomorrow then. |
Set up the Kits changes to cover all the KIT VELOCITY KEYBOARD VIEW Changes in one section.
Alright, I updated the changelog entry. I covered everything I could think of that the user could notice, but please let me know if that is more than is needed. |
It looks like the build checks fail because I took out the precalculate function from the .cpp. It didn't build right when I took out the override in of the .h, so do I need to leave it in the .cpp but empty? Or do something else? |
Deleting it from both |
When I do that it gives this error when building: Edit: |
@Metamere just put the empty body |
Fixed final build check issue.
Head branch was pushed to by a user without write access
Okay, that all makes sense now, and I got it fixed. Thanks for your patience! |
598cd8e
Instead of only having square pads for the velocity keyboard view, allow for rectangular pads, and choose dimensions for those rectangles (and squares) to maximize the used control surface area. I made an interactive demo using p5.js to prototype the behavior, so you can test out my proposed scheme and compare it to the current one here: https://openprocessing.org/sketch/2492402
In my scheme, the number of pads in each zoom level has a more gradual curve, with each carefully set up to maximize usability. It is functional in its current state, but I would like to add a couple of refinements related to the pad brightness and color selection.