gazebosim / gz-physics

Abstract physics interface designed to support simulation and rapid development of robot applications.
https://gazebosim.org
Apache License 2.0
63 stars 38 forks source link

bullet-featherstone: Fix loading mesh from relative path #599

Closed iche033 closed 4 months ago

iche033 commented 4 months ago

🦟 Bug fix

Summary

When loading a model sdf that contains relative path to meshes, bullet featherstone implementation will fail to load the mesh and print out errors like these:

[Err] [SystemPaths.cc:525] Could not resolve file [box.obj]
[Err] [MeshManager.cc:193] Unable to find file[box.obj]

bullet featherstone implementation works differently from other physics engines. It does not rely on gz-sim to load the mesh and attach it using the AttachMeshFeature. Instead, it uses its own mesh manager for loading meshes. So we'll also need to resolve the mesh URI here in gz-physics. This is done using the asFullPath function taken from gz-sim. We should consider refactoring this function, e.g. to gz-common, in the main branches so it can be reused in gz-physics.

To test

You can download these world sdf and box mesh model files for testing:

mkdir -p box_mesh_test/box
wget https://gist.githubusercontent.com/iche033/9943a9bcf6ed9eede40ee3e9dbb4f8ad/raw/2f837c5221c3d56ee13f6191de4c0a449c36b0db/box_mesh_test.sdf
wget https://gist.githubusercontent.com/iche033/d13a8d8e0c05308f98ce1e5e105500ce/raw/681a2be4a2d53d36c9f0e415640258674ef977e5/model.sdf -O box_mesh_test/box/model.sdf
wget https://gist.githubusercontent.com/iche033/8c57b8882085dbce615419d5d472530c/raw/754f620ab034d218c0cda02453c4664968fe5935/box.obj -O box_mesh_test/box/box.obj
wget https://gist.githubusercontent.com/iche033/e59504e1b67f37f2ad30588d16fadb1b/raw/9a4dd30007345f017ca44f7da671eca6cddb9843/model.config -O box_mesh_test/box/model.config

and run the test world with bullet-featherstone physics engine in gz-sim:

gz sim -v 4 box_mesh_test.sdf --physics-engine gz-physics-bullet-featherstone-plugin

Hit play and the box mesh should fall, collide (and then explode) instead of falling through the ground :)

Checklist

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.

azeey commented 4 months ago

sdf::ParserConfig has an option to store resolved URIs. @mjcarroll I remember this was implemented in https://github.com/gazebosim/sdformat/pull/1147 to support Bullet-featherstone, but I don't see it actually being used in gz-sim.

codecov[bot] commented 4 months ago

Codecov Report

Attention: 19 lines in your changes are missing coverage. Please review.

Comparison is base (ea90ada) 78.52% compared to head (d1cdbc7) 78.34%.

Files Patch % Lines
bullet-featherstone/src/SDFFeatures.cc 0.00% 19 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## gz-physics6 #599 +/- ## =============================================== - Coverage 78.52% 78.34% -0.19% =============================================== Files 140 140 Lines 7670 7688 +18 =============================================== Hits 6023 6023 - Misses 1647 1665 +18 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mjcarroll commented 4 months ago

but I don't see it actually being used in gz-sim

That shouldn't be the case, I looked a bit, but can't find any substantial reason it was excluded in the final merge?

iche033 commented 4 months ago

Just checking how sdf::ParserConfig can be used here to resolve URIs relative to model.sdf. I see sdf::ParserConfig can resolve URIs based on user provided callback. In this case we want to resolve the mesh URI path relative to meshURI->FilePath(). Does the user callback help here?

mjcarroll commented 4 months ago

IIRC, your callback should be fired when the mesh URI is parsed. You can then choose to resolve the URL however you want and then that's what sdformat will return to anyone parsing the sdf file. Previously, the mesh URI would just be passed directly from the SDF file to the downstream user, this gives an opportunity to resolve that URI in a different way before the consumer sees it.

iche033 commented 4 months ago

ok so we want to do this in gz-sim to resolve all URIs before passing to gz-phsyics. The tricky part is that the user callback takes a single URI string argument so we'll then need to figure out how to pass different meshURI->FilePath() to the callback each time it tries to resolve an URI.

iche033 commented 4 months ago

proposed an alternative approach in https://github.com/gazebosim/sdformat/pull/1373

iche033 commented 4 months ago

closing this in favor of the new approach in https://github.com/gazebosim/sdformat/pull/1373