-
Notifications
You must be signed in to change notification settings - Fork 98
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: Added lights attached to the world #875
Conversation
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: ahcorde <[email protected]>
ed473a5
to
4b09785
Compare
Signed-off-by: Ashton Larkin <[email protected]>
Signed-off-by: Ashton Larkin <[email protected]>
Signed-off-by: ahcorde <[email protected]>
… ahcorde/usd_to_sdf_transforms
…nto ahcorde/usd_to_sdf_lights
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: Ashton Larkin <[email protected]>
Signed-off-by: Ashton Larkin <[email protected]>
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: ahcorde <[email protected]>
… ahcorde/usd_to_sdf_transforms
Signed-off-by: ahcorde <[email protected]>
…nto ahcorde/usd_to_sdf_lights
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.
There is some data that is not common between these two formats, I would say that some of the data might be "lost in translation"
I assume that these differences are due to the different formats and can't be addressed easily?
Signed-off-by: ahcorde <[email protected]>
…nitionrobotics/sdformat into ahcorde/usd_to_sdf_transforms
Signed-off-by: ahcorde <[email protected]>
…nitionrobotics/sdformat into ahcorde/usd_to_sdf_transforms
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: ahcorde <[email protected]>
…nto ahcorde/usd_to_sdf_lights
Signed-off-by: ahcorde <[email protected]>
1878882
to
7299701
Compare
Signed-off-by: Ashton Larkin <[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.
I have a few questions before merging. Otherwise, I think this looks good.
usd/src/usd_parser/USDWorld.cc
Outdated
// In general USD models used in Issac Sim define the model path | ||
// under a root path for example: | ||
// -> /robot_name/robot_name_link0 | ||
// But sometimes for enviroments it uses just a simple path: | ||
// -> /ground_plane | ||
// -> /wall_0 | ||
// the shortName variable defines if this is the first case when it's | ||
// False or when it's true then it's the second case. | ||
// This conversion might only work with Issac Sim USDs | ||
if (primPathTokens.size() >= 2) | ||
{ | ||
bool shortName = false; | ||
if (primPathTokens.size() == 2) | ||
{ | ||
if (prim.IsA<pxr::UsdGeomGprim>() || (primType == "Plane")) | ||
{ | ||
if (primName != "imu") | ||
{ | ||
linkName = "/" + primPathTokens[0]; | ||
shortName = true; | ||
} | ||
} | ||
} | ||
if (!shortName) | ||
{ | ||
linkName = "/" + primPathTokens[0] + "/" + primPathTokens[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.
Is this code mainly used for finding the parent prim of the light? I find it a little odd that we are manually parsing through the first 2 levels of the prim path to find the parent. I imagine this could be problematic if the light's parent isn't in the first two levels of the stage. Is there a better way to find the parent of a light that offers more flexibility/is less error prone?
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.
I didn't find a better way to do it, I'm open to suggestions
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.
I can't think of a better way at the moment - since we are pressed for time, I added a TODO note in 847271a and created an issue in #927.
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: Ashton Larkin <[email protected]>
Signed-off-by: ahcorde [email protected]
🎉 New feature
Summary
USD -> SDF: Added lights attached to the world
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.