Skip to content
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

Add clips to tracks and tracks to track containers only after the constructor completes #7594

Draft
wants to merge 34 commits into
base: master
Choose a base branch
from
Draft
Changes from 1 commit
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
4e9cab1
Delay adding tracks to track containers until the constructor completes
sakertooth Dec 7, 2024
532ac1b
Make addTrack a templated function
sakertooth Dec 8, 2024
548314d
Make addClip a templated function
sakertooth Dec 8, 2024
4405910
Move Clip construction code that requires a parent track to an update…
sakertooth Dec 8, 2024
8b52a93
Remove track parameter in Clip constructor
sakertooth Dec 8, 2024
f20d308
Remove track container parameter in Track constructor
sakertooth Dec 8, 2024
4764b04
Remove Track::create declarations
sakertooth Dec 8, 2024
e4a7619
Set track container to nullptr when creating tracks
sakertooth Dec 8, 2024
a0fa59f
Rename addTrack(const QDomElement&) to createTrack(const QDomElement&)
sakertooth Dec 8, 2024
46ba381
Add canAddClip function to avoid coding errors (adding a clip to a ty…
sakertooth Dec 8, 2024
ee02496
Add LMMS_EXPORT to AutomationTrack and SampleTrack declarations
sakertooth Dec 8, 2024
949f91d
Delay updating pattern store on construction of PatternTrack using Qt…
sakertooth Dec 8, 2024
eb1ad3d
Add requestChangesGuard in Track::addClip
sakertooth Dec 16, 2024
5cbe7d6
Make onAddedToTrack pure virtual
sakertooth Dec 16, 2024
31043c4
Remove concepts TODO comment
sakertooth Feb 2, 2025
5eb1283
Remove calls to movePosition(0)
sakertooth Feb 2, 2025
719b57f
Move track dependent initialization to onAddedToTrack
sakertooth Feb 2, 2025
7f5875a
Manage ownership of clips using std::unique_ptr's
sakertooth Feb 3, 2025
8549bee
Make sure to add clip when created
sakertooth Feb 3, 2025
b590ccf
Fix crash when deleting clips
sakertooth Feb 3, 2025
77aea52
Simplify repeated actions of adding and creating a new clip
sakertooth Feb 3, 2025
9b78d49
Rename addTrack to addNewTrack
sakertooth Feb 3, 2025
59dca59
Manage ownership of tracks with std::unique_ptr's
sakertooth Feb 3, 2025
289b113
Replace addSampleTrack, addAutomationTrack, and addPatternTrack with …
sakertooth Feb 3, 2025
e094224
Prevent misuse by only allowing adding new clips to tracks
sakertooth Feb 3, 2025
45ff4ae
Rename createTrack to addNewTrack
sakertooth Feb 3, 2025
cd7041e
Remove destroyedClip signal
sakertooth Feb 3, 2025
a53fd94
Remvoe destroyedTrack signal
sakertooth Feb 3, 2025
2496637
Remove forward declartion of Clip
sakertooth Feb 3, 2025
b6beb52
Remove updateAfterTrackAdd
sakertooth Feb 3, 2025
8f49a88
Replace signal slot connection for updating pattern store when adding…
sakertooth Feb 3, 2025
4b2328d
Make defining onAddedToTrack optional
sakertooth Feb 3, 2025
eb4fa7c
Fix segfaults when deleting clips and tracks
sakertooth Feb 3, 2025
b59b242
Fix deadlocks when removing tracks
sakertooth Feb 3, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Remove track parameter in Clip constructor
sakertooth committed Dec 8, 2024
commit 8b52a93cf4709b45336469cddc6db52e7ce64105
3 changes: 1 addition & 2 deletions include/AutomationClip.h
Original file line number Diff line number Diff line change
@@ -67,7 +67,7 @@ class LMMS_EXPORT AutomationClip : public Clip

using TimemapIterator = timeMap::const_iterator;

AutomationClip( AutomationTrack * _auto_track );
AutomationClip();
AutomationClip( const AutomationClip & _clip_to_copy );
~AutomationClip() override = default;

@@ -226,7 +226,6 @@ public slots:
mutable QMutex m_clipMutex;
#endif

AutomationTrack * m_autoTrack;
std::vector<jo_id_t> m_idsToResolve;
objectVector m_objects;
timeMap m_timeMap; // actual values
4 changes: 3 additions & 1 deletion include/Clip.h
Original file line number Diff line number Diff line change
@@ -53,7 +53,7 @@ class LMMS_EXPORT Clip : public Model, public JournallingObject
mapPropertyFromModel(bool,isMuted,setMuted,m_mutedModel);
mapPropertyFromModel(bool,isSolo,setSolo,m_soloModel);
public:
Clip( Track * track );
Clip();
~Clip() override;

virtual void onAddedToTrack(Track* track) {}
@@ -63,6 +63,8 @@ class LMMS_EXPORT Clip : public Model, public JournallingObject
return m_track;
}

void setTrack(Track* track) { m_track = track; }

inline const QString & name() const
{
return m_name;
2 changes: 1 addition & 1 deletion include/InlineAutomation.h
Original file line number Diff line number Diff line change
@@ -81,7 +81,7 @@ class InlineAutomation : public FloatModel, public sharedObject
{
if( m_autoClip == nullptr )
{
m_autoClip = new AutomationClip( nullptr );
m_autoClip = new AutomationClip();
m_autoClip->addObject( this );
}
return m_autoClip;
11 changes: 3 additions & 8 deletions include/MidiClip.h
Original file line number Diff line number Diff line change
@@ -27,15 +27,12 @@
#define LMMS_MIDI_CLIP_H

#include "Clip.h"
#include "InstrumentTrack.h"
#include "Note.h"


namespace lmms
{


class InstrumentTrack;

namespace gui
{
class MidiClipView;
@@ -52,7 +49,7 @@ class LMMS_EXPORT MidiClip : public Clip
MelodyClip
} ;

MidiClip( InstrumentTrack* instrumentTrack );
MidiClip();
MidiClip( const MidiClip& other );
~MidiClip() override;

@@ -104,7 +101,7 @@ class LMMS_EXPORT MidiClip : public Clip

inline InstrumentTrack * instrumentTrack() const
{
return m_instrumentTrack;
return static_cast<InstrumentTrack*>(getTrack());
}

bool empty();
@@ -136,8 +133,6 @@ protected slots:

void resizeToFirstTrack();

InstrumentTrack * m_instrumentTrack;

Type m_clipType;

// data-stuff
2 changes: 1 addition & 1 deletion include/PatternClip.h
Original file line number Diff line number Diff line change
@@ -37,7 +37,7 @@ namespace lmms
class PatternClip : public Clip
{
public:
PatternClip(Track* track);
PatternClip();
~PatternClip() override = default;

void onAddedToTrack(Track* track) override;
4 changes: 2 additions & 2 deletions include/SampleClip.h
Original file line number Diff line number Diff line change
@@ -47,8 +47,8 @@ class SampleClip : public Clip
Q_OBJECT
mapPropertyFromModel(bool,isRecord,setRecord,m_recordModel);
public:
SampleClip(Track* track, Sample sample, bool isPlaying);
SampleClip(Track* track);
SampleClip(Sample sample, bool isPlaying);
SampleClip();
SampleClip( const SampleClip& orig );
~SampleClip() override;

1 change: 1 addition & 0 deletions include/Track.h
Original file line number Diff line number Diff line change
@@ -124,6 +124,7 @@ class LMMS_EXPORT Track : public Model, public JournallingObject
static_assert(std::is_base_of_v<Clip, T>, "T must be a kind of Clip");
auto clip = new T(std::forward<Args>(args)...);
m_clips.push_back(clip);
clip->setTrack(this);
clip->onAddedToTrack(this);
emit clipAdded(clip);
return clip;
6 changes: 3 additions & 3 deletions plugins/MidiImport/MidiImport.cpp
Original file line number Diff line number Diff line change
@@ -337,10 +337,10 @@ bool MidiImport::readSMF( TrackContainer* tc )
auto dt = Engine::getSong()->addTrack<AutomationTrack>(Engine::getSong());

dt->setName(tr("MIDI Time Signature Denominator"));
auto timeSigNumeratorPat = new AutomationClip(nt);
auto timeSigNumeratorPat = static_cast<AutomationClip*>(nt->createClip());
timeSigNumeratorPat->setDisplayName(tr("Numerator"));
timeSigNumeratorPat->addObject(&timeSigMM.numeratorModel());
auto timeSigDenominatorPat = new AutomationClip(dt);
auto timeSigDenominatorPat = static_cast<AutomationClip*>(dt->createClip());
timeSigDenominatorPat->setDisplayName(tr("Denominator"));
timeSigDenominatorPat->addObject(&timeSigMM.denominatorModel());

@@ -365,7 +365,7 @@ bool MidiImport::readSMF( TrackContainer* tc )
// Tempo stuff
auto tt = Engine::getSong()->addTrack<AutomationTrack>(Engine::getSong());
tt->setName(tr("Tempo"));
auto tap = new AutomationClip(tt);
auto tap = static_cast<AutomationClip*>(tt->createClip());
tap->setDisplayName(tr("Tempo"));
tap->addObject(&Engine::getSong()->tempoModel());
if( tap )
23 changes: 4 additions & 19 deletions src/core/AutomationClip.cpp
Original file line number Diff line number Diff line change
@@ -45,12 +45,11 @@ const float AutomationClip::DEFAULT_MIN_VALUE = 0;
const float AutomationClip::DEFAULT_MAX_VALUE = 1;


AutomationClip::AutomationClip( AutomationTrack * _auto_track ) :
Clip( _auto_track ),
AutomationClip::AutomationClip() :
Clip(),
#if (QT_VERSION < QT_VERSION_CHECK(5,14,0))
m_clipMutex(QMutex::Recursive),
#endif
m_autoTrack( _auto_track ),
m_objects(),
m_tension( 1.0 ),
m_progressionType( ProgressionType::Discrete ),
@@ -65,11 +64,10 @@ AutomationClip::AutomationClip( AutomationTrack * _auto_track ) :


AutomationClip::AutomationClip( const AutomationClip & _clip_to_copy ) :
Clip( _clip_to_copy.m_autoTrack ),
Clip(),
#if (QT_VERSION < QT_VERSION_CHECK(5,14,0))
m_clipMutex(QMutex::Recursive),
#endif
m_autoTrack( _clip_to_copy.m_autoTrack ),
m_objects( _clip_to_copy.m_objects ),
m_tension( _clip_to_copy.m_tension ),
m_progressionType( _clip_to_copy.m_progressionType )
@@ -86,19 +84,6 @@ AutomationClip::AutomationClip( const AutomationClip & _clip_to_copy ) :
// Sets the node's clip to this one
m_timeMap[POS(it)].setClip(this);
}
if (!getTrack()){ return; }
switch( getTrack()->trackContainer()->type() )
{
case TrackContainer::Type::Pattern:
setAutoResize( true );
break;

case TrackContainer::Type::Song:
// move down
default:
setAutoResize( false );
break;
}
}

bool AutomationClip::addObject( AutomatableModel * _obj, bool _search_dup )
@@ -1044,7 +1029,7 @@ AutomationClip * AutomationClip::globalAutomationClip(
}
}

auto a = new AutomationClip(t);
auto a = static_cast<AutomationClip*>(t->createClip());
a->addObject( _m, false );
return a;
}
13 changes: 3 additions & 10 deletions src/core/Clip.cpp
Original file line number Diff line number Diff line change
@@ -35,16 +35,9 @@

namespace lmms
{

/*! \brief Create a new Clip
*
* Creates a new clip for the given track.
*
* \param _track The track that will contain the new object
*/
Clip::Clip( Track * track ) :
Model( track ),
m_track( track ),
Clip::Clip() :
Model(nullptr),
m_track(nullptr),
m_startPosition(),
m_length(),
m_mutedModel( false, this, tr( "Mute" ) ),
4 changes: 2 additions & 2 deletions src/core/PatternClip.cpp
Original file line number Diff line number Diff line change
@@ -35,8 +35,8 @@ namespace lmms
{


PatternClip::PatternClip(Track* track) :
Clip(track)
PatternClip::PatternClip() :
Clip()
{
setAutoResize( false );
}
10 changes: 5 additions & 5 deletions src/core/SampleClip.cpp
Original file line number Diff line number Diff line change
@@ -38,8 +38,8 @@
namespace lmms
{

SampleClip::SampleClip(Track* _track, Sample sample, bool isPlaying)
: Clip(_track)
SampleClip::SampleClip(Sample sample, bool isPlaying)
: Clip()
, m_sample(std::move(sample))
, m_isPlaying(false)
{
@@ -66,13 +66,13 @@ SampleClip::SampleClip(Track* _track, Sample sample, bool isPlaying)
connect( this, SIGNAL(positionChanged()), this, SLOT(updateTrackClips()));
}

SampleClip::SampleClip(Track* track)
: SampleClip(track, Sample(), false)
SampleClip::SampleClip()
: SampleClip(Sample(), false)
{
}

SampleClip::SampleClip(const SampleClip& orig) :
SampleClip(orig.getTrack(), orig.m_sample, orig.m_isPlaying)
SampleClip(orig.m_sample, orig.m_isPlaying)
{
}

2 changes: 1 addition & 1 deletion src/tracks/AutomationTrack.cpp
Original file line number Diff line number Diff line change
@@ -58,7 +58,7 @@ gui::TrackView* AutomationTrack::createView( gui::TrackContainerView* tcv )

Clip* AutomationTrack::createClip()
{
return addClip<AutomationClip>(this);
return addClip<AutomationClip>();
}


2 changes: 1 addition & 1 deletion src/tracks/InstrumentTrack.cpp
Original file line number Diff line number Diff line change
@@ -805,7 +805,7 @@ bool InstrumentTrack::play( const TimePos & _start, const fpp_t _frames,

Clip* InstrumentTrack::createClip()
{
return addClip<MidiClip>(this);
return addClip<MidiClip>();
}


18 changes: 8 additions & 10 deletions src/tracks/MidiClip.cpp
Original file line number Diff line number Diff line change
@@ -39,9 +39,8 @@
namespace lmms
{

MidiClip::MidiClip( InstrumentTrack * _instrument_track ) :
Clip( _instrument_track ),
m_instrumentTrack( _instrument_track ),
MidiClip::MidiClip() :
Clip(),
m_clipType( Type::BeatClip ),
m_steps( TimePos::stepsPerBar() )
{
@@ -52,8 +51,7 @@ MidiClip::MidiClip( InstrumentTrack * _instrument_track ) :


MidiClip::MidiClip( const MidiClip& other ) :
Clip( other.m_instrumentTrack ),
m_instrumentTrack( other.m_instrumentTrack ),
Clip(),
m_clipType( other.m_clipType ),
m_steps( other.m_steps )
{
@@ -83,14 +81,14 @@ void MidiClip::resizeToFirstTrack()
{
// Resize this track to be the same as existing tracks in the pattern
const TrackContainer::TrackList & tracks =
m_instrumentTrack->trackContainer()->tracks();
getTrack()->trackContainer()->tracks();
for (const auto& track : tracks)
{
if (track->type() == Track::Type::Instrument)
{
if (track != m_instrumentTrack)
if (track != getTrack())
{
const auto& instrumentTrackClips = m_instrumentTrack->getClips();
const auto& instrumentTrackClips = getTrack()->getClips();
const auto currentClipIt = std::find(instrumentTrackClips.begin(), instrumentTrackClips.end(), this);
unsigned int currentClip = currentClipIt != instrumentTrackClips.end() ?
std::distance(instrumentTrackClips.begin(), currentClipIt) : -1;
@@ -453,8 +451,8 @@ MidiClip * MidiClip::nextMidiClip() const

MidiClip * MidiClip::adjacentMidiClipByOffset(int offset) const
{
auto& clips = m_instrumentTrack->getClips();
int clipNum = m_instrumentTrack->getClipNum(this) + offset;
auto& clips = getTrack()->getClips();
int clipNum = getTrack()->getClipNum(this) + offset;
if (clipNum < 0 || static_cast<size_t>(clipNum) >= clips.size()) { return nullptr; }
return dynamic_cast<MidiClip*>(clips[clipNum]);
}
2 changes: 1 addition & 1 deletion src/tracks/PatternTrack.cpp
Original file line number Diff line number Diff line change
@@ -146,7 +146,7 @@ gui::TrackView* PatternTrack::createView(gui::TrackContainerView* tcv)

Clip* PatternTrack::createClip()
{
return addClip<PatternClip>(this);
return addClip<PatternClip>();
}


2 changes: 1 addition & 1 deletion src/tracks/SampleTrack.cpp
Original file line number Diff line number Diff line change
@@ -180,7 +180,7 @@ gui::TrackView * SampleTrack::createView( gui::TrackContainerView* tcv )

Clip * SampleTrack::createClip()
{
return addClip<SampleClip>(this);
return addClip<SampleClip>();
}


4 changes: 2 additions & 2 deletions tests/src/tracks/AutomationTrackTest.cpp
Original file line number Diff line number Diff line change
@@ -59,7 +59,7 @@ private slots:
{
using namespace lmms;

AutomationClip c(nullptr);
AutomationClip c;
c.setProgressionType(AutomationClip::ProgressionType::Linear);
c.putValue(0, 0.0, false);
c.putValue(100, 1.0, false);
@@ -76,7 +76,7 @@ private slots:
{
using namespace lmms;

AutomationClip c(nullptr);
AutomationClip c;
c.setProgressionType(AutomationClip::ProgressionType::Discrete);
c.putValue(0, 0.0, false);
c.putValue(100, 1.0, false);