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 9 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
19 changes: 9 additions & 10 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,30 +24,27 @@ 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)
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 -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)
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 +62,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 administrator privileges.

## 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)
12 changes: 3 additions & 9 deletions src/utilities/include/public.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,14 @@
#include <glm/gtc/constants.hpp>
#include <glm/gtx/compatibility.hpp>
#include <glm/gtx/rotate_vector.hpp>
#include <iomanip>
#include <iostream>
#include <limits>
#include <memory>
#include <sstream>
#include <unordered_map>
#include <vector>

#ifdef MANIFOLD_DEBUG
#include <iomanip>
#include <iostream>
#include <sstream>
#endif
pca006132 marked this conversation as resolved.
Show resolved Hide resolved

namespace manifold {

constexpr float kTolerance = 1e-5;
Expand Down Expand Up @@ -454,8 +451,6 @@ struct ExecutionParams {
bool suppressErrors = false;
};

#ifdef MANIFOLD_DEBUG

inline std::ostream& operator<<(std::ostream& stream, const Box& box) {
return stream << "min: " << box.min.x << ", " << box.min.y << ", "
<< box.min.z << ", "
Expand Down Expand Up @@ -514,7 +509,6 @@ void Diff(const std::vector<T>& a, const std::vector<T>& b) {
std::cout << std::endl;
}
/** @} */
#endif
} // namespace manifold

#undef HOST_DEVICE
Loading
Loading