[SofaKernel] implement activer's code at CollisionModel.

[remibessard]

add SphereActiver class in SphereModel so that some controller inheriting from this class can compute sphere(s) de/activation during execution

---

This PR:

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

Reviewers will merge only if all these checks are true.

[damienmarchal]

Hi remi,

Thanks for the PR.
Would it be possible to have that the top of the collision model hierarchy so that the activation filter feature is available for every model instead of Sphere ?

[remibessard]

Actually this feature is already available for PointModel and LineModel. That's why I just added it in the same way for SphereModel.
But you're right, we can try to add it at the top of collision model hierarchy.
One thing is that sometimes when a controller is heritating from this "PrimitiveActiver" we want to implement different behaviours for several type of primitive; I guess that with the generalization approach we would have to implement a controller for each behaviour (not that bad), or you can help me understand how we could avoid doing so ?

[epernod]

Hi, can you explain me in which case you need a contoller to inherite from this class to active or not the sphereModel compare to having a controller that search for the sphereModel and change the Data activate
is it to be able to activate only a set of spheres?

[remibessard]

Indeed epernod, for each sphere/line/point the LocalMinDistance component checks if it is activated or not, not the whole collisionModel component.
Unless I am mistaken this process is/was? used by some BeamAdapter controllers to de/activate a set of points/lines during the execution.
In my case I have a beam and an insertion point through which the beam is sliding. I want the collision spheres/lines on one side of this insertion point to be activated, the other ones deactivated.
If there is another way to achieve the same result, I will be glad to use it instead of using this additionnal code.

[fredroy]

instead of Datastd::string, use Link for component path and stuff

[remibessard]

Again, I applied the same pattern as it was coded for Point/Line activers.
You probably mean I should correct Sphere Line and Point activers then ? Would not it break some plugin controller code ? Do we care about it ?

[fredroy]

You are right about breaking things, but it is for the sake of cleaning 🤽‍♂ (just add breaking flag)
Personally I would bring at least this part (Link) to the other line/point activers ; especially if we suppose that the task #1262 wont be completed before a while...
Just my cent though 🦆

[damienmarchal]

To me, if a feature is duplicated in more than two component...is a strong indication that it is time to re-factor it. I may be wrong but do #1262 task so complex ?

[remibessard]

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

[fredroy]

👍

[damienmarchal]

I just gave it a look and it seems easy to move up the whole code at CollisionModel level... so please do so, code will be cleaner, shorter and will offer a more consistent interface to users.

In addition I see no problem in breaking code that does not follows Sofa guidelines. So renaming activated() into isActive() and other stuff like that would be welcome to :)