Pass additional data to level set SDF function

[harmanpa]

Please consider this or something similar for the SDF function when calling level set from the C bindings.

This allows a developer to pass some context to the level set function, such as a pointer to an object in the host language, so that parallel calls can be threadsafe.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[geoffder]

What is the additional context being used for, could you give an example? Typically an sdf should be a pure computation from a point in space to a distance, so no synchronization should be required.

[codecov]

Codecov Report

Patch coverage has no change and project coverage change: +90.35 🎉

Comparison is base (8b7bf42) 0.00% compared to head (1a749fa) 90.35%.

Additional details and impacted files

@@             Coverage Diff             @@
##           master     #427       +/-   ##
===========================================
+ Coverage        0   90.35%   +90.35%     
===========================================
  Files           0       35       +35     
  Lines           0     4396     +4396     
===========================================
+ Hits            0     3972     +3972     
- Misses          0      424      +424     

see 35 files with indirect coverage changes

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

[pca006132]

Perhaps he wants to implement closure as a function + object that contains values of captured variables, so he wants a way of passing that object into the call.

[harmanpa]

In our case it is for Java bindings, it would be a struct with the methodid and objectid

[geoffder]

Sorry I'm not really familiar with Java, but if you are able to pass a function pointer that interacts with the provided object pointer, why can't the function whose pointer is being passed do the work (calculating distance)? The function can't have captured variables (even if they are read only)?

[harmanpa]

Only if we have a single static function, so it would need a blocking mechanism (in Java or C) to handle the possibility of more than 1 level-set function being called concurrently. Passing an extra pointer is a normal pattern with numerical solvers in C/Fortran, it's not just a Java-specific request.

[pca006132]

Not familiar with Java as well, but this is a standard way of implementing OOP and functions with captured variables in C.
https://stackoverflow.com/a/351745

[geoffder]

As in calling level_set multiple times concurrently with the same closure? I hadn't really thought of that as a possibility. Anyway, I'm not opposed to it, just trying to understand this gap in my support of patterns in other languages wrt closures. I think that some comments noting intended/potential uses/when you'd need it would help future users to decide whether they are fine with the basic or context added version.

Would level_set_context, with the argument named ctx be clearer?

[harmanpa]

As in calling level_set multiple times concurrently with the same closure?

Yes.

Would level_set_context, with the argument named ctx be clearer?

Yes that sounds better.

[elalish]

I don't have a strong opinion on this - I leave it to @pca006132 or @geoffder to approve. Can you sign the CLA? It's pretty generic, but unfortunately required.

[geoffder]

@harmanpa Are you planning to update this with the renaming and add a comment describing what the intended purpose of the new function is (vs the regular manifold_level_set as discussed above? Also, to pass the CI the changes should be run through an autoformat (whatever the default is with the clangd lsp works for me).

[harmanpa]

Yes I do intend to @geoffder , sorry I've been travelling, also weirdly Google rejected my CLA signing so I need to do that again

[elalish]

@harmanpa Any luck with the CLA? I happy to help if you're having trouble.

[harmanpa]

@elalish CLA is sorted now, thank you :-)

@geoffder I've pushed updated header/implementation, could you take a look please?

There is one conflict reported by github: the #include <cstdint> in manifoldc.h should be inside the #ifdef __cplusplus so I moved it, it seems git sees that as a conflict.

[geoffder]

I resolved the conflict, but manifoldc.cpp is failing the clang linter. Otherwise, I think this looks good.

[geoffder]

Thanks!