diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c6175165..ab362be4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,9 +14,18 @@ jobs: name: Ubuntu Jammy CI steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Compile and test id: ci uses: gazebo-tooling/action-gz-ci@jammy with: codecov-enabled: true + noble-ci: + runs-on: ubuntu-latest + name: Ubuntu Noble CI + steps: + - name: Checkout + uses: actions/checkout@v4 + - name: Compile and test + id: ci + uses: gazebo-tooling/action-gz-ci@noble diff --git a/CMakeLists.txt b/CMakeLists.txt index a25ee1f5..745deacc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 3.10.2 FATAL_ERROR) +cmake_minimum_required(VERSION 3.22.1 FATAL_ERROR) #============================================================================ # Initialize the project diff --git a/examples/CMakeLists.txt b/examples/CMakeLists.txt index ab609f92..04418a8f 100644 --- a/examples/CMakeLists.txt +++ b/examples/CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 3.5 FATAL_ERROR) +cmake_minimum_required(VERSION 3.22.1 FATAL_ERROR) project(gz-common-examples) diff --git a/graphics/include/gz/common/SubMesh.hh b/graphics/include/gz/common/SubMesh.hh index a20e6b99..cfb8f4fe 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 @@ -158,7 +157,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 +223,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; @@ -410,7 +409,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 { diff --git a/graphics/src/SubMesh.cc b/graphics/src/SubMesh.cc index 3318e9ac..050c7d2e 100644 --- a/graphics/src/SubMesh.cc +++ b/graphics/src/SubMesh.cc @@ -18,8 +18,8 @@ #include #include #include -#include #include +#include #include "gz/math/Helpers.hh" @@ -566,10 +566,67 @@ void SubMesh::FillArrays(double **_vertArr, int **_indArr) const } } +namespace { +// Simple way to find neighbors by grouping all vertices +// by X coordinate in (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->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->groups.find(_point.X()); + // find smaller acceptable value + while (it != this->groups.begin()) + { + auto prev = it; + --prev; + if (!gz::math::equal(prev->first, _point.X())) + break; + it = prev; + } + while (it != this->groups.end() + && gz::math::equal(it->first, _point.X())) + { + for (const auto index : it->second) + if (this->vertices[index] == _point) + _v(index); + ++it; + } + } + + // Indexes of vertices grouped by X coordinate + private: std::map> groups; + // Const reference to a vertices vector + private: const std::vector &vertices; +}; +} // namespace + ////////////////////////////////////////////////// void SubMesh::RecalculateNormals() { - if (this->dataPtr->normals.size() < 3u) + if (this->dataPtr->primitiveType != SubMesh::TRIANGLES + || this->dataPtr->indices.empty() + || this->dataPtr->indices.size() % 3u != 0) return; // Reset all the normals @@ -579,6 +636,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) { @@ -590,14 +649,11 @@ 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) + for (const auto &point : {v1, v2, v3}) + neighbors.Visit(point, [&](const unsigned int index) { - this->dataPtr->normals[j] += n; - } - } + this->dataPtr->normals[index] += n; + }); } // Normalize the results diff --git a/graphics/src/SubMesh_TEST.cc b/graphics/src/SubMesh_TEST.cc index 112ca572..b51c1515 100644 --- a/graphics/src/SubMesh_TEST.cc +++ b/graphics/src/SubMesh_TEST.cc @@ -569,3 +569,32 @@ 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 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; + submesh->AddVertex(i-jitter, i, i); + submesh->AddVertex(i+1, i+1, i+1); + submesh->AddVertex(i, i, -static_cast(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 + // because of neighbour vertex + ASSERT_NE(submesh->Normal(0), submesh->Normal(1)); +} diff --git a/graphics/src/VHACD/VHACD.h b/graphics/src/VHACD/VHACD.h index 6daec08d..82c9f5ae 100644 --- a/graphics/src/VHACD/VHACD.h +++ b/graphics/src/VHACD/VHACD.h @@ -113,6 +113,12 @@ #include #include +// Local modification done by @iche033 +// The m_voxelHullCount variable type is changed from +// uint32_t to std::atomic +// to fix data race issue picked up by TSAN +#include + #include #include #include @@ -5965,12 +5971,12 @@ class VoxelHull std::unordered_map m_voxelIndexMap; // Maps from a voxel coordinate space into a vertex index space std::vector m_vertices; std::vector m_indices; - static uint32_t m_voxelHullCount; + static std::atomic m_voxelHullCount; IVHACD::Parameters m_params; VHACDCallbacks* m_callbacks{ nullptr }; }; -uint32_t VoxelHull::m_voxelHullCount = 0; +std::atomic VoxelHull::m_voxelHullCount = 0; VoxelHull::VoxelHull(const VoxelHull& parent, SplitAxis axis, diff --git a/tutorials/profiler.md b/tutorials/profiler.md index 94cfe110..94fd37df 100644 --- a/tutorials/profiler.md +++ b/tutorials/profiler.md @@ -57,7 +57,7 @@ Update your CMakeLists.txt to the following. Note that the profiler must be enabled at compile time in order to function. ```{.cpp} -cmake_minimum_required(VERSION 2.8 FATAL_ERROR) +cmake_minimum_required(VERSION 3.22.1 FATAL_ERROR) # Find the gz-common library find_package(gz-common6 QUIET REQUIRED COMPONENTS profiler)