From 1ca29624e3453ef2e302ca811a64e7eec4e9b48a Mon Sep 17 00:00:00 2001 From: Maksim Derbasov Date: Mon, 13 May 2024 02:48:20 +0900 Subject: [PATCH 01/10] SubMesh::RecalculateNormals improvement Signed-off-by: Maksim Derbasov --- graphics/src/SubMesh.cc | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/graphics/src/SubMesh.cc b/graphics/src/SubMesh.cc index 53726dc0..0c9a2bd8 100644 --- a/graphics/src/SubMesh.cc +++ b/graphics/src/SubMesh.cc @@ -576,7 +576,8 @@ void SubMesh::FillArrays(double **_vertArr, int **_indArr) const ////////////////////////////////////////////////// void SubMesh::RecalculateNormals() { - if (this->dataPtr->normals.size() < 3u) + if (this->dataPtr->indices.size() == 0 + || this->dataPtr->indices.size() % 3u != 0) return; // Reset all the normals @@ -597,14 +598,9 @@ void SubMesh::RecalculateNormals() this->dataPtr->vertices[this->dataPtr->indices[i+2]]; gz::math::Vector3d n = gz::math::Vector3d::Normal(v1, v2, v3); - for (unsigned int j = 0; j < this->dataPtr->vertices.size(); ++j) - { - gz::math::Vector3d v = this->dataPtr->vertices[j]; - if (v == v1 || v == v2 || v == v3) - { - this->dataPtr->normals[j] += n; - } - } + this->dataPtr->normals[this->dataPtr->indices[i]] += n; + this->dataPtr->normals[this->dataPtr->indices[i+1]] += n; + this->dataPtr->normals[this->dataPtr->indices[i+2]] += n; } // Normalize the results From 66c69c7b518699a714266c49932a94825c1997d0 Mon Sep 17 00:00:00 2001 From: Maksim Derbasov Date: Mon, 20 May 2024 22:57:55 +0900 Subject: [PATCH 02/10] Added test, reverted lib code Signed-off-by: Maksim Derbasov --- graphics/src/SubMesh.cc | 21 +++++++++++++++++++++ graphics/src/SubMesh_TEST.cc | 24 ++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/graphics/src/SubMesh.cc b/graphics/src/SubMesh.cc index 0c9a2bd8..532934a5 100644 --- a/graphics/src/SubMesh.cc +++ b/graphics/src/SubMesh.cc @@ -598,9 +598,30 @@ void SubMesh::RecalculateNormals() this->dataPtr->vertices[this->dataPtr->indices[i+2]]; gz::math::Vector3d n = gz::math::Vector3d::Normal(v1, v2, v3); +#if 1 + /* + i7-11800H @ Ubuntu22 VM; 2^14 triangles + SubMeshTest.NormalsRecalculation (1105 ms) + SubMeshTest.NormalsRecalculation (1158 ms) + SubMeshTest.NormalsRecalculation (1117 ms) + SubMeshTest.NormalsRecalculation (1137 ms) + SubMeshTest.NormalsRecalculation (1156 ms) + */ + for (unsigned int j = 0; j < this->dataPtr->vertices.size(); ++j) + { + gz::math::Vector3d v = this->dataPtr->vertices[j]; + if (v == v1 || v == v2 || v == v3) + { + this->dataPtr->normals[j] += n; + } + } +#else + // same env, ~1ms + // FAILS: ASSERT_NE(submesh->Normal(0), submesh->Normal(1)); this->dataPtr->normals[this->dataPtr->indices[i]] += n; this->dataPtr->normals[this->dataPtr->indices[i+1]] += n; this->dataPtr->normals[this->dataPtr->indices[i+2]] += n; +#endif } // Normalize the results diff --git a/graphics/src/SubMesh_TEST.cc b/graphics/src/SubMesh_TEST.cc index 1d332f91..c00e1255 100644 --- a/graphics/src/SubMesh_TEST.cc +++ b/graphics/src/SubMesh_TEST.cc @@ -585,3 +585,27 @@ TEST_F(SubMeshTest, Volume) boxSub.AddIndex(1); EXPECT_DOUBLE_EQ(0.0, boxSub.Volume()); } + +///////////////////////////////////////////////// +TEST_F(SubMeshTest, NormalsRecalculation) +{ + auto submesh = std::make_shared(); + submesh->SetPrimitiveType(common::SubMesh::TRIANGLES); + + constexpr unsigned int elements = 65536/4; + for (unsigned int i = 0; i < elements; ++i) { + submesh->AddVertex(i, i, i); + submesh->AddVertex(i+1, i+1, i+1); + submesh->AddVertex(i, i, -i); + + submesh->AddIndex(3*i); + submesh->AddIndex(3*i+1); + submesh->AddIndex(3*i+2); + } + + ASSERT_EQ(submesh->IndexCount() % 3, 0u); + submesh->RecalculateNormals(); + ASSERT_EQ(submesh->NormalCount(), submesh->VertexCount()); + // Same triangle, but different normals + ASSERT_NE(submesh->Normal(0), submesh->Normal(1)); +} From 3882da119886ede5f529b2a5a87bff6658b4d9be Mon Sep 17 00:00:00 2001 From: Maksim Derbasov Date: Mon, 20 May 2024 23:17:01 +0900 Subject: [PATCH 03/10] Typos fix Signed-off-by: Maksim Derbasov --- graphics/include/gz/common/SubMesh.hh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/graphics/include/gz/common/SubMesh.hh b/graphics/include/gz/common/SubMesh.hh index 938a93d2..e8e87de3 100644 --- a/graphics/include/gz/common/SubMesh.hh +++ b/graphics/include/gz/common/SubMesh.hh @@ -158,7 +158,7 @@ namespace gz public: gz::math::Vector3d Vertex(const unsigned int _index) const; /// \brief Get the raw vertex pointer. This is unsafe, it is the - /// caller's responsability to ensure it's not indexed out of bounds. + /// caller's responsibility to ensure it's not indexed out of bounds. /// The valid range is [0; VertexCount()) /// \return Raw vertices public: const gz::math::Vector3d* VertexPtr() const; @@ -224,7 +224,7 @@ namespace gz public: int Index(const unsigned int _index) const; /// \brief Get the raw index pointer. This is unsafe, it is the - /// caller's responsability to ensure it's not indexed out of bounds. + /// caller's responsibility to ensure it's not indexed out of bounds. /// The valid range is [0; IndexCount()) /// \return Raw indices public: const unsigned int* IndexPtr() const; @@ -416,7 +416,7 @@ namespace gz GZ_UTILS_IMPL_PTR(dataPtr) }; - /// \brief Vertex to node weighted assignement for skeleton animation + /// \brief Vertex to node weighted assignment for skeleton animation /// visualization class GZ_COMMON_GRAPHICS_VISIBLE NodeAssignment { From 3ea5134896ffeaef2c6f3083cec4a87d7b27cf27 Mon Sep 17 00:00:00 2001 From: Maksim Derbasov Date: Sat, 20 Jul 2024 19:46:34 +0900 Subject: [PATCH 04/10] Group vertexes by X Signed-off-by: Maksim Derbasov --- graphics/src/SubMesh.cc | 78 ++++++++++++++++++++++++++++++++++-- graphics/src/SubMesh_TEST.cc | 9 ++++- 2 files changed, 81 insertions(+), 6 deletions(-) diff --git a/graphics/src/SubMesh.cc b/graphics/src/SubMesh.cc index 532934a5..8382e6d7 100644 --- a/graphics/src/SubMesh.cc +++ b/graphics/src/SubMesh.cc @@ -20,6 +20,7 @@ #include #include #include +#include #include "gz/math/Helpers.hh" @@ -573,10 +574,53 @@ void SubMesh::FillArrays(double **_vertArr, int **_indArr) const } } +namespace { +// Simple way to finding neighbors by grouping all vertexes +// by X coordinate with (ordered) map. KD-tree maybe better +// but not sure about construction overhead +struct Neighbors { + Neighbors(const std::vector &_indices, + const std::vector &_vertices) + : vertices(_vertices) + { + for (unsigned int i = 0; i < _indices.size(); ++i) + { + const auto index = _indices[i]; + this->neighbors[_vertices[index].X()].push_back(index); + } + } + + template + void Visit(const gz::math::Vector3d &_point, Visitor _v) + { + auto it = this->neighbors.find(_point.X()); + // find smaller acceptable value + while (it != this->neighbors.begin()) + { + auto prev = it; + --prev; + if (!gz::math::equal(prev->first, _point.X())) + break; + it = prev; + } + while (gz::math::equal(it->first, _point.X())) + { + for (const auto index : it->second) + if (this->vertices[index] == _point) + _v(index); + ++it; + } + } + + private: std::map> neighbors; + private: const std::vector &vertices; +}; +} // namespace + ////////////////////////////////////////////////// void SubMesh::RecalculateNormals() { - if (this->dataPtr->indices.size() == 0 + if (this->dataPtr->indices.empty() || this->dataPtr->indices.size() % 3u != 0) return; @@ -587,6 +631,8 @@ void SubMesh::RecalculateNormals() if (this->dataPtr->normals.size() != this->dataPtr->vertices.size()) this->dataPtr->normals.resize(this->dataPtr->vertices.size()); + Neighbors neighbors(this->dataPtr->indices, this->dataPtr->vertices); + // For each face, which is defined by three indices, calculate the normals for (unsigned int i = 0; i < this->dataPtr->indices.size(); i+= 3) { @@ -598,7 +644,7 @@ void SubMesh::RecalculateNormals() this->dataPtr->vertices[this->dataPtr->indices[i+2]]; gz::math::Vector3d n = gz::math::Vector3d::Normal(v1, v2, v3); -#if 1 +#if 0 /* i7-11800H @ Ubuntu22 VM; 2^14 triangles SubMeshTest.NormalsRecalculation (1105 ms) @@ -618,9 +664,33 @@ void SubMesh::RecalculateNormals() #else // same env, ~1ms // FAILS: ASSERT_NE(submesh->Normal(0), submesh->Normal(1)); - this->dataPtr->normals[this->dataPtr->indices[i]] += n; + /*this->dataPtr->normals[this->dataPtr->indices[i]] += n; this->dataPtr->normals[this->dataPtr->indices[i+1]] += n; - this->dataPtr->normals[this->dataPtr->indices[i+2]] += n; + this->dataPtr->normals[this->dataPtr->indices[i+2]] += n;*/ + + for (const auto& point : {v1, v2, v3}) + neighbors.Visit(point, [&](unsigned int index) + { + this->dataPtr->normals[index] += n; + }); + + //auto& nv1 = neighbors[v1]; + //nv1.insert(this->dataPtr->indices[i]); + /*for (const auto index: neighbors[v1]) + this->dataPtr->normals[index] += n;*/ + + //neighbors[v2].insert(this->dataPtr->indices[i+1]); + //auto& nv2 = neighbors[v2]; + //nv2.insert(this->dataPtr->indices[i+1]); + /*for (const auto index: neighbors[v2]) + this->dataPtr->normals[index] += n;*/ + + //neighbors[v3].insert(this->dataPtr->indices[i+2]); + //auto& nv3 = neighbors[v3]; + //nv3.insert(this->dataPtr->indices[i+2]); + /*for (const auto index: neighbors[v3]) + this->dataPtr->normals[index] += n;*/ + #endif } diff --git a/graphics/src/SubMesh_TEST.cc b/graphics/src/SubMesh_TEST.cc index c00e1255..8f299bab 100644 --- a/graphics/src/SubMesh_TEST.cc +++ b/graphics/src/SubMesh_TEST.cc @@ -592,9 +592,13 @@ TEST_F(SubMeshTest, NormalsRecalculation) auto submesh = std::make_shared(); submesh->SetPrimitiveType(common::SubMesh::TRIANGLES); - constexpr unsigned int elements = 65536/4; + constexpr unsigned int elements = 16384; for (unsigned int i = 0; i < elements; ++i) { - submesh->AddVertex(i, i, i); + // sub to X less than _epsilon from even elements + // expect that the 2nd vertex should be matched with + // the 1st of next triangle + const auto jitter = i % 2 ? 1e-7 : 0.0; + submesh->AddVertex(i-jitter, i, i); submesh->AddVertex(i+1, i+1, i+1); submesh->AddVertex(i, i, -i); @@ -607,5 +611,6 @@ TEST_F(SubMeshTest, NormalsRecalculation) submesh->RecalculateNormals(); ASSERT_EQ(submesh->NormalCount(), submesh->VertexCount()); // Same triangle, but different normals + // because of neighbour vertex ASSERT_NE(submesh->Normal(0), submesh->Normal(1)); } From e22e6ffc221637ca00ec5e25446473d5d6b738b0 Mon Sep 17 00:00:00 2001 From: Maksim Derbasov Date: Sat, 20 Jul 2024 19:50:50 +0900 Subject: [PATCH 05/10] Clean-up Signed-off-by: Maksim Derbasov --- graphics/src/SubMesh.cc | 45 +---------------------------------------- 1 file changed, 1 insertion(+), 44 deletions(-) diff --git a/graphics/src/SubMesh.cc b/graphics/src/SubMesh.cc index 8382e6d7..697cdc4f 100644 --- a/graphics/src/SubMesh.cc +++ b/graphics/src/SubMesh.cc @@ -644,54 +644,11 @@ void SubMesh::RecalculateNormals() this->dataPtr->vertices[this->dataPtr->indices[i+2]]; gz::math::Vector3d n = gz::math::Vector3d::Normal(v1, v2, v3); -#if 0 - /* - i7-11800H @ Ubuntu22 VM; 2^14 triangles - SubMeshTest.NormalsRecalculation (1105 ms) - SubMeshTest.NormalsRecalculation (1158 ms) - SubMeshTest.NormalsRecalculation (1117 ms) - SubMeshTest.NormalsRecalculation (1137 ms) - SubMeshTest.NormalsRecalculation (1156 ms) - */ - for (unsigned int j = 0; j < this->dataPtr->vertices.size(); ++j) - { - gz::math::Vector3d v = this->dataPtr->vertices[j]; - if (v == v1 || v == v2 || v == v3) - { - this->dataPtr->normals[j] += n; - } - } -#else - // same env, ~1ms - // FAILS: ASSERT_NE(submesh->Normal(0), submesh->Normal(1)); - /*this->dataPtr->normals[this->dataPtr->indices[i]] += n; - this->dataPtr->normals[this->dataPtr->indices[i+1]] += n; - this->dataPtr->normals[this->dataPtr->indices[i+2]] += n;*/ - - for (const auto& point : {v1, v2, v3}) + for (const auto &point : {v1, v2, v3}) neighbors.Visit(point, [&](unsigned int index) { this->dataPtr->normals[index] += n; }); - - //auto& nv1 = neighbors[v1]; - //nv1.insert(this->dataPtr->indices[i]); - /*for (const auto index: neighbors[v1]) - this->dataPtr->normals[index] += n;*/ - - //neighbors[v2].insert(this->dataPtr->indices[i+1]); - //auto& nv2 = neighbors[v2]; - //nv2.insert(this->dataPtr->indices[i+1]); - /*for (const auto index: neighbors[v2]) - this->dataPtr->normals[index] += n;*/ - - //neighbors[v3].insert(this->dataPtr->indices[i+2]); - //auto& nv3 = neighbors[v3]; - //nv3.insert(this->dataPtr->indices[i+2]); - /*for (const auto index: neighbors[v3]) - this->dataPtr->normals[index] += n;*/ - -#endif } // Normalize the results From b0605b805946d6cec4f597cd8fa294c0cbda08e1 Mon Sep 17 00:00:00 2001 From: Maksim Derbasov Date: Sat, 20 Jul 2024 23:55:15 +0900 Subject: [PATCH 06/10] Update comment in graphics/src/SubMesh.cc Co-authored-by: Dmitry Skorobogaty <72853514+skorobogatydmitry@users.noreply.github.com> Signed-off-by: Maksim Derbasov --- graphics/src/SubMesh.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graphics/src/SubMesh.cc b/graphics/src/SubMesh.cc index 697cdc4f..71c69437 100644 --- a/graphics/src/SubMesh.cc +++ b/graphics/src/SubMesh.cc @@ -575,7 +575,7 @@ void SubMesh::FillArrays(double **_vertArr, int **_indArr) const } namespace { -// Simple way to finding neighbors by grouping all vertexes +// Simple way to find neighbors by grouping all vertices // by X coordinate with (ordered) map. KD-tree maybe better // but not sure about construction overhead struct Neighbors { From 0ba046adf3ef61940a13190c273756f8069c9ffb Mon Sep 17 00:00:00 2001 From: Maksim Derbasov Date: Sun, 21 Jul 2024 03:23:00 +0900 Subject: [PATCH 07/10] Const tweak Signed-off-by: Maksim Derbasov --- graphics/include/gz/common/SubMesh.hh | 1 - graphics/src/SubMesh.cc | 8 ++++---- graphics/src/SubMesh_TEST.cc | 6 +++--- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/graphics/include/gz/common/SubMesh.hh b/graphics/include/gz/common/SubMesh.hh index e8e87de3..3915997f 100644 --- a/graphics/include/gz/common/SubMesh.hh +++ b/graphics/include/gz/common/SubMesh.hh @@ -20,7 +20,6 @@ #include #include #include -#include #include #include diff --git a/graphics/src/SubMesh.cc b/graphics/src/SubMesh.cc index 71c69437..5f50f712 100644 --- a/graphics/src/SubMesh.cc +++ b/graphics/src/SubMesh.cc @@ -18,7 +18,6 @@ #include #include #include -#include #include #include @@ -578,7 +577,8 @@ namespace { // Simple way to find neighbors by grouping all vertices // by X coordinate with (ordered) map. KD-tree maybe better // but not sure about construction overhead -struct Neighbors { +struct Neighbors +{ Neighbors(const std::vector &_indices, const std::vector &_vertices) : vertices(_vertices) @@ -591,7 +591,7 @@ struct Neighbors { } template - void Visit(const gz::math::Vector3d &_point, Visitor _v) + void Visit(const gz::math::Vector3d &_point, Visitor _v) const { auto it = this->neighbors.find(_point.X()); // find smaller acceptable value @@ -645,7 +645,7 @@ void SubMesh::RecalculateNormals() gz::math::Vector3d n = gz::math::Vector3d::Normal(v1, v2, v3); for (const auto &point : {v1, v2, v3}) - neighbors.Visit(point, [&](unsigned int index) + neighbors.Visit(point, [&](const unsigned int index) { this->dataPtr->normals[index] += n; }); diff --git a/graphics/src/SubMesh_TEST.cc b/graphics/src/SubMesh_TEST.cc index 8f299bab..9da91603 100644 --- a/graphics/src/SubMesh_TEST.cc +++ b/graphics/src/SubMesh_TEST.cc @@ -592,9 +592,9 @@ TEST_F(SubMeshTest, NormalsRecalculation) auto submesh = std::make_shared(); submesh->SetPrimitiveType(common::SubMesh::TRIANGLES); - constexpr unsigned int elements = 16384; - for (unsigned int i = 0; i < elements; ++i) { - // sub to X less than _epsilon from even elements + constexpr unsigned int triangles = 16384; + for (unsigned int i = 0; i < triangles; ++i) { + // sub to X less than _epsilon from even triangles // expect that the 2nd vertex should be matched with // the 1st of next triangle const auto jitter = i % 2 ? 1e-7 : 0.0; From fe8b0081c2130076b91d920532180ec0f18867d9 Mon Sep 17 00:00:00 2001 From: Maksim Derbasov Date: Tue, 23 Jul 2024 10:22:35 +0900 Subject: [PATCH 08/10] Fixing out of border access Signed-off-by: Maksim Derbasov --- graphics/src/SubMesh.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/graphics/src/SubMesh.cc b/graphics/src/SubMesh.cc index 5f50f712..21413a1a 100644 --- a/graphics/src/SubMesh.cc +++ b/graphics/src/SubMesh.cc @@ -603,7 +603,8 @@ struct Neighbors break; it = prev; } - while (gz::math::equal(it->first, _point.X())) + while (it != this->neighbors.end() + && gz::math::equal(it->first, _point.X())) { for (const auto index : it->second) if (this->vertices[index] == _point) From 833962ce19bfd6e8da990a1f6b3a2c6b6f4b4699 Mon Sep 17 00:00:00 2001 From: Maksim Derbasov Date: Tue, 23 Jul 2024 10:44:51 +0900 Subject: [PATCH 09/10] Fix warning in test Signed-off-by: Maksim Derbasov --- graphics/src/SubMesh_TEST.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graphics/src/SubMesh_TEST.cc b/graphics/src/SubMesh_TEST.cc index 9da91603..eb07afed 100644 --- a/graphics/src/SubMesh_TEST.cc +++ b/graphics/src/SubMesh_TEST.cc @@ -600,7 +600,7 @@ TEST_F(SubMeshTest, NormalsRecalculation) const auto jitter = i % 2 ? 1e-7 : 0.0; submesh->AddVertex(i-jitter, i, i); submesh->AddVertex(i+1, i+1, i+1); - submesh->AddVertex(i, i, -i); + submesh->AddVertex(i, i, -static_cast(i)); submesh->AddIndex(3*i); submesh->AddIndex(3*i+1); From cd01fa66ccf499bd519b0edc3da8d2bcb6981c18 Mon Sep 17 00:00:00 2001 From: Maksim Derbasov Date: Thu, 25 Jul 2024 01:28:31 +0900 Subject: [PATCH 10/10] :Iteration Signed-off-by: Maksim Derbasov --- graphics/src/SubMesh.cc | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/graphics/src/SubMesh.cc b/graphics/src/SubMesh.cc index 21413a1a..23fc8bad 100644 --- a/graphics/src/SubMesh.cc +++ b/graphics/src/SubMesh.cc @@ -575,7 +575,7 @@ void SubMesh::FillArrays(double **_vertArr, int **_indArr) const namespace { // Simple way to find neighbors by grouping all vertices -// by X coordinate with (ordered) map. KD-tree maybe better +// by X coordinate in (ordered) map. KD-tree maybe better // but not sure about construction overhead struct Neighbors { @@ -586,16 +586,24 @@ struct Neighbors for (unsigned int i = 0; i < _indices.size(); ++i) { const auto index = _indices[i]; - this->neighbors[_vertices[index].X()].push_back(index); + this->groups[_vertices[index].X()].push_back(index); } } + // When we have a concrete point to check, we are looking for + // a group inside a map with a same X. + // Then we check neighbors with the smaller X until + // it's in tolerance of the math::equal function. + // Starting from smallest X, which is in a tolerance range, + // testing all points in group for equality. In case of equality, + // call a Visitor with element index as an argument. + // Continue until a greater side of X tolerance range reached. template void Visit(const gz::math::Vector3d &_point, Visitor _v) const { - auto it = this->neighbors.find(_point.X()); + auto it = this->groups.find(_point.X()); // find smaller acceptable value - while (it != this->neighbors.begin()) + while (it != this->groups.begin()) { auto prev = it; --prev; @@ -603,7 +611,7 @@ struct Neighbors break; it = prev; } - while (it != this->neighbors.end() + while (it != this->groups.end() && gz::math::equal(it->first, _point.X())) { for (const auto index : it->second) @@ -613,7 +621,9 @@ struct Neighbors } } - private: std::map> neighbors; + // Indexes of vertices grouped by X coordinate + private: std::map> groups; + // Const reference to a vertices vector private: const std::vector &vertices; }; } // namespace @@ -621,7 +631,8 @@ struct Neighbors ////////////////////////////////////////////////// void SubMesh::RecalculateNormals() { - if (this->dataPtr->indices.empty() + if (this->dataPtr->primitiveType != SubMesh::TRIANGLES + || this->dataPtr->indices.empty() || this->dataPtr->indices.size() % 3u != 0) return;