Update math functions to C++ standard library #7685

rubiefawn · 2025-02-03T22:44:42Z

This updates usages of sin(), cos(), tan(), pow(), exp(), log(), log10(), sqrt(), fmod(), fabs(), and fabsf(), excluding any usages that look like they might be part of a submodule or 3rd-party code. There's probably some math functions not listed above that haven't been updated yet.

I haven't touched any of the following items. If these should be updated, I can do that:

plugins/CarlaBase/*
plugins/LadspaEffect/*
plugins/ZynAddSubFx/*
src/3rdparty/*
Any file matching *test*

Note

Updating these function usages was brought up in #7558, somewhat related, probably not important.

This updates usages of sin, cos, tan, pow, exp, log, log10, sqrt, fmod, fabs, and fabsf, excluding any usages that look like they might be part of a submodule or 3rd-party code. There's probably some std math functions not listed here that haven't been updated yet.

lmao one always sneaks by

Rossmaxx

It took a bit to review this, and here it is.

Also i could only grab a few cases where pow -> exp2 /fastPow10f is applicable. There's lots more you can change. Would appreciate if you did.

plugins/Compressor/Compressor.cpp

plugins/Eq/EqCurve.cpp

plugins/Eq/EqFilter.h

plugins/SpectrumAnalyzer/SaProcessor.cpp

src/core/AutomationClip.cpp

src/core/LadspaManager.cpp

src/gui/editors/AutomationEditor.cpp

- std::pow(2, x) -> std::exp2(x) - std::pow(10, x) -> lmms::fastPow10f(x) - std::pow(x, 2) -> x * x, std::pow(x, 3) -> x * x * x, etc. - Resolve TODOs, fix typos, and so forth Co-authored-by: Rossmaxx <74815851+Rossmaxx@users.noreply.github.com>

I mistakenly introduced a bug in my recent PR regarding template constants, in which a -1 that was supposed to appear outside of an abs() instead was moved inside it, screwing up the generated waveform. I fixed that and also simplified the function by factoring out the phase domain wrapping using the new `ediv()` function from this PR. It should behave how it's supposed to now... assuming all my parentheses are in the right place lol

What else is lmms::numbers for?

Rossmaxx

This is left

include/lmms_math.h

Co-authored-by: Rossmaxx <74815851+Rossmaxx@users.noreply.github.com>

Rossmaxx

LGTM

rubiefawn · 2025-02-04T08:48:39Z

Based off the myriad of link errors in the CI, it appears that functions defined in headers are not inline by default after all. I'll fix it tomorrow, for now I sleep

Rossmaxx · 2025-02-04T08:57:51Z

I thought they were inlined. Sorry @rubiefawn for confusing you.

LostRobotMusic · 2025-02-04T15:48:00Z

Only class member functions are marked as inline by default. All others aren't. What file it's in doesn't matter.

(Crucially, "marked inline by default" does not mean "inlined by default", the compiler will happily ignore your request if it thinks it knows better.)

For functions, constexpr implies inline so this just re-adds inline to the one that isn't constexpr yet

include/lmms_math.h

plugins/Lb302/Lb302.cpp

plugins/Organic/Organic.cpp

src/core/DrumSynth.cpp

messmerd · 2025-02-04T20:30:52Z

src/core/DrumSynth.cpp

+		w = std::sin(p);
+		break;
+	case 1: // sine^2
+		w = std::abs(2.f * std::sin(.5f * p)) - 1.f;


Starting with the original:

static_cast<float>(fabs(2.0f * static_cast<float>(sin(fmod(0.5f * ph, TwoPi))) - 1.f));

I've tested that the sin(fmod(0.5f * ph, TwoPi)) part is equivalent to:

std::sin(ph / 2)

Plugging that back in, we get:

Suggested change

w = std::abs(2.f * std::sin(.5f * p)) - 1.f;

w = std::abs(2 * std::sin(ph / 2) - 1);

I've confirmed that this result is identical to the original.

The "original" referenced as well as your suggested change have the - 1 inside the abs(), which is actually an error ~~I accidentally introduced in my last PR~~ introduced in b2f2fc4. The changes I made here fix that, and reference the original implementation (from
before b2f2fc4 screwed it up). Here are the graphs of both; note that the incorrect version has a range of [0, 3] while the correct one has a range of [-1, 1]:

src/core/DrumSynth.cpp

messmerd · 2025-02-04T21:08:55Z

src/core/DrumSynth.cpp

+		w = p * 2.f * numbers::inv_pi_v<float>;
+		if (w > 1.f) { w = 2.f - w; }


Suggested change

w = p * 2.f * numbers::inv_pi_v<float>;

if (w > 1.f) { w = 2.f - w; }

w = std::abs((2 / numbers::inv_pi_v<float>) * std::remainder(ph, 2 * numbers::inv_pi_v<float>)) - 1;

I found this function which works.

std::remainder is not the same as Desmos's mod. In Desmos it would be x - round(x / y) * y.

When you do that, you'll get a triangle wave. However, after testing it some more, both your triangle wave and mine appear to be off from the original. I'll look into it some more and see what I can find out.

Updated graph; thanks for pointing out the difference between mod() and std::remainder().

messmerd · 2025-02-04T21:31:28Z

src/core/DrumSynth.cpp

+		if (w > 1.f) { w = 2.f - w; }
+		break;
+	case 3: // sawtooth
+		w = p * numbers::inv_pi_v<float> - 1.f;


Suggested change

w = p * numbers::inv_pi_v<float> - 1.f;

w = (ph - (2 * numbers::inv_pi_v<float>) * static_cast<int>(ph / (2 * numbers::inv_pi_v<float>))) / numbers::inv_pi_v<float> - 1;

I think the previous equation was pretty good.

I'm confused; the previous equation is not what is suggested here?

Updated graph with new code changes:

rubiefawn · 2025-02-05T05:19:57Z

Regarding several of the DrumSynth suggested changes, what is the benefit to using std::remainder() over std::fmod() when the former doesn't wrap negative inputs to positive outputs (the triangle waveform, for example, is only valid with input domain [0, τ])? ~~If std::remainder() is significantly faster, then I'm down to use it, I'll just have to prove the other code can't call waveform() with negative phases.~~ Updated to not use std::fmod() either

Co-authored-by: Dalton Messmer <messmer.dalton@gmail.com>

No ediv(), no std::fmod(), no std::remainder(), just std::floor(). It should all work for negative phase inputs as well. If I end up needing ediv() in the future, I can add it then.

This reuses more work and is also a lot more easy to visualize. It's probably a meaningless micro-optimization, but it might be worth changing it back to a switch-case and just calculating ph_tau and saw01 at the beginning of the function in all code paths, even if it goes unused for the first two cases. Guess I'll see if anybody has strong opinions about it.

messmerd · 2025-02-08T04:10:58Z

src/core/DrumSynth.cpp

+	// sine
+	if (form == 0) { return std::sin(ph); }
+	// sine^2
+	if (form == 1) { return 2.f * std::abs(std::sin(0.5f * ph)) - 1.f; }


Suggested change

if (form == 1) { return 2.f * std::abs(std::sin(0.5f * ph)) - 1.f; }

if (form == 1) { return std::abs(2.f * std::sin(0.5f * ph) - 1.f); }

If ph==0, your version would output -1 while the old function would output +1.

Here's what I've been using to test my functions:
https://gist.github.com/messmerd/5ee2e0078b574b9878bdd7224192531e

I've added your functions there too for comparison

If ph==0, your version would output -1 while the old function would output +1.

Yes, and the old function wasn't supposed to. It's an error introduced in b2f2fc4; see the graphs of the functions: #7685 (comment)

Consider the original code prior to b2f2fc4:

// original code case 1: w = (float)fabs(2.0f*(float)sin(fmod(0.5f*ph,TwoPi)))-1.f; // delete casts, it's all floats anyways case 1: w = fabs(2.f * sin(fmod(0.5f * ph, TwoPi))) - 1.f; // remove unnecessary fmod(), sin() is well-defined for inputs outside [0, τ] case 1: w = fabs(2.f * sin(0.5f * ph)) - 1.f; // convert c-style math functions to std::* c++ counterparts case 1: w = std::abs(2.f * std::sin(0.5f * ph)) - 1.f; // new code if (form == 1) { return 2.f * std::abs(std::sin(0.5f * ph)) - 1.f; } // new code, fixed for consistency if (form == 1) { return std::abs(2.f * std::sin(0.5f * ph)) - 1.f; }

So my "new" version is in fact identical to the original, intended version, with the exception of the 2.f * being outside the abs() rather than inside it. This was an unintentional change, but has no effect on the result in this case. I'll move it back inside to make it match the original better for a cleaner diff history though. (Edit: that has now been done)

rubiefawn mentioned this pull request Feb 3, 2025

Use templates for common geometric constants #7558

Merged

fix std::sqrt typo

9d74185

lmao one always sneaks by

Rossmaxx reviewed Feb 4, 2025

View reviewed changes

rubiefawn and others added 5 commits February 3, 2025 23:51

Apply code review suggestions

3f60985

- std::pow(2, x) -> std::exp2(x) - std::pow(10, x) -> lmms::fastPow10f(x) - std::pow(x, 2) -> x * x, std::pow(x, 3) -> x * x * x, etc. - Resolve TODOs, fix typos, and so forth Co-authored-by: Rossmaxx <74815851+Rossmaxx@users.noreply.github.com>

Merge remote-tracking branch 'origin/cpp-math' into cpp-math

896f0ed

Annotate magic numbers with TODOs for C++20

72bbdff

On second thought, why wait?

8affecb

What else is lmms::numbers for?

Rossmaxx reviewed Feb 4, 2025

View reviewed changes

include/lmms_math.h Outdated Show resolved Hide resolved

include/lmms_math.h Outdated Show resolved Hide resolved

rubiefawn and others added 2 commits February 4, 2025 01:27

begone inline

9cd2476

Co-authored-by: Rossmaxx <74815851+Rossmaxx@users.noreply.github.com>

begone other inline

0696bbb

Co-authored-by: Rossmaxx <74815851+Rossmaxx@users.noreply.github.com>

Rossmaxx approved these changes Feb 4, 2025

View reviewed changes

Rossmaxx requested a review from messmerd February 4, 2025 08:29

Re-inline function in lmms_math.h

7d266ca

For functions, constexpr implies inline so this just re-adds inline to the one that isn't constexpr yet

messmerd reviewed Feb 4, 2025

View reviewed changes

rubiefawn and others added 4 commits February 4, 2025 22:51

Formatting fixes, readability improvements

14e7d41

Co-authored-by: Dalton Messmer <messmer.dalton@gmail.com>

Fix previously missed pow() calls, cleanup

cbc8749

Co-authored-by: Dalton Messmer <messmer.dalton@gmail.com>

Just delete ediv() entirely lmao

5a2aafa

No ediv(), no std::fmod(), no std::remainder(), just std::floor(). It should all work for negative phase inputs as well. If I end up needing ediv() in the future, I can add it then.

messmerd reviewed Feb 8, 2025

View reviewed changes

Move multiplication inside abs()

f4e0b29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update math functions to C++ standard library #7685

Update math functions to C++ standard library #7685

rubiefawn commented Feb 3, 2025

Rossmaxx left a comment

Rossmaxx left a comment

Rossmaxx left a comment

rubiefawn commented Feb 4, 2025

Rossmaxx commented Feb 4, 2025

LostRobotMusic commented Feb 4, 2025

messmerd Feb 4, 2025

rubiefawn Feb 5, 2025 •

edited

Loading

messmerd Feb 4, 2025

rubiefawn Feb 5, 2025

messmerd Feb 8, 2025

rubiefawn Feb 8, 2025

messmerd Feb 4, 2025

rubiefawn Feb 5, 2025

rubiefawn Feb 8, 2025

rubiefawn commented Feb 5, 2025 •

edited

Loading

messmerd Feb 8, 2025

rubiefawn Feb 8, 2025 •

edited

Loading

	w = std::abs(2.f * std::sin(.5f * p)) - 1.f;
	w = std::abs(2 * std::sin(ph / 2) - 1);

		w = p * 2.f * numbers::inv_pi_v<float>;
		if (w > 1.f) { w = 2.f - w; }

	w = p * 2.f * numbers::inv_pi_v<float>;
	if (w > 1.f) { w = 2.f - w; }
	w = std::abs((2 / numbers::inv_pi_v<float>) * std::remainder(ph, 2 * numbers::inv_pi_v<float>)) - 1;

	w = p * numbers::inv_pi_v<float> - 1.f;
	w = (ph - (2 * numbers::inv_pi_v<float>) * static_cast<int>(ph / (2 * numbers::inv_pi_v<float>))) / numbers::inv_pi_v<float> - 1;

	if (form == 1) { return 2.f * std::abs(std::sin(0.5f * ph)) - 1.f; }
	if (form == 1) { return std::abs(2.f * std::sin(0.5f * ph) - 1.f); }

Update math functions to C++ standard library #7685

Are you sure you want to change the base?

Update math functions to C++ standard library #7685

Conversation

rubiefawn commented Feb 3, 2025

Rossmaxx left a comment

Choose a reason for hiding this comment

Rossmaxx left a comment

Choose a reason for hiding this comment

Rossmaxx left a comment

Choose a reason for hiding this comment

rubiefawn commented Feb 4, 2025

Rossmaxx commented Feb 4, 2025

LostRobotMusic commented Feb 4, 2025

Choose a reason for hiding this comment

rubiefawn Feb 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rubiefawn commented Feb 5, 2025 • edited Loading

Choose a reason for hiding this comment

rubiefawn Feb 8, 2025 • edited Loading

Choose a reason for hiding this comment

rubiefawn Feb 5, 2025 •

edited

Loading

rubiefawn commented Feb 5, 2025 •

edited

Loading

rubiefawn Feb 8, 2025 •

edited

Loading