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 build #78

Merged
merged 72 commits into from Jul 16, 2019
Merged

Windows build #78

merged 72 commits into from Jul 16, 2019

Conversation

simogasp
Copy link
Member

@simogasp simogasp commented Jun 15, 2018

  • port the cpu part on windows (MSVC)
  • replace boost thread by std thread
  • replace custom code for backtrace with boost::stacktrace

@@ -17,7 +17,7 @@ bool Mgmt::Measurement::doPrint( ) const

void Mgmt::Measurement::print( std::ostream& ostr ) const
{
if( not _probe ) return;
Copy link
Member

Choose a reason for hiding this comment

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

It's not that windows doesn't support "not", it's that gcc and clang are too lax. You must include <iso646.h> to get and/not/etc. "keywords".

@simogasp
Copy link
Member Author

simogasp commented Jun 17, 2018 via email

@fabiencastan
Copy link
Member

That's cool then, personally I'd prefer to use and not or as they are more readable.

I would prefer to not mix multiple ways to do the same thing. We should use standard symbols as it's done everywhere in our libs.

But you can do it in Meshroom (in python) ;)

This was referenced Jun 20, 2018
@zvrba
Copy link
Member

zvrba commented Jun 21, 2018

I agree with Fabien, using and, or, etc. instead of standard symbols tends to obscure the code. It's not a well-known feature. (Actually, my guess is that they exist mainly to support non-ascii systems (EBCDIC) so you aren't forced to use digraphs or trigraphs.)

INSTALL.md Outdated
@@ -20,12 +20,13 @@ $ git clone https://github.com/alicevision/CCTag.git
Most of the dependencies can be installed from the common repositories (apt, yum etc):

- Eigen3 (libeigen3-dev)
- Boost >= 1.53 ([core, thread, system, filesystem, serialization, thread, exception, chrono, date-time, program-options, timer]-dev)
- Boost >= 1.53 ([accumulators, atomic, chrono, core, date-time, exception, filesystem, math, program-options, ptr-container, system, serialization, timer, thread]-dev)
Copy link
Member Author

Choose a reason for hiding this comment

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

stacktrace?

# Windows requires more work, and Mac is probably still hopeless.
cmake_minimum_required(VERSION 3.4)
# Version 3.14 is needed for boost-stacktrace
cmake_minimum_required(VERSION 3.14)
Copy link
Member Author

Choose a reason for hiding this comment

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

3.14 is overkilling, 3.5 is the first one with Boost:boost targets


set(CUDA_cpp "")

message( STATUS "Declare CCTag library" )
add_library(CCTag ${CCTag_cpp})
set_target_properties(CCTag PROPERTIES VERSION ${PROJECT_VERSION})

target_include_directories(CCTag
target_include_directories(CCTag
PUBLIC "$<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}>"
"$<INSTALL_INTERFACE:$<INSTALL_PREFIX>/include>"
PUBLIC ${Boost_INCLUDE_DIRS} ${Eigen_INCLUDE_DIR} ${OpenCV_INCLUDE_DIRS}
Copy link
Member Author

Choose a reason for hiding this comment

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

${Boost_INCLUDE_DIRS} can now be removed. When you specify the target Boost::boost in target_link_libraries() it automagically add the relevant includes and definitions

@@ -1,12 +1,12 @@
cmake_minimum_required(VERSION 3.4)
cmake_minimum_required(VERSION 3.14)
Copy link
Member Author

Choose a reason for hiding this comment

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

same here

Copy link
Member

Choose a reason for hiding this comment

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

Boost::stacktrace has been added in boost 1.65 ref and the FindBoost (versioned in CMake...) has been added in 3.13 ref. So I can eventually downgrade the requirement to 3.13.

${OpenCV_LIBS}
Boost::filesystem Boost::timer
)

if(IL_FOUND OR DevIL_FOUND)
Copy link
Member Author

Choose a reason for hiding this comment

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

maybe replace them with DevIL_FOUND

@fabiencastan fabiencastan force-pushed the devWindozeFixes branch 5 times, most recently from 4d2b9dc to ee16bc5 Compare July 16, 2019 11:16
Needed to avoid build errors with boost exception and MSVC.
@fabiencastan fabiencastan merged commit 7768c18 into develop Jul 16, 2019
@fabiencastan fabiencastan deleted the devWindozeFixes branch July 16, 2019 11:52
@simogasp simogasp added this to the V1.0.0 milestone Jul 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants