From 94c4f729076909698c393f9fea7f284ab9a50a90 Mon Sep 17 00:00:00 2001
From: Alessio Quaglino <quaglino@google.com>
Date: Tue, 18 Apr 2023 08:15:52 -0700
Subject: [PATCH] Specify is volume is too small or negative in mesh error
 messages. Fixes #799.

PiperOrigin-RevId: 525151156
Change-Id: Ia0e9d8b900b8c372e9229da934a4eb7aeb1a3b7a
---
 src/user/user_mesh.cc       |  6 ++++--
 src/user/user_objects.h     |  2 +-
 test/user/user_mesh_test.cc | 24 +++++++++++++++++++++++-
 3 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/src/user/user_mesh.cc b/src/user/user_mesh.cc
index a921b4cb..00c83f7e 100644
--- a/src/user/user_mesh.cc
+++ b/src/user/user_mesh.cc
@@ -124,7 +124,7 @@ mjCMesh::mjCMesh(mjCModel* _model, mjCDef* _def) {
   invalidorientation.first = -1;
   invalidorientation.second = -1;
   validarea = true;
-  validvolume = true;
+  validvolume = 1;
   valideigenvalue = true;
   validinequality = true;
   processed = false;
@@ -1060,7 +1060,7 @@ void mjCMesh::Process() {
 
     // require positive volume
     if (GetVolumeRef(type) < mjMINVAL) {
-      validvolume = false;
+      validvolume = GetVolumeRef(type) < 0 ? -1 : 0;
       return;
     }
 
@@ -1196,6 +1196,8 @@ void mjCMesh::CheckMesh() {
                    name.c_str(), invalidorientation.first, invalidorientation.second);
   if (!validarea)
     throw mjCError(this, "mesh surface area is too small: %s", name.c_str());
+  if (validvolume<0)
+    throw mjCError(this, "mesh volume is negative (misoriented triangles): %s", name.c_str());
   if (!validvolume)
     throw mjCError(this, "mesh volume is too small: %s", name.c_str());
   if (!valideigenvalue)
diff --git a/src/user/user_objects.h b/src/user/user_objects.h
index fdc362a3..ef6d6bb1 100644
--- a/src/user/user_objects.h
+++ b/src/user/user_objects.h
@@ -511,7 +511,7 @@ class mjCMesh: public mjCBase {
   // mesh properties that indicate a well-formed mesh
   std::pair<int, int> invalidorientation;     // indices of invalid edge; -1 if none
   bool validarea;                             // false if the area is too small
-  bool validvolume;                           // false if the volume is too small
+  int validvolume;                            // 0: volume is too small, -1: volume is negative
   bool valideigenvalue;                       // false if inertia eigenvalue is too small
   bool validinequality;                       // false if inertia inequality is not satisfied
   bool processed;                             // false if the mesh has not been processed yet
diff --git a/test/user/user_mesh_test.cc b/test/user/user_mesh_test.cc
index 902fb7fa..e511a795 100644
--- a/test/user/user_mesh_test.cc
+++ b/test/user/user_mesh_test.cc
@@ -464,7 +464,29 @@ TEST_F(MjCMeshTest, VolumeTooSmall) {
   EXPECT_THAT(error.data(), HasSubstr("mesh volume is too small"));
 }
 
-  TEST_F(MjCMeshTest, VolumeTooSmallAllowedWorld) {
+TEST_F(MjCMeshTest, VolumeNegative) {
+  static constexpr char xml[] = R"(
+  <mujoco>
+    <compiler exactmeshinertia="true"/>
+    <asset>
+      <mesh name="example_mesh"
+        vertex="0 0 0  1 0 0  0 1 0  0 0 1"
+        face="3 0 2  0 3 1  1 3 2  0 1 2" />
+    </asset>
+    <worldbody>
+      <body>
+        <geom type="mesh" mesh="example_mesh"/>
+      </body>
+    </worldbody>
+  </mujoco>
+  )";
+  std::array<char, 1024> error;
+  mjModel* model = LoadModelFromString(xml, error.data(), error.size());
+  EXPECT_THAT(model, testing::IsNull());
+  EXPECT_THAT(error.data(), HasSubstr("mesh volume is negative"));
+}
+
+TEST_F(MjCMeshTest, VolumeTooSmallAllowedWorld) {
   static constexpr char xml[] = R"(
   <mujoco>
     <asset>