Skip to content

Feature: chord keyboard#2385

Merged
seangoodvibes merged 21 commits into
SynthstromAudible:communityfrom
madeline-scyphers:feature/chord-keyboard
Aug 10, 2024
Merged

Feature: chord keyboard#2385
seangoodvibes merged 21 commits into
SynthstromAudible:communityfrom
madeline-scyphers:feature/chord-keyboard

Conversation

@madeline-scyphers

@madeline-scyphers madeline-scyphers commented Aug 5, 2024

Copy link
Copy Markdown
Contributor

Description

It adds a chord keyboard as an additional keyboard option, accessible the same way as other keyboards instrument supporting keyboards (press keyboard button, then hold it while turning select knob).

Each column is a note (chormatic, all 12 notes), with infinite scrolling with horizontal encoder. Each pad is a different chord. So the first row is just roots, second is major triads, 3rd is minor triads, etc. Vertical encoder scrolls up to more chords.

Holding a note on a row and pressing in the horizontal encoder and turning changes the voicing for all the chords on that row (hold any major triad, pressing and turning horizontal encoder changes ALL major triad voicings, not just the one you are holding).

Future Improvements

  • Allow changing chord rows (vertical enc and pad press, kind of like in song or clip mode?)
  • User defined chords, voicings, and chord orderings
  • allow moving between chord families easily, maybe holding a pad and pressed in vert enc switches from G7 to G9 to G7b9, etc
  • Have the pads be dimmer or brighter based on something that makes sense to help with navigation (key or tonal centers, just light up certain columns or rows, etc)
  • Make the bottom row always be the root might be nice so that way you can always play a melody or bass line no matter how far up in the chords you are
  • Maybe allow a bottom row step sequencer to punch in chords instead having to record them
trim.2C31860D-6869-478C-ABD0-7B1C9A47C080.MOV

@madeline-scyphers madeline-scyphers marked this pull request as draft August 5, 2024 04:23
@github-actions

github-actions Bot commented Aug 5, 2024

Copy link
Copy Markdown
Contributor

Test Results

102 tests  +3   102 ✅ +3   0s ⏱️ ±0s
 15 suites +1     0 💤 ±0 
 15 files   +1     0 ❌ ±0 

Results for commit 4108a46. ± Comparison against base commit 17761c9.

♻️ This comment has been updated with latest results.

@madeline-scyphers madeline-scyphers marked this pull request as ready for review August 6, 2024 04:53
@seangoodvibes seangoodvibes changed the title [DRAFT] Feature: chord keyboard Feature: chord keyboard Aug 6, 2024
@seangoodvibes seangoodvibes enabled auto-merge August 6, 2024 05:22
@seangoodvibes seangoodvibes disabled auto-merge August 6, 2024 05:22
Comment on lines +161 to +174
int32_t voicingOffset[kUniqueChords] = {0};
uint32_t chordRowOffset = 0;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better put these two state trackers in the KeyboardStateChord struct or here? If they are kept there, then we could more easily reuse the ChordList class for the chord columns if we want to add more functionality to those. If that isn't important, then just unifying those chord columns with these chord definitions, without the ChordList class could be enough. Thoughts?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd go with your gut feel. Either way it doesn't look super difficult to change in the future if needed, and I think you might be the most likely person to extend it into chord columns anyway!

@madeline-scyphers madeline-scyphers force-pushed the feature/chord-keyboard branch 2 times, most recently from 77ee670 to d41cede Compare August 8, 2024 17:35

@m-m-adams m-m-adams left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some nits but code looks good!

@@ -0,0 +1,142 @@
/*
* Copyright © 2016-2023 Synthstrom Audible Limited

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feel free to claim copyright on new files where you wrote the code

Comment on lines +161 to +174
int32_t voicingOffset[kUniqueChords] = {0};
uint32_t chordRowOffset = 0;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd go with your gut feel. Either way it doesn't look super difficult to change in the future if needed, and I think you might be the most likely person to extend it into chord columns anyway!


/// Will be called with offset 0 to recalculate bounds on clip changes
virtual void handleHorizontalEncoder(int32_t offset, bool shiftEnabled) = 0;
virtual void handleHorizontalEncoder(int32_t offset, bool shiftEnabled,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only change that I'm not sure about since it sort of breaks the metaphor of scrolling the keyboard under your fingers

That said I'm also not sure how useful that is beyond familiarity since you'll usually just have one pad held in this keyboard anyway, and your discord posts about it really speak to how useful this is so I think we should give it a try

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For documentation, this is what I put in the discord

"The reason I wasn't doing that was with the other keyboards SHIFT + :hturn: had more to do with the keyboard layout.

If it is SHIFT, and you have to be pressing a chord to change voicings, it pretty much requires sticky SHIFT. It would be near impossible to hold SHIFT + :hturn: + :padpress: . I don't know how much that is a draw back.

Then considerations for future next steps,

I want to add some more functionality like changing Chord order, which should be fairly easy to do if handleVerticalEncoder is passed PressedPad presses[kMaxNumKeyboardPadPresses], then you could just drag them like you drag notes in clip view or song view.

The bigger thing is that I would like to maybe add an ability to change notes from one chord to the next inside a family. If you are on a G7, you could move to G9, move to G7b9, etc. This would potentially allow reducing the height of the chord keyboard, maybe. But then I don't know how to do that controls wise. I thought about maybe :vpressturn: + :padpress: or SHIFT + :vturn:/:hturn: + :padpress: could be used for this. It feels like the chord keyboard might just have more functionality than the other keyboards.

If we had horizontal dimmer or white bars (every 4 rows or whatever) for easier navigation, that could be configurable with shift + :hturn: or we could use SHIFT + :hturn: to offset the root note row octave wise or allow more rows of the root note for playing a bass line while doing chords.

Just things I have thought about for other useful features and controls with the chord keyboard for the next round. I didn't want to make this initial PR too big
Also since I have so many future plans for the chord keyboard feature and control wise. I was thinking if it does make it into 1.2, putting it behind a community features menu for now. I don't think it needs to be behind one forever since it doesn't impact basically anything when it isn't in use, but if it is behind that menu it allow it to be more fluid to change until it is finalized"

else {
drawDot = 0;
}
*thisChar = '\0';

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit but maybe just null initialize noteName={0} and then just use noteName[0] for the note code and noteName[1] for the sharp/flat? It's a bit confusing to read iterator code that's not really iterating and that avoids having noteName[2] be garbage for natural notes

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This helped me figure out why I was why I was getting garbage if I just used noteCodeToString straight like elsewhere in the code base, and why I partially reimplemented it. So thanks!

Comment thread src/deluge/gui/ui/keyboard/layout/chord_keyboard.cpp Outdated
Basic proof of concept chord keyboard
just a few chords and horizontal scrolling works
it has colored columns, but those don't change with horizontal scrolling
yet
The chord name is now displayed when pressing the chord
The chord keyboard class is entirely responsible for displaying
There is an override in `instrument_clip_minder` to not display note
name when the keyboard type is chord keyboard and keyboard is active
Also, had to reimplement some of `noteCodeToString` because on some
of the columns I was getting garbage characters (F#@ for example instead
of F#) or if you played two chords together, the it wouldn't display
properly.
Keyboards now know if the x Enc is being pressed when the
handle horizontal enc funtion is being called and what notes
are being pressed. This allowed me to change voicings on
the chords being pressed.
The keyboards currently implemented had their signature changed
but nothing about their implementation changed

Also added Chords class instead of Chords struct
This allowed me to better do bounds checks for voicings.
So if some chord doesn't have as many voicings,
it will just return the last defined voicing.
Also add interval consts for easier chord defining
@seangoodvibes seangoodvibes added this pull request to the merge queue Aug 10, 2024
Merged via the queue into SynthstromAudible:community with commit 395546d Aug 10, 2024
seangoodvibes added a commit to seangoodvibes/DelugeFirmware that referenced this pull request Aug 10, 2024
Added back edits from SynthstromAudible#2391 that were reverted by mistake by SynthstromAudible#2385
seangoodvibes added a commit to seangoodvibes/DelugeFirmware that referenced this pull request Aug 10, 2024
Added back edits from SynthstromAudible#2391 that were reverted by mistake by SynthstromAudible#2385
github-merge-queue Bot pushed a commit that referenced this pull request Aug 11, 2024
Added back edits from #2391 that were reverted by mistake by #2385
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.

3 participants