From 784055520d09620a75f0a3deab5b1e3a6796f77f Mon Sep 17 00:00:00 2001 From: Nicola Loi Date: Sat, 26 Jul 2025 15:27:34 +0200 Subject: [PATCH 1/4] Fix thread safety of UniformTSDFVolume::ExtractVoxelGrid --- CHANGELOG.md | 1 + .../integration/UniformTSDFVolume.cpp | 47 +++++++++++++------ 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff978597cd5..203e2bc5c22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,7 @@ - Exposed `get_plotly_fig` and modified `draw_plotly` to return the `Figure` it creates. (PR #7258) - Fix build with librealsense v2.44.0 and upcoming VS 2022 17.13 (PR #7074) - Fix `deprecated-declarations` warnings when compiling code with C++20 standard (PR #7303) +- Fix thread safety of UniformTSDFVolume::ExtractVoxelGrid (PR #7314) ## 0.13 diff --git a/cpp/open3d/pipelines/integration/UniformTSDFVolume.cpp b/cpp/open3d/pipelines/integration/UniformTSDFVolume.cpp index 336572d9203..dcda5d80f04 100644 --- a/cpp/open3d/pipelines/integration/UniformTSDFVolume.cpp +++ b/cpp/open3d/pipelines/integration/UniformTSDFVolume.cpp @@ -257,28 +257,45 @@ std::shared_ptr UniformTSDFVolume::ExtractVoxelGrid() voxel_grid->voxel_size_ = voxel_length_; voxel_grid->origin_ = origin_; + // Create a vector to hold voxels for each thread in the parallel region, + // since access to voxel_grid->voxels_ (std::unordered_map) is not + // thread-safe. + std::vector> per_thread_voxels; + +#pragma omp parallel num_threads(utility::EstimateMaxThreads()) + { +#pragma omp single + { per_thread_voxels.resize(omp_get_num_threads()); } + int thread_id = omp_get_thread_num(); + #ifdef _WIN32 -#pragma omp parallel for schedule(static) \ - num_threads(utility::EstimateMaxThreads()) +#pragma omp for schedule(static) #else -#pragma omp parallel for collapse(2) schedule(static) \ - num_threads(utility::EstimateMaxThreads()) +#pragma omp for collapse(2) schedule(static) #endif - for (int x = 0; x < resolution_; x++) { - for (int y = 0; y < resolution_; y++) { - for (int z = 0; z < resolution_; z++) { - const int ind = IndexOf(x, y, z); - const float w = voxels_[ind].weight_; - const float f = voxels_[ind].tsdf_; - if (w != 0.0f && f < 0.98f && f >= -0.98f) { - double c = (f + 1.0) * 0.5; - Eigen::Vector3d color = Eigen::Vector3d(c, c, c); - Eigen::Vector3i index = Eigen::Vector3i(x, y, z); - voxel_grid->voxels_[index] = geometry::Voxel(index, color); + for (int x = 0; x < resolution_; x++) { + for (int y = 0; y < resolution_; y++) { + for (int z = 0; z < resolution_; z++) { + const int ind = IndexOf(x, y, z); + const float w = voxels_[ind].weight_; + const float f = voxels_[ind].tsdf_; + if (w != 0.0f && f < 0.98f && f >= -0.98f) { + double c = (f + 1.0) * 0.5; + Eigen::Vector3d color(c, c, c); + Eigen::Vector3i index(x, y, z); + per_thread_voxels[thread_id].emplace_back(index, color); + } } } } } + + for (const auto &thread_vector : per_thread_voxels) { + for (const auto &voxel : thread_vector) { + voxel_grid->voxels_[voxel.grid_index_] = voxel; + } + } + return voxel_grid; } From 6958dfd5cb91a67abf5c1e17995ae0d0d567e5b6 Mon Sep 17 00:00:00 2001 From: Nicola Loi Date: Sat, 26 Jul 2025 16:35:07 +0200 Subject: [PATCH 2/4] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 203e2bc5c22..64c9090b092 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,7 +63,7 @@ - Exposed `get_plotly_fig` and modified `draw_plotly` to return the `Figure` it creates. (PR #7258) - Fix build with librealsense v2.44.0 and upcoming VS 2022 17.13 (PR #7074) - Fix `deprecated-declarations` warnings when compiling code with C++20 standard (PR #7303) -- Fix thread safety of UniformTSDFVolume::ExtractVoxelGrid (PR #7314) +- Fix thread safety of UniformTSDFVolume::ExtractVoxelGrid (PR #7315) ## 0.13 From 7958240a4c69fdfa12d9fe0d4f3995ff898b6ea0 Mon Sep 17 00:00:00 2001 From: Nicola Loi Date: Sun, 27 Jul 2025 13:08:54 +0200 Subject: [PATCH 3/4] fix for windows tests --- .../pipelines/integration/UniformTSDFVolume.cpp | 4 ++-- cpp/open3d/utility/Parallel.cpp | 16 ++++++++++++++++ cpp/open3d/utility/Parallel.h | 6 ++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/cpp/open3d/pipelines/integration/UniformTSDFVolume.cpp b/cpp/open3d/pipelines/integration/UniformTSDFVolume.cpp index dcda5d80f04..f58cbf7fc3f 100644 --- a/cpp/open3d/pipelines/integration/UniformTSDFVolume.cpp +++ b/cpp/open3d/pipelines/integration/UniformTSDFVolume.cpp @@ -265,8 +265,8 @@ std::shared_ptr UniformTSDFVolume::ExtractVoxelGrid() #pragma omp parallel num_threads(utility::EstimateMaxThreads()) { #pragma omp single - { per_thread_voxels.resize(omp_get_num_threads()); } - int thread_id = omp_get_thread_num(); + { per_thread_voxels.resize(utility::GetNumThreads()); } + int thread_id = utility::GetThreadNum(); #ifdef _WIN32 #pragma omp for schedule(static) diff --git a/cpp/open3d/utility/Parallel.cpp b/cpp/open3d/utility/Parallel.cpp index 79989eeeddd..40eacc5c3d5 100644 --- a/cpp/open3d/utility/Parallel.cpp +++ b/cpp/open3d/utility/Parallel.cpp @@ -45,6 +45,22 @@ int EstimateMaxThreads() { #endif } +int GetNumThreads() { +#ifdef _OPENMP + return omp_get_num_threads(); +#else + return 1; // No parallelism available. +#endif +} + +int GetThreadNum() { +#ifdef _OPENMP + return omp_get_thread_num(); +#else + return 0; // No parallelism available, so thread ID is always 0. +#endif +} + bool InParallel() { // TODO: when we add TBB/Parallel STL support to ParallelFor, update this. #ifdef _OPENMP diff --git a/cpp/open3d/utility/Parallel.h b/cpp/open3d/utility/Parallel.h index ad37dfaa1be..09fb28830a1 100644 --- a/cpp/open3d/utility/Parallel.h +++ b/cpp/open3d/utility/Parallel.h @@ -13,6 +13,12 @@ namespace utility { /// Estimate the maximum number of threads to be used in a parallel region. int EstimateMaxThreads(); +/// Returns the number of threads in the current parallel region. +int GetNumThreads(); + +/// Returns the thread ID in the current parallel region. +int GetThreadNum(); + /// Returns true if in an parallel section. bool InParallel(); From 175386232899fc04140b049fac2fcb4010bbbdad Mon Sep 17 00:00:00 2001 From: Nicola Loi Date: Sun, 24 Aug 2025 12:40:26 +0200 Subject: [PATCH 4/4] review --- .../integration/UniformTSDFVolume.cpp | 24 ++++++++++++------- cpp/open3d/utility/Parallel.cpp | 8 ------- cpp/open3d/utility/Parallel.h | 3 --- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/cpp/open3d/pipelines/integration/UniformTSDFVolume.cpp b/cpp/open3d/pipelines/integration/UniformTSDFVolume.cpp index f58cbf7fc3f..628e152ebec 100644 --- a/cpp/open3d/pipelines/integration/UniformTSDFVolume.cpp +++ b/cpp/open3d/pipelines/integration/UniformTSDFVolume.cpp @@ -260,13 +260,16 @@ std::shared_ptr UniformTSDFVolume::ExtractVoxelGrid() // Create a vector to hold voxels for each thread in the parallel region, // since access to voxel_grid->voxels_ (std::unordered_map) is not // thread-safe. - std::vector> per_thread_voxels; + std::vector>> + per_thread_voxels; -#pragma omp parallel num_threads(utility::EstimateMaxThreads()) + int num_threads = utility::EstimateMaxThreads(); + +#pragma omp parallel num_threads(num_threads) { #pragma omp single - { per_thread_voxels.resize(utility::GetNumThreads()); } - int thread_id = utility::GetThreadNum(); + { per_thread_voxels.resize(num_threads); } + auto &thread_voxels = per_thread_voxels[utility::GetThreadNum()]; #ifdef _WIN32 #pragma omp for schedule(static) @@ -283,17 +286,22 @@ std::shared_ptr UniformTSDFVolume::ExtractVoxelGrid() double c = (f + 1.0) * 0.5; Eigen::Vector3d color(c, c, c); Eigen::Vector3i index(x, y, z); - per_thread_voxels[thread_id].emplace_back(index, color); + thread_voxels.emplace_back(std::make_pair( + index, geometry::Voxel(index, color))); } } } } } + size_t total_voxels = 0; for (const auto &thread_vector : per_thread_voxels) { - for (const auto &voxel : thread_vector) { - voxel_grid->voxels_[voxel.grid_index_] = voxel; - } + total_voxels += thread_vector.size(); + } + voxel_grid->voxels_.reserve(total_voxels); + + for (const auto &thread_vector : per_thread_voxels) { + voxel_grid->voxels_.insert(thread_vector.begin(), thread_vector.end()); } return voxel_grid; diff --git a/cpp/open3d/utility/Parallel.cpp b/cpp/open3d/utility/Parallel.cpp index 40eacc5c3d5..21ce66c6cf1 100644 --- a/cpp/open3d/utility/Parallel.cpp +++ b/cpp/open3d/utility/Parallel.cpp @@ -45,14 +45,6 @@ int EstimateMaxThreads() { #endif } -int GetNumThreads() { -#ifdef _OPENMP - return omp_get_num_threads(); -#else - return 1; // No parallelism available. -#endif -} - int GetThreadNum() { #ifdef _OPENMP return omp_get_thread_num(); diff --git a/cpp/open3d/utility/Parallel.h b/cpp/open3d/utility/Parallel.h index 09fb28830a1..aa33552fd9b 100644 --- a/cpp/open3d/utility/Parallel.h +++ b/cpp/open3d/utility/Parallel.h @@ -13,9 +13,6 @@ namespace utility { /// Estimate the maximum number of threads to be used in a parallel region. int EstimateMaxThreads(); -/// Returns the number of threads in the current parallel region. -int GetNumThreads(); - /// Returns the thread ID in the current parallel region. int GetThreadNum();