From b32107d403acf4c9956c37c2f51a2f4ed2beb2db Mon Sep 17 00:00:00 2001 From: xuchenhan-tri Date: Thu, 11 Apr 2024 11:42:46 -0700 Subject: [PATCH] Make RenderMaterial optional in RenderMesh (#21289) --- geometry/geometry_state.cc | 13 +- geometry/render/render_material.cc | 68 ++++---- geometry/render/render_material.h | 26 ++- geometry/render/render_mesh.cc | 22 ++- geometry/render/render_mesh.h | 29 +++- geometry/render/test/render_material_test.cc | 83 +++++---- geometry/render/test/render_mesh_test.cc | 158 +++++++++++------- .../render_gl/internal_render_engine_gl.cc | 26 ++- .../render_vtk/internal_render_engine_vtk.cc | 10 +- 9 files changed, 262 insertions(+), 173 deletions(-) diff --git a/geometry/geometry_state.cc b/geometry/geometry_state.cc index 877cda01da65..3f3c71cc0d66 100644 --- a/geometry/geometry_state.cc +++ b/geometry/geometry_state.cc @@ -1882,12 +1882,6 @@ void GeometryState::RegisterDrivenPerceptionMesh(GeometryId geometry_id) { DRAKE_DEMAND(geometry.is_deformable()); DRAKE_DEMAND(geometry.has_perception_role()); const PerceptionProperties& properties = *geometry.perception_properties(); - // TODO(xuchenhan-tri): Each render engine customizes its own default diffuse - // color. By setting a default value here, we are subverting the engine, - // preventing it from applying its own logic. Lack of a diffuse color should - // propagate all the way down to the engine for the engine to resolve. - const auto default_rgba = properties.GetPropertyOrDefault( - "phong", "diffuse", Rgba{1.0, 1.0, 1.0, 1.0}); const VolumeMesh* control_mesh_ptr = geometry.reference_mesh(); DRAKE_DEMAND(control_mesh_ptr != nullptr); @@ -1902,11 +1896,10 @@ void GeometryState::RegisterDrivenPerceptionMesh(GeometryId geometry_id) { // control volume mesh as the render mesh. driven_meshes.emplace_back(internal::MakeDrivenSurfaceMesh(control_mesh)); render_meshes.emplace_back(MakeRenderMeshFromTriangleSurfaceMesh( - driven_meshes.back().triangle_surface_mesh(), properties, - default_rgba)); + driven_meshes.back().triangle_surface_mesh(), properties)); } else { - render_meshes = internal::LoadRenderMeshesFromObj(render_meshes_file, - properties, default_rgba); + render_meshes = + internal::LoadRenderMeshesFromObj(render_meshes_file, properties, {}); for (const internal::RenderMesh& render_mesh : render_meshes) { driven_meshes.emplace_back(MakeTriangleSurfaceMesh(render_mesh), control_mesh); diff --git a/geometry/render/render_material.cc b/geometry/render/render_material.cc index 1412ba4539aa..a8c6c96815e2 100644 --- a/geometry/render/render_material.cc +++ b/geometry/render/render_material.cc @@ -15,6 +15,12 @@ namespace internal { using drake::internal::DiagnosticPolicy; +RenderMaterial MakeDiffuseMaterial(const Rgba& diffuse) { + RenderMaterial result; + result.diffuse = diffuse; + return result; +} + void MaybeWarnForRedundantMaterial(const GeometryProperties& props, std::string_view mesh_name, const DiagnosticPolicy& policy) { @@ -39,11 +45,10 @@ void MaybeWarnForRedundantMaterial(const GeometryProperties& props, } } -RenderMaterial MakeMeshFallbackMaterial(const GeometryProperties& props, - const std::filesystem::path& mesh_path, - const Rgba& default_diffuse, - const DiagnosticPolicy& policy, - UvState uv_state) { +std::optional MaybeMakeMeshFallbackMaterial( + const GeometryProperties& props, const std::filesystem::path& mesh_path, + const std::optional& default_diffuse, const DiagnosticPolicy& policy, + UvState uv_state) { // If a material is indicated *at all* in the properties, that // defines the material. if (props.HasProperty("phong", "diffuse") || @@ -60,35 +65,38 @@ RenderMaterial MakeMeshFallbackMaterial(const GeometryProperties& props, return DefineMaterial(props, Rgba(1, 1, 1), policy, uv_state); } + std::optional default_result{std::nullopt}; + if (default_diffuse.has_value()) { + default_result = MakeDiffuseMaterial(*default_diffuse); + } + if (mesh_path.empty()) { + return default_result; + } // Checks for foo.png for mesh filename foo.*. This the legacy behavior we // want to do away with. - RenderMaterial material; - - // This is the fall-through condition; the final priority in the protocol. - material.diffuse = default_diffuse; + std::filesystem::path alt_texture_path(mesh_path); + alt_texture_path.replace_extension("png"); + // If we find foo.png but can't access it, we'll *silently* treat it like + // we didn't find it. No value in warning for a behavior we're cutting. + if (!std::ifstream(alt_texture_path).is_open()) { + return default_result; + } - if (!mesh_path.empty()) { - std::filesystem::path alt_texture_path(mesh_path); - alt_texture_path.replace_extension("png"); - // If we find foo.png but can't access it, we'll *silently* treat it like - // we didn't find it. No value in warning for a behavior we're cutting. - if (std::ifstream(alt_texture_path).is_open()) { - if (uv_state == UvState::kFull) { - material.diffuse_map = alt_texture_path; - } else { - policy.Warning(fmt::format( - "A png file of the same name as the mesh has been found ('{}'), " - "but the mesh doesn't define {} texture coordinates. The map will " - "be omitted leaving a flat white color.", - alt_texture_path.string(), - uv_state == UvState::kNone ? "any" : "a complete set of")); - } - // A white color will leave the texture unmodulated. In the case where - // we couldn't apply the texture, flat white also corresponds to the - // warning message. - material.diffuse = Rgba(1, 1, 1); - } + RenderMaterial material; + if (uv_state == UvState::kFull) { + material.diffuse_map = alt_texture_path; + } else { + policy.Warning(fmt::format( + "A png file of the same name as the mesh has been found ('{}'), " + "but the mesh doesn't define {} texture coordinates. The map will " + "be omitted leaving a flat white color.", + alt_texture_path.string(), + uv_state == UvState::kNone ? "any" : "a complete set of")); } + // A white color will leave the texture unmodulated. In the case where + // we couldn't apply the texture, flat white also corresponds to the + // warning message. + material.diffuse = Rgba(1, 1, 1); return material; } diff --git a/geometry/render/render_material.h b/geometry/render/render_material.h index e3c52ef92692..013bb535b768 100644 --- a/geometry/render/render_material.h +++ b/geometry/render/render_material.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include "drake/common/diagnostic_policy.h" #include "drake/geometry/geometry_properties.h" @@ -29,6 +30,16 @@ struct RenderMaterial { bool from_mesh_file{false}; }; +/* Creates a RenderMaterial with the specified diffuse color. + + Consumers of RenderMesh can call this function to produce an untextured + RenderMaterial with the prescribed diffuse color when presented with RenderMesh + instances that do not come with their own material definitions. + + @param diffuse The RGBA color to be used as the diffuse color for the + material. */ +RenderMaterial MakeDiffuseMaterial(const Rgba& diffuse); + /* Dispatches a warning to the given diagnostic policy if the props contain a material definition. It is assumed an intrinsic material has already been found for the named mesh. */ @@ -38,7 +49,8 @@ void MaybeWarnForRedundantMaterial( /* If a mesh definition doesn't include a single material (e.g., as in an .mtl file for an .obj mesh), this function applies a cascading priority for - otherwise defining a material for the mesh. + potentially defining a material. Failing everything in the priority list, it + will return std::nullopt. The material is defined with the following protocol: @@ -48,17 +60,21 @@ void MaybeWarnForRedundantMaterial( - Otherwise, if an image can be located with a "compatible name" (e.g., foo.png for a mesh foo.obj), a material with an unmodulated texture is created. - - Finally, a diffuse material is created with the given default_diffuse - color value. + - Otherwise, if a default_diffuse value is provided, a material is created + with the given default_diffuse color value. + - Finally, if no material is defined, std::nullopt is returned. In such a + case, a consumer of the returned mesh can generate its own material using + its default diffuse color with MakeDiffuseMaterial(). Such a material would + be compliant with the heuristic defined in @ref geometry_materials. References to textures will be included in the material iff they can be read and the `uv_state` is full. Otherwise, a warning will be dispatched. @pre The mesh (named by `mesh_filename`) is a valid mesh and did not have an acceptable material definition). */ -RenderMaterial MakeMeshFallbackMaterial( +std::optional MaybeMakeMeshFallbackMaterial( const GeometryProperties& props, const std::filesystem::path& mesh_path, - const Rgba& default_diffuse, + const std::optional& default_diffuse, const drake::internal::DiagnosticPolicy& policy, UvState uv_state); /* Creates a RenderMaterial from the given set of geometry properties. If no diff --git a/geometry/render/render_mesh.cc b/geometry/render/render_mesh.cc index 8ba10089717f..0ada152a74ff 100644 --- a/geometry/render/render_mesh.cc +++ b/geometry/render/render_mesh.cc @@ -94,7 +94,8 @@ RenderMaterial MakeMaterialFromMtl(const tinyobj::material_t& mat, vector LoadRenderMeshesFromObj( const std::filesystem::path& obj_path, const GeometryProperties& properties, - const Rgba& default_diffuse, const DiagnosticPolicy& policy) { + const std::optional& default_diffuse, + const DiagnosticPolicy& policy) { tinyobj::ObjReaderConfig config; config.triangulate = true; config.vertex_color = false; @@ -264,8 +265,8 @@ vector LoadRenderMeshesFromObj( DRAKE_DEMAND(positions.size() == uvs.size()); /* Now we need to partition the prepped geometry data. Each material used - will lead to a unique `RenderMesh` and `RenderMaterial`. Note: the obj may - have declared distinct *objects*. We are erasing that distinction as + will lead to a unique `RenderMesh` and possibly a `RenderMaterial`. Note: the + obj may have declared distinct *objects*. We are erasing that distinction as irrelevant for rendering the mesh as a rigid structure. */ vector meshes; for (const auto& [mat_index, tri_indices] : material_triangles) { @@ -279,7 +280,7 @@ vector LoadRenderMeshesFromObj( if (mat_index == -1) { /* This is the default material. No material was assigned to the faces. We'll apply the fallback logic. */ - mesh_data.material = MakeMeshFallbackMaterial( + mesh_data.material = MaybeMakeMeshFallbackMaterial( properties, obj_path, default_diffuse, policy, mesh_data.uv_state); } else { mesh_data.material = @@ -332,11 +333,10 @@ vector LoadRenderMeshesFromObj( RenderMesh MakeRenderMeshFromTriangleSurfaceMesh( const TriangleSurfaceMesh& mesh, - const GeometryProperties& properties, const Rgba& default_diffuse, - const DiagnosticPolicy& policy) { + const GeometryProperties& properties, const DiagnosticPolicy& policy) { RenderMesh result; - result.material = MakeMeshFallbackMaterial(properties, "", default_diffuse, - policy, UvState::kNone); + result.material = + MaybeMakeMeshFallbackMaterial(properties, "", {}, policy, UvState::kNone); const int vertex_count = mesh.num_vertices(); const int triangle_count = mesh.num_triangles(); result.positions.resize(vertex_count, 3); @@ -367,8 +367,7 @@ RenderMesh MakeRenderMeshFromTriangleSurfaceMesh( RenderMesh MakeFacetedRenderMeshFromTriangleSurfaceMesh( const TriangleSurfaceMesh& mesh, - const GeometryProperties& properties, const Rgba& default_diffuse, - const DiagnosticPolicy& policy) { + const GeometryProperties& properties, const DiagnosticPolicy& policy) { // The simple solution is to create a *new* mesh where every triangle has its // own vertices and then pass to MakeRenderMeshFromTriangleSurfaceMesh(). // If this ever becomes an onerous burden, we can do that directly into the @@ -386,8 +385,7 @@ RenderMesh MakeFacetedRenderMeshFromTriangleSurfaceMesh( } const TriangleSurfaceMesh faceted(std::move(triangles), std::move(vertices)); - return MakeRenderMeshFromTriangleSurfaceMesh(faceted, properties, - default_diffuse, policy); + return MakeRenderMeshFromTriangleSurfaceMesh(faceted, properties, policy); } TriangleSurfaceMesh MakeTriangleSurfaceMesh( diff --git a/geometry/render/render_mesh.h b/geometry/render/render_mesh.h index 0bf7817c7fee..b7f157d357a8 100644 --- a/geometry/render/render_mesh.h +++ b/geometry/render/render_mesh.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include @@ -37,8 +38,12 @@ struct RenderMesh { meaningful. */ UvState uv_state{UvState::kNone}; - /* The specification of the material associated with this mesh data. */ - RenderMaterial material; + /* The specification of the material associated with this mesh data. + `material` may be undefined (`std::nullopt`). Why it is undefined depends on + the origin of the `RenderMesh`. The consumer of the mesh is free to define + the material as they see fit by defining an arbitrary bespoke material or + using a utility like MakeDiffuseMaterial(). */ + std::optional material; }; // TODO(SeanCurtis-TRI): All of this explanation, and general guidance for what @@ -59,6 +64,16 @@ struct RenderMesh { - Specifying a texture but failing to provide texture coordinates. - etc. + For faces with no material referenced in the obj file, this function creates a + RenderMesh that may or may not have a material by using the fallback logic. If + a `default_diffuse` value is provided, then a RenderMaterial is guaranteed to + be created. If no `default_diffuse` is provided, then the RenderMesh _may_ have + no material assigned. See MaybeMakeMeshFallbackMaterial() for more details. In + particular, if no material is assigned, then all heuristics in + MaybeMakeMeshFallbackMaterial() have already been attempted, and the only way + for the consumer of the RenderMesh to obtain a material is through + MakeDiffuseMaterial(). + Note: This API is similar to ReadObjToTriangleSurfaceMesh, but it differs in several ways: @@ -90,10 +105,10 @@ struct RenderMesh { they are present. */ std::vector LoadRenderMeshesFromObj( const std::filesystem::path& obj_path, const GeometryProperties& properties, - const Rgba& default_diffuse, + const std::optional& default_diffuse, const drake::internal::DiagnosticPolicy& policy = {}); -/* Constructs a render mesh from a triangle surface mesh. +/* Constructs a render mesh (without material) from a triangle surface mesh. The normal of a vertex v is computed using area-weighted normals of the triangles containing vertex v. In particular, for a watertight mesh, this will @@ -103,13 +118,13 @@ std::vector LoadRenderMeshesFromObj( UvState will be set to UvState::kNone and all uv coordinates are set to zero. The material of the resulting render mesh is created using the protocol in - MakeMeshFallbackMaterial() with the given `properties` and `default_diffuse`. + MaybeMakeMeshFallbackMaterial() with the given `properties`. The returned RenderMesh has the same number of positions, vertex normals, and uv coordinates as `mesh.num_vertices()`. */ RenderMesh MakeRenderMeshFromTriangleSurfaceMesh( const TriangleSurfaceMesh& mesh, - const GeometryProperties& properties, const Rgba& default_diffuse, + const GeometryProperties& properties, const drake::internal::DiagnosticPolicy& policy = {}); /* A variant of MakeRenderMeshFromTriangleSurfaceMesh(). In this case, the @@ -117,7 +132,7 @@ RenderMesh MakeRenderMeshFromTriangleSurfaceMesh( as a faceted mesh. */ RenderMesh MakeFacetedRenderMeshFromTriangleSurfaceMesh( const TriangleSurfaceMesh& mesh, - const GeometryProperties& properties, const Rgba& default_diffuse, + const GeometryProperties& properties, const drake::internal::DiagnosticPolicy& policy = {}); /* Converts from RenderMesh to TriangleSurfaceMesh. Only connectivity and diff --git a/geometry/render/test/render_material_test.cc b/geometry/render/test/render_material_test.cc index 0fd9201a40d9..d2d3a5a3988f 100644 --- a/geometry/render/test/render_material_test.cc +++ b/geometry/render/test/render_material_test.cc @@ -157,7 +157,7 @@ TEST_F(DefineMaterialTest, DiffuseMapUvCoverageError) { } } -/* Tests the MakeMeshFallbackMaterial() function. This function should only +/* Tests the MaybeMakeMeshFallbackMaterial() function. This function should only be called if the mesh has no intrinsic material. Its unique duties are: - Determine if a material is defined *at all* in the geometry properties and @@ -171,7 +171,8 @@ TEST_F(DefineMaterialTest, DiffuseMapUvCoverageError) { Note: the only diagnostic policy messages that get sent are attributable to DefineMaterial(), so they are not directly accounted for in this test. */ -class MakeMeshFallbackMaterialTest : public test::DiagnosticPolicyTestBase { +class MaybeMakeMeshFallbackMaterialTest + : public test::DiagnosticPolicyTestBase { protected: static Rgba default_diffuse() { return Rgba(0.125, 0.25, 0.375, 0.5); } }; @@ -180,19 +181,31 @@ class MakeMeshFallbackMaterialTest : public test::DiagnosticPolicyTestBase { material. This doesn't test the case where foo.png *does* exist, but isn't available. We're not testing the "unaccessible foo.png" case. Not worth it in light of its imminent death. */ -TEST_F(MakeMeshFallbackMaterialTest, DefaultDiffuseMaterial) { +TEST_F(MaybeMakeMeshFallbackMaterialTest, DefaultDiffuseMaterial) { PerceptionProperties props; - const RenderMaterial mat = - MakeMeshFallbackMaterial(props, "no_png_for_this.obj", default_diffuse(), - diagnostic_policy_, UvState::kFull); + const std::optional mat = MaybeMakeMeshFallbackMaterial( + props, "no_png_for_this.obj", default_diffuse(), diagnostic_policy_, + UvState::kFull); + ASSERT_TRUE(mat.has_value()); + EXPECT_TRUE(mat->diffuse_map.empty()); + EXPECT_EQ(mat->diffuse, default_diffuse()); +} - EXPECT_TRUE(mat.diffuse_map.empty()); - EXPECT_EQ(mat.diffuse, default_diffuse()); +/* No material defined in the properties and no foo.png --> default-colored + material. The default diffuse color is not provided either. No + material-generating condition is met and std::nullopt is expected. */ +TEST_F(MaybeMakeMeshFallbackMaterialTest, NoMaterial) { + PerceptionProperties props; + + const std::optional mat = + MaybeMakeMeshFallbackMaterial(props, "no_png_for_this.obj", std::nullopt, + diagnostic_policy_, UvState::kFull); + EXPECT_FALSE(mat.has_value()); } /* No material defined in the properties, but foo.png exists and is available.*/ -TEST_F(MakeMeshFallbackMaterialTest, ValidFooPngMaterial) { +TEST_F(MaybeMakeMeshFallbackMaterialTest, ValidFooPngMaterial) { PerceptionProperties props; const std::string tex_name = FindResourceOrThrow("drake/geometry/render/test/meshes/box.png"); @@ -218,11 +231,12 @@ TEST_F(MakeMeshFallbackMaterialTest, ValidFooPngMaterial) { for (const TestCase& test_case : cases) { SCOPED_TRACE(test_case.description); - const RenderMaterial mat = - MakeMeshFallbackMaterial(props, obj_path.string(), default_diffuse(), - diagnostic_policy_, test_case.uv_state); - EXPECT_EQ(mat.diffuse_map, test_case.expected_texture); - EXPECT_EQ(mat.diffuse, Rgba(1, 1, 1)); + const std::optional mat = MaybeMakeMeshFallbackMaterial( + props, obj_path.string(), default_diffuse(), diagnostic_policy_, + test_case.uv_state); + ASSERT_TRUE(mat.has_value()); + EXPECT_EQ(mat->diffuse_map, test_case.expected_texture); + EXPECT_EQ(mat->diffuse, Rgba(1, 1, 1)); if (!test_case.error.empty()) { EXPECT_THAT( TakeWarning(), @@ -234,7 +248,7 @@ TEST_F(MakeMeshFallbackMaterialTest, ValidFooPngMaterial) { } /* The presence of any material property should create a material. - MakeMeshFallbackMaterial() defers to DefineMaterial() in the presence of + MaybeMakeMeshFallbackMaterial() defers to DefineMaterial() in the presence of material properties. For diffuse color, it passes a white default. We just need evidence that suggests it gets called as expected. @@ -246,42 +260,45 @@ TEST_F(MakeMeshFallbackMaterialTest, ValidFooPngMaterial) { As we extend the set of material properties Drake knows about, we should add an independent test for each one, and each should likewise be added into the PropertiesHaveEverything test. */ -TEST_F(MakeMeshFallbackMaterialTest, PropertiesHaveDiffuseColor) { +TEST_F(MaybeMakeMeshFallbackMaterialTest, PropertiesHaveDiffuseColor) { PerceptionProperties props; props.AddProperty("phong", "diffuse", Rgba(0.25, 0.5, 0.75, 0.5)); - const RenderMaterial mat = - MakeMeshFallbackMaterial(props, "doesn't_matter.obj", default_diffuse(), - diagnostic_policy_, UvState::kFull); + const std::optional mat = MaybeMakeMeshFallbackMaterial( + props, "doesn't_matter.obj", default_diffuse(), diagnostic_policy_, + UvState::kFull); - EXPECT_TRUE(mat.diffuse_map.empty()); - EXPECT_EQ(mat.diffuse, props.GetProperty("phong", "diffuse")); + ASSERT_TRUE(mat.has_value()); + EXPECT_TRUE(mat->diffuse_map.empty()); + EXPECT_EQ(mat->diffuse, props.GetProperty("phong", "diffuse")); } -TEST_F(MakeMeshFallbackMaterialTest, PropertiesHaveDiffuseMap) { +TEST_F(MaybeMakeMeshFallbackMaterialTest, PropertiesHaveDiffuseMap) { PerceptionProperties props; const std::string tex_name = FindResourceOrThrow("drake/geometry/render/test/diag_gradient.png"); props.AddProperty("phong", "diffuse_map", tex_name); - const RenderMaterial mat = - MakeMeshFallbackMaterial(props, "doesn't_matter.obj", default_diffuse(), - diagnostic_policy_, UvState::kFull); + const std::optional mat = MaybeMakeMeshFallbackMaterial( + props, "doesn't_matter.obj", default_diffuse(), diagnostic_policy_, + UvState::kFull); - EXPECT_EQ(mat.diffuse_map, tex_name); - EXPECT_EQ(mat.diffuse, Rgba(1, 1, 1)); + ASSERT_TRUE(mat.has_value()); + EXPECT_EQ(mat->diffuse_map, tex_name); + EXPECT_EQ(mat->diffuse, Rgba(1, 1, 1)); } -TEST_F(MakeMeshFallbackMaterialTest, PropertiesHaveEverything) { +TEST_F(MaybeMakeMeshFallbackMaterialTest, PropertiesHaveEverything) { PerceptionProperties props; const std::string tex_name = FindResourceOrThrow("drake/geometry/render/test/diag_gradient.png"); props.AddProperty("phong", "diffuse_map", tex_name); props.AddProperty("phong", "diffuse", Rgba(0.25, 0.5, 0.75, 0.5)); - const RenderMaterial mat = - MakeMeshFallbackMaterial(props, "doesn't_matter.obj", default_diffuse(), - diagnostic_policy_, UvState::kFull); + const std::optional mat = MaybeMakeMeshFallbackMaterial( + props, "doesn't_matter.obj", default_diffuse(), diagnostic_policy_, + UvState::kFull); - EXPECT_EQ(mat.diffuse_map, tex_name); - EXPECT_EQ(mat.diffuse, props.GetProperty("phong", "diffuse")); + ASSERT_TRUE(mat.has_value()); + EXPECT_EQ(mat->diffuse_map, tex_name); + EXPECT_EQ(mat->diffuse, props.GetProperty("phong", "diffuse")); } } // namespace diff --git a/geometry/render/test/render_mesh_test.cc b/geometry/render/test/render_mesh_test.cc index 0a5bf550aec6..321b2fa4599f 100644 --- a/geometry/render/test/render_mesh_test.cc +++ b/geometry/render/test/render_mesh_test.cc @@ -383,8 +383,9 @@ TEST_F(LoadRenderMeshFromObjTest, MaterialLibraryColorOnly) { obj_path, empty_props(), kDefaultDiffuse, diagnostic_policy_); ASSERT_EQ(result.size(), 1); const RenderMesh& mesh = result[0]; - EXPECT_EQ(mesh.material.diffuse, Rgba(0.5, 1, 1)); - EXPECT_EQ(mesh.material.diffuse_map, ""); + ASSERT_TRUE(mesh.material.has_value()); + EXPECT_EQ(mesh.material->diffuse, Rgba(0.5, 1, 1)); + EXPECT_EQ(mesh.material->diffuse_map, ""); } /* The OBJ has a single intrinsic material which only defines diffuse texture. @@ -406,8 +407,9 @@ TEST_F(LoadRenderMeshFromObjTest, MaterialLibraryTextureOnly) { obj_path, empty_props(), kDefaultDiffuse, diagnostic_policy_); ASSERT_EQ(result.size(), 1); const RenderMesh& mesh = result[0]; - EXPECT_EQ(mesh.material.diffuse, Rgba(1, 1, 1)); - EXPECT_THAT(mesh.material.diffuse_map, + ASSERT_TRUE(mesh.material.has_value()); + EXPECT_EQ(mesh.material->diffuse, Rgba(1, 1, 1)); + EXPECT_THAT(mesh.material->diffuse_map, testing::EndsWith("diag_gradient.png")); } @@ -429,8 +431,9 @@ TEST_F(LoadRenderMeshFromObjTest, MaterialLibraryColorAndTexture) { obj_path, empty_props(), kDefaultDiffuse, diagnostic_policy_); ASSERT_EQ(result.size(), 1); const RenderMesh& mesh = result[0]; - EXPECT_EQ(mesh.material.diffuse, Rgba(1, 1, 0)); - EXPECT_THAT(mesh.material.diffuse_map, + ASSERT_TRUE(mesh.material.has_value()); + EXPECT_EQ(mesh.material->diffuse, Rgba(1, 1, 0)); + EXPECT_THAT(mesh.material->diffuse_map, testing::EndsWith("diag_gradient.png")); } @@ -462,8 +465,9 @@ TEST_F(LoadRenderMeshFromObjTest, MaterialLibraryDislocatedTexture) { obj_path, empty_props(), kDefaultDiffuse, diagnostic_policy_); ASSERT_EQ(result.size(), 1); const RenderMesh& mesh = result[0]; - EXPECT_EQ(mesh.material.diffuse, Rgba(1, 1, 1)); - EXPECT_EQ(mesh.material.diffuse_map, (temp_dir() / "diag_gradient.png")); + ASSERT_TRUE(mesh.material.has_value()); + EXPECT_EQ(mesh.material->diffuse, Rgba(1, 1, 1)); + EXPECT_EQ(mesh.material->diffuse_map, (temp_dir() / "diag_gradient.png")); } /* The OBJ has multiple intrinsic materials *defined* in the .mtl file. But only @@ -484,8 +488,9 @@ TEST_F(LoadRenderMeshFromObjTest, MaterialLibraryMultipleDefinedIntrinsic) { obj_path, empty_props(), kDefaultDiffuse, diagnostic_policy_); ASSERT_EQ(result.size(), 1); const RenderMesh& mesh = result[0]; - EXPECT_EQ(mesh.material.diffuse, Rgba(1, 0, 1)); - EXPECT_EQ(mesh.material.diffuse_map, ""); + ASSERT_TRUE(mesh.material.has_value()); + EXPECT_EQ(mesh.material->diffuse, Rgba(1, 0, 1)); + EXPECT_EQ(mesh.material->diffuse_map, ""); } /* The OBJ has a single intrinsic material which defines a bad texture. The @@ -507,8 +512,9 @@ TEST_F(LoadRenderMeshFromObjTest, MaterialLibraryColorAndBadTexture) { obj_path, empty_props(), kDefaultDiffuse, diagnostic_policy_); ASSERT_EQ(result.size(), 1); const RenderMesh& mesh = result[0]; - EXPECT_EQ(mesh.material.diffuse, Rgba(1, 0.5, 1)); - EXPECT_EQ(mesh.material.diffuse_map, ""); + ASSERT_TRUE(mesh.material.has_value()); + EXPECT_EQ(mesh.material->diffuse, Rgba(1, 0.5, 1)); + EXPECT_EQ(mesh.material->diffuse_map, ""); EXPECT_THAT( TakeWarning(), testing::HasSubstr("requested an unavailable diffuse texture image")); @@ -534,8 +540,9 @@ TEST_F(LoadRenderMeshFromObjTest, MaterialLibraryTextureButNoUvs) { EXPECT_THAT(TakeWarning(), testing::MatchesRegex(".*requested a diffuse texture.*doesn't " "define any texture coordinates.*")); - EXPECT_EQ(mesh.material.diffuse, Rgba(1, 1, 0)); - EXPECT_EQ(mesh.material.diffuse_map, ""); + ASSERT_TRUE(mesh.material.has_value()); + EXPECT_EQ(mesh.material->diffuse, Rgba(1, 1, 0)); + EXPECT_EQ(mesh.material->diffuse_map, ""); } /* The OBJ has a single intrinsic material with a texture but no uvs. The @@ -560,8 +567,9 @@ TEST_F(LoadRenderMeshFromObjTest, MaterialLibraryTextureButPartialUvs) { TakeWarning(), testing::MatchesRegex(".*requested a diffuse texture.*doesn't define a " "complete set of texture coordinates.*")); - EXPECT_EQ(mesh.material.diffuse, Rgba(1, 1, 0)); - EXPECT_EQ(mesh.material.diffuse_map, ""); + ASSERT_TRUE(mesh.material.has_value()); + EXPECT_EQ(mesh.material->diffuse, Rgba(1, 1, 0)); + EXPECT_EQ(mesh.material->diffuse_map, ""); } /* This test merely exposes a known flaw in the code. Images are defined @@ -588,8 +596,9 @@ TEST_F(LoadRenderMeshFromObjTest, MaterialLibraryObjMtlDislocation) { obj_path, empty_props(), kDefaultDiffuse, diagnostic_policy_); ASSERT_EQ(result.size(), 1); const RenderMesh& mesh = result[0]; - EXPECT_EQ(mesh.material.diffuse, Rgba(1, 1, 0)); - EXPECT_EQ(mesh.material.diffuse_map, ""); + ASSERT_TRUE(mesh.material.has_value()); + EXPECT_EQ(mesh.material->diffuse, Rgba(1, 1, 0)); + EXPECT_EQ(mesh.material->diffuse_map, ""); // Because of our limited access to parsing information, this will look like // the image is unavailable. EXPECT_THAT( @@ -620,14 +629,15 @@ TEST_F(LoadRenderMeshFromObjTest, MultipleValidIntrinsicMaterials) { obj_path, empty_props(), kDefaultDiffuse, diagnostic_policy_); ASSERT_EQ(meshes.size(), 2); for (const auto& mesh : meshes) { + ASSERT_TRUE(mesh.material.has_value()); if (mesh.indices.rows() == 1) { // test_material_1 applied to a single triangle. - EXPECT_EQ(mesh.material.diffuse, Rgba(1, 0, 1)); - EXPECT_EQ(mesh.material.diffuse_map, ""); + EXPECT_EQ(mesh.material->diffuse, Rgba(1, 0, 1)); + EXPECT_EQ(mesh.material->diffuse_map, ""); } else if (mesh.indices.rows() == 2) { // test_material_2 applied to two triangles. - EXPECT_EQ(mesh.material.diffuse, Rgba(1, 0, 0)); - EXPECT_EQ(mesh.material.diffuse_map, ""); + EXPECT_EQ(mesh.material->diffuse, Rgba(1, 0, 0)); + EXPECT_EQ(mesh.material->diffuse_map, ""); } else { DRAKE_UNREACHABLE(); } @@ -657,8 +667,9 @@ TEST_F(LoadRenderMeshFromObjTest, MaterialFallbackObjMtlDislocationAbsolute) { obj_path, empty_props(), kDefaultDiffuse, diagnostic_policy_); ASSERT_EQ(result.size(), 1); const RenderMesh& mesh = result[0]; - EXPECT_EQ(mesh.material.diffuse, kDefaultDiffuse); - EXPECT_EQ(mesh.material.diffuse_map, ""); + ASSERT_TRUE(mesh.material.has_value()); + EXPECT_EQ(mesh.material->diffuse, kDefaultDiffuse); + EXPECT_EQ(mesh.material->diffuse_map, ""); // The tinyobj warning that the material library can't be found. EXPECT_THAT(TakeWarning(), testing::HasSubstr("not found in a path")); } @@ -683,14 +694,15 @@ TEST_F(LoadRenderMeshFromObjTest, MaterialFallbackDefaultedFaces) { obj_path, empty_props(), kDefaultDiffuse, diagnostic_policy_); ASSERT_EQ(meshes.size(), 2); for (const auto& mesh : meshes) { + ASSERT_TRUE(mesh.material.has_value()); if (mesh.indices.rows() == 1) { // default material applied to a single triangle. - EXPECT_EQ(mesh.material.diffuse, kDefaultDiffuse); - EXPECT_EQ(mesh.material.diffuse_map, ""); + EXPECT_EQ(mesh.material->diffuse, kDefaultDiffuse); + EXPECT_EQ(mesh.material->diffuse_map, ""); } else if (mesh.indices.rows() == 2) { // test_material applied to two triangles. - EXPECT_EQ(mesh.material.diffuse, Rgba(0.5, 1, 1)); - EXPECT_EQ(mesh.material.diffuse_map, ""); + EXPECT_EQ(mesh.material->diffuse, Rgba(0.5, 1, 1)); + EXPECT_EQ(mesh.material->diffuse_map, ""); } else { DRAKE_UNREACHABLE(); } @@ -714,8 +726,9 @@ TEST_F(LoadRenderMeshFromObjTest, MaterialFallbackBadMaterialName) { obj_path, empty_props(), kDefaultDiffuse, diagnostic_policy_); ASSERT_EQ(result.size(), 1); const RenderMesh& mesh = result[0]; - EXPECT_EQ(mesh.material.diffuse, kDefaultDiffuse); - EXPECT_EQ(mesh.material.diffuse_map, ""); + ASSERT_TRUE(mesh.material.has_value()); + EXPECT_EQ(mesh.material->diffuse, kDefaultDiffuse); + EXPECT_EQ(mesh.material->diffuse_map, ""); EXPECT_THAT(TakeWarning(), testing::HasSubstr("material [ 'bad_material' ] not found")); } @@ -736,8 +749,9 @@ TEST_F(LoadRenderMeshFromObjTest, MaterialFallbackMissingMtl) { obj_path, empty_props(), kDefaultDiffuse, diagnostic_policy_); ASSERT_EQ(result.size(), 1); const RenderMesh& mesh = result[0]; - EXPECT_EQ(mesh.material.diffuse, kDefaultDiffuse); - EXPECT_EQ(mesh.material.diffuse_map, ""); + ASSERT_TRUE(mesh.material.has_value()); + EXPECT_EQ(mesh.material->diffuse, kDefaultDiffuse); + EXPECT_EQ(mesh.material->diffuse_map, ""); EXPECT_THAT( TakeWarning(), testing::HasSubstr("Material file [ not_really_a.mtl ] not found")); @@ -759,8 +773,9 @@ TEST_F(LoadRenderMeshFromObjTest, MaterialFallbackNoAppliedMaterial) { obj_path, empty_props(), kDefaultDiffuse, diagnostic_policy_); ASSERT_EQ(result.size(), 1); const RenderMesh& mesh = result[0]; - EXPECT_EQ(mesh.material.diffuse, kDefaultDiffuse); - EXPECT_EQ(mesh.material.diffuse_map, ""); + ASSERT_TRUE(mesh.material.has_value()); + EXPECT_EQ(mesh.material->diffuse, kDefaultDiffuse); + EXPECT_EQ(mesh.material->diffuse_map, ""); } /* The OBJ references no material library. The material should be the fallback @@ -777,8 +792,35 @@ TEST_F(LoadRenderMeshFromObjTest, MaterialFallbackNoMaterialLibrary) { obj_path, empty_props(), kDefaultDiffuse, diagnostic_policy_); ASSERT_EQ(result.size(), 1); const RenderMesh& mesh = result[0]; - EXPECT_EQ(mesh.material.diffuse, kDefaultDiffuse); - EXPECT_EQ(mesh.material.diffuse_map, ""); + ASSERT_TRUE(mesh.material.has_value()); + EXPECT_EQ(mesh.material->diffuse, kDefaultDiffuse); + EXPECT_EQ(mesh.material->diffuse_map, ""); +} + +/* The OBJ references no material library. The material should be the fallback + material. If a diffuse color is specified through the properties, then it is + used to make a material. However, with no property specified *and* no default + diffuse value, the parsed RenderMesh doesn't have any material. */ +TEST_F(LoadRenderMeshFromObjTest, MaterialFallbackWithNoDefaultDiffuse) { + const fs::path obj_path = WriteFile(R"""( + v 0 0 0 + v 1 1 1 + v 2 2 2 + vn 0 1 0 + f 1//1 2//1 3//1)""", + "no_mtllib.obj"); + Rgba diffuse(0.1, 0.2, 0.3, 0.4); + PerceptionProperties props; + props.AddProperty("phong", "diffuse", diffuse); + vector result_with_material = LoadRenderMeshesFromObj( + obj_path, props, std::nullopt, diagnostic_policy_); + ASSERT_EQ(result_with_material.size(), 1); + EXPECT_TRUE(result_with_material[0].material.has_value()); + + vector result_without_material = LoadRenderMeshesFromObj( + obj_path, empty_props(), std::nullopt, diagnostic_policy_); + ASSERT_EQ(result_without_material.size(), 1); + EXPECT_FALSE(result_without_material[0].material.has_value()); } /* If the mesh has a material library and material properties are *also* @@ -806,8 +848,9 @@ TEST_F(LoadRenderMeshFromObjTest, RedundantMaterialWarnings) { ASSERT_EQ(result.size(), 1); const RenderMesh& mesh = result[0]; // We still get the intrinsic material. - EXPECT_EQ(mesh.material.diffuse, Rgba(0.5, 1, 1)); - EXPECT_EQ(mesh.material.diffuse_map, ""); + ASSERT_TRUE(mesh.material.has_value()); + EXPECT_EQ(mesh.material->diffuse, Rgba(0.5, 1, 1)); + EXPECT_EQ(mesh.material->diffuse_map, ""); EXPECT_THAT( TakeWarning(), testing::MatchesRegex(".*has its own materials.*'phong', 'diffuse'.*")); @@ -836,8 +879,9 @@ TEST_F(LoadRenderMeshFromObjTest, UvStatePassedToFallback) { ASSERT_EQ(result.size(), 1); const RenderMesh& mesh = result[0]; // Inability to apply a valid texture leaves the material flat white. - EXPECT_EQ(mesh.material.diffuse, Rgba(1, 1, 1)); - EXPECT_EQ(mesh.material.diffuse_map, ""); + ASSERT_TRUE(mesh.material.has_value()); + EXPECT_EQ(mesh.material->diffuse, Rgba(1, 1, 1)); + EXPECT_EQ(mesh.material->diffuse_map, ""); EXPECT_THAT(TakeWarning(), testing::MatchesRegex( ".*'diffuse_map'.* doesn't define a complete set of.*")); @@ -856,8 +900,8 @@ TEST_F(LoadRenderMeshFromObjTest, PropagateFromMeshFileFlag) { const RenderMesh mesh_data = LoadRenderMeshesFromObj( filename, empty_props(), kDefaultDiffuse, diagnostic_policy_)[0]; - const RenderMaterial material = mesh_data.material; - EXPECT_EQ(from_mesh_file, material.from_mesh_file); + ASSERT_TRUE(mesh_data.material.has_value()); + EXPECT_EQ(from_mesh_file, mesh_data.material->from_mesh_file); } } @@ -870,10 +914,8 @@ GTEST_TEST(MakeRenderMeshFromTriangleSurfaceMeshTest, SingleTriangle) { triangles.emplace_back(0, 1, 2); const TriangleSurfaceMesh tri_mesh(std::move(triangles), std::move(vertices)); const PerceptionProperties empty_props; - const Rgba default_diffuse(0.1, 0.2, 0.3, 0.4); - - RenderMesh render_mesh = MakeRenderMeshFromTriangleSurfaceMesh( - tri_mesh, empty_props, default_diffuse); + RenderMesh render_mesh = + MakeRenderMeshFromTriangleSurfaceMesh(tri_mesh, empty_props); // Check geometry. Eigen::Matrix expected_positions( @@ -909,19 +951,7 @@ GTEST_TEST(MakeRenderMeshFromTriangleSurfaceMeshTest, SingleTriangle) { EXPECT_EQ(render_mesh.uv_state, UvState::kNone); // Check material - EXPECT_EQ(render_mesh.material.diffuse, default_diffuse); - EXPECT_EQ(render_mesh.material.diffuse_map, std::filesystem::path{}); - EXPECT_FALSE(render_mesh.material.from_mesh_file); - - // Make render mesh with diffuse properties set. - PerceptionProperties props_with_diffuse; - Rgba expected_diffuse(0.1, 0.1, 0.7, 0.9); - props_with_diffuse.AddProperty("phong", "diffuse", expected_diffuse); - render_mesh = MakeRenderMeshFromTriangleSurfaceMesh( - tri_mesh, props_with_diffuse, default_diffuse); - EXPECT_EQ(render_mesh.material.diffuse, expected_diffuse); - EXPECT_EQ(render_mesh.material.diffuse_map, std::filesystem::path{}); - EXPECT_FALSE(render_mesh.material.from_mesh_file); + EXPECT_FALSE(render_mesh.material.has_value()); } // Tests that the vertex normals are indeed computed using area weighted average @@ -954,9 +984,8 @@ GTEST_TEST(MakeRenderMeshFromTriangleSurfaceMeshTest, AreaWeightedNormals) { 3); // Area sqrt(3)/2, normal (1, 1, 1)/sqrt(3) const TriangleSurfaceMesh tri_mesh(std::move(triangles), std::move(vertices)); const PerceptionProperties empty_props; - Rgba default_diffuse(0.1, 0.2, 0.3, 0.4); - const RenderMesh render_mesh = MakeRenderMeshFromTriangleSurfaceMesh( - tri_mesh, empty_props, default_diffuse); + const RenderMesh render_mesh = + MakeRenderMeshFromTriangleSurfaceMesh(tri_mesh, empty_props); // Compare the computed normals with the pen-and-paper result. Note that this // tetrahedron has been selected so that the weighted normal for v0 points in @@ -980,8 +1009,7 @@ GTEST_TEST(MakeRenderMeshFromTriangleSurfaceMeshTest, RoundTrip) { LoadRenderMeshesFromObj(filename, empty_props, kDefaultDiffuse)[0]; const TriangleSurfaceMesh tri_mesh = MakeTriangleSurfaceMesh(render_mesh); const RenderMesh roundtrip_render_mesh = - MakeRenderMeshFromTriangleSurfaceMesh(tri_mesh, empty_props, - kDefaultDiffuse); + MakeRenderMeshFromTriangleSurfaceMesh(tri_mesh, empty_props); EXPECT_EQ(render_mesh.positions, roundtrip_render_mesh.positions); EXPECT_EQ(render_mesh.normals, roundtrip_render_mesh.normals); EXPECT_EQ(render_mesh.indices, roundtrip_render_mesh.indices); @@ -1021,7 +1049,7 @@ GTEST_TEST(MakeFacetedRenderMeshFromTriangleSurfaceMeshTest, Simple) { const TriangleSurfaceMesh tri_mesh(std::move(triangles), std::move(vertices)); const RenderMesh render_mesh = MakeFacetedRenderMeshFromTriangleSurfaceMesh( - tri_mesh, PerceptionProperties(), Rgba(0.1, 0.2, 0.3, 0.4)); + tri_mesh, PerceptionProperties()); ASSERT_EQ(render_mesh.indices.rows(), 2); // Three unique vertices per triangle. diff --git a/geometry/render_gl/internal_render_engine_gl.cc b/geometry/render_gl/internal_render_engine_gl.cc index 38ad11a157b4..a2b54302a073 100644 --- a/geometry/render_gl/internal_render_engine_gl.cc +++ b/geometry/render_gl/internal_render_engine_gl.cc @@ -24,7 +24,8 @@ namespace internal { using Eigen::Vector2d; using Eigen::Vector3d; using geometry::internal::LoadRenderMeshesFromObj; -using geometry::internal::MakeMeshFallbackMaterial; +using geometry::internal::MakeDiffuseMaterial; +using geometry::internal::MaybeMakeMeshFallbackMaterial; using geometry::internal::RenderMaterial; using geometry::internal::RenderMesh; using geometry::internal::UvState; @@ -855,7 +856,7 @@ void RenderEngineGl::ImplementMeshesForFile(void* user_data, if (gl_mesh.mesh_material.has_value()) { material = gl_mesh.mesh_material.value(); } else { - material = MakeMeshFallbackMaterial( + material = *MaybeMakeMeshFallbackMaterial( data.properties, filename, parameters_.default_diffuse, drake::internal::DiagnosticPolicy(), gl_mesh.uv_state); } @@ -886,12 +887,14 @@ bool RenderEngineGl::DoRegisterDeformableVisual( CreateGlGeometry(render_mesh, /* is_deformable */ true); DRAKE_DEMAND(mesh_index >= 0); gl_mesh_indices.emplace_back(mesh_index); - + const RenderMaterial& material = + render_mesh.material.has_value() + ? *render_mesh.material + : MakeDiffuseMaterial(parameters_.default_diffuse); PerceptionProperties mesh_properties(properties); mesh_properties.UpdateProperty("phong", "diffuse_map", - render_mesh.material.diffuse_map.string()); - mesh_properties.UpdateProperty("phong", "diffuse", - render_mesh.material.diffuse); + material.diffuse_map.string()); + mesh_properties.UpdateProperty("phong", "diffuse", material.diffuse); RegistrationData data{id, RigidTransformd::Identity(), mesh_properties}; AddGeometryInstance(mesh_index, &data, kUnitScale); } @@ -1258,8 +1261,12 @@ void RenderEngineGl::CacheConvexHullMesh(const Convex& convex, geometry::internal::MakeTriangleFromPolygonMesh(hull); RenderMesh render_mesh = geometry::internal::MakeFacetedRenderMeshFromTriangleSurfaceMesh( - tri_hull, data.properties, parameters_.default_diffuse); - + tri_hull, data.properties); + // Fall back to the default diffuse material if and only if no material has + // been assigned. + if (!render_mesh.material.has_value()) { + render_mesh.material = MakeDiffuseMaterial(parameters_.default_diffuse); + } const int mesh_index = CreateGlGeometry(render_mesh); DRAKE_DEMAND(mesh_index >= 0); // Note: the material is left as std::nullopt, so that the instance of this @@ -1306,7 +1313,8 @@ void RenderEngineGl::CacheFileMeshesMaybe(const std::string& filename, // Only store materials defined by the obj file; otherwise let instances // define their own (see ImplementMeshesForFile()). - const RenderMaterial& material = render_mesh.material; + DRAKE_DEMAND(render_mesh.material.has_value()); + const RenderMaterial& material = *render_mesh.material; if (material.from_mesh_file) { file_meshes.back().mesh_material = material; } diff --git a/geometry/render_vtk/internal_render_engine_vtk.cc b/geometry/render_vtk/internal_render_engine_vtk.cc index 698ca5f14f0e..938b96108de5 100644 --- a/geometry/render_vtk/internal_render_engine_vtk.cc +++ b/geometry/render_vtk/internal_render_engine_vtk.cc @@ -63,6 +63,7 @@ using Eigen::Vector3d; using Eigen::Vector4d; using geometry::internal::DefineMaterial; using geometry::internal::LoadRenderMeshesFromObj; +using geometry::internal::MakeDiffuseMaterial; using geometry::internal::RenderMaterial; using geometry::internal::RenderMesh; using math::RigidTransformd; @@ -223,7 +224,10 @@ void RenderEngineVtk::ImplementGeometry(const Convex& convex, void* user_data) { geometry::internal::MakeTriangleFromPolygonMesh(convex.GetConvexHull()); RenderMesh render_mesh = geometry::internal::MakeFacetedRenderMeshFromTriangleSurfaceMesh( - tri_hull, data.properties, default_diffuse_); + tri_hull, data.properties); + if (!render_mesh.material.has_value()) { + render_mesh.material = MakeDiffuseMaterial(default_diffuse_); + } // We don't use convex.scale() because it's already built in to the convex // hull. ImplementRenderMesh(std::move(render_mesh), /* scale =*/1.0, data); @@ -530,7 +534,9 @@ RenderEngineVtk::RenderEngineVtk(const RenderEngineVtk& other) void RenderEngineVtk::ImplementRenderMesh(RenderMesh&& mesh, double scale, const RegistrationData& data) { - const RenderMaterial material = mesh.material; + const RenderMaterial material = mesh.material.has_value() + ? *mesh.material + : MakeDiffuseMaterial(default_diffuse_); vtkSmartPointer mesh_source = CreateVtkMesh(std::move(mesh));