Skip to content

Add missing C API bindings#1633

Merged
elalish merged 1 commit into
elalish:masterfrom
zmerlynn:feat/c-api-missing-bindings
Apr 10, 2026
Merged

Add missing C API bindings#1633
elalish merged 1 commit into
elalish:masterfrom
zmerlynn:feat/c-api-missing-bindings

Conversation

@zmerlynn

@zmerlynn zmerlynn commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add 16 new C API functions exposing Manifold and MeshGL/MeshGL64
    methods that had no C binding:
    • manifold_get_tolerance, manifold_set_tolerance, manifold_simplify,
      manifold_num_prop_vert
    • manifold_meshgl{,64}_tolerance, _run_flags_length, _run_flags,
      _num_run, _update_normals
  • Add thread safety note to the C header: objects are not safe for concurrent
    access, even read-only, due to lazy evaluation with mutable internal state.
  • Add tests for all new bindings.

Motivation

These gaps were found while building manifold-csg,
a new Rust FFI binding to the C API. Missing accessors for runFlags, tolerance,
NumRun, and UpdateNormals prevent lossless round-tripping of MeshGL data.
GetTolerance/SetTolerance/Simplify are useful public Manifold methods
with no C-side equivalent (manifold_epsilon exists but calls the GetEpsilon
testing hook rather than the public GetTolerance).

The thread safety note documents the mutable shared_ptr lazy-evaluation
pattern in Manifold and CrossSection, which is relevant for FFI consumers
that need to decide whether concurrent read access is safe (it isn't).

Test plan

  • All 15 CBIND tests pass (4 new: tolerance, num_prop_vert,
    meshgl_run_accessors, meshgl_update_normals)
  • No changes to existing tests or behavior

🤖 Generated with Claude Code

@codecov

codecov Bot commented Apr 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.24%. Comparing base (d88e295) to head (bd92b98).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1633      +/-   ##
==========================================
+ Coverage   95.22%   95.24%   +0.02%     
==========================================
  Files          34       34              
  Lines        7657     7657              
==========================================
+ Hits         7291     7293       +2     
+ Misses        366      364       -2     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pca006132

Copy link
Copy Markdown
Collaborator

IIRC the usage of the shared ptrs should be guarded by a mutex, so operations should be thread safe. I haven't really looked into that (I should have), but if it is not thread safe we should fix that.

@zmerlynn

Copy link
Copy Markdown
Contributor Author

Good point — I took a look and it seems like Manifold::pNode_ itself isn't guarded, even though CsgOpNode::impl_ uses ConcurrentSharedPtr with a mutex. The race is in GetCsgLeafNode() which reassigns pNode_ from const methods without synchronization.

That said, this is orthogonal to the bindings PR, so I'll remove the thread safety comment from this PR and follow up with a separate issue to discuss the fix. (A std::call_once approach looks promising — low overhead on the fast path and naturally shareable across copies.)

@zmerlynn zmerlynn force-pushed the feat/c-api-missing-bindings branch from 3d536ac to 4efccc1 Compare April 10, 2026 03:57
New bindings:
- manifold_get_tolerance, manifold_set_tolerance, manifold_simplify,
  manifold_num_prop_vert
- manifold_meshgl{,64}_tolerance, _run_flags_length, _run_flags,
  _num_run, _update_normals

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@zmerlynn zmerlynn force-pushed the feat/c-api-missing-bindings branch from 4efccc1 to bd92b98 Compare April 10, 2026 03:58
@zmerlynn zmerlynn changed the title Add missing C API bindings and document thread safety Add missing C API bindings Apr 10, 2026

@elalish elalish left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thank you! Our original C API contributor is long gone, so it's great to see this getting some love.

I'm also excited to see the Rust bindings you're working on - we've had several users interested in having that. It'll be great to add to the list when it's ready.

@elalish elalish merged commit 93c1b4b into elalish:master Apr 10, 2026
36 checks passed
@zmerlynn zmerlynn deleted the feat/c-api-missing-bindings branch April 19, 2026 23:41
@elalish elalish mentioned this pull request May 23, 2026
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.

3 participants