From b4317edd433a9bbe62e76988e46c5a8c829410cd Mon Sep 17 00:00:00 2001
From: Spekular <Spekular@users.noreply.github.com>
Date: Mon, 18 Apr 2022 17:25:15 +0200
Subject: [PATCH] Make loop markers snap to same increments as clips (#6313)

* Make loop markers snap to same increments as clips

* Fix snap size when loop points meet

* Switch to signal-slot based communication for proportional snap changes

* Move end if user clicks at exact center of loop

* Changes from reviews

* More detailed comment

* Properly handle zero length loop when ctrl is held

* More compact selection of action, m_loopPos ordering

* More robust sort
---
 include/TimeLineWidget.h       |  5 +++++
 src/gui/TimeLineWidget.cpp     | 35 ++++++++++------------------------
 src/gui/editors/SongEditor.cpp |  7 +++++++
 3 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/include/TimeLineWidget.h b/include/TimeLineWidget.h
index 915553c4d..7809c042e 100644
--- a/include/TimeLineWidget.h
+++ b/include/TimeLineWidget.h
@@ -180,6 +180,10 @@ public slots:
 	{
 		updatePosition( TimePos() );
 	}
+	void setSnapSize( const float snapSize )
+	{
+		m_snapSize = snapSize;
+	}
 	void toggleAutoScroll( int _n );
 	void toggleLoopPoints( int _n );
 	void toggleBehaviourAtStop( int _n );
@@ -217,6 +221,7 @@ private:
 	int m_xOffset;
 	int m_posMarkerX;
 	float m_ppb;
+	float m_snapSize;
 	Song::PlayPos & m_pos;
 	const TimePos & m_begin;
 	const Song::PlayModes m_mode;
diff --git a/src/gui/TimeLineWidget.cpp b/src/gui/TimeLineWidget.cpp
index e4a4ba711..f5159d640 100644
--- a/src/gui/TimeLineWidget.cpp
+++ b/src/gui/TimeLineWidget.cpp
@@ -59,6 +59,7 @@ TimeLineWidget::TimeLineWidget( const int xoff, const int yoff, const float ppb,
 	m_xOffset( xoff ),
 	m_posMarkerX( 0 ),
 	m_ppb( ppb ),
+	m_snapSize( 1.0 ),
 	m_pos( pos ),
 	m_begin( begin ),
 	m_mode( mode ),
@@ -336,20 +337,8 @@ void TimeLineWidget::mousePressEvent( QMouseEvent* event )
 		const TimePos t = m_begin + static_cast<int>( qMax( event->x() - m_xOffset - m_moveXOff, 0 ) * TimePos::ticksPerBar() / m_ppb );
 		const TimePos loopMid = ( m_loopPos[0] + m_loopPos[1] ) / 2;
 
-		if( t < loopMid )
-		{
-			m_action = MoveLoopBegin;
-		}
-		else if( t > loopMid )
-		{
-			m_action = MoveLoopEnd;
-		}
-
-		if( m_loopPos[0] > m_loopPos[1]  )
-		{
-			qSwap( m_loopPos[0], m_loopPos[1] );
-		}
-
+		m_action = t < loopMid ? MoveLoopBegin : MoveLoopEnd;
+		std::sort(std::begin(m_loopPos), std::end(m_loopPos));
 		m_loopPos[( m_action == MoveLoopBegin ) ? 0 : 1] = t;
 	}
 
@@ -391,7 +380,8 @@ void TimeLineWidget::mouseMoveEvent( QMouseEvent* event )
 		case MoveLoopEnd:
 		{
 			const int i = m_action - MoveLoopBegin; // i == 0 || i == 1
-			if( event->modifiers() & Qt::ControlModifier )
+			const bool control = event->modifiers() & Qt::ControlModifier;
+			if (control)
 			{
 				// no ctrl-press-hint when having ctrl pressed
 				delete m_hint;
@@ -400,21 +390,16 @@ void TimeLineWidget::mouseMoveEvent( QMouseEvent* event )
 			}
 			else
 			{
-				m_loopPos[i] = t.quantize(1.0);
+				m_loopPos[i] = t.quantize(m_snapSize);
 			}
 			// Catch begin == end
-			if( m_loopPos[0] == m_loopPos[1] )
+			if (m_loopPos[0] == m_loopPos[1])
 			{
+				const int offset = control ? 1 : m_snapSize * TimePos::ticksPerBar();
 				// Note, swap 1 and 0 below and the behavior "skips" the other
 				// marking instead of pushing it.
-				if( m_action == MoveLoopBegin ) 
-				{
-					m_loopPos[0] -= TimePos::ticksPerBar();
-				}
-				else
-				{
-					m_loopPos[1] += TimePos::ticksPerBar();
-				}
+				if (m_action == MoveLoopBegin) { m_loopPos[0] -= offset; }
+				else { m_loopPos[1] += offset; }
 			}
 			update();
 			break;
diff --git a/src/gui/editors/SongEditor.cpp b/src/gui/editors/SongEditor.cpp
index b1acc486f..c70705aaf 100644
--- a/src/gui/editors/SongEditor.cpp
+++ b/src/gui/editors/SongEditor.cpp
@@ -101,6 +101,11 @@ SongEditor::SongEditor( Song * song ) :
 			 m_positionLine, SLOT( update() ) );
 	connect( this, SIGNAL( zoomingValueChanged( float ) ),
 			 m_positionLine, SLOT( zoomChange( float ) ) );
+			 
+	// Ensure loop markers snap to same increments as clips. Zoom & proportional
+	// snap changes are handled in zoomingChanged() and toggleProportionalSnap()
+	connect(m_snappingModel, &ComboBoxModel::dataChanged,
+		[this]() { m_timeLine->setSnapSize(getSnapSize()); });
 
 
 	// add some essential widgets to global tool-bar
@@ -462,6 +467,7 @@ void SongEditor::setEditModeSelect()
 void SongEditor::toggleProportionalSnap()
 {
 	m_proportionalSnap = !m_proportionalSnap;
+	m_timeLine->setSnapSize(getSnapSize());
 }
 
 
@@ -840,6 +846,7 @@ void SongEditor::zoomingChanged()
 					setPixelsPerBar( pixelsPerBar() );
 	realignTracks();
 	updateRubberband();
+	m_timeLine->setSnapSize(getSnapSize());
 	
 	emit zoomingValueChanged( m_zoomLevels[m_zoomingModel->value()] );
 }