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

Support for MSVC compilers #46

Open
wants to merge 17 commits into
base: 35-investigate-migrating-to-meson
Choose a base branch
from

Conversation

ErichZimmer
Copy link
Contributor

@ErichZimmer ErichZimmer commented Apr 4, 2024

Preface

By default, MSVC compilers cannot be used to compile shared/dynamic libraries since symbols are not exported by default. However, manually exporting symbols if a MSVC compiler is detected is relatively straight forward and is covered in this pull request. To support MSVC, necessary classes and members of classes are exported using preprocessor directives and conforming to C4251. (e.g., no dll interface warnings). As such, openpivcore compiles using VS 2019 and VS 2022 without the use of SIMD (pocketfft uses GNU vector extensions which has no MSVC port).

New:

  • openpiv/core/dllexports.h defines export macro

Changes:

  • openpiv/core/rect.h now exports necessary public class members
  • openpiv/loaders/image_loader.h now exports necessary public class members
  • openpiv/loaders/pnm_image_loader.h now exports necessary public class members
  • openpiv/loaders/tiff_image_loader.h now exports necessary public class members
  • README.md Some minor enhancements

Work in Progress:

  • Support MSVC compilers
  • Fix Linux build
  • Add MSVC compilers to CI testing

@ErichZimmer ErichZimmer added the feature adding a new feature to the code label Apr 4, 2024
@ErichZimmer
Copy link
Contributor Author

Currently, the Linux tests have the following error:

image_utils_test - pnm_load_save_test
-------------------------------------------------------------------------------
../tests/image_utils_test.cpp:55
...............................................................................
../tests/image_utils_test.cpp:92: FAILED:
REQUIRE( im == reloaded )
with expansion:
image<g<uint16_t>>[(0,0) -> [2560,1230]][0x7ffe654f85a0] data @
0x7f0cd9aad010
==
image<g<uint16_t>>[(0,0) -> [2560,1230]][0x7ffe654f85d0] data @
0x7f0cd94ab010
min: g0, max: g250, scale: 262
[1711951144551534893] (139693195174656) INFO: registered: image/x-portable-anymap
[1711951144551546976] (139693195174656) INFO: registered: image/tiff
[1711951144578343752] (139693195174656) DEBUG: PNM� [2560, 1[230](https://github.com/ErichZimmer/openpiv-c--
qt/actions/runs/8504472916/job/23291430344#step:8:231)], 65535
[1711951144610999082] (139693195174656) DEBUG: image: 0x7ffe654f89a0
[1711951144612776143] (139693195174656) DEBUG: peaks: [image_view<g>[(59,59) -> [3,3]][0x7ffe654f89a0] data
@ 0x5616ce6b4420, image_view<g>[(49,49) -> [3,3]][0x7ffe654f89a0] data @ 0x5616ce6b4420,
image_view<g>[(39,39) -> [3,3]][0x7ffe654f89a0] data @ 0x5616ce6b4420]

I have no idea why the current branch does not have this issue and my branch does, even after spending the better half of today on this issue. The build logs are also practically identical...

@ErichZimmer
Copy link
Contributor Author

All tests now pass. It seams that using multi-threaded testing procedures were the cause of these errors.

@ErichZimmer ErichZimmer changed the title Support for MSVC compilers [WIP] Support for MSVC compilers Apr 7, 2024
Some restructuring to make the flow of the READEME file better.
The dependencies section was not clear enough on compilers and python. It made it look like meson were installing python and c++ compilers as dependencies.
 - Revise MinGW compiler statement on Windows build environment
- Add Intec oneAPI to known working Windows c/c++ compilers
- Revise statement on using Windows build environment in Unix-like systems
 - Spelling Mistakes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature adding a new feature to the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants