-
-
Notifications
You must be signed in to change notification settings - Fork 912
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
base: develop
Are you sure you want to change the base?
Conversation
apps/CMakeLists.txt
Outdated
ADD_SUBDIRECTORY(InterfaceCOLMAP) | ||
ADD_SUBDIRECTORY(InterfaceMetashape) | ||
ADD_SUBDIRECTORY(DensifyPointCloud) | ||
ADD_SUBDIRECTORY(InterfaceOpenMVG) |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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}) |
There was a problem hiding this comment.
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}) |
There was a problem hiding this comment.
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> |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls explain
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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);
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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!
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:
to this file: 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 Errors
|
Adding
I specific issue seems to be here in
That |
Currently it is impossible to compile openMVS with gcc 12 without the following fixes: