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

Fix build with bullet double precision library #192

Closed
wants to merge 1 commit into from
Closed

Fix build with bullet double precision library #192

wants to merge 1 commit into from

Conversation

rapsys
Copy link

@rapsys rapsys commented Dec 9, 2022

This pull request should fix issue #191

@logzero
Copy link
Member

logzero commented Jan 23, 2023

I think it should be enough if you just cast the constants btScalar(0), btScalar(1) etc, would be less noisy that way.

@logzero
Copy link
Member

logzero commented Jan 23, 2023

Although I'd be also fine with explicit instantiation Min<btScalar>(...) etc.

@rapsys
Copy link
Author

rapsys commented Jan 23, 2023

Although I'd be also fine with explicit instantiation Min<btScalar>(...) etc.

Sorry, I am not a c++ expert, I don't see what you mean.

An example of code would be welcome.

@logzero
Copy link
Member

logzero commented Jan 24, 2023

Min/Max/Clamp are so called function templates expecting the same argument type. The compiler will complain if they get mixed type arguments (like float and double for example), thus the casting business.

Alternatively you can explicitly tell the compiler which type of function should be used (instantiated) by writing Min<float> or Min<double> etc and the compiler will cast the arguments accordingly if possible.

So instead of

Min(btScalar(a), btScalar(b));

you can write

Min<btScalar>(a, b);

@logzero
Copy link
Member

logzero commented Jan 28, 2023

I've pushed a slightly cleaner variant of the fix.

Thanks for looking into it.

@logzero logzero closed this Jan 28, 2023
@rapsys
Copy link
Author

rapsys commented Jan 29, 2023

Thanks for the upstream integration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants