LMMS
Add splitting to all types of clips
Makes the knife tool work for all types of clips, not just SampleClips.
Changes
Most of the code is copied from SampleClipView.cpp's splitClip() function, and then modified for each clip type.
- A couple changes had to be made to
ClipView.cppto allow splitting more than justSampleClips, and to hold off on dragging clips that cannot be resized (MidiClips) when knife mode is enabled. - Splitting
PatternClips was trivial. - Splitting
AutomationClips andMidiClips was more involved, as becausesetStartTimeOffset()appears to do nothing, it required copying the correct nodes over and offsetting them back by the length of the left clip. However, the original clip still had all the notes, which meant that if the user changed some of them, its length would snap back to full. I had difficulty finding a good way to delete certain notes via a loop, so I decided to instead spawn two new clips, one left and one right, populate them with notes, and delete the original clip.
Notes
- In
AutomationClipView.cpp, for some reason when splitting an automation clip, the new clips sometimes sets themselves to record mode. I added a temporary fix for this by setting the recording mode to the mode of the original clip. - ~~Because
MidiClips are only drawn in multiples of 1 bar, splitting them between bars led to buggy graphics. Because of this, I forced the split position to be a multiple of a bar.~~ - I am not an expert in how to have objects properly remove themselves without memory issues. I called ~~
remove()~~close()at the end ofspltiClip()forAutomationClipView.cppandMidiClipView.cpp, so I hope that takes care of everything.
Here are some things I noticed while testing it:
- In Knife mode, the knife mouse icon doesn't appear when hovering over midi clips, automation clips, and pattern clips like it does for sample clips.
- Undo works incorrectly for midi clip and automation clip splits - the old singular clip returns, but the two split clips also remain.
- ~The snapping size is different for midi clips - I can't split into anything smaller than a full measure.~ Nevermind, midi clips don't work like that
- Double-clicking in Knife mode performs the same action as double-clicking in Draw mode. This problem already existed for sample tracks, but now also exists for other types of tracks too. Double-clicking shouldn't do anything special in Knife mode.
- When splitting midi clips, if the clip is cut down the middle of a note, the clip size of the resulting left clip is too small and hides the note that passed through the cut point. If the two new clips are ordered correctly, you should be able to make the right clip display over top of the left clip without needing to touch the size of the left clip.
Unrelated bug: In the Piano Roll, the mouse icon when in Pitch Bend mode can disappear if you click on a note, then close the note's automation editor window.
Other than the issues above, everything seems to work fine. I'm pretty excited about this feature and hope we can get it merged soon.
I think I have fixed most of the bugs, however I don't understand how journalling works enough to fix the bug where the new clips don't disappear after undoing. I will continue to look into it, but I may need some help.
Also, I decided to remove the restriction on midi clips needing to be split on the bar, and instead implemented your suggestion on letting the left clip's length be longer and extend behind the right clip. So now they can be split at arbitrary positions!
Ah, nevermind, it seems that using close() instead of remove() makes it work! I will have to look into why that happens.
@regulus79 It seems that when I split a midi clip, the original singular clip still remains though it is invisible. You can hear it when the song plays since it plays twice as loud as before the clip was split, and if you clone the track, it becomes visible in the cloned track.
Okay that's an interesting bug... I think it may have to do with me using close() instead of remove() when removing the old clip. But that's odd, since it doesn't appear to happen for AutomationClips which also use close() (???)
After some testing, it appears that using remove() solves the issue. I also think I found the cause of the old problem when using remove(), which is that I was calling it after m_clip->getTrack()->restoreJournallingState();. Again, I'm not an expert in Journalling, but calling it before appears to fix the problem.
I fixed the problem with out/in values and tangents in automation clip splitting.
I'm testing currently and found an issue:
Steps to reproduce:
- Open a new project
- Make a midi clip that is 2 bar long
- Place 3 notes, at 1 bar 1 beat, at 1 bar 4 beat 12 tick, and at 2 bar 3 beat, the 2. one should be wide enough so its end is in the 2. bar
- Click on the knife tool
- Make a cut exactly at 1 bar 4 beat 24 tick by pressing ctrl + clicking The result is 2 clips, the left one is larger and behind the right one. I know (or at least I think) working with TimePos can be difficult, however I think this Issue should / could be fixed.
I think patterns work great, and AutomationClips still need work. If an automation clip is split ant the left side is edited, then it will snap back to the original size.
The result is 2 clips, the left one is larger and behind the right one. I know (or at least I think) working with TimePos can be difficult, however I think this Issue should / could be fixed.
This was considered in previous iterations of the pr, but I believe the current functionality is correct.
The odd behavior you are experiencing is due to the fact that midi clips automatically round their length up to a whole bar. If a note is cut partway through a note which crosses a bar, in order to maintain identical sounds before and after the split, it must let the note extend past the bar in the new left clip which can cause the new length to round back up to two bars. I agree that it does seem odd that the left clip can sometimes be extend further than the right clip, but that is how midi clips currently work in lmms. Additionally, the right clip must start at the split pos so that the split make sense, which means the right clip's length is determined by it's local position on the timeline relative to the split pos, which means it can sometimes not round up to meet a global bar, and thus sometimes it can appear shorter than the left clip in rare circumstances.
If you would like me to change midi clips to have arbitrary sizes, just lmk! I would be happy to do so.
If an automation clip is split ant the left side is edited, then it will snap back to the original size.
Whoops! I forgot about that. I may have to think about what the best solution here is.
Given my previous discussion with messmerd, and how I concluded that adding left-resizing support to AutomationClips would be the best option to accurately split cubic bezier clips, I think it may be beneficial to keep the current setup but simply disable automatic resizing if the automation clips has already been resized once. Or we could revert back to the old setup of erasing the nodes to the right of the split point, but as previously mentioned, this causes cubic bezier clips to be incorrectly split.
I was already planning on doing some work on the automation editor to more clearly convey to the user where the clip starts, so perhaps I can implement a fix for this when I do that.
Testing this.
I'm gonna try to make a TL;DR, correct me if I'm wrong.
As of now, you can only slice sample tracks. This implements MIDI and automation clip slicing. MIDI slicing is completely done. Automation slicing is done for discrete and linear automation, and there is discussion on how the Bezier one is supposed to be implemented. The options are:
- Disable it
- Enable it, approximate
- Enable it, do something called "left resizing" (?)
Am I missing anything? And what is this third option? Also, how are the approximations done?
Slicing a clip like this along the red line gets you to this:
I believe the MIDI note ends should be cut on that red point. Opinions?
Operation:
Result (visualized in two tracks, not actually like this in LMMS):
Potentially better result?
Another possibility, brought up by Punch:
This might really be a "pick one" situation, since making the user pick with a dropbox or similar would result in "slicing fatigue", LOL! On the other hand, a potential toggle button or something like that could also resolve this.
If we were to pick one and one only, the option brough up by Punch is definitely my favorite.
You slice, if you need to take parts out, you open the clip and fix it, or make another cut and remove that new slice alltogether. Seems to also be what most other DAWs are doing, as Kam demonstrated for Reaper in the Discord server.
As a reminder to myself, I should test splitting and rejoining, to make sure it works as intented.
As of now, you can only slice sample tracks. This implements MIDI and automation clip slicing.
Yes, and this PR also implements pattern clip splitting.
MIDI slicing is completely done.
Technically yes, though after reading Spekular's comment on the discord about the future goal of making all clips resizeable and simplifying splitting, midi clip splitting may be reworked, either now or in a future PR.
Automation slicing is done for discrete and linear automation, and there is discussion on how the Bezier one is supposed to be implemented. The options are:
- Disable it
- Enable it, approximate
- Enable it, do something called "left resizing" (?)
Those options are accurate, and this PR currently implements the third one.
Am I missing anything? And what is this third option?
So you know how sample clips are split? The actual sample data does not change, only the bounds of the clip change to "window" the correct part of the sample. If you have a sample 4 bars long and you split it in the middle, the two new clips created both reference the same sample data, but the first one starts at bar 0 and ends at bar 2, while the second one starts at bar 2 and ends at bar 4. You could also manually do this by duplicating the sample clip, and resizing them to be like that. But this requires the ability to resize the sample clip from both the beginning and the end. The first clip needs to be made shorter (resize from the right), while the second clip needs to be made shorter in the reverse direction (resize from the left). This is why left-resizing is necessary for this kind of splitting where the internal data is not changed, only the bounds of the clip.
The same method can be used to split automation clips. The actual automation curve data is not changed. Both new clips contain their own copy. To make a split, the bounds of the clips are changed in the same way as the sample clips.
Also, how are the approximations done?
For example, let's say you have a linear automation curve, going from 0 at bar 0 to 1 at bar 2, and you split it at bar 1. If we weren't using the previously described method and instead wanted to separate all the curve data into the two clips, we would have to put the first automation node in the first clip, and the second automation node in the second clip. But then each clip would contain a single point, and together they wouldn't make the original linear curve we had. To fix this, we need to add a third, middle point at bar 1, with the value 0.5. This gets copied to both clips, at the end of the first clip and at the start of the second clip. Now both clips have two points, and they make up a nice continuous line, just like we had before the split!
That works for linear and stepped automation curves. And it should work for cubic curves too, right? You just add a new node in the middle where it got split, and it should make everything work. But alas, due to LMMS's implementation of cubic hermite curves, it means that adding a new point, even if that point is exactly where the curve was, with the right tangents and everything, it cannot perfectly reproduce the original curve. Try it out by taking a cubic curve and adding a new node in the middle, right on the curve. It changes slightly.
I believe the MIDI note ends should be cut on that red point. Opinions?
Another possibility, brought up by Punch:
I see why these possibilities would make sense, but I'm not sure I'm on board with them. In particular, they change the way the song sounds.
Currently, splitting and merging clips do not change the output audio, only the gui. I am able to play a song, and while playing it split up a bunch of the midi clips without any changes occurring in the music.
The reason I was pushing for the third option when splitting automation clips was so that it would be precise. No changes occur in the automation curves due to splitting, unless you yourself decide to move one of the resulting clips. The same is true for sample clips and pattern clips. To my knowledge, the splitting is perfect. Shouldn't the same be true for midi clips?
I'm not super dedicated to either side, and to be honest I do see the usefulness of being able to split a long midi note into a bunch of shorter ones. And if other daws do it like this, then... idk.
Oh, I see, for automation clips you create two instances with bounds [start - slice position] and [slice position - end]. Makes perfect sense! I will test to make sure you can edit them individually without them affecting each other.
As for the MIDI slicing, I understand your concerns about changing how the song sounds! We'll see.
Pattern slicing sounds the same in both instances, so I'd say that's pretty good. I'll do some more testing later, when we agree on the MIDI slicing method.
After reading Spekular's comment on the discrod, and in an attempt to make all clips act similarly, I have implemented midi clip resizing into this PR. This means that midi clip splitting now works in the same manner as automation clip splitting. If this is not how the other devs intended clip splitting to function, let me know, and I can revert this change.
This also means that the piano roll, in addition to the automation editor, will have to be updated to convey the bounds of the current clip with respect to the notes. Spekular suggested adding a kind of overlay/darkening on the parts of the clip which are not visible.
I still have to make it so that NotePlayHandles which continue through the end of a midi clip are somehow cut off (if that is what we want to do?). I anyone wants, I could also make it so that notes which start before the clip start are made to start at the clip boundary.
I also plan to add a feature allowing users to delete all notes outside of the clip bounds, as a way to essentially turn a resized midi clip into a more "traditional" midi clip which some people may find easier to edit.
I have implemented Spekular's idea of having an overlay in the Piano Roll and/or Automation Editor on the parts of the timeline which are outside of the clip bounds to convey the size of the clip to users.
As a part of that, I moved the lambda xCoordOfTick from PianoRoll::paintEvent out of its scope in one of the if statements, down to the function level, that way I didn't have to redefine it when drawing the overlay. I also noticed that the lambda was being defined twice in that function in different scopes, so I deleted the second copy now that the whole function has access to the lambda.
This is in a working state, and should be ready to test.
Please be on the lookout for segfaults. I experienced a few while messing with clip splitting and trimming, (the buffer being passed to fluidsynth was a nullptr) which may have been due to my changes in NotePlayHandle. However, after I decided not to include clip trimming into this pr, I haven't been able to reproduce them.
~~I would love to hear your input on how playback should work in the PianoRoll. Currently, playback starts from the beginning of the notes no matter if the start offset is nonzero, but should it instead start from the beginning of the visible window? Should I also make it so that notes overlapping with the clip bounds in the piano roll get cut off/still play, just like in the song editor?~~ Implemented.
https://github.com/user-attachments/assets/285b5ec4-55fa-4329-9327-3368a51e0ce5
Noticed this. I'd make the clips resize if a new note is added, in general, since it retains the original behaviour of the MIDI clip, making it more natural to the end user.
I'd make the clips resize if a new note is added
The reason I made automatic resizing disabled after the clip has been manually resized is so that the user can take a large clip, resize it down, and edit it without it automatically snapping back to it's original size whenever they move a note. My thinking was that if the user has gone through the effort to resize a clip, they probably want it to stay like that.
Are you suggesting that the clip should still resize itself when a note is placed, but only if that note is out of bounds? That may work.
Maybe the note addition should work by expanding the MIDI clip lenght to the full bar, much like it does now. Or potentially to the length specified by the quantization measure. I'd think the first option is both easier to implement and more akin to the MIDI clip UX we have now, so I see it as the preferred method for handling this.
As for the expansion, it can either:
- keep the notes and just expand the clip length, which I think is perfectly fine, or
- be clear, like it is now when you add a new note
The first option is, same as the previous suggestion, less code and overall simpler. This does mean that adding, for example, an option to make the clip cut be a clip containing only the notes in the cut section would also be a neat feature. Just a simple right click option on the MIDI clip that turns this:
into this:
It would mean that you could make a MIDI clip from some other clip by:
- Creating a copy
- Selecting what you need
- Using the right click option, named something like "Keep only this section"
- Get that section, ready for being used for something else
Thanks for the quick reply!
The reason I made automatic resizing disabled after the clip has been manually resized is so that the user can take a large clip, resize it down, and edit it without it automatically snapping back to it's original size whenever they move a note. My thinking was that if the user has gone through the effort to resize a clip, they probably want it to stay like that.
Yeah, that makes a lot of sense, actually!
Are you suggesting that the clip should still resize itself when a note is placed, but only if that note is out of bounds? That may work.
That is exactly what I'm suggesting, yeah. It stays similar to what we have now, and thus isn't working against the user's muscle memory.
That is exactly what I'm suggesting, yeah. It stays similar to what we have now, and thus isn't working against the user's muscle memory.
Yes, though I'm not sure. What if the user is editing a small chunk of a long midi clip which they cut out using the knife tool, and they accidentally move one of the notes a little past the end while they are editing it? Then they would have to go back to the song editor and resize it back.
It would mean that you could make a MIDI clip from some other clip by:
- Creating a copy
- Selecting what you need
- Using the right click option, named something like "Keep only this section"
- Get that section, ready for being used for something else
I was actually planning to implement a feature exactly like this, but I paused due to some errors I was encountering. However, I think I can take another try at it.
I think this would be a kind of "best of both worlds", where you can turn resized clips back into ordinary clips.