Support loading mesh by mesh name in <mesh><uri> #2007
Conversation
Signed-off-by: Ian Chen <[email protected]>
Codecov Report
@@ Coverage Diff @@
## gz-sim7 #2007 +/- ##
===========================================
+ Coverage 64.99% 65.03% +0.04%
===========================================
Files 353 354 +1
Lines 28618 28655 +37
===========================================
+ Hits 18600 18636 +36
- Misses 10018 10019 +1
|
Signed-off-by: Ian Chen <[email protected]>
| #include <gz/msgs/entity_factory.pb.h> | ||
|
|
||
| #include <gz/common/Console.hh> | ||
| #include <gz/common/Util.hh> | ||
| #include <gz/math/Pose3.hh> | ||
| #include <gz/transport/Node.hh> | ||
| #include <gz/utils/ExtraTestMacros.hh> |
There was a problem hiding this comment.
I think these are already in alphabetical order? The msgs one is up top because it's a .h file.
not yet. That needs some discussion |
Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Ian Chen <[email protected]>
src/rendering/SceneManager.cc
Outdated
| meshUri = fullPath; | ||
| descriptor.mesh = meshManager->Load(meshUri); | ||
| } | ||
| else if (common::URI(_geom.MeshShape()->Uri()).Scheme() == "name") |
There was a problem hiding this comment.
Should this be checked first? If we have name://foo.dae, but there's a foo.dae somewhere in the file search path, what would be the behavior?
There was a problem hiding this comment.
ok swapped order of check so that the scheme determines how the mesh will be loaded. So ror name://foo.dae it'll assume it's an in-memory mesh. 4f381ce
src/rendering/SceneManager.cc
Outdated
| common::MeshManager::Instance(); | ||
| std::string meshUri; | ||
| rendering::MeshDescriptor descriptor; | ||
| if (meshManager->IsValidFilename(_geom.MeshShape()->Uri())) |
There was a problem hiding this comment.
I see this logic is duplicated in Physics.cc. Can it be refactored?
There was a problem hiding this comment.
refactored into a helper function in Util class. 4f381ce
test/integration/mesh_uri.cc
Outdated
| EXPECT_LT(0u, g_count); | ||
|
|
||
| // Empty scene - camera should see just red background | ||
| unsigned int bufferSize = g_image.width() *g_image.height() * 3u; |
There was a problem hiding this comment.
We need to lock the mutex before accessing g_image.
There was a problem hiding this comment.
added lock. 4f381ce
Signed-off-by: Ian Chen <[email protected]>
include/gz/sim/Util.hh
Outdated
| /// \brief Load a mesh from a Mesh SDF DOM | ||
| /// \param[in] _meshSdf Mesh SDF DOM | ||
| /// \return The loaded mesh or null if the mesh can not be loaded. | ||
| GZ_SIM_VISIBLE const common::Mesh *loadMesh(const sdf::Mesh *_meshSdf); |
There was a problem hiding this comment.
I would prefer we use const refs for parameters instead of a pointer so it's left up to the caller to ensure that the pointer is valid. This also lines up with our style guide (https://google.github.io/styleguide/cppguide.html#Inputs_and_Outputs)
| GZ_SIM_VISIBLE const common::Mesh *loadMesh(const sdf::Mesh *_meshSdf); | |
| GZ_SIM_VISIBLE const common::Mesh *loadMesh(const sdf::Mesh &_meshSdf); |
There was a problem hiding this comment.
yep done, switched to use const ref. 68816bb
Signed-off-by: Ian Chen <[email protected]>
🎉 New feature
Partially addresses gazebosim/gz-common#404, specifically gazebosim/gz-common#404 (comment) about handing over in-memory mesh data to gazebo.
Summary
Allow users to specify name of mesh in SDF via
<mesh><uri>.Addresses this use case: User has created
common:Meshobject and added it to the MeshManager via AddMesh call. They now want to spawn a new model with this mesh using an SDF string that has<mesh><uri>set to the name of theircommon::Mesh.Added integration test demonstrating the above use case. It uses
unit_box(that comes with MeshManager) as an example but user should be able to supply their own mesh.Test it
Checklist
codecheckpassed (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-bymessages.