Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[All] Remove SOFA_ENABLE_LEGACY_HEADERS usage #4813

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

fredroy
Copy link
Contributor

@fredroy fredroy commented Jul 5, 2024

A continuation of

but much bigger and less trivial.

This PR removes SOFA_ENABLE_LEGACY_HEADERS, implying:

  • no more collections/deprecated projects which was to ensure the continuity of Sofa.NG
  • some renaming in MultiThreading and SofaCUDA.

The deletion of the layer compat between old and new modules was acted for 23.06 so not a big deal by itself (only 1.5 year late🤷‍♂️ ).
But some olden modules still have some code/components:

  • SofaGraphComponent: with a Gravity component
  • SofaMiscCollision: with DefaultCollisionGroupManager, SolverMerger components and RayTriangleVisitor visitor
  • SleepController: with a SleepControllercomponent
  • SofaValidation: with various measuring stuff components.
    There is also SofaExporter which had some forgotten(?) examples of components which were moved. So the examples have been just moved as well

So what should be done for these components?
IMO (not done yet)

  • Gravity seems useless/not usable.
  • SleepController might maybe probably be interesting
  • DefaultCollisionGroupManager, SolverMerger were bogus and RayTriangleVisitor not used at all
  • SofaValidation may be totally transformed as a plugin (as in applications/plugins)

To be discussed.


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).
@fredroy fredroy added pr: status wip Development in the pull-request is still in progress pr: clean Cleaning the code pr: dev meeting topic PR to be discussed in sofa-dev meeting labels Jul 5, 2024
@fredroy
Copy link
Contributor Author

fredroy commented Jul 5, 2024

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

@fredroy fredroy force-pushed the remove_collections_and_misccollis branch from 7365d3d to d7398bd Compare July 5, 2024 08:06
@fredroy fredroy 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 Jul 9, 2024
@fredroy
Copy link
Contributor Author

fredroy commented Jul 9, 2024

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

@fredroy fredroy force-pushed the remove_collections_and_misccollis branch from e95ef09 to b3d6f23 Compare July 11, 2024 08:03
@fredroy fredroy force-pushed the remove_collections_and_misccollis branch from b3d6f23 to f25b6e9 Compare July 11, 2024 15:09
@hugtalbot
Copy link
Contributor

  • Gravity should be moved into MechanicalLoad
  • SleepController , @bakpaul will take a look
  • SofaMiscCollision should be fully removed
  • SofaValidation: archive
  • SofaExporter: to check
@fredroy fredroy added pr: status wip Development in the pull-request is still in progress and removed pr: dev meeting topic PR to be discussed in sofa-dev meeting pr: status to review To notify reviewers to review this pull-request labels Jul 17, 2024
@bakpaul
Copy link
Contributor

bakpaul commented Jul 18, 2024

So I took a look at the SleepController. It simply puts context to sleep where the mechanical object has a max velocity under a certain threshold. Then wake them up again only when a collision occurs with another object that is moving.
Three remarks :

  • Being put to sleep disables any mechanical visitor to be applied on the node and deactivates its constraint corrections. So no more computing (no system build, no integration, only collision detection)
  • This works well only for scenes where external interaction only arise between two objects colliding, neither by the mean of a change of external forces (dynamic vector field for instance), or of constraint state changes not managed by the collision pipeline (for instance statically defined interaction constraints such as cable constraints).
  • The code seems overly complicated for what it performs but certainly does what it says it does. I didn't try it but it looks ok.

Sincerely, I cannot see myself advising anyone to use it in their scene given the fact that the use case doesn't apply for the majority of the scenes (especially for soft robotics). So I wouldn't mind never seeing it again...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: clean Cleaning the code pr: status wip Development in the pull-request is still in progress
3 participants