From 3434f5d9c774bf56d3cf4dd26d0beca8d9c509f1 Mon Sep 17 00:00:00 2001
From: Baruch Tabanpour <btaba@google.com>
Date: Mon, 28 Jul 2025 10:48:41 -0700
Subject: [PATCH] Compute convex hull for collision pairs.

PiperOrigin-RevId: 788074030
Change-Id: I3d4bbe71f824db82e998e374d95a5c49995af806
---
 doc/changelog.rst            |  2 +
 src/user/user_model.cc       | 18 ++++++++-
 test/user/user_model_test.cc | 74 ++++++++++++++++++++++++++++++++++++
 3 files changed, 92 insertions(+), 2 deletions(-)

diff --git a/doc/changelog.rst b/doc/changelog.rst
index 89cb99da..32fa7595 100644
--- a/doc/changelog.rst
+++ b/doc/changelog.rst
@@ -30,6 +30,8 @@ Bug fixes
 - Fixed a bug that caused object lists in the child to have missing elements after attaching an mjSpec. This was caused
   by adding to the lists only the objects that belong to the tree of the requested body, but this causes to skip objects
   that were attached, since they belong to the tree of the parent.
+- Fixed a bug where the convex hull of a collision mesh was not being computed if the mesh could only collide via a
+  :ref:`contact pair<contact-pair>`.
 
 Version 3.3.4 (July 8, 2025)
 ----------------------------
diff --git a/src/user/user_model.cc b/src/user/user_model.cc
index 657f45e7..616154f9 100644
--- a/src/user/user_model.cc
+++ b/src/user/user_model.cc
@@ -4481,11 +4481,25 @@ void mjCModel::TryCompile(mjModel*& m, mjData*& d, const mjVFS* vfs) {
   // map names to asset references
   IndexAssets(/*discard=*/false);
 
+  // compile pairs for convex hull check
+  // TODO(quaglino): Consolidate the two calls to pair->Compile() in TryCompile.
+  for (auto pair : pairs_) pair->Compile();
+
   // mark meshes that need convex hull
   for (int i=0; i < geoms_.size(); i++) {
+    bool is_in_pair = false;
+    for (const mjCPair* pair : pairs_) {
+      if ((pair->geom1 && pair->geom1->id == geoms_[i]->id) ||
+          (pair->geom2 && pair->geom2->id == geoms_[i]->id)) {
+        is_in_pair = true;
+        break;
+      }
+    }
+
     if (geoms_[i]->mesh &&
-        (geoms_[i]->spec.type == mjGEOM_MESH || geoms_[i]->spec.type == mjGEOM_SDF) &&
-        (geoms_[i]->spec.contype || geoms_[i]->spec.conaffinity ||
+        (geoms_[i]->spec.type == mjGEOM_MESH ||
+         geoms_[i]->spec.type == mjGEOM_SDF) &&
+        (geoms_[i]->spec.contype || geoms_[i]->spec.conaffinity || is_in_pair ||
          geoms_[i]->mesh->spec.inertia == mjMESH_INERTIA_CONVEX)) {
       geoms_[i]->mesh->SetNeedHull(true);
     }
diff --git a/test/user/user_model_test.cc b/test/user/user_model_test.cc
index 746aa39e..8c581421 100644
--- a/test/user/user_model_test.cc
+++ b/test/user/user_model_test.cc
@@ -218,6 +218,80 @@ TEST_F(UserCModelTest, NestedZeroMassBodiesFail) {
   mj_deleteModel(model);
 }
 
+TEST_F(UserCModelTest, ConvexHullForCollisionMeshes) {
+  static constexpr char xml[] = R"(
+  <mujoco>
+    <asset>
+      <mesh name="mesh_no_hull" vertex="0 0 0  1 0 0  0 1 0  0 0 1"
+            face="0 2 1  0 1 3  0 3 2  1 2 3"/>
+      <mesh name="mesh_with_hull_contype" vertex="0 0 0  1 0 0  0 1 0  0 0 1"
+            face="0 2 1  0 1 3  0 3 2  1 2 3"/>
+      <mesh name="mesh_with_hull_conaffinity" vertex="0 0 0  1 0 0  0 1 0  0 0 1"
+            face="0 2 1  0 1 3  0 3 2  1 2 3"/>
+    </asset>
+    <worldbody>
+      <geom name="geom_no_hull" type="mesh" mesh="mesh_no_hull" contype="0" conaffinity="0"/>
+      <geom name="geom_with_hull_contype" type="mesh" mesh="mesh_with_hull_contype" contype="1"/>
+      <geom name="geom_with_hull_conaffinity" type="mesh" mesh="mesh_with_hull_conaffinity" conaffinity="1"/>
+    </worldbody>
+  </mujoco>)";
+
+  std::array<char, 1024> error;
+  mjModel* model = LoadModelFromString(xml, error.data(), error.size());
+  ASSERT_THAT(model, NotNull()) << error.data();
+
+  int no_hull_id = mj_name2id(model, mjOBJ_MESH, "mesh_no_hull");
+  int with_hull_contype_id =
+      mj_name2id(model, mjOBJ_MESH, "mesh_with_hull_contype");
+  int with_hull_conaffinity_id =
+      mj_name2id(model, mjOBJ_MESH, "mesh_with_hull_conaffinity");
+
+  EXPECT_NE(no_hull_id, -1);
+  EXPECT_NE(with_hull_contype_id, -1);
+  EXPECT_NE(with_hull_conaffinity_id, -1);
+
+  // mesh_no_hull should not have a convex hull.
+  EXPECT_EQ(model->mesh_graphadr[no_hull_id], -1);
+
+  // mesh_with_hull_contype should have a convex hull.
+  EXPECT_NE(model->mesh_graphadr[with_hull_contype_id], -1);
+
+  // mesh_with_hull_conaffinity should have a convex hull.
+  EXPECT_NE(model->mesh_graphadr[with_hull_conaffinity_id], -1);
+
+  mj_deleteModel(model);
+}
+
+TEST_F(UserCModelTest, ConvexHullForPairCollisionMeshes) {
+  static constexpr char xml[] = R"(
+  <mujoco>
+    <asset>
+      <mesh name="mesh" vertex="0 0 0  1 0 0  0 1 0  0 0 1"
+            face="0 2 1  0 1 3  0 3 2  1 2 3"/>
+    </asset>
+    <worldbody>
+      <geom name="geom1" type="sphere" size="1"/>
+      <geom name="geom_mesh" type="mesh" mesh="mesh" contype="0" conaffinity="0"/>
+    </worldbody>
+    <contact>
+      <pair name="hello" geom1="geom1" geom2="geom_mesh"/>
+    </contact>
+  </mujoco>)";
+
+  std::array<char, 1024> error;
+  mjModel* model = LoadModelFromString(xml, error.data(), error.size());
+  ASSERT_THAT(model, NotNull()) << error.data();
+
+  int mesh_in_pair_id = mj_name2id(model, mjOBJ_MESH, "mesh");
+
+  EXPECT_NE(mesh_in_pair_id, -1);
+
+  // mesh_in_pair should have a convex hull because it is in a collision pair.
+  EXPECT_NE(model->mesh_graphadr[mesh_in_pair_id], -1);
+
+  mj_deleteModel(model);
+}
+
 // ------------- test automatic inference of nuser_xxx -------------------------
 
 using UserDataTest = MujocoTest;