third_party dependencies #523
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #523 +/- ##
=======================================
Coverage 90.36% 90.37%
=======================================
Files 35 35
Lines 4433 4435 +2
=======================================
+ Hits 4006 4008 +2
Misses 427 427
☔ View full report in Codecov by Sentry. |
we should make the library type implicit, so users can decide whether or not to build shared libraries.
| addRun(out, runNormalTransform, tri, meshIDtransform.at(meshID)); | ||
| Impl::Relation rel; | ||
| auto it = meshIDtransform.find(meshID); | ||
| if (it != meshIDtransform.end()) rel = it->second; |
There was a problem hiding this comment.
Is there a case where this is expected, or should this be an assert?
There was a problem hiding this comment.
I am not entirely sure, it seems that the our python test cases would somehow trigger this.
There was a problem hiding this comment.
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.
indeed, I think I saw this happening previously as well
There was a problem hiding this comment.
maybe we can make an issue for this and merge this first?
| pkg_check_modules(TBB tbb) | ||
| endif() | ||
| if(NOT TBB_FOUND) | ||
| message(STATUS "tbb not found, downloading from source") |
|
|
||
| find_package(GTest 1.12) | ||
| if((NOT GTest_FOUND) OR NOT(GTest_VERSION GREATER_EQUAL 1.12)) | ||
| message(STATUS "GTest not found, downloading from source") |
There was a problem hiding this comment.
@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.
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.
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.
There was a problem hiding this comment.
Awesome! Not familiar with FetchContent..
There was a problem hiding this comment.
Nice - would be good to add that trick to the README somewhere.
|
I am not familiar with Tracy either :) We should be able to add more client markups to make the profiler easier to use.
|
no need to use sudo, and we need some debug flags for good stack capture
|
Sadly it seems that some features are not available on OSX. |
* use system gtest if possible * optional tracy support * fix meshid not found situation * try fix CI * fix ubuntu build * disable windows tbb build first * update tbb config * format * fix cmake BUILD_SHARED_LIBS we should make the library type implicit, so users can decide whether or not to build shared libraries. * revert public.h changes * remove version requirement for gtest * update tracy instructions no need to use sudo, and we need some debug flags for good stack capture * fix js build
Reincarnation of #402, this uses system gtest if possible and automatically download gtest from github otherwise (e.g. for windows). It will also download tbb and compile from source if the user specified
MANIFOLD_PAR=TBBand CMake is unable to find the installation.I also added optional dependency to Tracy profiler, which is a handy graphical profiler.