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

Add bevy_xpbd_2d_f64 and bevy_xpbd_3d_f64 crates #7

Closed
wants to merge 5 commits into from

Conversation

johanhelsing
Copy link
Contributor

A drawback of substepping is that it does not damp out high
frequency vibrations due to reduced numerical damping. This can
yield visual jittering. However, numerical damping can easily be
reintroduced by adding true physical damping. Also, for small time
steps, double precision floating point numbers are required. While
doubles are as fast as floats on CPUs, they currently still reduce the
performance on GPUs. Updating constraint directions after each
projection might cause instabilities when simulating tall stacks or
piles of objects. Investigating this problem is one of our directions
of future work.

Had a go at making crates with different precision levels, since it seems there are good reasons to prefer f64.

What do you think? It adds a bit of complexity to the crate, though... perhaps it's better to just scrap the f32 versions, like in @Aceeri's https://github.com/Jondolf/bevy_xpbd/compare/main...Aceeri:bevy_xpbd:doubles?expand=1 ?

@johanhelsing johanhelsing changed the title Add bevy_xpbd_2d_f64 crate Add bevy_xpbd_2d_f64 and bevy_xpbd_3d_f64 crates Mar 16, 2023
@Jondolf
Copy link
Owner

Jondolf commented Mar 16, 2023

We should definitely make f64 an option, but I think it'd be better to keep f32 as the default since it's what bevy and most crates use, and it's also a bit more efficient in terms of space and SIMD.

I'm not entirely sure what the best approach would be, but intuitively I'd say that we should just add an f64 feature and use #cfg(not(feature = "f64")) and #cfg(feature = "f64") to make a distinction between f32 and f64. In my opinion this would be simpler than having both f32 and f64 features and two additional crates for f64.

With this approach we probably won't be able to have examples where you can choose the type, so we would either have to choose a default (probably f32) or opt for some other way to implement examples. Would there be any other drawbacks to this approach that I'm missing?

@johanhelsing
Copy link
Contributor Author

johanhelsing commented Mar 16, 2023

cargo features are supposed to be additive, the reasoning being that if something depends on bevy_xpbd_2d and one of your dependencies (let's call it foo) depend on bevy_xpbd_2d as well, your dependency should be able to safely require extra features for bevy_xpbd_2d.

If that feature is f64, but you expect to operate on f32 it will most likely break your app. On the other hand, if the dependency just uses another crate, everyone will be happy (though they would get different physics worlds :/).

However, I'm not sure how common this scenario would be, but that's why I felt it was kind of "dirty" to have it be the same crate.

Same reason why 2d and 3d are different crates, no?

But yeah, probably will be simpler to have it just in one crate, maybe it's better at a starting point, then we can see how common of a problem this would be.

From my experiments with increased number of substeps (low sub-dt), though f32s low precision really seems to be a problem though.

@Jondolf
Copy link
Owner

Jondolf commented Mar 16, 2023

Yeah, I can understand the feature additivity argument as well. Simultaneously having one physics world based on f32 and another based on f64 feels like a pretty uncommon scenario though, and there would probably be some workarounds for those cases as well.

I was also on the fence regarding 2D and 3D, but ended up making them into separate crates since it felt like there was a decently large and concrete difference in both implementation and usage. Also, it made it easy to make separate 2D and 3D examples.

I'm still not entirely sure which approach is better though; both have their pros and cons.

@Jondolf Jondolf added the C-Enhancement New feature or request label Mar 16, 2023
@johanhelsing
Copy link
Contributor Author

Yeah, lets go with only two crates for now.

@johanhelsing johanhelsing mentioned this pull request Mar 17, 2023
@Jondolf
Copy link
Owner

Jondolf commented Mar 17, 2023

By the way, this seems to break the examples, since there are 2 executables for each example, one for f32 and another for f64. I'm not sure if there's a way to specify which crate's example to run (unless we make an example runner binary)? Manually setting the features didn't seem to be enough.

@johanhelsing
Copy link
Contributor Author

Yeah, i made two of the examples work on both kinds of precision, by using the Vector and Scalar types, but they are worse examples aa a result (cubes and move_marbles)

@johanhelsing
Copy link
Contributor Author

Closing in favor of #20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants