Use std::optional for colour interfaces and storage (#6991)

include/Clip.h

@@ -25,6 +25,8 @@
 #ifndef LMMS_CLIP_H
 #define LMMS_CLIP_H
 
+#include <optional>
+
 #include <QColor>
 
 #include "AutomatableModel.h"
@@ -109,24 +111,8 @@ public:
 		return m_autoResize;
 	}
 
-	QColor color() const
-	{
-		return m_color;
-	}
-
-	void setColor( const QColor & c )
-	{
-		m_color = c;
-	}
-
-	bool hasColor();
-
-	void useCustomClipColor( bool b );
-
-	bool usesCustomClipColor()
-	{
-		return m_useCustomClipColor;
-	}
+	auto color() const -> const std::optional<QColor>& { return m_color; }
+	void setColor(const std::optional<QColor>& color);
 
 	virtual void movePosition( const TimePos & pos );
 	virtual void changeLength( const TimePos & length );
@@ -177,8 +163,7 @@ private:
 
 	bool m_selectViewOnCreate;
 
-	QColor m_color;
-	bool m_useCustomClipColor;
+	std::optional<QColor> m_color;
 
 	friend class ClipView;
 

include/ClipView.h

@@ -25,6 +25,7 @@
 #ifndef LMMS_GUI_CLIP_VIEW_H
 #define LMMS_GUI_CLIP_VIEW_H
 
+#include <optional>
 
 #include <QVector>
 
@@ -184,6 +185,7 @@ protected:
 
 	virtual void paintTextLabel(QString const & text, QPainter & painter);
 
+	auto hasCustomColor() const -> bool;
 
 protected slots:
 	void updateLength();
@@ -241,7 +243,7 @@ private:
 	bool mouseMovedDistance( QMouseEvent * me, int distance );
 	TimePos draggedClipPos( QMouseEvent * me );
 	int knifeMarkerPos( QMouseEvent * me );
-	void setColor(const QColor* color);
+	void setColor(const std::optional<QColor>& color);
 	//! Return true iff the clip could be split. Currently only implemented for samples
 	virtual bool splitClip( const TimePos pos ){ return false; };
 	void updateCursor(QMouseEvent * me);

include/Mixer.h

@@ -76,12 +76,8 @@ class MixerChannel : public ThreadableJob
 		bool requiresProcessing() const override { return true; }
 		void unmuteForSolo();
 
-		void setColor (QColor newColor)
-		{
-			m_color = newColor;
-		}
-
-		std::optional<QColor> m_color;
+		auto color() const -> const std::optional<QColor>& { return m_color; }
+		void setColor(const std::optional<QColor>& color) { m_color = color; }
 
 		std::atomic_int m_dependenciesMet;
 		void incrementDeps();
@@ -89,8 +85,9 @@ class MixerChannel : public ThreadableJob
 		
 	private:
 		void doProcessing() override;
-};
 
+		std::optional<QColor> m_color;
+};
 
 class MixerRoute : public QObject
 {

include/Track.h

@@ -189,15 +189,9 @@ public:
 	{
 		return m_processingLock.tryLock();
 	}
-	
-	QColor color()
-	{
-		return m_color.value();
-	}
-	bool useColor()
-	{
-		return m_color.has_value();
-	}
+
+	auto color() const -> const std::optional<QColor>& { return m_color; }
+	void setColor(const std::optional<QColor>& color);
 
 	bool isMutedBeforeSolo() const
 	{
@@ -220,9 +214,6 @@ public slots:
 
 	void toggleSolo();
 
-	void setColor(const QColor& c);
-	void resetColor();
-
 private:
 	TrackContainer* m_trackContainer;
 	Type m_type;

src/core/AutomationClip.cpp

@@ -830,10 +830,10 @@ void AutomationClip::saveSettings( QDomDocument & _doc, QDomElement & _this )
 	_this.setAttribute( "prog", QString::number( static_cast<int>(progressionType()) ) );
 	_this.setAttribute( "tens", QString::number( getTension() ) );
 	_this.setAttribute( "mute", QString::number( isMuted() ) );
-	
-	if( usesCustomClipColor() )
+
+	if (const auto& c = color())
 	{
-		_this.setAttribute( "color", color().name() );
+		_this.setAttribute("color", c->name());
 	}
 
 	for( timeMap::const_iterator it = m_timeMap.begin();
@@ -919,10 +919,9 @@ void AutomationClip::loadSettings( const QDomElement & _this )
 		}
 	}
 	
-	if( _this.hasAttribute( "color" ) )
+	if (_this.hasAttribute("color"))
 	{
-		useCustomClipColor( true );
-		setColor( _this.attribute( "color" ) );
+		setColor(QColor{_this.attribute("color")});
 	}
 
 	int len = _this.attribute( "len" ).toInt();

src/core/Clip.cpp

@@ -48,9 +48,7 @@ Clip::Clip( Track * track ) :
 	m_startPosition(),
 	m_length(),
 	m_mutedModel( false, this, tr( "Mute" ) ),
-	m_selectViewOnCreate( false ),
-	m_color( 128, 128, 128 ),
-	m_useCustomClipColor( false )
+	m_selectViewOnCreate{false}
 {
 	if( getTrack() )
 	{
@@ -185,19 +183,10 @@ void Clip::setStartTimeOffset( const TimePos &startTimeOffset )
 	m_startTimeOffset = startTimeOffset;
 }
 
-
-
-void Clip::useCustomClipColor( bool b )
+void Clip::setColor(const std::optional<QColor>& color)
 {
-	if (b == m_useCustomClipColor) { return; }
-	m_useCustomClipColor = b;
+	m_color = color;
 	emit colorChanged();
 }
 
-
-bool Clip::hasColor()
-{
-	return usesCustomClipColor() || getTrack()->useColor();
-}
-
 } // namespace lmms

src/core/Mixer.cpp

@@ -721,7 +721,7 @@ void Mixer::clearChannel(mix_ch_t index)
 	ch->m_volumeModel.setDisplayName( ch->m_name + ">" + tr( "Volume" ) );
 	ch->m_muteModel.setDisplayName( ch->m_name + ">" + tr( "Mute" ) );
 	ch->m_soloModel.setDisplayName( ch->m_name + ">" + tr( "Solo" ) );
-	ch->m_color = std::nullopt;
+	ch->setColor(std::nullopt);
 
 	// send only to master
 	if( index > 0)
@@ -759,7 +759,7 @@ void Mixer::saveSettings( QDomDocument & _doc, QDomElement & _this )
 		ch->m_soloModel.saveSettings( _doc, mixch, "soloed" );
 		mixch.setAttribute( "num", i );
 		mixch.setAttribute( "name", ch->m_name );
-		if (ch->m_color.has_value()) { mixch.setAttribute("color", ch->m_color->name()); }
+		if (const auto& color = ch->color()) { mixch.setAttribute("color", color->name()); }
 
 		// add the channel sends
 		for (const auto& send : ch->m_sends)
@@ -805,9 +805,9 @@ void Mixer::loadSettings( const QDomElement & _this )
 		m_mixerChannels[num]->m_muteModel.loadSettings( mixch, "muted" );
 		m_mixerChannels[num]->m_soloModel.loadSettings( mixch, "soloed" );
 		m_mixerChannels[num]->m_name = mixch.attribute( "name" );
-		if( mixch.hasAttribute( "color" ) )
+		if (mixch.hasAttribute("color"))
 		{
-			m_mixerChannels[num]->m_color = QColor(mixch.attribute("color"));
+			m_mixerChannels[num]->setColor(QColor{mixch.attribute("color")});
 		}
 
 		m_mixerChannels[num]->m_fxChain.restoreState( mixch.firstChildElement(

src/core/PatternClip.cpp

@@ -62,9 +62,9 @@ void PatternClip::saveSettings(QDomDocument& doc, QDomElement& element)
 	element.setAttribute( "len", length() );
 	element.setAttribute("off", startTimeOffset());
 	element.setAttribute( "muted", isMuted() );
-	if( usesCustomClipColor() )
+	if (const auto& c = color())
 	{
-		element.setAttribute( "color", color().name() );
+		element.setAttribute("color", c->name());
 	}
 }
 
@@ -90,20 +90,14 @@ void PatternClip::loadSettings(const QDomElement& element)
 		if (!element.hasAttribute("usestyle"))
 		{
 			// for colors saved in 1.3-onwards
-			setColor(element.attribute("color"));
-			useCustomClipColor(true);
+			setColor(QColor{element.attribute("color")});
 		}
-		else
+		else if (element.attribute("usestyle").toUInt() == 0)
 		{
 			// for colors saved before 1.3
-			setColor(QColor(element.attribute("color").toUInt()));
-			useCustomClipColor(element.attribute("usestyle").toUInt() == 0);
+			setColor(QColor{element.attribute("color").toUInt()});
 		}
 	}
-	else
-	{
-		useCustomClipColor(false);
-	}
 }
 
 

src/core/SampleClip.cpp

@@ -271,9 +271,9 @@ void SampleClip::saveSettings( QDomDocument & _doc, QDomElement & _this )
 	}
 
 	_this.setAttribute( "sample_rate", m_sampleBuffer->sampleRate());
-	if( usesCustomClipColor() )
+	if (const auto& c = color())
 	{
-		_this.setAttribute( "color", color().name() );
+		_this.setAttribute("color", c->name());
 	}
 	if (m_sampleBuffer->reversed())
 	{
@@ -304,14 +304,9 @@ void SampleClip::loadSettings( const QDomElement & _this )
 	setMuted( _this.attribute( "muted" ).toInt() );
 	setStartTimeOffset( _this.attribute( "off" ).toInt() );
 
-	if( _this.hasAttribute( "color" ) )
+	if (_this.hasAttribute("color"))
 	{
-		useCustomClipColor( true );
-		setColor( _this.attribute( "color" ) );
-	}
-	else
-	{
-		useCustomClipColor(false);
+		setColor(QColor{_this.attribute("color")});
 	}
 
 	if(_this.hasAttribute("reversed"))

src/core/Track.cpp

@@ -64,8 +64,7 @@ Track::Track( Type type, TrackContainer * tc ) :
 	m_mutedModel( false, this, tr( "Mute" ) ), /*!< For controlling track muting */
 	m_soloModel( false, this, tr( "Solo" ) ), /*!< For controlling track soloing */
 	m_simpleSerializingMode( false ),
-	m_clips(),        /*!< The clips (segments) */
-	m_color(std::nullopt)
+	m_clips()        /*!< The clips (segments) */
 {	
 	m_trackContainer->addTrack( this );
 	m_height = -1;
@@ -263,14 +262,9 @@ void Track::loadSettings( const QDomElement & element )
 	// Older project files that didn't have this attribute will set the value to false (issue 5562)
 	m_mutedBeforeSolo = QVariant( element.attribute( "mutedBeforeSolo", "0" ) ).toBool();
 
-	if( element.hasAttribute( "color" ) )
+	if (element.hasAttribute("color"))
 	{
-		QColor newColor = QColor(element.attribute("color"));
-		setColor(newColor);
-	}
-	else
-	{
-		resetColor();
+		setColor(QColor{element.attribute("color")});
 	}
 
 	if( m_simpleSerializingMode )
@@ -633,19 +627,12 @@ void Track::toggleSolo()
 	}
 }
 
-void Track::setColor(const QColor& c)
+void Track::setColor(const std::optional<QColor>& color)
 {
-	m_color = c;
+	m_color = color;
 	emit colorChanged();
 }
 
-void Track::resetColor()
-{
-	m_color = std::nullopt;
-	emit colorChanged();
-}
-
-
 BoolModel *Track::getMutedModel()
 {
 	return &m_mutedModel;

src/gui/MixerLine.cpp

@@ -163,9 +163,9 @@ void MixerLine::drawMixerLine( QPainter* p, const MixerLine *mixerLine, bool isA
 	int width = mixerLine->rect().width();
 	int height = mixerLine->rect().height();
 	
-	if (channel->m_color.has_value() && !muted)
+	if (channel->color().has_value() && !muted)
 	{
-		p->fillRect(mixerLine->rect(), channel->m_color->darker(isActive ? 120 : 150));
+		p->fillRect(mixerLine->rect(), channel->color()->darker(isActive ? 120 : 150));
 	}
 	else
 	{
@@ -415,36 +415,34 @@ void MixerLine::setStrokeInnerInactive( const QColor & c )
 	m_strokeInnerInactive = c;
 }
 
-
 // Ask user for a color, and set it as the mixer line color
 void MixerLine::selectColor()
 {
-	auto channel = Engine::mixer()->mixerChannel( m_channelIndex );
-	auto new_color = ColorChooser(this).withPalette(ColorChooser::Palette::Mixer)->getColor(channel->m_color.value_or(backgroundActive().color()));
-	if(!new_color.isValid()) { return; }
-	channel->setColor (new_color);
+	const auto channel = Engine::mixer()->mixerChannel(m_channelIndex);
+	const auto newColor = ColorChooser{this}
+		.withPalette(ColorChooser::Palette::Mixer)
+		->getColor(channel->color().value_or(backgroundActive().color()));
+	if (!newColor.isValid()) { return; }
+	channel->setColor(newColor);
 	Engine::getSong()->setModified();
 	update();
 }
 
-
 // Disable the usage of color on this mixer line
 void MixerLine::resetColor()
 {
-	Engine::mixer()->mixerChannel(m_channelIndex)->m_color = std::nullopt;
+	Engine::mixer()->mixerChannel(m_channelIndex)->setColor(std::nullopt);
 	Engine::getSong()->setModified();
 	update();
 }
 
-
 // Pick a random color from the mixer palette and set it as our color
 void MixerLine::randomizeColor()
 {
-	auto channel = Engine::mixer()->mixerChannel( m_channelIndex );
-	channel->setColor (ColorChooser::getPalette(ColorChooser::Palette::Mixer)[rand() % 48]);
+	const auto channel = Engine::mixer()->mixerChannel(m_channelIndex);
+	channel->setColor(ColorChooser::getPalette(ColorChooser::Palette::Mixer)[std::rand() % 48]);
 	Engine::getSong()->setModified();
 	update();
 }
 
-
 } // namespace lmms::gui

src/gui/clips/ClipView.cpp

@@ -136,7 +136,7 @@ ClipView::ClipView( Clip * clip,
 	connect(m_trackView->getTrack(), &Track::colorChanged, this, [this]
 	{
 		// redraw if clip uses track color
-		if (!m_clip->usesCustomClipColor()) { update(); }
+		if (!m_clip->color().has_value()) { update(); }
 	});
 
 	m_trackView->getTrackContentWidget()->addClipView( this );
@@ -340,45 +340,35 @@ void ClipView::updatePosition()
 	m_trackView->trackContainerView()->update();
 }
 
-
-
-
 void ClipView::selectColor()
 {
 	// Get a color from the user
-	QColor new_color = ColorChooser( this ).withPalette( ColorChooser::Palette::Track )->getColor( m_clip->color() );
-	if (new_color.isValid()) { setColor(&new_color); }
+	const auto newColor = ColorChooser{this}
+		.withPalette(ColorChooser::Palette::Track)
+		->getColor(m_clip->color().value_or(palette().background().color()));
+	if (newColor.isValid()) { setColor(newColor); }
 }
 
-
-
-
 void ClipView::randomizeColor()
 {
-	setColor(&ColorChooser::getPalette(ColorChooser::Palette::Mixer)[rand() % 48]);
+	setColor(ColorChooser::getPalette(ColorChooser::Palette::Mixer)[std::rand() % 48]);
 }
 
-
-
-
 void ClipView::resetColor()
 {
-	setColor(nullptr);
+	setColor(std::nullopt);
 }
 
-
-
-
 /*! \brief Change color of all selected clips
  *
- *  \param color The new QColor. Pass nullptr to use the Track's color.
+ *  \param color The new color.
  */
-void ClipView::setColor(const QColor* color)
+void ClipView::setColor(const std::optional<QColor>& color)
 {
 	std::set<Track*> journaledTracks;
 
 	auto selectedClips = getClickedClips();
-	for (auto clipv: selectedClips)
+	for (auto clipv : selectedClips)
 	{
 		auto clip = clipv->getClip();
 		auto track = clip->getTrack();
@@ -397,25 +387,13 @@ void ClipView::setColor(const QColor* color)
 			track->addJournalCheckPoint();
 		}
 
-		if (color)
-		{
-			clip->useCustomClipColor(true);
-			clip->setColor(*color);
-		}
-		else
-		{
-			clip->useCustomClipColor(false);
-		}
+		clip->setColor(color);
 		clipv->update();
 	}
 
 	Engine::getSong()->setModified();
 }
 
-
-
-
-
 /*! \brief Change the ClipView's display when something
  *  being dragged enters it.
  *
@@ -1483,11 +1461,7 @@ TimePos ClipView::quantizeSplitPos( TimePos midiPos, bool shiftMode )
 QColor ClipView::getColorForDisplay( QColor defaultColor )
 {
 	// Get the pure Clip color
-	auto clipColor = m_clip->hasColor()
-					? m_clip->usesCustomClipColor()
-						? m_clip->color()
-						: m_clip->getTrack()->color()
-					: defaultColor;
+	auto clipColor = m_clip->color().value_or(m_clip->getTrack()->color().value_or(defaultColor));
 
 	// Set variables
 	QColor c, mutedCustomColor;
@@ -1498,7 +1472,7 @@ QColor ClipView::getColorForDisplay( QColor defaultColor )
 	// Change the pure color by state: selected, muted, colored, normal
 	if( isSelected() )
 	{
-		c = m_clip->hasColor()
+		c = hasCustomColor()
 			? ( muted
 				? mutedCustomColor.darker( 350 )
 				: clipColor.darker( 150 ) )
@@ -1508,7 +1482,7 @@ QColor ClipView::getColorForDisplay( QColor defaultColor )
 	{
 		if( muted )
 		{
-			c = m_clip->hasColor()
+			c = hasCustomColor()
 				? mutedCustomColor.darker( 250 )
 				: mutedBackgroundColor();
 		}
@@ -1522,5 +1496,9 @@ QColor ClipView::getColorForDisplay( QColor defaultColor )
 	return c;
 }
 
+auto ClipView::hasCustomColor() const -> bool
+{
+	return m_clip->color().has_value() || m_clip->getTrack()->color().has_value();
+}
 
 } // namespace lmms::gui

src/gui/clips/MidiClipView.cpp

@@ -562,7 +562,7 @@ void MidiClipView::paintEvent( QPaintEvent * )
 		QColor noteFillColor = muted ? getMutedNoteFillColor().lighter(200)
 									 : (c.lightness() > 175 ? getNoteFillColor().darker(400) : getNoteFillColor());
 		QColor noteBorderColor = muted ? getMutedNoteBorderColor()
-									   : ( m_clip->hasColor() ? c.lighter( 200 ) : getNoteBorderColor() );
+									   : (hasCustomColor() ? c.lighter(200) : getNoteBorderColor());
 
 		bool const drawAsLines = height() < 64;
 		if (drawAsLines)

src/gui/clips/SampleClipView.cpp

@@ -231,11 +231,7 @@ void SampleClipView::paintEvent( QPaintEvent * pe )
 		p.fillRect( rect(), c );
 	}
 
-	auto clipColor = m_clip->hasColor()
-			? (m_clip->usesCustomClipColor()
-				? m_clip->color()
-				: m_clip->getTrack()->color())
-			: painter.pen().brush().color();
+	auto clipColor = m_clip->color().value_or(m_clip->getTrack()->color().value_or(painter.pen().brush().color()));
 
 	p.setPen(clipColor);
 

src/gui/tracks/InstrumentTrackView.cpp

@@ -227,7 +227,7 @@ void InstrumentTrackView::createMixerLine()
 	auto channel = Engine::mixer()->mixerChannel(channelIndex);
 
 	channel->m_name = getTrack()->name();
-	if (getTrack()->useColor()) { channel->setColor (getTrack()->color()); }
+	channel->setColor(getTrack()->color());
 
 	assignMixerLine(channelIndex);
 }

src/gui/tracks/SampleTrackView.cpp

@@ -228,7 +228,7 @@ void SampleTrackView::createMixerLine()
 	auto channel = Engine::mixer()->mixerChannel(channelIndex);
 
 	channel->m_name = getTrack()->name();
-	if (getTrack()->useColor()) { channel->setColor (getTrack()->color()); }
+	channel->setColor(getTrack()->color());
 
 	assignMixerLine(channelIndex);
 }

src/gui/tracks/TrackOperationsWidget.cpp

@@ -172,11 +172,11 @@ void TrackOperationsWidget::paintEvent( QPaintEvent * pe )
 
 	p.fillRect(rect(), palette().brush(QPalette::Window));
 
-	if( m_trackView->getTrack()->useColor() && ! m_trackView->getTrack()->getMutedModel()->value() ) 
+	if (m_trackView->getTrack()->color().has_value() && !m_trackView->getTrack()->getMutedModel()->value()) 
 	{
 		QRect coloredRect( 0, 0, 10, m_trackView->getTrack()->getHeight() );
-		
-		p.fillRect( coloredRect, m_trackView->getTrack()->color() );
+
+		p.fillRect(coloredRect, m_trackView->getTrack()->color().value());
 	}
 
 	p.drawPixmap(2, 2, embed::getIconPixmap(m_trackView->isMovingTrack() ? "track_op_grip_c" : "track_op_grip"));
@@ -265,15 +265,15 @@ void TrackOperationsWidget::removeTrack()
 
 void TrackOperationsWidget::selectTrackColor()
 {
-	QColor new_color = ColorChooser( this ).withPalette( ColorChooser::Palette::Track )-> \
-		getColor( m_trackView->getTrack()->color() );
+	const auto newColor = ColorChooser{this}
+		.withPalette(ColorChooser::Palette::Track)
+		->getColor(m_trackView->getTrack()->color().value_or(Qt::white));
 
-	if( ! new_color.isValid() )
-	{ return; }
+	if (!newColor.isValid()) { return; }
 
-	auto track = m_trackView->getTrack();
+	const auto track = m_trackView->getTrack();
 	track->addJournalCheckPoint();
-	track->setColor(new_color);
+	track->setColor(newColor);
 	Engine::getSong()->setModified();
 }
 
@@ -281,7 +281,7 @@ void TrackOperationsWidget::resetTrackColor()
 {
 	auto track = m_trackView->getTrack();
 	track->addJournalCheckPoint();
-	track->resetColor();
+	track->setColor(std::nullopt);
 	Engine::getSong()->setModified();
 }
 
@@ -298,16 +298,13 @@ void TrackOperationsWidget::resetClipColors()
 {
 	auto track = m_trackView->getTrack();
 	track->addJournalCheckPoint();
-	for (auto clip: track->getClips())
+	for (auto clip : track->getClips())
 	{
-		clip->useCustomClipColor(false);
+		clip->setColor(std::nullopt);
 	}
 	Engine::getSong()->setModified();
 }
 
-
-
-
 /*! \brief Update the trackOperationsWidget context menu
  *
  *  For all track types, we have the Clone and Remove options.

src/tracks/MidiClip.cpp

@@ -360,9 +360,9 @@ void MidiClip::saveSettings( QDomDocument & _doc, QDomElement & _this )
 	_this.setAttribute( "type", static_cast<int>(m_clipType) );
 	_this.setAttribute( "name", name() );
 	
-	if( usesCustomClipColor() )
+	if (const auto& c = color())
 	{
-		_this.setAttribute( "color", color().name() );
+		_this.setAttribute("color", c->name());
 	}
 	// as the target of copied/dragged MIDI clip is always an existing
 	// MIDI clip, we must not store actual position, instead we store -1
@@ -394,17 +394,12 @@ void MidiClip::loadSettings( const QDomElement & _this )
 	m_clipType = static_cast<Type>( _this.attribute( "type"
 								).toInt() );
 	setName( _this.attribute( "name" ) );
-	
-	if( _this.hasAttribute( "color" ) )
+
+	if (_this.hasAttribute("color"))
 	{
-		useCustomClipColor( true );
-		setColor( _this.attribute( "color" ) );
+		setColor(QColor{_this.attribute("color")});
 	}
-	else
-	{
-		useCustomClipColor(false);
-	}
-	
+
 	if( _this.attribute( "pos" ).toInt() >= 0 )
 	{
 		movePosition( _this.attribute( "pos" ).toInt() );