[SceneChecker] Check that only one topology per node is allowed

[epernod]

A node including <MeshTopology /> and <TopologyContainer /> component is possible and will crash if some topological changes are performed.

Other component could perform this->getContext()->getMeshTopology() which will return either one or the other depending on the order in the Node I assume.

---

Suggested labels:

[damienmarchal]

Hi Erik,

Thanks for reporting this bug.

I looked into BaseContext and found the following:

core::topology::Topology* BaseContext::getTopology() const
core::topology::BaseMeshTopology* BaseContext::getMeshTopology() const
core::topology::BaseMeshTopology* BaseContext::getLocalMeshTopology() const
core::topology::BaseMeshTopology* BaseContext::getActiveMeshTopology() const

All of them only returns the "first" topology found which can cause a lot of trouble to users.

I think your suggestion to provide warning message using the scene checker is a good idea unless there is good reasons for ppl to have multiple topologies in the same node (I don't know if this is the case)

I can give it a try.

[jnbrunet]

You could have two topologies if, for example, a mesh contains both hexahedral and tetrahedral elements inside one simulated object. You will have one mechanical object with all the positions of the mesh, two topologies and two forcefields, all in one node. Of course, you could split those in two or more nodes, but then you will complexify the scene with subset mappings...

[damienmarchal]

According to @jnbrunet 's feedback I would say that the right approach to fix this is to stop using implicit and hidden link between objects.

In defrost we are doing this kind of pattern (in pseudo code) this:

class MyObject {
     ....
     SingleLink<Topology> topologyLink;
}
MyObject::init(){
    /// Check if the link is explicitely set, 
    if( !topologyLink.isSet() ){
        /// If this is not the case so fallback to get the first topology in the context (buisness as usual)               topologyLink.setLinkTo( getTopology() );
    }

    /// Here we use linked object. 
}

Such an approach:

• is compatible with our existing scene base (when no link is specified then automatically retrieve something from the context)

But it brings the following:

• As there is systematically a SingleLink for each getContext query, users can see in the GUI which object is actually linked to and so he can detect more easily what is wrong.

• With the SingleLink it become possible for the user to specify object should be used instead of the one retrived from the context (eg when you want the second object instead of the first one).

So to me we juste need to generalize this way of doing to all sofa objects.
Any suggestion and feedback welcome.

[jnbrunet]

@damienmarchal +1

I would add that those kind of functions:

core::topology::Topology* BaseContext::getTopology() const
core::topology::BaseMeshTopology* BaseContext::getMeshTopology() const
core::topology::BaseMeshTopology* BaseContext::getLocalMeshTopology() const
core::topology::BaseMeshTopology* BaseContext::getActiveMeshTopology() const

should be made depreciated as they imply that only one of such component can be contained inside a node.

[epernod]

digging into old issues and in fact this is reaching the top of my todo list before the 19.12
To sum up:

• Depreciate all getXXXMeshTopology for 19.12
• Use explicit SingleLink
• if easy will see to automatically add a SingleLink each time a getXXXMeshTopology is used for the in-between period.

thanks for your inputs.

[epernod]

This is not anymore mandatory if scene are well created. That is to say, setting link l_topology in each component relying on a topology. Thus the getMeshTopology method is not used.