-
Notifications
You must be signed in to change notification settings - Fork 107
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
Make SDF -> USD code a separate component of SDF #816
Conversation
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
* Fix cmake warning about newlines * Find python3 in cmake, fix warning (#328) Signed-off-by: Steve Peters <scpeters@openrobotics.org>
* Added ToElement conversion for physics and atmosphere Signed-off-by: Nate Koenig <nate@openrobotics.org> * spelling Signed-off-by: Nate Koenig <nate@openrobotics.org> Co-authored-by: Nate Koenig <nate@openrobotics.org>
* Aded ToElement conversion for shapes Signed-off-by: Nate Koenig <nate@openrobotics.org> * doxygen Signed-off-by: Ian Chen <ichen@osrfoundation.org> Co-authored-by: Nate Koenig <nate@openrobotics.org> Co-authored-by: Ian Chen <ichen@osrfoundation.org>
Merge 6 -> 9
* Working on material DOM ToElement Signed-off-by: Nate Koenig <nate@openrobotics.org> * Update material ToElement Signed-off-by: Nate Koenig <nate@openrobotics.org> * Use floats Signed-off-by: Nate Koenig <nate@openrobotics.org> * Fix build Signed-off-by: Nate Koenig <nate@openrobotics.org> Co-authored-by: Nate Koenig <nate@openrobotics.org>
Signed-off-by: Nate Koenig <nate@openrobotics.org> Co-authored-by: Nate Koenig <nate@openrobotics.org> Co-authored-by: Louise Poubel <louise@openrobotics.org>
* Added ToElement conversion for Collision, Surface, and Visual Signed-off-by: Nate Koenig <nate@openrobotics.org> * Update src/Visual_TEST.cc Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com> Co-authored-by: Nate Koenig <nate@openrobotics.org> Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
Signed-off-by: Jenn Nguyen <jenn@openrobotics.org> Co-authored-by: Marco A. Gutiérrez <marco@openrobotics.org> Co-authored-by: Addisu Z. Taddese <addisu@openrobotics.org>
* Added ToElement for ParticleEmitter and Link Signed-off-by: Nate Koenig <nate@openrobotics.org> * expect double eq Signed-off-by: Ian Chen <ichen@osrfoundation.org> Co-authored-by: Nate Koenig <nate@openrobotics.org> Co-authored-by: Ian Chen <ichen@osrfoundation.org>
* Adding toelement for model and actor Signed-off-by: Nate Koenig <nate@openrobotics.org> * Updated tests Signed-off-by: Nate Koenig <nate@openrobotics.org> Co-authored-by: Nate Koenig <nate@openrobotics.org>
Adds a sentence to the behavior of //model/static to avoid confusion on what might happen if it is used within a model that is itself included in another model. Also adds some clarification to //model/@canonical_link. Signed-off-by: FirefoxMetzger <sebastian@wallkoetter.net> Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org> Co-authored-by: Addisu Z. Taddese <addisu@openrobotics.org>
* Added more ToElement functions Signed-off-by: Nate Koenig <nate@openrobotics.org> * Revert Link toelement changes Signed-off-by: Nate Koenig <nate@openrobotics.org> * Added toelement conversion for world, scene, sky, gui Signed-off-by: Nate Koenig <nate@openrobotics.org> * Fix element insertion, and updated tests Signed-off-by: Nate Koenig <nate@openrobotics.org> * Update docs Signed-off-by: Nate Koenig <nate@openrobotics.org> * Updates Signed-off-by: Nate Koenig <nate@openrobotics.org> * minor doxygen update Signed-off-by: Ian Chen <ichen@osrfoundation.org> Co-authored-by: Nate Koenig <nate@openrobotics.org> Co-authored-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Nate Koenig <nate@openrobotics.org> Co-authored-by: Nate Koenig <nate@openrobotics.org>
* Support URI in the Model DOM Signed-off-by: Nate Koenig <nate@openrobotics.org> * One minor test Signed-off-by: Nate Koenig <nate@openrobotics.org> * placement frame and static Signed-off-by: Nate Koenig <nate@openrobotics.org> * relative_to Signed-off-by: Nate Koenig <nate@openrobotics.org> * Added nested include test Signed-off-by: Nate Koenig <nate@openrobotics.org> * Updates Signed-off-by: Nate Koenig <nate@openrobotics.org> * useincludetag Signed-off-by: Nate Koenig <nate@openrobotics.org> Co-authored-by: Nate Koenig <nate@openrobotics.org>
* Added plugin to SDF DOM Signed-off-by: Nate Koenig <nate@openrobotics.org> * tweaks Signed-off-by: Nate Koenig <nate@openrobotics.org> * Fixed doxygen Signed-off-by: Nate Koenig <nate@openrobotics.org> * Updates Signed-off-by: Nate Koenig <nate@openrobotics.org> * Update plugin copy and tests Signed-off-by: Nate Koenig <nate@openrobotics.org> * Remove string and add move functions Signed-off-by: Nate Koenig <nate@openrobotics.org> * Fix build and tests Signed-off-by: Nate Koenig <nate@openrobotics.org> * Fix windows warnings Signed-off-by: Nate Koenig <nate@openrobotics.org> Co-authored-by: Nate Koenig <nate@openrobotics.org>
Signed-off-by: Jenn Nguyen <jenn@openrobotics.org>
The buildFrameAttachedToGraph and buildPoseRelativeToGraph have overloads for the type of parent element, but the code in each overload is very similar to each other. This refactors these functions so there's less code duplication. Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org> Co-authored-by: Steve Peters <scpeters@openrobotics.org>
…used angles (#689) * Ruby option to print in_degrees or snap_to_degrees Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Basic PrintConfig added Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * PrintConfig gets passed into printing implementations of Element and Param Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Adding basic test for print options Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Reverting to PrintConfig with basic API Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Moved creation of PrintConfig into ign functions Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Param value GetPoseAsString and tests Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Moved attribute painting to its own function, fixed test strings Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Added basic tests for pose rotation input as quaternions Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Using different flags for ign sdf -p, allow snapping to different values Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Disabling test on windows, fixing comment Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Remove stale function, fixed linting Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Adding tolerance as a argument, added tests Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Use 3 spaces when changing rotation formats or snapping to degrees Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Added check for tolerance larger than snapping interval Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Moving PrintAttributes to ElementPrivate to remain ABI stability Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Using true/false instead of 1/0 Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Remove use of SDF_ASSERT in GetAsString Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Added tests for //include/pose Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Adding parsing passing test for empty quat_xyzw pose Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Added check for default string values to be modified when rotation_format is defined Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Added tests Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Reparsing translates default value into string to be used if values have not been assigned to param Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Using StringFromValueImpl for getting strings from all ParamVariants Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Refactor pose string from value into its own function Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Fixing casting erroerror, added documentation and tests for tolerance < interval Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Correcting stale comments Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Fixing snapToInterval math, added more tests Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Removed unneeded visibility macro Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Adding return documentation and using const reference to variant instead of pointer Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Returning string directly, removing stale _config, reverting strValue to nullopt Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Remove use of assertions Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Suggested changes to #729 (#748) Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org> * Using three space delimiter between position and rotation if attributes are set Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Added comment regarding use of default PrintConfig in Reparse Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Adding equality comparison for PrintConfig Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Removed stale include Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Uniied string and value parsing behavior, and modified necessary tests Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Overloaded function to maintain ABI stability Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Fixing missing space in test for exec command Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Adding comment regarding attributeExceptions Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Indenting help message, adding test for shuffling command flags Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Modifying cmd flag shuffling test to handling flags with arguments too Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Removed Get from PrintConfig getter functions Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Using std optional's converting constructor Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Modified snapToInterval implementation, added test Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Added bool type specific value parser, values are parsed using ParamStreamer by default Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Reverting all unnecessary changes made in sdf12 to old tests Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Added comparison for PreserveIncludes Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Check for 'type' attribute in unknown elements as well, in order to parse booleans into true/false, instead of 1/0 Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Only checking for pose related PrintConfig options for returning string instead of default PrintConfig Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Added comment regarding sanitizing -0 in test outputs Signed-off-by: Aaron Chong <aaronchongth@gmail.com> Co-authored-by: Addisu Z. Taddese <addisu@openrobotics.org>
This replaces most of the custom cmake code in libsdformat with the functionality provided by ignition-cmake2. The root CMakeLists.txt is much shorter now and most of the cmake folder has been deleted. This is made possible by the NO_IGNITION_PREFIX and REPLACE_IGNITION_INCLUDE_PATH parameters added to ign_configure_project in ign-cmake#190 and ign-cmake#191. Closes #181. Other details: * Use FindIgnURDFDOM from ign-cmake#193 * Use HIDE_SYMBOLS_BY_DEFAULT from ign-cmake#196 * Set LEGACY_PROJECT_PREFIX from ign-cmake#199 Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
The USE_INTERNAL_URDF logic for include and windows compiler definitions needs to be repeated for tests that add parser_urdf.cc via target_sources. An interface library is used to deduplicate the cmake logic. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Account for new behavior of nested models and :: syntax. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Merge part of 10 to 11
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Merge sdf10 -> sdf11
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Merge sdf11 to sdf12
Signed-off-by: Louise Poubel <louise@openrobotics.org> * Add line with unrecognized type Signed-off-by: Steve Peters <scpeters@openrobotics.org> * Fix for custom element with attribute 'type' (#811) * Reverted changes to parser for custom element with attribute 'type', added fix in ParamPrivate::ValueFromStringImpl instead Signed-off-by: Aaron Chong <aaronchongth@gmail.com> Co-authored-by: Steve Peters <scpeters@openrobotics.org> Co-authored-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Ashton Larkin <42042756+adlarkin@users.noreply.github.com>
Codecov Report
@@ Coverage Diff @@
## adlarkin/sdf_to_usd #816 +/- ##
======================================================
Coverage ? 90.70%
======================================================
Files ? 78
Lines ? 12432
Branches ? 0
======================================================
Hits ? 11276
Misses ? 1156
Partials ? 0 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.
@scpeters and @ahcorde: I have a few questions about what I've done so far, would you mind responding to my comments/questions below? Also, for your initial review, I'd recommend only looking at the changes from commit 26c1a47, since all of the previous commits were a merge from sdf12
so that I could build on top of the cmake changes made in #780.
|
||
######################################## | ||
# Find PXR | ||
ign_find_package(pxr REQUIRED_BY usd PKGCONFIG pxr) |
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'm not sure if the PKGCONFIG pxr
portion should be here. If I didn't include this, I would see the following warning: https://github.com/ignitionrobotics/ign-cmake/blob/a1eb36a9a6283bb1fe5b4c755a2b3669572e0c08/cmake/IgnUtils.cmake#L378-L391
Can someone confirm if adding PKGCONFIG pxr
is correct or not?
######################################## | ||
# Find ignition common | ||
ign_find_package(ignition-common4 COMPONENTS graphics REQUIRED_BY usd) | ||
set(IGN_COMMON_VER ${ignition-common4_VERSION_MAJOR}) |
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.
We need to have a dependency on ign-common
in order to parse meshes to/from SDF and USD. I realize that this is a new dependency that we probably don't want to have, but since it's only needed by the usd
component, is it okay to have this new dependency?
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.
yes, it's ok
You should now have an executable named `usdConverter`, which can be used to convert a SDF world file to a USD file. | ||
The following command converts the example `shapes.sdf` file to its USD representation, stored in a file called `shapes.usda` (run this command from the `build` directory): | ||
```bash | ||
./usdConverter ../shapes.sdf shapes.usda |
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.
right now, this is not working - whenever I try to run the executable generated by the example, I get the following error:
./usdConverter: error while loading shared libraries: libusd_usdPhysics.so: cannot open shared object file: No such file or directory
This was not happening with this executable before turning all of this code into a separate component, so perhaps the component is not being created correctly at the moment. Does anyone know what's happening here? cc @scpeters
#include "sdf_usd_parser/world.hh" | ||
#include "sdf/world.hh" |
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 tried following the file structure for components in ign-common
:
<component name>
|__include
| |__<project name>
| |__component header files
|__src
|__component source files
This means that for the USD component, header files are in the path usd/include/sdformat
, and when it's time to #include
them in the component's source files, they have the same prefix as sdformat's header files (sdf/
). This doesn't result in naming collisions right now because the component header files are not upper case, but a lot of the USD component header files are essentially just a lower case name of the sdformat header files, which can make this code confusing to read (example: world.hh
vs World.hh
). Should I change the component header file structure somehow so that the #include
path is unique (for example, something like #include "sdf/usd/..."
)?
# Build the unit tests | ||
ign_build_tests(TYPE UNIT SOURCES ${gtest_sources} LIB_DEPS ${usd_target}) |
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 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.
all of the usd
header files should be in a usd
subfolder, like usd/include/sdf/usd/*
I created this small PR #817, we will have a lot of conflics and the review will be a mess with the current prototype. With this small PR I hope we can start to merge things into |
closing in favor of #817 |
🎉 New feature
Summary
This updates the work being done in #762 to make the SDF -> USD conversion code into a separate cmake component. This is now possible thanks to #780.
The commit history in this PR appears messy because at the moment, the
adlarkin/sdf_to_usd
branch is not up to date withsdf12
- it's not up to date because merging insdf12
would change the cmake code in #762, but the current code structure in #762 isn't compatible with the recent cmake updates from #780. So, this PR merged in the recent changes fromsdf12
and then updated the structure of #762 to be compatible withsdf12
via a cmake component. To make it easier to view the actual changes being made in this PR, view the history/files changed starting at commit 26c1a47.Test it
The old
usdConverter
executable that converts code from SDF to USD has been moved to a standalone example. To test if this component works, you can try to build this example and then test the resulting executable: https://github.com/ignitionrobotics/sdformat/tree/adlarkin/sdf_to_usd_cmake/examples/usdConverterChecklist
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.