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

C#: Add float and double overloads to Mathf #71583

Merged
merged 1 commit into from
Jan 26, 2023

Conversation

raulsntos
Copy link
Member

@raulsntos raulsntos commented Jan 17, 2023

Background and motivation

  • Users complain that they have to add too many conversions to their code to handle mixing double and float 1.
    Situations where you have to mix floating-type values may happen often since the engine uses double for scalar values and float for vector values.
  • Users want to use our available math methods with double values.
    Currently they are implemented using real_t which defaults to float, this can be changed to double with --precision=double but I see no reason not to offer the ability to use double APIs even when using --precision=single.
  • Math operations that only use float may offer better performance by using System.MathF instead 2.
    Currently we use System.Math which uses double and then convert to float, there's a tiny performance cost and we may want to optimize this given how most of our math is used in areas where float is all that is required.

PR design

This PR aims to solve the mentioned issues by adding float and double overloads to all methods of Mathf. This allows the methods to be usable with float, double or real_t, regardless of the --precision flag.

  • By implementing double and float overloads in the same class, users can seamlessly use Mathf regardless of the floating type that they're using. This should help reduce the need for converting to float or real_t and allows for mixing different floating types which would return a double.
    This is the same behavior you would come to expect from math operations that mix float and double:
    float a = 4.0f;
    double b = 2.0;
    var c = a * b; // c is a double
  • Methods that take double are also available in builds that use --precision=single, allowing users to use the float or double methods depending on their use case.
  • Having dedicated double and float methods allow us to use different implementations optimized for the situation. The double methods use System.Math just like before, but now the float methods can take advantage of System.MathF which should result in a minor performance improvement.
  • Use aggressive inlining in hopes that the JIT replaces calls to Mathf with calls to the methods in System.Math and System.MathF directly.

Constants

One of the problems with this approach compared to alternative designs (see section below) is that when we only have one Math class we can't have constant overloads, so we end up with one of these options:

  • Constants are float:
    This looses precision that may be useful to users working with double.
  • Constants are double:
    This requires converting to float every time users are using float-based APIs.
  • Constants are both float and double:
    This means we need to duplicate every constant and give them different names, which tends to be ugly (e.g.: PiF and PiD).
  • Constants are real_t:
    This is the the way constants are currently defined today in Mathf, no changes.

Because I'm not really convinced with any of the options I choose to keep the status quo with this PR, constants remain defined as real_t just as before, this should allow ease of usage when working with real_t.

For users that require float or double constants my recommendation would be to get them from System.Math, System.MathF, System.Double and System.Single which contains most of the constants available in Mathf. Users are also free to define their own constants in their projects if they need to, and even publish libraries to NuGet for reusability.

Alternative Designs

Multiple Math classes

We could implement the the Math methods in separate classes such as MathF and MathD, this has the benefit of allowing us to use the same name for the constants but has the following problems:

  • Users need to explicitly use the right Math class.
    I'd expect most users would end up using MathD simply because it works everywhere since float implicitly converts to double, missing out on minor performance improvements.
  • Requires conversions when working with real_t.
    When working with real_t values, such as when using vector types, users can't assume the vector uses float so they'll be forced to use MathD and then convert to real_t. This could be fixed if we also provide a third Math class for real_t but that increases the maintainability burden.
  • Users can already use System.Math and System.MathF so this approach doesn't really provide as many benefits.

Generic Math

The new Generic Math feature introduced in .NET 7.0 would allow us to implement a Math class with generic T methods where T is INumber<T> which means our methods would work with any numeric type (int, long, float, double, ...) and even any non-primitive type like a custom user-defined type as long as it implements the INumber<T> interface.

Unfortunately, this would require using .NET 7.0 and we're currently using .NET 6.0 (we may consider switching to .NET 7.0 for the 4.0 release but nothing has been decided so far).

Also, considering how important performance is for Math, I wonder if generic math is fast enough. This might not be an issue when using full AOT though, but we also don't support that today in Godot.

Related issues and PRs

Footnotes

  1. https://github.com/godotengine/godot-proposals/issues/5403

  2. https://github.com/godotengine/godot-proposals/issues/5509

- Add `float` and `double` overloads to all methods of `Mathf`.
This allows the methods to be usable with `float`, `double` or `real_t`.
- Use `System.MathF` in the `float` overloads which may result in
better performance.
- Constants remain as `real_t` for the time being.
- Add aggresive inlining for methods that wrap `System.Math` calls.
@akien-mga akien-mga merged commit bacf594 into godotengine:master Jan 26, 2023
@akien-mga
Copy link
Member

Thanks!

@raulsntos raulsntos deleted the dotnet/math branch January 26, 2023 23:09
@akien-mga akien-mga changed the title C#: Add float an double overloads to Mathf C#: Add float and double overloads to Mathf Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

About the System.MathF and Godot.Mathf on .NET6
3 participants