Skip to content
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

Enable StoreResolvedURIs when loading SDF #2323

Closed
wants to merge 2 commits into from
Closed

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Feb 23, 2024

🎉 New feature

Depends on gazebosim/sdformat#1373

Summary

When ParserConfig's SetStoreResolvedURIs option is set to true, the URIs will be resolved / expanded and stored in the SDF parsed. Any consumer of the SDF will not have to resolve the URIs themselves.

One use case for this is that the bullet-featherstone implementation in gz-physics needs to load meshes from the URI element in SDF. Without resolved URIs, the bullet-featherstone engine will need to implement its own logic to resolve all mesh URIs which may result in duplicate code, e.g. gazebosim/gz-physics#599.

For context, this ParserConfig option was added in gazebosim/sdformat#1147 to help with bullet-featherstone work but for some reason it was not set at the time (gazebosim/gz-physics#599 (comment)).

While testing, I ran into some unused code for mesh URIs in rendering/SceneManager.cc so removed that as well.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • 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]>
@iche033 iche033 requested a review from mjcarroll as a code owner February 23, 2024 21:35
@github-actions github-actions bot added the 🌱 garden Ignition Garden label Feb 23, 2024
@iche033 iche033 added the needs upstream release Blocked by a release of an upstream library label Feb 23, 2024
@@ -119,7 +119,9 @@ Server::Server(const ServerConfig &_config)
// resources are downloaded. Blocking here causes the GUI to block with
// a black screen (search for "Async resource download" in
// 'src/gui_main.cc'.
errors = sdfRoot.Load(filePath);
auto &parserConfig = sdf::ParserConfig::GlobalConfig();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is modifying the global config. The comment on the commit says otherwise 🙃

Copy link
Contributor Author

@iche033 iche033 Mar 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah the comment is meant to say "do no overwrite the findFile callbacks in global config". The original implementation passed an empty ParserConfig so it overwrote the callbacks set earlier in the function

@iche033
Copy link
Contributor Author

iche033 commented Mar 29, 2024

closing this in favor of #2349 that targets harmonic

@iche033 iche033 closed this Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden needs upstream release Blocked by a release of an upstream library
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants