Fix snapping logic in Range

[aaronfranke]

This PR is a fix for issue #107095. Fixes #107095.

This PR changes Range's step feature to use r128.h instead of the existing algorithm.

In master:

0.1 -> 0.09999999999126885
0.2 -> 0.19999999999708962
0.3 -> 0.29999999998835847
0.4 -> 0.39999999999417923
0.5 -> 0.5
0.6 -> 0.5999999999912689
0.7 -> 0.6999999999970896
0.8 -> 0.7999999999883585
0.9 -> 0.8999999999941792
1.0 -> 1.0
1.1 -> 1.0999999999912689
1.2 -> 1.1999999999970896
1.3 -> 1.2999999999883585

With this PR: everything works as expected.

Old PR description:

This PR is a partial fix for issue #107095, but not a full fix.

This PR changes Range's step feature to use Math::snapped instead of this custom algorithm.

With this PR:

0.1 -> 0.09999999999999999
0.2 -> 0.19999999999999998
0.3 -> 0.3
0.4 -> 0.39999999999999997
0.5 -> 0.5
0.6 -> 0.6
0.7 -> 0.7
0.8 -> 0.7999999999999999
0.9 -> 0.8999999999999999
1.0 -> 1.0
1.1 -> 1.0999999999999999
1.2 -> 1.2

The new results with this PR are approximately 99.9995% closer to the desired value, and in some cases, match exactly.

Ok... but what about fixing this further? Unfortunately, it's not really possible to do that. The step value we're plugging into Math::snapped simply doesn't have enough precision for this. When a property hint has a step size of 0.001, that gets converted into the nearest possible float, which is technically 0.00100000000000000002081668171172. And, the number 0.1 is technically not a multiple of that value, even though it is a multiple of the decimal 0.001.

The only solution I can think of to fix this further would be to use 128-bit floats instead of 64-bit floats, store the step size in 128-bit, do the math in 128-bit, and then store the computed value as 64-bit. If we were using C++23 or later, we could switch to std::float128_t. Without C++23, the best we could do is platform-specific compiler extensions or a library. My recommendation is, for now, merge the 99.9995% fix in this PR while Godot uses C++17, and then we can switch the step size to 128-bit in the future when Godot requires C++23.

[ze2j]

Before if Range's minimum was 0.1 and the step was 0.2, it would snap to values such as 0.1, 0.3, 0.5, and so on, but now it will snap to 0.1, 0.2, 0.4, 0.6, and so on. This behavior is kinda weird, and wasn't documented anywhere - in fact, the documentation explicitly states the behavior in this PR: "value will always be rounded to a multiple of this property's value" (see here).

For me, this behavior is expected and very natural. The minimum value also acts as an offset and gives a lot more control on the range values. With this change, how to specify a range of some odd numbers for example?

[aaronfranke]

@ze2j Updated the PR to restore that functionality, and document it. To my surprise... this is actually still way more precise than in master, even though at a glance it looks like it's functionally the same. It's actually a toss-up which is more precise, in some cases it's actually more precise to do it this way, somehow. The note about eventually switching to 128-bit floats is still the case, though, as it's still wrong for some values as it is now.

[ze2j]

The accuracy difference may be explained by the ::round implementation (which is platform dependent).

This PR has also the added benefit to be more predictable than the master one as the ::floor(f + 0.5) trick force the rounding method (toward plus infinity instead of away from 0).

[clayjohn]

Does this improve the serialization situation reported in #107095?

[aaronfranke]

@clayjohn Yes.

[clayjohn]

So will this close #107095? In your description you say it is only a partial fix.

I'm hoping we can close #107095 after we merge this since it's a release blocker right now.

[aaronfranke]

We could either close it, or consider that this PR demotes it to no longer being a release blocker. Either way works for me.

[Ivorforce]

Sounds like it's an improvement over master. Code looks good to me.

My 2c, I'd say this keeps #107095 at "Bad" as a release blocker, since it doesn't fully fix it.

Ok... but what about fixing this further? Unfortunately, it's not really possible to do that.
The only solution I can think of to fix this further would be to use 128-bit floats instead of 64-bit floats, store the step size in 128-bit, do the math in 128-bit, and then store the computed value as 64-bit.

There's a better solution: Store step_size as a fixed-point number. This should allow us to store the exact value the user entered in UI (e.g. 0.01), instead of a floating point approximation, and snap the values for storage (e.g. FixedPoint snap(double val, FixedPoint snap)) accurately as well. I think this is more appropriate than a 128 bit float.

This does not require c++20, but it does require quite a lot more implementation work. So it may not be an option for 4.5.

[aaronfranke]

@Ivorforce I forgot about fixed point, good idea! Godot actually already has thirdparty/misc/r128.h in its source code, so I've updated the PR to use that. It works for all cases now (as long as users don't try to use values above 2^63 because then it will explode... but doubles become useless far before then anyway, so it's very unlikely).

[Ivorforce]

I'm happy to see that fixed points solve the issue for good! :)

The new version looks kinda slow. I suppose it's not used in any performance critical operations (just UI)?

[KoBeWi]

Range is UI-only. Aside from editing numeric properties, it's used for things like progress bars, but nothing really performance critical.

[Repiteo]

Thanks!

[aaronfranke]

Changes to Range turned out to be risky, so I'm un-marking this as cherry-pick-able.