diff --git a/CMakeLists.txt b/CMakeLists.txt index c218882f0..4fac3babd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -444,7 +444,8 @@ endif (NOT CUSTOM_BLAS) set_ceres_threading_model("${CERES_THREADING_MODEL}") if (BUILD_BENCHMARKS) - find_package(benchmark QUIET) + # Version 1.3 was first to provide import targets + find_package(benchmark 1.3 QUIET) if (benchmark_FOUND) message("-- Found Google benchmark library. Building Ceres benchmarks.") else() @@ -612,6 +613,15 @@ add_compile_options( $<$,$,$>:-Wno-deprecated-declarations> $<$:/wd4996>) +if (CMAKE_VERSION VERSION_LESS 3.12) + # Disable the definiton of min/max macros within the project + if (WIN32) + add_definitions (-DNOMINMAX) + endif (WIN32) +else (CMAKE_VERSION VERSION_LESS 3.12) + add_compile_definitions($<$:NOMINMAX>) +endif (CMAKE_VERSION VERSION_LESS 3.12) + # Configure the Ceres config.h compile options header using the current # compile options and put the configured header into the Ceres build # directory. Note that the ceres/internal subdir in /config where diff --git a/include/ceres/cubic_interpolation.h b/include/ceres/cubic_interpolation.h index 134bad285..3ca6b11b4 100644 --- a/include/ceres/cubic_interpolation.h +++ b/include/ceres/cubic_interpolation.h @@ -191,7 +191,7 @@ struct Grid1D { } EIGEN_STRONG_INLINE void GetValue(const int n, double* f) const { - const int idx = std::min(std::max(begin_, n), end_ - 1) - begin_; + const int idx = (std::min)((std::max)(begin_, n), end_ - 1) - begin_; if (kInterleaved) { for (int i = 0; i < kDataDimension; ++i) { f[i] = static_cast(data_[kDataDimension * idx + i]); @@ -402,9 +402,9 @@ struct Grid2D { EIGEN_STRONG_INLINE void GetValue(const int r, const int c, double* f) const { const int row_idx = - std::min(std::max(row_begin_, r), row_end_ - 1) - row_begin_; + (std::min)((std::max)(row_begin_, r), row_end_ - 1) - row_begin_; const int col_idx = - std::min(std::max(col_begin_, c), col_end_ - 1) - col_begin_; + (std::min)((std::max)(col_begin_, c), col_end_ - 1) - col_begin_; const int n = (kRowMajor) ? num_cols_ * row_idx + col_idx : num_rows_ * col_idx + row_idx; diff --git a/include/ceres/internal/householder_vector.h b/include/ceres/internal/householder_vector.h index e5fc9bfa1..7700208be 100644 --- a/include/ceres/internal/householder_vector.h +++ b/include/ceres/internal/householder_vector.h @@ -82,11 +82,11 @@ void ComputeHouseholderVector(const XVectorType& x, v->head(v->rows() - 1) /= v_pivot; } -template -Eigen::Matrix ApplyHouseholderVector( +template +typename Derived::PlainObject ApplyHouseholderVector( const XVectorType& y, - const Eigen::Matrix& v, - const Scalar& beta) { + const Eigen::MatrixBase& v, + const typename Derived::Scalar& beta) { return (y - v * (beta * (v.transpose() * y))); } diff --git a/include/ceres/internal/numeric_diff.h b/include/ceres/internal/numeric_diff.h index c44733f6d..351845c05 100644 --- a/include/ceres/internal/numeric_diff.h +++ b/include/ceres/internal/numeric_diff.h @@ -121,7 +121,7 @@ struct NumericDiff { // thus ridders_relative_initial_step_size is used. if (kMethod == RIDDERS) { min_step_size = - std::max(min_step_size, options.ridders_relative_initial_step_size); + (std::max)(min_step_size, options.ridders_relative_initial_step_size); } // For each parameter in the parameter block, use finite differences to @@ -132,7 +132,7 @@ struct NumericDiff { num_residuals_internal); for (int j = 0; j < parameter_block_size_internal; ++j) { - const double delta = std::max(min_step_size, step_size(j)); + const double delta = (std::max)(min_step_size, step_size(j)); if (kMethod == RIDDERS) { if (!EvaluateRiddersJacobianColumn(functor, @@ -296,7 +296,7 @@ struct NumericDiff { // norm_error is supposed to decrease as the finite difference tableau // generation progresses, serving both as an estimate for differentiation // error and as a measure of differentiation numerical stability. - double norm_error = std::numeric_limits::max(); + double norm_error = (std::numeric_limits::max)(); // Loop over decreasing step sizes until: // 1. Error is smaller than a given value (ridders_epsilon), @@ -342,7 +342,7 @@ struct NumericDiff { options.ridders_step_shrink_factor; // Compute the difference between the previous value and the current. - double candidate_error = std::max( + double candidate_error = (std::max)( (current_candidates->col(k) - current_candidates->col(k - 1)) .norm(), (current_candidates->col(k) - previous_candidates->col(k - 1)) diff --git a/include/ceres/internal/port.h b/include/ceres/internal/port.h index 2bc44f465..4275b0e15 100644 --- a/include/ceres/internal/port.h +++ b/include/ceres/internal/port.h @@ -63,4 +63,26 @@ // 202002L) #endif // !defined(CERES_HAS_CPP20) +// Prevents symbols from being substituted by the corresponding macro definition +// under the same name. For instance, min and max are defined as macros on +// Windows (unless NOMINMAX is defined) which causes compilation errors when +// defining or referencing symbols under the same name. +// +// To be robust in all cases particularly when NOMINMAX cannot be used, use this +// macro to annotate min/max declarations/definitions. Examples: +// +// int max CERES_PREVENT_MACRO_SUBSTITUTION(); +// min CERES_PREVENT_MACRO_SUBSTITUTION(a, b); +// max CERES_PREVENT_MACRO_SUBSTITUTION(a, b); +// +// NOTE: In case the symbols for which the substitution must be prevented are +// used within another macro, the substitution must be inhibited using parens as +// +// (std::numerical_limits::max)() +// +// since the helper macro will not work here. Do not use this technique in +// general case, because it will prevent argument-dependent lookup (ADL). +// +#define CERES_PREVENT_MACRO_SUBSTITUTION // Yes, it's empty + #endif // CERES_PUBLIC_INTERNAL_PORT_H_ diff --git a/include/ceres/jet.h b/include/ceres/jet.h index 10ce51e70..fba1e2ab6 100644 --- a/include/ceres/jet.h +++ b/include/ceres/jet.h @@ -443,15 +443,9 @@ using std::fmin; using std::fpclassify; using std::hypot; using std::isfinite; -using std::isgreater; -using std::isgreaterequal; using std::isinf; -using std::isless; -using std::islessequal; -using std::islessgreater; using std::isnan; using std::isnormal; -using std::isunordered; using std::log; using std::log10; using std::log1p; @@ -465,6 +459,47 @@ using std::sqrt; using std::tan; using std::tanh; +// MSVC (up to 1930) defines quiet comparison functions as template functions +// which causes compilation errors due to ambiguity in the template parameter +// type resolution for using declarations in the ceres namespace. Workaround the +// issue by defining specific overload and bypass MSVC standard library +// definitions. +#if defined(_MSC_VER) +inline bool isgreater(double lhs, + double rhs) noexcept(noexcept(std::isgreater(lhs, rhs))) { + return std::isgreater(lhs, rhs); +} +inline bool isless(double lhs, + double rhs) noexcept(noexcept(std::isless(lhs, rhs))) { + return std::isless(lhs, rhs); +} +inline bool islessequal(double lhs, + double rhs) noexcept(noexcept(std::islessequal(lhs, + rhs))) { + return std::islessequal(lhs, rhs); +} +inline bool isgreaterequal(double lhs, double rhs) noexcept( + noexcept(std::isgreaterequal(lhs, rhs))) { + return std::isgreaterequal(lhs, rhs); +} +inline bool islessgreater(double lhs, double rhs) noexcept( + noexcept(std::islessgreater(lhs, rhs))) { + return std::islessgreater(lhs, rhs); +} +inline bool isunordered(double lhs, + double rhs) noexcept(noexcept(std::isunordered(lhs, + rhs))) { + return std::isunordered(lhs, rhs); +} +#else +using std::isgreater; +using std::isgreaterequal; +using std::isless; +using std::islessequal; +using std::islessgreater; +using std::isunordered; +#endif + #ifdef CERES_HAS_CPP20 using std::lerp; using std::midpoint; @@ -1246,8 +1281,9 @@ struct numeric_limits> { static constexpr bool tinyness_before = std::numeric_limits::tinyness_before; - static constexpr ceres::Jet min() noexcept { - return ceres::Jet(std::numeric_limits::min()); + static constexpr ceres::Jet min + CERES_PREVENT_MACRO_SUBSTITUTION() noexcept { + return ceres::Jet((std::numeric_limits::min)()); } static constexpr ceres::Jet lowest() noexcept { return ceres::Jet(std::numeric_limits::lowest()); @@ -1271,8 +1307,9 @@ struct numeric_limits> { return ceres::Jet(std::numeric_limits::denorm_min()); } - static constexpr ceres::Jet max() noexcept { - return ceres::Jet(std::numeric_limits::max()); + static constexpr ceres::Jet max + CERES_PREVENT_MACRO_SUBSTITUTION() noexcept { + return ceres::Jet((std::numeric_limits::max)()); } }; @@ -1326,8 +1363,8 @@ struct NumTraits> { }; }; - static inline Real highest() { return Real(std::numeric_limits::max()); } - static inline Real lowest() { return Real(-std::numeric_limits::max()); } + static inline Real highest() { return Real((std::numeric_limits::max)()); } + static inline Real lowest() { return Real(-(std::numeric_limits::max)()); } }; // Specifying the return type of binary operations between Jets and scalar types diff --git a/include/ceres/manifold.h b/include/ceres/manifold.h index 14a5792cc..458eec059 100644 --- a/include/ceres/manifold.h +++ b/include/ceres/manifold.h @@ -285,6 +285,9 @@ class CERES_EXPORT ProductManifold final : public Manifold { public: ProductManifold(const ProductManifold&) = delete; ProductManifold& operator=(const ProductManifold&) = delete; + // NOTE: Do not remove the trivial destructor as this will cause linker errors + // in MSVC builds. + ~ProductManifold() override; // NOTE: The constructor takes ownership of the input // manifolds. @@ -307,7 +310,7 @@ class CERES_EXPORT ProductManifold final : public Manifold { ManifoldPtr& manifold = manifolds_[i]; manifold = std::move(manifolds_array[i]); - buffer_size_ = std::max( + buffer_size_ = (std::max)( buffer_size_, manifold->TangentSize() * manifold->AmbientSize()); ambient_size_ += manifold->AmbientSize(); tangent_size_ += manifold->TangentSize(); diff --git a/include/ceres/tiny_solver.h b/include/ceres/tiny_solver.h index 16fd2a84a..86a655dc0 100644 --- a/include/ceres/tiny_solver.h +++ b/include/ceres/tiny_solver.h @@ -250,7 +250,7 @@ class TinySolver { const Scalar max_diagonal = 1e32; for (int i = 0; i < lm_diagonal_.rows(); ++i) { lm_diagonal_[i] = std::sqrt( - u * std::min(std::max(jtj_(i, i), min_diagonal), max_diagonal)); + u * (std::min)((std::max)(jtj_(i, i), min_diagonal), max_diagonal)); jtj_regularized_(i, i) += lm_diagonal_[i] * lm_diagonal_[i]; } @@ -306,7 +306,7 @@ class TinySolver { } Scalar tmp = Scalar(2 * rho - 1); - u = u * std::max(Scalar(1 / 3.), 1 - tmp * tmp * tmp); + u = u * (std::max)(Scalar(1 / 3.), Scalar(1) - tmp * tmp * tmp); v = 2; } else { diff --git a/internal/ceres/CMakeLists.txt b/internal/ceres/CMakeLists.txt index ea0bed865..fa538fed4 100644 --- a/internal/ceres/CMakeLists.txt +++ b/internal/ceres/CMakeLists.txt @@ -287,7 +287,7 @@ if(BUILD_SHARED_LIBS) # CERES_STATIC_DEFINE is generated by the GenerateExportHeader CMake module # used to autogerate export.h. The macro should not be renamed without # updating the corresponding generate_export_header invocation. - target_compile_definitions(ceres_static PRIVATE CERES_STATIC_DEFINE) + target_compile_definitions(ceres_static PUBLIC CERES_STATIC_DEFINE) else() # In a static library build, not additional access layer is necessary as all # symbols are visible. @@ -399,14 +399,6 @@ install(TARGETS ceres ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}) if (BUILD_TESTING AND GFLAGS) - add_library(test_util STATIC - evaluator_test_utils.cc - numeric_diff_test_utils.cc - test_util.cc) - - target_include_directories(test_util PUBLIC ${Ceres_SOURCE_DIR}/internal) - target_link_libraries (test_util PUBLIC ceres_static gflags) - add_library(gtest gmock_gtest_all.cc gmock_main.cc) if (BUILD_SHARED_LIBS) # Define gtest-specific shared library flags for compilation. @@ -417,12 +409,21 @@ if (BUILD_TESTING AND GFLAGS) target_compile_definitions(gtest PRIVATE GTEST_CREATE_SHARED_LIBRARY=1 GFLAGS_DLL_DEFINE_FLAG=GTEST_API_ - INTERFACE GTEST_LINKED_AS_SHARED_LIBRARY=1) + INTERFACE GTEST_LINKED_AS_SHARED_LIBRARY=1 + GFLAGS_DLL_DECLARE_FLAG=GTEST_API_) endif() target_include_directories(gtest PRIVATE ${Ceres_SOURCE_DIR}/internal/ceres) target_link_libraries(gtest PRIVATE Ceres::ceres gflags) + add_library(test_util STATIC + evaluator_test_utils.cc + numeric_diff_test_utils.cc + test_util.cc) + + target_include_directories(test_util PUBLIC ${Ceres_SOURCE_DIR}/internal) + target_link_libraries (test_util PUBLIC ceres_static gflags gtest) + macro (CERES_TEST NAME) add_executable(${NAME}_test ${NAME}_test.cc) # Pull in local headers from the generated test directories when ceres_test() diff --git a/internal/ceres/autodiff_benchmarks/CMakeLists.txt b/internal/ceres/autodiff_benchmarks/CMakeLists.txt index 610ebc334..99af15268 100644 --- a/internal/ceres/autodiff_benchmarks/CMakeLists.txt +++ b/internal/ceres/autodiff_benchmarks/CMakeLists.txt @@ -9,9 +9,3 @@ endif() add_executable(autodiff_benchmarks autodiff_benchmarks.cc) add_dependencies_to_benchmark(autodiff_benchmarks) target_compile_options(autodiff_benchmarks PRIVATE ${CERES_BENCHMARK_FLAGS}) - -# All other flags + fast-math -list(APPEND CERES_BENCHMARK_FAST_MATH_FLAGS ${CERES_BENCHMARK_FLAGS} "-ffast-math") -add_executable(autodiff_benchmarks_fast_math autodiff_benchmarks.cc) -add_dependencies_to_benchmark(autodiff_benchmarks_fast_math) -target_compile_options(autodiff_benchmarks_fast_math PRIVATE ${CERES_BENCHMARK_FAST_MATH_FLAGS}) diff --git a/internal/ceres/dense_cholesky_test.cc b/internal/ceres/dense_cholesky_test.cc index 0e842074f..c5189b8cf 100644 --- a/internal/ceres/dense_cholesky_test.cc +++ b/internal/ceres/dense_cholesky_test.cc @@ -97,19 +97,25 @@ TEST_P(DenseCholeskyTest, FactorAndSolve) { } } -INSTANTIATE_TEST_SUITE_P(_, - DenseCholeskyTest, - ::testing::Values(EIGEN +namespace { + +// NOTE: preprocessor directives in a macro are not standard conforming +decltype(auto) MakeValues() { + return ::testing::Values(EIGEN #ifndef CERES_NO_LAPACK - , - LAPACK + , + LAPACK #endif #ifndef CERES_NO_CUDA - , - CUDA + , + CUDA #endif - ), - ParamInfoToString); + ); +} + +} // namespace + +INSTANTIATE_TEST_SUITE_P(_, DenseCholeskyTest, MakeValues(), ParamInfoToString); } // namespace internal } // namespace ceres diff --git a/internal/ceres/jet_test.cc b/internal/ceres/jet_test.cc index f06c599e5..6a76e1d55 100644 --- a/internal/ceres/jet_test.cc +++ b/internal/ceres/jet_test.cc @@ -30,7 +30,11 @@ // The floating-point environment access and modification is only meaningful // with the following pragma. +#ifdef _MSC_VER +#pragma fenv_access(on) +#else #pragma STDC FENV_ACCESS ON +#endif #include "ceres/jet.h" diff --git a/internal/ceres/levenberg_marquardt_strategy_test.cc b/internal/ceres/levenberg_marquardt_strategy_test.cc index 0e7ec8dc0..86d35a254 100644 --- a/internal/ceres/levenberg_marquardt_strategy_test.cc +++ b/internal/ceres/levenberg_marquardt_strategy_test.cc @@ -148,7 +148,7 @@ TEST(LevenbergMarquardtStrategy, CorrectDiagonalToLinearSolver) { // are versions of glog which are not in the google namespace. using namespace google; -#if defined(_MSC_VER) +#if defined(GLOG_NO_ABBREVIATED_SEVERITIES) // Use GLOG_WARNING to support MSVC if GLOG_NO_ABBREVIATED_SEVERITIES // is defined. EXPECT_CALL(log, diff --git a/internal/ceres/manifold.cc b/internal/ceres/manifold.cc index 451286a41..1f4781eae 100644 --- a/internal/ceres/manifold.cc +++ b/internal/ceres/manifold.cc @@ -312,6 +312,20 @@ bool SubsetManifold::MinusJacobian(const double* x, return true; } +// While the trivial destructor is not necessary, MSVC (1930) complains in +// shared library builds by issuing a linker error: +// +// bundle_adjuster.obj : error LNK2019: unresolved external symbol +// "__declspec(dllimport) const ceres::ProductManifold::`vftable'" +// (__imp_??_7ProductManifold@ceres@@6B@) referenced in function "public: +// __cdecl ceres::ProductManifold::ProductManifold(class +// ceres::QuaternionManifold *,class ceres::EuclideanManifold *)" +// (??$?0VQuaternionManifold@ceres@@VEuclideanManifold@1@@ProductManifold@ceres@@QEAA@PEAVQuaternionManifold@1@PEAVEuclideanManifold@1@@Z) +// +// Providing a trivial implementation here resolves the issue. +ProductManifold::~ProductManifold() = default; + int ProductManifold::AmbientSize() const { return ambient_size_; } int ProductManifold::TangentSize() const { return tangent_size_; } diff --git a/internal/ceres/miniglog/glog/logging.h b/internal/ceres/miniglog/glog/logging.h index f03c9140f..5604bd46c 100644 --- a/internal/ceres/miniglog/glog/logging.h +++ b/internal/ceres/miniglog/glog/logging.h @@ -132,7 +132,7 @@ const int FATAL = ::FATAL; // added, all log output is also sent to each sink through the send function. // In this implementation, WaitTillSent() is called immediately after the send. // This implementation is not thread safe. -class CERES_NO_EXPORT LogSink { +class CERES_EXPORT LogSink { public: virtual ~LogSink() = default; virtual void send(LogSeverity severity, @@ -146,7 +146,7 @@ class CERES_NO_EXPORT LogSink { }; // Global set of log sinks. The actual object is defined in logging.cc. -extern CERES_NO_EXPORT std::set log_sinks_global; +extern CERES_EXPORT std::set log_sinks_global; inline void InitGoogleLogging(const char* /* argv */) { // Do nothing; this is ignored. @@ -169,7 +169,7 @@ inline void RemoveLogSink(LogSink* sink) { log_sinks_global.erase(sink); } // defined, output is directed to std::cerr. This class should not // be directly instantiated in code, rather it should be invoked through the // use of the log macros LG, LOG, or VLOG. -class CERES_NO_EXPORT MessageLogger { +class CERES_EXPORT MessageLogger { public: MessageLogger(const char* file, int line, const char* tag, int severity) : file_(file), line_(line), tag_(tag), severity_(severity) { @@ -288,7 +288,7 @@ class CERES_NO_EXPORT MessageLogger { // This class is used to explicitly ignore values in the conditional // logging macros. This avoids compiler warnings like "value computed // is not used" and "statement has no effect". -class CERES_NO_EXPORT LoggerVoidify { +class CERES_EXPORT LoggerVoidify { public: // This has to be an operator with a precedence lower than << but // higher than ?: diff --git a/internal/ceres/rotation_test.cc b/internal/ceres/rotation_test.cc index d97e38588..d6de650c3 100644 --- a/internal/ceres/rotation_test.cc +++ b/internal/ceres/rotation_test.cc @@ -88,27 +88,29 @@ MATCHER(IsNormalizedQuaternion, "") { MATCHER_P(IsNearQuaternion, expected, "") { // Quaternions are equivalent upto a sign change. So we will compare // both signs before declaring failure. - bool near = true; + bool is_near = true; + // NOTE: near (and far) can be defined as macros on the Windows platform (for + // ancient pascal calling convention). Do not use these identifiers. for (int i = 0; i < 4; i++) { if (fabs(arg[i] - expected[i]) > kTolerance) { - near = false; + is_near = false; break; } } - if (near) { + if (is_near) { return true; } - near = true; + is_near = true; for (int i = 0; i < 4; i++) { if (fabs(arg[i] + expected[i]) > kTolerance) { - near = false; + is_near = false; break; } } - if (near) { + if (is_near) { return true; }