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 capsule test for local AABB computation #423

Conversation

tehbelinda
Copy link
Contributor

@tehbelinda tehbelinda commented Nov 25, 2019

This change is Reviewable

Copy link
Contributor

@SeanCurtis-TRI SeanCurtis-TRI left a comment

Choose a reason for hiding this comment

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

:LGTM: with one nit.

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


test/geometry/shape/test_capsule.cpp, line 127 at r1 (raw file):

    Capsuled capsule_d(rd, ld);
    testLocalAABBComputation(capsule_d, 1e-15);
    double rf = static_cast<float>(pair.first);

nit rf and lf should be floats.

As an alternative, if you put each test in its own block, you could re-use the variable names, merely changing the type.

@tehbelinda
Copy link
Contributor Author


test/geometry/shape/test_capsule.cpp, line 127 at r1 (raw file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

nit rf and lf should be floats.

As an alternative, if you put each test in its own block, you could re-use the variable names, merely changing the type.

I was trying to keep it consistent with the rest of the file, though I see those are all doubles so I'll change them to floats too

@tehbelinda tehbelinda force-pushed the capsule-local-aabb-test branch from 2b02130 to 8312a60 Compare November 25, 2019 19:37
Copy link
Contributor

@SeanCurtis-TRI SeanCurtis-TRI left a comment

Choose a reason for hiding this comment

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

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


test/geometry/shape/test_capsule.cpp, line 127 at r1 (raw file):

Previously, tehbelinda (Bel) wrote…

I was trying to keep it consistent with the rest of the file, though I see those are all doubles so I'll change them to floats too

Good call.

@tehbelinda
Copy link
Contributor Author

looks like the same flakey test as the last PR :S

Copy link
Contributor

@SeanCurtis-TRI SeanCurtis-TRI left a comment

Choose a reason for hiding this comment

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

I'm going to go ahead and squash this.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@SeanCurtis-TRI SeanCurtis-TRI merged commit 6aeb7a1 into flexible-collision-library:master Nov 26, 2019
@tehbelinda
Copy link
Contributor Author

Ok thanks

@tehbelinda tehbelinda deleted the capsule-local-aabb-test branch November 26, 2019 21:35
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