mujoco: Add support for GetContactsFromLastStep feature#889
Draft
iche033 wants to merge 4 commits intomujoco_prototypefrom
Draft
mujoco: Add support for GetContactsFromLastStep feature#889iche033 wants to merge 4 commits intomujoco_prototypefrom
iche033 wants to merge 4 commits intomujoco_prototypefrom
Conversation
iche033
changed the title
iche033
force-pushed
the
iche033/mujoco_get_contacts
branch
from
5ab3e5a to
ab4ab42
Compare
Signed-off-by: Ian Chen <iche@intrinsic.ai>
iche033
force-pushed
the
iche033/mujoco_get_contacts
branch
from
ab4ab42 to
d93aa03
Compare
This was referenced Mar 6, 2026
azeey
reviewed
Mar 10, 2026
CMakeLists.txt
Outdated
Comment on lines
+56
to
+89
| # Ensure LTO is disabled for the mujoco target and its dependencies so it | ||
| # builds a standard shared library | ||
| if(TARGET mujoco) | ||
| set_target_properties(mujoco PROPERTIES INTERPROCEDURAL_OPTIMIZATION OFF) | ||
|
|
||
| # Workaround for https://github.com/google-deepmind/mujoco/pull/3069 | ||
| if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR CMAKE_C_COMPILER_ID STREQUAL "GNU") | ||
| target_compile_options(mujoco PRIVATE -Wno-error=stringop-truncation) | ||
| endif() | ||
|
|
||
| # MuJoCo's build system forces CMAKE_INTERPROCEDURAL_OPTIMIZATION ON for its | ||
| # dependencies (ccd, lodepng, etc.). We need to disable it for them too, | ||
| # otherwise they are built as LTO bitcode which cannot be properly bundled | ||
| # into a non-LTO shared library using --whole-archive. | ||
| # See, issue: https://github.com/google-deepmind/mujoco/issues/2904 | ||
| set(MUJOCO_DEPS ccd lodepng qhullstatic_r tinyobjloader tinyxml2) | ||
| foreach(dep ${MUJOCO_DEPS}) | ||
| if(TARGET ${dep}) | ||
| set_target_properties(${dep} PROPERTIES INTERPROCEDURAL_OPTIMIZATION OFF) | ||
| endif() | ||
| endforeach() | ||
|
|
||
| if(NOT APPLE AND NOT MSVC) | ||
| # Prevent bundled static libraries (like tinyxml2) from exporting symbols, | ||
| # avoiding conflicts with system versions. | ||
| target_link_options(mujoco PRIVATE "-Wl,--exclude-libs,ALL") | ||
| endif() | ||
| endif() | ||
|
|
||
| # We use target_link_libraries to use the proper mujoco::mujoco alias | ||
| add_library(mujoco_fetched INTERFACE IMPORTED) | ||
| target_link_libraries(mujoco_fetched INTERFACE mujoco::mujoco) | ||
| target_link_options(mujoco_fetched INTERFACE | ||
| "-Wl,-rpath,$<TARGET_FILE_DIR:mujoco>") |
Contributor
There was a problem hiding this comment.
I found that the main problem was the use of FetchContent which caused targets and cmake flags from mujoco and its dependencies to leak into gz-physics targets. For example, the tinyxml2 static library was being linked against the dartsim plugin. I've changed it to use ExternalProject_Add which is similar to compiling mujoco externally (d7e5de5). I now have all (unskipped) tests passing on Linux and macOS. Can you give it a look?
Contributor
Author
There was a problem hiding this comment.
Removed the cmake changes in this PR as I confirmed that d7e5de5 fixes the tinyxml2 symbol issue.
I also removed the LTO changes in CMake. Turns out it's because on my machine I also have lld installed and it's picking that up as the linker instead of default gnu bfd. After removing lld, libmujoco.so is linked properly with correct symbols.
Signed-off-by: Ian Chen <iche@intrinsic.ai>
added 2 commits
March 10, 2026 23:24
Signed-off-by: Ian Chen <iche@intrinsic.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎉 New feature
Addresses #299
Builds on top of #811
Summary
Added the GetContactsFromLastStep feature for mujoco plugin.
geomIdToShapeInfomap to associate internal MuJoCo geom indices with ShapeInfo entitiesmjSpec, otherwise collisions test fails for the case when a fixed joint is created between a link andworldmjsBody*pointers (instead of names) for parent-child resolution. Names are not always correct since mujoco uses scoped names.Other changes:
Test it
The tests below should pass.
Checklist
codecheckpassed (See contributing)Generated-by: Gemini 3
Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-byandGenerated-bymessages.Backports: If this is a backport, please use Rebase and Merge instead.