Topology changes for SphereCollisionModel on EdgeSetTopologyContainer

[ScheiklP]

Hi, removing SphereCollisionModel elements from a EdgeSetTopologyContainer currently does not work, because

TopologicalChangeManager::removeItemsFromSphereModel calls getCollisionTopology https://github.com/sofa-framework/sofa/blob/d42e3547dffaf385448fb077d466fc0bebc39bc9/Sofa/GUI/Component/src/sofa/gui/component/performer/TopologicalChangeManager.cpp#L297 which is not implemented in SphereCollisionModel, and the default from BaseMeshTopology returns nullptr. https://github.com/sofa-framework/sofa/blob/471a3df6a377f92155f34ab4a75e931ec9559f7f/Sofa/framework/Core/src/sofa/core/CollisionModel.h#L357

I tried porting over the topology linking from TriangleModel, but it seems I am missing something, because it does not work :D https://cloud.ipr.kit.edu/s/ZHW7Jkz5yE6wb42

Is anyone seeing something obvious?

Cheers, Paul

---

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).

[hugtalbot]

Hi @ScheiklP

Thanks for the PR.

The SphereCollisionModel works historically without a topology (relying on the mstate). The tests are failing due to the absence of a PointSetTopology. What could be done is to change the warning into an info saying that no topology is found therefore no topological change will be supported. What do you think?

Another change in order to make the component topologically robust would be to make the data radius as a PointData:

Data< VecReal > radius; ///< Radius of each sphere

should become

PointData< Real > radius; ///< Radius of each sphere

(connected to a topology handler)

[ScheiklP]

Hi @ScheiklP

Thanks for the PR.

The SphereCollisionModel works historically without a topology (relying on the mstate). The tests are failing due to the absence of a PointSetTopology. What could be done is to change the warning into an info saying that no topology is found therefore no topological change will be supported. What do you think?

Another change in order to make the component topologically robust would be to make the data radius as a PointData:

Data< VecReal > radius; ///< Radius of each sphere

should become

PointData< Real > radius; ///< Radius of each sphere

(connected to a topology handler)

Hi @Hugo, the SphereCollisionModel are in a node together with an EdgeSetTopologyContainer that is now also correctly found and bound to l_topology and m_topology.

Is there something else to do, to correctly connect it to m_mstate?

Or maybe updateFromTopology() should do more than it does now? :D

[alxbilger]

Is this PR ready to be reviewed? If yes, the label must be changed

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

[ScheiklP]

Hi @alxbilger , nope it is still WIP. This one https://github.com/sofa-framework/sofa/pull/3239 should be done first anyway :)

[epernod]

Hi @ScheiklP , what is the status of this PR?

[ScheiklP]

Hi @ScheiklP , what is the status of this PR?

Hi @epernod , I am actually not sure. I will close it for now, and if I stumble upon needing the feature at some point, I will reopen it.