Modified Sample and Automation Tracks so that new clips match the Snapping Size#8267
Modified Sample and Automation Tracks so that new clips match the Snapping Size#8267AbbyBuchholz wants to merge 5 commits intoLMMS:masterfrom
Conversation
… the" This reverts commit 20c9f04.
AbbyBuchholz
changed the title
regulus79
left a comment
regulus79
left a comment
There was a problem hiding this comment.
This review was super rushed, so I just wrote what I noticed/came to mind.
It appears this PR also prevents users from adding clips which start on top of other clips, which was not described in the PR description(?) (But I totally agree, that is a good feature. It is kind of weird how you can sometimes add an empty clip on another when you mean to click it or something.)
|
|
||
| #include "AutomationEditor.h" | ||
| #include "AutomationClip.h" | ||
| #include "embed.h" |
There was a problem hiding this comment.
Why is "embed.h" necessary to be included here?
|
|
||
| const tick_t snapTicks = static_cast<tick_t>(snapSize * TimePos::ticksPerBar()); | ||
|
|
||
| return std::max(snapTicks, static_cast<tick_t>(TimePos::ticksPerBar() / 16)); |
There was a problem hiding this comment.
Why is the minimum clip size ticksPerBar() / 16?
| return m_startTimeOffset; | ||
| } | ||
|
|
||
| tick_t Clip::getDefaultClipLength() |
There was a problem hiding this comment.
Since this function does not depend on anything regarding clips, only the editor, would it make sense to move it to TrackContainer?
| { | ||
| if (pos < existingClip->endPosition() && endPos > existingClip->startPosition()) | ||
| { | ||
| return nullptr; |
There was a problem hiding this comment.
I'm concerned about returning nullptr here, since TrackContentWidget.cpp line 541 appears to call functions on the returned clip without checking if it's nullptr or not, when pasting clips.
| auto p = new AutomationClip(this); | ||
| p->movePosition(pos); | ||
| return p; | ||
| const TimePos endPos = pos + TimePos(Clip::getDefaultClipLength()); |
There was a problem hiding this comment.
If Clip::getDefaultClipLength were moved to TrackContainer, I believe you could do m_trackContainer->defaultClipLength(), so you also wouldn't need to include Clip.h.
|
Getting it to work for |
2 participants
Worked on issue #8038, added two helper functions (autoResize, getDefaultClipLength) for sizing to Clip.cpp and modified SampleTrack.cpp and AutomationTrack.cpp's createClip functions to use it, thus allowing default snapping sized clips for both editors.