Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RemoveDuplicatedTriangles feature using tensor API #6409

Closed
wants to merge 5 commits into from
Closed

Add RemoveDuplicatedTriangles feature using tensor API #6409

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
98fe062
Add RemoveDuplicatedTriangles feature using tensor API
PierreIntel Oct 5, 2023
9c53c9d
Add C++ and python documentation
PierreIntel Oct 5, 2023
a6ed51c
Change check of numbers of triangles to 2 and return if the number of…
PierreIntel Oct 12, 2023
3aa93dd
Use the SelectFacesByMask() function to remove duplicates triangles w…
PierreIntel Oct 12, 2023
c890dc8
use the operator[] and sort instead of multiples if
PierreIntel Oct 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 92 additions & 0 deletions cpp/open3d/t/geometry/TriangleMesh.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,98 @@ TriangleMesh &TriangleMesh::Rotate(const core::Tensor &R,
return *this;
}

TriangleMesh &TriangleMesh::RemoveDuplicatedTriangles() {
core::Tensor triangles = GetTriangleIndices();
int64_t num_triangles = triangles.GetLength();
if (num_triangles < 1) {
utility::LogError("Mesh must have at least two triangles.");
}
PierreIntel marked this conversation as resolved.
Show resolved Hide resolved

// unordered_map to keep track of existing triangles
// hash function with code::Tensor not implemented so we use std::tuple
typedef std::tuple<int, int, int> Index3;
std::unordered_map<Index3, size_t, utility::hash_tuple<Index3>>
triangle_to_old_index;

// Check is mesh has normals triangles
bool has_tri_normal = HasTriangleNormals();
// index of triangle to keep (unique triangle)
std::vector<int64_t> index_to_keep;
int64_t new_size = 0;
for (int64_t i = 0; i < num_triangles; i++) {
Index3 triangle_in_tuple;
// Extract vertices of the triangle
core::Tensor triangle_to_check =
triangles.GetItem({core::TensorKey::Index(i)});
int64_t vertice_0 =
triangle_to_check.GetItem({core::TensorKey::Index(0)})
.Item<int64_t>();
int64_t vertice_1 =
triangle_to_check.GetItem({core::TensorKey::Index(1)})
.Item<int64_t>();
int64_t vertice_2 =
triangle_to_check.GetItem({core::TensorKey::Index(2)})
.Item<int64_t>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to ensure this works for different dtypes for vertices (specifically int32_t and int64_t). This will output junk values for int32_t vertices.

minor:

  • operator[] is shorter: triangle_to_check[0].Item<dtype>()
  • Would using std::rotate to reorder elements in the tuple be cleaner than the multiple ifs ?
  • Can you comment on parallelization here? Is it possible and would it provide a useful speedup?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::rotate can be used but it will need multiple ifs too to know which vertices to rotate. Example {2,1,0} will need 2 rotations to get {0,1,2}. What can be used is instantiate a std::vector<int64_t> of 3 elements, sort it with std::sort and next initialize the tuple for the unordered map.
Also elements of triangle_vec are int64_t like that it can get vertices in int32_t or int64_t.

        std::vector<int64_t> triangle_vec;
        if (triangles.GetDtype() == core::Int32)
        {
            auto vertice_0 = triangle_to_check[0].Item<int32_t>();
            auto vertice_1 = triangle_to_check[1].Item<int32_t>();
            auto vertice_2 = triangle_to_check[2].Item<int32_t>();
            triangle_vec = {vertice_0, vertice_1, vertice_2};
        }
        else if (triangles.GetDtype() == core::Int64)
        {
            auto vertice_0 = triangle_to_check[0].Item<int64_t>();
            auto vertice_1 = triangle_to_check[1].Item<int64_t>();
            auto vertice_2 = triangle_to_check[2].Item<int64_t>();
            triangle_vec = {vertice_0, vertice_1, vertice_2};
        }

        // We first need to find the minimum index. Because triangle (0-1-2)
        // and triangle (2-0-1) are the same.
        std::sort(triangle_vec.begin(), triangle_vec.end());
        triangle_in_tuple =
                        std::make_tuple(triangle_vec[0], triangle_vec[1], triangle_vec[2]);

The parallelization is possible but we need to be sure that the access to the unordered map is in a critical section to avoid multiple accesses adding the duplicate triangle in the unordered map.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in c890dc8

// We first need to find the minimum index. Because triangle (0-1-2)
// and triangle (2-0-1) are the same.
if (vertice_0 <= vertice_1) {
if (vertice_1 <= vertice_2)
triangle_in_tuple =
std::make_tuple(vertice_0, vertice_1, vertice_2);
else
triangle_in_tuple =
std::make_tuple(vertice_0, vertice_2, vertice_1);
} else {
if (vertice_1 <= vertice_2)
triangle_in_tuple =
std::make_tuple(vertice_1, vertice_2, vertice_0);
else
triangle_in_tuple =
std::make_tuple(vertice_1, vertice_0, vertice_2);
}
// check is triangle is already present in the mesh
if (triangle_to_old_index.find(triangle_in_tuple) ==
triangle_to_old_index.end()) {
triangle_to_old_index[triangle_in_tuple] = i;
index_to_keep.push_back(i);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would using a boolean mask of triangles to keep, instead of a vector of indices be faster? Tensors support indexing with a Boolean mask, similar to numpy. See SelectFacesByMask().

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it is possible to use SelectFacesByMask() but this function doesn't select Normals Triangles faces ?
So if I need to loop over the normals triangles I can keep the actual loop instead of have both SleectFacesByMask() for triangles and the loop for normals triangles.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I succeed to manage it in 3aa93dd

new_size++;
}
}

// do this only if there are some triangle to remove
if (new_size != num_triangles) {
core::Tensor triangles_without_dup = core::Tensor::Empty(
{new_size, 3}, triangles.GetDtype(), GetDevice());

core::Tensor triangles_normals, triangles_normals_without_dup;

// fill only is there are normals triangle to avoid errors
if (has_tri_normal) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you support other triangle attributes? (e.g. triangle colors, etc.) An important feature of the Tensor API data structures is support for generic triangle and vertex attributes.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes the triangle colors are manage in this : 3aa93dd

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to support arbitrary user defined attributes. Examples are labels, covariance, opacity, etc. See SelectFacesByMask for how to support arbitrary attributes.

triangles_normals = GetTriangleNormals();
triangles_normals_without_dup = core::Tensor::Empty(
{new_size, 3}, triangles.GetDtype(), GetDevice());
}

for (int64_t i = 0; i < new_size; i++) {
core::Tensor triangle_to_keep = triangles.GetItem(
{core::TensorKey::Index(index_to_keep[i])});
triangles_without_dup.SetItem({core::TensorKey::Index(i)},
triangle_to_keep);
if (has_tri_normal) {
core::Tensor triangle_normal_to_keep =
triangles_normals.GetItem(
{core::TensorKey::Index(index_to_keep[i])});
triangles_normals_without_dup.SetItem(
{core::TensorKey::Index(i)}, triangle_normal_to_keep);
}
}

SetTriangleIndices(triangles_without_dup);
if (has_tri_normal) SetTriangleNormals(triangles_normals_without_dup);
}
return *this;
}

TriangleMesh &TriangleMesh::NormalizeNormals() {
if (HasVertexNormals()) {
SetVertexNormals(GetVertexNormals().Contiguous());
Expand Down
4 changes: 4 additions & 0 deletions cpp/open3d/t/geometry/TriangleMesh.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,10 @@ class TriangleMesh : public Geometry, public DrawableGeometry {
/// \return Rotated TriangleMesh
TriangleMesh &Rotate(const core::Tensor &R, const core::Tensor &center);

/// \brief Remove duplicated triangles from the TriangleMesh
/// \return TriangleMesh without duplicated triangles
TriangleMesh &RemoveDuplicatedTriangles();

/// Normalize both triangle normals and vertex normals to length 1.
TriangleMesh &NormalizeNormals();

Expand Down
3 changes: 3 additions & 0 deletions cpp/pybind/t/geometry/trianglemesh.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,9 @@ The attributes of the triangle mesh have different levels::
"Scale points.");
triangle_mesh.def("rotate", &TriangleMesh::Rotate, "R"_a, "center"_a,
"Rotate points and normals (if exist).");
triangle_mesh.def("remove_duplicated_triangles",
&TriangleMesh::RemoveDuplicatedTriangles,
"Remove duplicated triangles from a triangle");

triangle_mesh.def(
"normalize_normals", &TriangleMesh::NormalizeNormals,
Expand Down
50 changes: 50 additions & 0 deletions cpp/tests/t/geometry/TriangleMesh.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,56 @@ TEST_P(TriangleMeshPermuteDevices, Rotate) {
core::Tensor::Init<float>({{2, 2, 1}, {2, 2, 1}}, device)));
}

TEST_P(TriangleMeshPermuteDevices, RemoveDuplicatedTriangles) {
core::Device device = GetParam();

t::geometry::TriangleMesh mesh(device);

mesh.SetVertexPositions(core::Tensor::Init<float>({{0, 0, 0},
{1, 0, 0},
{0, 0, 1},
{1, 0, 1},
{0, 1, 0},
{1, 1, 0},
{0, 1, 1},
{1, 1, 1}},
device));

mesh.SetTriangleIndices(core::Tensor::Init<int64_t>({{4, 7, 5},
{4, 6, 7},
{0, 2, 4},
{2, 6, 4},
{0, 1, 2},
{1, 3, 2},
{1, 5, 7},
{1, 7, 3},
{2, 3, 7},
{2, 7, 6},
{4, 6, 7},
{0, 4, 1},
{1, 4, 5}},
device));

mesh.RemoveDuplicatedTriangles();

EXPECT_EQ(mesh.GetTriangleIndices().GetLength(), 12);

EXPECT_TRUE(mesh.GetTriangleIndices().AllClose(
core::Tensor::Init<int64_t>({{4, 7, 5},
{4, 6, 7},
{0, 2, 4},
{2, 6, 4},
{0, 1, 2},
{1, 3, 2},
{1, 5, 7},
{1, 7, 3},
{2, 3, 7},
{2, 7, 6},
{0, 4, 1},
{1, 4, 5}},
device)));
}

TEST_P(TriangleMeshPermuteDevices, NormalizeNormals) {
core::Device device = GetParam();

Expand Down
26 changes: 26 additions & 0 deletions python/test/t/geometry/test_trianglemesh.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,3 +417,29 @@ def test_pickle(device):
mesh.vertex.positions.cpu().numpy())
np.testing.assert_equal(mesh_load.triangle.indices.cpu().numpy(),
mesh.triangle.indices.cpu().numpy())


@pytest.mark.parametrize("device", list_devices())
def test_remove_duplicated_triangles(device):

mesh = o3d.t.geometry.TriangleMesh()
vertex_positions = o3c.Tensor(
[[0.0, 0.0, 0.0], [1.0, 0.0, 0.0], [0.0, 0.0, 1.0], [1.0, 0.0, 1.0],
[0.0, 1.0, 0.0], [1.0, 1.0, 0.0], [0.0, 1.0, 1.0], [1.0, 1.0, 1.0]],
o3c.float32, device)
triangle_indices = o3c.Tensor(
[[4, 7, 5], [4, 6, 7], [0, 2, 4], [2, 6, 4], [0, 1, 2], [1, 3, 2],
[1, 5, 7], [1, 7, 3], [2, 3, 7], [2, 7, 6], [4, 6, 7], [0, 4, 1],
[1, 4, 5]], o3c.int64, device)
mesh.vertex.positions = vertex_positions
mesh.triangle.indices = triangle_indices

mesh.remove_duplicated_triangles()

assert mesh.triangle.indices.shape == (12, 3)
np.testing.assert_allclose(
mesh.triangle.indices.cpu().numpy(),
o3c.Tensor(
[[4, 7, 5], [4, 6, 7], [0, 2, 4], [2, 6, 4], [0, 1, 2], [1, 3, 2],
[1, 5, 7], [1, 7, 3], [2, 3, 7], [2, 7, 6], [0, 4, 1], [1, 4, 5]],
o3c.int64, device).cpu().numpy())