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

Missing C bindings + revamp composition with opaque vectors #382

Merged
merged 6 commits into from
Mar 20, 2023
Merged

Missing C bindings + revamp composition with opaque vectors #382

merged 6 commits into from
Mar 20, 2023

Conversation

geoffder
Copy link
Collaborator

This brings the C bindings up to date with the latest additions to CrossSection as well as adding a couple that were missing from the Manifold coverage as well (Boolean and BatchBoolean).

With the addition of CrossSection's Decompose, I decided I should take the opportunity to remedy some silliness I introduced with the multistep Decompose due to my desire to stick with arrays uniformly, without using an opaque vector API. Now Decompose for both classes returns an opaque vector, and the basic methods have been added to support this.

@codecov
Copy link

codecov bot commented Mar 19, 2023

Codecov Report

Patch coverage: 87.17% and project coverage change: +0.43 🎉

Comparison is base (d713986) 85.48% compared to head (d3c7807) 85.91%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #382      +/-   ##
==========================================
+ Coverage   85.48%   85.91%   +0.43%     
==========================================
  Files          36       36              
  Lines        4415     4444      +29     
==========================================
+ Hits         3774     3818      +44     
+ Misses        641      626      -15     
Impacted Files Coverage Δ
src/utilities/include/public.h 59.48% <ø> (ø)
src/cross_section/src/cross_section.cpp 71.03% <85.29%> (+9.70%) ⬆️
src/manifold/src/constructors.cpp 94.41% <100.00%> (-0.16%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Owner

@elalish elalish left a comment

Choose a reason for hiding this comment

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

Excellent cleanup, thank you!

return ms;
ManifoldManifoldVec *manifold_decompose(void *mem, ManifoldManifold *m) {
auto comps = from_c(m)->Decompose();
return to_c(new (mem) std::vector<Manifold>(comps));
Copy link
Owner

Choose a reason for hiding this comment

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

👍

@@ -75,9 +75,6 @@ class Manifold {
*/
///@{
static Manifold Compose(const std::vector<Manifold>&);

Components GetComponents() const;
std::vector<Manifold> Decompose(Components components) const;
Copy link
Owner

Choose a reason for hiding this comment

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

💯

@geoffder geoffder merged commit 2a63971 into elalish:master Mar 20, 2023
@geoffder geoffder deleted the update-c-bindings branch March 20, 2023 05:21
@elalish elalish mentioned this pull request Nov 3, 2023
cartesian-theatrics pushed a commit to SovereignShop/manifold that referenced this pull request Mar 11, 2024
…ish#382)

- Add bindings to `CrossSection::{Compose,Decompose,NumVert,NumContour,Warp}` 
- Add bindings to `Boolean` and `BatchBoolean` for both `Manifold` and `CrossSection`
- Add `ManifoldManifoldVec` and `ManifoldCrossSectionVec` opaque types for `std::vector` pointers to help clean up bindings involving vectors (particularly when they are returned with dynamic size, *e.g.* `manifold_decompose`
- Remove two-step Components workaround previously introduced to allow return of C array from `manifold_decompose`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants