Skip to content

Use collision-tag meshes/info for collision geoms (primitives, STL/DAE/OBJ) - #264

Open
saikishor wants to merge 5 commits into
mainfrom
use/collision_tags/select_collision_meshes
Open

Use collision-tag meshes/info for collision geoms (primitives, STL/DAE/OBJ)#264
saikishor wants to merge 5 commits into
mainfrom
use/collision_tags/select_collision_meshes

Conversation

@saikishor

Copy link
Copy Markdown
Member

Description

First of a series of PRs splitting up #209 into smaller, reviewable pieces (see that PR for the original context, motivation, and before/after screenshots). Fixes some of the concerns listed in #120.

This PR makes the URDF→MJCF converter treat <visual> and <collision> tags independently instead of always deriving collision geometry from whatever mesh was used for rendering:

  • Mesh filepaths are now read from both <visual> and <collision> tags (previously only one source was used), and converted to .obj as needed.
  • If a link defines a <collision> tag, its mesh/geometry is what drives physics — no longer just a byproduct of the visual mesh.
  • If a link has no <collision> tag, a collision is synthesized from its <visual> (matching the previous behaviour), so nothing regresses for URDFs that don't define collision geometry.
  • This synthesis happens at the URDF level (before MuJoCo fuses fixed-jointed bodies), so it stays correct per-link even after body fusion.

Is this a user-facing behavior change?

Yes — links that define separate collision geometry in their URDF will now get that geometry in the generated MJCF instead of their visual mesh.

@saikishor
saikishor requested a review from ndunkelb-nasa August 6, 2026 12:38
@codecov-commenter

codecov-commenter commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.39%. Comparing base (e6a6160) to head (5e8920c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #264      +/-   ##
==========================================
+ Coverage   81.20%   81.39%   +0.19%     
==========================================
  Files          30       30              
  Lines        5123     5203      +80     
  Branches      558      559       +1     
==========================================
+ Hits         4160     4235      +75     
- Misses        670      674       +4     
- Partials      293      294       +1     
Flag Coverage Δ
unittests 81.39% <100.00%> (+0.19%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...co_ros2_control/tests/test_urdf_to_mujoco_utils.py 99.82% <100.00%> (+0.01%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ndunkelb-nasa

Copy link
Copy Markdown
Collaborator

@saikishor to do some initial testing, I was trying to see if I could get this to run in our primary workspace, but I get some pretty quick errors when I try to run any urdf -> mjcf conversion there. I wonder if we need some more tests to handle cases with different types of meshes based on what we find out here.

I am trying to look into what the issue is, but if you are curious as well, I am just opening up our workspace on jazzy-devel, checking out this PR for mujoco_ros2_control and running the following.

ros2 launch clr_mujoco_config generate_clr_mjcf.launch.py

In the decomposition section, getting this error

[make_mjcf_from_robot_description.py-1] Traceback (most recent call last):
[make_mjcf_from_robot_description.py-1]   File "/home/er4-user/ws/install/mujoco_ros2_control/lib/mujoco_ros2_control/make_mjcf_from_robot_description.py", line 531, in <module>
[make_mjcf_from_robot_description.py-1]     main()
[make_mjcf_from_robot_description.py-1]   File "/home/er4-user/ws/install/mujoco_ros2_control/lib/mujoco_ros2_control/make_mjcf_from_robot_description.py", line 508, in main
[make_mjcf_from_robot_description.py-1]     fix_mujoco_description(
[make_mjcf_from_robot_description.py-1]   File "/home/er4-user/ws/install/mujoco_ros2_control/lib/mujoco_ros2_control/make_mjcf_from_robot_description.py", line 323, in fix_mujoco_description
[make_mjcf_from_robot_description.py-1]     dom = mrc.update_obj_assets(dom, output_filepath, mesh_info_dict)
[make_mjcf_from_robot_description.py-1]           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[make_mjcf_from_robot_description.py-1]   File "/home/er4-user/ws/install/mujoco_ros2_control/lib/python3.12/site-packages/mujoco_ros2_control/urdf_to_mujoco_utils.py", line 465, in update_obj_assets
[make_mjcf_from_robot_description.py-1]     sub_dom = minidom.parse(mesh_path)
[make_mjcf_from_robot_description.py-1]               ^^^^^^^^^^^^^^^^^^^^^^^^
[make_mjcf_from_robot_description.py-1]   File "/usr/lib/python3.12/xml/dom/minidom.py", line 1985, in parse
[make_mjcf_from_robot_description.py-1]     return expatbuilder.parse(file)
[make_mjcf_from_robot_description.py-1]            ^^^^^^^^^^^^^^^^^^^^^^^^
[make_mjcf_from_robot_description.py-1]   File "/usr/lib/python3.12/xml/dom/expatbuilder.py", line 907, in parse
[make_mjcf_from_robot_description.py-1]     with open(file, 'rb') as fp:
[make_mjcf_from_robot_description.py-1]          ^^^^^^^^^^^^^^^^
[make_mjcf_from_robot_description.py-1] FileNotFoundError: [Errno 2] No such file or directory: 'mjcf_data/assets/decomposed/finger_v6__1/finger_v6__1/finger_v6__1.xml'
[ERROR] [make_mjcf_from_robot_description.py-1]: process has died [pid 22869, exit code 1, cmd '/home/er4-user/ws/install/mujoco_ros2_control/lib/mujoco_ros2_control/make_mjcf_from_robot_description.py --urdf /tmp/tmpmulds1d3.urdf --convert_stl_to_obj --save_only --ros-args'].

Whereas on main, it doesn't try to decompose an __1 version.

@saikishor

saikishor commented Aug 13, 2026

Copy link
Copy Markdown
Member Author

@ndunkelb-nasa can you test it with the last PR #269? Yo see if it happens?. If not, I can take a look and add those cases or verify why it is not working

@saikishor

saikishor commented Aug 14, 2026

Copy link
Copy Markdown
Member Author

@ndunkelb-nasa I tried to test it, but I don't have access to clr_mujoco_config package

@ndunkelb-nasa

Copy link
Copy Markdown
Collaborator

Yeah, that runs all the way through, but still has a bunch of issues after running. In this video, you can see the result of running the generation from main on the left, and the result from running the generation on #269 on the right, then running the output in base mujoco simulate (these both start paused, then I unpause each). Obviously I know that with these changes, I will need to change how the contact disabling and stuff works, but there are a bunch of other problems that you can see in the videos, like daes/objs not keeping their visual meshes throughout the process.

new_generation.webm

Also something seems to have been super crazy with the scaling of this as you can see lol (also starts paused then I unpause).

collision_scaling.webm

There seems to be quite a few things going on here, which is why I was hoping to test things in smaller chunks and verify functionality, rather than trying to fix 4-5 intertwined things from the final PR.

@ndunkelb-nasa I tried to test it, but I don't have access to clr_mujoco_config package

Hmm, you should definitely be able to run, the package is here, and gets pulled into the ws with submodules from the instructions on the front page of clr_ws, but don't want to necessarily force you to set all that up...Maybe the solution is to make a minimal example that breaks for this new implementation? Unfortunately there seem to be a bunch of issues.

@saikishor

Copy link
Copy Markdown
Member Author

@ndunkelb-nasa sure, thanks for the info. I'll take a look and get back to you

@scastro-nasa

scastro-nasa commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

FYI the collision issue with the massive mesh seems to be on our side: NASA-JSC-Robotics/surface_robotics_mockups_description#21

Also @saikishor I'm starting to look at this set of PRs, so I might be putting up some PRs to your branches for your review.

(EDIT: and I may have accidentally pushed to your branch first, but I force-pushed it back)

@saikishor

Copy link
Copy Markdown
Member Author

FYI the collision issue with the massive mesh seems to be on our side: NASA-JSC-Robotics/surface_robotics_mockups_description#21

Also @saikishor I'm starting to look at this set of PRs, so I might be putting up some PRs to your branches for your review.

(EDIT: and I may have accidentally pushed to your branch first, but I force-pushed it back)

No worries. Thanks a lot for taking a look at this.

@scastro-nasa scastro-nasa left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I've merged my change into here and this branch works standalone now with our setup.

NOTE, however, that the default option remains to use the visual meshes, and I added a --use_collision_tags argument that makes it behave like the original version of this PR. I think this will be preferable as it doesn't break user workflows by default, but rather gives them the option.

Will now move up the stack and review the remaining PRs.

@scastro-nasa
scastro-nasa force-pushed the use/collision_tags/select_collision_meshes branch from f5a7a8f to 5e8920c Compare August 31, 2026 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants