Skip to content

Conversation

@damienmarchal
Copy link
Contributor

@damienmarchal damienmarchal commented Jun 26, 2025

There is currently no component to generate a 3D surface as the "implicit" surface of a scalar field.

I searched into the code base and the closest I found was ImplicitSurfaceMapping... but:

  • all applyJ are missing, so it is in fact geometric engine more than a mapping
  • it does not mesh any ScalarField but only the one produced by an equation of hardcoded metaballs, with a fixed radius, and centered on the mechanical object position. I suspect this was done for SPH and fluid simulations to render the fluid surface.

So... I made this PR, which introduce a FieldToSurfaceMesh component. Meshing any ScalarField.

def createScene(root : Sofa.Core.Node):
    root.addObject("RequiredPlugin", name="SofaImplicitField")

    # Makes two spherical field, one using the component coded in c++, the other using a pure python implementation
    field1 = root.addObject(Sphere(name="field1", center=[0,0,0]))          
    field2= root.addObject("SphericalField", name="field2", center=[2,0,0])  

    root.addChild("Visual")
    
    # Generates one surface with low resolution from the field1 
    root.Visual.addObject("FieldToSurfaceMesh", name="polygonizer1",
                          field=field1.linkpath, min=[-1,-1,-1], max=[1,1,1], step=0.1)    

    # Generates a second surface surface with low resolution from the field1 
    root.Visual.addObject("FieldToSurfaceMesh", name="polygonizer2",
                          field=field2.linkpath, min=[1,-1,-1], max=[3,1,1],  step=0.01)    

    # Connect the geometry to a visual model 
    root.Visual.addObject("OglModel", name="renderer", 
                        position=root.Visual.polygonizer2.points.linkpath, 
                        triangles=root.Visual.polygonizer2.triangles.linkpath)
                        

This produce this kind of view:
image
Or by changing the step:
image

After this PR:

  • I would be very interesting to implement an asynchronous version if this component so we can mesh multiple object in parallel. This requires the component connected to this one to also be compatible with asynchronous operations (i.e. being updated when their inputs data changes)
  • it would be great to factorize the different marching cubes implementation (in ImplicitSurfaceMapping and in FieldToSurfaceMesh)... but this would requires refactoring of ImplicitSurfaceMapping, so I first did the ImplicitSurfaceMapping PR.
  • the debugDraw and draw() function should be removed after the merge of PR [Sofa.GUI.Common] Handle object selection in BaseViewer #5636 and Sofa.Qt and imGui friends, (but in the meantime it is useful :))

By submitting this pull request, I acknowledge that
I have read, understand, and agree SOFA Developer Certificate of Origin (DCO).


Reviewers will merge this pull-request only if

  • it builds with SUCCESS for all platforms on the CI.
  • it does not generate new warnings.
  • it does not generate new unit test failures.
  • it does not generate new scene test failures.
  • it does not break API compatibility.
  • it is more than 1 week old (or has fast-merge label).

@damienmarchal damienmarchal added the pr: status wip Development in the pull-request is still in progress label Jun 26, 2025
@hugtalbot hugtalbot added the pr: enhancement About a possible enhancement label Jun 27, 2025
@damienmarchal damienmarchal force-pushed the pr-add-field-to-surface-mesh branch from b769e33 to 8144c9e Compare August 27, 2025 12:40
@damienmarchal damienmarchal added pr: status to review To notify reviewers to review this pull-request pr: new feature Implement a new feature and removed pr: enhancement About a possible enhancement pr: status wip Development in the pull-request is still in progress labels Aug 27, 2025
@damienmarchal
Copy link
Contributor Author

damienmarchal commented Sep 3, 2025

Poke: @bakpaul, @epernod

About wether it is an engine, a normal component, a topological mapping or mechanical mapping, here is my understanding.

  1. In sofa, a TopologicalMapping is a mapping between a BaseMeshTopology and an another BaseMeshTopology.
    Given that ScalarFields are, by far, not MeshTopology, FieldToSurfaceMesh cannot be such a mapping

    From a terminology point of view, the naming of TopologicalMapping in SOFA is questonnable as in fact it only cover discreet topologies (meshes and not the "implicit" topology emerging from the extraction of the iso-alues of a scalar field).

    From an architectural/OOP point of view in its current state a TopologicalMapping is not inhering from Mapping or BaseMapping... so TopologicalMapping cannot be considered as neither a "Mapping".

  2. In sofa, a MechanicalMapping is a mapping between a MechanicalObject and an another MechanicalObject
    Given that FieldToSurfaceMesh has nothing to do with mechanics, it is only there for geometry processing (producing triangles and points), I don't see how it could be a MechanicalMapping

  3. Could it be inheriting from Mapping or BaseMapping,
    In term of naming why not... but the actual Mapping and BaseMapping contains mechanical stuffs... and thus maybe they are closer to a something that should be called a BaseMechanicalMapping than a BaseMapping.

  4. Maybe a general refactoring of the Mapping in sofa would be good, actually I think it would be very cool to generalize the computation of derivatives/grad/hessian through a general mapping (geometrical, mechanical, etc...). But this seems a far target, probably connected to have a fully differentiable "sofa". A mapping is a transformation to and from.. with differential operator.

  5. There is one subtile difference between Engines and Mapping that are worth mentioning.
    By design, mappings are "manipulating" an output object. This means they need a link to the target object to be created and operates. When these targets are DataTypes templated objects the mapping has to be template as well (eg: MechanicalMapping:: "Vec3,Vec3", "Rigid3,Vec3").

Data Engine are not operating that way, they process data in inputs...and stores the results in output field. Users of the component are free to link the output datas to any other component's compatible data field. It can be a component saving to file, or to visualize the geometry in opengl, storing it in a MeshTopologyContainer. I see much more flexibility in data engine design.

So

  • Given that FieldToSurfaceMesh cannot be MechanicalMapping (as it produces triangles) neither TopologicalMapping (is it has no MeshTopology as its inputs),
  • Given that we are far from refactoring the whole Mapping hierarchy
  • Given that the component behave exactly like any other engine...
    => then it should be an engine.

About the case of ImplicitSurfaceMapping.
It is a weird component because it inherit from MechanicalMapping, but it but does not fulfill Mapping's interfaces because of some of the pure virtual method are in fact overriden with "not implemented" .
In addition it also output extra "stuff" (edges, triangles) as data field.
Which is exactly what a DataEngine would do but not a MechanicalMapping.
Fixing it means would probably imply to clarify/refactoring the Mapping hierarchy (4) and what we expect from each entity.

So I will just factor the duplicated marching cube code.
I could also investigate, as Fedroy pointed, it should inherit from and engine instead of using the updateCallback mechanisme... but I suspect this will interfere with the other PR that make asynchronous version of this code.

@damienmarchal
Copy link
Contributor Author

@bakpaul, do you have any idea what MarchingCubeUtility is for ? I'm not sure I understand why it takes a char* data as inputs.

@damienmarchal
Copy link
Contributor Author

damienmarchal commented Sep 3, 2025

Some news @bakpaul

Moving the marching cube from FIeldToSurfaceMesh in a separated file was pretty easy and thus the introduced component is now independent of the marching cube algorithm and the algorithm is now available to other component.

But using it, to refactor ImplicitSurfaceMapping is complex. Because ImplicitSurfaceMapping is in fact hard coding a ScalarField. And that the hard coded incremental version of the MarchingCube seems to sweeps along one axis with an acceleration mechanism to evaluate efficiently the field generated by spherical kernels (the ScalarFIeld).

Untangle ImplicitSurfaceMapping, while maintaining more or less the same level of performance need serious refactoring and/or algorithmic work. So I would suggest to let the refactoring of ImplicitSurfaceMapping to use the MarchingCube that is in MarchingCube.h and use a real ScalarField (kind of GaussianKernel) for the one that want it.

In addition, I'm not interested to work on the ImplicitSurfaceMapping part right now, because performance oriented ScalarField are in my TODO list: #5673 and I expect that the refactoring would profit of that.

Before giving up, I will see if I can do something with a lambda function on MarchingCube::generateSurfaceMesh()

NB: not so good for ImplicitFieldMapping performance
image
image

@damienmarchal
Copy link
Contributor Author

damienmarchal commented Sep 4, 2025

Hello @bakpaul,

I spend the day to refactoring the ImplicitSurfaceMapping to separate the field approximation using wyvill's function from the MarchingCube algorithm itself. As expected, there is no free lunch, this come with a performance drop from 500 fps to 300fps on the default scene.

@bakpaul
Copy link
Contributor

bakpaul commented Sep 5, 2025

Hi @damienmarchal, great to see the work you did to try to factorize the code.
Your conclusion means that instead of spending 2ms per time step we now spend 3ms. I don't know who's using this code right now, but I think that what you did is a good tradeoff between speed and maintenance. Let's discuss this at the next dev meeting.

@damienmarchal
Copy link
Contributor Author

damienmarchal commented Sep 5, 2025

Hi @bakpaul

Thanks for the feedback, to me the ImplicitSurfaceMapping component was developped to render SPH fluides simulations, as it was very common circa 2006-2010, and the implementation is elegant to maximize efficiency.

Nowaydays other rendering approach are possible to implement high performances rendering of fluids. So investing time on these is probably wiser.

So with this in mind and if having a 1.5 time slower version of ImplicitSurfaceMapping is ok, then I will clean the code (as it is still very drafty). I will keep you informed when it is ready for review.

@damienmarchal damienmarchal force-pushed the pr-add-field-to-surface-mesh branch 2 times, most recently from 84dd899 to a0c0044 Compare October 17, 2025 07:14
@damienmarchal damienmarchal force-pushed the pr-add-field-to-surface-mesh branch from a0c0044 to e390490 Compare October 17, 2025 07:15
@damienmarchal damienmarchal added pr: status to review To notify reviewers to review this pull-request and removed pr: status wip Development in the pull-request is still in progress labels Oct 17, 2025
@damienmarchal
Copy link
Contributor Author

damienmarchal commented Oct 17, 2025

@hugtalbot I integrated the fix

@bakpaul
Copy link
Contributor

bakpaul commented Oct 22, 2025

Teh scene you introduce triggers an error, could you fix it ?

@damienmarchal
Copy link
Contributor Author

SUre , thank for the feedback

@damienmarchal damienmarchal added the pr: status wip Development in the pull-request is still in progress label Oct 30, 2025
@damienmarchal damienmarchal removed the pr: status wip Development in the pull-request is still in progress label Oct 30, 2025
@hugtalbot
Copy link
Contributor

[ci-build][with-all-tests]

@damienmarchal
Copy link
Contributor Author

So much "update"

@hugtalbot
Copy link
Contributor

Hi @damienmarchal
PR does not seem to pass

@hugtalbot
Copy link
Contributor

Error might come from my wrong-merge, so restart the CI and see what happens

@damienmarchal
Copy link
Contributor Author

damienmarchal commented Nov 10, 2025

@hugtalbot do you know if the error from the CI or the PR ?

@bakpaul bakpaul added pr: status ready Approved a pull-request, ready to be squashed and removed pr: status to review To notify reviewers to review this pull-request labels Nov 12, 2025
@epernod
Copy link
Contributor

epernod commented Nov 12, 2025

sry can't merge: "This branch is out-of-date with the base branch"

@hugtalbot hugtalbot merged commit bd18bf6 into sofa-framework:master Nov 13, 2025
7 of 11 checks passed
@hugtalbot hugtalbot added this to the v25.12 milestone Dec 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: new feature Implement a new feature pr: status ready Approved a pull-request, ready to be squashed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants