Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for compile errors in GCC 12 #824

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Conversation

vdobro
Copy link

@vdobro vdobro commented Jun 5, 2022

Currently it is impossible to compile openMVS with gcc 12 without the following fixes:

  • Add Common or IO to libraries to link against in some of the targets in /apps
  • Adding -fpermissive to Common because of a discarded const qualifier in Common/Types.inl:3062. An alternative would require all of its consumers to drop const qualifiers
  • linking against Eigen using its own supplied config for find_package
  • Adjusting to updated symbol and file names from openMVG

ADD_SUBDIRECTORY(InterfaceCOLMAP)
ADD_SUBDIRECTORY(InterfaceMetashape)
ADD_SUBDIRECTORY(DensifyPointCloud)
ADD_SUBDIRECTORY(InterfaceOpenMVG)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This 2 interfaces are deprecated, pls remove them;
OpenMVG has its own interface to OpenMVS
VisualSFM has accuracy issues, should not be used

if(EIGEN3_FOUND)
LIST(APPEND OpenMVS_EXTRA_INCLUDES ${EIGEN3_INCLUDE_DIR})
INCLUDE_DIRECTORIES(${EIGEN3_INCLUDE_DIR})
find_package(Eigen3 3.4 REQUIRED)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls test these chages with older linux distribution and windows and mac too

@@ -5,7 +5,7 @@ else()
endif()
FILE(GLOB LIBRARY_FILES_H "*.h" "*.inl")

cxx_executable_with_flags(DensifyPointCloud "Apps" "${cxx_default}" "MVS;${OpenMVS_EXTRA_LIBS}" ${LIBRARY_FILES_C} ${LIBRARY_FILES_H})
cxx_executable_with_flags(DensifyPointCloud "Apps" "${cxx_default}" "MVS;Common;${OpenMVS_EXTRA_LIBS}" ${LIBRARY_FILES_C} ${LIBRARY_FILES_H})
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should not be needed, Common is linked to MVS; pls find out why this is needed here and it worked fine will all linux distros till now

@@ -5,7 +5,7 @@ else()
endif()
FILE(GLOB LIBRARY_FILES_H "*.h" "*.inl")

cxx_executable_with_flags(InterfaceCOLMAP "Apps" "${cxx_default}" "MVS;${OpenMVS_EXTRA_LIBS}" ${LIBRARY_FILES_C} ${LIBRARY_FILES_H})
cxx_executable_with_flags(InterfaceCOLMAP "Apps" "${cxx_default}" "MVS;IO;${OpenMVS_EXTRA_LIBS}" ${LIBRARY_FILES_C} ${LIBRARY_FILES_H})
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

@@ -38,7 +38,9 @@
#undef R2D
#include <openMVG/sfm/sfm_data.hpp>
#include <openMVG/sfm/sfm_data_io.hpp>
#include <openMVG/image/image.hpp>
#include <openMVG/image/image_io.hpp>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool, but the interface should be disabled anyway

@@ -451,7 +451,6 @@ macro(optimize_default_compiler_settings)
add_extra_compiler_option(-Werror=sequence-point)
add_extra_compiler_option(-Wformat)
add_extra_compiler_option(-Werror=format-security -Wformat)
add_extra_compiler_option(-Wstrict-prototypes)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls explain

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In comment below you can see that compiler is producing a warning:

cc1plus: warning: command-line option ‘-Wstrict-prototypes’ is valid for C/ObjC but not for C++

@@ -13,6 +13,7 @@ endif()

# Link its dependencies
TARGET_LINK_LIBRARIES(Common ${Boost_LIBRARIES} ${OpenCV_LIBS})
TARGET_COMPILE_OPTIONS(Common INTERFACE -fpermissive)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls detail, I did not understand the comment from the PR; at line 3062 all looks fine to me:

			fImage.write(cv::Mat::template ptr<const float>(--i), rowbytes);

Copy link

@bialasjaroslaw bialasjaroslaw Jun 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunatelly either new version of OpenCV has some kind of a bug or GCC12 is behaving differently and selecting non const template version of a ptr function (line 713 of mat.inl.hpp) and though const qualifier is not respected for this. I can paste exact error message if you are able to help with that. I tried to solve this myself, but I am not so smart, so I removed this code, as I am not using it anywhere (PFM image files). As I understand -fpermissive will remove such restrictions.

[ 24%] Building CXX object libs/IO/CMakeFiles/IO.dir/OBJ.cpp.o
cc1plus: warning: command-line option ‘-Wstrict-prototypes’ is valid for C/ObjC but not for C++
In file included from /home/jbialas/openMVS/libs/Common/Types.h:2789,
   			 from /home/jbialas/openMVS/libs/Common/Common.h:176,
   			 from /home/jbialas/openMVS/libs/IO/Common.h:18,
   			 from /home/jbialas/openMVS_build/libs/IO/CMakeFiles/IO.dir/cmake_pch.hxx:5,
   			 from <command-line>:
/home/jbialas/openMVS/libs/Common/Types.inl: In instantiation of ‘bool SEACAVE::TImage<TYPE>::Save(const SEACAVE::String&) const [with TYPE = SEACAVE::TPixel<unsigned char>]’:
/home/jbialas/openMVS/libs/IO/OBJ.cpp:77:39:   required from here
/home/jbialas/openMVS/libs/Common/Types.inl:3062:72: error: passing ‘const SEACAVE::TImage<SEACAVE::TPixel<unsigned char> >’ as ‘this’ argument discards qualifiers [-fpermissive]
3062 |						 fImage.write(cv::Mat::template ptr<const float>(--i), rowbytes);
     |									  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~
In file included from /usr/include/opencv4/opencv2/core/mat.hpp:3773,
   			 from /usr/include/opencv4/opencv2/core.hpp:58,
   			 from /usr/include/opencv4/opencv2/opencv.hpp:52,
   			 from /home/jbialas/openMVS/libs/Common/Types.h:143:
/usr/include/opencv4/opencv2/core/mat.inl.hpp:713:6: note:   in call to ‘_Tp* cv::Mat::ptr(int) [with _Tp = const float]’
 713 | _Tp* Mat::ptr(int y)
     |	  ^~~
make[2]: *** [libs/IO/CMakeFiles/IO.dir/build.make:237: libs/IO/CMakeFiles/IO.dir/OBJ.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:405: libs/IO/CMakeFiles/IO.dir/all] Error 2
make: *** [Makefile:136: all] Error 2
 

Copy link

@alkavan alkavan Jul 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cdcseacave I'm having the same issue on Fedora 36 and gcc 12.1.1 20220507 (Red Hat 12.1.1-1) when building the v2.0.1 tag, with boost 1.76 and and latest vcglib.

OPENMVS_VERSION="2.0.1"

# Configure CMake
cmake "../third_party/openMVS-${OPENMVS_VERSION}" \
  -DCMAKE_BUILD_TYPE=Release \
  -DBUILD_SHARED_LIBS=OFF \
  -DCMAKE_INSTALL_PREFIX=/opt/3dr \
  -DBOOST_ROOT="../third_party/boost_1_76_0/stage" \
  -DVCG_ROOT="../third_party/vcglib" \
  -DBoost_USE_STATIC_LIBS=ON

Would be happy to provide more information so we can solve this!

@alkavan
Copy link

alkavan commented Jul 23, 2022

I've tried to build with OpenCV 3.4.18 and 4.6.0 that I compiled externally, and with OpenCV 4.5.5 that I have on my system, in all cases I still get that error.

I also tried to add this line:

TARGET_COMPILE_OPTIONS(Common INTERFACE -fpermissive)

to this file: libs/Common/CMakeLists.txt like suggested here, but that didn't seemed to help.

This does feel like a GCC 12 related error, since I know that last time I built successfully was on Fedora 35 and that was with GCC 11. I can't understand what exactly is going on, I did dig on the internet that this probably means that cv::Mat::template ptr<const float>(--i) missing a const qualifier...

@cdcseacave @bialasjaroslaw

Errors

In file included from ./openMVS-2.0.1/apps/TextureMesh/../../libs/MVS/../Common/Types.h:2789,
                 from ./openMVS-2.0.1/apps/TextureMesh/../../libs/MVS/../Common/Common.h:176,
                 from ./openMVS-2.0.1/apps/TextureMesh/../../libs/MVS/Common.h:42,
                 from ./openMVS-2.0.1/apps/TextureMesh/TextureMesh.cpp:32:
./openMVS-2.0.1/apps/TextureMesh/../../libs/MVS/../Common/Types.inl: In instantiation of ‘bool SEACAVE::TImage<TYPE>::Save(const SEACAVE::String&) const [with TYPE = SEACAVE::TColor<unsigned char>]’:
./openMVS-2.0.1/apps/TextureMesh/TextureMesh.cpp:327:13:   required from here
./openMVS-2.0.1/apps/TextureMesh/../../libs/MVS/../Common/Types.inl:3062:72: error: passing ‘const SEACAVE::TImage<SEACAVE::TColor<unsigned char> >’ as ‘this’ argument discards qualifiers [-fpermissive]
 3062 |                         fImage.write(cv::Mat::template ptr<const float>(--i), rowbytes);
In file included from ./openMVS-2.0.1/apps/InterfaceMetashape/../../libs/MVS/../Common/Types.h:2789,
                 from ./openMVS-2.0.1/apps/InterfaceMetashape/../../libs/MVS/../Common/Common.h:176,
                 from ./openMVS-2.0.1/apps/InterfaceMetashape/../../libs/MVS/Common.h:42,
                 from ./openMVS-2.0.1/apps/InterfaceMetashape/InterfaceMetashape.cpp:32:
./openMVS-2.0.1/apps/InterfaceMetashape/../../libs/MVS/../Common/Types.inl: In instantiation of ‘bool SEACAVE::TImage<TYPE>::Save(const SEACAVE::String&) const [with TYPE = SEACAVE::TPixel<unsigned char>]’:
./openMVS-2.0.1/apps/InterfaceMetashape/InterfaceMetashape.cpp:503:29:   required from here
./openMVS-2.0.1/apps/InterfaceMetashape/../../libs/MVS/../Common/Types.inl:3062:72: error: passing ‘const SEACAVE::TImage<SEACAVE::TPixel<unsigned char> >’ as ‘this’ argument discards qualifiers [-fpermissive]
 3062 |                         fImage.write(cv::Mat::template ptr<const float>(--i), rowbytes);

@alkavan
Copy link

alkavan commented Jul 23, 2022

Adding add_compile_options(-fpermissive) to the main CMakeLists.txt file of the project did do the job!

./openMVS-2.0.1/apps/TextureMesh/../../libs/MVS/../Common/Types.inl:3062:72: warning: passing ‘const SEACAVE::TImage<SEACAVE::TColor<unsigned char> >’ as ‘this’ argument discards qualifiers [-fpermissive]
 3062 |                         fImage.write(cv::Mat::template ptr<const float>(--i), rowbytes);

I specific issue seems to be here in /usr/include/opencv4/opencv2/core/mat.inl.hpp:

template<typename _Tp, int n> inline
const _Tp* Mat::ptr(const Vec<int, n>& idx) const
{
    CV_DbgAssert( elemSize() == sizeof(_Tp) );
    return Mat::ptr<_Tp>(idx.val);
}

That const qualifier at the end?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants