-
Notifications
You must be signed in to change notification settings - Fork 91
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
usd -> sdf: Read transforms #871
Conversation
Codecov Report
@@ Coverage Diff @@
## sdf12 #871 +/- ##
==========================================
- Coverage 87.95% 87.72% -0.24%
==========================================
Files 101 102 +1
Lines 14756 14906 +150
==========================================
+ Hits 12979 13076 +97
- Misses 1777 1830 +53
Continue to review full report at Codecov.
|
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
8955a74
to
3a9b349
Compare
Signed-off-by: ahcorde <ahcorde@gmail.com>
d09ce09
to
576f9bd
Compare
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: Ashton Larkin <42042756+adlarkin@users.noreply.github.com>
Signed-off-by: Ashton Larkin <42042756+adlarkin@users.noreply.github.com>
… ahcorde/usd_to_sdf_transforms
c0eaf1d
to
866ba4f
Compare
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: Ashton Larkin <42042756+adlarkin@users.noreply.github.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
… ahcorde/usd_to_sdf_transforms
@osrf-jenkins retest this please |
@koonpeng would you mind taking a look at #871 (comment) and giving some feedback? |
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: Ashton Larkin <42042756+adlarkin@users.noreply.github.com>
Signed-off-by: Ashton Larkin <42042756+adlarkin@users.noreply.github.com>
Signed-off-by: Ashton Larkin <42042756+adlarkin@users.noreply.github.com>
Signed-off-by: Ashton Larkin <42042756+adlarkin@users.noreply.github.com>
37c78f1
to
da475d4
Compare
Signed-off-by: ahcorde <ahcorde@gmail.com>
As I mentioned in this comment #924 (review) The suggestions to this PR #924 are breaking the rotations The gripper has a wrong rotation |
Signed-off-by: Ashton Larkin <42042756+adlarkin@users.noreply.github.com> Co-authored-by: ahcorde <ahcorde@gmail.com>
usd/src/usd_parser/USDTransforms.cc
Outdated
ignition::math::Quaterniond qX, qY, qZ; | ||
ignition::math::Angle angleX(IGN_DTOR(rotationEuler[0])); | ||
ignition::math::Angle angleY(IGN_DTOR(rotationEuler[1])); | ||
ignition::math::Angle angleZ(IGN_DTOR(rotationEuler[2])); | ||
qX = ignition::math::Quaterniond(angleX.Normalized().Radian(), 0, 0); | ||
qY = ignition::math::Quaterniond(0, angleY.Normalized().Radian(), 0); | ||
qZ = ignition::math::Quaterniond(0, 0, angleZ.Normalized().Radian()); | ||
|
||
if (op == kXFormOpRotateZYX) | ||
{ | ||
std::swap(angleX, angleZ); | ||
} | ||
t.SetRotation((qX * qY) * qZ); |
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.
Since ign-math already does Euler angles, my recommendation would be to use that instead of creating 3 quaternions here. I think this can be simplified as:
// SDFormat/ign-math uses extrinsic XYZ rotations, so it's the same as `rotateXYZ`
ignition::math::Quaterniond rot(IGN_DTOR(rotationEuler[0]), IGN_DTOR(rotationEuler[1]), IGN_DTOR(rotationEuler[2]))
if (op == kxFormOpRotateZYX){
rot.Invert();
}
t.SetRotation(rot);
See https://graphics.pixar.com/usd/release/api/class_usd_geom_xformable.html#details for USD rotation order
See http://sdformat.org/tutorials?tut=specify_pose&cat=specification& for SDFormat rotation order.
usd/src/usd_parser/USDTransforms.cc
Outdated
qZ = ignition::math::Quaterniond(0, 0, angleZ.Normalized().Radian()); | ||
|
||
if (op == kXFormOpRotateZYX) | ||
{ |
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.
The angles are swapped here, but the angles are not used after this line, so it doesn't affect the end result. See my recommendation above.
EXPECT_TRUE(ignition::math::equal( | ||
_rotation.value().Roll(), | ||
usdTransforms.Rotation().value().Roll(), 0.1)); | ||
EXPECT_TRUE(ignition::math::equal( | ||
_rotation.value().Pitch(), | ||
usdTransforms.Rotation().value().Pitch(), 0.1)); | ||
EXPECT_TRUE(ignition::math::equal( | ||
_rotation.value().Yaw(), | ||
usdTransforms.Rotation().value().Yaw(), 0.1)); |
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.
This is fine. I would probably lower the tolerance a bit. If you want, you could just do
EXPECT_TRUE(_rotation->Equal(*usdTransforms.Rotation(), tol));
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde ahcorde@gmail.com
🎉 New feature
Summary
These functions allow to read the transform of a prim. It allows to read the tranforms from a prim to its parents and it should stop when the name given to the functions is equal to the name of the parent.
Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-by
messages.