Piano Roll Strum Tool #7725

regulus79 · 2025-02-21T02:30:25Z

This PR adds a complex strum tool to the PianoRoll, allowing the user to take a selection of chords, and drag around the notes to shape the strum exactly how they want it.

Additionally, holding Shift and moving the mouse up/down will make the notes in the strum follow a power curve, for greater flexibility.

The current shortcut for the strum tool is Shift+J (Shift+S was already taken) I'm not sure why I chose J, but if you have a better idea feel free to share it.

Demo

strum_demo.mp4

How it works

I want you to think for a moment--how does a strum work? It's not a matter of shifting each note linearly based on their key, since after all, the notes may not be evenly spaced in the chord. You have to shift them based on the order you want them to be played in. So that's fine, just iterate up/down a chord, and shift the notes a bit based on their index, right? Well, ok, but how do you handle strumming multiple chords at once? That is the difficult part of the problem.

I couldn't solve it LOL. So instead I just did the easy thing and decided to group the selected notes into chords based on whether or not they were overlapping. So each chord is a little "island" of notes which are all connected if you squash the piano roll vertically. Then I perform the same strumming algorithm on each chord individually. I think it works pretty well, but there are some edge cases, like if two chords overlap, which may cause problems.

But there's another problem! What if two chords have different number of notes? Well, for simple strumming up/down, it's fine, just multiply the shift of each note by some proportionality constant and you're good to go. But what if you want to strum from a note in the middle of the chord? Well, I decided that it would make sense to strum all chords based on the ratio of where the strumming takes place in the chord. So if you start the strum in the middle of the chord, all other chords will be strummed from the middle. If you start it 3/4 of the way up, all the other chords will be strummed from 3/4 of the way up, relative to the number of notes they have (not the key positions!)

In the code, it looks like this:

When the strum tool is activated, the chord groups are created from the currently selected notes.
When the strum tool is pressed down, the currently clicked note's position within its chord is calculated, and all the selected note positions are saved with Note::setOldPos.
When the strum tool is held down and dragged, all of the notes in each chord are shifted based on their index relative to the clicked note ratio, along with some warping if Shift is held down.

Changes

The base code was copied from the Knife action, so there are many similarities in how it is handled.

Added Strum as an Action and EditMode
Added setStrumAction() and cancelStrumAction() methods, modeled after the Knife tool's methods.
A vector of NoteVectors, m_selectedChords was added to store the list of notes for each chord to be strummed.
A method setupSelectedChords() was added to initialize the vector of chords.
The methods updateStrumPos() was added to update the note positions as the mouse moves during the strum.
Various variables were added to store the start/end time/mouse height of the strum action, its state, and the strum note height ratio

germona · 2025-02-21T06:46:22Z

Waow, Regulus, that looks very promising, in fact it can do much more than just strumming.

At 1:25 you drag the bottom note of the chord to the left of the beat, and that my friend, is just what a real guitar player would do. He starts the strum before the beat, and the last note of the strum is exactly ON the beat of a bar, remember in a 4/4 measure there are 4 beats in a bar.

Now, is there a way to use this feature in lets say an appimage of lmms?

Thank you very much for your time.

Jean

qnebra · 2025-02-21T08:12:45Z

Now, is there a way to use this feature in lets say an appimage of lmms?

Thank you very much for your time.

Jean

Builds for this pull request are in "Actions" tab. For example here:
https://github.com/LMMS/lmms/actions/runs/13449705715

bratpeki · 2025-02-21T08:33:27Z

Looks cool! Will be checking out!

bratpeki · 2025-02-21T08:34:46Z

Seems like I accidentally took down the already present reviewers, my bad 😆

germona · 2025-02-21T10:22:53Z

Now, is there a way to use this feature in lets say an appimage of lmms?
Thank you very much for your time.
Jean

Builds for this pull request are in "Actions" tab. For example here: https://github.com/LMMS/lmms/actions/runs/13449705715

I am a total newbe when it comes to make/makeInstall. For some reason it keeps giving me errors by the dozen. So how can i download and use these builds from your link?
I am on linux Kernel: 6.12.12-2-MANJARO CPU: Intel i5-3570 (4) @ 3.800GHz
I am running LMMS 1.3 and 1.2.2 as appimages.
Thnks

bratpeki · 2025-02-21T11:09:08Z

@germona, please use https://lmms.io/download/pull-request/7725. You can do this for any active PR! 😄

germona · 2025-02-21T11:58:33Z

@germona, please use https://lmms.io/download/pull-request/7725. You can do this for any active PR! 😄

Ok i have downloaded the appimage and the strumming works even better than i could have imagined.......
Woaw LMMS is awesome.

szeli1

I tested this feature and found some issues:

In strum mode the user can click notes that aren't selected, this can result in unpredictable behaviour.

The notes can "slide under the piano keys" past the widget boundary on the left side. I believe this is unwanted.

include/PianoRoll.h

src/gui/editors/PianoRoll.cpp

szeli1 · 2025-02-22T12:34:54Z

src/gui/editors/PianoRoll.cpp

+		int i = 0;
+		for (Note* note: chord)
+		{
+			float heightRatio = 1.f * i / (chord.size() - 1);


Suggested change

float heightRatio = 1.f * i / (chord.size() - 1);

float heightRatio = static_cast<float>(i) / static_cast<float>(chord.size() - 1);

This doesn't look that simple, you could ignore this

Does this improve performance or would the compiler automatically do this?

I'm not sure, I think it will result in the same code at the end of the day, but it is more explicit

zonkmachine · 2025-02-22T13:44:32Z

The notes can "slide under the piano keys" past the widget boundary on the left side. I believe this is unwanted.

If your chord starts at 'one', yes. Maybe have it restricted by default and then have an accelerator key to allow full motion to the left? I'm not sure what a negative note position in the pattern really means. I'll try and test this some more.

Co-authored-by: szeli1 <[email protected]>

regulus79 · 2025-02-22T18:01:35Z

I have added a std::max which prevents note from sliding before the start.

zonkmachine · 2025-02-23T23:21:59Z

What behavior were you expecting?

Hm. Confusion ensues. Need to think about it...

bratpeki · 2025-02-24T21:06:53Z

I can see some issue were found, please tag me when that is addressed and I'll take a look myself.

bratpeki · 2025-02-26T10:41:41Z

Looks terrific! Taking a look now.

bratpeki · 2025-02-26T10:59:06Z

It's outside the scope of this PR, but the ability to make selections while in the strum tool would be a great QOL feature. Additionally, using Ctrl+A while in strum mode would be cool.

The undo works great. It would be very nice if the selection could remain when undoing the strum, as it is a different action to strumming, and thus shouldn't disappear when only strumming is undone.

Consider the user messed up and wants to try the strum again. Currently, he has to:

undo,
get out of strum mode,
go into selection mode or use Ctrl+LMB in draw mode,
make the selection again,
go back into strum mode, and
redo it.

And suppose he messes up again... 🤣

Alternatively (but not as good as keeping the selection), moving the keyboard shortcut somewhere closer to Shift+S (Selection) is also a good move.

RainbowShatter · 2025-02-26T12:53:38Z

Awesome!!

regulus79 · 2025-02-26T22:11:21Z

Thanks for testing!

It's outside the scope of this PR, but the ability to make selections while in the strum tool would be a great QOL feature. Additionally, using Ctrl+A while in strum mode would be cool.

I was originally intending to add this, but when I tried, it made everything waaaayy more complicated, and handling the logic with temporarily switching modes was not working very well, leading to a lot of edge case bugs.

The undo works great. It would be very nice if the selection could remain when undoing the strum, as it is a different action to strumming, and thus shouldn't disappear when only strumming is undone.

That sounds like a great idea. Unfortunately, I do not fully understand how the undo system works, so implementing it would be difficult.

Also fyi, you can exit from strum/knife mode by rightclicking.

szeli1 · 2025-02-26T22:16:09Z

great. It would be very nice if the selection could remain when undoing the strum, as it is a different action to strumming, and thus shouldn't disappear when only strumming is undone.

You can't implement this easily. I do not recommend requiring this feature (or implementing it in this pr). It would be nice, but it isn't worth it. The Journalling system wasn't made for this.

germona · 2025-02-27T06:13:03Z

@regulus79 I think the tool, how it is now is perfect. One can do the basic things a guitar player would do, and that is UpStrums and DownStrums. You can select one or more or all chords and ad a strum to them making sure that the last note of the strum is ON the beat of a bar.
And exactly that you can do by grabbing the bottom note and drag it a few midi ticks to the left, in case of a downStrum.
In case of an UpStrum you grab the top note and drag it a few midi ticks to the left.

I don't know how it works with these push requests, but how big is the chance that this feature will be included in a new version of LMMS?

Anyway thank you a lot for this wonderful feature.
Jean

szeli1

Did you fix the issues about notes not moving I mentioned?

If yes, then I will approve this.

include/PianoRoll.h

regulus79 · 2025-02-27T20:31:05Z

Did you fix the issues about notes not moving I mentioned?

Could you describe what the expected behavior is supposed to be? (and maybe could you give a way to reproduce the issue?) zonkmachine gave a test project which exhibited a similar(?) effect when strumming, but I am fairly certain that in that case it was not a bug.

Or well, I mean the strum tool in that case was not malfunctioning. I have it set so that chords which have different numbers of notes are strummed the same amount proportionally, with respect to each note's top/bottom note. So if one chord has five notes and one has four, and the user drags the bottom of one of the chords to the right, say, 20 ticks, then the notes in each of the chords will be spread out so that they span 20 ticks. The chord with 5 notes will have its notes spread 4 ticks apart, and the chord with 4 notes will have its note spread 5 ticks apart. This can sometimes look a bit strange, especially if the chords are right next to each other, but the notes are still spaced correctly, and it should still sound right.

If you can come up with a better strumming algorithm, I would love to hear it. But so far this is the best idea I have.

germona · 2025-02-28T05:58:57Z

The chord with 5 notes will have its notes spread 4 ticks apart, and the chord with 4 notes will have its note spread 5 ticks apart.
That is exactly how it should be. Please don't change anything there.

szeli1

I wasn't sure why the notes on the top of a chords didn't move when notes from other chords did move a the same height, but I see now that this is intended.

I tested and I approve this change.

bratpeki · 2025-03-01T17:03:21Z

Great PR! Just two requests:

Could it be possible that, if no selection is made, the strum is done over the notes in-line with the cursor?
If we're not implementing selections while in strum-mode, could it be possible to disable all the buttons I've shown below, since none of them are active while you're in strum mode, and the user should be aware of this. Currently, we're telling the user he's both in strum and selection at the same time, which isn't true.

regulus79 · 2025-03-01T17:55:37Z

Could it be possible that, if no selection is made, the strum is done over the notes in-line with the cursor?

Technically yes. But currently I keep the logic simple and only recalculate the chords when the strum tool is activated, whereas this would require calculating them every time the user clicks. One of the reasons I only calculate the chords at the very start is so that they still remain "separate" even when they are really distorted and overlapping. It might get complicated to have two different logics for whether notes are selected vs not.

If we're not implementing selections while in strum-mode, could it be possible to disable all the buttons I've shown below, since none of them are active while you're in strum mode, and the user should be aware of this. Currently, we're telling the user he's both in strum and selection at the same time, which isn't true.

This issue already exists in master with the knife tool. I can look into it, but I think it would require some effort to rework the way the gui handles the actions. Could we maybe deal with it in another PR?

bratpeki · 2025-03-01T18:28:02Z

But currently I keep the logic simple and only recalculate the chords when the strum tool is activated, whereas this would require calculating them every time the user clicks

Would it be possible to get the "time" where the cursor is located and work on the notes in that section?

It might get complicated to have two different logics for whether notes are selected vs not

Maybe so, but it would be nice to uniform the behavior. You can move one note or a selection of notes. We're also adding cutting for selected notes as well as a single note. If a functionality logically can be used with and without the selection, I think we should provide it, for the sake of UX.

This issue already exists in master with the knife tool

Even more reason to work on it! That would probably mean this is best suited for another PR, when this is merged.

Could we maybe deal with it in another PR?

Sure, if it is easier for you, but I'd think these are closely related to the core functionality. I could open an issue called "Improving the strum tool" where I list both of these issues and we can start work from there. If you're up, I can approve this an we can move it to the new issue!

regulus79 · 2025-03-01T19:36:13Z

Done!

Please note that using the strum tool without a selection may lead to odd behaviors if you strum a chord so that it overlaps with other notes, and then try to strum it back (since it will then calculate the selected chord as being all of those notes) Or if you strum a chord so long that it's notes no longer overlap each other.

regulus79 · 2025-03-01T20:17:47Z

Alright, I've also added the ability to ctrl-select while in strum mode!

bratpeki · 2025-03-01T20:35:05Z

Amazing! To not go out of scope, that works just for the strum tool? Or is it universal?

regulus79 · 2025-03-01T20:37:53Z

Universal as in, for all tools? I only added it for the strum tool; I don't think it would really work for tools like the knife tool because ctrl does something (unquantization) in the knife tool.

bratpeki · 2025-03-01T23:56:26Z

Totally understandable! I'll be testing this ASAP. Thank you!

regulus79 · 2025-03-04T23:12:41Z

Due to the way I detect chords being prone to categorizing two chords as one if they overlap even slightly, I have reverted the ability to strum chords while not having any notes selected. @bratpeki found its behavior to be undesirable when strumming chords which are right next to each other, as they easily morph into one chord and become very odd to strum.

regulus79 added 7 commits February 20, 2025 18:36

Initial implementation

ccc7bd7

Clean up and add comments

bc08542

Set icon to arp_free

c2d8a40

Minor change

10b6c40

Refactor chord grouping into new method and clean up code

3fdd8ab

Change std::powf to std::pow

0075af8

Update clip length after strumming

4aa3711

bratpeki self-assigned this Feb 21, 2025

bratpeki requested review from bratpeki, DomClark, tresf and sakertooth February 21, 2025 08:33

tresf removed their request for review February 21, 2025 15:55

Don't strum if the user isn't clicking a note

d6d9299

zonkmachine self-requested a review February 21, 2025 23:46

szeli1 suggested changes Feb 22, 2025

View reviewed changes

regulus79 and others added 4 commits February 22, 2025 12:30

Fix style

24b555e

Co-authored-by: szeli1 <[email protected]>

Update comment

b3d87b7

Co-authored-by: szeli1 <[email protected]>

Update comments

98e05f3

Use size_t in loop, and prevent notes from going before 0 ticks

04c0992

Fix behavior when dragging unselected notes

e2f28fc

regulus79 added 2 commits February 23, 2025 18:06

Add comments to PianoRoll.h

7941472

Add comment to PianoRoll.cpp

bae8e04

szeli1 reviewed Feb 27, 2025

View reviewed changes

include/PianoRoll.h Outdated Show resolved Hide resolved

Change header comments to //!

5044c00

szeli1 approved these changes Feb 28, 2025

View reviewed changes

Let the strum tool be used without a selection

351967c

regulus79 added 2 commits March 1, 2025 15:12

Allow ctrl selection while strumming

7c1b36f

Fix segfault

0aa1eb8

Revert unselected strum feature

8d12842

regulus79 added 2 commits March 4, 2025 18:17

Merge branch 'master' into pianoroll-strum

ec7266d

Add check for valid midi clip

96048e2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Piano Roll Strum Tool #7725

Piano Roll Strum Tool #7725

regulus79 commented Feb 21, 2025

germona commented Feb 21, 2025

qnebra commented Feb 21, 2025

bratpeki commented Feb 21, 2025

bratpeki commented Feb 21, 2025

germona commented Feb 21, 2025

bratpeki commented Feb 21, 2025 •

edited

Loading

germona commented Feb 21, 2025

szeli1 left a comment

szeli1 Feb 22, 2025

regulus79 Feb 22, 2025

szeli1 Feb 23, 2025

zonkmachine commented Feb 22, 2025

regulus79 commented Feb 22, 2025

zonkmachine commented Feb 23, 2025

bratpeki commented Feb 24, 2025

bratpeki commented Feb 26, 2025

bratpeki commented Feb 26, 2025

RainbowShatter commented Feb 26, 2025

regulus79 commented Feb 26, 2025

szeli1 commented Feb 26, 2025 •

edited

Loading

germona commented Feb 27, 2025

szeli1 left a comment

regulus79 commented Feb 27, 2025

germona commented Feb 28, 2025

szeli1 left a comment

bratpeki commented Mar 1, 2025

regulus79 commented Mar 1, 2025

bratpeki commented Mar 1, 2025

regulus79 commented Mar 1, 2025

regulus79 commented Mar 1, 2025

bratpeki commented Mar 1, 2025

regulus79 commented Mar 1, 2025

bratpeki commented Mar 1, 2025

regulus79 commented Mar 4, 2025

	float heightRatio = 1.f * i / (chord.size() - 1);
	float heightRatio = static_cast<float>(i) / static_cast<float>(chord.size() - 1);

Piano Roll Strum Tool #7725

Are you sure you want to change the base?

Piano Roll Strum Tool #7725

Conversation

regulus79 commented Feb 21, 2025

Demo

How it works

Changes

germona commented Feb 21, 2025

qnebra commented Feb 21, 2025

bratpeki commented Feb 21, 2025

bratpeki commented Feb 21, 2025

germona commented Feb 21, 2025

bratpeki commented Feb 21, 2025 • edited Loading

germona commented Feb 21, 2025

szeli1 left a comment

Choose a reason for hiding this comment

szeli1 Feb 22, 2025

Choose a reason for hiding this comment

regulus79 Feb 22, 2025

Choose a reason for hiding this comment

szeli1 Feb 23, 2025

Choose a reason for hiding this comment

zonkmachine commented Feb 22, 2025

regulus79 commented Feb 22, 2025

zonkmachine commented Feb 23, 2025

bratpeki commented Feb 24, 2025

bratpeki commented Feb 26, 2025

bratpeki commented Feb 26, 2025

RainbowShatter commented Feb 26, 2025

regulus79 commented Feb 26, 2025

szeli1 commented Feb 26, 2025 • edited Loading

germona commented Feb 27, 2025

szeli1 left a comment

Choose a reason for hiding this comment

regulus79 commented Feb 27, 2025

germona commented Feb 28, 2025

szeli1 left a comment

Choose a reason for hiding this comment

bratpeki commented Mar 1, 2025

regulus79 commented Mar 1, 2025

bratpeki commented Mar 1, 2025

regulus79 commented Mar 1, 2025

regulus79 commented Mar 1, 2025

bratpeki commented Mar 1, 2025

regulus79 commented Mar 1, 2025

bratpeki commented Mar 1, 2025

regulus79 commented Mar 4, 2025

bratpeki commented Feb 21, 2025 •

edited

Loading

szeli1 commented Feb 26, 2025 •

edited

Loading