Skip to content

Commit

Permalink
Fix for Issue #5924: (#5938)
Browse files Browse the repository at this point in the history
* Fix for Issue #5924:
Changed TriangleMesh::materials_ to be a std::vector<std::pair<std::string, Material>> so that the order of materials when loading a mesh is respected.
Using the unordered map caused issues when a Default Material and a texture were loaded and the texture would end up as the second material in the iterator.
Now TriangleMesh::triangle_material_ids_ will indicate what material index should be used.
A warning is given when passing a TriangleMesh to the visualizer and more than one material index is found, and the minimum value found in TriangleMesh::triangle_material_ids_ is used.
If no triangle_material_ids_ exist then the first material in the order they are loaded will be used.

* Updated CHANGELOG.md with info about fixing Issue #5924

* Updates to work with changes to the translation of tensor mesh materials

---------
  • Loading branch information
dbs4261 authored Oct 14, 2023
1 parent a1ae217 commit 3ddc69b
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 17 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* Fix raycasting scene: Allow setting of number of threads that are used for building a raycasting scene
* Fix Python bindings for CUDA device synchronization, voxel grid saving (PR #5425)
* Support msgpack versions without cmake
* Changed TriangleMesh to store materials in a list so they can be accessed by the material index (PR #5938)
* Support multi-threading in the RayCastingScene function to commit scene (PR #6051).
* Fix some bad triangle generation in TriangleMesh::SimplifyQuadricDecimation

Expand Down
2 changes: 1 addition & 1 deletion cpp/open3d/geometry/TriangleMesh.h
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,7 @@ class TriangleMesh : public MeshBase {
std::unordered_map<std::string, Image> additionalMaps;
};

std::unordered_map<std::string, Material> materials_;
std::vector<std::pair<std::string, Material>> materials_;

/// List of material ids.
std::vector<int> triangle_material_ids_;
Expand Down
11 changes: 6 additions & 5 deletions cpp/open3d/io/file_format/FileASSIMP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -237,12 +237,13 @@ bool ReadTriangleMeshUsingASSIMP(
}

// Now load the materials
mesh.materials_.resize(scene->mNumMaterials);
for (size_t i = 0; i < scene->mNumMaterials; ++i) {
auto* mat = scene->mMaterials[i];

// create material structure to match this name
auto& mesh_material =
mesh.materials_[std::string(mat->GetName().C_Str())];
// Set the material structure to match this name
auto& mesh_material = mesh.materials_[i].second;
mesh.materials_[i].first = mat->GetName().C_Str();

using MaterialParameter =
geometry::TriangleMesh::Material::MaterialParameter;
Expand Down Expand Up @@ -277,9 +278,9 @@ bool ReadTriangleMeshUsingASSIMP(

// For legacy visualization support
if (mesh_material.albedo) {
mesh.textures_.push_back(*mesh_material.albedo->FlipVertical());
mesh.textures_.emplace_back(*mesh_material.albedo->FlipVertical());
} else {
mesh.textures_.push_back(geometry::Image());
mesh.textures_.emplace_back();
}
}

Expand Down
7 changes: 5 additions & 2 deletions cpp/open3d/io/file_format/FileOBJ.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,11 @@ bool ReadTriangleMeshFromOBJ(const std::string& filename,
using MaterialParameter =
geometry::TriangleMesh::Material::MaterialParameter;

for (auto& material : materials) {
auto& meshMaterial = mesh.materials_[material.name];
mesh.materials_.resize(materials.size());
for (std::size_t i = 0; i < materials.size(); ++i) {
auto& material = materials[i];
mesh.materials_[i].first = material.name;
auto& meshMaterial = mesh.materials_[i].second;

meshMaterial.baseColor = MaterialParameter::CreateRGB(
material.diffuse[0], material.diffuse[1], material.diffuse[2]);
Expand Down
4 changes: 3 additions & 1 deletion cpp/open3d/t/geometry/TriangleMesh.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,9 @@ open3d::geometry::TriangleMesh TriangleMesh::ToLegacy() const {
// Convert material if the t geometry has a valid one
auto &tmat = GetMaterial();
if (tmat.IsValid()) {
auto &legacy_mat = mesh_legacy.materials_["Mat1"];
mesh_legacy.materials_.emplace_back();
mesh_legacy.materials_.front().first = "Mat1";
auto &legacy_mat = mesh_legacy.materials_.front().second;
// Convert scalar properties
if (tmat.HasBaseColor()) {
legacy_mat.baseColor.f4[0] = tmat.GetBaseColor().x();
Expand Down
23 changes: 18 additions & 5 deletions cpp/open3d/visualization/visualizer/O3DVisualizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -941,11 +941,24 @@ struct O3DVisualizer::Impl {

// Finally assign material properties if geometry is a triangle mesh
if (tmesh && tmesh->materials_.size() > 0) {
// Only a single material is supported for TriangleMesh so we
// just grab the first one we find. Users should be using
// TriangleMeshModel if they have a model with multiple
// materials.
auto &mesh_material = tmesh->materials_.begin()->second;
std::size_t material_index;
if (tmesh->HasTriangleMaterialIds()) {
auto minmax_it = std::minmax_element(
tmesh->triangle_material_ids_.begin(),
tmesh->triangle_material_ids_.end());
if (*minmax_it.first != *minmax_it.second) {
utility::LogWarning(
"Only a single material is "
"supported for TriangleMesh visualization, "
"only the first referenced material will be "
"used. Use TriangleMeshModel if more than one "
"material is required.");
}
material_index = *minmax_it.first;
} else {
material_index = 0;
}
auto &mesh_material = tmesh->materials_[material_index].second;
mat.base_color = {mesh_material.baseColor.r(),
mesh_material.baseColor.g(),
mesh_material.baseColor.b(),
Expand Down
11 changes: 8 additions & 3 deletions cpp/tests/t/geometry/TriangleMesh.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,9 @@ TEST_P(TriangleMeshPermuteDevices, FromLegacy) {
Eigen::Vector2d(0.4, 0.5), Eigen::Vector2d(0.6, 0.7),
Eigen::Vector2d(0.8, 0.9), Eigen::Vector2d(1.0, 1.1)};

auto& mat = legacy_mesh.materials_["Mat1"];
legacy_mesh.materials_.emplace_back();
legacy_mesh.materials_.front().first = "Mat1";
auto& mat = legacy_mesh.materials_.front().second;
mat.baseColor = mat.baseColor.CreateRGB(1, 1, 1);

core::Dtype float_dtype = core::Float32;
Expand Down Expand Up @@ -497,8 +499,11 @@ TEST_P(TriangleMeshPermuteDevices, ToLegacy) {
Pointwise(FloatEq(), {0.8, 0.9}),
Pointwise(FloatEq(), {1.0, 1.1})}));

EXPECT_TRUE(legacy_mesh.materials_.count("Mat1") > 0);
auto& mat = legacy_mesh.materials_["Mat1"];
auto mat_iterator = std::find_if(
legacy_mesh.materials_.begin(), legacy_mesh.materials_.end(),
[](const auto& pair) -> bool { return pair.first == "Mat1"; });
EXPECT_TRUE(mat_iterator != legacy_mesh.materials_.end());
auto& mat = mat_iterator->second;
EXPECT_TRUE(Eigen::Vector4f(mat.baseColor.f4) ==
Eigen::Vector4f(1, 1, 1, 1));
EXPECT_TRUE(mat.baseMetallic == 0.0);
Expand Down

0 comments on commit 3ddc69b

Please sign in to comment.