Skip to content

Commit

Permalink
Respond to reviewer comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
mitiguy committed Oct 1, 2024
1 parent 49474df commit 0fb5b54
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 13 deletions.
1 change: 1 addition & 0 deletions geometry/test/bad_geometryA.obj
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# File: bad_geometryA.obj
# The geometry in this file produces invalid mass and inertia properties
# due to the winding (order of the vertices) in some of the faces.
# In Drake, the calculated volume of this mesh is zero (volume = 0).
# Related files: bad_geometryB.obj and bad_geometry_corrected.obj
# ----------------------------------------------------------------------

Expand Down
1 change: 1 addition & 0 deletions geometry/test/bad_geometryB.obj
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# File: bad_geometryB.obj
# The geometry in this file produces invalid mass and inertia properties
# due to the winding (order of the vertices) in some of the faces.
# In Drake, the calculated volume of this mesh is negative (volume = -0.5).
# Related files: bad_geometryA.obj and bad_geometry_corrected.obj
# -----------------------------------------------------------------------

Expand Down
14 changes: 11 additions & 3 deletions multibody/tree/geometry_spatial_inertia.cc
Original file line number Diff line number Diff line change
Expand Up @@ -130,17 +130,25 @@ SpatialInertia<double> CalcSpatialInertia(

// Volume should be inherently positive.
const double volume = vol_times_six / 6.0;
constexpr double kEpsilon = std::numeric_limits<double>::epsilon();
// Throw an exception if volume is negative or nearly zero. This test of
// "reasonably positive" volume is more stringent than the mass ≥ 0 test in
// SpatialInertia::IsPhysicallyValid(). Reminder: The volume of a mesh can be
// calculated (and should be positive) whereas spatial inertia does not deal
// with volume. Instead, spatial inertia deals with mass, including idealized
// zero volume massive objects such as particles, rods, and plates.
constexpr double kEpsilon = 8 * std::numeric_limits<double>::epsilon();
if (volume <= kEpsilon) {
const std::string error_message = fmt::format(
"{}(): The calculated volume of a triangle surface mesh is {} whereas "
"a reasonable positive value was expected. The mesh may have bad "
"geometry, e.g., the winding (order of the vertices) of some faces do "
"not produce outward normals.", __func__, volume);
"geometry, e.g., it is an open mesh or the winding (order of vertices) "
"of one or more faces do not produce outward normals.",
__func__, volume);
throw std::logic_error(error_message);
// Note: In a Wavefront .obj file, each face's vertices should be stored
// in a counter-clockwise order by default.
}

const double mass = density * volume;
const Vector3d p_GoGcm = accum_com / (vol_times_six * 4);
// We can compute I = C.trace * 1₃ - C. Two key points:
Expand Down
1 change: 1 addition & 0 deletions multibody/tree/geometry_spatial_inertia.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ SpatialInertia<double> CalcSpatialInertia(const geometry::Shape& shape,
If these requirements are not met, a value *will* be returned, but its value
is meaningless.
@throws std::exception if the volume of `mesh` is negative or nearly zero.
@pydrake_mkdoc_identifier{mesh} */
SpatialInertia<double> CalcSpatialInertia(
const geometry::TriangleSurfaceMesh<double>& mesh, double density);
Expand Down
30 changes: 20 additions & 10 deletions multibody/tree/test/geometry_spatial_inertia_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -159,25 +159,35 @@ GTEST_TEST(GeometrySpatialInertiaTest, Convex) {
/* tolerance = */ 1e-1));
}

// Check exception messages when CalcSpatialInertia(Shape) is called on a file
// with bad geometry (e.g. not watertight or bad outward normals, or ...).
// Throw an exception message when CalcSpatialInertia(Shape) calculates an
// invalid volume for an associated geometry file.
GTEST_TEST(GeometrySpatialInertiaTest, ExceptionOnBadGeometry) {
std::string geometry_file_path = FindResourceOrThrow(
"drake/geometry/test/bad_geometryA.obj");
// Throw an exception for the mesh in bad_geometryA.obj since
// its calculated volume is negative (volume = -0.5).
std::string geometry_file_path =
FindResourceOrThrow("drake/geometry/test/bad_geometryA.obj");
const geometry::Mesh bad_geometryA_obj(geometry_file_path, 1.0);
DRAKE_EXPECT_THROWS_MESSAGE(CalcSpatialInertia(bad_geometryA_obj, kDensity),
DRAKE_EXPECT_THROWS_MESSAGE(
CalcSpatialInertia(bad_geometryA_obj, kDensity),
".*volume of a triangle surface mesh is.* whereas a reasonable "
"positive value was expected. The mesh may have bad geometry.*");

geometry_file_path = FindResourceOrThrow(
"drake/geometry/test/bad_geometryB.obj");
// Throw an exception for the mesh in bad_geometryB.obj since
// its calculated volume is negative (volume = -0.5).
geometry_file_path =
FindResourceOrThrow("drake/geometry/test/bad_geometryB.obj");
const geometry::Mesh bad_geometryB_obj(geometry_file_path, 1.0);
DRAKE_EXPECT_THROWS_MESSAGE(CalcSpatialInertia(bad_geometryB_obj, kDensity),
DRAKE_EXPECT_THROWS_MESSAGE(
CalcSpatialInertia(bad_geometryB_obj, kDensity),
".*volume of a triangle surface mesh is.* whereas a reasonable "
"positive value was expected. The mesh may have bad geometry.*");

geometry_file_path = FindResourceOrThrow(
"drake/geometry/test/bad_geometry_corrected.obj");
// Ensure no exception is thrown for the mesh in bad_geometry_corrected.obj.
// This file has the same vertices as bad_geometryA_obj and bad_geometryB_obj,
// but its faces has vertices in an appropriate order.
// This mesh has an appropriate spatial inertia and positive volume.
geometry_file_path =
FindResourceOrThrow("drake/geometry/test/bad_geometry_corrected.obj");
const geometry::Mesh ok_geometry_obj(geometry_file_path, 1.0);
EXPECT_NO_THROW(CalcSpatialInertia(ok_geometry_obj, kDensity));
}
Expand Down

0 comments on commit 0fb5b54

Please sign in to comment.