Batch undo

include/ClipView.h

@@ -199,8 +199,7 @@ protected slots:
 		Resize,
 		ResizeLeft,
 		Split,
-		CopySelection,
-		ToggleSelected
+		CopyOrToggleSelect, // depends on drag vs click
 	} ;
 
 	static TextFloat * s_textFloat;

include/ProjectJournal.h

@@ -55,6 +55,8 @@ class ProjectJournal
 	bool canRedo() const;
 
 	void addJournalCheckPoint( JournallingObject *jo );
+	void beginBatchCheckPoint(bool append = false);
+	void endBatchCheckPoint();
 
 	bool isJournalling() const
 	{
@@ -112,13 +114,18 @@ class ProjectJournal
 		jo_id_t joID;
 		DataFile data;
 	} ;
-	typedef QStack<CheckPoint> CheckPointStack;
+	typedef std::vector<CheckPoint> BatchCheckPoint;
+	typedef std::vector<BatchCheckPoint> CheckPointStack;
+
+	void restoreCheckPoint(ProjectJournal::CheckPointStack& restore, ProjectJournal::CheckPointStack& backup);
 
 	JoIdMap m_joIDs;
 
 	CheckPointStack m_undoCheckPoints;
 	CheckPointStack m_redoCheckPoints;
 
+	//! Used to determine if checkpoints should be batched or not
+	int m_batchingCount = 0;
 	bool m_journalling;
 
 } ;

src/core/AutomatableModel.cpp

@@ -296,14 +296,15 @@ void AutomatableModel::setValue( const float value )
 {
 	m_oldValue = m_value;
 	++m_setValueDepth;
-	const float old_val = m_value;
+	float newValue = fittedValue(value);
 
-	m_value = fittedValue( value );
-	if( old_val != m_value )
+	if (m_oldValue != newValue)
 	{
 		// add changes to history so user can undo it
 		addJournalCheckPoint();
 
+		m_value = newValue;
+
 		// notify linked models
 		for( AutoModelVector::Iterator it = m_linkedModels.begin(); it != m_linkedModels.end(); ++it )
 		{

src/core/AutomationClip.cpp

@@ -842,6 +842,10 @@ void AutomationClip::loadSettings( const QDomElement & _this )
 		useCustomClipColor( true );
 		setColor( _this.attribute( "color" ) );
 	}
+	else
+	{
+		useCustomClipColor(false);
+	}
 
 	int len = _this.attribute( "len" ).toInt();
 	if( len <= 0 )

src/core/Clip.cpp

@@ -60,6 +60,8 @@ Clip::Clip( Track * track ) :
 	movePosition( 0 );
 	changeLength( 0 );
 	setJournalling( true );
+
+	connect(&m_mutedModel, &Model::dataChanged, this, &Model::dataChanged);
 }
 
 

src/core/ProjectJournal.cpp

@@ -24,6 +24,7 @@
 
 #include <cstdlib>
 
+#include "debug.h"
 #include "ProjectJournal.h"
 #include "Engine.h"
 #include "JournallingObject.h"
@@ -51,84 +52,142 @@ ProjectJournal::ProjectJournal() :
 
 void ProjectJournal::undo()
 {
-	while( !m_undoCheckPoints.isEmpty() )
-	{
-		CheckPoint c = m_undoCheckPoints.pop();
-		JournallingObject *jo = m_joIDs[c.joID];
+	restoreCheckPoint(m_undoCheckPoints, m_redoCheckPoints);
+}
 
-		if( jo )
-		{
-			DataFile curState( DataFile::JournalData );
-			jo->saveState( curState, curState.content() );
-			m_redoCheckPoints.push( CheckPoint( c.joID, curState ) );
 
-			bool prev = isJournalling();
-			setJournalling( false );
-			jo->restoreState( c.data.content().firstChildElement() );
-			setJournalling( prev );
-			Engine::getSong()->setModified();
-			break;
-		}
-	}
+
+
+void ProjectJournal::redo()
+{
+	restoreCheckPoint(m_redoCheckPoints, m_undoCheckPoints);
 }
 
 
 
-void ProjectJournal::redo()
+
+/*! \brief Take a backup of the current state before restoring the most recent checkpoint
+ *
+ *  \param restoreStack pop a checkpoint from this stack and restore it
+ *  \param backupStack append a checkpoint of the current state to this stack
+ */
+void ProjectJournal::restoreCheckPoint(ProjectJournal::CheckPointStack& restoreStack,
+										ProjectJournal::CheckPointStack& backupStack)
 {
-	while( !m_redoCheckPoints.isEmpty() )
+	while (!restoreStack.empty())
 	{
-		CheckPoint c = m_redoCheckPoints.pop();
-		JournallingObject *jo = m_joIDs[c.joID];
+		BatchCheckPoint backup;
 
-		if( jo )
+		// For every checkpoint (journaled object) in the last batch...
+		for (CheckPoint& restorePoint: restoreStack.back())
 		{
-			DataFile curState( DataFile::JournalData );
+			JournallingObject* jo = journallingObject(restorePoint.joID);
+			// TODO when can this be null?
+			if (!jo) { continue; }
+
+			// Create a backup of the object's current state
+			DataFile curState(DataFile::JournalData);
 			jo->saveState( curState, curState.content() );
-			m_undoCheckPoints.push( CheckPoint( c.joID, curState ) );
+			backup.push_back(CheckPoint(restorePoint.joID, curState));
 
+			// Restore object to its previous state
 			bool prev = isJournalling();
 			setJournalling( false );
-			jo->restoreState( c.data.content().firstChildElement() );
+			jo->restoreState(restorePoint.data.content().firstChildElement());
 			setJournalling( prev );
 			Engine::getSong()->setModified();
-			break;
 		}
+		restoreStack.pop_back();
+
+		// Keep trying until something is restored
+		if (backup.empty()) { continue; }
+
+		backupStack.push_back(backup);
+		return;
 	}
 }
 
+
+
+
 bool ProjectJournal::canUndo() const
 {
-	return !m_undoCheckPoints.isEmpty();
+	return !m_undoCheckPoints.empty();
 }
 
+
+
+
 bool ProjectJournal::canRedo() const
 {
-	return !m_redoCheckPoints.isEmpty();
+	return !m_redoCheckPoints.empty();
 }
 
 
 
+
 void ProjectJournal::addJournalCheckPoint( JournallingObject *jo )
 {
 	if( isJournalling() )
 	{
 		m_redoCheckPoints.clear();
 
+		// If we're not batching checkpoints, begin on a new one
+		if (m_batchingCount == 0 || m_undoCheckPoints.empty())
+		{
+			m_undoCheckPoints.emplace_back();
+		}
+		BatchCheckPoint& batch = m_undoCheckPoints.back();
+
+		// If this object already has a checkpoint in the batch, skip it
+		for (const CheckPoint& checkpoint: batch)
+		{
+			if (checkpoint.joID == jo->id()) { return; }
+		}
+
+		// Create a checkpoint and save it to the batch
 		DataFile dataFile( DataFile::JournalData );
 		jo->saveState( dataFile, dataFile.content() );
+		batch.push_back(CheckPoint(jo->id(), dataFile));
 
-		m_undoCheckPoints.push( CheckPoint( jo->id(), dataFile ) );
+		// Remove excessive checkpoints from the stack
 		if( m_undoCheckPoints.size() > MAX_UNDO_STATES )
 		{
-			m_undoCheckPoints.remove( 0, m_undoCheckPoints.size() - MAX_UNDO_STATES );
+			m_undoCheckPoints.erase(m_undoCheckPoints.begin(), m_undoCheckPoints.end() - MAX_UNDO_STATES);
 		}
 	}
 }
 
 
 
 
+/*! \brief Start adding checkpoints together as a batch
+ *
+ *  \param append - Don't create a new batch, use the last checkpoint
+ */
+void ProjectJournal::beginBatchCheckPoint(bool append)
+{
+	if (!isJournalling()) { return; }
+	// Begin on a new batch if we are not already batching, or if there is nothing to append to
+	if (append ? m_undoCheckPoints.empty() : m_batchingCount == 0) { m_undoCheckPoints.emplace_back(); }
+	++m_batchingCount;
+}
+
+
+
+
+void ProjectJournal::endBatchCheckPoint()
+{
+	if (!isJournalling()) { return; }
+	// If no checkpoints were added to the batch, remove it
+	if (m_undoCheckPoints.back().empty()) { m_undoCheckPoints.pop_back(); }
+	--m_batchingCount;
+	assert(m_batchingCount >= 0);
+}
+
+
+
+
 jo_id_t ProjectJournal::allocID( JournallingObject * _obj )
 {
 	jo_id_t id;

src/core/Song.cpp

@@ -779,29 +779,31 @@ void Song::stopExport()
 
 void Song::insertBar()
 {
+	Engine::projectJournal()->beginBatchCheckPoint();
 	m_tracksMutex.lockForRead();
 	for (Track* track: tracks())
 	{
-		// FIXME journal batch of tracks instead of each track individually
 		if (track->numOfClips() > 0) { track->addJournalCheckPoint(); }
 		track->insertBar(m_playPos[Mode_PlaySong]);
 	}
 	m_tracksMutex.unlock();
+	Engine::projectJournal()->endBatchCheckPoint();
 }
 
 
 
 
 void Song::removeBar()
 {
+	Engine::projectJournal()->beginBatchCheckPoint();
 	m_tracksMutex.lockForRead();
 	for (Track* track: tracks())
 	{
-		// FIXME journal batch of tracks instead of each track individually
 		if (track->numOfClips() > 0) { track->addJournalCheckPoint(); }
 		track->removeBar(m_playPos[Mode_PlaySong]);
 	}
 	m_tracksMutex.unlock();
+	Engine::projectJournal()->endBatchCheckPoint();
 }
 
 

src/core/Track.cpp

@@ -39,6 +39,7 @@
 #include "InstrumentTrack.h"
 #include "PatternStore.h"
 #include "PatternTrack.h"
+#include "ProjectJournal.h"
 #include "SampleTrack.h"
 #include "Song.h"
 
@@ -584,6 +585,9 @@ bar_t Track::length() const
  */
 void Track::toggleSolo()
 {
+	// The solo led will have created its own checkpoint already, so we append to that batch
+	Engine::projectJournal()->beginBatchCheckPoint(/*append*/ true);
+
 	const TrackContainer::TrackList & tl = m_trackContainer->tracks();
 
 	bool soloBefore = false;
@@ -638,6 +642,7 @@ void Track::toggleSolo()
 			}
 		}
 	}
+	Engine::projectJournal()->endBatchCheckPoint();
 }
 
 void Track::setColor(const QColor& c)