-
Notifications
You must be signed in to change notification settings - Fork 114
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
third_party dependencies #523
Changes from all commits
66c0ad0
4579ede
a80c57c
a7b9a05
f859d41
4577775
51155bc
c9dda15
00e403b
58ec421
cc25dbf
95374d9
4a4e81d
c749750
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,7 +15,7 @@ | |
project(utilities) | ||
|
||
file(GLOB_RECURSE SOURCE_FILES CONFIGURE_DEPENDS *.cpp) | ||
add_library(${PROJECT_NAME} OBJECT ${SOURCE_FILES}) | ||
add_library(${PROJECT_NAME} ${SOURCE_FILES}) | ||
|
||
message("CUDA Support: ${MANIFOLD_USE_CUDA}") | ||
message("Parallel Backend: ${MANIFOLD_PAR}") | ||
|
@@ -29,11 +29,32 @@ if(MANIFOLD_PAR STREQUAL "OMP") | |
target_compile_options(${PROJECT_NAME} PUBLIC -DMANIFOLD_PAR='O' -fopenmp) | ||
target_link_options(${PROJECT_NAME} PUBLIC -fopenmp) | ||
elseif(MANIFOLD_PAR STREQUAL "TBB") | ||
find_package(PkgConfig REQUIRED) | ||
pkg_check_modules(TBB REQUIRED tbb) | ||
target_include_directories(${PROJECT_NAME} PUBLIC ${TBB_INCLUDE_DIRS}) | ||
find_package(PkgConfig) | ||
if (PKG_CONFIG_FOUND) | ||
pkg_check_modules(TBB tbb) | ||
endif() | ||
if(NOT TBB_FOUND) | ||
message(STATUS "tbb not found, downloading from source") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💯 |
||
include(FetchContent) | ||
set(TBB_TEST OFF CACHE INTERNAL "" FORCE) | ||
set(TBB_STRICT OFF CACHE INTERNAL "" FORCE) | ||
FetchContent_Declare(TBB | ||
GIT_REPOSITORY https://github.com/oneapi-src/oneTBB.git | ||
# versions afterward forces -D_FORTIFY_SOURCE=2 | ||
# which conflicts with defaults with nixos... | ||
GIT_TAG v2021.10.0 | ||
GIT_SHALLOW TRUE | ||
GIT_PROGRESS TRUE | ||
) | ||
FetchContent_MakeAvailable(TBB) | ||
endif() | ||
target_compile_options(${PROJECT_NAME} PUBLIC -DMANIFOLD_PAR='T') | ||
target_link_libraries(${PROJECT_NAME} PUBLIC ${TBB_LINK_LIBRARIES}) | ||
if(TARGET TBB::tbb) | ||
target_link_libraries(${PROJECT_NAME} PUBLIC TBB::tbb) | ||
else() | ||
target_include_directories(${PROJECT_NAME} PUBLIC ${TBB_INCLUDE_DIRS}) | ||
target_link_libraries(${PROJECT_NAME} PUBLIC ${TBB_LINK_LIBRARIES}) | ||
endif() | ||
elseif(MANIFOLD_PAR STREQUAL "NONE") | ||
set(MANIFOLD_PAR "CPP") | ||
else() | ||
|
@@ -63,4 +84,4 @@ if(MANIFOLD_DEBUG) | |
PUBLIC -DMANIFOLD_DEBUG) | ||
endif() | ||
|
||
target_compile_features(${PROJECT_NAME} PUBLIC cxx_std_17) | ||
target_compile_features(${PROJECT_NAME} PUBLIC cxx_std_17) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,8 +14,6 @@ | |
|
||
project(manifold_test) | ||
|
||
add_subdirectory(third_party) | ||
|
||
enable_testing() | ||
|
||
set(SOURCE_FILES polygon_test.cpp cross_section_test.cpp manifold_test.cpp boolean_test.cpp sdf_test.cpp samples_test.cpp test_main.cpp) | ||
|
@@ -24,9 +22,45 @@ if(MANIFOLD_CBIND) | |
list(APPEND SOURCE_FILES manifoldc_test.cpp) | ||
endif() | ||
|
||
set(gtest_force_shared_crt ON CACHE BOOL "" FORCE) | ||
# Prevent installation of GTest with your project | ||
set(INSTALL_GTEST OFF CACHE BOOL "" FORCE) | ||
set(INSTALL_GMOCK OFF CACHE BOOL "" FORCE) | ||
|
||
find_package(GTest) | ||
if(NOT GTest_FOUND) | ||
message(STATUS "GTest not found, downloading from source") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kintel - what do you think of this approach? Would this make consuming our library easier for the likes of OpenSCAD? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks reasonable. It does make the build system depend on having access to github.com, which is a known issue from some countries, as well as for people with limited connectivity, but that's a separate challenge. For OpenSCAD we'll likely try to hit the find_package call, so this should work well for us! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes it depends on access to github by default, but if for some reason that is not possible and There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Awesome! Not familiar with FetchContent.. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice - would be good to add that trick to the README somewhere. |
||
include(FetchContent) | ||
FetchContent_Declare(googletest | ||
GIT_REPOSITORY https://github.com/google/googletest.git | ||
GIT_TAG v1.14.0 | ||
GIT_SHALLOW TRUE | ||
GIT_PROGRESS TRUE | ||
OVERRIDE_FIND_PACKAGE | ||
) | ||
FetchContent_MakeAvailable(googletest) | ||
endif() | ||
|
||
if(NOT TARGET GTest::GTest) | ||
add_library(GTest::GTest ALIAS gtest) | ||
add_library(GTest::Main ALIAS gtest_main) | ||
endif() | ||
|
||
add_executable(${PROJECT_NAME} ${SOURCE_FILES}) | ||
target_link_libraries(${PROJECT_NAME} polygon GTest::GTest manifold sdf samples samplesGPU cross_section) | ||
|
||
if (TRACY_ENABLE) | ||
include(FetchContent) | ||
FetchContent_Declare(tracy | ||
GIT_REPOSITORY https://github.com/wolfpld/tracy.git | ||
GIT_TAG v0.9.1 | ||
GIT_SHALLOW TRUE | ||
GIT_PROGRESS TRUE | ||
) | ||
FetchContent_MakeAvailable(tracy) | ||
target_link_libraries(${PROJECT_NAME} TracyClient) | ||
endif() | ||
|
||
if(MANIFOLD_CBIND) | ||
target_link_libraries(${PROJECT_NAME} manifoldc) | ||
endif() | ||
|
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.
Is there a case where this is expected, or should this be an assert?
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.
I am not entirely sure, it seems that the our python test cases would somehow trigger this.
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.
That seems worth investigating. I think it implies we have marked some faces as coming from an ID for which there is no transform. Those things should be 1:1. We definitely don't have much testing of this in the Python bindings, so we could have a bug there.
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.
indeed, I think I saw this happening previously as well
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.
maybe we can make an issue for this and merge this first?
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.
Sounds good.