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

Feature cmake basic structure for compilation #19

Merged
merged 29 commits into from
Sep 24, 2023
Merged

Conversation

kalwalt
Copy link
Member

@kalwalt kalwalt commented Jul 27, 2023

CMAKE implementation

Preamble

I will provide a minimal cmake implementation to compile the library.
If the library can be compiled with cmake i will provide also some basic test with googletest as i already started.

Final considerations

The library now can be compiled with the CMakeLists.txt file in the WebARKit directory. The result will be the WebARKitLib static lib (libWebARKitLib.a). The static lib is used to make some basic test, i would improve this, adding more of them. I haven't tested on Windows or Mac, i used the linux environment for simplicity. I haven't ttried to compile the libWebARKitLib.a with Emscripten ( with emcmake) but i think the cmake script should be a bit different, because we link the 3dparty zlib (included with OpenCV) but Emscripten has its own zlib. Anyway this will be space for another PR.

@kalwalt kalwalt added enhancement New feature or request C/C++ code concerning the C/C++ code design and improvements labels Jul 27, 2023
@kalwalt kalwalt self-assigned this Jul 30, 2023
@kalwalt
Copy link
Member Author

kalwalt commented Aug 1, 2023

I have tested under Windows and i always i get a SEH error, maybe is related to the platform. I made some improvements in the code that i will commit soon, but for now i'm testing under Ubuntu(WSL) to see if i get the same error or not.

@kalwalt
Copy link
Member Author

kalwalt commented Aug 1, 2023

Under Ubuntu(WSL) i got a similar error:

./hello_test
Running main() from /mnt/c/Users/perda/kalwalt-github/webarkit-testing/emscripten/WebARKitLib/tests/build/_deps/googletest-src/googletest/src/gtest_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from HelloTest
[ RUN      ] HelloTest.BasicAssertions
[info] Webarkit C++ lib v1.0.0 initalised.
free(): invalid size
Aborted

I think the issue is in the destructor and the versionString variable. I will try to fix this.

@kalwalt
Copy link
Member Author

kalwalt commented Aug 1, 2023

with commit d292c57 i solved the issue see the log running the test:

./hello_test
Running main() from /mnt/c/Users/perda/kalwalt-github/webarkit-testing/emscripten/WebARKitLib/tests/build/_deps/googletest-src/googletest/src/gtest_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from InitTest
[ RUN      ] InitTest.InitialiseBaseTest
[info] Webarkit C++ lib v1.0.0 initalised.
[       OK ] InitTest.InitialiseBaseTest (0 ms)
[----------] 1 test from InitTest (0 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (1 ms total)
[  PASSED  ] 1 test.

I need to test under windows but i think it will be fine. I have to change some cmake target includes because they are pointing to opencv and opencv_contrib folder cloned with webarkit_testing repository (they are submodules in there), instead they should be related to them from cmake FetchContent.

@kalwalt
Copy link
Member Author

kalwalt commented Sep 14, 2023

I'm not sure that cmake FetchContent is the best approach for compiling a lib, and i'm not sure that it always compile correctly, at least on Windows. Most of the time i had to re run the cmage configuration step to avoid missing includes from xfeatuers2d module.
I will make other tests and i can also prepare a github action script to run the test.
Based on these tests i will conider to opt to another approach: create opencv and opencv_contrib git submodule in this repository and adequate the cmake script, easier solution but has the downside to increase the size of the repository.

@kalwalt
Copy link
Member Author

kalwalt commented Sep 14, 2023

Now the action cript can run the build (cmake) but fails with this message:

fatal error: opencv2/xfeatures2d.hpp: No such file or directory
    6 | #include <opencv2/xfeatures2d.hpp>

this is the same message as i encountered compiling locally. i will see if this can be solved or i will found another solution.

@kalwalt
Copy link
Member Author

kalwalt commented Sep 14, 2023

from the action github log:
To be built: calib3d core features2d flann imgcodecs imgproc java video
you cam see that xfeatures2d is not added even i added twice here:

set(BUILD_opencv_xfeatures2d ON)

and:
set(BUILD_opencv_xfeatures2d ON)

and also the OPENCV_EXTRA_MODULES_PATH:
set(OPENCV_EXTRA_MODULES_PATH ../opencv_contrib-src/modules/)

and:
set(OPENCV_EXTRA_MODULES_PATH ../${opencv_contrib_SOURCE_DIR}/modules/)

@kalwalt
Copy link
Member Author

kalwalt commented Sep 15, 2023

Now it build process fails at the final stage:

[ 98%] Linking CXX executable bin/webarkit_test
/usr/bin/ld: cannot find -lopencv_xfeatures2d: No such file or directory
collect2: error: ld returned 1 exit status
make[2]: *** [CMakeFiles/webarkit_test.dir/build.make:121: bin/webarkit_test] Error 1
make[1]: *** [CMakeFiles/Makefile2:1148: CMakeFiles/webarkit_test.dir/all] Error 2

this because the xfeatures2d static lib is not generated.

@kalwalt
Copy link
Member Author

kalwalt commented Sep 17, 2023

I can't find a solution to the last issue: with FetchContent i can't compile the xfeatures2d external module, maybe this is possible but i find very complicated. As i did for webrakit-testing i will create opencv and opencv_contrib git submodules and the libs will be compiled as i already did. The advantage with the FetchContent proces was to avoid to grow the size of the repository but i think better to create a strong basis for the future development.

@kalwalt
Copy link
Member Author

kalwalt commented Sep 17, 2023

So inside the WebARKitLib folder i will create a deps (dependencies folder) where opencv and opencv_contrib will be cloned as submodules. Of course our opencv and opencv_contrib forks because we did our modifications in the code.

@kalwalt
Copy link
Member Author

kalwalt commented Sep 17, 2023

I will make a step back because opencv + opencv_contrib are more than one Gb when downloaded, so maybe i will found an alternative option. I can provide binaries and static libs as i already did in opencv-em for other projects. I can develop a script and a github action to upload precompiled libs and headers for opencv and opencv_contrib with opencv-em and then when someone wants compile WebARKitLib will download these.

@kalwalt
Copy link
Member Author

kalwalt commented Sep 23, 2023

i can build opencv builds with opencv-em webarkit/opencv-em#6 i have tested the tests with the new opencv pre-binaries, see this gist
Now i will make a new opencv-em release so it can be downloaded as a .zip file, and maybe we can grab with FetcContent? or at least populated with a shell script...

@kalwalt
Copy link
Member Author

kalwalt commented Sep 24, 2023

Tests failing because the last command was for Windows OS, i will fix and surely it will work.

@kalwalt
Copy link
Member Author

kalwalt commented Sep 24, 2023

Now, i think this PR is on the good track to be merged.

@kalwalt
Copy link
Member Author

kalwalt commented Sep 24, 2023

During the tests phase we get this warning:

CMake Warning (dev) at /home/kalwalt/.local/lib/python3.10/site-packages/cmake/data/share/cmake-3.27/Modules/FetchContent.cmake:1316 (message):
  The DOWNLOAD_EXTRACT_TIMESTAMP option was not given and policy CMP0135 is
  not set.  The policy's OLD behavior will be used.  When using a URL
  download, the timestamps of extracted files should preferably be that of
  the time of extraction, otherwise code that depends on the extracted
  contents might not be rebuilt if the URL changes.  The OLD behavior
  preserves the timestamps from the archive instead, but this is usually not
  what you want.  Update your project to the NEW behavior or specify the
  DOWNLOAD_EXTRACT_TIMESTAMP option with a value of true to avoid this
  robustness issue.
Call Stack (most recent call first):
  /mnt/c/Users/perda/kalwalt-github/webarkit-testing/emscripten/WebARKitLib/WebARKit/CMakeLists.txt:9 (FetchContent_Declare)      
This warning is for project developers.  Use -Wno-dev to suppress it.

I will see to solve this one and then i can merge it.

@kalwalt kalwalt merged commit 8341ec1 into dev Sep 24, 2023
1 check passed
@kalwalt kalwalt deleted the feature-cmake-improves branch June 6, 2024 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C/C++ code concerning the C/C++ code design and improvements enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant