Skip to content

Add f64 feature #20

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 14 commits into from
Mar 22, 2023
Merged

Add f64 feature #20

merged 14 commits into from
Mar 22, 2023

Conversation

johanhelsing
Copy link
Contributor

Alternative to #7, but uses features instead of crates.

@johanhelsing
Copy link
Contributor Author

I tried having only a f64 and not a f32 feature, but ran into issues, there is no way to specify that you want a dependency when you don't have a feature enabled, and we need to select either parry2d or parry2d-f64.

See: rust-lang/cargo#1839

@Jondolf
Copy link
Owner

Jondolf commented Mar 17, 2023 •
edited
Loading

Yeah, if we opt for this feature route, using f64 would require disabling default features and manually setting the features for f64 and the desired dimension. The original 4 crate method is a bit more complex, but it could be more versatile and explicit in general.

I'm still not sure which one is better, but I think we can start with this feature method since it's simpler. We can add separate crates later if it ends up being a better approach.

@johanhelsing johanhelsing mentioned this pull request Mar 19, 2023
Closed
@Jondolf Jondolf added the C-Enhancement New feature or request label Mar 19, 2023
@Jondolf
Copy link
Owner

Jondolf commented Mar 19, 2023

Would it make sense to create a math module and put the math types and impls there? It would declutter lib.rs a bit. We could add the modules math/single.rs and math/double.rs and reimport the types in math/mod.rs depending on the float type.

@johanhelsing
Copy link
Contributor Author

Would it make sense to create a math module and put the math types and impls there?

Sounds like a good idea. I don't have time to pick this up again until Thursday at the earliest. So feel free to take over if you'd like to.

Jondolf added 4 commits March 20, 2023 21:27
@Jondolf
Merge remote-tracking branch 'origin/main' into f64-feature
0cb119f
@Jondolf
Fix dependencies and features
5859258 
- Added parry f64 variants for simd and enhanced_determinism features

- Added f32 as 2D default feature

- Added dep: prefixes to dependencies enabled by features

- Added "?" for features of optional dependencies
@Jondolf
Create math module for math types and impls
b1b81ff 
`f32` types are in `math/single.rs`.

`f64` types are in `math/double.rs`.

`math/mod.rs` reimports the correct types depending on the features.
@Jondolf
Make Clippy happy
cce09fa
@Jondolf
Copy link
Owner

Jondolf commented Mar 20, 2023 •
edited
Loading

Okay, everything should be good to go now. I just fixed some dependency/feature stuff, created the math module and "fixed" Clippy warnings.

Edit: Nevermind, CI failed

@johanhelsing
Copy link
Contributor Author

I like how the math module turned out. Definitely a lot less clutter now 👍

Jondolf added 3 commits March 21, 2023 18:25
@Jondolf
Update insta snapshots for f64
e65ee7e
@Jondolf
Fix insta snapshots
0f629d7
@Jondolf
Make all examples require f32
20eaacb 
Previously they were generic over the float type, but it
was a bit more complex and didn't reflect actual usage.
@Jondolf
Copy link
Owner

Jondolf commented Mar 21, 2023

Okay, now everything should be working.

@Jondolf Jondolf merged commit daf62fe into Jondolf:main Mar 22, 2023
@johanhelsing johanhelsing deleted the f64-feature branch March 22, 2023 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jondolf Jondolf Jondolf left review comments

Assignees
No one assigned
Labels
C-Enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@johanhelsing @Jondolf