-
Notifications
You must be signed in to change notification settings - Fork 2.4k
[FEATURE] Add support of 'contype'/'conaffinity' for primitive and mesh entities. #1438
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
base: main
Are you sure you want to change the base?
Conversation
Can you add a unit test for each feature added? |
|
||
scene.add_entity(gs.morphs.Plane()) | ||
|
||
# NOTE: contype and conaffinity are 32-bit integer bitmasks used for contact filtering of contact pairs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this comment on the very top of this file.
color=(1.0, 0.0, 0.0, 1.0), | ||
), | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not skip line between each add_entity
@@ -197,7 +197,7 @@ def _compute_collision_pair_validity(self): | |||
continue | |||
|
|||
# contype and conaffinity | |||
if links_entity_idx[i_la] == links_entity_idx[i_lb] and not ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not OK with this change. I see what you are trying to achieve and it makes sense, but it has unacceptable side-effects I think.
Typically, if you import twice the same robot, then the same mask will apply to each robot, which means that some parts of different robots will not be able to collide with each other anymore. To be more specific, if contype/conaffinity is defined on some humanoid robot in a way to prevent the hip from colliding with the tight, and if you add 2 robots, the hip of the first robot will not be able to collide with the tight of the other robot either, which is probably undesirable.
I would suggest to add is_collision_mask_local: bool = True
to RigidEntity morph options. This boolean would control whether contype and conaffinity masks are entity-local and global for the entire scene. If there are global, there is nothing to do, but if there are global, you must find a way to update the masks accordingly, which seems to be tricky.
What do you think @YilingQiao ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing out. Yes I think this can cause a problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer the revised version for several reasons:
-
This matches MuJoCo’s behavior (i.e., the entity is not considered when filtering based on coaffinity). We don’t need to introduce additional concepts.
https://github.com/google-deepmind/mujoco/blob/4f43a7d08e4f1289f4d76a4c48b8f42aca5bb38b/src/engine/engine_collision_driver.c#L1479 -
Importing the same robot twice is not a common use case. We haven’t done this in our demos or applications. We might not have to increase the system complexity / divergence from mujoco to support a rare condition.
-
If users really want to use two robots, they can modify the coaffinity bitmask of the two robots—perhaps not through a well-exposed API, but by manually editing the coaffinity in the XML file—to ensure that the pair across entities is not filtered out.
-
Or we can have
is_collision_mask_local
, but maybe default toFalse
to match mujoco, unless some advanced users want?
@@ -12,7 +12,8 @@ | |||
|
|||
""" | |||
We define all types of morphologies here: shape primitives, meshes, URDF, MJCF, and soft robot description files. | |||
These are independent of backend solver type and are shared by different solvers. E.g. a mesh can be either loaded as a rigid object / MPM object / FEM object. | |||
These are independent of backend solver type and are shared by different solvers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please refrain yourself from reformatting documentation that you did not update (I mean adding or removing actual content). This is making the diff much harder to read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, just wanted to enforce the 120 char rule for the file.
Description
Extend
contype
andconaffinity
to entity level.How Has This Been / Can This Be Tested?
This can be used for RL setup where you only want the plane to collide with object not a hand for grasping task.
Screenshots (if appropriate):
Screen.Recording.2025-07-19.at.12.48.36.AM.mov
Note: contype and conaffinity are 32-bit integer bitmasks used for contact filtering of contact pairs.
When the contype of one geom and the conaffinity of the other geom share a common bit set to 1, two geoms can collide.
Checklist:
Submitting Code Changes
section of CONTRIBUTING document.