[geometry] RenderEngineGl loads .mtl files (#19747)

geometry/render/render_material.h

@@ -19,6 +19,10 @@ struct RenderMaterial {
    `diffuse_map` is empty, it acts as the multiplicative identity. */
   Rgba diffuse;
   std::filesystem::path diffuse_map;
+
+  /* Whether the material definition comes from the mesh itself, e.g., an .mtl
+   file, as opposed to the user specification or an implied texture. */
+  bool from_mesh_file{false};
 };
 
 /* Dispatches a warning to the given diagnostic policy if the props contain a

geometry/render/render_mesh.cc

@@ -35,6 +35,8 @@ RenderMaterial MakeMaterialFromMtl(const tinyobj::material_t& mat,
                                    bool has_tex_coord,
                                    const DiagnosticPolicy& policy) {
   RenderMaterial result;
+
+  result.from_mesh_file = true;
   result.diffuse.set(mat.diffuse[0], mat.diffuse[1], mat.diffuse[2],
                      mat.dissolve);
   result.diffuse_map = [&mat, &obj_path]() -> std::string {

geometry/render/render_mesh.h

@@ -40,8 +40,7 @@ struct RenderMesh {
   // texture material to a RenderMesh that didn't originally parse into having
   // a texture material. It may or may not work. It also means that this
   // cannot be a struct; structs don't get to maintain invariants.
-  /* This flag indicates that this mesh has texture coordinates to support
-   maps.
+  /* This flag indicates that this mesh has texture coordinates to support maps.
    If True, the values of `uvs` will be nontrivial.
    If False, the values of `uvs` will be all zeros, but will still have the
    correct size.  */
@@ -59,9 +58,9 @@ struct RenderMesh {
 /* Returns a single instance of RenderMesh from the indicated obj file.
 
  The material definition will come from the obj's mtl file iff a single material
- is applied to all faces in the obj. Otherwise, it applies the fallback
- material protocol documented in MakeMeshFallbackMaterial() (the only time in
- which the `properties` and `default_diffuse` parameters are used).
+ is applied to all faces in the obj. Otherwise, it applies the fallback material
+ protocol documented in MakeMeshFallbackMaterial() (the only time in which the
+ `properties` and `default_diffuse` parameters are used).
 
  As long as there is a single material applied to all faces in the obj file,
  the material will be derived from the material library, even if the material

geometry/render/test/render_mesh_test.cc

@@ -748,6 +748,24 @@ TEST_F(LoadRenderMeshFromObjTest, RedundantMaterialWarnings) {
       testing::MatchesRegex(".*has its own materials.*'phong', 'diffuse'.*"));
 }
 
+/* Tests if the `from_mesh_file` flag is correctly propagated. */
+TEST_F(LoadRenderMeshFromObjTest, PropagateFromMeshFileFlag) {
+  for (const bool from_mesh_file : {false, true}) {
+    // N.B. box_no_mtl.obj doesn't exist in the source tree and is generated
+    // from box.obj by stripping out material data by the build system.
+    const std::string filename =
+        from_mesh_file
+            ? FindResourceOrThrow("drake/geometry/render/test/meshes/box.obj")
+            : FindResourceOrThrow(
+                  "drake/geometry/render/test/meshes/box_no_mtl.obj");
+
+    const RenderMesh mesh_data = LoadRenderMeshFromObj(
+        filename, empty_props(), kDefaultDiffuse, diagnostic_policy_);
+    const RenderMaterial material = mesh_data.material;
+    EXPECT_EQ(from_mesh_file, material.from_mesh_file);
+  }
+}
+
 }  // namespace
 }  // namespace internal
 }  // namespace geometry

geometry/render_gl/BUILD.bazel

@@ -78,6 +78,7 @@ drake_cc_library_ubuntu_only(
         ":internal_shape_meshes",
         ":internal_texture_library",
         ":render_engine_gl_params",
+        "//common:diagnostic_policy",
         "//geometry/render:render_engine",
         "//geometry/render:render_mesh",
         "//systems/sensors:image",

geometry/render_gl/internal_render_engine_gl.cc

@@ -19,21 +19,23 @@ namespace internal {
 
 using Eigen::Vector2d;
 using Eigen::Vector3d;
-using geometry::internal::RenderMesh;
 using geometry::internal::LoadRenderMeshFromObj;
+using geometry::internal::MakeMeshFallbackMaterial;
+using geometry::internal::RenderMaterial;
+using geometry::internal::RenderMesh;
 using math::RigidTransformd;
+using render::ColorRenderCamera;
+using render::DepthRenderCamera;
+using render::RenderCameraCore;
+using render::RenderEngine;
+using render::RenderLabel;
 using std::make_shared;
 using std::make_unique;
 using std::shared_ptr;
 using std::string;
 using std::unique_ptr;
 using std::unordered_map;
 using std::vector;
-using render::ColorRenderCamera;
-using render::DepthRenderCamera;
-using render::RenderCameraCore;
-using render::RenderEngine;
-using render::RenderLabel;
 using systems::sensors::ColorD;
 using systems::sensors::ColorI;
 using systems::sensors::ImageDepth32F;
@@ -473,6 +475,19 @@ void main() {
 })""";
 };
 
+// Given a filename (e.g., of a mesh), this produces a string that we use in
+// our maps to guarantee we only load the file once.
+std::string GetPathKey(const std::string& filename) {
+  std::error_code path_error;
+  const fs::path path = fs::canonical(filename, path_error);
+  if (path_error) {
+    throw std::runtime_error(
+        fmt::format("RenderEngineGl: unable to access the file {}; {}",
+                    filename, path_error.message()));
+  }
+  return path.string();
+}
+
 }  // namespace
 
 RenderEngineGl::RenderEngineGl(RenderEngineGlParams params)
@@ -615,28 +630,28 @@ void RenderEngineGl::InitGlState() {
 void RenderEngineGl::ImplementMesh(int geometry_index,
                                    void* user_data,
                                    const Vector3<double>& scale,
-                                   const std::string& file_name) {
+                                   const std::string& filename_in) {
   const RegistrationData& data = *static_cast<RegistrationData*>(user_data);
   PerceptionProperties temp_props(data.properties);
 
   const OpenGlGeometry& geometry = geometries_[geometry_index];
   temp_props.AddProperty(
       kInternalGroup, kHasTexCoordProperty, geometry.has_tex_coord);
 
-  // In order to maintain compatibility with RenderEngineVtk, we need to provide
-  // functionality in which a mesh of the name foo.obj can be matched to a
-  // potential png called foo.png. We rely on the fact that passing in a diffuse
-  // map that doesn't refer to a real file will silently fall back to rgba
-  // diffuse. So, we'll create a copy of the user data, set the diffuse_map
-  // property to appropriately named image and let it percolate through. We
-  // can't and don't want to change the underlying properties because they are
-  // visible to the user.
-  if (!temp_props.HasProperty("phong", "diffuse_map")) {
-    std::filesystem::path file_path(file_name);
-    const string png_name = file_path.replace_extension(".png").string();
-    temp_props.AddProperty("phong", "diffuse_map", png_name);
+  const std::string file_key = GetPathKey(filename_in);
+  RenderMaterial material;
+  // If there is a material associated with the mesh, we will use it. Otherwise,
+  // we recreate the fallback material based on user data and defaults.
+  if (meshes_[file_key].mesh_material.has_value()) {
+    material = meshes_[file_key].mesh_material.value();
+  } else {
+    material = MakeMeshFallbackMaterial(data.properties, filename_in,
+                                        parameters_.default_diffuse,
+                                        diagnostic_policy_);
   }
-
+  temp_props.UpdateProperty("phong", "diffuse_map",
+                            material.diffuse_map.string());
+  temp_props.UpdateProperty("phong", "diffuse", material.diffuse);
   RegistrationData temp_data{data.id, data.X_WG, temp_props};
   AddGeometryInstance(geometry_index, &temp_data, scale);
 }
@@ -957,7 +972,7 @@ int RenderEngineGl::GetBox() {
 }
 
 int RenderEngineGl::GetMesh(const string& filename_in, RegistrationData* data) {
-  int mesh = -1;
+  int mesh_index = -1;
 
   // We're checking the input filename in case the user specified name has the
   // desired extension but is a symlink to some arbitrarily named cached file.
@@ -970,31 +985,29 @@ int RenderEngineGl::GetMesh(const string& filename_in, RegistrationData* data) {
     return -1;
   }
 
-  // Resolve to a canonical path.
-  std::error_code bad_path;
-  const std::string file_key = fs::canonical(filename_in, bad_path).string();
-  if (bad_path) {
-    throw std::runtime_error(fmt::format(
-        "RenderEngineGl: unable to access the requested mesh file '{}'; {}.",
-        filename_in, bad_path.message()));
-  }
-
+  const std::string file_key = GetPathKey(filename_in);
   if (meshes_.count(file_key) == 0) {
-    // TODO(SeanCurtis-TRI): We're ignoring the declared perception properties
-    //  for the mesh. We need to pass it in and return a mesh *and* the
-    //  resulting material properties.
-    RenderMesh mesh_data = LoadRenderMeshFromObj(
-        filename_in, PerceptionProperties(), parameters_.default_diffuse);
-    mesh = CreateGlGeometry(mesh_data);
-    meshes_.insert({file_key, mesh});
+    const RenderMesh mesh_data =
+        LoadRenderMeshFromObj(filename_in, PerceptionProperties(),
+                              parameters_.default_diffuse, diagnostic_policy_);
+    mesh_index = CreateGlGeometry(mesh_data);
+    const RenderMaterial material = mesh_data.material;
+
+    meshes_.insert({file_key, {.mesh_index = mesh_index}});
+    // We will also update RenderGlMesh's `mesh_material` if the material is
+    // defined by the mesh itself. By that, the material will be used every time
+    // the mesh is instantiated.
+    if (material.from_mesh_file) {
+      meshes_[file_key].mesh_material = material;
+    }
   } else {
-    mesh = meshes_[file_key];
+    mesh_index = meshes_[file_key].mesh_index;
   }
 
-  geometries_[mesh].throw_if_undefined(
+  geometries_[mesh_index].throw_if_undefined(
       fmt::format("Error creating object for mesh {}", filename_in).c_str());
 
-  return mesh;
+  return mesh_index;
 }
 
 std::tuple<GLint, GLenum, GLenum> RenderEngineGl::get_texture_format(

geometry/render_gl/internal_render_engine_gl.h

@@ -3,15 +3,18 @@
 #include <array>
 #include <map>
 #include <memory>
+#include <optional>
 #include <string>
 #include <tuple>
 #include <unordered_map>
 #include <vector>
 
 #include "drake/common/copyable_unique_ptr.h"
+#include "drake/common/diagnostic_policy.h"
 #include "drake/common/eigen_types.h"
 #include "drake/geometry/geometry_roles.h"
 #include "drake/geometry/render/render_engine.h"
+#include "drake/geometry/render/render_material.h"
 #include "drake/geometry/render/render_mesh.h"
 #include "drake/geometry/render_gl/internal_buffer_dim.h"
 #include "drake/geometry/render_gl/internal_opengl_context.h"
@@ -146,7 +149,7 @@ class RenderEngineGl final : public render::RenderEngine {
   //                         OpenGlGeometry.
   void ImplementMesh(int geometry_index, void* user_data,
                      const Vector3<double>& scale,
-                     const std::string& file_name);
+                     const std::string& filename_in);
 
   // @see RenderEngine::DoRegisterVisual().
   bool DoRegisterVisual(GeometryId id, const Shape& shape,
@@ -354,9 +357,19 @@ class RenderEngineGl final : public render::RenderEngine {
   // re-use the same geometry. For example, if we tracked them by "aspect ratio"
   // and allowed deviation within a small tolerance, then we could reuse them.
 
-  // Mapping from obj filename to the index into geometries_ containing the
-  // OpenGlGeometry representation of the mesh.
-  std::unordered_map<std::string, int> meshes_;
+  // A data struct that includes the index into geometries_ containing the
+  // OpenGlGeometry representation and an optional material definition of the
+  // mesh. `mesh_material` will have a value *only* when the mesh provides its
+  // own material definition; otherwise, it will remain std::nullopt and the
+  // material will be determined during mesh instantiation.
+  struct RenderGlMesh {
+    int mesh_index{};
+    std::optional<geometry::internal::RenderMaterial> mesh_material{
+        std::nullopt};
+  };
+
+  // Mapping from the obj's canonical filename to RenderGlMesh.
+  std::unordered_map<std::string, RenderGlMesh> meshes_;
 
   // These are caches of reusable RenderTargets. There is a unique render target
   // for each unique image size (BufferDim) and output image type. The
@@ -382,6 +395,9 @@ class RenderEngineGl final : public render::RenderEngine {
   // modify their copy of visuals_ (adding and removing geometries).
   std::unordered_map<GeometryId, OpenGlInstance> visuals_;
 
+  // The policy for handling warnings and errors.
+  drake::internal::DiagnosticPolicy diagnostic_policy_;
+
   // The direction *to* the light expressed in the camera frame.
   Vector3<float> light_dir_C_{0.0f, 0.0f, 1.0f};
 };