colmap · ahojnnes · Jun 13, 2023 · Jun 11, 2023 · Jun 12, 2023 · Jun 12, 2023
diff --git a/.azure-pipelines/build-ubuntu.yaml b/.azure-pipelines/build-ubuntu.yaml
@@ -3,9 +3,10 @@ parameters:
   ubuntuVersion: '20.04'
   cudaEnabled: false
   asanEnabled: false
+  cmakeBuildType: Release
 
 jobs:
-- job: ubuntu_build_${{ replace(parameters.ubuntuVersion, '.', '') }}_cuda_${{ parameters.cudaEnabled }}_asan_${{ parameters.asanEnabled }}
+- job: ubuntu_build_${{ replace(parameters.ubuntuVersion, '.', '') }}_cuda_${{ parameters.cudaEnabled }}_asan_${{ parameters.asanEnabled }}_${{ parameters.cmakeBuildType }}
   displayName: '${{ parameters.displayName }}'
   pool:
     vmImage: 'ubuntu-${{ parameters.ubuntuVersion }}'
@@ -69,12 +70,20 @@ jobs:
         echo '##vso[task.setvariable variable=CXX]/usr/bin/clang++-15'
       displayName: 'Install Clang'
 
+  - ${{ if eq(parameters.cmakeBuildType, 'ClangTidy') }}:
+    - script: |
+        sudo apt-get install -y clang-15 clang-tidy-15
+        echo '##vso[task.setvariable variable=CC]/usr/bin/clang-15'
+        echo '##vso[task.setvariable variable=CXX]/usr/bin/clang++-15'
+      displayName: 'Install Clang'
+
   - script: |
       cmake --version
       mkdir build
       cd build
       cmake .. \
         -GNinja \
+        -DCMAKE_BUILD_TYPE=${{ parameters.cmakeBuildType }} \
         -DTESTS_ENABLED=ON \
         -DCMAKE_CUDA_ARCHITECTURES=50 \
         -DASAN_ENABLED=${{ parameters.asanEnabled }}

diff --git a/.azure-pipelines/build.yaml b/.azure-pipelines/build.yaml
@@ -32,6 +32,11 @@ jobs:
       displayName: 'Ubuntu 22.04 (ASan)'
       ubuntuVersion: 22.04
       asanEnabled: true
+  - template: build-ubuntu.yaml
+    parameters:
+      displayName: 'Ubuntu 22.04 (ClangTidy)'
+      ubuntuVersion: 22.04
+      cmakeBuildType: ClangTidy
   - template: build-mac.yaml
     parameters:
       displayName: 'Mac 11'

diff --git a/.clang-tidy b/.clang-tidy
@@ -0,0 +1,5 @@
+Checks: >
+  performance-*,
+WarningsAsErrors: '*'
+FormatStyle: 'file'
+User: 'user'
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -194,6 +194,13 @@ else()
     set(IS_DEBUG OFF)
 endif()
 
+if("${CMAKE_BUILD_TYPE}" STREQUAL "ClangTidy")
+    find_program(CLANG_TIDY_EXE NAMES clang-tidy)
+    if(NOT CLANG_TIDY_EXE)
+        message(FATAL_ERROR "Could not find the clang-tidy executable, please set CLANG_TIDY_EXE")
+    endif()
+endif()
+
 if(IS_MSVC)
     # Some fixes for the Glog library.
     add_definitions("-DGLOG_NO_ABBREVIATED_SEVERITIES")
@@ -289,7 +296,7 @@ else()
     message(STATUS "Disabling OpenGL support")
 endif()
 
-if(CCACHE_ENABLED)
+if(CCACHE_ENABLED AND NOT "${CMAKE_BUILD_TYPE}" STREQUAL "ClangTidy")
     find_program(CCACHE ccache)
     if(CCACHE)
         message(STATUS "Enabling ccache support")

diff --git a/cmake/CMakeHelper.cmake b/cmake/CMakeHelper.cmake
@@ -114,6 +114,9 @@ macro(COLMAP_ADD_LIBRARY TARGET_NAME)
     add_library(${TARGET_NAME} STATIC ${ARGN})
     set_target_properties(${TARGET_NAME} PROPERTIES FOLDER
         ${COLMAP_TARGETS_ROOT_FOLDER}/${FOLDER_NAME})
+    if(CLANG_TIDY_EXE)
+        set_target_properties(${TARGET_NAME} PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_EXE};-header-filter=.*")
+    endif()
     install(TARGETS ${TARGET_NAME} DESTINATION lib/colmap)
 endmacro(COLMAP_ADD_LIBRARY)
 
@@ -143,6 +146,9 @@ macro(COLMAP_ADD_EXECUTABLE TARGET_NAME)
     else()
         install(TARGETS ${TARGET_NAME} DESTINATION bin/)
     endif()
+    if(CLANG_TIDY_EXE)
+        set_target_properties(${TARGET_NAME} PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_EXE};-header-filter=.*")
+    endif()
 endmacro(COLMAP_ADD_EXECUTABLE)
 
 # Wrapper for test executables.
@@ -152,6 +158,9 @@ macro(COLMAP_ADD_TEST TARGET_NAME)
         add_executable(${TARGET_NAME} ${ARGN})
         set_target_properties(${TARGET_NAME} PROPERTIES FOLDER
             ${COLMAP_TARGETS_ROOT_FOLDER}/${FOLDER_NAME})
+        if(CLANG_TIDY_EXE)
+            set_target_properties(${TARGET_NAME} PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_EXE};-header-filter=.*")
+        endif()
         target_link_libraries(${TARGET_NAME} colmap
                               ${Boost_UNIT_TEST_FRAMEWORK_LIBRARY})
         add_test("${FOLDER_NAME}/${TARGET_NAME}" ${TARGET_NAME})

diff --git a/src/colmap/base/camera_models_test.cc b/src/colmap/base/camera_models_test.cc
@@ -115,14 +115,16 @@ void TestModel(const std::vector<double>& params) {
       CameraModelIdToName(CameraModelNameToId(CameraModel::model_name)),
       CameraModel::model_name);
 
+  // NOLINTNEXTLINE(clang-analyzer-security.FloatLoopCounter)
   for (double u = -0.5; u <= 0.5; u += 0.1) {
+    // NOLINTNEXTLINE(clang-analyzer-security.FloatLoopCounter)
     for (double v = -0.5; v <= 0.5; v += 0.1) {
       TestWorldToImageToWorld<CameraModel>(params, u, v);
     }
   }
 
-  for (double x = 0; x <= 800; x += 50) {
-    for (double y = 0; y <= 800; y += 50) {
+  for (int x = 0; x <= 800; x += 50) {
+    for (int y = 0; y <= 800; y += 50) {
       TestImageToWorldToImage<CameraModel>(params, x, y);
     }
   }

diff --git a/src/colmap/base/database.cc b/src/colmap/base/database.cc
@@ -318,7 +318,7 @@ bool Database::ExistsImage(const image_t image_id) const {
   return ExistsRowId(sql_stmt_exists_image_id_, image_id);
 }
 
-bool Database::ExistsImageWithName(std::string name) const {
+bool Database::ExistsImageWithName(const std::string& name) const {
   return ExistsRowString(sql_stmt_exists_image_name_, name);
 }
 
@@ -470,9 +470,8 @@ FeatureDescriptors Database::ReadDescriptors(const image_t image_id) const {
   SQLITE3_CALL(sqlite3_bind_int64(sql_stmt_read_descriptors_, 1, image_id));
 
   const int rc = SQLITE3_CALL(sqlite3_step(sql_stmt_read_descriptors_));
-  const FeatureDescriptors descriptors =
-      ReadDynamicMatrixBlob<FeatureDescriptors>(
-          sql_stmt_read_descriptors_, rc, 0);
+  FeatureDescriptors descriptors = ReadDynamicMatrixBlob<FeatureDescriptors>(
+      sql_stmt_read_descriptors_, rc, 0);
 
   SQLITE3_CALL(sqlite3_reset(sql_stmt_read_descriptors_));
 

diff --git a/src/colmap/base/database.h b/src/colmap/base/database.h
@@ -75,7 +75,7 @@ class Database {
   // `image_id1` and `image_id2` does not matter.
   bool ExistsCamera(const camera_t camera_id) const;
   bool ExistsImage(const image_t image_id) const;
-  bool ExistsImageWithName(std::string name) const;
+  bool ExistsImageWithName(const std::string& name) const;
   bool ExistsKeypoints(const image_t image_id) const;
   bool ExistsDescriptors(const image_t image_id) const;
   bool ExistsMatches(const image_t image_id1, const image_t image_id2) const;

diff --git a/src/colmap/base/graph_cut.cc b/src/colmap/base/graph_cut.cc
@@ -156,13 +156,13 @@ void ComputeMinGraphCutStoerWagner(
                                  max_vertex_index + 1,
                                  edges.size());
 
+  const auto edge_weight = boost::get(boost::edge_weight, graph);
   const auto parities = boost::make_one_bit_color_map(
       boost::num_vertices(graph), boost::get(boost::vertex_index, graph));
+  const auto parity_map = boost::parity_map(parities);
 
-  *cut_weight =
-      boost::stoer_wagner_min_cut(graph,
-                                  boost::get(boost::edge_weight, graph),
-                                  boost::parity_map(parities));
+  // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDelete)
+  *cut_weight = boost::stoer_wagner_min_cut(graph, edge_weight, parity_map);
 
   cut_labels->resize(boost::num_vertices(graph));
   for (size_t i = 0; i < boost::num_vertices(graph); ++i) {

diff --git a/src/colmap/base/reconstruction.cc b/src/colmap/base/reconstruction.cc
@@ -920,7 +920,7 @@ bool Reconstruction::ExportCam(const std::string& path,
     const class Camera& camera = Camera(image.CameraId());
 
     SplitFileExtension(image.Name(), &name, &ext);
-    name = JoinPaths(path, name + ".cam");
+    name = JoinPaths(path, name.append(".cam"));
     std::ofstream file(name, std::ios::trunc);
 
     CHECK(file.is_open()) << name;

diff --git a/src/colmap/base/reconstruction_manager.cc b/src/colmap/base/reconstruction_manager.cc
@@ -38,13 +38,14 @@ namespace colmap {
 
 ReconstructionManager::ReconstructionManager() {}
 
-ReconstructionManager::ReconstructionManager(ReconstructionManager&& other)
+ReconstructionManager::ReconstructionManager(
+    ReconstructionManager&& other) noexcept
     : ReconstructionManager() {
   reconstructions_ = std::move(other.reconstructions_);
 }
 
 ReconstructionManager& ReconstructionManager::operator=(
-    ReconstructionManager&& other) {
+    ReconstructionManager&& other) noexcept {
   if (this != &other) {
     reconstructions_ = std::move(other.reconstructions_);
   }

diff --git a/src/colmap/base/reconstruction_manager.h b/src/colmap/base/reconstruction_manager.h
@@ -45,8 +45,8 @@ class ReconstructionManager {
   ReconstructionManager();
 
   // Move constructor and assignment.
-  ReconstructionManager(ReconstructionManager&& other);
-  ReconstructionManager& operator=(ReconstructionManager&& other);
+  ReconstructionManager(ReconstructionManager&& other) noexcept;
+  ReconstructionManager& operator=(ReconstructionManager&& other) noexcept;
 
   // The number of reconstructions managed.
   size_t Size() const;

diff --git a/src/colmap/base/triangulation_test.cc b/src/colmap/base/triangulation_test.cc
@@ -50,8 +50,9 @@ BOOST_AUTO_TEST_CASE(TestTriangulatePoint) {
 
   Eigen::Matrix3x4d proj_matrix1 = Eigen::MatrixXd::Identity(3, 4);
 
-  for (double qz = 0; qz < 1; qz += 0.2) {
-    for (double tx = 0; tx < 10; tx += 2) {
+  for (int z = 0; z < 5; ++z) {
+    const double qz = z / 5.0;
+    for (int tx = 0; tx < 10; tx += 2) {
       SimilarityTransform3 tform(
           1, Eigen::Vector4d(0.2, 0.3, 0.4, qz), Eigen::Vector3d(tx, 2, 3));
 

diff --git a/src/colmap/estimators/absolute_pose.cc b/src/colmap/estimators/absolute_pose.cc
@@ -272,7 +272,7 @@ void EPNPEstimator::ChooseControlPoints() {
   const Eigen::Matrix3d PW0tPW0 = PW0.transpose() * PW0;
   Eigen::JacobiSVD<Eigen::Matrix3d> svd(
       PW0tPW0, Eigen::ComputeFullV | Eigen::ComputeFullU);
-  const Eigen::Vector3d D = svd.singularValues();
+  const Eigen::Vector3d& D = svd.singularValues();
   const Eigen::Matrix3d Ut = svd.matrixU().transpose();
 
   for (int i = 1; i < 4; ++i) {
@@ -387,8 +387,7 @@ void EPNPEstimator::FindBetasApprox1(const Eigen::Matrix<double, 6, 10>& L6x10,
 
   Eigen::JacobiSVD<Eigen::Matrix<double, 6, 4>> svd(
       L_6x4, Eigen::ComputeFullV | Eigen::ComputeFullU);
-  Eigen::Matrix<double, 6, 1> Rho_temp = rho;
-  const Eigen::Matrix<double, 4, 1> b4 = svd.solve(Rho_temp);
+  const Eigen::Matrix<double, 4, 1> b4 = svd.solve(rho);
 
   if (b4[0] < 0) {
     (*betas)[0] = std::sqrt(-b4[0]);
@@ -419,8 +418,7 @@ void EPNPEstimator::FindBetasApprox2(const Eigen::Matrix<double, 6, 10>& L6x10,
 
   Eigen::JacobiSVD<Eigen::Matrix<double, 6, 3>> svd(
       L_6x3, Eigen::ComputeFullV | Eigen::ComputeFullU);
-  Eigen::Matrix<double, 6, 1> Rho_temp = rho;
-  const Eigen::Matrix<double, 3, 1> b3 = svd.solve(Rho_temp);
+  const Eigen::Matrix<double, 3, 1> b3 = svd.solve(rho);
 
   if (b3[0] < 0) {
     (*betas)[0] = std::sqrt(-b3[0]);
@@ -446,8 +444,7 @@ void EPNPEstimator::FindBetasApprox3(const Eigen::Matrix<double, 6, 10>& L6x10,
                                      Eigen::Vector4d* betas) {
   Eigen::JacobiSVD<Eigen::Matrix<double, 6, 5>> svd(
       L6x10.leftCols<5>(), Eigen::ComputeFullV | Eigen::ComputeFullU);
-  Eigen::Matrix<double, 6, 1> Rho_temp = rho;
-  const Eigen::Matrix<double, 5, 1> b5 = svd.solve(Rho_temp);
+  const Eigen::Matrix<double, 5, 1> b5 = svd.solve(rho);
 
   if (b5[0] < 0) {
     (*betas)[0] = std::sqrt(-b5[0]);
@@ -571,8 +568,8 @@ void EPNPEstimator::EstimateRT(Eigen::Matrix3d* R, Eigen::Vector3d* t) {
 
   Eigen::JacobiSVD<Eigen::Matrix3d> svd(
       abt, Eigen::ComputeFullV | Eigen::ComputeFullU);
-  const Eigen::Matrix3d abt_U = svd.matrixU();
-  const Eigen::Matrix3d abt_V = svd.matrixV();
+  const Eigen::Matrix3d& abt_U = svd.matrixU();
+  const Eigen::Matrix3d& abt_V = svd.matrixV();
 
   for (int i = 0; i < 3; ++i) {
     for (int j = 0; j < 3; ++j) {

diff --git a/src/colmap/estimators/absolute_pose_test.cc b/src/colmap/estimators/absolute_pose_test.cc
@@ -61,7 +61,9 @@ BOOST_AUTO_TEST_CASE(TestP3P) {
     points3D_faulty[i](0) = 20;
   }
 
+  // NOLINTNEXTLINE(clang-analyzer-security.FloatLoopCounter)
   for (double qx = 0; qx < 1; qx += 0.2) {
+    // NOLINTNEXTLINE(clang-analyzer-security.FloatLoopCounter)
     for (double tx = 0; tx < 1; tx += 0.1) {
       const SimilarityTransform3 orig_tform(
           1, Eigen::Vector4d(1, qx, 0, 0), Eigen::Vector3d(tx, 0, 0));
@@ -121,7 +123,9 @@ BOOST_AUTO_TEST_CASE(TestEPNP) {
     points3D_faulty[i](0) = 20;
   }
 
+  // NOLINTNEXTLINE(clang-analyzer-security.FloatLoopCounter)
   for (double qx = 0; qx < 1; qx += 0.2) {
+    // NOLINTNEXTLINE(clang-analyzer-security.FloatLoopCounter)
     for (double tx = 0; tx < 1; tx += 0.1) {
       const SimilarityTransform3 orig_tform(
           1, Eigen::Vector4d(1, qx, 0, 0), Eigen::Vector3d(tx, 0, 0));

diff --git a/src/colmap/estimators/affine_transform.cc b/src/colmap/estimators/affine_transform.cc
@@ -68,8 +68,7 @@ std::vector<AffineTransformEstimator::M_t> AffineTransformEstimator::Estimate(
 
   Eigen::Map<const Eigen::Matrix<double, 3, 2>> A_t(nullspace.data());
 
-  const std::vector<M_t> models = {A_t.transpose()};
-  return models;
+  return {A_t.transpose()};
 }
 
 void AffineTransformEstimator::Residuals(const std::vector<X_t>& points1,

diff --git a/src/colmap/estimators/affine_transform_test.cc b/src/colmap/estimators/affine_transform_test.cc
@@ -39,9 +39,9 @@
 using namespace colmap;
 
 BOOST_AUTO_TEST_CASE(TestAffineTransform) {
-  for (double x = 0; x < 1; x += 0.1) {
+  for (int x = 0; x < 10; ++x) {
     Eigen::Matrix<double, 2, 3> A;
-    A << x, 0.2, 0.3, 30, 0.2, 0.1;
+    A << x / 10.0, 0.2, 0.3, 30, 0.2, 0.1;
 
     std::vector<Eigen::Vector2d> src;
     src.emplace_back(x, 0);

diff --git a/src/colmap/estimators/coordinate_frame.cc b/src/colmap/estimators/coordinate_frame.cc
@@ -105,7 +105,7 @@ Eigen::Vector3d FindBestConsensusAxis(const std::vector<Eigen::Vector3d>& axes,
   double best_inlier_distance_sum = std::numeric_limits<double>::max();
 
   for (size_t i = 0; i < axes.size(); ++i) {
-    const Eigen::Vector3d ref_axis = axes[i];
+    const Eigen::Vector3d& ref_axis = axes[i];
     double inlier_distance_sum = 0;
     inlier_idxs.clear();
     for (size_t j = 0; j < axes.size(); ++j) {
@@ -203,7 +203,7 @@ Eigen::Matrix3d EstimateManhattanWorldFrame(
     std::vector<Eigen::Vector3d> horizontal_lines;
     std::vector<Eigen::Vector3d> vertical_lines;
     for (size_t i = 0; i < line_segments.size(); ++i) {
-      const auto line_segment = line_segments[i];
+      const auto& line_segment = line_segments[i];
       const Eigen::Vector3d line_segment_start =
           line_segment.start.homogeneous();
       const Eigen::Vector3d line_segment_end = line_segment.end.homogeneous();

diff --git a/src/colmap/estimators/essential_matrix.cc b/src/colmap/estimators/essential_matrix.cc
@@ -202,8 +202,7 @@ EssentialMatrixEightPointEstimator::Estimate(const std::vector<X_t>& points1,
   const Eigen::Matrix3d E = E_raw_svd.matrixU() * singular_values.asDiagonal() *
                             E_raw_svd.matrixV().transpose();
 
-  const std::vector<M_t> models = {E};
-  return models;
+  return {E};
 }
 
 void EssentialMatrixEightPointEstimator::Residuals(

diff --git a/src/colmap/estimators/fundamental_matrix.cc b/src/colmap/estimators/fundamental_matrix.cc
@@ -73,7 +73,7 @@ FundamentalMatrixSevenPointEstimator::Estimate(
 
   // 9 unknowns with 7 equations, so we have 2D null space.
   Eigen::JacobiSVD<Eigen::Matrix<double, 7, 9>> svd(A, Eigen::ComputeFullV);
-  const Eigen::Matrix<double, 9, 9> f = svd.matrixV();
+  const Eigen::Matrix<double, 9, 9>& f = svd.matrixV();
   Eigen::Matrix<double, 1, 9> f1 = f.col(7);
   Eigen::Matrix<double, 1, 9> f2 = f.col(8);
 
@@ -189,9 +189,7 @@ FundamentalMatrixEightPointEstimator::Estimate(
                             singular_values.asDiagonal() *
                             fmatrix_svd.matrixV().transpose();
 
-  const std::vector<M_t> models = {points2_norm_matrix.transpose() * F *
-                                   points1_norm_matrix};
-  return models;
+  return {points2_norm_matrix.transpose() * F * points1_norm_matrix};
 }
 
 void FundamentalMatrixEightPointEstimator::Residuals(

diff --git a/src/colmap/estimators/generalized_absolute_pose_test.cc b/src/colmap/estimators/generalized_absolute_pose_test.cc
@@ -63,7 +63,9 @@ BOOST_AUTO_TEST_CASE(Estimate) {
     points3D_faulty[i](0) = 20;
   }
 
+  // NOLINTNEXTLINE(clang-analyzer-security.FloatLoopCounter)
   for (double qx = 0; qx < 1; qx += 0.2) {
+    // NOLINTNEXTLINE(clang-analyzer-security.FloatLoopCounter)
     for (double tx = 0; tx < 1; tx += 0.1) {
       const int kRefTform = 1;
       const int kNumTforms = 3;

diff --git a/src/colmap/estimators/generalized_relative_pose.cc b/src/colmap/estimators/generalized_relative_pose.cc
@@ -137,8 +137,8 @@ Eigen::Vector3d ComputeRotationBetweenPoints(
 
   const Eigen::JacobiSVD<Eigen::Matrix3d> svd(
       Hcross, Eigen::ComputeFullU | Eigen::ComputeFullV);
-  const Eigen::Matrix3d V = svd.matrixV();
-  const Eigen::Matrix3d U = svd.matrixU();
+  const Eigen::Matrix3d& V = svd.matrixV();
+  const Eigen::Matrix3d& U = svd.matrixU();
 
   Eigen::Matrix3d R = V * U.transpose();
   if (R.determinant() < 0) {

diff --git a/src/colmap/estimators/generalized_relative_pose_test.cc b/src/colmap/estimators/generalized_relative_pose_test.cc
@@ -55,7 +55,9 @@ BOOST_AUTO_TEST_CASE(Estimate) {
                           RandomReal<double>(-10, 10));
   }
 
+  // NOLINTNEXTLINE(clang-analyzer-security.FloatLoopCounter)
   for (double qx = 0; qx < 0.4; qx += 0.1) {
+    // NOLINTNEXTLINE(clang-analyzer-security.FloatLoopCounter)
     for (double tx = 0; tx < 0.5; tx += 0.1) {
       const int kRefTform = 1;
       const int kNumTforms = 3;