Use templates for common geometric constants

[rubiefawn]

lmms_constants.h has several common constants such as PI and E, with variants of each for float, double, and long double. Many of these are currently unused in the repo but are of potential use. This change replaces LD_PI, D_PI, and F_PI with lmms::numbers::pi, and so on, in the style of C++20's <numbers>.

Currently, the constants are set using literal values, but once C++20 is fully ready these can be changed to their std::numbers::* equivalents instead.

[messmerd]

You should use inline constexpr since the variables are in a header, though maybe it would be best to wait for C++20 (#7554) and GCC 10 where we'll get the <numbers> header.

[rubiefawn]

Updated to account for <numbers> and C++20:

• Marked all lmms_constants.h constants with an exact equivalent in <numbers> as deprecated
• Removed all lmms_constants.h constants where no variant is currently in use
• Using inline constexpr
• Using std::floating_point concept instead of typename

[messmerd]

This won't work. I suggested waiting for C++20 and the compiler upgrade since we are in the process of doing that now.

Currently we are using C++17 and GCC 9.3 for Linux and MinGW builds, so we can't use the <numbers> header or concepts yet.

If you wanted, maybe you could convert our mathematical constants to use the same names as those in <numbers>, but in the lmms::numbers namespace. Then once we upgrade to C++20, switching to <numbers> will be as easy as swapping out lmms::numbers::pi for std::numbers::pi. That may be quite a bit of work for you though, so it's up to you.

EDIT: The constants should probably be in a new lmms::numbers namespace to avoid collisions. It would basically just be a reimplementation of the parts of the numbers header that we use.

[rubiefawn]

I'm aware that we don't have C++20 ready yet. I made the suggested changes (and more) in advance under the assumption that this PR was now dependent on #7554, based off of your suggestion. If this is not the case I can change it back, I'm good with whatever.

My original intention with this PR was small in scope; I just wanted to clean up lmms_constants.h. That being said, I love your suggestion for lmms::numbers, and if updating all usage of the constants is on the table, I'd love to explore that.

[messmerd]

Ah gotcha. Sorry for the miscommunication. The C++20 support could happen tomorrow if people could review the PR, but upgrading our MinGW compiler and dependencies will be tricky and take some time.

I think the lmms::numbers idea would be good to implement. Function scope usages could use using namespace numbers; so that it doesn't need to be typed out fully as the clunkier numbers::pi or numbers::pi_v<float> in places where that would look better.
I'm not sure how big a task it would be, but it would probably be fairly easy with vscode or some other program to just search through src/, include/, and plugins/ and replace all.

[sakertooth]

If I understand @messmerd correctly, I agree with him. We should wait for C++20, remove lmms_constants.h, and then just use the numeric library provided by the standard. The more code we can cut down and remove, I see as a win.

[rubiefawn]

Just to clarify, there are several useful constants in lmms_constants.h that have no overlap with C++20 <numbers>. This PR isn't about removing that header entirely, it's just to clean it up. I agree with the "less is more" sentiment though.

On that note, I noticed panning_constants.h and panning.h and am wondering if those can be merged into one file (or even just moved to lmms_constants.h). Those could be considered further candidates for constant cleanup.

The current plan is to wait for #7554 before I make further changes. Input is welcome in the meantime!

[sakertooth]

#7554 is here, so you can try and make the switch to the numbers header @rubiefawn (we have partial C++20 support, but I think we should have the numbers header at least).

On that note, I noticed panning_constants.h and panning.h and am wondering if those can be merged into one file (or even just moved to lmms_constants.h). Those could be considered further candidates for constant cleanup.

It's up to you really (or anyone else that wants to give it a go). I personally think it could go multiple ways with no apparent downside.

[messmerd]

@sakertooth Nope, like most C++20 features, we're waiting for GCC 10.

See: https://en.cppreference.com/w/cpp/compiler_support/20
(Mathematical constants P0631R8)

[sakertooth]

Hard to classify it as an upgrade then, but I guess it's a step in the right direction..

[messmerd]

I'm using both class type NTTP and designated initializers in the pin connector now, so it's not nothing. Once I'm done with the pin connector, I'd like to upgrade to GCC 11 on Linux and MinGW 13 for the cross-compiled Windows builds. Then we'll have nearly all C++20 features including <numbers>.

[rubiefawn]

This is the second time I've gotten duplicated commit history after pushing. Not sure if GitHub just doesn't like rebase or if it's a skill issue (likely the latter). How do I avoid this in the future?

[Rossmaxx]

Don't worry, we do squash on merge.

[Rossmaxx]

Wait, seems like lots of unnecessary diffs (from previously merged prs)

[Rossmaxx]

Some thoughts which came across when I looked at the diff.

[Rossmaxx]

Looks good.
Untested but style and diff looks ok.

[rubiefawn]

What else does this need before it can be merged?

[Rossmaxx]

Some testing to check for regressions?