-
Notifications
You must be signed in to change notification settings - Fork 41
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
[Citadel] [TPE] Add function to get an Entity's bounding box #59
Conversation
Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Ian Chen <[email protected]>
Codecov Report
@@ Coverage Diff @@
## ign-physics2 #59 +/- ##
================================================
+ Coverage 81.53% 81.81% +0.27%
================================================
Files 103 104 +1
Lines 3434 3492 +58
================================================
+ Hits 2800 2857 +57
- Misses 634 635 +1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM in general with one minor comment. Also it looks like it's having the same link error LNK2001
and LNK1120
on windows again.
namespace tpelib { | ||
|
||
////////////////////////////////////////////////// | ||
math::AxisAlignedBox transformAxisAlignedBox( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add more documentation on the math behind this function? I find it a bit hard to understand by just reading the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added more doc and testing linker fix in 558cbb7
Signed-off-by: Ian Chen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding documentation, LGTM! The CI error seems to be about dartsim and unrelated to this PR.
Signed-off-by: Ian Chen <[email protected]>
all checks passed, merging! actually.. need to merge with with |
Add helper function that returns an entity's bounding box by recursively merging it's children's bounding boxes.