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

[trivial fix] Fix build when ccd_real_t == float #498

Conversation

v4hn
Copy link
Contributor

@v4hn v4hn commented Oct 7, 2020

libccd supports single or double precision builds.
It seems like the developers build it with double precision.
But if you actually require double precision you should check for it in the configuration.

Without the patch the build fails because stl does not support std::max(double, float) (problems with the initializer_list overload being preferred).
A fact that by itself is rather frustrating as well...


This change is Reviewable

Copy link
Member

@sherm1 sherm1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @v4hn! :lgtm: pending minor cosmetic fix (line length)

Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @v4hn)


include/fcl/narrowphase/detail/convexity_based_algorithm/gjk_libccd-inl.h, line 1696 at r1 (raw file):

  const ccd_real_t v0_dist =
      std::sqrt(ccdVec3Len2(&nearest_edge->vertex[0]->v.v));
  const ccd_real_t plane_threshold = kEps * std::max(static_cast<ccd_real_t>(1.0), v0_dist);

nit: please keep line length to <= 80

libccd supports single or double precision builds.
It seems like the developers build it with double precision.
But if you actually require double precision you should check for it in the configuration.
@v4hn v4hn force-pushed the pr-master-support-libccd-single-precision branch from 53fcc22 to 35769a2 Compare October 7, 2020 15:17
@v4hn
Copy link
Contributor Author

v4hn commented Oct 7, 2020

I just pushed an additional line break.

Sorry, I'm not sure whether I'm expected to do anything at reviewable because the page keeps loading forever saying "Resuming session". ❓

Copy link
Member

@sherm1 sherm1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Reviewable if the reviewer's comment begins with "nit" or "minor" you can resolve it yourself by answering with a comment beginning with "done" (meaning: you fixed it) or "ok" (meaning: I thought about it but am going to leave it this way for [reason]).

But it's all good now -- thanks!

Reviewed 1 of 1 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@sherm1 sherm1 merged commit 887c759 into flexible-collision-library:master Oct 7, 2020
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