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

Windows port #29

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

AltVanguard
Copy link

Hi,

I've made a basic Windows port of DM-VIO. I wanted to check how much I have broken in the Linux build (e.g. I most likely broke sse2neon in the CMake), but I regrettably haven't got around to it in the past months so I've decided to open this pull request so you can take a look at it as is. No hard feelings if this doesn't get merged :)

My changes fall into these 3 categories:

  • Get dependencies via Conan: This provides a consistent way to acquire dependencies and the same versions of them on all platforms. I updated the build instructions in the readme accordingly. Meanwhile, I modernized the main CMake a tiny bit (e,g, not pulling in *_INCLUDE_DIRs, and instead relying on target_link_libraries). Conan only had a newer version of GTSAM and there were some breaking API changes in it that had to be resolved.
  • Use stdlib APIs: This one was pretty straightforward. No new thirdparty dependencies, all Linux APIs were replaced with their standard counterparts.
  • Fix warnings: Some warnings actually impacted the ability to link with the library under MSVC, so there are some changes that seem superficial, but were required. I left the less impactful MSVC warnings ignored in the CMake, but of course I recommend removing the ignores and eliminating the warnings themselves.

Let me know if you are interested in this port at all. If you have a specific issue with it, I'll see what I can do about it.

 - include <Eigen/LU>, otherwise Eigen::Martix<>::inverse() would be undefined
 - fix forward declares. if a struct is forward declared as a class, function symbols might be erroneously marked external in the object file
@lukasvst
Copy link
Owner

Hi,

thanks a lot, it's great to see a version that can work on Windows!

I think merging it with master could be complicated without breaking building on other systems, and unfortunately I'll be very busy the next weeks.

But I think it is already very useful to other people as is, with this PR open :)

@djblazkowicz
Copy link

djblazkowicz commented Jan 19, 2023

Hello,

I couldn't find a more appropriate discussion thread for this so I thought I'll comment here so that people interested in this might be able to find it.

I am using this fork on Windows, compiled on Windows 10 20H2 with Visual Studio 2019.

When building dm-vio, I was getting a bunch of "M_PI undeclared identifier" errors, which I was able to fix by adding _USE_MATH_DEFINES to preprocessor definitions.

With the associated fork of Pangolin 0.6, I was getting build failures due to errors in include\mpark\variant.hpp.
Related issue and fix: stevenlovegrove/Pangolin#609

When using the TUMVI datasets, I noticed that the symbolic link in the dso/cam0 folders do not work on windows, so I had to move the images folder to dso/cam0 to solve the problem.

Once all this was sorted, I was able to successfully run dmvio_dataset.exe with the following windows command line

dmvio_dataset.exe ^
files=path to dataset/dso/cam0/images ^
vignette=path to dataset/dso/cam0/vignette.png ^
imuFile=path to dataset/dso/imu.txt ^
gtFile=path to dataset/dso/gt_imu.csv ^
calib=path to dm-vio/configs/tumvi_calib/camera02.txt ^
gamma=path to dm-vio/configs/tumvi_calib/pcalib.txt ^
imuCalib=path to dm-vio/configs/tumvi_calib/camchain.yaml ^
mode=0 ^
use16Bit=1 ^
preset=0 ^
nogui=0 ^
resultsPrefix=path to results ^
settingsFile=path to dmvio/configs/tumvi.yaml ^
start=2 ^
useimu=1

I hope anyone interested may find this useful!

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.

3 participants