Make SDF -> USD code a separate component of SDF #816

adlarkin · 2022-01-07T00:13:16Z

🎉 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 in sdf12 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 from sdf12 and then updated the structure of #762 to be compatible with sdf12 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/usdConverter

Checklist

Signed all commits for DCO
Added tests
Added example and/or tutorial
Updated documentation (as needed)
Updated migration guide (as needed)
codecheck passed (See contributing)
All tests passed (See test coverage)
While waiting for a review on your PR, please help review another open pull request to support the maintainers

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.

Signed-off-by: Ian Chen <[email protected]>

* Fix cmake warning about newlines * Find python3 in cmake, fix warning (#328) Signed-off-by: Steve Peters <[email protected]>

* Added ToElement conversion for physics and atmosphere Signed-off-by: Nate Koenig <[email protected]> * spelling Signed-off-by: Nate Koenig <[email protected]> Co-authored-by: Nate Koenig <[email protected]>

* Aded ToElement conversion for shapes Signed-off-by: Nate Koenig <[email protected]> * doxygen Signed-off-by: Ian Chen <[email protected]> Co-authored-by: Nate Koenig <[email protected]> Co-authored-by: Ian Chen <[email protected]>

Merge 6 -> 9

* Working on material DOM ToElement Signed-off-by: Nate Koenig <[email protected]> * Update material ToElement Signed-off-by: Nate Koenig <[email protected]> * Use floats Signed-off-by: Nate Koenig <[email protected]> * Fix build Signed-off-by: Nate Koenig <[email protected]> Co-authored-by: Nate Koenig <[email protected]>

Signed-off-by: Nate Koenig <[email protected]> Co-authored-by: Nate Koenig <[email protected]> Co-authored-by: Louise Poubel <[email protected]>

* Added ToElement conversion for Collision, Surface, and Visual Signed-off-by: Nate Koenig <[email protected]> * Update src/Visual_TEST.cc Co-authored-by: Alejandro Hernández Cordero <[email protected]> Co-authored-by: Nate Koenig <[email protected]> Co-authored-by: Alejandro Hernández Cordero <[email protected]>

Signed-off-by: Jenn Nguyen <[email protected]> Co-authored-by: Marco A. Gutiérrez <[email protected]> Co-authored-by: Addisu Z. Taddese <[email protected]>

* Added ToElement for ParticleEmitter and Link Signed-off-by: Nate Koenig <[email protected]> * expect double eq Signed-off-by: Ian Chen <[email protected]> Co-authored-by: Nate Koenig <[email protected]> Co-authored-by: Ian Chen <[email protected]>

* Adding toelement for model and actor Signed-off-by: Nate Koenig <[email protected]> * Updated tests Signed-off-by: Nate Koenig <[email protected]> Co-authored-by: Nate Koenig <[email protected]>

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 <[email protected]> Signed-off-by: Addisu Z. Taddese <[email protected]> Co-authored-by: Addisu Z. Taddese <[email protected]>

* Added more ToElement functions Signed-off-by: Nate Koenig <[email protected]> * Revert Link toelement changes Signed-off-by: Nate Koenig <[email protected]> * Added toelement conversion for world, scene, sky, gui Signed-off-by: Nate Koenig <[email protected]> * Fix element insertion, and updated tests Signed-off-by: Nate Koenig <[email protected]> * Update docs Signed-off-by: Nate Koenig <[email protected]> * Updates Signed-off-by: Nate Koenig <[email protected]> * minor doxygen update Signed-off-by: Ian Chen <[email protected]> Co-authored-by: Nate Koenig <[email protected]> Co-authored-by: Ian Chen <[email protected]>

Signed-off-by: Nate Koenig <[email protected]> Co-authored-by: Nate Koenig <[email protected]>

* Support URI in the Model DOM Signed-off-by: Nate Koenig <[email protected]> * One minor test Signed-off-by: Nate Koenig <[email protected]> * placement frame and static Signed-off-by: Nate Koenig <[email protected]> * relative_to Signed-off-by: Nate Koenig <[email protected]> * Added nested include test Signed-off-by: Nate Koenig <[email protected]> * Updates Signed-off-by: Nate Koenig <[email protected]> * useincludetag Signed-off-by: Nate Koenig <[email protected]> Co-authored-by: Nate Koenig <[email protected]>

* Added plugin to SDF DOM Signed-off-by: Nate Koenig <[email protected]> * tweaks Signed-off-by: Nate Koenig <[email protected]> * Fixed doxygen Signed-off-by: Nate Koenig <[email protected]> * Updates Signed-off-by: Nate Koenig <[email protected]> * Update plugin copy and tests Signed-off-by: Nate Koenig <[email protected]> * Remove string and add move functions Signed-off-by: Nate Koenig <[email protected]> * Fix build and tests Signed-off-by: Nate Koenig <[email protected]> * Fix windows warnings Signed-off-by: Nate Koenig <[email protected]> Co-authored-by: Nate Koenig <[email protected]>

Signed-off-by: Jenn Nguyen <[email protected]>

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 <[email protected]> Co-authored-by: Steve Peters <[email protected]>

Signed-off-by: Louise Poubel <[email protected]>

…used angles (#689) * Ruby option to print in_degrees or snap_to_degrees Signed-off-by: Aaron Chong <[email protected]> * Basic PrintConfig added Signed-off-by: Aaron Chong <[email protected]> * PrintConfig gets passed into printing implementations of Element and Param Signed-off-by: Aaron Chong <[email protected]> * Adding basic test for print options Signed-off-by: Aaron Chong <[email protected]> * Reverting to PrintConfig with basic API Signed-off-by: Aaron Chong <[email protected]> * Moved creation of PrintConfig into ign functions Signed-off-by: Aaron Chong <[email protected]> * Param value GetPoseAsString and tests Signed-off-by: Aaron Chong <[email protected]> * Moved attribute painting to its own function, fixed test strings Signed-off-by: Aaron Chong <[email protected]> * Added basic tests for pose rotation input as quaternions Signed-off-by: Aaron Chong <[email protected]> * Using different flags for ign sdf -p, allow snapping to different values Signed-off-by: Aaron Chong <[email protected]> * Disabling test on windows, fixing comment Signed-off-by: Aaron Chong <[email protected]> * Remove stale function, fixed linting Signed-off-by: Aaron Chong <[email protected]> * Adding tolerance as a argument, added tests Signed-off-by: Aaron Chong <[email protected]> * Use 3 spaces when changing rotation formats or snapping to degrees Signed-off-by: Aaron Chong <[email protected]> * Added check for tolerance larger than snapping interval Signed-off-by: Aaron Chong <[email protected]> * Moving PrintAttributes to ElementPrivate to remain ABI stability Signed-off-by: Aaron Chong <[email protected]> * Using true/false instead of 1/0 Signed-off-by: Aaron Chong <[email protected]> * Remove use of SDF_ASSERT in GetAsString Signed-off-by: Aaron Chong <[email protected]> * Added tests for //include/pose Signed-off-by: Aaron Chong <[email protected]> * Adding parsing passing test for empty quat_xyzw pose Signed-off-by: Aaron Chong <[email protected]> * Added check for default string values to be modified when rotation_format is defined Signed-off-by: Aaron Chong <[email protected]> * Added tests Signed-off-by: Aaron Chong <[email protected]> * Reparsing translates default value into string to be used if values have not been assigned to param Signed-off-by: Aaron Chong <[email protected]> * Using StringFromValueImpl for getting strings from all ParamVariants Signed-off-by: Aaron Chong <[email protected]> * Refactor pose string from value into its own function Signed-off-by: Aaron Chong <[email protected]> * Fixing casting erroerror, added documentation and tests for tolerance < interval Signed-off-by: Aaron Chong <[email protected]> * Correcting stale comments Signed-off-by: Aaron Chong <[email protected]> * Fixing snapToInterval math, added more tests Signed-off-by: Aaron Chong <[email protected]> * Removed unneeded visibility macro Signed-off-by: Aaron Chong <[email protected]> * Adding return documentation and using const reference to variant instead of pointer Signed-off-by: Aaron Chong <[email protected]> * Returning string directly, removing stale _config, reverting strValue to nullopt Signed-off-by: Aaron Chong <[email protected]> * Remove use of assertions Signed-off-by: Aaron Chong <[email protected]> * Suggested changes to #729 (#748) Signed-off-by: Addisu Z. Taddese <[email protected]> * Using three space delimiter between position and rotation if attributes are set Signed-off-by: Aaron Chong <[email protected]> * Added comment regarding use of default PrintConfig in Reparse Signed-off-by: Aaron Chong <[email protected]> * Adding equality comparison for PrintConfig Signed-off-by: Aaron Chong <[email protected]> * Removed stale include Signed-off-by: Aaron Chong <[email protected]> * Uniied string and value parsing behavior, and modified necessary tests Signed-off-by: Aaron Chong <[email protected]> * Overloaded function to maintain ABI stability Signed-off-by: Aaron Chong <[email protected]> * Fixing missing space in test for exec command Signed-off-by: Aaron Chong <[email protected]> * Adding comment regarding attributeExceptions Signed-off-by: Aaron Chong <[email protected]> * Indenting help message, adding test for shuffling command flags Signed-off-by: Aaron Chong <[email protected]> * Modifying cmd flag shuffling test to handling flags with arguments too Signed-off-by: Aaron Chong <[email protected]> * Removed Get from PrintConfig getter functions Signed-off-by: Aaron Chong <[email protected]> * Using std optional's converting constructor Signed-off-by: Aaron Chong <[email protected]> * Modified snapToInterval implementation, added test Signed-off-by: Aaron Chong <[email protected]> * Added bool type specific value parser, values are parsed using ParamStreamer by default Signed-off-by: Aaron Chong <[email protected]> * Reverting all unnecessary changes made in sdf12 to old tests Signed-off-by: Aaron Chong <[email protected]> * Added comparison for PreserveIncludes Signed-off-by: Aaron Chong <[email protected]> * 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 <[email protected]> * Only checking for pose related PrintConfig options for returning string instead of default PrintConfig Signed-off-by: Aaron Chong <[email protected]> * Added comment regarding sanitizing -0 in test outputs Signed-off-by: Aaron Chong <[email protected]> Co-authored-by: Addisu Z. Taddese <[email protected]>

Signed-off-by: Louise Poubel <[email protected]>

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 <[email protected]>

Signed-off-by: Steve Peters <[email protected]>

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 <[email protected]>

Signed-off-by: Steve Peters <[email protected]>

…to_11

Account for new behavior of nested models and :: syntax. Signed-off-by: Steve Peters <[email protected]>

Merge part of 10 to 11

Signed-off-by: Steve Peters <[email protected]>

Merge sdf10 -> sdf11

Signed-off-by: Steve Peters <[email protected]>

Merge sdf11 to sdf12

Signed-off-by: Louise Poubel <[email protected]> * Add line with unrecognized type Signed-off-by: Steve Peters <[email protected]> * 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 <[email protected]> Co-authored-by: Steve Peters <[email protected]> Co-authored-by: Aaron Chong <[email protected]>

Signed-off-by: Ashton Larkin <[email protected]>

codecov-commenter · 2022-01-07T00:24:19Z

Codecov Report

❗ No coverage uploaded for pull request base (adlarkin/sdf_to_usd@b70ce48). Click here to learn what that means.
The diff coverage is n/a.

@@                  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.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b70ce48...26c1a47. Read the comment docs.

adlarkin

@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.

adlarkin · 2022-01-07T00:15:34Z

CMakeLists.txt

+
+  ########################################
+  # Find PXR
+  ign_find_package(pxr REQUIRED_BY usd PKGCONFIG pxr)


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?

adlarkin · 2022-01-07T00:16:38Z

CMakeLists.txt

+  ########################################
+  # Find ignition common
+  ign_find_package(ignition-common4 COMPONENTS graphics REQUIRED_BY usd)
+  set(IGN_COMMON_VER ${ignition-common4_VERSION_MAJOR})


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?

yes, it's ok

adlarkin · 2022-01-07T00:19:24Z

examples/usdConverter/README.md

+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


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

adlarkin · 2022-01-07T00:33:29Z

examples/usdConverter/usdConverter.cc

-#include "sdf_usd_parser/world.hh"
+#include "sdf/world.hh"


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/...")?

adlarkin · 2022-01-07T00:37:12Z

usd/src/CMakeLists.txt

+# Build the unit tests
+ign_build_tests(TYPE UNIT SOURCES ${gtest_sources} LIB_DEPS ${usd_target})


There are no tests in this PR right now, but the tests being written in #796 will eventually make use of this cmake code (#796 will need to be updated to reflect the cmake component structure being done in this PR).

scpeters

all of the usd header files should be in a usd subfolder, like usd/include/sdf/usd/*

ahcorde · 2022-01-07T11:41:34Z

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 sdf12

adlarkin · 2022-01-07T17:22:22Z

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 sdf12

Thanks for breaking this down into something simpler, @ahcorde! Since we now have #817, should I close this PR?

adlarkin · 2022-01-19T22:41:34Z

closing in favor of #817

iche033 and others added 30 commits November 8, 2021 12:00

add enable_orientation to 1.6 spec (#686)

2349364

Signed-off-by: Ian Chen <[email protected]>

Backport cmake/test fixes to sdf6 (#761)

0c6e7ad

* Fix cmake warning about newlines * Find python3 in cmake, fix warning (#328) Signed-off-by: Steve Peters <[email protected]>

Added ToElement conversion for physics and atmosphere (#771)

3b385c9

* Added ToElement conversion for physics and atmosphere Signed-off-by: Nate Koenig <[email protected]> * spelling Signed-off-by: Nate Koenig <[email protected]> Co-authored-by: Nate Koenig <[email protected]>

Aded ToElement conversion for shapes (#772)

520b74f

* Aded ToElement conversion for shapes Signed-off-by: Nate Koenig <[email protected]> * doxygen Signed-off-by: Ian Chen <[email protected]> Co-authored-by: Nate Koenig <[email protected]> Co-authored-by: Ian Chen <[email protected]>

Merge branch 'sdf6' into sdf9

9ba8ea1

Merge pull request #774 from scpeters/merge_6_9

3266e33

Merge 6 -> 9

Added Geometry ToElement function (#776)

b28967c

Signed-off-by: Nate Koenig <[email protected]> Co-authored-by: Nate Koenig <[email protected]> Co-authored-by: Louise Poubel <[email protected]>

PrintConfig option to preserve includes when converting to string (#749)

93c045c

Signed-off-by: Jenn Nguyen <[email protected]> Co-authored-by: Marco A. Gutiérrez <[email protected]> Co-authored-by: Addisu Z. Taddese <[email protected]>

Model actor toelement functions (#782)

2f21fb2

* Adding toelement for model and actor Signed-off-by: Nate Koenig <[email protected]> * Updated tests Signed-off-by: Nate Koenig <[email protected]> Co-authored-by: Nate Koenig <[email protected]>

Support adding and clearing sensors from a joint (#785)

4a6b5b6

Signed-off-by: Nate Koenig <[email protected]> Co-authored-by: Nate Koenig <[email protected]>

Fix loading nested include with custom attributes (#789)

41ad073

Signed-off-by: Jenn Nguyen <[email protected]>

9 ➡️ 10

e5c29da

Signed-off-by: Louise Poubel <[email protected]>

9 ➡️ 10 (#795)

6b53f95

Signed-off-by: Louise Poubel <[email protected]>

Prepare for 10.7.0~pre1 (#799)

1a2c406

Signed-off-by: Steve Peters <[email protected]>

Prepare for 10.7.0~pre2 (#802)

1741bb0

Signed-off-by: Steve Peters <[email protected]>

Prepare for 10.7.0, final Dome release (#804)

5f326e9

Signed-off-by: Steve Peters <[email protected]>

Merge commit '6b53f954b8b74d0bdba33982d59c25da8b280ee4' into merge_9_…

38a69ab

…to_11

sdf_custom: fix nested model expectations (#807)

bff502f

Account for new behavior of nested models and :: syntax. Signed-off-by: Steve Peters <[email protected]>

Merge pull request #807 from scpeters/merge_9_to_11

5c6e870

Merge part of 10 to 11

scpeters and others added 9 commits December 29, 2021 13:28

Merge branch 'sdf10' into merge_10_11

09c06ea

Fix compiler warnings (#808)

f03ec70

Signed-off-by: Steve Peters <[email protected]>

Merge pull request #808 from scpeters/merge_10_11

206e698

Merge sdf10 -> sdf11

Merge branch 'sdf11' into merge_11_to_12

08eccbf

Fix compiler warnings (#810)

15e6793

Signed-off-by: Steve Peters <[email protected]>

Merge pull request #810 from scpeters/merge_11_to_12

8785b7a

Merge sdf11 to sdf12

Merge branch 'sdf12' into adlarkin/sdf_to_usd_cmake

63cc4c0

make SDF to USD a separate component of sdformat

26c1a47

Signed-off-by: Ashton Larkin <[email protected]>

adlarkin changed the title ~~Adlarkin/sdf to usd cmake~~ Make SDF -> USD code a separate component of SDF Jan 7, 2022

adlarkin requested review from scpeters and ahcorde January 7, 2022 00:39

adlarkin commented Jan 7, 2022

View reviewed changes

scpeters requested changes Jan 7, 2022

View reviewed changes

ahcorde mentioned this pull request Jan 7, 2022

make SDF to USD a separate component of sdformat #817

Merged

8 tasks

adlarkin closed this Jan 19, 2022

adlarkin deleted the adlarkin/sdf_to_usd_cmake branch March 24, 2022 15:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make SDF -> USD code a separate component of SDF #816

Make SDF -> USD code a separate component of SDF #816

adlarkin commented Jan 7, 2022

codecov-commenter commented Jan 7, 2022

adlarkin left a comment •

edited

Loading

adlarkin Jan 7, 2022

adlarkin Jan 7, 2022

scpeters Jan 7, 2022

adlarkin Jan 7, 2022

adlarkin Jan 7, 2022

adlarkin Jan 7, 2022

scpeters left a comment

ahcorde commented Jan 7, 2022

adlarkin commented Jan 7, 2022

adlarkin commented Jan 19, 2022

		# Build the unit tests
		ign_build_tests(TYPE UNIT SOURCES ${gtest_sources} LIB_DEPS ${usd_target})

		#include "sdf_usd_parser/world.hh"
		#include "sdf/world.hh"

Make SDF -> USD code a separate component of SDF #816

Make SDF -> USD code a separate component of SDF #816

Conversation

adlarkin commented Jan 7, 2022

🎉 New feature

Summary

Test it

Checklist

codecov-commenter commented Jan 7, 2022

Codecov Report

adlarkin left a comment • edited Loading

Choose a reason for hiding this comment

adlarkin Jan 7, 2022

Choose a reason for hiding this comment

adlarkin Jan 7, 2022

Choose a reason for hiding this comment

scpeters Jan 7, 2022

Choose a reason for hiding this comment

adlarkin Jan 7, 2022

Choose a reason for hiding this comment

adlarkin Jan 7, 2022

Choose a reason for hiding this comment

adlarkin Jan 7, 2022

Choose a reason for hiding this comment

scpeters left a comment

Choose a reason for hiding this comment

ahcorde commented Jan 7, 2022

adlarkin commented Jan 7, 2022

adlarkin commented Jan 19, 2022

adlarkin left a comment •

edited

Loading