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

third_party dependencies #523

Merged
merged 14 commits into from
Aug 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/manifold.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ jobs:
- name: Install dependencies
run: |
apt-get -y update
DEBIAN_FRONTEND=noninteractive apt install -y libomp-dev libassimp-dev git libtbb-dev pkg-config libpython3-dev python3 python3-distutils python3-pip lcov
DEBIAN_FRONTEND=noninteractive apt install -y libgtest-dev libomp-dev libassimp-dev git libtbb-dev pkg-config libpython3-dev python3 python3-distutils python3-pip lcov
pip install trimesh
- uses: actions/checkout@v3
with:
Expand Down Expand Up @@ -80,7 +80,7 @@ jobs:
- name: Install dependencies
run: |
apt-get -y update
DEBIAN_FRONTEND=noninteractive apt install -y libomp-dev libassimp-dev git libtbb-dev pkg-config libpython3-dev python3 python3-distutils python3-pip
DEBIAN_FRONTEND=noninteractive apt install -y libgtest-dev libomp-dev libassimp-dev git libtbb-dev pkg-config libpython3-dev python3 python3-distutils python3-pip
- uses: actions/checkout@v3
with:
submodules: recursive
Expand Down Expand Up @@ -144,7 +144,7 @@ jobs:
timeout-minutes: 30
strategy:
matrix:
parallel_backend: [NONE]
parallel_backend: [NONE, TBB]
cuda_support: [OFF]
max-parallel: 1
runs-on: windows-2019
Expand Down Expand Up @@ -194,7 +194,7 @@ jobs:
steps:
- name: Install common dependencies
run: |
brew install pkg-config assimp
brew install pkg-config googletest assimp
pip install trimesh
- name: Install OpenMP
if: matrix.parallel_backend == 'OMP'
Expand Down
33 changes: 21 additions & 12 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,30 +24,37 @@ option(MANIFOLD_DEBUG "Enable debug tracing/timing" OFF)
option(MANIFOLD_USE_CUDA "Enable GPU support via CUDA" OFF)
option(MANIFOLD_PYBIND "Build python bindings" ON)
option(MANIFOLD_CBIND "Build C (FFI) bindings" OFF)
option(TRACY_ENABLE "Use tracy profiling" OFF)

if(TRACY_ENABLE)
option(CMAKE_BUILD_TYPE "Build type" RelWithDebInfo)
if(NOT MSVC)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-omit-frame-pointer")
endif()
else()
option(CMAKE_BUILD_TYPE "Build type" Release)
endif()

set(MANIFOLD_PAR "NONE" CACHE STRING "Parallel backend, either \"TBB\" or \"OpenMP\" or \"NONE\"")
set(MANIFOLD_FLAGS -O3)

if(EMSCRIPTEN)
message("Building for Emscripten")
set(MANIFOLD_FLAGS -fwasm-exceptions -D_LIBCUDACXX_HAS_THREAD_API_EXTERNAL -D_LIBCUDACXX_HAS_THREAD_API_CUDA)
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -sALLOW_MEMORY_GROWTH=1 -fwasm-exceptions")
set(MANIFOLD_FLAGS -fexceptions -D_LIBCUDACXX_HAS_THREAD_API_EXTERNAL -D_LIBCUDACXX_HAS_THREAD_API_CUDA)
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -sALLOW_MEMORY_GROWTH=1 -fexceptions -sDISABLE_EXCEPTION_CATCHING=0")
set(MANIFOLD_PYBIND OFF)
endif()

option(PYBIND11_FINDPYTHON "Enable PyBind to perform FindPython for you" ON)
if(MANIFOLD_PYBIND)
find_package(Python COMPONENTS Interpreter Development.Module REQUIRED)
endif()

option(BUILD_TEST_CGAL "Build CGAL performance comparisons" OFF)

option(BUILD_SHARED_LIBS "Build dynamic/shared libraries" OFF)
set(PYBIND11_DIR ${PROJECT_SOURCE_DIR}/bindings/python/third_party/pybind11)
set(THRUST_INC_DIR ${PROJECT_SOURCE_DIR}/src/third_party/thrust)

if(BUILD_SHARED_LIBS OR MANIFOLD_CBIND)
# Allow shared libraries to be relocatable (add Place Independent Code flag).
# Also include when statically linking C bindings to avoid issues with bundling
# artefacts in host languages using them for FFI.
add_compile_options(-fPIC)
endif()

if(MANIFOLD_USE_CUDA)
enable_language(CUDA)
find_package(CUDA REQUIRED)
Expand All @@ -65,14 +72,16 @@ endif()

if(NOT MSVC)
set(WARNING_FLAGS -Werror -Wall -Wno-sign-compare -Wno-unused)
add_compile_options(
set(MANIFOLD_FLAGS
"${MANIFOLD_FLAGS}"
"$<$<COMPILE_LANGUAGE:CXX>:${WARNING_FLAGS}>"
"$<$<COMPILE_LANGUAGE:CUDA>:-Xcompiler=${WARNING_FLAGS}>")
endif()

if(CODE_COVERAGE AND NOT MSVC)
set(COVERAGE_FLAGS -coverage -fno-inline-small-functions -fkeep-inline-functions -fkeep-static-functions)
add_compile_options(
set(MANIFOLD_FLAGS
"${MANIFOLD_FLAGS}"
"$<$<COMPILE_LANGUAGE:CXX>:${COVERAGE_FLAGS}>"
"$<$<COMPILE_LANGUAGE:CUDA>:-Xcompiler=-coverage>")
add_link_options("-coverage")
Expand Down
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,11 @@ For more detailed documentation, please refer to the C++ API.

Contributions are welcome! A lower barrier contribution is to simply make a PR that adds a test, especially if it repros an issue you've found. Simply name it prepended with DISABLED_, so that it passes the CI. That will be a very strong signal to me to fix your issue. However, if you know how to fix it yourself, then including the fix in your PR would be much appreciated!

## Profiling

There is now basic support for the [Tracy profiler](https://github.com/wolfpld/tracy) for our tests.
To enable tracing, compile with `-DTRACY_ENABLE=on` cmake option, and run the test with Tracy server running.

## About the author

This library was started by [Emmett Lalish](https://elalish.blogspot.com/). I am currently a Google employee and this is my 20% project, not an official Google project. At my day job I'm the maintainer of [\<model-viewer\>](https://modelviewer.dev/). I was the first employee at a 3D video startup, [Omnivor](https://www.omnivor.io/), and before that I worked on 3D printing at Microsoft, including [3D Builder](https://www.microsoft.com/en-us/p/3d-builder/9wzdncrfj3t6?activetab=pivot%3Aoverviewtab). Originally an aerospace engineer, I started at a small DARPA contractor doing seedling projects, one of which became [Sea Hunter](https://en.wikipedia.org/wiki/Sea_Hunter). I earned my doctorate from the University of Washington in control theory and published some [papers](https://www.researchgate.net/scientific-contributions/75011026_Emmett_Lalish).
48 changes: 42 additions & 6 deletions flake.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 9 additions & 5 deletions flake.nix
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
{
inputs.flake-utils.url = "github:numtide/flake-utils";
inputs.nixpkgs.url = "nixpkgs/nixos-unstable";
inputs.gtest-src = {
url = "github:google/googletest/v1.14.0";
flake = false;
};

outputs = { self, nixpkgs, flake-utils }:
outputs = { self, nixpkgs, flake-utils, gtest-src }:
flake-utils.lib.eachDefaultSystem
(system:
let
Expand All @@ -25,7 +29,7 @@
"manifold-${parallel-backend}";
version = "beta";
src = self;
nativeBuildInputs = (with pkgs; [ cmake (python39.withPackages (ps: with ps; [ trimesh ])) ]) ++ build-tools ++
nativeBuildInputs = (with pkgs; [ cmake (python39.withPackages (ps: with ps; [ trimesh ])) gtest ]) ++ build-tools ++
(if cuda-support then with pkgs.cudaPackages; [ cuda_nvcc cuda_cudart cuda_cccl pkgs.addOpenGLRunpath ] else [ ]);
cmakeFlags = [
"-DMANIFOLD_PYBIND=ON"
Expand Down Expand Up @@ -75,6 +79,8 @@
emscripten
tbb
lcov
gtest
tracy
] ++ additional;
};
in
Expand All @@ -98,9 +104,7 @@
export EM_CACHE=$(pwd)/.emscriptencache
mkdir build
cd build
mkdir cache
export EM_CACHE=$(pwd)/cache
emcmake cmake -DCMAKE_BUILD_TYPE=Release ..
emcmake cmake -DCMAKE_BUILD_TYPE=Release -DFETCHCONTENT_SOURCE_DIR_GOOGLETEST=${gtest-src} ..
'';
buildPhase = ''
emmake make -j''${NIX_BUILD_CORES}
Expand Down
2 changes: 1 addition & 1 deletion src/collider/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
project (collider)

file(GLOB_RECURSE SOURCE_FILES CONFIGURE_DEPENDS *.cpp)
add_library(${PROJECT_NAME} OBJECT ${SOURCE_FILES})
add_library(${PROJECT_NAME} ${SOURCE_FILES})

if(MANIFOLD_USE_CUDA)
set_source_files_properties(${SOURCE_FILES} PROPERTIES LANGUAGE CUDA)
Expand Down
2 changes: 1 addition & 1 deletion src/cross_section/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
project (cross_section)

file(GLOB_RECURSE SOURCE_FILES CONFIGURE_DEPENDS *.cpp)
add_library(${PROJECT_NAME} OBJECT ${SOURCE_FILES})
add_library(${PROJECT_NAME} ${SOURCE_FILES})

target_include_directories(${PROJECT_NAME}
PUBLIC ${PROJECT_SOURCE_DIR}/include
Expand Down
5 changes: 4 additions & 1 deletion src/manifold/src/manifold.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,10 @@ MeshGL Manifold::GetMeshGL(glm::ivec3 normalIdx) const {
out.triVerts[3 * tri + i] = impl.halfedge_[3 * oldTri + i].startVert;

if (meshID != lastID) {
addRun(out, runNormalTransform, tri, meshIDtransform.at(meshID));
Impl::Relation rel;
auto it = meshIDtransform.find(meshID);
if (it != meshIDtransform.end()) rel = it->second;
Copy link
Owner

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?

Copy link
Collaborator Author

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.

Copy link
Owner

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.

Copy link
Collaborator Author

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

Copy link
Collaborator Author

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?

Copy link
Owner

Choose a reason for hiding this comment

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

Sounds good.

addRun(out, runNormalTransform, tri, rel);
meshIDtransform.erase(meshID);
lastID = meshID;
}
Expand Down
2 changes: 1 addition & 1 deletion src/polygon/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
project (polygon)

file(GLOB_RECURSE SOURCE_FILES CONFIGURE_DEPENDS *.cpp)
add_library(${PROJECT_NAME} OBJECT ${SOURCE_FILES})
add_library(${PROJECT_NAME} ${SOURCE_FILES})

target_include_directories(${PROJECT_NAME}
PUBLIC ${PROJECT_SOURCE_DIR}/include
Expand Down
2 changes: 1 addition & 1 deletion src/third_party/graphlite/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
project (graphlite)

add_library(${PROJECT_NAME} OBJECT src/connected_components.cpp)
add_library(${PROJECT_NAME} src/connected_components.cpp)

target_include_directories(${PROJECT_NAME}
PUBLIC ${PROJECT_SOURCE_DIR}/include
Expand Down
33 changes: 27 additions & 6 deletions src/utilities/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Expand All @@ -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")
Copy link
Owner

Choose a reason for hiding this comment

The 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()
Expand Down Expand Up @@ -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)
38 changes: 36 additions & 2 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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")
Copy link
Owner

Choose a reason for hiding this comment

The 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?

Copy link
Contributor

Choose a reason for hiding this comment

The 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!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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 find_package still cannot find the package, you can specify the directory by setting -DFETCHCONTENT_SOURCE_DIR_GOOGLETEST. I tried using this for the nix package (it blocks internet connection when building) and it works.

Copy link
Contributor

@kintel kintel Aug 9, 2023

Choose a reason for hiding this comment

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

Awesome! Not familiar with FetchContent..

Copy link
Owner

Choose a reason for hiding this comment

The 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()
Expand Down
Loading