From 5f4d0cab9b44e4b8df9d67909ee4bed78b794c7d Mon Sep 17 00:00:00 2001
From: DigArtRoks <69391149+DigArtRoks@users.noreply.github.com>
Date: Thu, 10 Sep 2020 22:47:06 +0200
Subject: [PATCH] Fix for issue #3816 - FM or heavy PM in TripleOscillator
 makes outputs odd for some target waveforms. (#5651)

The internal waveforms of the class Oscillator produces the wrong amplitude when the input is a
negative phase. When doing PM or FM, negative phases may occur. When a negative phase is e.g. passed
to the the saw sample, it produces values less than -1.0, hence going out of range.

Converted all fraction calls to absFraction calls.

Removed the +2 in the function Oscillator::recalcPhase. The comment here was that it was needed to avoid
negative phases in case of PM. But by converting fraction to absFraction in the waveforms, negative phases
are not an issue anymore. On top of that the m_phase variable gains about 2 extra bits in precision.
As side effect of that, it improves the behaviour of the issue #2047 - TripleOscillator: Oscillators are getting out of sync.
Though I did not investigate it thoroughly over different notes and samplerates.

Add documentation to the fraction and absFraction functions in lmms_math.h as it was not immediately clear by the name what the
functions do. Correct the implementation of the functions in case the flag __INTEL_COMPILER is set. (floorf rounds always down).
---
 include/Oscillator.h    | 10 +++++-----
 include/lmms_math.h     | 23 +++++++++++++++++++++--
 src/core/Oscillator.cpp |  3 +--
 3 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/include/Oscillator.h b/include/Oscillator.h
index 408e69dbc..912bcdc09 100644
--- a/include/Oscillator.h
+++ b/include/Oscillator.h
@@ -97,7 +97,7 @@ public:
 
 	static inline sample_t triangleSample( const float _sample )
 	{
-		const float ph = fraction( _sample );
+		const float ph = absFraction( _sample );
 		if( ph <= 0.25f )
 		{
 			return ph * 4.0f;
@@ -111,17 +111,17 @@ public:
 
 	static inline sample_t sawSample( const float _sample )
 	{
-		return -1.0f + fraction( _sample ) * 2.0f;
+		return -1.0f + absFraction( _sample ) * 2.0f;
 	}
 
 	static inline sample_t squareSample( const float _sample )
 	{
-		return ( fraction( _sample ) > 0.5f ) ? -1.0f : 1.0f;
+		return ( absFraction( _sample ) > 0.5f ) ? -1.0f : 1.0f;
 	}
 
 	static inline sample_t moogSawSample( const float _sample )
 	{
-		const float ph = fraction( _sample );
+		const float ph = absFraction( _sample );
 		if( ph < 0.5f )
 		{
 			return -1.0f + ph * 4.0f;
@@ -131,7 +131,7 @@ public:
 
 	static inline sample_t expSample( const float _sample )
 	{
-		float ph = fraction( _sample );
+		float ph = absFraction( _sample );
 		if( ph > 0.5f )
 		{
 			ph = 1.0f - ph;
diff --git a/include/lmms_math.h b/include/lmms_math.h
index 7c68c6bfb..3fd76896e 100644
--- a/include/lmms_math.h
+++ b/include/lmms_math.h
@@ -42,22 +42,41 @@ using namespace std;
 
 static inline float absFraction( const float _x )
 {
-	return( _x - ( _x >= 0.0f ? floorf( _x ) : floorf( _x ) - 1 ) );
+	return( _x - floorf( _x ) );
 }
 
 static inline float fraction( const float _x )
 {
-	return( _x - floorf( _x ) );
+	return( _x - floorf( _x ) - ( _x >= 0.0f ? 0.0 : 1.0 ) );
 }
 
 #else
 
+/*!
+ * @brief Returns the wrapped fractional part of a float, a value between 0.0f and 1.0f.
+ *
+ * absFraction( 2.3) =>  0.3
+ * absFraction(-2.3) =>  0.7
+ *
+ * Note that this not the same as the absolute value of the fraction (as the function name suggests).
+ * If the result is interpreted as a phase of an oscillator, it makes that negative phases are
+ * converted to positive phases.
+ */
 static inline float absFraction( const float _x )
 {
 	return( _x - ( _x >= 0.0f ? static_cast<int>( _x ) :
 						static_cast<int>( _x ) - 1 ) );
 }
 
+/*!
+ * @brief Returns the fractional part of a float, a value between -1.0f and 1.0f.
+ *
+ * fraction( 2.3) =>  0.3
+ * fraction(-2.3) => -0.3
+ *
+ * Note that if the return value is used as a phase of an oscillator, that the oscillator must support
+ * negative phases.
+ */
 static inline float fraction( const float _x )
 {
 	return( _x - static_cast<int>( _x ) );
diff --git a/src/core/Oscillator.cpp b/src/core/Oscillator.cpp
index b7dc5bff5..ecd1d6ea7 100644
--- a/src/core/Oscillator.cpp
+++ b/src/core/Oscillator.cpp
@@ -316,8 +316,7 @@ inline void Oscillator::recalcPhase()
 		m_phaseOffset = m_ext_phaseOffset;
 		m_phase += m_phaseOffset;
 	}
-	m_phase = absFraction( m_phase )+2;	// make sure we're not running
-						// negative when doing PM
+	m_phase = absFraction( m_phase );
 }