Skip to content

Commit

Permalink
Fixes createTCO method on some classes
Browse files Browse the repository at this point in the history
	Classes that inherit from "Track", also inherit the createTCO method. That method takes a MidiTime position as an argument, but except on the class SampleTrack that argument was ignored. That lead to unnecessary calls to TCO->movePosition after creating a TCO in many parts of the codebase (making the argument completely redundant) and even to a bug on the BBEditor, caused by a call to createTCO that expected the position to be set on the constructor (see issue LMMS#5673).

	That PR adds code to move the TCO to the appropriate position inside the constructor of the classes that didn't have it, fixes the code style on the SampleTrack createTCO method and removes the now unneeded calls to movePosition from source files on src/ and plugins/. On Track::loadSettings there was a call to saveJournallingState(false) followed immediately by restoreJournallingState() which was deleted because it's redundant (probably a left over from copying the code from pasteSelection?).
  • Loading branch information
IanCaio committed Oct 4, 2020
1 parent 8939b14 commit 7e10026
Show file tree
Hide file tree
Showing 7 changed files with 19 additions and 24 deletions.
4 changes: 1 addition & 3 deletions plugins/HydrogenImport/HydrogenImport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -314,10 +314,8 @@ bool HydrogenImport::readSong()

int i = pattern_id[patId]+song_num_tracks;
Track *t = ( BBTrack * ) s->tracks().at( i );
TrackContentObject *tco = t->createTCO( pos );
tco->movePosition( pos );
t->createTCO( pos );


if ( pattern_length[patId] > best_length )
{
best_length = pattern_length[patId];
Expand Down
6 changes: 2 additions & 4 deletions plugins/MidiImport/MidiImport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,7 @@ class smfMidiCC
{
MidiTime pPos = MidiTime( time.getBar(), 0 );
ap = dynamic_cast<AutomationPattern*>(
at->createTCO(0) );
ap->movePosition( pPos );
at->createTCO( pPos ) );
ap->addObject( objModel );
}

Expand Down Expand Up @@ -287,8 +286,7 @@ class smfMidiChannel
if (!newPattern || n->pos() > lastEnd + DefaultTicksPerBar)
{
MidiTime pPos = MidiTime(n->pos().getBar(), 0);
newPattern = dynamic_cast<Pattern*>(it->createTCO(0));
newPattern->movePosition(pPos);
newPattern = dynamic_cast<Pattern*>(it->createTCO( pPos ));
}
lastEnd = n->pos() + n->length();

Expand Down
11 changes: 2 additions & 9 deletions src/core/Track.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1860,7 +1860,7 @@ bool TrackContentWidget::pasteSelection( MidiTime tcoPos, const QMimeData * md,

TrackContentObject * tco = t->createTCO( pos );
tco->restoreState( tcoElement );
tco->movePosition( pos );
tco->movePosition( pos ); // Because we restored the state, we need to move the TCO again.
if( wasSelection )
{
tco->selectViewOnCreate( true );
Expand Down Expand Up @@ -1914,11 +1914,7 @@ void TrackContentWidget::mousePressEvent( QMouseEvent * me )
getTrack()->addJournalCheckPoint();
const MidiTime pos = getPosition( me->x() ).getBar() *
MidiTime::ticksPerBar();
TrackContentObject * tco = getTrack()->createTCO( pos );

tco->saveJournallingState( false );
tco->movePosition( pos );
tco->restoreJournallingState();
getTrack()->createTCO( pos );
}
}

Expand Down Expand Up @@ -2570,8 +2566,6 @@ void Track::loadSettings( const QDomElement & element )
TrackContentObject * tco = createTCO(
MidiTime( 0 ) );
tco->restoreState( node.toElement() );
saveJournallingState( false );
restoreJournallingState();
}
}
node = node.nextSibling();
Expand Down Expand Up @@ -2759,7 +2753,6 @@ void Track::createTCOsForBB( int bb )
{
MidiTime position = MidiTime( numOfTCOs(), 0 );
TrackContentObject * tco = createTCO( position );
tco->movePosition( position );
tco->changeLength( MidiTime( 1, 0 ) );
}
}
Expand Down
7 changes: 4 additions & 3 deletions src/tracks/AutomationTrack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,11 @@ TrackView * AutomationTrack::createView( TrackContainerView* tcv )



TrackContentObject * AutomationTrack::createTCO( const MidiTime & )
TrackContentObject * AutomationTrack::createTCO( const MidiTime & _pos )
{
return new AutomationPattern( this );
AutomationPattern *p = new AutomationPattern( this );
p->movePosition( _pos );
return p;
}


Expand Down Expand Up @@ -133,7 +135,6 @@ void AutomationTrackView::dropEvent( QDropEvent * _de )
TrackContentObject * tco = getTrack()->createTCO( pos );
AutomationPattern * pat = dynamic_cast<AutomationPattern *>( tco );
pat->addObject( mod );
pat->movePosition( pos );
}
}

Expand Down
3 changes: 3 additions & 0 deletions src/tracks/BBTrack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,9 @@ TrackContentObject * BBTrack::createTCO( const MidiTime & _pos )
bbtco->setColor( *s_lastTCOColor );
bbtco->setUseStyleColor( false );
}

bbtco->movePosition( _pos );

return bbtco;
}

Expand Down
6 changes: 4 additions & 2 deletions src/tracks/InstrumentTrack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -713,9 +713,11 @@ bool InstrumentTrack::play( const MidiTime & _start, const fpp_t _frames,



TrackContentObject * InstrumentTrack::createTCO( const MidiTime & )
TrackContentObject * InstrumentTrack::createTCO( const MidiTime & _pos )
{
return new Pattern( this );
Pattern *p = new Pattern( this );
p->movePosition( _pos );
return p;
}


Expand Down
6 changes: 3 additions & 3 deletions src/tracks/SampleTrack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -746,10 +746,10 @@ TrackView * SampleTrack::createView( TrackContainerView* tcv )



TrackContentObject * SampleTrack::createTCO(const MidiTime & pos)
TrackContentObject * SampleTrack::createTCO(const MidiTime & _pos)
{
SampleTCO * sTco = new SampleTCO(this);
sTco->movePosition(pos);
SampleTCO * sTco = new SampleTCO( this );
sTco->movePosition( _pos );
return sTco;
}

Expand Down

0 comments on commit 7e10026

Please sign in to comment.