Attempt at simplifying the file structure and CMake build

[starseeker]

Collapses the various features into libmanifold, with configure options to enable/disable them.

Groups public headers in include/manifold.

Adjustments to the exported targets to try and avoid invoking the download steps in users of an installed Manifold library.

[starseeker]

Follow-on from #945 and #953

[starseeker]

@elalish I'll pick back up with the EXPORT config logic on Monday... getting closer at least.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.76%. Comparing base (d437097) to head (79bfc42).
Report is 109 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #961      +/-   ##
==========================================
- Coverage   91.84%   86.76%   -5.08%     
==========================================
  Files          37       61      +24     
  Lines        4976     8492    +3516     
  Branches        0     1044    +1044     
==========================================
+ Hits         4570     7368    +2798     
- Misses        406     1124     +718     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[elalish]

Thanks for putting this all together - and great to see it passing our CI. @kintel @fire @wtholliday please put your two cents in here too - would be nice to not have to redo this again, and I don't have a strong opinion on structure besides making it easy to consume downstream.

[wtholliday]

please put your two cents in here too

I'm not qualified to evaluate the cmake stuff, but the directory structure makes me happy :)

[pca006132]

btw maybe we should take the chance to change MANIFOLD_PAR to ON/OFF as well, we no longer have multiple backends anyway

[starseeker]

btw maybe we should take the chance to change MANIFOLD_PAR to ON/OFF as well, we no longer have multiple backends anyway

Do you want to make it binary just in CMake, or also in the C++ source code? (i.e. #if MANIFOLD_PAR == 'T' -> #ifdef MANIFOLD_PAR)

[pca006132]

btw maybe we should take the chance to change MANIFOLD_PAR to ON/OFF as well, we no longer have multiple backends anyway

Do you want to make it binary just in CMake, or also in the C++ source code? (i.e. #if MANIFOLD_PAR == 'T' -> #ifdef MANIFOLD_PAR)

Both.

[starseeker]

By the way, should that .vscode directory be committed into the repo?

[elalish]

By the way, should that .vscode directory be committed into the repo?

I've gotten this question before - I suppose the correct answer is no, but I did it intentionally because when I clone the repo on a new machine (and I'm always using VSCode - who isn't anymore?) it's really convenient to have it all automatically set up. Is it bothering anyone?

[pca006132]

By the way, should that .vscode directory be committed into the repo?

IMO no, I think people often use their own vscode config for the project anyway. @elalish what do you think?

[wtholliday]

Is it bothering anyone?

I thought it was nice, because I'm not a regular vscode user (Xcode usually) and vscode automatically did the code formatting, which I assume is part of your config. Only issue I had was that cmake fails within vscode on my mac, which perhaps is a config issue?

[cmake] CMake Error: Error: generator : Ninja
[cmake] Does not match the generator used previously: Unix Makefiles
[cmake] Either remove the CMakeCache.txt file and CMakeFiles directory or choose a different binary directory.

I blow away the whole build dir and get the same error. Works outside of vscode.

[elalish]

@wtholliday Interesting, I'm not aware of Ninja being part of VSCode by default, but maybe something has changed?

[starseeker]

About vec_view.h and iters.h - the former is referenced by cross_section.h, and the latter by parallel.h - that's why they got categorized as public headers, since cross_section.h and parallel.h are public? I can check if they're actually needed in those headers...

[starseeker]

Is cross_section.h intended to be a public header? If so, it looks like vec_view.h needs to be too...

[elalish]

Is cross_section.h intended to be a public header? If so, it looks like vec_view.h needs to be too...

Ah, I see. Yeah, parallel.h became public recently and I didn't realize it pulled in iter.h. And vec_view apparently came in to both cross_section and manifold.h with WarpBatch. Okay, I guess that's fine as is, thanks.

[elalish]

Do we think this is ready to merge? If so I can update our CI so it's expecting these test runs.

[starseeker]

Ah, I see. Yeah, parallel.h became public recently and I didn't realize it pulled in iter.h. And vec_view apparently came in to both cross_section and manifold.h with WarpBatch. Okay, I guess that's fine as is, thanks.

Oh, whoops - I make parallel.h non-public to hide iter.h. Should I put it back?

[starseeker]

Is cross_section.h intended to be a public header? If so, it looks like vec_view.h needs to be too...

Ah, I see. Yeah, parallel.h became public recently and I didn't realize it pulled in iter.h. And vec_view apparently came in to both cross_section and manifold.h with WarpBatch. Okay, I guess that's fine as is, thanks.

OK, I put iter.h and parallel.h back. As far as I know this can be merged, as long as nobody else has spotted any issues. It'll probably be easier to do any additional refinements in other pull requests in any case.

[starseeker]

Rebased, corrected indenting on other CMakeLists.txt files. Assuming the adjustment to CMake spacing didn't inadvertently break something, that should be all I've got for this PR.

[elalish]

Thanks - really glad to have your help cleaning up our CMake and structure.

[starseeker]

Thanks - really glad to have your help cleaning up our CMake and structure.

My pleasure - thank you for being willing to do so! It makes life much easier for me updating to newer Manifold versions (I was doing some custom modding on my end prior to this) so it's a textbook win-win.