From ccb493b80581aed527cb252b1aa839a8011283b3 Mon Sep 17 00:00:00 2001 From: Sergey Sharybin Date: Wed, 2 Apr 2025 10:25:49 +0200 Subject: [PATCH] Libmv: Abstract parallel range in camera intrinsics The goal is to be able to switch away from OpenMP by default. In order to achieve this a new parallel_for() function is added, which follows the same declaration as the one from TBB. For now the simplest formulation is used where range is provided by start and last indices (the last one is excluded from the range). The side effect is that Libmv now expects the threading limits to be imposed by the default thread area (or have an explicit thread area with the limit in the caller). In a way this is similar to other external libraries. Pull Request: https://projects.blender.org/blender/blender/pulls/136834 --- intern/libmv/CMakeLists.txt | 7 +++ intern/libmv/intern/camera_intrinsics.cc | 11 +--- intern/libmv/intern/camera_intrinsics.h | 4 -- intern/libmv/intern/stub.cc | 4 -- .../simple_pipeline/camera_intrinsics.cc | 16 +---- .../libmv/simple_pipeline/camera_intrinsics.h | 9 --- .../simple_pipeline/camera_intrinsics_impl.h | 35 +++++------ intern/libmv/libmv/threading/parallel_for.h | 60 +++++++++++++++++++ .../libmv/threading/parallel_for_test.cc | 49 +++++++++++++++ source/blender/blenkernel/BKE_tracking.h | 1 - source/blender/blenkernel/intern/tracking.cc | 5 -- .../blenkernel/intern/tracking_util.cc | 2 - source/blender/editors/space_clip/clip_ops.cc | 2 - 13 files changed, 134 insertions(+), 71 deletions(-) create mode 100644 intern/libmv/libmv/threading/parallel_for.h create mode 100644 intern/libmv/libmv/threading/parallel_for_test.cc diff --git a/intern/libmv/CMakeLists.txt b/intern/libmv/CMakeLists.txt index 965d0028020..89128f63de6 100644 --- a/intern/libmv/CMakeLists.txt +++ b/intern/libmv/CMakeLists.txt @@ -29,6 +29,10 @@ if(WITH_LIBMV) add_definitions(${GLOG_DEFINES}) add_definitions(-DLIBMV_GFLAGS_NAMESPACE=${GFLAGS_NAMESPACE}) + if(WITH_TBB) + add_definitions(-DLIBMV_USE_TBB_THREADS) + endif() + list(APPEND INC ${GFLAGS_INCLUDE_DIRS} ${GLOG_INCLUDE_DIRS} @@ -45,6 +49,7 @@ if(WITH_LIBMV) list(APPEND LIB extern_ceres + PUBLIC bf::dependencies::optional::tbb ${GLOG_LIBRARIES} ${GFLAGS_LIBRARIES} @@ -182,6 +187,7 @@ if(WITH_LIBMV) libmv/simple_pipeline/resect.h libmv/simple_pipeline/tracks.h libmv/threading/threading.h + libmv/threading/parallel_for.h libmv/tracking/brute_region_tracker.h libmv/tracking/hybrid_region_tracker.h libmv/tracking/kalman_filter.h @@ -231,6 +237,7 @@ if(WITH_LIBMV) blender_add_test_executable("libmv_brute_region_tracker" "./libmv/tracking/brute_region_tracker_test.cc" "${INC}" "${INC_SYS}" "libmv_test_dataset;bf_intern_libmv;extern_ceres") blender_add_test_executable("libmv_klt_region_tracker" "./libmv/tracking/klt_region_tracker_test.cc" "${INC}" "${INC_SYS}" "libmv_test_dataset;bf_intern_libmv;extern_ceres") blender_add_test_executable("libmv_pyramid_region_tracker" "./libmv/tracking/pyramid_region_tracker_test.cc" "${INC}" "${INC_SYS}" "libmv_test_dataset;bf_intern_libmv;extern_ceres") + blender_add_test_executable("libmv_parallel_for" "./libmv/threading/parallel_for_test.cc" "${INC}" "${INC_SYS}" "libmv_test_dataset;bf_intern_libmv;extern_ceres") endif() else() list(APPEND SRC diff --git a/intern/libmv/intern/camera_intrinsics.cc b/intern/libmv/intern/camera_intrinsics.cc index 3a90dd472b7..6541a71354e 100644 --- a/intern/libmv/intern/camera_intrinsics.cc +++ b/intern/libmv/intern/camera_intrinsics.cc @@ -74,10 +74,7 @@ void libmv_cameraIntrinsicsUpdate( int image_height = libmv_camera_intrinsics_options->image_height; /* Try avoid unnecessary updates, so pre-computed distortion grids - * are not freed. - */ - - camera_intrinsics->SetThreads(libmv_camera_intrinsics_options->num_threads); + * are not freed. */ if (camera_intrinsics->focal_length() != focal_length) { camera_intrinsics->SetFocalLength(focal_length, focal_length); @@ -177,12 +174,6 @@ void libmv_cameraIntrinsicsUpdate( } } -void libmv_cameraIntrinsicsSetThreads(libmv_CameraIntrinsics* libmv_intrinsics, - int threads) { - CameraIntrinsics* camera_intrinsics = (CameraIntrinsics*)libmv_intrinsics; - camera_intrinsics->SetThreads(threads); -} - void libmv_cameraIntrinsicsExtractOptions( const libmv_CameraIntrinsics* libmv_intrinsics, libmv_CameraIntrinsicsOptions* camera_intrinsics_options) { diff --git a/intern/libmv/intern/camera_intrinsics.h b/intern/libmv/intern/camera_intrinsics.h index 9e9e593dbe5..519eca2d3f5 100644 --- a/intern/libmv/intern/camera_intrinsics.h +++ b/intern/libmv/intern/camera_intrinsics.h @@ -20,7 +20,6 @@ enum { typedef struct libmv_CameraIntrinsicsOptions { // Common settings of all distortion models. - int num_threads; int distortion_model; int image_width, image_height; double focal_length; @@ -52,9 +51,6 @@ void libmv_cameraIntrinsicsUpdate( const libmv_CameraIntrinsicsOptions* libmv_camera_intrinsics_options, libmv_CameraIntrinsics* libmv_intrinsics); -void libmv_cameraIntrinsicsSetThreads(libmv_CameraIntrinsics* libmv_intrinsics, - int threads); - void libmv_cameraIntrinsicsExtractOptions( const libmv_CameraIntrinsics* libmv_intrinsics, libmv_CameraIntrinsicsOptions* camera_intrinsics_options); diff --git a/intern/libmv/intern/stub.cc b/intern/libmv/intern/stub.cc index 456ab74234d..6e7e7567267 100644 --- a/intern/libmv/intern/stub.cc +++ b/intern/libmv/intern/stub.cc @@ -218,10 +218,6 @@ void libmv_cameraIntrinsicsUpdate( libmv_CameraIntrinsics* /*libmv_intrinsics*/) { } -void libmv_cameraIntrinsicsSetThreads( - libmv_CameraIntrinsics* /*libmv_intrinsics*/, int /*threads*/) { -} - void libmv_cameraIntrinsicsExtractOptions( const libmv_CameraIntrinsics* /*libmv_intrinsics*/, libmv_CameraIntrinsicsOptions* camera_intrinsics_options) { diff --git a/intern/libmv/libmv/simple_pipeline/camera_intrinsics.cc b/intern/libmv/libmv/simple_pipeline/camera_intrinsics.cc index b86e316b139..107479896f8 100644 --- a/intern/libmv/libmv/simple_pipeline/camera_intrinsics.cc +++ b/intern/libmv/libmv/simple_pipeline/camera_intrinsics.cc @@ -29,15 +29,14 @@ namespace libmv { namespace internal { LookupWarpGrid::LookupWarpGrid() - : offset_(NULL), width_(0), height_(0), overscan_(0.0), threads_(1) { + : offset_(NULL), width_(0), height_(0), overscan_(0.0) { } LookupWarpGrid::LookupWarpGrid(const LookupWarpGrid& from) : offset_(NULL), width_(from.width_), height_(from.height_), - overscan_(from.overscan_), - threads_(from.threads_) { + overscan_(from.overscan_) { if (from.offset_) { offset_ = new Offset[width_ * height_]; memcpy(offset_, from.offset_, sizeof(Offset) * width_ * height_); @@ -53,11 +52,6 @@ void LookupWarpGrid::Reset() { offset_ = NULL; } -// Set number of threads used for threaded buffer distortion/undistortion. -void LookupWarpGrid::SetThreads(int threads) { - threads_ = threads; -} - } // namespace internal CameraIntrinsics::CameraIntrinsics() @@ -99,12 +93,6 @@ void CameraIntrinsics::SetPrincipalPoint(double cx, double cy) { ResetLookupGrids(); } -// Set number of threads used for threaded buffer distortion/undistortion. -void CameraIntrinsics::SetThreads(int threads) { - distort_.SetThreads(threads); - undistort_.SetThreads(threads); -} - void CameraIntrinsics::ImageSpaceToNormalized(double image_x, double image_y, double* normalized_x, diff --git a/intern/libmv/libmv/simple_pipeline/camera_intrinsics.h b/intern/libmv/libmv/simple_pipeline/camera_intrinsics.h index efe0735bd93..5a73086f82d 100644 --- a/intern/libmv/libmv/simple_pipeline/camera_intrinsics.h +++ b/intern/libmv/libmv/simple_pipeline/camera_intrinsics.h @@ -82,9 +82,6 @@ class LookupWarpGrid { // This will tag the grid for update without re-computing it. void Reset(); - // Set number of threads used for threaded buffer distortion/undistortion. - void SetThreads(int threads); - private: // This structure contains an offset in both x,y directions // in an optimized way sawing some bytes per pixel in the memory. @@ -120,9 +117,6 @@ class LookupWarpGrid { // Overscan of the image being processed by this grid. double overscan_; - - // Number of threads which will be used for buffer istortion/undistortion. - int threads_; }; } // namespace internal @@ -160,9 +154,6 @@ class CameraIntrinsics { // Set principal point in pixels. void SetPrincipalPoint(double cx, double cy); - // Set number of threads used for threaded buffer distortion/undistortion. - void SetThreads(int threads); - // Convert image space coordinates to normalized. void ImageSpaceToNormalized(double image_x, double image_y, diff --git a/intern/libmv/libmv/simple_pipeline/camera_intrinsics_impl.h b/intern/libmv/libmv/simple_pipeline/camera_intrinsics_impl.h index c8c4700f5c6..fb97896e844 100644 --- a/intern/libmv/libmv/simple_pipeline/camera_intrinsics_impl.h +++ b/intern/libmv/libmv/simple_pipeline/camera_intrinsics_impl.h @@ -18,6 +18,8 @@ // FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS // IN THE SOFTWARE. +#include "libmv/threading/parallel_for.h" + namespace libmv { namespace { @@ -59,15 +61,12 @@ void LookupWarpGrid::Compute(const CameraIntrinsics& intrinsics, int width, int height, double overscan) { - double w = (double)width / (1.0 + overscan); - double h = (double)height / (1.0 + overscan); - double aspx = (double)w / intrinsics.image_width(); - double aspy = (double)h / intrinsics.image_height(); -#if defined(_OPENMP) -# pragma omp parallel for schedule(static) \ - num_threads(threads_) if (threads_ > 1 && height > 100) -#endif - for (int y = 0; y < height; y++) { + const double w = (double)width / (1.0 + overscan); + const double h = (double)height / (1.0 + overscan); + const double aspx = (double)w / intrinsics.image_width(); + const double aspy = (double)h / intrinsics.image_height(); + + parallel_for(0, height, [&](const int y) { for (int x = 0; x < width; x++) { double src_x = (x - 0.5 * overscan * w) / aspx, src_y = (y - 0.5 * overscan * h) / aspy; @@ -80,18 +79,18 @@ void LookupWarpGrid::Compute(const CameraIntrinsics& intrinsics, if (fx == 256) { fx = 0; ix++; - } // NOLINT + } if (fy == 256) { fy = 0; iy++; - } // NOLINT + } // Use nearest border pixel if (ix < 0) { ix = 0, fx = 0; - } // NOLINT + } if (iy < 0) { iy = 0, fy = 0; - } // NOLINT + } if (ix >= width - 2) ix = width - 2; if (iy >= height - 2) @@ -103,7 +102,7 @@ void LookupWarpGrid::Compute(const CameraIntrinsics& intrinsics, (unsigned char)fy}; offset_[y * width + x] = offset; } - } + }); } template @@ -132,11 +131,7 @@ void LookupWarpGrid::Apply(const PixelType* input_buffer, int height, int channels, PixelType* output_buffer) { -#if defined(_OPENMP) -# pragma omp parallel for schedule(static) \ - num_threads(threads_) if (threads_ > 1 && height > 100) -#endif - for (int y = 0; y < height; y++) { + parallel_for(0, height, [&](const int y) { for (int x = 0; x < width; x++) { Offset offset = offset_[y * width + x]; const int pixel_index = @@ -152,7 +147,7 @@ void LookupWarpGrid::Apply(const PixelType* input_buffer, (256 * 256); } } - } + }); } } // namespace internal diff --git a/intern/libmv/libmv/threading/parallel_for.h b/intern/libmv/libmv/threading/parallel_for.h new file mode 100644 index 00000000000..515669f5543 --- /dev/null +++ b/intern/libmv/libmv/threading/parallel_for.h @@ -0,0 +1,60 @@ +// Copyright (c) 2025 libmv authors. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to +// deal in the Software without restriction, including without limitation the +// rights to use, copy, modify, merge, publish, distribute, sublicense, and/or +// sell copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS +// IN THE SOFTWARE. + +// TBB-compatible implementation of parallel_for() functions. +// +// The parallel_for() function applies given functor for every index within the +// given range. The range might be provided in multiple ways. +// +// Supports multiple underlying threading implementations (in the order of +// preference): +// - TBB, requires LIBMV_USE_TBB_THREADS to be defined. +// - OpenMP, requires compiler to use -fopenmp, used when _OPENMP is defined. +// - Single-threaded fall-back. +// +// The function occupies all threads of the default theading pool. + +#ifndef LIBMV_THREADING_PARALLEL_FOR_H_ +#define LIBMV_THREADING_PARALLEL_FOR_H_ + +#if defined(LIBMV_USE_TBB_THREADS) +# include +#endif + +namespace libmv { + +// Run the function f() for all indices within [first, last). +template +void parallel_for(const Index first, const Index last, const Function& f) { +#if defined(LIBMV_USE_TBB_THREADS) + tbb::parallel_for(first, last, f); +#else +# if defined(_OPENMP) +# pragma omp parallel for schedule(static) +# endif + for (Index i = first; i < last; ++i) { + f(i); + } +#endif +} + +} // namespace libmv + +#endif // LIBMV_THREADING_PARALLEL_FOR_H_ diff --git a/intern/libmv/libmv/threading/parallel_for_test.cc b/intern/libmv/libmv/threading/parallel_for_test.cc new file mode 100644 index 00000000000..6b619e5b870 --- /dev/null +++ b/intern/libmv/libmv/threading/parallel_for_test.cc @@ -0,0 +1,49 @@ +// Copyright (c) 2025 libmv authors. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to +// deal in the Software without restriction, including without limitation the +// rights to use, copy, modify, merge, publish, distribute, sublicense, and/or +// sell copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS +// IN THE SOFTWARE. + +#include "libmv/threading/parallel_for.h" + +#include +#include + +#include "libmv/threading/threading.h" +#include "testing/testing.h" + +namespace libmv { + +TEST(ParallelFor, SimpleRange) { + const int num_elements = 100; + + std::vector data; + mutex data_mutex; + + parallel_for(0, num_elements, [&](const int i) { + scoped_lock lock(data_mutex); + data.push_back(i); + }); + + std::sort(data.begin(), data.end()); + + for (int i = 0; i < num_elements; ++i) { + EXPECT_EQ(data[i], i); + } +} + +} // namespace libmv diff --git a/source/blender/blenkernel/BKE_tracking.h b/source/blender/blenkernel/BKE_tracking.h index fccb010871c..eb0e8c3e78b 100644 --- a/source/blender/blenkernel/BKE_tracking.h +++ b/source/blender/blenkernel/BKE_tracking.h @@ -482,7 +482,6 @@ void BKE_tracking_distortion_update(struct MovieDistortion *distortion, struct MovieTracking *tracking, int calibration_width, int calibration_height); -void BKE_tracking_distortion_set_threads(struct MovieDistortion *distortion, int threads); struct MovieDistortion *BKE_tracking_distortion_copy(struct MovieDistortion *distortion); struct ImBuf *BKE_tracking_distortion_exec(struct MovieDistortion *distortion, struct MovieTracking *tracking, diff --git a/source/blender/blenkernel/intern/tracking.cc b/source/blender/blenkernel/intern/tracking.cc index 47c1ae16a2a..498fe2c9136 100644 --- a/source/blender/blenkernel/intern/tracking.cc +++ b/source/blender/blenkernel/intern/tracking.cc @@ -2304,11 +2304,6 @@ void BKE_tracking_distortion_update(MovieDistortion *distortion, libmv_cameraIntrinsicsUpdate(&camera_intrinsics_options, distortion->intrinsics); } -void BKE_tracking_distortion_set_threads(MovieDistortion *distortion, int threads) -{ - libmv_cameraIntrinsicsSetThreads(distortion->intrinsics, threads); -} - MovieDistortion *BKE_tracking_distortion_copy(MovieDistortion *distortion) { MovieDistortion *new_distortion; diff --git a/source/blender/blenkernel/intern/tracking_util.cc b/source/blender/blenkernel/intern/tracking_util.cc index 66cdb10ba25..dee31a7f331 100644 --- a/source/blender/blenkernel/intern/tracking_util.cc +++ b/source/blender/blenkernel/intern/tracking_util.cc @@ -493,8 +493,6 @@ void tracking_cameraIntrinscisOptionsFromTracking( tracking_principal_point_normalized_to_pixel( camera->principal_point, calibration_width, calibration_height, principal_px); - camera_intrinsics_options->num_threads = BLI_system_thread_count(); - camera_intrinsics_options->focal_length = camera->focal; camera_intrinsics_options->principal_point_x = principal_px[0]; diff --git a/source/blender/editors/space_clip/clip_ops.cc b/source/blender/editors/space_clip/clip_ops.cc index abaa4b62c4a..9e31f88772e 100644 --- a/source/blender/editors/space_clip/clip_ops.cc +++ b/source/blender/editors/space_clip/clip_ops.cc @@ -1253,13 +1253,11 @@ static void do_movie_proxy(void *pjv, const int efra = clip->len; if (build_undistort_count) { - int threads = BLI_system_thread_count(); int width, height; BKE_movieclip_get_size(clip, nullptr, &width, &height); distortion = BKE_tracking_distortion_new(&clip->tracking, width, height); - BKE_tracking_distortion_set_threads(distortion, threads); } for (int cfra = sfra; cfra <= efra; cfra++) {