-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add splitting (and resizing) to all types of clips #7477
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
Changes from all commits
09fccac
e010f28
ff8a47d
8767dfd
fb2a222
70cf4dc
06b645e
4afdc87
c287cd0
0f92817
6108c5d
360a5d1
273520c
607e810
790a01f
1c45810
292e998
da2e97c
c1f2928
de01890
2995ba8
72356bf
ac8a1c4
19b6916
b2db31a
aa773a0
39a8bb3
a39be95
8f3955a
df5b49b
bf7d1f2
6d13da1
19837bc
f8b763c
7974445
650b8b4
858330a
e1269e7
aa655ec
037497d
3c858b5
e8c25f0
6df2daf
f0134ba
ccbb00d
690f841
c7c4ff4
599680a
bad83a0
959a99b
b0740b9
eb973df
781beea
51007bd
4e20e2b
cd895c3
7eab22f
a86f509
aa725ad
49cc5f7
37dd961
2321726
9c08dee
92363ab
bcb3521
6b2ef42
9ebd3ff
6a7d880
5d3e35e
3cdb590
38665bd
bbfa4ac
868b987
d3ca57b
c969158
e7faa8a
bf23323
43d2f18
1e7cfe7
1be3077
85935c6
abe7103
378c999
0658332
9c4308a
278ee11
93fb996
16b2dad
0eac1ad
3b98db5
b3cd3d9
e3aaaab
74b2afd
0581e62
88a3d7b
212eaca
c5d3ec6
9bfa48b
f865579
76bae6b
716783e
aa8041b
eb03363
d9b5673
a1aa7e7
870ce02
23f0c13
9475741
0d765d5
eadd040
b925db1
ea31c05
ef10305
e51b5f8
7c63007
b56391f
5bc9de8
d37ca59
e3a06af
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,22 +32,24 @@ | |
| namespace lmms | ||
| { | ||
|
|
||
| class InlineAutomation : public FloatModel, public sharedObject | ||
| class InlineAutomation : public FloatModel | ||
| { | ||
| public: | ||
| InlineAutomation() : | ||
| FloatModel(), | ||
| sharedObject(), | ||
| m_autoClip( nullptr ) | ||
| FloatModel() | ||
| { | ||
| } | ||
|
|
||
| InlineAutomation(const InlineAutomation& _copy) : | ||
| FloatModel(_copy.value(), _copy.minValue(), _copy.maxValue(), _copy.step<float>()), | ||
| m_autoClip(_copy.m_autoClip->clone()) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this allocating the automation clip on the heap each time? Is this called every time a note is placed?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably yes? But it is deleted when the note is deleted.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have made it so that
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm worried about how this will conflict with #7594 if we are changing the lifetime of our clips.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Would a simple fix be to add a separate call to Edit: I believe the only things which would need to be changed are the new copy constructors (make it so that they do not add themselves to a track), and then change the places they are used in so that they are manually added to a track. Fortunately, I believe the only places the copy constructors are used are in the |
||
| { | ||
| m_autoClip->clearObjects(); | ||
| m_autoClip->addObject(this); | ||
| } | ||
|
|
||
| ~InlineAutomation() override | ||
| { | ||
| if( m_autoClip ) | ||
| { | ||
| delete m_autoClip; | ||
| } | ||
| } | ||
|
|
||
| virtual float defaultValue() const = 0; | ||
|
|
@@ -81,18 +83,18 @@ class InlineAutomation : public FloatModel, public sharedObject | |
| { | ||
| if( m_autoClip == nullptr ) | ||
| { | ||
| m_autoClip = new AutomationClip( nullptr ); | ||
| m_autoClip = std::make_unique<AutomationClip>(nullptr); | ||
| m_autoClip->addObject( this ); | ||
| } | ||
| return m_autoClip; | ||
| return m_autoClip.get(); | ||
| } | ||
|
|
||
| void saveSettings( QDomDocument & _doc, QDomElement & _parent ) override; | ||
| void loadSettings( const QDomElement & _this ) override; | ||
|
|
||
|
|
||
| private: | ||
| AutomationClip * m_autoClip; | ||
| std::unique_ptr<AutomationClip> m_autoClip; | ||
|
|
||
| } ; | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@regulus79
PatternCliphas a Qt slot calledupdateLengththat isn't marked withoverride, and it's causing some warnings in the macOS builds:Should it be made a regular method (not a slot) and marked with
override?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right! I saw that warning come up a while back, and I was thinking of making a PR to fix it, but it was such a minor change, and it wasn't failing the builds so I sort of forgot about it.
The only issue with making it a normal method might be that it's used as a slot when setting up the connections in
SampleClip::SampleClipfor tempo and timesig changes. I wonder if it would still work if we kept it a slot and just addedoverrideto it in ? Or maybe we should make it aslotinClip.h?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know of anywhere else where we do that, so I think you should see if it will work as a regular method first. I'm pretty sure it will work, though I could be wrong.
I wouldn't