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

f32 vs f64 in Godot APIs #510

Open
Bromeon opened this issue Nov 30, 2023 · 8 comments · May be fixed by #703
Open

f32 vs f64 in Godot APIs #510

Bromeon opened this issue Nov 30, 2023 · 8 comments · May be fixed by #703
Labels
c: engine Godot classes (nodes, resources, ...) c: register Register classes, functions and other symbols to GDScript quality-of-life No new functionality, but improves ergonomics/internals

Comments

@Bromeon
Copy link
Member

Bromeon commented Nov 30, 2023

The confusion around the use of different floating-point types keeps coming up. A lot of people lose time and wonder why they need to convert the delta: f64 parameter when working with Vector2, which is f32 based. Even our Hello World tutorial needs to deal with this, distracting from more important topics. At the same time, it is arguable whether people typically need the 8-byte precision in games (although it's definitely nice to have it when truly needed).

In C++ and GDScript, this is less of a problem. GDScript only has one type float (which is 8 bytes except in vectors etc. if single precision builds are used). C++ has both but allows implicit conversions, so multiplying double with float is not such a ceremony as in Rust.

I see multiple options here.

  1. Make all API types f32 by default.

    • Allow people to opt-in to f64. Either we couple this to the existing double-precision feature, which is technically slightly different, but it could make sense that people who care about this enough would also want double-precision.
    • Alternatively, a separate feature. I would need to have very good reasons here; we already have lots of Cargo features and I want to avoid their proliferation -- to keep usage simple and since it's impossible to test all combinations properly. I'd almost say we should try combining it with the existing feature first, and then wait for concrete user feedback.
  2. Magic on the API boundary.

    • For engine methods, we could accept impl Into<f64> parameters. However, return types would still need to be f64 or f32.
    • For user-defined #[func] methods, proc-macros could silently translate f32 in parameter and return-type position to f64. This would however be a departure from "use pure Rust with attributes" and could also confuse static analysis tools, as well as users that encounter signatures different from the trait methods.
  3. Keep using both f32 and f64, but integrate them wherever possible.

    • Vector operators would accept both parameter types.
    • This falls short since vector fields still have one type, and return types also have one type. We will still need conversions, just in some places not. Not sure if this truly makes code clearer.
  4. Use custom types.

    • Either a generic Float that can be converted to both f32 and f64. It could have some integrations like multiplications with vectors etc. But in practice, it won't simplify that much -- you'll use delta.as_f32() instead of delta as f32, so ergonomics are questionable.
    • Use specialized types like Duration, Angle etc. While very interesting by itself, we might not be ready to add those, as we'd need to manually annotate the entire Godot API surface.
    • In general, I'm not sure we win that much here. Rust doesn't have implicit conversions (apart from the impl Into parameter trick), so having a new type means mostly that you now even need to convert to f64.
    • Also, it makes the type system more complex -- there is f32, f64, real and now this.
  5. Don't care.

    • We say that the current solution is the best possible one. This would be the rusty approach, saying "things are maximally explicit, your problem if the code is verbose".
    • Not sure if I particularly like this approach, especially in gamedev where pragmatic approaches and productivity are often preferred. See also Ergonomics and Panics for a similar discussion.
    • There is clear indication from Discord help requests that the status quo brings some confusion. We don't have a good explanation why f32 and f64 are mixed apart from "that's what Godot does". And maybe that's enough of an explanation, but to some extent it's a cop-out.
@Bromeon Bromeon added quality-of-life No new functionality, but improves ergonomics/internals c: register Register classes, functions and other symbols to GDScript c: engine Godot classes (nodes, resources, ...) labels Nov 30, 2023
@StatisMike
Copy link
Contributor

StatisMike commented Nov 30, 2023

IMO 5. is the best option from integration safety standpoint. Additional as f32 shouldn't be much of productivity loss - IMO it makes user stop and wonder why this takes f32 while I get f64? only on the first usage. I was one of these users. Just boldface in FAQ on the top. It may seem like a cop-out, but IMO it is the safest and most reasonable way.

From other points, first point of 2. seems fairly safe and reasonable. I don't think that we should touch return types - I don't know if somehow-somewhere the return type conversion wouldn't make us differ too much from Godot. Return types from Godot methods won't always go straight into another Godot methods straight away. On the other hand Godot itself seems to be doing something like hidden type conversion with its Float on entrypoints, so we should be safe there and in-line with its behavior. If anyone want to pass raw f64 delta to some Rust function during processing they would be able to.

I would also take 1. into consideration if we would like to keep things explicit. Also seems reasonable, but I have some uneasy feeling about that which I can't really articulate.

@lilizoey
Copy link
Member

Note that we can actually define a function like this atm:

#[func]
fn foo(&self, f: f32) -> f32 {
  f
}

@bluenote10
Copy link
Contributor

I would be in favor of (5) as well. Rationale:

  • The conversions don't bother me much. I actually find it helpful to be reminded of where I have full vs half precision. After all we can always just spend one line let delta = delta as f32; if we locally prefer to have it in f32 only.
  • The other solutions sound like they involve quite a bit of complexity/magic that could make it harder to understand what is happening in the bindings.

My least favorite would be (1), in particular the first sub-bullet-point variation. Looking at the Godot source shows that there are quite a few things that are explicitly typed as double (crude measurement: double appears almost 5000 times in Godot's headers alone), and that extra precision seems to forward into the function interfaces. I haven't looked in detail for which interfaces the full precision might be particularly relevant (mostly timing related things come to mind), but it would be a pity if the full precision gets thrown away in the bindings by default. Coupling the feature to double-precision would then be rather painful: Consider a use case that needs an f64 in just one particular function interface for the extra precision -- but then it requires to switch the entire engine to 64-bit including rendering pipelines.

@grtwje
Copy link

grtwje commented Dec 1, 2023

Going against the consensus so far, I like (1). :)

I respect the Rust purity arguments above, but I also get tired of writing monstrosities like:

fn integrate_forces(&mut self, &mut physics_state: Gd<PhysicsDirectBodyState2D>) {
    let mut xform = physics_state.get_transform();
    xform.origin.x = wrapf(
        f64::from(xform.origin.x),
        0.0,
        f64::from(self.screen_size.x),
    ) as f32;

since Vector2 is f32, but the godot::engine::utilities take f64. (I'm new to Rust and Godot. Maybe I'm just doing it wrong.)

Another option would be to extend the API. For example, pub fn wrapf_32 or pub fn wrapf<T>.

@bluenote10
Copy link
Contributor

If you use wrapf a lot why don't you just add a helper function on your side and use that instead?

fn wrapf(x: f32, min: f32, max: f32) -> f32 {
    godot::engine::utilities(x as f64, min as f64, max as f64) as f32
}

Note that the majority of things in godot::engine::utilities can be just done directly in Rust like clamp/floor/ceil/min/max, all the trigonometric functions etc. No need for FFI calls in most cases. wrapf doesn't have a direct equivalence in Rust to my knowledge, but if you need it frequently you might even want to implement it natively as an extension trait, something like that (untested):

trait FloatWrap {
    fn wrap(self, lower: f32, upper: f32) -> f32;
}

impl FloatWrap for f32 {
    fn wrap(self, lower: f32, upper: f32) -> f32 {
        let range = upper - lower;
        let mut result = (self - lower) % range;
        
        if result < 0.0 {
            result += range;
        }

        result + lower
    }
}

And than your example becomes:

fn integrate_forces(&mut self, &mut physics_state: Gd<PhysicsDirectBodyState2D>) {
    let mut xform = physics_state.get_transform();
    xform.origin.x = xform.origin.x.wrap(0.0, self.screen_size.x);
}

My point is: There are always rather simple options to work around the "noise" if needed. In my opinion, this doesn't justify just throwing away any double precision in all bindings everywhere.

@Bromeon
Copy link
Member Author

Bromeon commented Dec 1, 2023

I actually find it helpful to be reminded of where I have full vs half precision.

Generally, I agree.

Anything around linear algebra should be f32 (or real), in my opinion. Vectors are often foundational in gamedev logic, so using f64 in some but not all places doesn't add precision but just noise. People would need to do component-wise math or use a separate vector library to truly benefit from the extra precision.

This is already mostly the case, but there are remaining inconsistencies in Godot. Transform2D::tdotx() returns f64 but is implemented as a real operation. Same for determinant() and others. In these cases, we could probably just fix the extension_api.json or have our builtin wrapper use f32 (which is already the case).


After all we can always just spend one line let delta = delta as f32; if we locally prefer to have it in f32 only.

My point is: There are always rather simple options to work around the "noise" if needed. In my opinion, this doesn't justify just throwing away any double precision in all bindings everywhere.

I don't think we should just accept this 🙂 Noise is noise, and in this example, every single process() and physics_process() function ever written has to become more complex than necessary, just to accomodate for seemingly arbitrary API decisions. Do we really need f64 precision for delta? I guess it could reduce the error when summing up frame deltas over larger periods of time, but is that measurement even accurate enough? For anything related to game logic, one should use physics_process. That one has a constant frame time and can also be more accurately obtained via Engine::get_physics_ticks_per_second.

The magic that lilizoey mentioned might be a trade-off, but for now it works more by accident than by design 😉 and if the signature differs from the trait, it means it's less discoverable, IDE completions will still give you the f64 one, API docs will still show f64, etc.


Overall, I get the feeling that we need to look more closely at individual APIs before making a big decision. Maybe if we manually set delta: f32 and a couple more similar cases, we would already relieve a lot of the pain, and remaining f64 cases would be OK. For wrapf and related utility functions, we could consider a more general approach like FloatExt::lerp.

So if anyone encounters concrete cases of friction, please post them here!

@ogapo
Copy link

ogapo commented Dec 1, 2023

I'd prefer #5 as well. As much as I agree ergonomics are paramount in games, a cast seems fairly minor and I'm a little worried about introducing subtle bugs due to round tripping f64's from C++ through gdext and back into Godot. If we squeeze through f32's "under the hood" there it can be pretty surprising for some godot code using a 3rd party gdext module for instance to get back not what they sent in.

@Bromeon
Copy link
Member Author

Bromeon commented May 6, 2024

This came up in C#, but was decided against 😀 godotengine/godot#74070
Reasons primarily being API stability and complexity through class hierarchy. They do agree with my point that double (f64) has negative effects on usability, though.

Nonetheless, an important argument was mentioned:

The double precision version of the delta is still available through the GetProcessDeltaTime() and GetPhysicsProcessDeltaTime(). I optimized the methods to return a cached value instead of going through an internal call. I also added two matching properties to reduce verbosity.

For people who really need double in single-precision builds -- which I'd wager is the minority -- it is available through the above functions.

Since a lot of people mentioned "a bit of boilerplate doesn't hurt" in this discussion, I'm sure it's fine if the rare f64 cases now get that boilerplate, and common users stay free of it 😛

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: engine Godot classes (nodes, resources, ...) c: register Register classes, functions and other symbols to GDScript quality-of-life No new functionality, but improves ergonomics/internals
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants