Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

linearToLogScale and linearToLogScale in include/lmms_math.h do something completely unrelated to their function names #5344

Open
Cyp opened this issue Dec 12, 2019 · 8 comments
Milestone

Comments

@Cyp
Copy link
Contributor

Cyp commented Dec 12, 2019

eˣ = exp(x) = pow(M_E, x) ≠ x^e = pow(x, M_E) = "logToLinearScale"
log(x) ≠ pow(x, 1/M_E) = "linearToLogScale"

They seem to be used only in AutomatableModel.

@Cyp Cyp added the bug label Dec 12, 2019
@JohannesLorenz
Copy link
Contributor

Here are the functions:

lmms/include/lmms_math.h

Lines 197 to 208 in a9e3e70

static inline float logToLinearScale( float min, float max, float value )
{
if( min < 0 )
{
const float mmax = qMax( qAbs( min ), qAbs( max ) );
const float val = value * ( max - min ) + min;
float result = signedPowf( val / mmax, F_E ) * mmax;
return isnan( result ) ? 0 : result;
}
float result = powf( value, F_E ) * ( max - min ) + min;
return isnan( result ) ? 0 : result;
}

lmms/include/lmms_math.h

Lines 212 to 225 in a9e3e70

static inline float linearToLogScale( float min, float max, float value )
{
static const float EXP = 1.0f / F_E;
const float valueLimited = qBound( min, value, max);
const float val = ( valueLimited - min ) / ( max - min );
if( min < 0 )
{
const float mmax = qMax( qAbs( min ), qAbs( max ) );
float result = signedPowf( valueLimited / mmax, EXP ) * mmax;
return isnan( result ) ? 0 : result;
}
float result = powf( val, EXP ) * ( max - min ) + min;
return isnan( result ) ? 0 : result;
}

@JohannesLorenz
Copy link
Contributor

static in a header function does make no sense

@JohannesLorenz JohannesLorenz added this to the 1.2.2 milestone Jan 11, 2020
@PhysSong
Copy link
Member

@JohannesLorenz Why do you marked this for 1.2? Shouldn't we take care of compatibility issues?

static in a header function does make no sense

Not always, see this: https://gist.github.com/htfy96/50308afc11678d2e3766a36aa60d5f75

@JohannesLorenz
Copy link
Contributor

@PhysSong It's invalid and it's in stable-1.2. It might even generate wrong sounds.

Though... If we write a conversion function, that might break more than it might fix, and there are currently not many complaints from musicians. I think we can milestone it 1.3.

@JohannesLorenz JohannesLorenz modified the milestones: 1.2.2, 1.3.0 Feb 3, 2020
@PhysSong
Copy link
Member

PhysSong commented Feb 7, 2020

I think we may implement a 'power' scale(x^a) and upgrade the existing weird-and-fake logscale to it. In this way, we can support the old behavior while adding the correct behavior by converting the old 'logscale' to x^(1/e). However, it may confuse users.

@Rossmaxx
Copy link
Contributor

So basically, the powf() parameters should be swapped but wasn't swapped due to a backward compat issue. How much impact will this make?

@Rossmaxx
Copy link
Contributor

If there's a compat break, might as well re milestone it but let someone clarify.

@Rossmaxx
Copy link
Contributor

Oops

@Rossmaxx Rossmaxx reopened this Jun 17, 2024
@Rossmaxx Rossmaxx modified the milestones: 1.3, 1.3+ Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants