Color submenu, undoable colors, renaming signals (#6165)

* TCO coloring: submenu, randomize, undoable, renaming signals/slots

* Track coloring: submenu, undoable, renaming signals/slots

* FX color submenu

* Set modified on color change

* Use lambda to update TCOView after track color change

* More renaming, fix memory leak

Co-authored by DomClark

include/FxLine.h

@@ -104,8 +104,8 @@ private:
 public slots:
 	void renameChannel();
 	void resetColor();
-	void changeColor();
-	void randomColor();
+	void selectColor();
+	void randomizeColor();
 
 private slots:
 	void renameFinished();

include/SampleTCOView.h

@@ -45,7 +45,7 @@ public slots:
 
 
 protected:
-	void contextMenuEvent( QContextMenuEvent * _cme ) override;
+	void constructContextMenu(QMenu* cm) override;
 	void mousePressEvent( QMouseEvent * _me ) override;
 	void mouseReleaseEvent( QMouseEvent * _me ) override;
 	void dragEnterEvent( QDragEnterEvent * _dee ) override;

include/Track.h

@@ -198,8 +198,8 @@ public slots:
 
 	void toggleSolo();
 
-	void trackColorChanged( QColor & c );
-	void trackColorReset();
+	void setColor(const QColor& c);
+	void resetColor();
 
 private:
 	TrackContainer* m_trackContainer;
@@ -229,6 +229,7 @@ signals:
 	void destroyedTrack();
 	void nameChanged();
 	void trackContentObjectAdded( TrackContentObject * );
+	void colorChanged();
 
 } ;
 

include/TrackContentObject.h

@@ -135,8 +135,6 @@ public:
 	TimePos startTimeOffset() const;
 	void setStartTimeOffset( const TimePos &startTimeOffset );
 
-	void updateColor();
-
 	// Will copy the state of a TCO to another TCO
 	static void copyStateTo( TrackContentObject *src, TrackContentObject *dst );
 
@@ -148,7 +146,7 @@ signals:
 	void lengthChanged();
 	void positionChanged();
 	void destroyedTCO();
-	void trackColorChanged();
+	void colorChanged();
 
 
 private:

include/TrackContentObjectView.h

@@ -126,8 +126,9 @@ public slots:
 	void remove();
 	void update() override;
 
-	void changeClipColor();
-	void useTrackColor();
+	void selectColor();
+	void randomizeColor();
+	void resetColor();
 
 protected:
 	enum ContextMenuAction
@@ -231,6 +232,7 @@ private:
 	bool mouseMovedDistance( QMouseEvent * me, int distance );
 	TimePos draggedTCOPos( QMouseEvent * me );
 	int knifeMarkerPos( QMouseEvent * me );
+	void setColor(const QColor* color);
 	//! Return true iff TCO could be split. Currently only implemented for samples
 	virtual bool splitTCO( const TimePos pos ){ return false; };
 	void updateCursor(QMouseEvent * me);

include/TrackOperationsWidget.h

@@ -50,10 +50,10 @@ private slots:
 	void cloneTrack();
 	void removeTrack();
 	void updateMenu();
-	void changeTrackColor();
-	void randomTrackColor();
+	void selectTrackColor();
+	void randomizeTrackColor();
 	void resetTrackColor();
-	void useTrackColor();
+	void resetTCOColors();
 	void toggleRecording(bool on);
 	void recordingOn();
 	void recordingOff();
@@ -71,9 +71,6 @@ private:
 
 signals:
 	void trackRemovalScheduled( TrackView * t );
-	void colorChanged( QColor & c );
-	void colorParented();
-	void colorReset();
 
 } ;
 

src/core/BBTCO.cpp

@@ -87,13 +87,16 @@ void BBTCO::loadSettings( const QDomElement & element )
 	}
 	
 	// for colors saved before 1.3
-	else
+	else if(element.hasAttribute("color"))
 	{
-		if( element.hasAttribute( "color" ) )
-		{ setColor( QColor( element.attribute( "color" ).toUInt() ) ); }
+		setColor(QColor(element.attribute("color").toUInt()));
 		
 		// usestyle attribute is no longer used
 	}
+	else
+	{
+		useCustomClipColor(false);
+	}
 }
 
 

src/core/SampleTCO.cpp

@@ -299,6 +299,10 @@ void SampleTCO::loadSettings( const QDomElement & _this )
 		useCustomClipColor( true );
 		setColor( _this.attribute( "color" ) );
 	}
+	else
+	{
+		useCustomClipColor(false);
+	}
 
 	if(_this.hasAttribute("reversed"))
 	{

src/core/Track.cpp

@@ -270,8 +270,12 @@ void Track::loadSettings( const QDomElement & element )
 
 	if( element.hasAttribute( "color" ) )
 	{
-		m_color.setNamedColor( element.attribute( "color" ) );
-		m_hasColor = true;
+		QColor newColor = QColor(element.attribute("color"));
+		setColor(newColor);
+	}
+	else
+	{
+		resetColor();
 	}
 
 	if( m_simpleSerializingMode )
@@ -647,23 +651,17 @@ void Track::toggleSolo()
 	}
 }
 
-void Track::trackColorChanged( QColor & c )
+void Track::setColor(const QColor& c)
 {
-	for (int i = 0; i < numOfTCOs(); i++)
-	{
-		m_trackContentObjects[i]->updateColor();
-	}
 	m_hasColor = true;
 	m_color = c;
+	emit colorChanged();
 }
 
-void Track::trackColorReset()
+void Track::resetColor()
 {
-	for (int i = 0; i < numOfTCOs(); i++)
-	{
-		m_trackContentObjects[i]->updateColor();
-	}
 	m_hasColor = false;
+	emit colorChanged();
 }
 
 

src/core/TrackContentObject.cpp

@@ -182,20 +182,13 @@ void TrackContentObject::setStartTimeOffset( const TimePos &startTimeOffset )
 	m_startTimeOffset = startTimeOffset;
 }
 
-// Update TCO color if it follows the track color
-void TrackContentObject::updateColor()
-{
-	if( ! m_useCustomClipColor )
-	{
-		emit trackColorChanged();
-	}
-}
 
 
 void TrackContentObject::useCustomClipColor( bool b )
 {
+	if (b == m_useCustomClipColor) { return; }
 	m_useCustomClipColor = b;
-	updateColor();
+	emit colorChanged();
 }
 
 

src/gui/SampleTCOView.cpp

@@ -63,80 +63,23 @@ void SampleTCOView::updateSample()
 
 
 
-void SampleTCOView::contextMenuEvent( QContextMenuEvent * _cme )
+void SampleTCOView::constructContextMenu(QMenu* cm)
 {
-	// Depending on whether we right-clicked a selection or an individual TCO we will have
-	// different labels for the actions.
-	bool individualTCO = getClickedTCOs().size() <= 1;
+	cm->addSeparator();
 
-	if( _cme->modifiers() )
-	{
-		return;
-	}
-
-	QMenu contextMenu( this );
-
-	if( fixedTCOs() == false )
-	{
-		contextMenu.addAction(
-			embed::getIconPixmap( "cancel" ),
-			individualTCO
-				? tr("Delete (middle mousebutton)")
-				: tr("Delete selection (middle mousebutton)"),
-			[this](){ contextMenuAction( Remove ); } );
-
-		contextMenu.addSeparator();
-
-		contextMenu.addAction(
-			embed::getIconPixmap( "edit_cut" ),
-			individualTCO
-				? tr("Cut")
-				: tr("Cut selection"),
-			[this](){ contextMenuAction( Cut ); } );
-	}
-
-	contextMenu.addAction(
-		embed::getIconPixmap( "edit_copy" ),
-		individualTCO
-			? tr("Copy")
-			: tr("Copy selection"),
-		[this](){ contextMenuAction( Copy ); } );
-
-	contextMenu.addAction(
-		embed::getIconPixmap( "edit_paste" ),
-		tr( "Paste" ),
-		[this](){ contextMenuAction( Paste ); } );
-
-	contextMenu.addSeparator();
-
-	contextMenu.addAction(
-		embed::getIconPixmap( "muted" ),
-		(individualTCO
-			? tr("Mute/unmute (<%1> + middle click)")
-			: tr("Mute/unmute selection (<%1> + middle click)")).arg(UI_CTRL_KEY),
-		[this](){ contextMenuAction( Mute ); } );
 
 	/*contextMenu.addAction( embed::getIconPixmap( "record" ),
 				tr( "Set/clear record" ),
 						m_tco, SLOT( toggleRecord() ) );*/
 
-	contextMenu.addAction(
+	cm->addAction(
 		embed::getIconPixmap("flip_x"),
 		tr("Reverse sample"),
 		this,
 		SLOT(reverseSample())
 	);
 
-	contextMenu.addSeparator();
 
-	contextMenu.addAction( embed::getIconPixmap( "colorize" ),
-			tr( "Set clip color" ), this, SLOT( changeClipColor() ) );
-	contextMenu.addAction( embed::getIconPixmap( "colorize" ),
-			tr( "Use track color" ), this, SLOT( useTrackColor() ) );
-
-	constructContextMenu( &contextMenu );
-
-	contextMenu.exec( QCursor::pos() );
 }
 
 

src/gui/TrackContentObjectView.cpp

@@ -124,8 +124,13 @@ TrackContentObjectView::TrackContentObjectView( TrackContentObject * tco,
 			this, SLOT( updatePosition() ) );
 	connect( m_tco, SIGNAL( destroyedTCO() ), this, SLOT( close() ) );
 	setModel( m_tco );
-	connect( m_tco, SIGNAL( trackColorChanged() ), this, SLOT( update() ) );
-	connect( m_trackView->getTrackOperationsWidget(), SIGNAL( colorParented() ), this, SLOT( useTrackColor() ) );
+	connect(m_tco, SIGNAL(colorChanged()), this, SLOT(update()));
+
+	connect(m_trackView->getTrack(), &Track::colorChanged, this, [this]
+	{
+		// redraw if TCO uses track color
+		if (!m_tco->usesCustomClipColor()) { update(); }
+	});
 
 	m_trackView->getTrackContentWidget()->addTCOView( this );
 	updateLength();
@@ -333,25 +338,73 @@ void TrackContentObjectView::updatePosition()
 
 
 
-void TrackContentObjectView::changeClipColor()
+void TrackContentObjectView::selectColor()
 {
 	// Get a color from the user
 	QColor new_color = ColorChooser( this ).withPalette( ColorChooser::Palette::Track )->getColor( m_tco->color() );
-	if( ! new_color.isValid() )
-	{ return; }
-
-	// Use that color
-	m_tco->setColor( new_color );
-	m_tco->useCustomClipColor( true );
-	update();
+	if (new_color.isValid()) { setColor(&new_color); }
 }
 
 
 
-void TrackContentObjectView::useTrackColor()
+
+void TrackContentObjectView::randomizeColor()
 {
-	m_tco->useCustomClipColor( false );
-	update();
+	setColor(&ColorChooser::getPalette(ColorChooser::Palette::Mixer)[rand() % 48]);
+}
+
+
+
+
+void TrackContentObjectView::resetColor()
+{
+	setColor(nullptr);
+}
+
+
+
+
+/*! \brief Change color of all selected TCOs
+ *
+ *  \param color The new QColor. Pass nullptr to use the Track's color.
+ */
+void TrackContentObjectView::setColor(const QColor* color)
+{
+	std::set<Track*> journaledTracks;
+
+	auto selectedTCOs = getClickedTCOs();
+	for (auto tcov: selectedTCOs)
+	{
+		auto tco = tcov->getTrackContentObject();
+		auto track = tco->getTrack();
+
+		// TODO journal whole Song or group of TCOs instead of one journal entry for each track
+
+		// If only one TCO changed, store that in the journal
+		if (selectedTCOs.length() == 1)
+		{
+			tco->addJournalCheckPoint();
+		}
+		// If multiple TCOs changed, store whole Track in the journal
+		// Check if track has been journaled already by trying to add it to the set
+		else if (journaledTracks.insert(track).second)
+		{
+			track->addJournalCheckPoint();
+		}
+
+		if (color)
+		{
+			tco->useCustomClipColor(true);
+			tco->setColor(*color);
+		}
+		else
+		{
+			tco->useCustomClipColor(false);
+		}
+		tcov->update();
+	}
+
+	Engine::getSong()->setModified();
 }
 
 
@@ -1050,10 +1103,12 @@ void TrackContentObjectView::contextMenuEvent( QContextMenuEvent * cme )
 
 	contextMenu.addSeparator();
 
-	contextMenu.addAction( embed::getIconPixmap( "colorize" ),
-			tr( "Set clip color" ), this, SLOT( changeClipColor() ) );
-	contextMenu.addAction( embed::getIconPixmap( "colorize" ),
-			tr( "Use track color" ), this, SLOT( useTrackColor() ) );
+	QMenu colorMenu (tr("Clip color"), this);
+	colorMenu.setIcon(embed::getIconPixmap("colorize"));
+	colorMenu.addAction(tr("Change"), this, SLOT(selectColor()));
+	colorMenu.addAction(tr("Reset"), this, SLOT(resetColor()));
+	colorMenu.addAction(tr("Pick random"), this, SLOT(randomizeColor()));
+	contextMenu.addMenu(&colorMenu);
 
 	constructContextMenu( &contextMenu );
 

src/gui/TrackView.cpp

@@ -102,12 +102,6 @@ TrackView::TrackView( Track * track, TrackContainerView * tcv ) :
 	connect( &m_track->m_soloModel, SIGNAL( dataChanged() ),
 			m_track, SLOT( toggleSolo() ), Qt::DirectConnection );
 
-	connect( &m_trackOperationsWidget, SIGNAL( colorChanged( QColor & ) ),
-			m_track, SLOT( trackColorChanged( QColor & ) ) );
-
-	connect( &m_trackOperationsWidget, SIGNAL( colorReset() ),
-			m_track, SLOT( trackColorReset() ) );
-
 	// create views for already existing TCOs
 	for( Track::tcoVector::iterator it =
 					m_track->m_trackContentObjects.begin();

src/gui/widgets/FxLine.cpp

@@ -252,9 +252,14 @@ void FxLine::contextMenuEvent( QContextMenuEvent * )
 	}
 	contextMenu->addAction( embed::getIconPixmap( "cancel" ), tr( "Remove &unused channels" ), this, SLOT( removeUnusedChannels() ) );
 	contextMenu->addSeparator();
-	contextMenu->addAction( embed::getIconPixmap( "colorize" ), tr( "Set channel color" ), this, SLOT( changeColor() ) );
-	contextMenu->addAction( embed::getIconPixmap( "colorize" ), tr( "Remove channel color" ), this, SLOT( resetColor() ) );
-	contextMenu->addAction( embed::getIconPixmap( "colorize" ), tr( "Pick random channel color" ), this, SLOT( randomColor() ) );
+
+	QMenu colorMenu(tr("Color"), this);
+	colorMenu.setIcon(embed::getIconPixmap("colorize"));
+	colorMenu.addAction(tr("Change"), this, SLOT(selectColor()));
+	colorMenu.addAction(tr("Reset"), this, SLOT(resetColor()));
+	colorMenu.addAction(tr("Pick random"), this, SLOT(randomizeColor()));
+	contextMenu->addMenu(&colorMenu);
+
 	contextMenu->exec( QCursor::pos() );
 	delete contextMenu;
 }
@@ -415,12 +420,13 @@ void FxLine::setStrokeInnerInactive( const QColor & c )
 
 
 // Ask user for a color, and set it as the mixer line color
-void FxLine::changeColor()
+void FxLine::selectColor()
 {
 	auto channel = Engine::fxMixer()->effectChannel( m_channelIndex );
 	auto new_color = ColorChooser(this).withPalette(ColorChooser::Palette::Mixer)->getColor(channel->m_color);
 	if(!new_color.isValid()) { return; }
 	channel->setColor (new_color);
+	Engine::getSong()->setModified();
 	update();
 }
 
@@ -429,14 +435,16 @@ void FxLine::changeColor()
 void FxLine::resetColor()
 {
 	Engine::fxMixer()->effectChannel( m_channelIndex )->m_hasColor = false;
+	Engine::getSong()->setModified();
 	update();
 }
 
 
 // Pick a random color from the mixer palette and set it as our color
-void FxLine::randomColor()
+void FxLine::randomizeColor()
 {
 	auto channel = Engine::fxMixer()->effectChannel( m_channelIndex );
 	channel->setColor (ColorChooser::getPalette(ColorChooser::Palette::Mixer)[rand() % 48]);
+	Engine::getSong()->setModified();
 	update();
 }

src/gui/widgets/TrackOperationsWidget.cpp

@@ -112,6 +112,7 @@ TrackOperationsWidget::TrackOperationsWidget( TrackView * parent ) :
 	connect( m_trackView->getTrack()->getMutedModel(), SIGNAL( dataChanged() ),
 			this, SLOT( update() ) );
 
+	connect(m_trackView->getTrack(), SIGNAL(colorChanged()), this, SLOT(update()));
 }
 
 
@@ -268,7 +269,7 @@ void TrackOperationsWidget::removeTrack()
 	}
 }
 
-void TrackOperationsWidget::changeTrackColor()
+void TrackOperationsWidget::selectTrackColor()
 {
 	QColor new_color = ColorChooser( this ).withPalette( ColorChooser::Palette::Track )-> \
 		getColor( m_trackView->getTrack()->color() );
@@ -276,35 +277,43 @@ void TrackOperationsWidget::changeTrackColor()
 	if( ! new_color.isValid() )
 	{ return; }
 
-	emit colorChanged( new_color );
-
+	auto track = m_trackView->getTrack();
+	track->addJournalCheckPoint();
+	track->setColor(new_color);
 	Engine::getSong()->setModified();
-	update();
 }
 
 void TrackOperationsWidget::resetTrackColor()
 {
-	emit colorReset();
+	auto track = m_trackView->getTrack();
+	track->addJournalCheckPoint();
+	track->resetColor();
 	Engine::getSong()->setModified();
-	update();
 }
 
-void TrackOperationsWidget::randomTrackColor()
+void TrackOperationsWidget::randomizeTrackColor()
 {
 	QColor buffer = ColorChooser::getPalette( ColorChooser::Palette::Track )[ rand() % 48 ];
-
-	emit colorChanged( buffer );
+	auto track = m_trackView->getTrack();
+	track->addJournalCheckPoint();
+	track->setColor(buffer);
 	Engine::getSong()->setModified();
-	update();
 }
 
-void TrackOperationsWidget::useTrackColor()
+void TrackOperationsWidget::resetTCOColors()
 {
-	emit colorParented();
+	auto track = m_trackView->getTrack();
+	track->addJournalCheckPoint();
+	for (auto tco: track->getTCOs())
+	{
+		tco->useCustomClipColor(false);
+	}
 	Engine::getSong()->setModified();
 }
 
 
+
+
 /*! \brief Update the trackOperationsWidget context menu
  *
  *  For all track types, we have the Clone and Remove options.
@@ -345,15 +354,14 @@ void TrackOperationsWidget::updateMenu()
 	}
 
 	toMenu->addSeparator();
-	toMenu->addAction( embed::getIconPixmap( "colorize" ),
-						tr( "Change color" ), this, SLOT( changeTrackColor() ) );
-	toMenu->addAction( embed::getIconPixmap( "colorize" ),
-						tr( "Reset color to default" ), this, SLOT( resetTrackColor() ) );
-	toMenu->addAction( embed::getIconPixmap( "colorize" ),
-						tr( "Set random color" ), this, SLOT( randomTrackColor() ) );
-	toMenu->addSeparator();
-	toMenu->addAction( embed::getIconPixmap( "colorize" ),
-						tr( "Clear clip colors" ), this, SLOT( useTrackColor() ) );
+
+	QMenu* colorMenu = toMenu->addMenu(tr("Track color"));
+	colorMenu->setIcon(embed::getIconPixmap("colorize"));
+	colorMenu->addAction(tr("Change"), this, SLOT(selectTrackColor()));
+	colorMenu->addAction(tr("Reset"), this, SLOT(resetTrackColor()));
+	colorMenu->addAction(tr("Pick random"), this, SLOT(randomizeTrackColor()));
+	colorMenu->addSeparator();
+	colorMenu->addAction(tr("Reset clip colors"), this, SLOT(resetTCOColors()));
 }
 
 

src/tracks/Pattern.cpp

@@ -422,6 +422,10 @@ void Pattern::loadSettings( const QDomElement & _this )
 		useCustomClipColor( true );
 		setColor( _this.attribute( "color" ) );
 	}
+	else
+	{
+		useCustomClipColor(false);
+	}
 	
 	if( _this.attribute( "pos" ).toInt() >= 0 )
 	{